from_path_no_follow test runs touch with nonportable date format

[EliahKagan]

Current behavior 😯

The from_path_no_follow test calls the file_metadata.sh fixture that uses touch -d with a date format that not all touch implementations support:

https://github.com/Byron/gitoxide/blob/29898e3010bd3332418c683f2ac96aff5c8e72fa/gix-index/tests/fixtures/file_metadata.sh#L4-L5

Given the ext4-related goal, this may only be significant on systems where the filesystem is usually ext4, i.e., Linux-based systems. However, because not all Linux-based systems with a touch command have the GNU coreutils implementation, this is nonetheless significant.

With BusyBox on Alpine Linux

On Alpine Linux, on which touch is provided by BusyBox, all tests except one are able to pass so long as bash is installed (apk add bash). This is as briefly noted in #1486 (comment). The one failing test is:

FAIL [   0.012s] gix-index-tests::integrate index::fs::metadata::from_path_no_follow

The details of the failure are:

        FAIL [   0.012s] gix-index-tests::integrate index::fs::metadata::from_path_no_follow

--- STDOUT:              gix-index-tests::integrate index::fs::metadata::from_path_no_follow ---

running 1 test
test index::fs::metadata::from_path_no_follow ... FAILED

failures:

failures:
    index::fs::metadata::from_path_no_follow

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 53 filtered out; finished in 0.01s


--- STDERR:              gix-index-tests::integrate index::fs::metadata::from_path_no_follow ---
thread 'index::fs::metadata::from_path_no_follow' panicked at tests/tools/src/lib.rs:566:17:
fixture script of cd "fixtures/generated-do-not-edit/file_metadata/1236651845-unix" && env -u GIT_ASKPASS -u GIT_DIR -u SSH_ASKPASS GIT_AUTHOR_DATE="2000-01-01 00:00:00 +0000" GIT_AUTHOR_EMAIL="[email protected]" GIT_AUTHOR_NAME="author" GIT_COMMITTER_DATE="2000-01-02 00:00:00 +0000" GIT_COMMITTER_EMAIL="[email protected]" GIT_COMMITTER_NAME="committer" GIT_CONFIG_COUNT="4" GIT_CONFIG_GLOBAL=":" GIT_CONFIG_KEY_0="commit.gpgsign" GIT_CONFIG_KEY_1="tag.gpgsign" GIT_CONFIG_KEY_2="init.defaultBranch" GIT_CONFIG_KEY_3="protocol.file.allow" GIT_CONFIG_SYSTEM=":" GIT_CONFIG_VALUE_0="false" GIT_CONFIG_VALUE_1="false" GIT_CONFIG_VALUE_2="main" GIT_CONFIG_VALUE_3="always" GIT_TERMINAL_PROMPT="false" MSYS=" winsymlinks:nativestrict" "/home/ek/repos/gitoxide/gix-index/tests/fixtures/file_metadata.sh" failed: stdout:
stderr: touch: invalid date '2446-05-10 22:38:55.111111111'

note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

That is on Alpine Linux 3.17. Output of the whole test run is in this gist.

Manual confirmation on Alpine Linux

Running the command manually on that system produces the same invalid date error:

Glub:~/tmp$ touch -d "2446-05-10 22:38:55.111111111" future
touch: invalid date '2446-05-10 22:38:55.111111111'

Manual confirmation with BusyBox on a GNU/Linux system

The effect can be observed, with the same error message, on a GNU/Linux system where touch is provided by GNU coreutils and supports the syntax, if BusyBox is installed, by trying the command manually with busybox. This is from an Ubuntu 22.04 LTS system:

ek@Glub:~/tmp$ touch -d "2446-05-10 22:38:55.111111111" future
ek@Glub:~/tmp$ rm future
ek@Glub:~/tmp$ busybox touch -d "2446-05-10 22:38:55.111111111" future
touch: invalid date '2446-05-10 22:38:55.111111111'
ek@Glub:~/tmp[1]$ busybox touch -d "2006-05-10 22:38:55.111111111" future
touch: invalid date '2006-05-10 22:38:55.111111111'

The last command shows that the problem is not related to the date being out of range: even when a date in 2006 is used, the same invalid date error occurs. The reason I checked that was...

Note that these systems are both called Glub because they are WSL distributions installed on the same machine, but they are separate distributions with separate filesystems (the tmp directories are separate). I did verify with mount that the filesystems are ext4, and I did also verify on a "bare metal" system that the effect also occurs.

With GNU coreutils on a 32-bit Ubuntu 18.04.6 LTS ESM system

Some GNU/Linux systems with GNU coreutils also cannot run this command, and I am not sure why. A cursory examination of the GNU coreutils commit history does not seem to reveal the reason, but that is far from sufficient to rule out the possibility that older versions of GNU coreutils don't support the syntax. My suspicion, however, is that the system being 32-bit is a contributing factor, because on this system with GNU coreutils touch, a date in the same format is accepted when it is in 2006:

ek@Kip:~$ apt list coreutils
Listing... Done
coreutils/bionic,now 8.28-1ubuntu1 i386 [installed]
ek@Kip:~$ touch -d "2446-05-10 22:38:55.111111111" future
touch: invalid date format ‘2446-05-10 22:38:55.111111111’
ek@Kip:~[1]$ touch -d "2006-05-10 22:38:55.111111111" future
ek@Kip:~$ ls -l future
-rw-rw-r-- 1 ek ek 0 May 10  2006 future

This is on a 32-bit Ubuntu 18.04 LTS system with extended security maintenance enabled (so this system is still supported, in the sense that it is not yet end-of-life, even though it has reached the end of standard support). One should expect a number of such systems to be in continued use because, while old, this is the most recent version of Ubuntu that has both (a) support from Canonical (though doing so requires enabling Ubuntu Pro which is only free-of-charge under specific limited conditions), and (b) a 32-bit x86 release.

Although gitoxide builds and seems to work fine on this system, the machine is somewhat slow, and I have not recently attempted to run tests from the test suite on it or other 32-bit x86 systems.

A red herring

This may superficially appear connected to #1488 because that is recent and they both concern dates, but it is completely unrelated. The affected code and (as far as I can tell) behavior are independent. I have also verified that the problem occurs both before and after that change.

Expected behavior 🤔

The test attempts to create a file with the latest date representable in the ext4 filesystem, and thus appears intended to be able to succeed on any Linux-based system with the expected command-line tools, so long as the filesystem is ext4. So I think the expected behavior is to successfully create the file even if the touch command does not support all the formats supported by GNU coreutils touch.

The fixture currently always runs, because it is explicitly .gitignored. But this appears intentional.

I'm not sure what the best way to fix this is, but I have a few ideas:

• Two tests set timestamps with filetime::set_file_mtime. Perhaps that or another timestamp-related function could be used instead of calling the external touch command.

• I'm not sure how portable this is, but specifying a later date in a recognized format works and sets the timestamp to the latest possible date and time even though that is earlier than what was specified:

  Glub:~/tmp$ touch -d "3000-05-10" future
  Glub:~/tmp$ stat future
    File: future
    Size: 0               Blocks: 0          IO Block: 4096   regular empty file
  Device: 820h/2080d      Inode: 57200       Links: 1
  Access: (0644/-rw-r--r--)  Uid: ( 1000/      ek)   Gid: ( 1000/      ek)
  Access: 2446-05-10 22:38:55.000000000 +0000
  Modify: 2446-05-10 22:38:55.000000000 +0000
  Change: 2024-08-07 19:25:15.019646726 +0000
  

  The above shows the command on Alpine Linux 3.17. The nanoseconds show as all zeros when this is done, but that also happens both with GNU coreutils touch and with BusyBox touch. If it is intended that the nanoseconds show a higher value, then there may be a separate bug related to that as well, which may or may not be feasible to fix with the same change that fixes this.

  In the 32-bit Ubuntu 18.04 system I tested on, the above technique did not work and gave the same error, suggesting that the error is really due to range rather than the format, on that system. Otherwise this seems to work both on GNU/Linux systems and on Alpine Linux. I'm not sure about other systems.

  Edit: I've opened #1496 to propose a variation on this, where it tries the current approach first and, if that fails, tries again with the more portable date syntax specifying a date just outside the range.

