remove unnecessary 'git credential approve' calls

[technoweenie]

It is still true, though, that git credential approve is executed in a new process for each transferred file (on checkout), correct? Note that it is not re-prompting for credentials--it is getting cached credentials, but it seems to still re-authorize for each file transfer, and after each successful authorization, it calls 'credential approve' to re-cache the approved credentials. On Windows, this overhead raises the bar for which types of files are worth tracking with LFS. For example, it was not worth using LFS to track DLLs in a particular repo of mine, because (1) the DLLs often compressed moderately well, and (2) the file sizes varied widely enough that transferring many smaller/medium DLLs via LFS took longer, in large part due to the credential approve calls.

/cc #2328 (comment)

[technoweenie]

I was able to replicate this with a bit of extra tracing. This patch was applied to the current master (which is beyond LFS v2.3.4), and run with Git v2.14.3. Minus the custom patches, this should match behavior with any recent LFS that has the credential cache enabled. It was enabled by default in LFS v2.3.3+.

diff --git a/lfsapi/creds.go b/lfsapi/creds.go
index 187043b1..c6839e83 100644
--- a/lfsapi/creds.go
+++ b/lfsapi/creds.go
@@ -307,6 +307,8 @@ func (c *credentialCacher) Reject(creds Creds) error {
 }

 func (c *credentialCacher) Approve(creds Creds) error {
+       tracerx.Printf("creds: git credential cache approve (%q, %q, %q)",
+               creds["protocol"], creds["host"], creds["path"])
        err := c.helper.Approve(creds)
        if err == nil {
                c.cmu.Lock()
@@ -332,6 +334,8 @@ func (h *commandCredentialHelper) Reject(creds Creds) error {
 }

 func (h *commandCredentialHelper) Approve(creds Creds) error {
+       tracerx.Printf("creds: git credential helper approve (%q, %q, %q)",
+               creds["protocol"], creds["host"], creds["path"])
        _, err := h.exec("approve", creds)
        return err
 }

Git Credential Behavior

First, it seems that Git itself always calls store on the helper itself. I'm not sure if Git is interacting directly with the credential helper, or if it's going through git credential and it's just not showing up in the trace. My hunch is the latter.

$ GIT_TRACE=1 git clone https://my-git-server/technoweenie/lfs-pull-bug
12:22:49.972579 git.c:340               trace: built-in: git 'clone' 'https://my-git-server/technoweenie/lfs-pull-bug'
Cloning into 'lfs-pull-bug'...
12:22:49.982650 run-command.c:626       trace: run_command: 'git-remote-https' 'origin' 'https://my-git-server/technoweenie/lfs-pull-bug'
12:22:50.234679 run-command.c:626       trace: run_command: 'git credential-osxkeychain get'
12:22:50.241247 git.c:572               trace: exec: 'git-credential-osxkeychain' 'get'
12:22:50.241650 run-command.c:626       trace: run_command: 'git-credential-osxkeychain' 'get'
12:22:50.325235 run-command.c:626       trace: run_command: 'git credential-osxkeychain store'
12:22:50.331867 git.c:572               trace: exec: 'git-credential-osxkeychain' 'store'
12:22:50.332400 run-command.c:626       trace: run_command: 'git-credential-osxkeychain' 'store'

For comparison, here's what the trace output looks like if the credential helper doesn't have creds for that host:

12:09:16.617330 run-command.c:626       trace: run_command: 'git credential-osxkeychain get'
12:09:16.622668 git.c:572               trace: exec: 'git-credential-osxkeychain' 'get'
12:09:16.623104 run-command.c:626       trace: run_command: 'git-credential-osxkeychain' 'get'
Username for 'https://my-git-server': technoweenie
Password for 'https://technoweenie@my-git-server':
12:09:36.632585 run-command.c:626       trace: run_command: 'git credential-osxkeychain store'
12:09:36.638982 git.c:572               trace: exec: 'git-credential-osxkeychain' 'store'
12:09:36.639469 run-command.c:626       trace: run_command: 'git-credential-osxkeychain' 'store'

It appears Git always calls store, regardless of whether it prompted me or not.

LFS Behavior

On the first LFS Batch request, LFS attempts an unauthenticated request (for public repositories). If it gets a 401, it retries with creds. Note that this doesn't prompt me again since it's using the same creds properties as the clone request above.

12:22:50.968958 trace git-lfs: HTTP: 401
12:22:50.969068 trace git-lfs: HTTP: {"message":"Must authenticate to access this API.","documentation_url":"https://help.github.com/enterprise"}
12:22:50.969179 trace git-lfs: setting repository access to basic
12:22:50.969191 trace git-lfs: exec: git 'config' '--replace-all' 'lfs.https://my-git-server/technoweenie/lfs-pull-bug.git/info/lfs.access' 'basic'
12:22:50.974888 trace git-lfs: api: http response indicates "basic" authentication. Resubmitting...
12:22:50.974947 trace git-lfs: creds: git credential fill ("https", "my-git-server", "technoweenie/lfs-pull-bug")
12:22:51.005809 trace git-lfs: Filled credentials for https://my-git-server/technoweenie/lfs-pull-bug
12:22:51.005899 trace git-lfs: HTTP: POST https://my-git-server/technoweenie/lfs-pull-bug.git/info/lfs/objects/batch
12:22:51.102669 trace git-lfs: HTTP: 200
12:22:51.102702 trace git-lfs: creds: git credential cache approve ("https", "my-git-server", "")
12:22:51.102709 trace git-lfs: creds: git credential helper approve ("https", "my-git-server", "")

The next Batch request looks similar, but it doesn't bother with the unauthenticated request attempt. This matches Git's behavior above. I think the reason is the request was successful, but Git and LFS can't tell if the credential helper had to prompt the user (meaning an approve is necessary to save it), or if the credential helper already had it (technically, approve isn't necessary here). So to be safe, it calls approve.

12:22:52.404363 trace git-lfs: creds: git credential cache ("https", "my-git-server", "technoweenie/lfs-pull-bug")
12:22:52.404369 trace git-lfs: Filled credentials for https://my-git-server/technoweenie/lfs-pull-bug
12:22:52.404404 trace git-lfs: HTTP: POST https://my-git-server/technoweenie/lfs-pull-bug.git/info/lfs/objects/batch
12:22:52.404462 trace git-lfs: Read filter-process request.
12:22:52.528844 trace git-lfs: HTTP: 200
12:22:52.528878 trace git-lfs: creds: git credential cache approve ("https", "my-git-server", "")
12:22:52.528888 trace git-lfs: creds: git credential helper approve ("https", "my-git-server", "")

GitHub's LFS implementation doesn't require the credential helper, since the LFS Batch API returns the necessary Authorization header to authenticate. So, it definitely doesn't need to approve anything. However, LFS does it anyway, even if it's not attempting to fill the creds from the helper. Note the empty creds values.

12:22:51.257379 trace git-lfs: HTTP: GET https://my-git-server/lfs/1368/objects/f2ca1bb6c7e907d06dafe4687e579fce76b37e4e93b7605022da52e6ccc26fd2
12:22:51.468848 trace git-lfs: HTTP: 200
12:22:51.468873 trace git-lfs: creds: git credential cache approve ("", "", "")
12:22:51.468877 trace git-lfs: creds: git credential helper approve ("", "", "")

TODO

• Teach the LFS credential cacher to only call approve the first time. LFS should only ever call git credential approve for a given url/path once in the lifetime of a single LFS command.
• Teach the LFS HTTP client to only call Approve() on the credential helper if it was used for a request. I don't know how other LFS servers work, but GitHub's doesn't require it for transferring specific objects.
• Add approve tracing to LFS so we can confirm a fix.