Re: [PATCH] commit: Add -f, --fixes <commit> option to add Fixes: line

> On 10/27/2013 08:14 AM, Josh Triplett wrote:
>> On Sun, Oct 27, 2013 at 06:42:44AM +0100, Michael Haggerty wrote:
>>> On 10/27/2013 02:34 AM, Josh Triplett wrote:
>>> I wonder if the two features could
>>> be combined in some way?
>>>
>>> The main difference between the two features is how they are intended to
>>> be used: --fixup is to fix a commit that hasn't been pushed yet (where
>>> the user intends to squash the commits together), whereas --fixes is to
>>> mark a commit as a fix to a commit that has already been pushed (where
>>> the commits will remain separate).  But there seems to be a common
>>> concept here.
>>>
>>> For example, what happens if a --fixes commit is "rebase -i"ed at the
>>> same time as the commit that it fixes?  It might make sense to do the
>>> autosquash thing just like with a --fixup/--squash commit.  (Otherwise
>>> the SHA-1 in the "Fixes:" line will become invalid anyway.)
>>
>> Most definitely not, no, at least not without an explicit option to
>> enable that.  Consider the case of backporting a series of patches and
>> preserving the relative history of those patches, to make it easier to
>> match up a set of patches.  At most, it might be a good idea for
>> cherry-pick or similar to provide an updated Fixes tag for the new hash
>> of the older commit.  Personally, I'd argue against doing this even with
>> --autosquash.  I could see the argument for an --autosquash-fixes, but I
>> can't think of a real-world scenario where what would come up.
>>
>> Generally, if history is still editable, you should just squash in the
>> fix to the original commit, and if history is no longer editable (which
>> is the use case for "Fixes:" lines), the squash case simply won't come
>> up, offering little point to adding special support for that case.
>
> In your last paragraph you explain exactly why these two features are
> similar and why it is thinkable to make the way that they are handled
> depend on the context.  Exactly because one would never rebase a
> "Fixes:" commit and the commit it is fixing at the same time, they would
> never be squashed together.  And ISTM that in most cases whenever they
> *are* being rebased at the same time, then one would want to squash them
> together.  So it might be possible to mark both types of commits the
> same way and then squash/not squash them depending on the context and
> the --autosquash option.

>>> I see that there a consistency check that the --fixes argument is a
>>> valid commit.  But is there/should there be a check that it is an
>>> ancestor of the commit being created?  Is there/should there be a check
>>> that both of these facts remain true if the the commit containing it is
>>> rebased, cherry-picked, etc?
>>
>> That sounds like a nice future enhancement, sure.  I don't have any plans to
>> add such a check myself, though.  Also note that --fixup and --squash
>> don't have such a check either; if you want to add one, you should add
>> it for all three options at once.
>
> A hook-based solution could do this.  But a built-in "all-purpose"
> handler like "footer.Fixes.arg=commit", which was intended to be
> reusable, wouldn't be able to do such footer-specific extra work without
> having to create new special cases in git each time.