commands: unambiguously resolve remote references #3285
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We have used the getRemoteRefs since [1] to return a slice of all remote references, without distinguishing which remote they came from. The reason for doing so was that we had had modified each reference's "name" (i.e., the last element of splitting the full reference name by '/'). This was OK to do, since we modified the reference's name (i.e., the last component of the fully qualified reference name as read from Git when splitting on the '/' character) to include the remote name. This allowed us to call '(*git.Ref).Refspec()', which would theoretically format us a correctly printed reference name, for all kinds of references. This is broken in practice, so to prepare for a future commit that will fix it, let's return a map of remote name to all references present on that remote, such that we can format each based on its remote and full name individually. [1]: ce89eb3 (commands/command_migrate: teach how to find all remote refs, 2017-06-09)
Since 5e0f81d (Adding utility methods to get recent refs, 2015-08-17), we have treated references beginning with the prefix "refs/remotes/tags" as being of type "remote tag". This is not a namespace that upstream Git uses in practice to denote remote tags. Instead, tags are automatically namespaced into our local refs/tags, hence rendering this prefix (and its type) meaningless. As such, let's remove it so that the 'git lfs migrate' (et. al.) commands do not get confused about references that do not exist. Co-authored-by: brian m. carlson <[email protected]>
In 'formatRefName()', we special-case references belonging to a remote in order to round-trip a reference name that 'git-rev-parse(1)' (et. al.) will understand. This is done because the `*git.Ref` type does not retain information about the remote to which it belongs, so we add it back here as a shortcut to avoid making this change throughout the codebase. Falling back to ref.Name, however, can introduce ambiguity when there is a top-level reference of the same name. For example, calling 'formatRefName()' on a reference named 'refs/heads/foo' when there is a reference named simply, 'foo' will cause Git to complain. Avoid this by calling `(*git.Ref) Refspec()`, which reintroduces the prefix thus eliminating any ambiguity. Co-authored-by: brian m. carlson <[email protected]>
Since 'formatRefName()' no longer has multiple case statements, let's simplify the body to use an `if`, instead. Co-authored-by: brian m. carlson <[email protected]>
Per our work in the previous commit(s), it is now suitable to call formatRefName() again thus avoiding our "hack" to prepend the remote name to the reference before calling '(*git.Ref) Refspec()'. As such, let's do that. Co-authored-by: brian m. carlson <[email protected]>
bk2204
approved these changes
Sep 26, 2018
|
Will this fix be in git 2.19.1 do you think? |
|
This will be in Git LFS 2.6.0, which we don't yet have a release date for, although it will likely be soonish. This is a separate project from Git, although it is bundled in Git for Windows. Git for Windows will likely pick it up automatically once we do the release. I can't say how soon afterwards, though. |
|
@bk2204 Oops yes I meant the next Git-LFS release sorry. Ok will keep an eye out, tnx! |
|
I'm on git-lfs 2.6.1 but this still happens: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request fixes a long-standing bug wherein a reference beginning with
refs/remotes/tagswould incorrectly be round-tripped back to Git, causinggit lfs migrateand others to complain of a reference that does not exist.Closes: #3281.
/cc @git-lfs/core
/cc @zezba9000, #3281