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

forbid-new-submodules is broken #609

Closed
m-khvoinitsky opened this issue Jun 3, 2021 · 3 comments
Closed

forbid-new-submodules is broken #609

m-khvoinitsky opened this issue Jun 3, 2021 · 3 comments
Labels

Comments

@m-khvoinitsky
Copy link
Contributor

m-khvoinitsky commented Jun 3, 2021

The hook does not have an explicit types section.
Default types: [file] does not match submodules (which are directories). It works only if you commit some regular file together with committing new submodule.
Specifying types: [directory] helps but not much: it works as a pre-commit but doesn't as pre-push or any other way when you specify --from-ref and --to-ref.
I'd suggest stopping ignoring provided by pre-commit list of files and check git status for provided files instead of diff.

@asottile
Copy link
Member

asottile commented Jun 3, 2021

@m-khvoinitsky in order to add a submodule .gitmodules needs to be edited -- are you sure about your assertions? can you describe what you're seeing instead of prescribing a solution? I suspect you've got an XY problem

@m-khvoinitsky
Copy link
Contributor Author

in order to add a submodule .gitmodules needs to be edited

That's correct if you're adding submodules intentionally. But if user tries to commit some another git repository inside current one by accident, git complains but obediently adds a "commit" (type 160000) entry in the tree. I was hoping that this hook would prevent such situations too.

I suspect you've got an XY problem

May be it is. If so, a separate hook which rejects type 160000 entries without corresponding records in .gitmodules suits my needs better. However, implementing proposed chages would make this hook useful in both usecases (which makes sense because rejecting "incomplete" submodule is still rejecting a submodule).
And the (separate) problem with --from-ref and --to-ref is still a problem — current hook is useless in CI.

@asottile
Copy link
Member

asottile commented Jun 3, 2021

feel free to take a stab if you think it's worth it, I'm just not sure you're solving something that's actually a problem

m-khvoinitsky added a commit to m-khvoinitsky/pre-commit-hooks that referenced this issue Jun 23, 2021
…s committed (without any other file); support --from-ref and --to-ref; fixes pre-commit#609
m-khvoinitsky added a commit to m-khvoinitsky/pre-commit-hooks that referenced this issue Jul 6, 2021
…s committed (without any other file); support --from-ref and --to-ref; fixes pre-commit#609
m-khvoinitsky added a commit to m-khvoinitsky/pre-commit-hooks that referenced this issue Jul 19, 2021
…s committed (without any other file); support --from-ref and --to-ref; fixes pre-commit#609
m-khvoinitsky added a commit to m-khvoinitsky/pre-commit-hooks that referenced this issue Jul 19, 2021
…s committed (without any other file); support --from-ref and --to-ref; fixes pre-commit#609
m-khvoinitsky added a commit to m-khvoinitsky/pre-commit-hooks that referenced this issue Jul 25, 2021
…s committed (without any other file); support --from-ref and --to-ref; fixes pre-commit#609
asottile pushed a commit to m-khvoinitsky/pre-commit-hooks that referenced this issue Aug 2, 2021
…s committed (without any other file); support --from-ref and --to-ref; fixes pre-commit#609
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants