Dluksza include ref in upload request

[technoweenie]

Re-applied #969 against the current master, fixing some merge conflicts.

[technoweenie]

Echoing @sinbad's comments in #969 (comment):

I'm not crazy about how this makes the LFS push invoked by the pre-push hook semantically different to using git lfs push.

I'm sure it's not intentional. The fetch command sends it unless the --all flag is used. scanAll() only returns a slice of lfs pointers with no ref information.

None of the git lfs push calls because of how uploadPointers() is used.

I think scanAll() would have to change to return LFS pointers along with the ref that they're in. But I don't think git rev-list returns that info. If it did, it could loop through the refs, starting transfer queues for each one, like the lfs pre-push command.

[technoweenie]

Instead of using git rev-list --all, any thoughts on getting the refs first with git show-ref --heads --tags? At least for pushes. Fetches probably do want the remote refs too (but not stash).

[sinbad]

If it did, it could loop through the refs, starting transfer queues for each one, like the lfs pre-push command.

I assume you're talking about git lfs push --all only? The problem I can see with the above is that the same LFS object can be reached by multiple refs, so you're going to get upload requests for the same oid duplicated in different queues. You'd really have to serialise the pushing of each ref to avoid that - it probably would only result in duplicated uploads where the one that finishes last is thrown away (same as if 2 people uploaded simultaneously) but it becomes so much more likely it's worth avoiding.

[technoweenie]

I assume you're talking about git lfs push --all only?

No, all code paths just build up a list of lfs pointers first, before uploading.

https://github.com/github/git-lfs/blob/master/commands/command_push.go#L55-L74

The pre-push command relies on the transfer queue's Wait() function.

https://github.com/github/git-lfs/blob/6b00ebc996a7cd2b28fdcd027fe527c48975c113/commands/command_pre_push.go#L59-L74

You're right that it does send the OIDs to the batch api multiple times, potentially. But it doesn't transfer the same one multiple times in a single command. We could probably solve this by tracking the successful uploads, and preventing them from being added to another ref's transfer queue.

[technoweenie]

I made a few code spikes that illustrate how we could refactor those commands. #1038 changes lfs push --all so it runs git show-ref to get the list of local refs. #1040 updates lfs pre-push so it doesn't attempt to upload duplicate objects that have been uploaded in the same process (from another ref).

[technoweenie]

Wow, this is a major yak shave :/ Redoing this in #1128....