Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

git: include SHA1 in ref-less revisions #2982

Merged
merged 2 commits into from
May 2, 2018

Conversation

ttaylorr
Copy link
Contributor

This pull request likely fixes an issue pointed out by @dalemyers in #2974:

As part of our builds running on VSTS, the first thing it does is to clone our repo and checkout the commit we are building. Sometimes though, a few times in a hundred, we get a failure like this:

2018-04-04T09:05:07.8098800Z $ git-lfs fetch origin e0b225c0e5c2747b2ab4270238ef783113fcd673
2018-04-04T09:05:07.8112210Z fetch: Fetching reference e0b225c0e5c2747b2ab4270238ef783113fcd673
2018-04-04T09:05:07.8126000Z Could not scan for Git LFS files
2018-04-04T09:05:07.8132650Z 
2018-04-04T09:05:07.8145800Z Error in git ls-tree: exit status 128 fatal: not a tree object

There appears to be no pattern in the commits that fail. Retrying basically always works. It's just a weird intermittent issue.

The issue, I believe is related to invocations of git-lfs-fetch(1) that use a sha1 instead of a qualified reference name. For example:

$ git lfs fetch origin 987c137
# ...

Code later in the command finds the attached ref by calling git.ResolveRefs(), and then uses ref.Sha throughout:

s := fetchRef(ref.Sha, filter)

all the way down (via fetchRef(), pointerstoFetchForRef(), lfs.GitScanner.ScanTree(), lfs.runScanTree(), lfs.lsTreeBlobs()) to git.LsTree():

git-lfs/git/git.go

Lines 223 to 232 in 1a98fa5

func LsTree(ref string) (*subprocess.BufferedCmd, error) {
return gitNoLFSBuffered(
"ls-tree",
"-r", // recurse
"-l", // report object size (we'll need this)
"-z", // null line termination
"--full-tree", // start at the root regardless of where we are in it
ref,
)
}

If ref.Sha is empty, Git will not be able to scan it:

$ git ls-tree -r -l -z --full-tree ""
fatal: Not a valid object name

To ensure that Git LFS faithfully reports the Sha1 to scan, assign it when we parse a non-symbolic "reference". (While looking at this code, I noticed that the terminology in this part of the project is incorrect, but that will be for a later change).

/cc @git-lfs/core

@@ -245,7 +245,8 @@ func ResolveRef(ref string) (*Ref, error) {

if len(lines) == 1 {
// ref is a sha1 and has no symbolic-full-name
fullref.Name = lines[0] // fullref.Sha
fullref.Name = lines[0]
fullref.Sha = lines[0]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks sensible. I wonder, if it would make sense to check here if lines[0] is not empty and/or contains a 40 char string? However, in a quick check I wasn't able to make rev-parse return an empty result with positive error code (I guess this is not possible).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only thing that could cause confusion in the future is that fullref.Name is a hash. But that has been there before. So we are not worse than before I guess 😄

@ttaylorr
Copy link
Contributor Author

ttaylorr commented May 2, 2018

@larsxschneider thanks!

@ttaylorr ttaylorr merged commit 75d7f1e into master May 2, 2018
@ttaylorr ttaylorr deleted the git-include-sha1-in-ref-parse branch May 2, 2018 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants