Re: More builtin git-am issues..

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

 



On Fri, Sep 04, 2015 at 04:52:42PM -0700, Linus Torvalds wrote:

> On Fri, Sep 4, 2015 at 4:47 PM, Linus Torvalds
> <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
> >
> > I *think* it's this part:
> >
> >                 if (!(found_rfc2822 ||
> >                       is_cherry_picked_from_line(buf + i, k - i - 1)))
> >                         return 0;
> >
> > which basically returns 0 for _any_ line in the footer that doesn't
> > match the found_rfc2822 format.
> 
> Confirmed. I hacked up a version that just doesn't do that check at
> all, and it works fine (but obviously only on well-formatted emails
> that really do have a sign-off).
> 
> So I think that logic should basically be extended to saying
> 
>  - if any line in the last chunk has a "Signed-off-by:", set a flag.
> 
>  - at the end of the loop, if that flag wasn't set, return 0.

Do we want to make Signed-off-by a special token here? What about "Cc:",
and other project-specific trailers? You wouldn't want to end up with:

  [some comment]
  Cc: somebody

  Signed-off-by: somebody else

It's not a problem for git-am, which should be taking in patches that
are already signed-off (and after all, if your project does not use
signoffs, why are you using "am -s"?). But won't "git commit -s" run
into the same problem?

We could look for an arbitrary rfc2822-ish string, but I'd be worried
slightly about false positives, like:

  This is a paragraph with arbitrary text. But one
  of the lines will use a colon, and that a causes a
  problem: because of wrapping, this line kind of looks
  like a trailer.

Maybe only the final line needs to look rfc2822-ish? Or maybe we can
constrain the content that _doesn't_ look like a trailer line. Is it
sufficient to allow a block encased in "[]", rather than arbitrary text?

All that being said, I think we can punt on the harder issues for now,
and just restore the original git-am.sh behavior (which it looks
like was fairly naive) for the upcoming release, so that there is no
regression.

-Peff
--
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]