Add --system config commands, elevate install config to system scope when root/packaged

[javabrett]

These changes modify the packaged install experience, pulling-up config changes currently made in the current user "global" config (~/.gitconfig), to instead be made at the "system" config (/etc/gitconfig). That way, Git LFS is installed once per-system, and does not require additional configuration should a different user start using Git LFS and therefore require the smudge/clean filters.

Note that these changes presume that it is safe to have the Git LFS filters installed and that they do and will continue to run as fast noops in non-Git LFS repos.

The elevation from global-to-system occurs automatically provided:

1. The installation runs as root
2. --local is not specified.

Additionally, post-install and pre-remove scripts are added to deb and rpm packages. Since these run as root, they automatically elevate and install to /etc/gitconfig.

I don't know how this plays-out on Windows.

[technoweenie]

I guess I was envisioning --system and --global, and hadn't really thought of it detecting root access. Should the output of the command change to match?

$ git lfs install
Git LFS initialized to system config.

@andyneff Do the debian and rpm changes look good?

@shiftkey: Does Git for Windows run as an admin? May be a moot point if the maintainer wants to keep hard coding the config values.

@strich: Would the Windows Installer need any changes to take advantage? We'd have to confirm that it can detect windows admin access.

[strich]

Would the Windows Installer need any changes to take advantage? We'd have to confirm that it can detect windows admin access.

The Windows installer already requests admin elevation, so that'll be fine.

I could optionally add an install flag to install using --local I suppose.

[andyneff]

RPM 👍

Don't know about deb, @ssgelm ?

FWIW, I would prefer a --system or --global instead of ever detecting root and acting differently, just to match how other git operations normally work.

[ssgelm]

I think the deb changes are fine.

[ssgelm]

Also +1 to having a --system flag instead of magic.

[technoweenie]

Considering this is an install command, I think assuming root makes sense. I wouldn't expect users to read the docs to decide if they want git lfs install --global or git lfs install --system. That said, I agree with @strich's concerns about compatibility across all our platforms (expressed in chat).

What if we did both? By default, it installs to the system config, or global config if you don't have access. Then add --global and --system options to force those without checking user access (in addition to the existing --local flag).

[javabrett]

Is there an issue, or a wiki, tracking the roadmap-detail for installation-experience? There's been a number of suggestions around user-experiences around the installation workflow and timing of the installation and configuration of LFS, especially as it relates to when they cloned a repo with LFS-pointers etc. A reasonable number of user-issues relate to install/timing and install-config scope etc. e.g. #1167 .

Some thoughts:

• Expect that most users on Linux will install via a RPM or DEB package, and expect that wherever possible a) this completes the installation process, no-need to run git lfs install to obtain user-default or system-level installation/configuration/activation and b) whatever config is done is done equally for all users.
• There might be a form-factor issue here with git lfs install, because correct me if I am wrong, but it needs to install the filters per this issue, which can be performed at various scopes, but it also needs to install the pre-push hook, which can only be done within the scope of a local repo/working-copy. This raises for me the question - should there be an install-time pre-config step at all? Is it possible to simply detect a) a repo with Git LFS artifacts in it and b) the first Git LFS command run within a repo, and set local configuration accordingly? That might be too difficult - are there any root-level marker files? Is it possible to simply catch everything just-in-time and create the appropriate LFS filters and hooks? The main impact of course is the clone-workflow, since you can't set the filters locally before fetching the clone, unless you inspect it remotely. But it might be reasonable for that workflow to require a second command to then resolve the binaries?
• This change seems necessary as things stand now, since "global" Git config is in-fact user-config, and this doesn't look right for system-wide RPM or DEB installation, and having to run an additional command post-install to "install" is confusing. But if the above held, and we were able to perform just-in-time configurations, preferably at local/clone scope only, perhaps requiring some different user-workflows for initial clones etc., then this change makes no sense. In other words, if we could avoid static configuration outside of a working-copy, that seems preferable. And if at least some common-enough end-user workflows would require a git lfs install --local in the repo before they can work properly, then there's still a confusing issue present. And we might stop using the word "install" for repo-level operations, if it confuses with binary/runtime-level operations. "activate"? If auto-activation can't be achieved.
• Perhaps the system-level filter that gets installed could be a meta-filter, that is capable of detecting LFS clones and installing all required configuration locally-only? Very much thinking out-loud here ...

[shiftkey]

@technoweenie

Does Git for Windows run as an admin? May be a moot point if the maintainer wants to keep hard coding the config values.

The installer does, but there is also a PortableGit version that doesn't require elevation - it just unpacks itself to a directory and does the bare essentials to give you a Git environment...

