Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ValueError on Windows when config is on a different drive than the git repo #2530

Open
SitiSchu opened this issue Sep 28, 2022 · 4 comments · May be fixed by #2531
Open

ValueError on Windows when config is on a different drive than the git repo #2530

SitiSchu opened this issue Sep 28, 2022 · 4 comments · May be fixed by #2531

Comments

@SitiSchu
Copy link

SitiSchu commented Sep 28, 2022

search tried in the issue tracker

is:issue relpath

describe your issue

pre-commit will fail with ValueError: path is on mount 'C:', start on mount 'D:' when the config file is on C:\ but the git repository is on D:\

Steps to reproduce:

  • Create config file on your C: drive (for example %USERPROFILE%\.pre-commit-config.yaml)
  • Create git repo on D: drive
  • Run pre-commit: pre-commit run --config=%USERPROFILE%\.pre-commit-config.yaml

Commenting out line 164 in main.py (args.config = os.path.relpath(args.config)) fixes this issue. Is it necessary that the config path is relative to the working directory?

pre-commit --version

pre-commit 2.20.0

.pre-commit-config.yaml

repos:
-   repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v2.3.0
    hooks:
    -   id: trailing-whitespace

~/.cache/pre-commit/pre-commit.log (if present)

version information

pre-commit version: 2.20.0
git --version: git version 2.36.1
sys.version:
    3.9.9 (tags/v3.9.9:ccb0e6a, Nov 15 2021, 18:08:50) [MSC v.1929 64 bit (AMD64)]
sys.executable: D:\temp\prctest\venv\Scripts\python.exe
os.name: nt
sys.platform: win32

error information

An unexpected error has occurred: ValueError: path is on mount 'C:', start on mount 'D:'
Traceback (most recent call last):
  File "D:\temp\prctest\venv\lib\site-packages\pre_commit\error_handler.py", line 73, in error_handler
    yield
  File "D:\temp\prctest\venv\lib\site-packages\pre_commit\main.py", line 343, in main
    _adjust_args_and_chdir(args)
  File "D:\temp\prctest\venv\lib\site-packages\pre_commit\main.py", line 164, in _adjust_args_and_chdir
    args.config = os.path.relpath(args.config)
  File "D:\local_software\PYTHON\3.9.9\Windows_NT.x64\lib\ntpath.py", line 703, in relpath
    raise ValueError("path is on mount %r, start on mount %r" % (
ValueError: path is on mount 'C:', start on mount 'D:'
@asottile
Copy link
Member

it's not necessary that it's a relative path, but it does make error messages better

feel free to send a patch

@SitiSchu
Copy link
Author

I'd be open to creating a Pull Request for this. Which option would you prefer?

  1. Remove the os.relpath completely -> Worse error messages
  2. Wrap the os.relpath in a try/except -> pythonic, will keep error messages the same as they are right now except in this edge case
  3. Compare the drive of the two paths using os.path.splitdrive before using os.relpath, effectively 2. but not using try/except

@asottile
Copy link
Member

3 sounds error prone 1 sounds bad so probably something like 2

@SitiSchu SitiSchu linked a pull request Sep 28, 2022 that will close this issue
SitiSchu added a commit to SitiSchu/pre-commit that referenced this issue Sep 28, 2022
m-rsha pushed a commit to m-rsha/pre-commit that referenced this issue Oct 30, 2022
@Donabbaskid

This comment was marked as spam.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

3 participants