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

> On Sun, Oct 27, 2013 at 06:42:44AM +0100, Michael Haggerty wrote:
>> On 10/27/2013 02:34 AM, Josh Triplett wrote:
>> [...]
>> First of all, let me show my ignorance.  How formalized is the use of
>> metadata lines at the end of a commit message?  I don't remember seeing
>> documentation about such lines in general (as opposed to documentation
>> about particular types of lines).  Is the format defined well enough
>> that tools that don't know about a particular line could nonetheless
>> preserve it correctly?  Is there/should there be a standard recommended
>> order of metadata lines?  (For example, should "Fixes:" lines always
>> appear before "Signed-off-by" lines, or vice versa?)  If so, is it
>> documented somewhere and preserved by tools when such lines are
>> added/modified?  Should there be support for querying such lines?
> 
> While it isn't very well documented in git itself, metadata lines are
> quite standardized.  See Documentation/SubmittingPatches and
> Documentation/development-process/5.Posting in the Linux kernel, for an
> explanation of "Reported-by:", "Tested-by:", "Reviewed-by:",
> "Suggested-by:", and "Acked-by:".  And git itself looks for a very
> specific format; the has_conforming_footer function looks for a footer
> consisting exclusively of rfc2822-style (email-style) header lines to
> decide whether to append "Signed-off-by:" (and now "Fixes:") directly to
> that block or to create a new block.

> [...]
>> 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.

> [...]
>> 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.