golang/go

cmd/go: symbolic links not dropped from repo

rogpeppe opened this issue · 8 comments

go version go1.11rc1 linux/amd64

The zip unarchiving used by cmd/go/internal/modfetch does not respect
symbolic links or file modes. This means that if there's a test directory that
contains symlinks or executable files (e.g. shell scripts) and there are
tests that rely on those properties, they will fail when the module is
a dependency and tests are run on it (for example with go test all).

Currently a symbolic link is treated as a plain file - it is unarchived as a normal
file with the symbolic link as its contents, which seems wrong even if we
decide not to support symlinks in modules. Similarly, the module
content hash check will treat a symbolic link identically to a regular file.

dsnet commented

Per #24057 (comment), I think this is working as intended.

@dsnet Thanks a lot for that link - I hadn't seen that. Given that's a deliberate decision, I'm fine with it (very happy actually), but I do think that somewhere you should at least get some error or warning that your symbolic link has been silently turned into a plain file.

rsc commented

Can you please give an example? Symbolic links are supposed to be silently elided from the underlying version control repo. It sounds like one of the converters is including them in the archive incorrectly.

See this repo: https://github.com/rogpeppe/test2
The tests directory in the top-level module has a symlink in it which is not elided on import.

rsc commented

Good candidate for backport once we fix it. Probably git archive is putting symlinks into the zip and I didn't realize that was a thing that would need to be filtered out.

Change https://golang.org/cl/153819 mentions this issue: cmd/go/internal/modfetch: skip symlinks in (*coderepo).Zip

@gopherbot, please backport to Go 1.11: incorrect module contents produce incorrect go.sum entries, and the fix is tiny.

Backport issue(s) opened: #29191 (for 1.11).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.