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

pre-push should forbid unrelated staged changes while pushing #2486

Open
bnchou opened this issue Aug 16, 2022 · 7 comments · May be fixed by #3113
Open

pre-push should forbid unrelated staged changes while pushing #2486

bnchou opened this issue Aug 16, 2022 · 7 comments · May be fixed by #3113
Labels

Comments

@bnchou
Copy link

bnchou commented Aug 16, 2022

search tried in the issue tracker

yes

describe your issue

I configured dotnet format as push hook in my .pre-commit-config.yaml
I installed them using

pre-commit install --install-hooks --hook-type pre-commit --hook-type pre-push

I committed a modification with dotnet format not following correct rules, it passes
I try to push, it is rejected with error

 error: failed to push some refs to

And it corrects my file after dotnet format
When I try to push again, I have the same error.

When I add the file in staged changes without commit it, I can push.
I expect to be rejected even if I put the correct file in staged changes.

pre-commit --version

pre-commit 2.20.0

.pre-commit-config.yaml

default_stages: [commit]
repos:
  - repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v4.1.0
    hooks:
      - id: check-added-large-files
      - id: check-case-conflict
  - repo: local
    hooks:
    - id: dotnet-format
      name: dotnet-format
      language: system
      entry: dotnet format --include
      types_or: ["c#"]
      stages : [push]

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

No response

@asottile
Copy link
Member

you'll have to provide more information and a reproduction

@asottile
Copy link
Member

also show the full output, do not redact anything

@bnchou
Copy link
Author

bnchou commented Aug 16, 2022

Not related to dotnet format
.pre-commit-config.yaml

default_stages: [commit]
repos:
  - repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v4.1.0
    hooks:
      - id: check-added-large-files
      - id: check-case-conflict
      - id: check-merge-conflict
      - id: check-symlinks
      - id: check-yaml
        files: .*\.(yaml|yml)$
      - id: detect-private-key
      - id: end-of-file-fixer
      - id: mixed-line-ending
      - id: trailing-whitespace
        stages: [push]

Installation

 pre-commit install --install-hooks --hook-type pre-commit --hook-type pre-push

Output:

pre-commit installed at .git/hooks/pre-commit
pre-commit installed at .git/hooks/pre-push

Create a file with some whitespaces at end

spacestotrail        

Add and commit the file

git add spaces.txt 
git commit -m "Add spaces"

Output

check for added large files..............................................Passed
check for case conflicts.................................................Passed
check for merge conflicts................................................Passed
check for broken symlinks............................(no files to check)Skipped
check yaml...........................................(no files to check)Skipped
detect private key.......................................................Passed
fix end of files.........................................................Passed
mixed line ending........................................................Passed
[features/pre-commit-issue 9768261] Add spaces
 1 file changed, 1 insertion(+)
 create mode 100644 spaces.txt

Try to git push

fix end of files.........................................................Passed
trim trailing whitespace.................................................Failed
- hook id: trailing-whitespace
- exit code: 1
- files were modified by this hook

Fixing spaces.txt

error: failed to push some refs to '*****'

Add the spaces.txt in staged to commit but don't commit anything

git add spaces.txt 

Try to git push

fix end of files.........................................................Passed
trim trailing whitespace.................................................Passed
Enumerating objects: 4, done.
Counting objects: 100% (4/4), done.
Delta compression using up to 4 threads
Compressing objects: 100% (2/2), done.
Writing objects: 100% (3/3), 295 bytes | 98.00 KiB/s, done.
Total 3 (delta 1), reused 0 (delta 0), pack-reused 0
remote: Analyzing objects... (3/3) (91 ms)
remote: Storing packfile... done (86 ms)
remote: Storing index... done (48 ms)
To ****
   52c39c7..9768261  features/pre-commit-issue -> features/pre-commit-issue

It succeeds to push with spaces at the end

@asottile
Copy link
Member

hmmm interesting -- the "unstaged changes" check I guess really only makes sense for commit -- it should additionally reject push if there's staged things

@asottile asottile added the bug label Aug 17, 2022
@asottile asottile changed the title dotnet format pre-push hook allow push when correct file is in staged changes pre-push should forbid unrelated staged changes while pushing Aug 17, 2022
@pre-commit pre-commit deleted a comment from ShafinKhadem Oct 25, 2022
@asottile

This comment was marked as off-topic.

@JosephRangel
Copy link

is this issue solved?

@asottile
Copy link
Member

is the issue closed?

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

Successfully merging a pull request may close this issue.

3 participants