-
Notifications
You must be signed in to change notification settings - Fork 78
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
testscript: some fixes for Plan 9 #115
Conversation
Agh. Maybe we should use a different name? Not being able to use the path separator env var in all platforms is unfortunate. At the same time, I honestly never test on Plan9. |
I meant a different name than |
Sorry, maybe I should've create a separate PR for the As for the |
Ah, gotcha. I don't think any of the other single-character env names are particularly intuitive, to be honest. I'd go with |
bd283d9
to
4c1293e
Compare
We're changing the name because ${/} doesn't work on Plan 9. Plan 9 environment variables are stored as files in `/env`, so they can't have slash in them. Update rogpeppe#90
This is a port of this CL from upstream: https://go-review.googlesource.com/126608
4c1293e
to
815d671
Compare
We dropped this, my apologies. The change LGTM with one nit: backwards compatibility. We need to keep Also, does plan9 need to expose both |
tbh I prefer the solution chosen in this CL: https://go-review.googlesource.com/c/go/+/416554 |
Closing as https://go-review.googlesource.com/c/go/+/416554 has been merged and seems to address the same issue. |
Plan 9 environment variables names can't have slash in them.
Removing this is not a big loss considering it's not available in
cmd/go
testscripts and it's also not documented.Update #90