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

Jump to bottom

x/tools/go/packages/packagestest: cleanup fails silently and leaves test junk #29558

Open
4a6f656c opened this issue Jan 4, 2019 · 2 comments
Open

x/tools/go/packages/packagestest: cleanup fails silently and leaves test junk #29558

4a6f656c opened this issue Jan 4, 2019 · 2 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
Unreleased

Comments

@4a6f656c
Copy link
Contributor

4a6f656c commented Jan 4, 2019

The Exported.Cleanup function from x/tools/go/packages/packagestest can fail silently and leave temporary files around - currently running go test in x/tools/import results in around 75MB of test junk being left behind (at least under OpenBSD and I would suspect most other BSD/Unix like platforms), the majority being from TestSimpleCases.

The cause of the issue is that one of the sub-directories is created as 0555, which means that the subsequent os.RemoveAll call fails due to insufficient permissions:

$ ls -l /tmp/TestSimpleCases_import_grouping_not_path_dependent_no_groups_Modules592366644/modcache/pkg/mod/ 
total 20
drwxr-xr-x  6 joel  joel  512 Jan  1 23:53 github.com/
drwxr-xr-x  3 joel  joel  512 Jan  1 23:53 golang.org/
dr-xr-xr-x  3 joel  joel  512 Jan  1 23:53 local.com@v1.0.0/
dr-xr-xr-x  6 joel  joel  512 Jan  1 23:53 manypackages.com@v1.0.0/
dr-xr-xr-x  3 joel  joel  512 Jan  1 23:53 rsc.io@v1.0.0/

There are possibly three separate issues here:

  1. Either these directories need to be created with permissions 0755, or the Exported.Cleanup function needs to chmod u+w before trying to os.RemoveAll.

  2. The Exported.Cleanup function should not really fail silently, since these kinds of issues go undetected until a disk fills up.

  3. There may be an issue with the code that is creating these directories (which should be using 0755 instead of 0555), if it is outside the test harness.
@gopherbot gopherbot added this to the Unreleased milestone Jan 4, 2019
@heschi
Copy link
Contributor

heschi commented Jan 4, 2019

Sorry, https://golang.org/cl/156078 was hung up in review for a while. Should be fixed now.

@heschi heschi closed this as completed Jan 4, 2019
@heschi
Copy link
Contributor

heschi commented Jan 4, 2019 •
edited

Maybe this should stay open to decide if Cleanup should do something about the error, per the second point. I'm not sure what that would be, since it doesn't currently return an error, and it doesn't have a testing.T to report to.

@matloob @ianthehat

@heschi heschi reopened this Jan 4, 2019
@julieqiu julieqiu added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 28, 2019
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Sep 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
4 participants
@julieqiu @4a6f656c @gopherbot @heschi