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

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
Labels
FrozenDueToAge

Comments

@bradfitz
Copy link
Contributor

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 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 commented Oct 2, 2014

Comment 2:

Thanks. We have found the little issue. Nick will send a CL.

@gopherbot
Copy link

Comment 3:

CL https://golang.org/cl/149300046 mentions this issue.

@0intro
Copy link
Member

0intro commented Oct 15, 2014

Comment 4:

Owner changed to @0intro.

@gopherbot
Copy link

Comment 5:

CL https://golang.org/cl/158970045 mentions this issue.

@0intro
Copy link
Member

0intro commented Oct 16, 2014

Comment 6:

This issue was closed by revision 70896a7.

Status changed to Fixed.

@golang golang locked and limited conversation to collaborators Jun 25, 2016
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jun 25, 2018
@0intro @wheatman
syscall: don't cache environment variables on Plan 9
ecbec1f 
Fixes golang#8849.

LGTM=bradfitz, aram
R=bradfitz, rsc, aram
CC=golang-codereviews
https://golang.org/cl/158970045
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jun 26, 2018
@0intro @wheatman
syscall: don't cache environment variables on Plan 9
997191a 
Fixes golang#8849.

LGTM=bradfitz, aram
R=bradfitz, rsc, aram
CC=golang-codereviews
https://golang.org/cl/158970045
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jul 9, 2018
@0intro @wheatman
syscall: don't cache environment variables on Plan 9
80e45ed 
Fixes golang#8849.

LGTM=bradfitz, aram
R=bradfitz, rsc, aram
CC=golang-codereviews
https://golang.org/cl/158970045
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jul 30, 2018
@0intro @wheatman
syscall: don't cache environment variables on Plan 9
0ad18d9 
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.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@bradfitz @0intro @gopherbot