-
-
Notifications
You must be signed in to change notification settings - Fork 767
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
_has_unstaged_config
check reports false negative if the config has been symlinked in place
#2814
Comments
is there a problem? does it matter? |
I wouldn't call it a problem, an annoyance at most. Making unstaged changes to the |
ah ok I understand now -- I think I was missing the context initially -- would you be interested in writing a patch for this? should be a pretty easy test as well (there's a similar one for the existing behaviour) |
I think the former makes more sense -- the value of the passed in config is "important" for some commands like init-templatedir even with symlinks involved |
Yes, I'll try to push something out this week. Thanks for taking the time to read and understand the problem. |
the one other thing I thought about is it's going to fail in weird ways if the file is outside the repo -- so might have to think a little bit more about that (since I know some people do wild things like |
Can I try to write a patch for this issue? I am a newbie; however, I realized the problem and reproduced it locally. |
sure! |
Is it allowed for a config to be a symlink? |
I don't think there's anything preventing it today |
search you tried in the issue tracker
config symlink
describe your issue
If the
.pre-commit-config.yaml
file is modified,pre-commit
reports an error:This is not the case if
.pre-commit-config.yaml
is a symlink. In that case,pre-commit
only prints out a warningTo replicate the issue
Taking a quick look at the code, the quick-and-dirty option is to resolve symlinks in the config path with
os.path.realpath
inpre_commit.commands.run:_has_unstaged_config
. A more elegant solution would be to resolve the symlink inpre_commit.main:_adjust_args_and_chdir
.The first option seems safer since it will not affect anything outside that check, but the second option feels cleaner. I'm open to implementing either and submitting a PR.
pre-commit --version
git rev: 14fa43d (current HEAD)
.pre-commit-config.yaml
Valid for any config
~/.cache/pre-commit/pre-commit.log (if present)
No response
The text was updated successfully, but these errors were encountered: