fix: handle ValueError from relpath on Windows cross-drive paths#3659
fix: handle ValueError from relpath on Windows cross-drive paths#3659bobo-xxx wants to merge 1 commit intopre-commit:mainfrom
Conversation
When the config file (or other paths) is on a different Windows drive than the git repository, os.path.relpath raises a ValueError because it cannot compute a relative path between paths on different drives. This change wraps all relpath calls in _adjust_args_and_chdir with ValueError suppression, keeping the absolute path when relpath fails. Fixes pre-commit#2530
There was a problem hiding this comment.
Pull request overview
Fixes a Windows-specific failure where os.path.relpath() raises ValueError for cross-drive paths by falling back to absolute paths instead of crashing.
Changes:
- Add a
ValueErrorfallback forrelpathoperations performed during_adjust_args_and_chdir. - Introduce
_try_relpath()and use it for file path normalization. - Add a regression test covering the cross-drive
ValueErrorscenario.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
pre_commit/main.py |
Suppresses ValueError from os.path.relpath() during argument normalization and adds _try_relpath() helper. |
tests/main_test.py |
Adds a regression test ensuring _adjust_args_and_chdir does not fail when relpath raises ValueError. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # on windows, different drives raises ValueError | ||
| try: |
There was a problem hiding this comment.
Grammar in the comment: “different drives raises ValueError” should be “different drives raise ValueError”.
|
|
||
| def test_adjust_args_and_chdir_relpath_valueerror_cross_drive(in_git_dir): | ||
| # regression test for https://github.com/pre-commit/pre-commit/issues/2530 | ||
| # on windows, different drives raises ValueError from relpath |
There was a problem hiding this comment.
Grammar in the comment: “different drives raises ValueError from relpath” should be “different drives raise ValueError from relpath”.
| # on windows, different drives raises ValueError from relpath | |
| # on windows, different drives raise ValueError from relpath |
| with contextlib.suppress(ValueError): | ||
| # on windows, different drives raises ValueError | ||
| args.config = os.path.relpath(args.config) | ||
| if args.command in {'run', 'try-repo'}: | ||
| args.files = [os.path.relpath(filename) for filename in args.files] | ||
| args.files = [ | ||
| _try_relpath(filename) for filename in args.files | ||
| ] | ||
| if args.commit_msg_filename is not None: | ||
| args.commit_msg_filename = os.path.relpath( | ||
| args.commit_msg_filename, | ||
| ) | ||
| with contextlib.suppress(ValueError): | ||
| # on windows, different drives raises ValueError | ||
| args.commit_msg_filename = os.path.relpath( | ||
| args.commit_msg_filename, | ||
| ) | ||
| if args.command == 'try-repo' and os.path.exists(args.repo): | ||
| args.repo = os.path.relpath(args.repo) | ||
| with contextlib.suppress(ValueError): | ||
| # on windows, different drives raises ValueError | ||
| args.repo = os.path.relpath(args.repo) |
There was a problem hiding this comment.
_try_relpath() is only used for args.files, while args.config, args.commit_msg_filename, and args.repo each reimplement the same ValueError fallback with separate contextlib.suppress(...) blocks. Consider using _try_relpath() consistently for all relpath conversions here to reduce repetition and keep the behavior uniform.
|
ai slop is not acceptable |
Description
When the config file (or other paths) is on a different Windows drive than the git repository,
os.path.relpathraises aValueErrorbecause it cannot compute a relative path between paths on different drives.This change wraps all
relpathcalls in_adjust_args_and_chdirwithValueErrorsuppression, keeping the absolute path whenrelpathfails.Fixes #2530
Testing
Added a regression test
test_adjust_args_and_chdir_relpath_valueerror_cross_drivethat simulates the cross-driveValueErrorscenario.All existing tests pass (29 passed, 1 skipped on Linux).