From e961c132c03b5bc856f71355aaed6d94be226186 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Thu, 24 Oct 2024 00:39:36 -0400 Subject: [PATCH 1/2] Avoid `rm -v` in fixtures for portability This removes `-v` from an `rm -R` command in `make_basic_repo.sh`, which can cause tests that needed to run that fixture to fail on systems where `rm` does not support `-v`. The script generates a fixtured used in numerous tests, and it is always run even if `GIX_TEST_IGNORE_ARCHIVES` is not set, since its output is `.gitignore`d. Because the script is so heavily used, this change allows many more tests to pass on systems where `rm` does not recognize `-v` than before. `rm -v` is actually POSIX, but it was standardized very recently, in 1003.1-2024; it was not even required in 1003.1-2017. Some Unix systems' `rm` commands do not recognize `-v`. This includes most (or all?) illumos and Solaris systems (except when `/usr/gnu/bin` is present and placed early in the `$PATH`). It seems likely that there are a number of other such systems. Regarding the history of `rm -v` in POSIX, see: - https://pubs.opengroup.org/onlinepubs/9699919799/utilities/rm.html - https://pubs.opengroup.org/onlinepubs/9799919799/utilities/rm.html - https://www.austingroupbugs.net/view.php?id=1154 - https://www.austingroupbugs.net/view.php?id=1365 - https://www.austingroupbugs.net/view.php?id=1487 Based on a `git grep -En 'rm( -[^v ;&|]*)+v'` search, this occurrence in `make_basic_repo.sh` appears to be the only place where `-v` is passed to `rm` in any gitoxide fixture script. Two nearby similar, conceptually related commands omit `-v`, and when this command (rather, the command that turned into it) was introduced in the first version of the script in a4cec4a (#399), another such command without `-v` was also present. This suggests that `-v` may have been used for exploratory purposes during development, such that the approach here of omitting `-v` it without replacing it with anything might be sufficient. However, it is possible for information shown when the fixture script runs and fails to be less informative as a result of this omission. An alternative approach to the one taken here could be to have the script carry out a tiny experiment to check if `-v` is supported, then use it only if it was found to be accepted. (Another option could be to use `find` with both `-print` and `-delete`, though that might be stronger than desired given that the affected `rm` command does not pass `-f`.) --- gix-discover/tests/fixtures/make_basic_repo.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gix-discover/tests/fixtures/make_basic_repo.sh b/gix-discover/tests/fixtures/make_basic_repo.sh index 4895aab8295..4bc41221ab5 100755 --- a/gix-discover/tests/fixtures/make_basic_repo.sh +++ b/gix-discover/tests/fixtures/make_basic_repo.sh @@ -35,7 +35,7 @@ rm -R worktrees/c-worktree-deleted (cd bare.git git worktree add ../worktrees/from-bare/c git worktree add ../worktrees/from-bare/d-private-dir-deleted - rm -R -v ./worktrees/d-private-dir-deleted + rm -R ./worktrees/d-private-dir-deleted ) git clone --bare --shared . bare-no-config.git From dffb6945508023eb87c14c79ba59bb23723193ff Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Thu, 24 Oct 2024 02:02:25 -0400 Subject: [PATCH 2/2] Avoid `cp -v` in fixtures for portability This change removes the `-v` option from the one `cp` invocation in `gix-dir/tests/fixtures/many.sh`, the one fixture script that uses it. This is roughly analogous to the preceding removal of `-v` from the one `mv` invocation that used that, and both fixture scripts are heavily used due to being used in multiple tests and due to being intentionally re-run even when `GIX_TEST_IGNORE_ARCHIVES` is not set. But while `rm -v` is standardized but only recently, `cp -v` is not standard. Even POSIX 1003.1-2024 does not require `cp` to recognize `-v`: - https://pubs.opengroup.org/onlinepubs/9799919799/utilities/cp.html See the preceding commit for general considerations, advantages and disadvantages, and alterntive approaches. As there, this currently just drops the `-v` without replacing it with anything, based on the idea that printing the list of files being operated on (here, copied) may not be important. - That assumption may be less well-founded here, since the affected command, introduced in 482d6f3 (#1285), does *not* have other nearby `cp` commands that are conceptually related but omit `-v`. - But, unlike there, it may be reasonable to consider the `-v` here to be a (minor) bug, since it is not standardized but it appears in test fixture code that seems intended to run all targets. Taken together, this and the preceding commit allow 106 formerly failing tests to pass, on the OmniOS system I used for testing. --- gix-dir/tests/fixtures/many.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gix-dir/tests/fixtures/many.sh b/gix-dir/tests/fixtures/many.sh index e5146933644..54ec1cf8dac 100755 --- a/gix-dir/tests/fixtures/many.sh +++ b/gix-dir/tests/fixtures/many.sh @@ -280,7 +280,7 @@ git init expendable-and-precious-nested-in-ignored-dir echo 'ignored/' > .gitignore git add .gitignore && git commit -m "init" mkdir -p ignored/other - cp -Rv ../expendable-and-precious ignored/d + cp -R ../expendable-and-precious ignored/d rm -Rf ignored/d/*-by-filematch ignored/d/some-* mkdir -p other/ignored && >other/ignored/a )