Re: obsolete index in wt_status_print after pre-commit hook runs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 15.07.2016 um 6:03 nachm. schrieb Junio C Hamano <gitster@xxxxxxxxx>:
> 
> Ahh, I misremembered.  2888605c (builtin-commit: fix partial-commit
> support, 2007-11-18) does consider the possibility that pre-commit
> may have modified the index contents after we take control back from
> that hook, so that is probably a good place to enumerate what got
> changed.  Getting the list before running the hook can give an
> out-of-date list, as you said.

I’ve been experimenting with two different workflows recently:

    (1) Identify problem files during the pre-commit hook;
        when found, fix them automatically in the index and let the commit continue.
    (2) Identify problem files during the pre-commit hook;
        when found, provide instructions to fix the problem (and possibly set a helpful
        Git alias to do it in one command), and abort the commit.  Require that the user fixup
        the index and try the commit again.

And here are my thoughts:

#1 seems to be quick and simple for the user, and it plays (mostly) nice with scripts
and IDEs that do commits autonomously, but I’m having trouble trusting that my
pre-commit hook made the *correct* changes (even though it’s worked nicely so far)
(i.e., I keep looking at the new HEAD commit to make sure it looks right, where
normally I just look at the index and make sure it looks right).

#2 is slightly more difficult to implement just because it has more moving parts,
however I’m finding that because I can interrogate the index after I manually run
the command to make the required changes to the index, and *before* I commit
again, I feel much more confident that I know what is going to be in my commit.
However, this approach doesn’t play well with automated scripts that assume that
a commit operation will always work.

In summary, I think I prefer #2 from a usability point of view, however I’m having
trouble proving that #1 is actually *bad* and should be disallowed.

Any thoughts?  Would it be better for the pre-commit hook to be officially allowed to
edit the index [1], or would it be better for the pre-commit hook to explicitly *not* be
allowed to edit the index [2], or would it be yet even better to simply leave it as it is?

[1] and possibly create a patch that teaches builtin/commit.c to reread the index
    after the pre-commit hook runs and before rendering the commit message template
[2] and possibly create a patch that teaches builtin/commit.c to detect changes to the
    index after the pre-commit hook runs

Thanks,
 - Andrew Keller

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]