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

cmd/internal/moddeps: TestAllDependencies may report a false positive when $TMPDIR is a symbolic link #44080

Closed
dmitshur opened this issue Feb 3, 2021 · 2 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone

Comments

@dmitshur
Copy link
Contributor

dmitshur commented Feb 3, 2021

TestAllDependencies uses t.TempDir to get a temporary directory where the Go tree being tested can be safely copied. t.TempDir uses os.TempDir. If the system temporary directory path involves symlinks, this can cause the test to report a false positive.

This is because one of the checks TestAllDependencies performs (when run without -short flag) is that go generate syscall internal/syscall/... in a copy of the GOROOT directory produces a zero diff. Generating the syscall packages results in the execution of golang.org/x/sys/windows/mkwinsyscall. mkwinsyscall has some logic to determine whether it's being called on a file inside the standard library, but that logic may report incorrect results when the standard library is inside a symlinked directory (this is issue #44079).

In practice, on macOS, the default value of $TMPDIR is a symlink:

$ echo $TMPDIR
/var/folders/zb/5p8cwfhj29gf_m8vdy8ylmlr00jwcj/T/
$ ls -la /var
lrwxr-xr-x@ 1 root  admin  11 Dec 10  2019 /var -> private/var

This issue is to investigate if anything can be done to improve this. If it's possible to at least detect this situation, then we can skip the test. Or maybe it's not feasible to do anything, then we can apply the label Unfortunate and close this.

CC @bcmills, @ianlancetaylor.

@dmitshur dmitshur added Testing An issue that has been verified to require only test changes, not just a test failure. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Feb 3, 2021
@dmitshur dmitshur added this to the Backlog milestone Feb 3, 2021
@bcmills
Copy link
Contributor

bcmills commented Feb 3, 2021

I suspect this can be addressed by either fixing cmd/go to set PWD appropriately for generate subcommands (#43862), or fixing mkwinsyscall to either accept an explicit flag for “is the package in std?” or correctly infer the answer itself (#44079).

@dmitshur dmitshur modified the milestones: Backlog, Go1.17 Feb 3, 2021
@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Feb 3, 2021
@dmitshur dmitshur self-assigned this Feb 3, 2021
@gopherbot
Copy link

Change https://golang.org/cl/304449 mentions this issue: cmd/internal/moddeps: fix false positive when $TMPDIR is symlinked

@golang golang locked and limited conversation to collaborators Mar 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Development

No branches or pull requests

3 participants