Fix handling of negative timestamps in Stat.

[sunfishcode]

Deprecate Stat::st_mtime etc., because they're unsigned and should be signed, and introduce a StatExt extension trait which adds mtime() etc. accessor functions which return the values in the appropriate signed type.

This StatExt trait can go away next time we have a semver breaking change, but for now this preserves compatibility.

[joshtriplett]

Why a StatExt extension trait rather than implementing these methods directly on the structure?

[sunfishcode]

Depending on the target and configuration, Stat is sometimes just a type alias for libc::stat or linux_raw_sys::general::stat, and sometimes a custom type. When it's a type alias, we can't define methods on it.

This is one of the things in progress for the next major version of rustix, to move to encapsulate all the libc/linux-raw-sys types, as having them in the public API causes a variety of problems. But that requires a breaking change too, so for now, an extension trait seems the best we can do.

[joshtriplett]

@sunfishcode Ah, I didn't realize it was ever directly exposing the underlying type. Fair enough.

This looks like a sensible interim fix, then.

[sunfishcode]

This is now released in rustix 0.38.31.

[Byron]

Thanks again for tackling this issue!

I was trying to use these improvements but had trouble doing so. The code looks like this (reproduced here for convenience)

            #[cfg(not(target_os = "aix"))]
            let seconds = self.0.st_mtime;
            #[cfg(target_os = "aix")]
            let seconds = self.0.st_mtim.tv_sec;

            // The fix I'd like to live without
            let seconds = seconds as i64;
            system_time_from_secs_nanos(seconds, nanoseconds.try_into().ok()?)

self.0 is of type rustix::fs::Stat. However, the trait implementation of rustix::fs::StatExt doesn't seem to provide an implementation for aix.

The solution seems to be to add such an implementation, but maybe there was a reason to not implement it in the first place.

In any way, without StatExt being implemented for all platforms I am probably better off leaving the gitoxide code as is, but with guidance I should be able to contribute what might be missing.

This also gave me the idea to use StatExt to taper over other code I had to write, but would rather not:

            #[cfg(not(any(target_os = "netbsd", target_os = "aix")))]
            let nanoseconds = self.0.st_mtime_nsec;
            #[cfg(target_os = "netbsd")]
            let nanoseconds = self.0.st_mtimensec;
            #[cfg(target_os = "aix")]
            let nanoseconds = self.0.st_mtim.tv_nsec;

Thanks for letting me know how to proceed here.

[sthibaul]

In https://github.com/bytecodealliance/rustix/pull/1064/files I added the implementation