phpro / grumphp

A PHP code-quality tool
MIT License
4.13k stars 430 forks source link

grumphp deletes entire repo contents with ignore_unstaged_changes: true #859

Closed dshafik closed 3 years ago

dshafik commented 3 years ago
Q A
Version GrumPHP 1.3.0
Bug? yes
New feature? no
Question? no
Documentation? no
Related tickets

My configuration

grumphp:
    ignore_unstaged_changes: true
    ascii:
        succeeded: ~
        failed: ~
    fixer:
        enabled: true
        fix_by_default: true
    parallel:
        enabled: true
        max_workers: 8
    hide_circumvention_tip: false
    tasks:
        phpversion:
            project: '7.3'
        phpstan:
            metadata:
                blocking: false
            use_grumphp_paths: false
            configuration: phpstan.neon.dist
        psalm:
            config: psalm.xml
            no_cache: true
            threads: 8
        phpcsfixer2:
            using_cache: true
            config: .php_cs.dist
            verbose: true

Steps to reproduce:

  1. Add ignore_unstaged_changes: true to your grumphp.yaml
  2. Edit two files
  3. Stage one file
  4. Commit

Result:

grumphp shows a notice that it is stashing changes, run the tasks successfully, and gives the notice it's restoring the stash. The commit succeeds.

All files in the repo are git rm'ed, git ls-files returns nothing.

You can see this in action up till the point grumphp outputs task output (sensitive). As soon as the tasks are finished it does the standard grumphp commit & unstash, and everything is deleted:

https://user-images.githubusercontent.com/58074/104081078-3e523b00-51e1-11eb-9efa-54be313b598e.mov

veewee commented 3 years ago

Hello @dshafik,

Thanks for reporting. Since this is a serious issue, I've tested this out locally. Here, it works as expected. The video you uploaded does not contain the rm'ed files. Can you tell me what is going on exactly after grumphp ran?

The flow of the ignore_unstaged_changes is like this when there are unstaged files during pre-commit:

So if everything succeeds, the unstaged files are available again. It is a mistory to me why all files from your existing git repo are removed.

Can you give me some more information?

dshafik commented 3 years ago

@veewee anywhere I can exchange the details privately? I have the full video but it contains private data.

veewee commented 3 years ago

Feel free to email me the information at Toon.verwerft@phpro.be

veewee commented 3 years ago

Some additional questions:

Can you consistently reproduce the issue? If so: what happens if you run the git stash commands above manually at the moment you are about to commit?

Can you also confirm that your git stash list doesn't contain other entries before committing?

dshafik commented 3 years ago

@veewee this happens every time I try it in my test branch. I do have other things in my stash, but nothing that should delete everything.

if I run the command git stash save --quit --keep-index grumphp-uniquestr it errors with:

error: unknown option `quit'`

Followed by the USAGE.

Using git version 2.24.3 (Apple Git-128) with hub version refs/heads/master (hub master as of ~2 weeks ago)

veewee commented 3 years ago

Ah woops, it is quiet instead of quit:

dshafik commented 3 years ago

Those commands work just fine. I have narrowed it down to grumphp git:pre-commit, going to dig a bit more

dshafik commented 3 years ago

I take that back, git stash save --quiet --keep-index grumphp-<uniquestring> is what does it. It's just not visible in git status until you git stash pop --quiet.

veewee commented 3 years ago

Ok, that looks like a git related issue to me.

You could debug it even further be checking what is inside git stash list. Are there multiple entries in there? You could drop those with git stash clear if you no longer need them. However, grumphp only does things on the stash that grumphp added. (unless another tool creates a stash in between)

You can also display the diff inside the last stash with git stash show.

dshafik commented 3 years ago

This is indeed a git bug! Unfortunately this is the default version of git that comes with Xcode 12, which many folks will be using on macOS Catalina/Big Sur.

It only specifically manifests when pop --quiet is used, is it possible you could switch to redirecting the output instead?

veewee commented 3 years ago

Nice catch!

Will have to play around with it a bit, but I think we can drop the --quiet flag, since we are not doing anything with the output. It will only be a bit noisier if you run grumphp in verbose mode, but that's not a big issue.

dshafik commented 3 years ago

@veewee thanks! And thanks for grumphp, it's enabled me to implement great commit hooks in like… hours, this issue not-withstanding :D

veewee commented 3 years ago

Fixed this in #861. Let me know if there is still something that doesn't work as expected.