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

syscall: plan9 environment variable handling #8849

Closed
bradfitz opened this issue Oct 1, 2014 · 6 comments
Closed

syscall: plan9 environment variable handling #8849

bradfitz opened this issue Oct 1, 2014 · 6 comments

Comments

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Oct 1, 2014

While adding syscall.Unsetenv, I notice that plan9's environment handling in pkg syscall
seems inconsistent.

It sometimes does copyenv, but sometimes not. And things like Getenv can lazily populate
it. It seems like there are possible bugs with different orderings, but I didn't look
too hard.

Talking to Russ, he points out that plan9 processes can share environments, so keeping a
cached copy of anything in syscall is probably wrong.

I will do the bare minimum to keep plan9 compiling, but somebody should probably own
cleaning this up.
@bradfitz
Copy link
Contributor Author

@bradfitz bradfitz commented Oct 2, 2014

Comment 1:

Sorry, I did end up breaking plan9 with https://golang.org/cl/82040044
I think the reason is that Getenv and Setenv update the cached environment but don't
update the envi map I added.
I would try to fix this, but I don't have a way to test and in my opinion this code
needs an overhaul, so I'll leave it to people who can test.
I would be happy to review, though.
@0intro
Copy link
Member

@0intro 0intro commented Oct 2, 2014

Comment 2:

Thanks. We have found the little issue. Nick will send a CL.
@gopherbot
Copy link

@gopherbot gopherbot commented Oct 2, 2014

Comment 3:

CL https://golang.org/cl/149300046 mentions this issue.
@0intro
Copy link
Member

@0intro 0intro commented Oct 15, 2014

Comment 4:

Owner changed to @0intro.

@gopherbot
Copy link

@gopherbot gopherbot commented Oct 15, 2014

Comment 5:

CL https://golang.org/cl/158970045 mentions this issue.
@0intro
Copy link
Member

@0intro 0intro commented Oct 16, 2014

Comment 6:

This issue was closed by revision 70896a7.

Status changed to Fixed.

@bradfitz bradfitz added fixed labels Oct 16, 2014
@golang golang locked and limited conversation to collaborators Jun 25, 2016
wheatman added a commit to wheatman/go-akaros that referenced this issue Jun 25, 2018
Fixes golang#8849.

LGTM=bradfitz, aram
R=bradfitz, rsc, aram
CC=golang-codereviews
https://golang.org/cl/158970045
wheatman added a commit to wheatman/go-akaros that referenced this issue Jun 26, 2018
Fixes golang#8849.

LGTM=bradfitz, aram
R=bradfitz, rsc, aram
CC=golang-codereviews
https://golang.org/cl/158970045
wheatman added a commit to wheatman/go-akaros that referenced this issue Jul 9, 2018
Fixes golang#8849.

LGTM=bradfitz, aram
R=bradfitz, rsc, aram
CC=golang-codereviews
https://golang.org/cl/158970045
wheatman added a commit to wheatman/go-akaros that referenced this issue Jul 30, 2018
Fixes golang#8849.

LGTM=bradfitz, aram
R=bradfitz, rsc, aram
CC=golang-codereviews
https://golang.org/cl/158970045
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.