Skip to content

Relative paths resolved without changing the working directory - #979

Merged
mauvilsa merged 2 commits into
mainfrom
avoid-chdir
Sep 17, 2026
Merged

mauvilsa merged 2 commits into
mainfrom
avoid-chdir

Conversation

@mauvilsa

@mauvilsa mauvilsa commented Sep 17, 2026

Copy link
Copy Markdown
Owner

What does this PR do?

Relative paths in a config file are resolved with respect to the directory of that file. This was done by temporarily changing the process working directory with os.chdir, which is process-global state, with several consequences, most visible for configs in temporary directories:

  • Removing the original working directory while a config was loading made restoring it raise FileNotFoundError.
  • os.getcwd() resolves symlinks, so a config under a symlinked directory gave back paths the user never wrote.
  • Parsing configs concurrently in threads resolved paths against each other's directories or failed outright.

Changes:

  • _paths.py: change_to_path_dir is renamed to path_dir_context, since it no longer changes any directory. It sets the new current_local_dir context variable, which Path.__init__ uses for local relative paths. Keeping it separate from _current_path_dir means a local path inside a remote config still resolves against the closest local directory.
  • _formatters.py: get_initial_working_directory is removed, the working directory is now always the initial one. _describe_origin falls back to the absolute path when the working directory is gone, so that a removed directory does not mask the parse error being reported.
  • _loaders_dumpers.py: jsonnet resolves import statements relative to the file name given to evaluate_snippet, previously found through the working directory, now anchored on current_local_dir. Its base name is used, which also fixes from_config for a config given as a relative path with a directory, e.g. sub/config.jsonnet, broken the same way before this change.
  • typing.py: a path dumped with path_dump_preserve_relative is reloaded by passing its stored cwd to the Path constructor, instead of the context manager, which required that directory to exist.

Code that relied on the working directory, e.g. a custom type that opens a relative path, must now use a path type such as Path_fr. A class_path naming a module next to the config file also stops working, since that resolved through the working directory; it was never a documented feature, so it can be added back on request. The migration guide lists this as breaking.

Not addressed: a working directory removed before parsing starts still raises FileNotFoundError, even for an absolute config path.

Before submitting

  • Did you read the contributing guideline?
  • If you used a coding agent, did you fully understand and validate all generated code and ensure it follows the contributing guidelines?
  • Did you update the documentation? (readme and public docstrings)
  • Did you write unit tests such that there is 100% coverage on related code? (required for bug fixes and new features)
  • Did you verify that new and existing tests pass locally?
  • If this is a bug fix, did you verify that the tests fail without the code fix?
  • Did you make sure that all changes preserve backward compatibility?
  • Did you update the CHANGELOG including a pull request link? (not for typos, docs, test updates, or minor internal changes/refactors)

@mauvilsa mauvilsa added bug Something isn't working refactor Improvements to the quality of the code labels Sep 17, 2026
Comment thread jsonargparse_tests/test_paths.py Dismissed
Comment thread jsonargparse_tests/test_paths.py Dismissed
Comment thread jsonargparse_tests/test_paths.py Dismissed
Comment thread jsonargparse_tests/test_paths.py Dismissed
Comment thread jsonargparse_tests/test_paths.py Dismissed
Comment thread jsonargparse_tests/test_paths.py Fixed
Comment thread jsonargparse_tests/test_paths.py Dismissed
Comment thread jsonargparse_tests/test_paths.py Dismissed
Comment thread jsonargparse_tests/test_paths.py Dismissed
Comment thread jsonargparse_tests/test_paths.py Dismissed
@codecov

codecov Bot commented Sep 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (b8c053c) to head (d9594b4).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #979   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           27        27           
  Lines         9079      9077    -2     
=========================================
- Hits          9079      9077    -2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@greptile-apps

This comment was marked as outdated.

Comment thread jsonargparse/_paths.py Outdated
Comment thread jsonargparse/_formatters.py Outdated
Comment thread jsonargparse/_loaders_dumpers.py Outdated
- Remove the sys.path modification from path_dir_context. It races through
  process-global state, and a class_path naming a module next to the config file
  was never a documented feature.
- Don't mask a parse error with a FileNotFoundError when the working directory
  has been removed, by falling back to the absolute path in _describe_origin.
- Anchor the jsonnet snippet name on the base name, so that from_config resolves
  imports for a config given as a relative path with a directory.
- Skip test_relative_path_context_cwd_removed in windows, where the working
  directory can't be removed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

@mauvilsa
mauvilsa merged commit 53947c6 into main Sep 17, 2026
32 checks passed
@mauvilsa
mauvilsa deleted the avoid-chdir branch September 17, 2026 05:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working refactor Improvements to the quality of the code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants