threads / bug / 52089

[BUG] "--show-current-patch" return a mail instead of a patch

Subject: [BUG] "--show-current-patch" return a mail instead of a patch

## tl;dr

7 messages between Oct 22, 2019 and Oct 23, 2019.

replies: 6people: 3as markdown or json

Jerome Pouiller· Oct 22, 2019, 10:18 UTC · lore
Hello all,

I try to use "git am" to apply a patch sent using "git send-email". This patch does not apply properly. I try to use "git am --show-current-patch" to understand the problem. However, since original mail is encoded in quoted- printable, data returned by --show-current-patch is not a valid patch.

I expected that --show-current-patch would return decoded version of original mail or something that looks like the output of "git format-patch" or at least a valid patch. Thus, it could be processed with "git apply" or "patch".

Currently I run "git mailinfo" manually to get the patch, but it is not very handy.

(I use git version 2.20.1 from Debian buster)
Thank you,
-- 
Jérôme Pouiller
Junio C Hamano· Oct 23, 2019, 02:24 UTC · re: Jerome Pouiller · lore

Re: [BUG] "--show-current-patch" return a mail instead of a patch

Jerome Pouiller <Jerome.Pouiller@silabs.com> writes:
Show 6 quoted lines
> Hello all,
>
> I try to use "git am" to apply a patch sent using "git send-email". This
> patch does not apply properly. I try to use "git am --show-current-patch"
> to understand the problem. However, since original mail is encoded in quoted-
> printable, data returned by --show-current-patch is not a valid patch.

I agree that --show-current-patch is a misdesigned feature. We'd be doing a better service to our users if we documented that the patch and log message are found at .git/rebase-apply/{patch,msg} instead of trying to hide the path.

Unfortunately, it is likely that those who added that feature have built their tooling around it to depend on its output being the full e-mail message "am" was fed (and split by "git mailsplit"). So I do not think we will be changing the output to the patch file only.

But even then, the documentation can be fixed without any backward compatibility issues. Perhaps like this?

 Documentation/git-am.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
index 6f6c34b0f4..f63b70325c 100644
--- a/Documentation/git-am.txt
+++ b/Documentation/git-am.txt
@@ -172,7 +172,7 @@ default.   You can use `--no-utf8` to override this.
 	untouched.
 
 --show-current-patch::
-	Show the patch being applied when "git am" is stopped because
+	Show the entire e-mail message "git am" has stopped at, because
 	of conflicts.
 
 DISCUSSION
Jerome Pouiller· Oct 23, 2019, 08:49 UTC · re: Junio C Hamano · lore

Re: [BUG] "--show-current-patch" return a mail instead of a patch

On Wednesday 23 October 2019 04:24:58 CEST Junio C Hamano wrote:
Show 33 quoted lines
> Jerome Pouiller <Jerome.Pouiller@silabs.com> writes:
> > I try to use "git am" to apply a patch sent using "git send-email". This
> > patch does not apply properly. I try to use "git am --show-current-patch"
> > to understand the problem. However, since original mail is encoded in quoted-
> > printable, data returned by --show-current-patch is not a valid patch.
> 
> I agree that --show-current-patch is a misdesigned feature.  We'd be
> doing a better service to our users if we documented that the patch
> and log message are found at .git/rebase-apply/{patch,msg} instead
> of trying to hide the path.
> 
> Unfortunately, it is likely that those who added that feature have
> built their tooling around it to depend on its output being the full
> e-mail message "am" was fed (and split by "git mailsplit").  So I do
> not think we will be changing the output to the patch file only.
> 
> But even then, the documentation can be fixed without any backward
> compatibility issues.  Perhaps like this?
> 
>  Documentation/git-am.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
> index 6f6c34b0f4..f63b70325c 100644
> --- a/Documentation/git-am.txt
> +++ b/Documentation/git-am.txt
> @@ -172,7 +172,7 @@ default.   You can use `--no-utf8` to override this.
>         untouched.
> 
>  --show-current-patch::
> -       Show the patch being applied when "git am" is stopped because
> +       Show the entire e-mail message "git am" has stopped at, because
>         of conflicts.

I agree with you: I think that manpage and/or output of "git am" should mention ".git/rebase-apply/patch" (that is exactly what I was looking for).

Maybe documentation of --show-current-patch should be clarified with a note like "This option is mainly for internal purpose. If you want to get current patch, rely on .git/rebase-apply/patch".

-- 
Jérôme Pouiller
Denton Liu· Oct 23, 2019, 08:55 UTC · re: Jerome Pouiller · lore

Re: [BUG] "--show-current-patch" return a mail instead of a patch

Hi Jerome,
On Wed, Oct 23, 2019 at 08:49:41AM +0000, Jerome Pouiller wrote:
Show 38 quoted lines
> On Wednesday 23 October 2019 04:24:58 CEST Junio C Hamano wrote:
> > Jerome Pouiller <Jerome.Pouiller@silabs.com> writes:
> > > I try to use "git am" to apply a patch sent using "git send-email". This
> > > patch does not apply properly. I try to use "git am --show-current-patch"
> > > to understand the problem. However, since original mail is encoded in quoted-
> > > printable, data returned by --show-current-patch is not a valid patch.
> > 
> > I agree that --show-current-patch is a misdesigned feature.  We'd be
> > doing a better service to our users if we documented that the patch
> > and log message are found at .git/rebase-apply/{patch,msg} instead
> > of trying to hide the path.
> > 
> > Unfortunately, it is likely that those who added that feature have
> > built their tooling around it to depend on its output being the full
> > e-mail message "am" was fed (and split by "git mailsplit").  So I do
> > not think we will be changing the output to the patch file only.
> > 
> > But even then, the documentation can be fixed without any backward
> > compatibility issues.  Perhaps like this?
> > 
> >  Documentation/git-am.txt | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
> > index 6f6c34b0f4..f63b70325c 100644
> > --- a/Documentation/git-am.txt
> > +++ b/Documentation/git-am.txt
> > @@ -172,7 +172,7 @@ default.   You can use `--no-utf8` to override this.
> >         untouched.
> > 
> >  --show-current-patch::
> > -       Show the patch being applied when "git am" is stopped because
> > +       Show the entire e-mail message "git am" has stopped at, because
> >         of conflicts.
> 
> I agree with you: I think that manpage and/or output of "git am" should
> mention ".git/rebase-apply/patch" (that is exactly what I was looking
> for).

