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

From: Andrew Keller <andrew@kellerfarm.com>
Date: Aug 5, 2016, 13:22 UTC
Message-ID: <B126EBED-AA93-4B5B-A932-149E4CB88C2B@kellerfarm.com>
In-Reply-To: <xmqq60rg5vq5.fsf@gitster.mtv.corp.google.com>

Am 04.08.2016 um 12:45 nachm. schrieb Junio C Hamano <gitster@pobox.com>:

Show 28 quoted lines

> Andrew Keller <andrew@kellerfarm.com> writes:
> 
>> 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.
> 
> Yeah, I agree with your argument from the usability and safety point
> of view.
> 
>> 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?
> 
> It is clear that our stance has been the third one so far.
> 
> Another thing I did not see in your analysis is what happens if the
> user is doing a partial commit, and how the changes made by
> pre-commit hook is propagated back to the main index and the working
> tree.
> 
> The HEAD may have a file with contents in the "original" state, the
> index may have the file with "update 1", and the working tree file
> may have it with "update 2".  After the commit is made, the user
> will continue working from a state where the HEAD and the index have
> "update 1", and the working tree has "update 2".  "git diff file"
> output before and after the commit will be identical (i.e. the
> difference between "update 1" and "update 2") as expected.

Excellent point — one I had discovered myself but neglected to include in my email. In my post-commit hook, I have logic in both versions of my experiment that disallows [1] fixing up diffs that are partially staged. Both scripts then update both the index and the working copy. (Sort of like how rebase works — clean working directory required, and then it updates the index and the work tree)

[1] In version #1, if any files it wants to change are partially staged, it
    prints a detailed error message and aborts the commit outright.  In
    version #2, the pre-commit hook sees the change it _wants_ to make,
    informs the user that he/she should run the fixup command, aborts
    the commit, and when the user runs the fixup command, the fixup
    command sees the partially staged file, prints the same detailed error
    message, and dies.

Thanks for your help on this. it’s really been interesting. I’ll leave it as-is for now.

Thanks,
 - Andrew Keller