This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: support.EnvironmentVarGuard broken
Type: behavior Stage: needs patch
Components: Tests Versions: Python 3.0
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: doerwalter, mark.dickinson
Priority: low Keywords: easy

Created on 2009-04-25 11:07 by doerwalter, last changed 2022-04-11 14:56 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
diff.txt doerwalter, 2009-04-25 11:11
diff2.txt doerwalter, 2009-04-25 11:21
Messages (13)
msg86465 - (view) Author: Walter Dörwald (doerwalter) * (Python committer) Date: 2009-04-25 11:07
support.EnvironmentVarGuard seems to be broken:

import os
from test import support

print(os.environ.get("HOME"))

with support.EnvironmentVarGuard() as env:
   env.unset("HOME")
   env.set("HOME", "foo")

print(os.environ.get("HOME"))

The output I get is:
   /Users/walter
   None

However I would have expected:
   /Users/walter
   /Users/walter

One solution would be to simply copy the exiting environment dictionary
in __enter__(), which would have the added advantage that code in the
with block could manipulate os.environ directly without going through
the EnvironmentGuardVariable.
msg86466 - (view) Author: Walter Dörwald (doerwalter) * (Python committer) Date: 2009-04-25 11:11
Here's a patch that changes EnvironmentVarGuard to make a copy of
os.environ at the start. The set and unset methods are useless now, but
I left them in for backwards compatibility. Should they be removed?
msg86467 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-04-25 11:13
Is efficiency a concern here?  I'm just imagining an environment with 
thousands of variables, but only a few changes being made---the old 
(broken) code just resets the variables that have changed, while the 
patched version resets all of the variables.
msg86468 - (view) Author: Walter Dörwald (doerwalter) * (Python committer) Date: 2009-04-25 11:16
If you want to restore only those environment variables that have change
you somehow have to record which *do* have changed, i.e. you'd have to
go through EnvironmentVarGuard again. I'm working on a patch that does that.
msg86469 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-04-25 11:18
Could you maybe get away with a single dict "changed",
which would just record previous values (with None representing
a value that was previously unset)?
msg86470 - (view) Author: Walter Dörwald (doerwalter) * (Python committer) Date: 2009-04-25 11:21
That's exactly what I was thinking too. Here's the patch. Running the
test suite now.
msg86471 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-04-25 11:30
Patch looks good to me, and (for py3k) all tests pass on my machine.

As far as I can tell, there's currently no test for EvironmentVarGuard 
itself, though it's tested indirectly by other tests.  It might be good to 
have some direct tests (including one for the broken behaviour you found).
msg86473 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-04-25 11:36
I take that back:  this patch appears to break test_importlib on
OS X 10.5.6 (py3k).  Not sure why.
msg86474 - (view) Author: Walter Dörwald (doerwalter) * (Python committer) Date: 2009-04-25 11:36
OK, I'll remove the clear method (which is a new feature) and then check
it in.
msg86479 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-04-25 11:48
It looks like the unset method should check whether the variable
is set before trying to unset it.

Perhaps this also needs to happen in the __exit__ method?
msg86480 - (view) Author: Walter Dörwald (doerwalter) * (Python committer) Date: 2009-04-25 11:49
This might have something to do with the _keymap hook.
msg86482 - (view) Author: Walter Dörwald (doerwalter) * (Python committer) Date: 2009-04-25 11:55
You're right checking both in unset() and __exit__() fixes the importlib
failures. I'll check in the fix.
msg86498 - (view) Author: Walter Dörwald (doerwalter) * (Python committer) Date: 2009-04-25 12:53
Checked in:
r71875 (trunk)
r71876 (release26-maint)
r71881 (py3k)
r71885 (release30-maint)
History
Date User Action Args
2022-04-11 14:56:48adminsetgithub: 50087
2009-04-25 12:53:52doerwaltersetstatus: open -> closed
resolution: fixed
messages: + msg86498
2009-04-25 11:55:16doerwaltersetmessages: + msg86482
2009-04-25 11:49:05doerwaltersetmessages: + msg86480
2009-04-25 11:48:07mark.dickinsonsetmessages: + msg86479
2009-04-25 11:36:55doerwaltersetmessages: + msg86474
2009-04-25 11:36:48mark.dickinsonsetmessages: + msg86473
2009-04-25 11:30:38mark.dickinsonsetmessages: + msg86471
2009-04-25 11:21:37doerwaltersetfiles: + diff2.txt

messages: + msg86470
2009-04-25 11:18:31mark.dickinsonsetmessages: + msg86469
2009-04-25 11:16:51doerwaltersetmessages: + msg86468
2009-04-25 11:13:08mark.dickinsonsetnosy: + mark.dickinson
messages: + msg86467
2009-04-25 11:11:07doerwaltersetfiles: + diff.txt

messages: + msg86466
2009-04-25 11:07:11doerwaltercreate