I am currently have a WIP patchset that will print the location of the failed patch file (.git/rebase-apply/patch) in the case of a failure as well as the line number. Will this be sufficient for your purposes?

Thanks,
Denton
Show 9 quoted lines
> 
> Maybe documentation of --show-current-patch should be clarified with a
> note like "This option is mainly for internal purpose. If you want to
> get current patch, rely on .git/rebase-apply/patch".
> 
> 
> -- 
> Jérôme Pouiller
> 
Jerome Pouiller· Oct 23, 2019, 10:15 UTC · re: Denton Liu · lore

Re: [BUG] "--show-current-patch" return a mail instead of a patch

On Wednesday 23 October 2019 10:55:03 CEST Denton Liu wrote:
Show 43 quoted lines
> On Wed, Oct 23, 2019 at 08:49:41AM +0000, Jerome Pouiller wrote:
> > On Wednesday 23 October 2019 04:24:58 CEST Junio C Hamano wrote:
> > > Jerome Pouiller <Jerome.Pouiller@silabs.com> writes:
> > > > I try to use "git am" to apply a patch sent using "git send-email". This
> > > > patch does not apply properly. I try to use "git am --show-current-patch"
> > > > to understand the problem. However, since original mail is encoded in quoted-
> > > > printable, data returned by --show-current-patch is not a valid patch.
> > >
> > > I agree that --show-current-patch is a misdesigned feature.  We'd be
> > > doing a better service to our users if we documented that the patch
> > > and log message are found at .git/rebase-apply/{patch,msg} instead
> > > of trying to hide the path.
> > >
> > > Unfortunately, it is likely that those who added that feature have
> > > built their tooling around it to depend on its output being the full
> > > e-mail message "am" was fed (and split by "git mailsplit").  So I do
> > > not think we will be changing the output to the patch file only.
> > >
> > > But even then, the documentation can be fixed without any backward
> > > compatibility issues.  Perhaps like this?
> > >
> > >  Documentation/git-am.txt | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
> > > index 6f6c34b0f4..f63b70325c 100644
> > > --- a/Documentation/git-am.txt
> > > +++ b/Documentation/git-am.txt
> > > @@ -172,7 +172,7 @@ default.   You can use `--no-utf8` to override this.
> > >         untouched.
> > >
> > >  --show-current-patch::
> > > -       Show the patch being applied when "git am" is stopped because
> > > +       Show the entire e-mail message "git am" has stopped at, because
> > >         of conflicts.
> >
> > I agree with you: I think that manpage and/or output of "git am" should
> > mention ".git/rebase-apply/patch" (that is exactly what I was looking
> > for).
> 
> I am currently have a WIP patchset that will print the location of the
> failed patch file (.git/rebase-apply/patch) in the case of a failure as
> well as the line number. Will this be sufficient for your purposes?

It would be a clear improvement (the perfection would be to be able to use mergetool with git-am :) ).

Thank you,
-- 
Jérôme Pouiller
Denton Liu· Oct 23, 2019, 11:09 UTC · re: Jerome Pouiller · lore

Re: [BUG] "--show-current-patch" return a mail instead of a patch

On Wed, Oct 23, 2019 at 10:15:15AM +0000, Jerome Pouiller wrote:
Show 7 quoted lines
> On Wednesday 23 October 2019 10:55:03 CEST Denton Liu wrote:
> > I am currently have a WIP patchset that will print the location of the
> > failed patch file (.git/rebase-apply/patch) in the case of a failure as
> > well as the line number. Will this be sufficient for your purposes?
> 
> It would be a clear improvement (the perfection would be to be able to
> use mergetool with git-am :) ).
You should be able to do that with the --3way (-3) flag.
Show 6 quoted lines
> 
> Thank you,
> 
> -- 
> Jérôme Pouiller
> 
Jerome Pouiller· Oct 23, 2019, 12:47 UTC · re: Denton Liu · lore

Re: [BUG] "--show-current-patch" return a mail instead of a patch

On Wednesday 23 October 2019 13:09:51 CEST Denton Liu wrote:
Show 10 quoted lines
> On Wed, Oct 23, 2019 at 10:15:15AM +0000, Jerome Pouiller wrote:
> > On Wednesday 23 October 2019 10:55:03 CEST Denton Liu wrote:
> > > I am currently have a WIP patchset that will print the location of the
> > > failed patch file (.git/rebase-apply/patch) in the case of a failure as
> > > well as the line number. Will this be sufficient for your purposes?
> >
> > It would be a clear improvement (the perfection would be to be able to
> > use mergetool with git-am :) ).
> 
> You should be able to do that with the --3way (-3) flag.

AFAIK, it only works if index is found in local repository, which is rarely the case :(

-- 
Jérôme Pouiller

← back to recent threads