Skip to content

Conversation

@bk2204
Copy link
Member

@bk2204 bk2204 commented May 1, 2023

There are actually two possible global configuration files, $HOME/.gitconfig and $XDG_CONFIG_HOME/git/config. However, the --global file by default modifies only the former if it's present. Add a --file option to git lfs install and git lfs uninstall so that users can choose which of those files they'd like to use.

Fixes #4947

There are actually two possible global configuration files,
`$HOME/.gitconfig` and `$XDG_CONFIG_HOME/git/config`.  However, the
`--global` file by default modifies only the former if it's present.
Add a `--file` option to `git lfs install` and `git lfs uninstall` so
that users can choose which of those files they'd like to use.

Update several tests to deal with our new, simpler error message so we
can avoid having to produce a huge number of different error messages.
@bk2204 bk2204 marked this pull request as ready for review May 2, 2023 19:08
@bk2204 bk2204 requested a review from a team as a code owner May 2, 2023 19:08
Copy link
Member

@chrisd8088 chrisd8088 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks!

@bk2204 bk2204 merged commit 8e96b5d into git-lfs:main May 3, 2023
@bk2204 bk2204 deleted the install-file branch May 3, 2023 12:39
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request May 15, 2025
The "git lfs install" and "git lfs uninstall" commands both accept
a --file option, which was introduced in commit
84ca7e7 of PR git-lfs#5355, which cause
the commands to add or remove Git LFS filter settings from a specified
Git configuration file rather than from any of the system, global,
local, or current working tree configuration files.

However, this option is not documented in either of our
git-lfs-install(1) or git-lfs-uninstall(1) manual pages, so we now
add entries for it into the lists of options supported by those commands.
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.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Nov 27, 2025
The "git lfs install" and "git lfs uninstall" commands both accept
a --file option, which was introduced in commit
84ca7e7 of PR git-lfs#5355, which cause the
commands to add or remove Git LFS filter settings from a specified Git
configuration file rather than from any of the system, default global
(i.e., "$HOME/.gitconfig"), local, or current working tree configuration
files.  This option is intended in part to assist users with an
alternative global configuration file located at the path given by
"$XDG_CONFIG_HOME/git/config".

However, this option is not documented in either of our
git-lfs-install(1) or git-lfs-uninstall(1) manual pages, so we now
add entries for it into the lists of options supported by those commands.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Nov 27, 2025
The "git lfs install" and "git lfs uninstall" commands both accept
a --file option, which was introduced in commit
84ca7e7 of PR git-lfs#5355, which cause the
commands to add or remove Git LFS filter settings from a specified Git
configuration file rather than from any of the system, default global
(i.e., "$HOME/.gitconfig"), local, or current working tree configuration
files.  This option is intended in part to assist users with an
alternative global configuration file located at the path given by
"$XDG_CONFIG_HOME/git/config".

However, this option is not documented in either of our
git-lfs-install(1) or git-lfs-uninstall(1) manual pages, so we now
add entries for it into the lists of options supported by those commands.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Proposal: add a --file flag to git lfs install in order to write the filter configuration to an arbitrary file

2 participants