seankhliao
commented
3 years ago I see the file is stored with Git LFS? I don't think the proxy has that setup
Open costela opened 3 years ago
seankhliao
commented
3 years ago I see the file is stored with Git LFS? I don't think the proxy has that setup
bcmills
commented
3 years ago For Git LFS, see also #38941 and #39720.
costela
commented
3 years ago @seankhliao oh man, that's embarassing. Totally overlooked LFS. Then this is just a duplicate of #38941.
Then the question becomes: shouldn't the locally generated zip also ignore all filters, to ensure the content seen by the proxy is the same as seen by go when not using the proxy?
The git archive call already takes some steps to normalize the output, but maybe the existence of git filters was overlooked? It feels a bit weird to have the sort of "environment changes" (whether git-lfs is installed locally or not) have such a disruptive (and opaque) effect.
bcmills
commented
3 years ago (Since #38941 timed out, let's continue the discussion here.)
bcmills
commented
3 years ago Picking up from https://github.com/golang/go/issues/38941#issuecomment-626754044:
The
gocommand usesgit archiveunder the hood.According to https://github.com/git-lfs/git-lfs/issues/1322#issuecomment-520559436,
If you have Git LFS enabled (i.e., the filter rules are properly set up via
git lfs install), a recent version ofgit archivewill include the LFS files in it, even in a bare repository.So it's not obvious to me why
GIT_LFS_SKIP_SMUDGE=1would be the right resolution here: why wouldgit-lfsusers expect Go modules to include (or omit) LFS files, and under what conditions? Does this behavior vary withgitandgit-lfsversions, and is upgrading to a more recentgitand/orgit-lfsbinary a viable workaround?
(I haven't used Git LFS, so the answers to those questions are not obvious to me.)
costela
commented
3 years ago I'm unfortunately not familiar with git internals, but I suspect the issue is not exclusive to LFS. Any comparable "git extension" requiring external binaries and opportunistic integration via .gitattributes ("opportunistic" in the sense that git will still work without them) will presumably fail in a similarly opaque way when running GOPROXY=direct go get in a system that happens to have them installed. (git-crypt is another example; though probably less relevant).
It seems unrealistic to expect proxy.golang.org to support all of these extensions. OTOH, there doesn't seem to be a clean way to disable all clean/smudge filters during local checksum generation (i.e.: git archive). The following patch works for me™, but feels like a huge hack:
diff --git a/src/cmd/go/internal/modfetch/codehost/git.go b/src/cmd/go/internal/modfetch/codehost/git.go
index 31921324a7..099837554a 100644
--- a/src/cmd/go/internal/modfetch/codehost/git.go
+++ b/src/cmd/go/internal/modfetch/codehost/git.go
@@ -820,7 +820,7 @@ func (r *gitRepo) ReadZip(rev, subdir string, maxSize int64) (zip io.ReadCloser,
// text file line endings. Setting -c core.autocrlf=input means only
// translate files on the way into the repo, not on the way out (archive).
// The -c core.eol=lf should be unnecessary but set it anyway.
- archive, err := Run(r.dir, "git", "-c", "core.autocrlf=input", "-c", "core.eol=lf", "archive", "--format=zip", "--prefix=prefix/", info.Name, args)
+ archive, err := Run(r.dir, "git", "-c", "core.autocrlf=input", "-c", "core.eol=lf", "archive", "--worktree-attributes", "--format=zip", "--prefix=prefix/", info.Name, args)
if err != nil {
if bytes.Contains(err.(*RunError).Stderr, []byte("did not match any files")) {
return nil, os.ErrNotExistThe documentation for the --worktree-attributes flag mentions only usage in a working-tree and we're running archive in a bare repository, so there isn't any guarantee its behavior won't change.
OneOfOne
commented
3 years ago Got hit by this as well, the patch from @costela worked since it was kinda urgent.
What version of Go are you using (
go version)?Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (
go env)?go envOutputWhat did you do?
I think I might have found data corruption on proxy.golang.org, and it's unfortunately unclear how it relates to the module's content.
First, the "correct" case: the zip file stored in
proxy.golang.orgmatches the checksum onsum.golang.org, as expected.NOTE: this repo was forked and tagged explicitly to debug this issue. The tag is new and hasn't been "moved".
Now we hit the problem with
GOPROXY=direct:If we compare the zip file received from
proxy.golang.orgto the one generated locally as a tempfile when usingGOPROXY=direct, we notice a discrepancy:$ diff -urN <(zipinfo goproxy.zip) <(zipinfo direct.zip)It looks like
proxy.golang.orgsilently truncated one binary file in the repository, so the checksum insum.golang.orgis for this "truncated" version, which doesn't correspond to the actual code in the original repository.What did you expect to see?
The checksum seen when downloading with or without
GOPROXY=directshould be the same.What did you see instead?
Checksum mismatch.