Skip to content

path/filepath: use a temp dir in path_test.go #24238

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

Closed
wants to merge 1 commit into from

Conversation

mostynb
Copy link
Contributor

@mostynb mostynb commented Mar 4, 2018

We should avoid writing temp files to GOROOT, since it might be readonly.

Fixes #23881

@googlebot googlebot added the cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change. label Mar 4, 2018
@gopherbot
Copy link
Contributor

This PR (HEAD: 5083e7e) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/#/c/go/+/98517 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Gerrit Bot:

Uploaded patch set 2: Commit message was updated.


Please don’t reply on this GitHub thread. Visit golang.org/cl/98517.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Mostyn Bramley-Moore:

Patch Set 2:

WiP, I'm trying to figure out how to make this test use absolute paths (I'm not sure sure if this makes sense), to avoid the os.Chdir call. If that turns out to be a no-go, then the tree var does not need to be local.


Please don’t reply on this GitHub thread. Visit golang.org/cl/98517.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

This PR (HEAD: 6ea846c) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/#/c/go/+/98517 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Andrew Bonventre:

Patch Set 3:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/98517.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

This PR (HEAD: 62d17f3) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/#/c/go/+/98517 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Gerrit Bot:

Uploaded patch set 4: Commit message was updated.


Please don’t reply on this GitHub thread. Visit golang.org/cl/98517.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit Bot:

Uploaded patch set 5: Commit message was updated.


Please don’t reply on this GitHub thread. Visit golang.org/cl/98517.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Mostyn Bramley-Moore:

Patch Set 5:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/98517.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Brad Fitzpatrick:

Patch Set 5: Run-TryBot+1


Please don’t reply on this GitHub thread. Visit golang.org/cl/98517.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 5:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=090843b7


Please don’t reply on this GitHub thread. Visit golang.org/cl/98517.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 5:

Build is still in progress...
This change failed on windows-386-2008:
See https://storage.googleapis.com/go-build-log/090843b7/windows-386-2008_de41298a.log

Consult https://build.golang.org/ to see whether it's a new failure. Other builds still in progress; subsequent failure notices suppressed until final report.


Please don’t reply on this GitHub thread. Visit golang.org/cl/98517.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 5: TryBot-Result-1

2 of 17 TryBots failed:
Failed on windows-386-2008: https://storage.googleapis.com/go-build-log/090843b7/windows-386-2008_de41298a.log
Failed on windows-amd64-2016: https://storage.googleapis.com/go-build-log/090843b7/windows-amd64-2016_15a61932.log

Consult https://build.golang.org/ to see whether they are new failures.


Please don’t reply on this GitHub thread. Visit golang.org/cl/98517.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Mostyn Bramley-Moore:

Patch Set 5:

These windows build failures look strange, but I don't have access to a windows machine to debug locally- is it OK for me to upload some diagnostic changes here? And if so, will the trybots automatically run on the new patchset?


Please don’t reply on this GitHub thread. Visit golang.org/cl/98517.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Giovanni Bajo:

Patch Set 5:

Patch Set 5:

These windows build failures look strange, but I don't have access to a windows machine to debug locally- is it OK for me to upload some diagnostic changes here? And if so, will the trybots automatically run on the new patchset?

Sure go ahead. We'll monitor and trigger trybots.


Please don’t reply on this GitHub thread. Visit golang.org/cl/98517.
After addressing review feedback, remember to publish your drafts!

We should avoid writing temp files to GOROOT, since it might be readonly.

Fixes golang#23881

Change-Id: I500ca0e0944b6053fd8fd2879ff88f5636424dab
@gopherbot
Copy link
Contributor

This PR (HEAD: de0211d) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/#/c/go/+/98517 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Mostyn Bramley-Moore:

Patch Set 6:

Reverted to the os.Chdir version. I think the problem with patchset 5 was pre-existing cleanup at the bottom of the TestWalk function which tried to delete too much after i prepended paths to 'tree'.


Please don’t reply on this GitHub thread. Visit golang.org/cl/98517.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Giovanni Bajo:

Patch Set 6: Run-TryBot+1


Please don’t reply on this GitHub thread. Visit golang.org/cl/98517.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 6:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=de461bac


Please don’t reply on this GitHub thread. Visit golang.org/cl/98517.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 6: TryBot-Result+1

TryBots are happy.


Please don’t reply on this GitHub thread. Visit golang.org/cl/98517.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Brad Fitzpatrick:

Patch Set 6: Code-Review+2


Please don’t reply on this GitHub thread. Visit golang.org/cl/98517.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

This PR is being closed because golang.org/cl/98517 has been merged.

@gopherbot gopherbot closed this Mar 5, 2018
gopherbot pushed a commit that referenced this pull request Mar 5, 2018
We should avoid writing temp files to GOROOT, since it might be readonly.

Fixes #23881

Change-Id: Iaa38ec404b303f0cf27fdfb7daf1ddd60fd5d1c9
GitHub-Last-Rev: de0211d
GitHub-Pull-Request: #24238
Reviewed-on: https://go-review.googlesource.com/98517
Run-TryBot: Giovanni Bajo <rasky@develer.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@mostynb mostynb deleted the issue_23881 branch March 6, 2018 07:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants