Move `contextlib.chdir` to become `shutil.chdir`

[jb2170]

Following on from Add umask to contextlib shutil, which is now pending its merge, this thread discusses relocating contextlib.chdir alongside umask in shutil. This may set an established location for environment-manipulating context managers, and in doing so resolves some feature-creep inside contextlib.

A preview of my suggested changes so far, including the process of gently deprecating contextlib.chdir, can be found here.

The causes which impel the separation

It is of the opinion that contextlib should consist of “tools for making and manipulating context managers”, a module primarily for base classes and agnostic decorators. It should not be a scratch pad for all stdlib context manager implementations.

chdir’s admission to contextlib was after much debate, but in retrospect there was an oversight in the scope of its behavior: chdir is not just a context manager that provides control flow within a Python script, or even just manipulates module-level attributes as redirect_std{out,err} do; it affects the execution environment of the whole process via syscalls!

chdir’s effect of temporarily changing the current working directory is done by the chdir wrapper around chdir(2) within the os module, whose only reason for import into contextlib is to provide this. A quick ctrl+f search of shutil.py gives over 200 matches for os., and it seems like a better home for chdir, next to the newly expected umask context manager, which also affects the process’s execution environment via the os.umask wrapper around umask(2). That’s one less import for contextlib!

With the addition of umask, the documentation for shutil was updated with the creation of a section titled ‘Context managers to modify process execution environments’. This is intended to set a precedent, that chdir can easily slot into. Some of the documentation for this section was salvaged and generalized from chdir too.

Deprecating contextlib.chdir

I’ve gone with

# contextlib.py
def chdir(path):
    import warnings
    from shutil import chdir as chdir_shutil
    warnings.warn('contextlib.chdir has been relocated to shutil.chdir',
                  DeprecationWarning, stacklevel=2)
    return chdir_shutil(path)

so far. Lazily importing shutil within the chdir function probably avoids overhead and a cyclic-dependency between shutil and contextlib. Please point out if there’s something wrong; I haven’t deprecated much before!

No timeline has been set for the deprecation.

Related possibilities for cleaning up contextlib

There have also been mentions in the previous thread that contextlib.redirect_std{out,err} may be too specific / side-effect heavy / better located in somewhere like shutil than in contextlib. Perhaps these could follow a similar process of relocation, or perhaps that’s for another thread.

[graingert (Thomas Grainger)]

moving chdir from contextlib to somewhere like shutil sounds great, however, I think that contextlib.chdir should stay a class, but on attribute access raise the deprecation warning using a module level __getattr__

def __getattr__(name):
    global chdir

    if name == 'chdir':
        warnings.warn(..., stacklevel=2)
        from shutil import chdir as _chdir
        chdir = _chdir
        return _chdir

    raise AttributeError(f"module {__name__!r} has no attribute {name!r}")

[hauntsaninja (Shantanu Jain)]

IMO it’s simply not worth the churn. As far as imports go, os is very cheap since it’s already imported in every Python process

[storchaka (Serhiy Storchaka)]

If move to shutil, you need to change the name. Some shutil functions are thin wrappers around corresponding os functions (chown(), get_terminal_size()), and more such wrappers can be added. You may want to add context managers for chown() and some future functions, but the name already taken. It is also confusing if shutil.chown() strikes immediately, but shutil.chdir() has no effect without with. So we need a name convention to clearly distinguish context managers which temporary change environment from ordinary functions which permanently change environment.

For example, test.support.os_helper has context managers change_cwd() and temp_umask().

[ncoghlan (Alyssa Coghlan)]

Regarding the stream redirection APIs, there was more discussion of potential locations back in Issue 15805: Add stdout redirection tool to contextlib - Python tracker than I recalled in the previous thread (although shutil didn’t come up - only io and sys were seriously considered, since the concept of the proposal was more “redirecting where print calls write to without having to modify the print calls themselves”, rather than conceiving of the problem as letting shell programmers do the same kind of scripting as they would do in the operating system’s native shell).

So I’m going to say “definitely not worth moving” when it comes to those.

That leaves chdir, and I was already dubious on the value of moving that even before @storchaka raised the following point:

