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

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

[Jerome Pouiller]

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

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

[Junio C Hamano]

Jerome Pouiller <Jerome.Pouiller@silabs.com> writes:

> 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

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

[Jerome Pouiller]

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).

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

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

[Denton Liu]

Hi Jerome,

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?

Thanks,

Denton

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

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

[Jerome Pouiller]

On Wednesday 23 October 2019 10:55:03 CEST Denton Liu wrote:
> 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

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

[Denton Liu]

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.

> 
> Thank you,
> 
> -- 
> Jérôme Pouiller
> 

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

[Jerome Pouiller]

On Wednesday 23 October 2019 13:09:51 CEST Denton Liu wrote:
> 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