• If the date just needs to be far in the future, and we know how far it needs to be in order to effectively exercise the test--perhaps just after 2026--then a simpler, portable touch whose effect is intuitive could be used. (This might still fail on systems that don't support late enough dates, but it should overcome any format-related problems.)

Git behavior

The issue is in a test fixture in gitoxide's test suite. There may be something similar in Git's test suite, but I am not aware of it. I could research that if it turns out to be valuable. A cursory search of the repository does not show tests in which touch -d commands are run.

Steps to reproduce 🕹

Steps other than setting up software on the Alpine Linux 3.17 test system are given above.

Using apk, I installed build-base to get common development tools including a C compiler and linker, as well as the usual additional tools required for building gitoxide in its default max feature set, such as pkgconf and cmake. Like FreeBSD, Alpine Linux does not ship with bash, so that must be installed too, since the test suite requires it.

As shown in the above-linked gist, this is in the default max configuration (I did not pass any feature-related flags to cargo nextest). Alpine Linux uses musl rather than glibc, and when installing rustup, it automatically selects the correct musl-based toolchain. But to get gitoxide to build, it was necessary to overcome a problem of build failures with musl and OpenSSL that, as noted in #1486 (comment), I believe are the same as those references in the release.yml workflow:

https://github.com/Byron/gitoxide/blob/29898e3010bd3332418c683f2ac96aff5c8e72fa/.github/workflows/release.yml#L113-L115

https://github.com/Byron/gitoxide/blob/29898e3010bd3332418c683f2ac96aff5c8e72fa/.github/workflows/release.yml#L148-L150

Also as noted in #1486 (comment), the fix was to install libressl-dev--which brings in libressl3.6-libcrypto, libressl3.6-libssl, and libressl3.6-libtls as dependencies--rather than openssl and openssl-dev. This doesn't seem to actually be working (#1493), and originally (including in the original draft of this issue) I had thought the problem was specific to using the binary on other systems but actually it happens on the system where it is built. However, that shouldn't be a problem for this issue or the procedure for reproducing it, because the problem here is not related to that, is in a shell script rather than binary code, and is not specific to particular build configurations.

[Byron]

Thanks for the incredibly detailed collection.

Having merged this PR first probably allows me to give a brief summary of how I thought we could act in the face of touch portability issues.

The PR fixes the issue by essentially falling back to a portable date format that still satisfies the test. In theory, there is no need for the nanosecond version in the first place, but I though it was useful to leave it as a warning for the future.

Setting mtimes in Rust might be more portable, and should probably be preferred, especially if sub-second precision is desirable.

Lastly, tests should probably not assume subsecond precision and be able to deal with filesystems that cannot support it (even if this isn't a matter discussed in this issue).

[EliahKagan]

Is this to say that there is no need to attempt to test the very edge of the range even when it is possible to do so, but just within a second of it?

I'll look into setting mtimes in Rust, which is already being done for some other tests:

https://github.com/Byron/gitoxide/blob/d51f330e9d364c6f7b068116b59bf5c0160e47af/gix-status/tests/status/index_as_worktree.rs#L561-L566

https://github.com/Byron/gitoxide/blob/d51f330e9d364c6f7b068116b59bf5c0160e47af/gix-status/tests/status/index_as_worktree.rs#L597-L599

As I noted in #1496:

This fixture's previous code did not seem to account for or explain the intended behavior with respect to time zone, and I have not attempted to address that here either.

To the best of my knowledge, the time zone is the only way touch -d uses locale-related, at least so long as the timestamp format is unambiguous (whether portable or not), though I have not researched this.

I think more substantial changes than those in #1496, including changing to setting the timestamp in Rust, should probably be done in a way that clearly accounts for issues of time zone and, if applicable, other locale issues.

If time zone is the only way locale-related information impacts this, then I am not sure touch -d should off the table for that reason, even in the test that currently avoids it, because I suspect that setting TZ=UTC might be sufficient.

If sub-second precision doesn't have to be attempted, then I can either not bother with it when writing Rust code to set the timestamp, or if for some reason this shouldn't be done in Rust, I can remove the non-portable commands. This could be done at the same time as accounting for the time zone, if applicable.

I wonder if the issue of whether the system clock itself is set to local time or to UTC is at all relevant to how the meaning of timestamps in the filesystem is interpreted. (When running a Unix-like operating system it is traditionally set to UTC, but it is sometimes set to local time instead, such as to solve clock skew problems when dual-booting with Windows, which defaults to assuming the system clock is set to local time.)

[Byron]

If time zone is the only way locale-related information impacts this, then I am not sure touch -d should off the table for that reason, even in the test that currently avoids it, because I suspect that setting TZ=UTC might be sufficient.

Indeed, touch can most certainly be taught to be timezone-independent. It should also be possible to do so with the format string, maybe Z suffixes do that (as vague remembrance).

I totally admit though that I am a bit outside my comfort zone here as I don't remember what the tests truly need - index tests probably rely on sub-second precision sometimes to test for edge-cases in dirty-detection, and there is probably much I am missing or have forgotten.

It seems you have a plan on the improvements to make, and I encourage you to go ahead with them choosing whichever means of setting mtimes that seem most appropriate.

Sorry for being so vague here, I hope that when seeing a PR it will all come together for me.