@storchaka

It is also confusing if shutil.chown() strikes immediately, but shutil.chdir() has no effect without with.

The fact that shutil already has thin wrappers around os APIs with just some ergonomic enhancements (accepting user and group names in the case of chown) is another argument against reopening the discussion around the chosen location for contextlib.chdir.

It’s far more borderline than anything else that is likely to come up in the future, and adding shutil.temp_umask ^[1] instead of contextlib.umask will help emphasise that contextlib.chdir is an exceptional case rather than a meaningful precedent.

---

1. Since Serhiy’s comment is also relevant to the umask PR, I’ve suggested changing the name of that addition from shutil.umask to shutil.temp_umask. ↩︎

[jb2170]

That’s a really nice way of doing it. Looking at some other modules like Lib/calendar.py and Lib/shutil.py that’s the style they use (__getattr__).

Using global chdir within __getattr__ to set chdir to shutil.chdir also causes the DeprecationWarning to print only once, which is less annoying than every time chdir is accessed.

def __getattr__(name):
    if name == 'chdir':
        global chdir
        import warnings
        from shutil import chdir as _chdir
        warnings.warn('contextlib.chdir has been relocated to shutil.chdir',
                      DeprecationWarning, stacklevel=2)
        chdir = _chdir
        return chdir

    raise AttributeError(f"module {__name__!r} has no attribute {name!r}")

[jb2170]

@storchaka

So we need a name convention to clearly distinguish context managers

Agreed. I’ve proposed umask_of for the umask context manager in the previous thread over temp_umask for a few reasons, most of all because it is idiomatic to read:

with umask_of(0o077):
    do_private_stuff()

@storchaka

change_cwd

To me change_cwd is too different a name from the chdir that it wraps. Just as umask_of fits the pattern ${WRAPPED}_of, perhaps chdir_of could be an acceptable name.

with chdir_of("/etc"):
    edit_config_files()

If ‘of’ seems like the wrong preposition for chdir_of perhaps ${WRAPPED}_{PREPOSITION} could be the defacto pattern, eg chdir_into.

with chdir_into("/etc"):
    edit_config_files()

[jb2170]

Now, if ever, is definitely the time to move contextlib.chdir to shutil.chdir_{of|into}.

With the documentation of shutil freshly re-organised to accommodate umask_of, its ‘_shutil-context-managers’ section would look a bit bare without chdir_{of|into}. There is also now an overlap in the documentation of contextlib and shutil, which is resolved by chdir’s move.

This is really part (2/2) of the umask-chdir pull requests, as they go together

[jb2170]

It’s ready here! (as a PR for discussion).

I went with chdir_of for the sake of consistency and simplicity. If another name is desired we can discuss.

The move-chdir branch is git (re)based on my add-umask branch, since the documentation style for chdir_of is conditional on that which is introduced by umask_of. Hence why the link is to my fork of the cpython repo.

[ncoghlan (Alyssa Coghlan)]

I’m still against the idea of programmatically deprecating contextlib.chdir in the near term. Maybe document it as deprecated and suggest importing the equivalent functionality from shutil instead, but the earliest that programmatic deprecation should be considered is when the last version without the shutil interface is no longer supported (which would be 3.13 assuming the shutil interface is added in 3.14, so it would drop out of support when 3.18 was released).

Regarding chdir as the verb form vs cwd as the noun form, that distinction already exists in the os module, due to os.getcwd() being the query counterpart to os.chdir(dirpath). cwd also shows up as a parameter name in the os.startfile Windows-only API, as well as in the cross-platform subprocess APIs.

I do like the idea of a common naming convention for these kinds of CM wrappers, but given the umask and chdir examples, my preferred convention would actually be ensure_{NOUN}, with the name being based on the related query API, rather than the modification API.

For umask, the modification API is the query API, so it’s ensure_umask either way.
For chdir, the corresponding query API is getcwd, so that convention gives ensure_cwd.

Until contextlib.chdir is actively deprecated (in 3.18 or so), the implementation of the latter can just be:

from contextlib import chdir as _ensure_cwd_impl

class ensure_cwd(_ensure_cwd_impl):
    pass

