add --worktree option to install and uninstall commands #4159
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chrisd8088
force-pushed
the
chrisd-install-worktree
branch
2 times, most recently
from
97b6203 to
4de76d0
Compare
chrisd8088
force-pushed
the
chrisd-install-worktree
branch
from
4de76d0 to
5704c66
Compare
We do some minor housekeeping to correct and clarify comments which didn't match their find/set/unset functions, and we move one function so we have a consistent ordering within each type.
chrisd8088
force-pushed
the
chrisd-install-worktree
branch
from
5704c66 to
c7c9f69
Compare
When uninstalling, if any "git config --remove-section" commands failed, we would previously ignore the error and proceed without reporting it. We now at least report the errors in a warning message printed to standard output, akin to the handling introduced to the "git lfs install" command in PR #2673. However, we likely can not alter the uninstall behaviour further and actually exit with an error code in these cases (skipping any attemp to uninstall our hooks) because users may depend on the existing willingness of "git lfs uninstall" to try to uninstall as much as possible, and may have scripts which depend on this behaviour. Thus we don't call os.Exit(2) in the manner introduced to "git lfs install" in PR #3624. On the one hand, this change means users who run an uninstall command when no relevant Git LFS filter configurations exist will see a "WARNING" message which may look somewhat scary. On the other hand, we already report other errors such as "no such file or directory" when hooks do not exist, and by reporting these git-config(1) errors we will be able to better support new options such as --worktree, where git-config may report error conditions such as when the worktreeConfig extension is not properly defined yet. So on balance, we aim for more rather than less error reporting. And separately, although the usage text for all command options is unused (because we override Cobra's usage function in run.go to always show the man page text instead), we still correct it here before we add any new options.
We add a --worktree option to "git lfs install" and "git lfs uninstall" and pass it through to "git config", but only when the Git version is at least 2.20.0, as prior Git versions do not support the --worktree option.
Add help text to document the --worktree option we've added to the "git lfs install" and "git lfs uninstall" commands, along with a reference to git-worktree(1) as it further explains the use of the extensions.worktreeConfig setting.
The test for installing with --local outside a repository should fail if the content of the captured error log doesn't match the expected error string. In practice, the captured error log was always empty because the requireInRepo() function prints to stdout, not stderr, and the test was capturing stderr. The test did not fail, however, because the "set +e" setting which allows the test to proceed past the expected failure of the "git lfs install --local" command continued to have effect such that the failure of the subsequent string comparison was ignored and the test passed. So in addition to capturing stdout rather than stderr, we also restrict the effect of the "set +e" setting to just the expected failure of the "git lfs install" command. For clarity, we rename the captured log file to out.log and do the same in a prior test which also captures stdout as well. We also clean up some whitespace, and remove some unneeded global configs from the test for failed permissions when installing with --local.
As the checks for local and global LFS configurations in our install and uninstall tests of the --local option vary somewhat in their breadth and their use of dummy text strings, we attempt to make them more consistent and complete before adding any tests for the new --worktree option. To bring additional consistency we add an uninstall test which confirms that using --local outside a repository fails, akin to the existing installation test of that case. And we also add matching install and uninstall tests to confirm the expected error message when conflicting --local and --system scope options are both supplied.
We add a number of tests, conditional on the Git version, to validate the functionality of the --worktree option when installing or uninstalling. Note that because this simply passed through to the "git config" command, we inherit the behaviour that --worktree functions like --local when only a single, default working tree is in use. And if we have a Git version older than 2.20.0, we confirm that the --worktree option is not supported.
chrisd8088
force-pushed
the
chrisd-install-worktree
branch
from
c7c9f69 to
f930c25
Compare
chrisd8088
changed the title
chrisd8088
added a commit
to chrisd8088/git-lfs
that referenced
this pull request
Nov 26, 2025
In commit 68da211 of PR git-lfs#2976 we revised the "git lfs uninstall" command so that when the --local option was specified, the command did not incorrectly output a message stating that the global Git LFS configuration settings had been removed. At the same time, we also updated the "uninstall --local" test which was then the precursor of our current t/t-uninstall.sh test script so that it tried to check that no message regarding the global configuration settings appeared in the output of the "git lfs uninstall" command when it was run with the --local option. This check uses the grep(1) command and its -v option, with the intention that the command should fail if the message text appears in its input. However, the -v option of the "grep" command does not cause the command to fail (i.e., exit with a non-zero value) if a line is found in its input which matches the provided regular expression. Rather, the -v option causes the "grep" command to filter out any lines from its input which match the expression, and then the exit status is determined in the usual manner, so that the command only returns a non-zero value if no other lines were seen in the input. In our "uninstall --local" test, we do not remove the global Git LFS configuration established for each test in its unique home directory by the setup() function of our t/testhelpers.sh shell library. As a result, when the test runs the "git lfs uninstall --local" command, this global Git LFS configuration is not changed, and so the command outputs a warning message that "some filter configuration was not removed". This message therefore appears in the log file in which the test captures the output of the "git lfs uninstall" command. The test then runs the "grep" command with the -v option to try to verify that the log file does not contain any message regarding the removal of the global Git LFS configuration settings, which it does not. If the log file was empty, the "grep" command would find no matching lines, return a non-zero status code, and the test would fail. However, as described above, the log file contains a different message, so the "grep" command finds a line which does not match the given pattern, returns a zero status code, and the test passes. This does not demonstrate what we intend, though, since the log file could also contain unexpected lines matching the pattern and they would simply be filtered by the "grep" command due to the use of the -v option. To resolve this problem, we revise the test so that it uses the -c option of the "grep" command instead of the -v option, and then checks that the number of lines the command finds which match the given pattern is zero. As described in a prior commit in this PR, we already use this idiom throughout many of our other test scripts, in part because it has several advantages over other possible techniques for ensuring that a file contains no lines matching a certain pattern. In particular, if the "grep" command were to fail with an error, it would not output an integer count value, which would cause the test to fail. We also revise a number of other tests which exhibit the same problem as the "uninstall --local" test in their use of the "grep" command's -v option. We have added these tests over time and have repeatedly copied the "uninstall --local" test's use of the "grep" command verbatim on the incorrect assumption that it worked as was originally intended. In commit f930c25 of PR git-lfs#4159 we added the t/t-uninstall-worktree.sh test script, along with several other test scripts, to validate that the "git lfs install" and "git lfs uninstall" commands now supported the new --worktree option we introduced in that PR. Two of the tests in the t/t-uninstall-worktree.sh script, the "uninstall --worktree with single working tree" and "uninstall --worktree with multiple working trees" tests, both replicate the use of the "grep" command with the -v option to try to demonstrate that the "git lfs uninstall" command with the --worktree option does not output a message regarding the removal of the global Git LFS configuration settings. In both of these tests we therefore now make the same change we made to the "uninstall --local" test by replacing the "grep" command's -v option with the -c option and then checking that the number of lines the command finds which match the given pattern is zero. In commit 84ca7e7 of PR git-lfs#5355 we updated the "git lfs install" and "git lfs uninstall" commands to support a new --file option that allows a user to specify the Git configuration file the commands should modify. The original purpose for which this option was added was to allow users to modify their alternative global Git configuration file, if they have one in the location given by the path "$XDG_CONFIG_HOME/git/config", since by default our commands only modify the global configuration file with the path "$HOME/.gitconfig". For this reason, when the "git lfs uninstall" command's file option is used, the command does output a message stating that the global Git LFS configuration settings have been removed. (Users are, of course, free to specify any file path with the --file option and so this message may not always be appropriate. While this may be an issue we should address, we defer any changes to the "git lfs uninstall" command to a future PR in order to constrain the scope of this PR to our test suite only.) When we introduced the --file option in PR git-lfs#5355, we also added one new test, the "uninstall --file" test in our t/t-uninstall.sh script. As with the tests we added in PR git-lfs#4159 when we introduced the --worktree option, the "uninstall --file" test also uses the "grep" command with its -v option to try to verify that the "git lfs uninstall" command with the --file option does not output a message regarding the removal of the global Git LFS configuration settings. However, as noted above, the "git lfs uninstall" command does output such a message when the --file option is provided. It also outputs the warning message seen in other tests, because the command notices that the Git LFS configuration settings in the "$HOME/.gitconfig" file have not been removed. For all the reasons we have described, this means the "grep" command succeeds and the test passes, because there is a line in the log file which does not match the pattern provided with the -v option. To resolve this particular problem seen in the "uninstall --file" test we simply remove the -v option, since at least for now we actually do expect the command to output a message stating that the global Git LFS configuration settings have been removed. We also add a comment to explain why this is the case, even though users may provide any file path with the --file option. (In the future, should we choose to revise the behaviour of the "git lfs uninstall" command's --file option and the messages it outputs, this test will now serve as a more accurate reflection of the command's present behaviour.) We also take the opportunity to revise the initial "uninstall outside repository" test in the t/t-uninstall.sh test script so that when it uses the "grep" command to check that the "git lfs uninstall" command has output a message stating that the global Git LFS configuration settings have been removed, the test passes the whole message as the "grep" command's pattern. This aligns the test's operation with that of the other tests which check for the presence or absence of the same message. Finally, note that all of the regular expressions used in the "grep" commands we modify in this commit do not properly escape the final "." character in the string they provide as a pattern, so the pattern will technically match any character and not just a literal "." character, as is intended. We could resolve this problem for the regular expressions of the specific "grep" commands we are modifying in this commit, either by adding the -F option to the "grep" commands or by escaping the "." character. However, since this issue affects multiple other patterns as well (including some in other test scripts), we would prefer to address this issue in a more comprehensive fashion. We therefore defer to a future PR any revisions of the patterns used by any "grep" commands in our test suite.
chrisd8088
added a commit
to chrisd8088/git-lfs
that referenced
this pull request
Nov 26, 2025
In commit 68da211 of PR git-lfs#2976 we revised the "git lfs uninstall" command so that when the --local option was specified, the command did not incorrectly output a message stating that the global Git LFS configuration settings had been removed. At the same time, we also updated the "uninstall --local" test which was then the precursor of our current t/t-uninstall.sh test script so that it tried to check that no message regarding the global configuration settings appeared in the output of the "git lfs uninstall" command when it was run with the --local option. This check uses the grep(1) command and its -v option, with the intention that the command should fail if the message text appears in its input. However, the -v option of the "grep" command does not cause the command to fail (i.e., exit with a non-zero value) if a line is found in its input which matches the provided regular expression. Rather, the -v option causes the "grep" command to filter out any lines from its input which match the expression, and then the exit status is determined in the usual manner, so that the command only returns a non-zero value if no other lines were seen in the input. In our "uninstall --local" test, we do not remove the global Git LFS configuration established for each test in its unique home directory by the setup() function of our t/testhelpers.sh shell library. As a result, when the test runs the "git lfs uninstall --local" command, this global Git LFS configuration is not changed, and so the command outputs a warning message that "some filter configuration was not removed". This message therefore appears in the log file in which the test captures the output of the "git lfs uninstall" command. The test then runs the "grep" command with the -v option to try to verify that the log file does not contain any message regarding the removal of the global Git LFS configuration settings, which it does not. If the log file was empty, the "grep" command would find no matching lines, return a non-zero status code, and the test would fail. However, as described above, the log file contains a different message, so the "grep" command finds a line which does not match the given pattern, returns a zero status code, and the test passes. This does not demonstrate what we intend, though, since the log file could also contain unexpected lines matching the pattern and they would simply be filtered by the "grep" command due to the use of the -v option. To resolve this problem, we revise the test so that it uses the -c option of the "grep" command instead of the -v option, and then checks that the number of lines the command finds which match the given pattern is zero. As described in a prior commit in this PR, we already use this idiom throughout many of our other test scripts, in part because it has several advantages over other possible techniques for ensuring that a file contains no lines matching a certain pattern. In particular, if the "grep" command were to fail with an error, it would not output an integer count value, which would cause the test to fail. We also revise a number of other tests which exhibit the same problem as the "uninstall --local" test in their use of the "grep" command's -v option. We have added these tests over time and have repeatedly copied the "uninstall --local" test's use of the "grep" command verbatim on the incorrect assumption that it worked as was originally intended. In commit f930c25 of PR git-lfs#4159 we added the t/t-uninstall-worktree.sh test script, along with several other test scripts, to validate that the "git lfs install" and "git lfs uninstall" commands now supported the new --worktree option we introduced in that PR. Two of the tests in the t/t-uninstall-worktree.sh script, the "uninstall --worktree with single working tree" and "uninstall --worktree with multiple working trees" tests, both replicate the use of the "grep" command with the -v option to try to demonstrate that the "git lfs uninstall" command with the --worktree option does not output a message regarding the removal of the global Git LFS configuration settings. In both of these tests we therefore now make the same change we made to the "uninstall --local" test by replacing the "grep" command's -v option with the -c option and then checking that the number of lines the command finds which match the given pattern is zero. In commit 84ca7e7 of PR git-lfs#5355 we updated the "git lfs install" and "git lfs uninstall" commands to support a new --file option that allows a user to specify the Git configuration file the commands should modify. The original purpose for which this option was added was to allow users to modify their alternative global Git configuration file, if they have one in the location given by the path "$XDG_CONFIG_HOME/git/config", since by default our commands only modify the global configuration file with the path "$HOME/.gitconfig". For this reason, when the "git lfs uninstall" command's file option is used, the command does output a message stating that the global Git LFS configuration settings have been removed. (Users are, of course, free to specify any file path with the --file option and so this message may not always be appropriate. While this may be an issue we should address, we defer any changes to the "git lfs uninstall" command to a future PR in order to constrain the scope of this PR to our test suite only.) When we introduced the --file option in PR git-lfs#5355, we also added one new test, the "uninstall --file" test in our t/t-uninstall.sh script. As with the tests we added in PR git-lfs#4159 when we introduced the --worktree option, the "uninstall --file" test also uses the "grep" command with its -v option to try to verify that the "git lfs uninstall" command with the --file option does not output a message regarding the removal of the global Git LFS configuration settings. However, as noted above, the "git lfs uninstall" command does output such a message when the --file option is provided. It also outputs the warning message seen in other tests, because the command notices that the Git LFS configuration settings in the "$HOME/.gitconfig" file have not been removed. For all the reasons we have described, this means the "grep" command succeeds and the test passes, because there is a line in the log file which does not match the pattern provided with the -v option. To resolve this particular problem seen in the "uninstall --file" test we simply remove the -v option, since at least for now we actually do expect the command to output a message stating that the global Git LFS configuration settings have been removed. We also add a comment to explain why this is the case, even though users may provide any file path with the --file option. (In the future, should we choose to revise the behaviour of the "git lfs uninstall" command's --file option and the messages it outputs, this test will now serve as a more accurate reflection of the command's present behaviour.) We also take the opportunity to revise the initial "uninstall outside repository" test in the t/t-uninstall.sh test script so that when it uses the "grep" command to check that the "git lfs uninstall" command has output a message stating that the global Git LFS configuration settings have been removed, the test passes the whole message as the "grep" command's pattern. This aligns the test's operation with that of the other tests which check for the presence or absence of the same message. Finally, note that all of the regular expressions used in the "grep" commands we modify in this commit do not properly escape the final "." character in the string they provide as a pattern, so the pattern will technically match any character and not just a literal "." character, as is intended. We could resolve this problem for the regular expressions of the specific "grep" commands we are modifying in this commit, either by adding the -F option to the "grep" commands or by escaping the "." character. However, since this issue affects multiple other patterns as well (including some in other test scripts), we would prefer to address this issue in a more comprehensive fashion. We therefore defer to a future PR any revisions of the patterns used by any "grep" commands in our test suite.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We add support for the
--worktreeoption in the Git LFS installation and uninstallation commands, when it is supported bygit-config(1)(i.e., with Git version 2.20.0 and higher).Note that when used in a single working tree environment,
--worktreehas the same effect as--local; this is due to the waygit-config(1)handles the option.We add tests for a variety of conditions such as repositories with single and multiple working trees, and with and without the
extensions.worktreeConfigGit configuration setting. We also expand and align some of our existing installation and uninstallation tests to make them more complete and consistent with each other.Of note, we also change the behaviour of the
git lfs uninstallcommand so that it reports errors from its invocations ofgit config --remove-sectioninstead of silencing them. This brings it more into line with the changes to thegit lfs installcommand in #2673. However, we don't exit early or with an error code in such conditions, per #3624, because we don't want to cause problems any existing users or scripts who depend on being able to repeatedly callgit lfs uninstalleven if it performs no actions.We make this change to the
git lfs uninstallbehaviour primarily so thatgit-config(1)cautions about missingworktreeConfigextension settings are not silenced, but also with the general principle of not hiding errors in mind.Also of note, we revise the
"install --local outside repository"test int/t-install.shbecause its use ofset +emasked the fact that it was capturing the wrong output (stderr instead of stdout) and therefore didn't actually validate what it seemed to test. So in addition to now capturing stdout rather than stderr, we also restrict the effect of theset +esetting to just the expected failure of thegit lfs installcommand. We can then use this test template in other existing and new tests where we expectgit lfs installorgit lfs uninstallto fail, including with our new--worktreeoption.Finally, we also make some other minor textual changes to clean up and reorganize configuration-related tests, code blocks, and command-line option descriptions (even though the latter are unused).
This PR should be easiest to review commit-by-commit, but should also not be too hard to read as a single set of changes.
Fixes #4152.
/cc @nagisa as reporter.