Skip to content

Conversation

@technoweenie
Copy link
Contributor

Fixes #2670. This PR fixes two common, but not fatal, problems that can arise when running git lfs install:

  1. The values from git lfs install --skip-smudge can not be upgraded with git lfs install
  2. An unexpected config value should not be overwritten. Instead, print a warning without changing the exit code. This way installations don't fail, but a user is told if their config values are unexpected.

@technoweenie technoweenie merged commit adbf666 into master Oct 17, 2017
@technoweenie technoweenie deleted the install-tweaks branch October 17, 2017 21:33
@git-lfs git-lfs deleted a comment from angkanasoisom Feb 1, 2018
chrisd8088 added a commit that referenced this pull request Jun 26, 2020
When uninstalling, if any "git config --remove-section" commands
failed, we would previously ignore the error and proceed.

We now deliver any errors in a manner equivalent to the
"git lfs install" command, which prints them to standard output
as warning text, and then exits without proceeding to try to
install any hooks.  In the uninstall case, we now act the same
way in the face of errors, stopping before uninstalling any hooks.

This brings us closer in line with the error handling of the
install command, as it has evolved through PRs #3624 and #2673.

Also, 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.
chrisd8088 added a commit that referenced this pull request Jun 26, 2020
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.
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.

Unable to clean update on Ubuntu with --skip-smudge enabled

2 participants