[Python-ideas] chdir context manager

[Terry Reedy]

On 1/19/2013 5:10 AM, Daniel Shahaf wrote:
> The following is a common pattern (used by, for example,
> shutil.make_archive):
>
>      save_cwd = os.getcwd()
>      try:
>          foo()
>      finally:
>          os.chdir(save_cwd)
>
> I suggest this deserves a context manager:
>
>      with saved_cwd():
>          foo()

This strikes me as not a proper context manager. A context manager 
should create a temporary, altered context. One way is to add something 
that is deleted on exit. File as context managers are the typical 
example. Another way is to alter something after saving the restore 
info, and restoring on exit. An example would be a context manager to 
temporarily change stdout. (Do we have one? If not, it would be at least 
as generally useful as this proposal.)

So to me, your proposal is only 1/2 or 2/3 of a context manager. (And 
'returns an open file descriptor for the saved directory' seems backward 
or wrong for a context manager.) It does not actually make a new 
context. A proper temp_cwd context manager should have one parameter, 
the new working directory, with chdir(new_cwd) in the enter method. To 
allow for conditional switching, the two chdir system calls could be 
conditional on new_cwd (either None or '' would mean no chdir calls).

Looking at your pattern, if foo() does not change cwd, the save and 
restore is pointless, even if harmless. If foo does change cwd, it 
should also restore it, whether explicitly or with a context manager 
temp_cwd.

Looking at your actual example, shutil.make_archive, the change and 
restore are conditional and asymmetrical.

   save_cwd = os.getcwd()
   if root_dir is not None:
     ...
     if not dry_run:
       os.chdir(root_dir)
   ...
   finally:
     if root_dir is not None:
     ...
       os.chdir(save_cwd)

The initial chdir is conditional on dry_run (undocumented, but passed on 
to the archive function), the restore is not. Since I believe not 
switching on dry_runs is just a minor optimization, I believe that that 
condition could be dropped and the code re-written as

   with new_cwd(root_dir):
     ...

I am aware that this would require a change in the finally logging, but 
that would be true of the original proposal also.

-- 
Terry Jan Reedy