With respect to the hard-coded configuration values - we can switch that out to use a git lfs ... command in a future update but that will run at packaging time, not when the end user runs the installer. We just need to ensure we handle install/update/uninstall correctly and everything should be ✨...

[technoweenie]

@javabrett Great points!

Expect that most users on Linux will install via a RPM or DEB package, and expect that wherever possible a) this completes the installation process

With my suggestion here, they can just run git lfs install --system and git lfs uninstall --system.

should there be an install-time pre-config step at all? Is it possible to simply detect a) a repo with Git LFS artifacts in it and b) the first Git LFS command run within a repo, and set local configuration accordingly?

It is possible. But I wouldn't want it to setup the filters in the local git config. The problem here is that git lfs uninstall can't work without scanning user data for LFS repositories. The nice thing with the current system is that the only per-repo thing people need to do is delete the .pre-push hook. It tells you how to do that when it fails due to git-lfs not existing.

Also, without any LFS filters setup, it's too easy to add and push files that should be in LFS if it's not installed.

[javabrett]

One side-note - I looked at:

// InstallOptions serves as an argument to Install().
type InstallOptions struct {
        Force bool
        Local bool
}

... noticing the bool for Local, false implying Global and that we now need a third System, and then looked at how go implements an enum (something about stringifying something, go:generate source-code generation etc.), which then seemed beyond my go-chops. Is that what would be required, promoting this boolean to an enum?

[javabrett]

Note to self more than anything - part of the issue with enforcement of LFS processes, or at least issuing warnings when something is missing, is that there is no option to have Git itself issue a warning or failure if a filter is missing or not-configured. Git treats this condition as a no-error noop.

[javabrett]

Added another commit to:

• add the --system option, making in mutually-exclusive to --local
• remove the euid detection and auto-promotion to system-config
• replace it with --system in the package post-install commands.

You can test the new switch with sudo git lfs install --system.

While testing this I ended up shaving yaks trying to work-out what on earth goes on with error-swallowing in subprocess/SimpleExec, when you get say a permissions-error trying to SetSystem as non-root. Those methods are swallowing errors, but that's not the only problem, there's something in SimpleExec too. Perhaps many git commands return non-zero status and tests need to ignore that?

I decided not to let it hold-up review of the additional commit. You will notice that if you run without sudo, so git lfs install --system as a regular user, LFS will report success, but there will of course be no change to /etc/gitconfig due to not being root. I compensated by leaving one euid check in now to print a warning about likely failure if you are not root.

I'll ask about the error-swallowing separately and link to here.

[javabrett]

Fixed the SimpleExec problem and stopped throwing-away Config Set errors (Config Unset needs to be ignored in terms of git config --unset return code, since it returns 5 if missing), so that failures are now nicely reported e.g.:

Error running git [config --system filter.lfs.clean git-lfs clean -- %f]: '' 'error: could not lock config file /etc/gitconfig: Permission denied' 'exit status 255'

[javabrett]

Looks like I've unintentionally used something new in Go 1.6 API - ExitError Stderr.

[javabrett]

Fixed so it compiles with Go 1.5 by pulling-down a couple of utility methods from Go 1.6 (in a single commit so it can be easily reverted to a Go 1.6 version). Removed the nasty bool for optional swallow-error in SimpleExec - it now properly returns an error but ensures that the Output string return is "" in such cases. I suspect this wrapper was/is compensating for commands which failed whilst writing guff to stdout instead of stderr.

[javabrett]

Global currently remains the default scope (and there's no explicit switch for it yet) for install.

Will it matter that having run a --system level install (either explicitly or as part of a package post-inst script), later install commands at either default global or --local scope will duplicate the filters at that lower scope? It appears that is what git config does. So you could end-up with filters installed in all of system/global/local scope.

Matters because I expect an in-repo install is still required at least once to install the pre-push hook. Is anything being worked-on to either auto-install that, or produce warnings when it is missing etc.? Should we be splitting install into components, since filters will now hopefully be commonly auto-installed at system-level, but a manual repo-level command remains. Or should filter install manually check higher scopes to see if it is even required?

[technoweenie]

Doh, I don't know why this was left out of v1.3. Sorry for dropping this on the floor.

Matters because I expect an in-repo install is still required at least once to install the pre-push hook.

Hooks are installed in the following commands: clean, fsck, push, smudge, track, and untrack. It can be added to more commands with lfs.InstallHooks(false).

There's also a git lfs update command that installs hooks. Ideally, people don't have to run git lfs install on every repo. I think checking for existing filter values before setting them makes sense too.

[technoweenie]

I fixed a merge conflict so we can ship this in v1.4: #1460. I couldn't think of a suitable shortcut for --system (since --skip-smudge is using -s), so I'm keeping it blank.

[technoweenie]

@javabrett Thanks again for tackling this 🤘