Alternatively, the implementation could be moved to shutil immediately, and the __getattr__ approach could be added to contextlib without the programmatic deprecation warning^[1]:

def __getattr__(name):
    global chdir

    if name == 'chdir':
        # This will be deprecated once 3.18 is released and 3.13 is no longer supported
        # (as all still supported versions will provide shutil.ensure_cwd at that point)
        from shutil import ensure_cwd as chdir
        return chdir

    raise AttributeError(f"module {__name__!r} has no attribute {name!r}")

While the latter technically breaks pickle compatibility (since only targets with shutil.ensure_cwd will be able to unpickle the result), this isn’t the kind of object that anyone is likely to try to transmit or save via pickle.

---

1. import statements (with or without as clauses) can bind global names directly, so there’s no need for a separate local variable name. ↩︎

[jb2170]

Based on the suggestion of umask_context in the previous thread, chdir_context or cwd_context would follow the pattern. I’d suggest chdir_context as this makes it clear the context manager changes directory, whereas the c in cwd makes it sound like we’re passing the current directory around.

As with ensure_umask, ensure_cwd sounds like too misleadingly like a assert os.getcwd() == path function, so I’d like to rule that out.

(I know we’re going back and forth a lot about the naming convention but it’s because we care )

As for the method of deprecation details I’d like to continue with the implementation of chdir_context being moved to shutil. Inside contextlib I like your suggested

def __getattr__(name):
    global chdir

    if name == 'chdir':
        # This will be deprecated once 3.18 is released and 3.13 is no longer supported
        # (as all still supported versions will provide shutil.chdir_context at that point)
        from shutil import chdir_context as chdir
        return chdir

    raise AttributeError(f"module {__name__!r} has no attribute {name!r}")

Though wouldn’t it be best to keep a deprecation warning that a developer will see when writing / updating / running their code, that prompts them to make the switchover, or is this somthing to do with Python versions that receive bug fixes but not new features or something?

Right now I’ll change the name to chdir_context.

[sirosen (Stephen Rosen)]

The proposed (no runtime warning) path gives the best experience for libraries which maintain compatibility with multiple Python versions and run their testsuites with warnings treated as errors.

It’s a question of what you want the upgrade experience to be for those users. With an immediate warning, they have to add a version-based dispatch. It’s easy and we end up doing it a lot, especially with particular libraries like importlib and typing. But it’s even better to not need to do that, and to be able to simply cut over to the new name in 3.18.

[jb2170]

Okay, so no warning, got it: When the sliding window of supported Python versions fully encompasses shutil.chdir_context’s existence, ie when versions <=3.13 are no longer supported and everyone is on >=3.14, which introduces shutil.chdir_context, that’s when the contextlib.chdir support can be withdrawn. That’s less intrusive all-round isn’t it.

[jb2170]

Proposed PR (rebased on Add shutil.umask_context). I’ll add a News entry and What’sNew.

Issue gh-128499.

Other than in Lib/test/ there are two existing uses of contextlib.chdir in the cpython repo, in Tools/wasm/{emscripten/__main__.py,wasi.py}. Should they still use the legacy import until 3.13 is dropped, or can they switch to the new shutil.chdir_context import right away as I’ve updated them to, because this is going in the main branch, and those are just tools for building Python?

[jb2170]

Still debating the name in the previous thread. substitute_cwd is a candidate.

[effigies (Chris Markiewicz)]

I’ve had an InDirectory() context manager forever, which I have deprecated in favor of contextlib.chdir. If you’re considering a new name, in_directory, in_cwd, or maybe using_cwd?

[xitop]

Theasurus suggests also:

with interim_umask(0o700): ...

with interim_cwd('/tmp'): ...

[barry (Barry Warsaw)]

My thoughts FWIW.

[ncoghlan (Alyssa Coghlan)]

@erlendaasland and I just rejected the proposal to add the umask context manager (see my issue closure message for the immediate rationale, as well as the comments in the other thread and on the linked PR).

Given that rejection, we can also once again kick the “contextlib.chdir is really in a weird spot” discussion into the future (it’s still weird, but the discrepancy is less glaring when there aren’t other process state management context managers available in shutil).