On Tue, 19 Aug 2025 at 16:35, Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Adam Dinwoodie <adam@xxxxxxxxxxxxx> writes: > > > The changes added by 39fc408562 (t/t1517: automate `git subcmd -h` tests > > outside a repository, 2025-08-08) to automatically loop over all "main" > > Git commands will, when run against an installed build using > > GIT_TEST_INSTALLED rather than the build in the build directory, include > > some extra git-gui commands that are installed by `make install`, or > > credential helpers that might be installed manually from the contrib > > directories. These fail the test, so record them as such. > > > > Signed-off-by: Adam Dinwoodie <adam@xxxxxxxxxxxxx> > > --- > > > > This re-roll adds a few more commands to those marked as known failures, > > notably credential helpers I see installed in various builds for the > > Nixpkgs packaging of Git. > > > > t/t1517-outside-repo.sh | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > I'd appreciate these efforts, but I am not sure if this is a losing > battle. Your ~/libexec/git-core/ directory, when GIT_TEST_INSTALLED > is in effect, likely has old commands that are retired, commands > that are added by third-parties (so that their users can say "git > frotz" and run their "frotz" software), and/or commands from the > future that the running t1517 has not seen yet (while bisecting and > running t1517 from an older commit, say). For example, I have these > differences... > > archimport.perl > citool > cvsexportcommit.perl > cvsimport.perl > cvsserver.perl > difftool--helper.sh > filter-branch.sh > gui > gui--askpass > instaweb.sh > last-modified > merge-octopus.sh > merge-one-file.sh > merge-resolve.sh > mergetool.sh > p4.py > quiltimport.sh > request-pull.sh > send-email.perl > submodule.sh > svn.perl > web--browse.sh > > ... in what t1517 $(git --list-cmds=main) sees between 'master' in > normal test mode and with GIT_TEST_INSTALLED set to ~/git/jch/bin > (i.e. the version I run for my everyday use). "last-modified" is an > example of a new-ish command that the t1517 test being run is not > yet aware of but included in GIT_TEST_INSTALLED. > > I am wondering if we are better off skipping this test, or at least > limiting to some known subset (e.g. "git --list-cmds=builtins") to > skip the files on disk when GIT_TEST_INSTALLED is in effect, instead > of "git --list-cmds=main" that is quite broad)? > > In any case, this is a strict improvement over the previous one, so > I'll replace and queue this for now, but we may want to rethink the > approach this test uses. Even without GIT_TEST_INSTALLED, the fake > GIT_EXEC_PATH we use during test has somewhat different from the > real thing, I suspect. > > Thanks. If someone's using GIT_TEST_INSTALLED, I think it's reasonable to expect them to keep their install directory fairly clean, so this test would only need to worry about things that might be there because they're included with Git. The cases I've patched for are all ones that are installed as part of the Nixpkgs Git build, which does copy in some things from contrib directories, but by design always installs into a new, empty root directory. None of which is to disagree with everything you've said. I imagine most people building Git aren't doing it using the Nixpkgs build processes, so if they're using GIT_TEST_INSTALLED, they're much more likely to be testing in an environment that includes other old scripts or tools or whatever. Having t1517 know what executables to check without requiring someone to remember to update a list is clearly valuable, but it seems that list should _somehow_ be built based on what's in the Git repository at build time, not what's in the user's environment. --list-cmds=builtins seems like it's more limited than ideal, but it might be a better approach than the current one. This is a balancing act I'll leave to people who are much more involved in the project development!
