sds / overcommit

A fully configurable and extendable Git hook manager
MIT License
3.92k stars 281 forks source link

Overcommit's precommit hook fumbles deleted/renamed files. #699

Open sleekweasel opened 4 years ago

sleekweasel commented 4 years ago

overcommit 0.47.0

This commit:

Changes to be committed:
  (use "git reset HEAD <file>..." to unstage)

        modified:   rake/rake_android.rb
        modified:   rake/rake_ios.rb
        modified:   rake/rake_mobileweb.rb
        modified:   shared/rake/teamcity.rb
        new file:   shared/rake/test/test_selection_ios_spec.rb
        deleted:    shared/rake/test/test_selection_mapper2_spec.rb
        renamed:    shared/rake/test/test_selection_mapper_spec.rb -> shared/rake/test/test_selection_merger_spec.rb
        modified:   shared/rake/test_run.rb
        renamed:    shared/rake/test_selection.rb -> shared/rake/test_selection_cal.rb
        renamed:    shared/rake/test_selection_mapper.rb -> shared/rake/test_selection_ios.rb
        new file:   shared/rake/test_selection_merger.rb

fails when running rspec tests because it still sees the moved/deleted files:

-rw-r--r-- 1 tim tim  3817 Dec 17 15:08 ./shared/rake/test_selection_cal.rb
-rw-r--r-- 1 tim tim 10924 Dec 17 16:13 ./shared/rake/test_selection_ios.rb
-rw-r--r-- 1 tim tim 14605 Dec 17 16:30 ./shared/rake/test_selection_mapper.rb
-rw-r--r-- 1 tim tim  3924 Dec 17 16:03 ./shared/rake/test_selection_merger.rb
-rw-r--r-- 1 tim tim  3766 Dec 17 16:30 ./shared/rake/test_selection.rb
-rw-r--r-- 1 tim tim  9576 Dec 17 16:13 ./shared/rake/test/test_selection_ios_spec.rb
-rw-r--r-- 1 tim tim  5893 Dec 17 16:30 ./shared/rake/test/test_selection_mapper2_spec.rb
-rw-r--r-- 1 tim tim 16812 Dec 17 16:30 ./shared/rake/test/test_selection_mapper_spec.rb
-rw-r--r-- 1 tim tim 14075 Dec 17 16:11 ./shared/rake/test/test_selection_merger_spec.rb

The deleted/renamed files shouldn't be showing up.

Failures:

  1) Test selection mapper generates mapping counts
     Failure/Error:
    ...
     # ./shared/rake/test/test_selection_mapper_spec.rb:354:in `block (2 levels) in <top (required)>'

Finished in 0.85755 seconds (files took 1.59 seconds to load)
141 examples, 1 failure

Failed examples:

rspec ./shared/rake/test/test_selection_mapper_spec.rb:343 # Test selection mapper generates mapping counts
sleekweasel commented 3 years ago

I've just encountered this again. Renamed files are still showing up in two places:

  1. as an old version of the file under the old name - as an untracked file, and
  2. as the latest version of the file under the new name.

Obviously, check-in triggers should filter out untracked files anyway, and we're doing that now - fixing a typo in our script - but it's still rather confusing.

sds commented 3 years ago

Thanks for the report. Can you clarify which hook was demonstrating this behavior?

Furthermore, can you confirm the version of git and overcommit you are currently using? (since it's been a while since you originally opened)

bert-mccutchen commented 2 years ago

I'll add onto this @sds since the conversation stopped. It's any hook that uses applicable_files, which is pretty much all of them.

Put simply, applicable_files is including deleted files. So when you are using the JsonSyntax linter for example, it will try to open a deleted file and raise an error like so:

No such file or directory @ rb_sysopen - ~/sandbox/DELETE_ME.json
~/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/overcommit-0.58.0/lib/overcommit/hook/pre_commit/json_syntax.rb:11:in `read'
~/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/overcommit-0.58.0/lib/overcommit/hook/pre_commit/json_syntax.rb:11:in `block in run'
~/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/overcommit-0.58.0/lib/overcommit/hook/pre_commit/json_syntax.rb:9:in `each'
~/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/overcommit-0.58.0/lib/overcommit/hook/pre_commit/json_syntax.rb:9:in `run'
~/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/overcommit-0.58.0/lib/overcommit/hook/base.rb:47:in `block in run_and_transform'
~/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/overcommit-0.58.0/lib/overcommit/utils.rb:260:in `with_environment'
~/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/overcommit-0.58.0/lib/overcommit/hook/base.rb:47:in `run_and_transform'
~/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/overcommit-0.58.0/lib/overcommit/hook_runner.rb:161:in `run_hook'
~/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/overcommit-0.58.0/lib/overcommit/hook_runner.rb:97:in `block in consume'
~/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/overcommit-0.58.0/lib/overcommit/hook_runner.rb:94:in `loop'
~/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/overcommit-0.58.0/lib/overcommit/hook_runner.rb:94:in `consume'

I'm using Git 2.34.0 and Overcommit 0.58.0.

bert-mccutchen commented 2 years ago

Ahh, correction! It's not when you actually run git commit but when you run overcommit -r!