search

guillaumeaubert / App-GitHooks

Plugin-based system to run specific actions and checks when git hooks are triggered.
https://metacpan.org/pod/App::GitHooks
Other
6 stars 3 forks source link

Broken handling of commits with "git mv" (Plugins) #38

Open sjn opened 8 years ago

sjn commented 8 years ago

Hi!

A bunch of plugins are struggling when files are renamed in git.

e.g. when doing a git mv test.pl test-001.pl, the resulting githooks output from git commit is

(1/1) test.pl   test-001.pl
    × The file has no #NOCOMMIT tags.
        fatal: cannot stat path 'test.pl        test-001.pl': No such file or directory at /home/e220025/perl5/lib/perl5/Git/Repository/Plugin/Blame.pm line
        109.
    × The file passes perl -c
        Can't open perl script "/home/e220025/src/test/test.pl  test-001.pl": No such file or directory
    × The file passes Perlcritic review.
        Failed to run PerlCritic: Argument 'file' is not a valid file path at /home/e220025/perl5/lib/perl5/App/GitHooks/Plugin/PerlCritic.pm line 119.
        .
    × The Perl interpreter line is correct
        Can't open '/home/e220025/src/test/test.pl      test-001.pl' for reading: 'No such file or directory' at
        /home/e220025/perl5/lib/perl5/App/GitHooks/Plugin/PerlInterpreter.pm line 126

Note: I haven't checked with all plugins available on CPAN, but seeing the symptoms are similar across several plugins make me assume this is a bug in App::Githooks somewhere, rather in the bugs themselves.

Thanks for some great modules, btw! :)

guillaumeaubert commented 8 years ago

I did some research, and I think you either have diff.renames explicitly set to true, or a very recent version of git (diff.renames got set to true by default in v2.9: https://github.com/git/git/commit/5d2a30d7d8777319c745804f040fa405d02169ce).

The underlying command used by App::GitHooks is git diff --cached --name-status -- .. I was able to replicate the issue on git v2.1.4 by adding -M.

App::GitHooks currently treats renames as a deletion and an addition:

D test1.txt
A test2.txt

The git change notes potential renames along with a similarity % in the output:

R100  test1.txt test2.txt

I'm inclined to create a new internal R git action when similarity=100 (StagedChanges.pm), and to treat anything below as separate Delete and Addition operations. This would maximize backward compatibility for the plugin ecosystem, while letting plugins make better decisions around moving operations that do not affect the content of files. For example, you may not want to check for trailing whitespace during a standalone file rename, something PreventTrailingWhitespace can't currently support without this change.

sjn commented 8 years ago

Sounds good to me! :)