Re: [PATCH v3] Re: git-am: fix maildir support regression: accept email file as patch

Nicolas Sebrecht [off-list ref] writes:

It is not about adding a new feature. It's about keeping compatibility
with maildir. The current version _rejects_ good patches.

You almost gave me a heart attack.  I wondered if the detection code, like
it did when it saw a stgit series, saw that the parameter was a directory
(in which case the original code blindly took it as a maildir), and added
an extra check to see each file in that specified directory is a mbox or
begins with "From: ", and reject it otherwise.

If it were what Giuseppe's patch did, clearly that would have been a
breakage that rejects a good maildir.  So I looked at the code again.

A very early part of check_patch_format says "a directory?  we say it is
mbox (even though we should call it maildir) and let split_patches() call
git-mailsplit, as it knows how to deal with a maildir".

So I do not think there is any breakage that rejects good input with his
patch.

I am not opposed to add support for individual pieces of e-mail without
forcing them to be in Berkeley mbox format.  Not everybody uses mbox
format, and it is a logical thing to do.  Also I do not think the amount
of new code necessary to do so is excessive, nor such a change is risky
even late in a cycle after -rc0.

I however _do_ have issues with labeling other's patch that did not break
any documented behaviour as a regression, even if it is to get extra
attention to the issue.  That's not how we do things.

The 15/07/09, Junio C Hamano wrote:

Nicolas Sebrecht [off-list ref] writes:

So I do not think there is any breakage that rejects good input with his
patch.

Exact. It broke my workflow (at least) and was able to do it because the
previous git-am was permiting it. Looking closer to the code made me
understand where I was wrong.

I am not opposed to add support for individual pieces of e-mail without
forcing them to be in Berkeley mbox format.  Not everybody uses mbox
format, and it is a logical thing to do.  Also I do not think the amount
of new code necessary to do so is excessive, nor such a change is risky
even late in a cycle after -rc0.

Will do, then.

I however _do_ have issues with labeling other's patch that did not break
any documented behaviour as a regression, even if it is to get extra
attention to the issue.  That's not how we do things.

Of course. As we was able to do more than documented, I did not see it
in first place. It's a wrong assumption coming from my initial
git-bisect, some "hidden" globbing shell, and my learning curve of the
current code.

It is _NOT_ in my intention to blame anyone.

-- 
Nicolas Sebrecht