go-internal
go-internal copied to clipboard
rogpeppe
testscript: some fixes for Plan 9
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
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 /, but still the same name across all OSs. How about slash, which is still short enough? path is too close to $PATH, which already has a different meaning.
Sorry, maybe I should've create a separate PR for the $path change, which is fixing a different issue on plan9. It's just a port of an upstream CL.
As for the / issue, I don't have any preference on a name. Any name without / (including names containing multi-byte unicode characers) will work on plan9. There are many options if you want something short: $|, $\, $÷, $χ, etc.
Ah, gotcha.
I don't think any of the other single-character env names are particularly intuitive, to be honest. I'd go with slash, but feel free to wait for @rogpeppe in case he disagrees.
We dropped this, my apologies.
The change LGTM with one nit: backwards compatibility. We need to keep ${/} working, on platforms other than plan9, otherwise we will likely be breaking users. The new form, ${slash}, would work on all platforms - and those projects wishing to support testing on plan9 can switch to that form.
Also, does plan9 need to expose both $path and $PATH? If not, I would expose just $path, like it's done for $HOME via homeEnvName.
tbh I prefer the solution chosen in this CL: https://go-review.googlesource.com/c/go/+/416554
We keep ${/} around but don't export it, so no need for $slash.
Any reason that wouldn't work here?
