git-format-patch little gripe

git-format-patch little gripe

[Luben Tuikov]

Yep, after more than a year, I simply cannot get used to it...
http://marc.theaimsgroup.com/?l=git&m=113259043217761&w=2
And as I've seen, other people brought that up too.

My brain simply says:
`` "git-format-patch" is singular and so the SHA-1 specified
   on the command line should generate a _single_ patch.''

Yeah, but no.

So I still make the mistake of:
# git-rev-list --no-merges --pretty=one-line HEAD -- <somepath>
    I select a commit
# git-format-patch -o /tmp/ <commit>
...
    Oh, f@#$!  <CTRL-C>
# git-format-patch -o /tmp/ <commit>^..<commit>

Is there a chance to make "git-format-patch" output a single
patch? of that commit only?

    Luben
P.S. And let "git-format-patches" go wild with the ideologies...
Sorry, needed to vent as I made this mistake twice already
today...

Re: git-format-patch little gripe

[Martin Langhoff]

On 11/3/06, Luben Tuikov <ltuikov@yahoo.com> wrote:
> Yep, after more than a year, I simply cannot get used to it...
> http://marc.theaimsgroup.com/?l=git&m=113259043217761&w=2
> And as I've seen, other people brought that up too.

Hi Luben,

reading the thread, it sounds like you have a couple of shells scripts
or aliases that do what you want already ;-)

And for the scenario you mention, where upstream has taken some of
your patches... git-format-patch automatically tries to skip those.
Works well for me at least (ducks).

Re: git-format-patch little gripe

[Luben Tuikov]

--- Martin Langhoff <martin.langhoff@gmail.com> wrote:
> On 11/3/06, Luben Tuikov <ltuikov@yahoo.com> wrote:
> > Yep, after more than a year, I simply cannot get used to it...
> > http://marc.theaimsgroup.com/?l=git&m=113259043217761&w=2
> > And as I've seen, other people brought that up too.
> 
> Hi Luben,
> 
> reading the thread, it sounds like you have a couple of shells scripts
> or aliases that do what you want already ;-)

Yeah, would you know it those scripts got lost somewhere.  I now
know better -- branch off of git "next" and store in USB key. :-)
So I generally work off of "next" merged to my own branch which includes
extras I've collected along the way.

But what wouldn't I give to have
  git-format-_patch_ -o /tmp/ <commit>
generate a _single_ patch just as its name implies...

   Luben

Re: git-format-patch little gripe

[Junio C Hamano]

Luben Tuikov <ltuikov@yahoo.com> writes:

> But what wouldn't I give to have
>   git-format-_patch_ -o /tmp/ <commit>
> generate a _single_ patch just as its name implies...

It would have been nicer if you made that argument as a reply to
one of these messages:

http://thread.gmane.org/gmane.comp.version-control.git/4266/focus=4279
http://thread.gmane.org/gmane.comp.version-control.git/4843/focus=4843
http://thread.gmane.org/gmane.comp.version-control.git/5440/focus=5446

It is not an ideology; it is called not breaking existing UI,
which is rather unfortunate, because its UI is not the greatest
in the world.

Back when it was done, it was not clear that we would have a
rich set of revision set operations as "--since=2.weeks",
"from..to", or "this...that" as we have today.  Even though we
have them now, these set operations do not mesh very well with
the patch-id based filtering format-patch needs to do.

In order to exclude patches that you have as commits that have
not been _merged_ into your upstream, but the change they
contain have already been _applied_, you would need "the other
set" which is roughly "rev-list yours..upstream".  Some of your
patches (i.e. "rev-list upstream..yours") may have already
applied to the upstream but obviously as different commits, and
you would fiter them by comparing the patch-ids of them and
those from "the other set".  

Unfortunately, other than the recent addition "this...that",
none of the revision set operation would give us the "other set"
that is efficient to use ("all the commits that is older than 2
weeks" is an obvious "other set" for "--since=2.weeks", but that
set goes all the way down to the initial commit, which is
obviously not what we want).

One thing we talked about but nobody stepped up to code [*1*] is
to give "git-format-patch --stdin" that reads list of commits,
and runs "git-diff-tree --pretty --stat --summary -p $commit".
With that, we could do something like:

	git rev-list linus..orinoco | git format-patch --stdin

Your "git-format-patch --single $commit" is a shorthand for a
degenerated special case of that pattern.

You cannot do patch-id based filtering with this form, but I see
that "single" is often wanted on the list and #git, and people
who want it do not care about patch-id based filtering at all.
And I do not think it is that "they do not realize how much they
would be missing without patch-id filtering", in this case.  So
the above command line would probably be Ok.

With --left-right (in "pu"), you could even do something a bit
fancier like this:

	git rev-list --left-right linus...8139cp linus...airo |
	git format-patch --stdin

The --left-right output option, when used in conjunction with
the symmetric difference set operator, prefixes each commit with
'<' (left) or '>' (right) to indicate which ancestry it belongs
to (in the above example, the commits only in the branch that
tracks Linus but not in branches 8139cp or airo are prefixed
with '<', and commits on 8139cp or airo branches that have not
been merged in linus are prefixed with '>').  --stdin could use
that and take '<' as "the other set", i.e. the ones to base
filtering of '>' commits on, and output the commits that came
with '>' prefix (but subtracting the ones that have equivalent
patches in the '<' set).


[Footnote]

*1* format-patch is primarily the tool for a patch submitter,
and I did its original version back when I was one.  For a long
time (an equivalent to "eternity" in git timescale back then)
Linus did not show _any_ interest in it (you can compare the
dates on the messages I quoted above with the commit date of
0acfc972), and I suspect one of the reasons is because he was
the toplevel maintainer and did not have a need for a tool like
that.  Now I am the toplevel maintainer here and I haven't felt
the need to update it myself for quite some time ("it works for
me"), which is a bit sad.

Re: git-format-patch little gripe

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 1037 bytes --]

On Fri, 03 Nov 2006 03:17:13 -0800, Junio C Hamano wrote:
>
> It is not an ideology; it is called not breaking existing UI,
> which is rather unfortunate, because its UI is not the greatest
> in the world.

So maybe we could use some new thing here with cleaner UI. Perhaps:

	git export

Paired with a new "git import" there would be a much more obvious way
of working for new users than "git format-patch" and "git am".

Meanwhile, I don't think export/import is the right place to do the
patch-id filtering, etc. That seems to work much better in the context
of something like StackedGit, (which I've started playing with
recently and been very impressed with). I'd really like to think about
some ways to bring some of the functionality of StackedGit into the
core tools. (We've currently got "git commit --amend" for example, but
what I often want is "git goto old-commit; #hack; git commit --amend;
git replay" or whatever, (in Stacked Git speak that is currently "stg
goto old-commit; #hack; stg refresh; stg push --all").

-Carl

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: git-format-patch little gripe

[Jakub Narebski]

Carl Worth wrote:

> (We've currently got "git commit --amend" for example, but
> what I often want is "git goto old-commit; #hack; git commit --amend;
> git replay" or whatever, (in Stacked Git speak that is currently "stg
> goto old-commit; #hack; stg refresh; stg push --all").

<we are on branch 'top', and assume that branch 'tmp' does not exist>
$ git checkout -b tmp old-commit
#hack
$ git commit --amend
$ git rebase --onto tmp old-commit top
$ git branch -D tmp

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: git-format-patch little gripe

[Luben Tuikov]

--- Junio C Hamano <junkio@cox.net> wrote:
> It would have been nicer if you made that argument as a reply to
> one of these messages:
> 
> http://thread.gmane.org/gmane.comp.version-control.git/4266/focus=4279
> http://thread.gmane.org/gmane.comp.version-control.git/4843/focus=4843
> http://thread.gmane.org/gmane.comp.version-control.git/5440/focus=5446

I'm amazed at people who can read selectively messages in mailing lists.
The amount of messages sometimes is overwhelming.

> It is not an ideology; it is called not breaking existing UI,
> which is rather unfortunate, because its UI is not the greatest
> in the world.

Yes, it is very unfortunate.  It's like the metric system... nobody would
want to switch to it if you give them a _choice_... so water is going
to be freezing at 32 degrees and boiling over at 212.

So unless someone submits and commits a patch to fix this, and then we
shelter in the storm, it will never change.

> set" which is roughly "rev-list yours..upstream".  Some of your
> patches (i.e. "rev-list upstream..yours") may have already

Yeah, that's exactly what I do.

> One thing we talked about but nobody stepped up to code [*1*] is
> to give "git-format-patch --stdin" that reads list of commits,
> and runs "git-diff-tree --pretty --stat --summary -p $commit".
> With that, we could do something like:
> 
> 	git rev-list linus..orinoco | git format-patch --stdin

That'd be swell to have.

> Your "git-format-patch --single $commit" is a shorthand for a
> degenerated special case of that pattern.

Yes, except that I'd have to paste the sha-1 into the terminal
rather than on the command line.  This again proves that the current
git-format-patch <sha-1> to mean "<sha-1>..HEAD" is horribly broken.

> You cannot do patch-id based filtering with this form, but I see
> that "single" is often wanted on the list and #git, and people
> who want it do not care about patch-id based filtering at all.

The reason it is wanted is because it is _intuitive_.  This is
what engineers (of different backgrounds) tend to intuitively think
and assume given git's structure and the nature and meaning of
what an <sha-1> is in git, only to be surprised later when that
assumption is completely broken by git-format-patch.

"git-format-patch <sha-1>" intuitively means "Show me the commit identified
by the <sha-1> in a patch format."

Now if it had been called "git-format-patches" then maybe the argument
would've been different.

> And I do not think it is that "they do not realize how much they
> would be missing without patch-id filtering", in this case.  So
> the above command line would probably be Ok.

I do not think they'd be missing _anything_.  After all,
"git-format-patch <sha-1>..HEAD" is also intuitive.

> With --left-right (in "pu"), you could even do something a bit
> fancier like this:
> 
> 	git rev-list --left-right linus...8139cp linus...airo |
> 	git format-patch --stdin
> 
> The --left-right output option, when used in conjunction with
> the symmetric difference set operator, prefixes each commit with
> '<' (left) or '>' (right) to indicate which ancestry it belongs
> to (in the above example, the commits only in the branch that

Yeah, this is swell too!

> *1* format-patch is primarily the tool for a patch submitter,
> and I did its original version back when I was one.  For a long
> time (an equivalent to "eternity" in git timescale back then)
> Linus did not show _any_ interest in it (you can compare the

Yes, this is sad.  There's other places where Linus hasn't shown
any interest which have been suffering ever since.  Someone needs
to make the decision after all, or at least appoint someone to make
the decision (and then blame them ;-) later).

> dates on the messages I quoted above with the commit date of
> 0acfc972), and I suspect one of the reasons is because he was
> the toplevel maintainer and did not have a need for a tool like
> that.  Now I am the toplevel maintainer here and I haven't felt
> the need to update it myself for quite some time ("it works for
> me"), which is a bit sad.

If I had the clout, I'd change "git-format-patch <sha-1>" to
mean "git-format-patch <sha-1>^..<sha-1>" in a heart-beat, without
even blinking.

So what if people find out their tools now generate only _one_ patch
instead of N?  They'll revisit their scripts and use "<sha-1>..HEAD",
(which says what it means), instead of the completely broken current
behavior.

Also I doubt how many people actually have coded in their scripts
"git-format-patch <sha-1>" to mead by default "<sha-1>..HEAD" -- I'd think
that maintainers always want to know the "end point" and would always
use the <a>..<b> format, instead of presuming where/what their HEAD is.

Sorry it's my rant.  I know it's not your fault.

    Luben

Re: git-format-patch little gripe

[Jakub Narebski]

Luben Tuikov wrote:

> --- Junio C Hamano <junkio@cox.net> wrote:

>> One thing we talked about but nobody stepped up to code [*1*] is
>> to give "git-format-patch --stdin" that reads list of commits,
>> and runs "git-diff-tree --pretty --stat --summary -p $commit".
>> With that, we could do something like:
>> 
>>      git rev-list linus..orinoco | git format-patch --stdin
> 
> That'd be swell to have.
> 
>> Your "git-format-patch --single $commit" is a shorthand for a
>> degenerated special case of that pattern.
> 
> Yes, except that I'd have to paste the sha-1 into the terminal
> rather than on the command line.  This again proves that the current
> git-format-patch <sha-1> to mean "<sha-1>..HEAD" is horribly broken.

First, you could have behavior you wanted by using additional option
--single, i.e. git-format-patch --single <sha-1> would mean
git-format-patch <sha-1>^..<sha-1>

But I agree that it is somewhat counter-intuitive that for all
other commands which use revision list <sha-1> means lineage
of <sha-1> while for git-format-patch it means <sha-1>..HEAD
... and we have <sha-1>.. shortcut (two characters more) to mean
the same.
 
Another command with non-obvious UI is git-format-branch, which
is not CLI equivalent of gitk/qgit (doesn't accept revision range
for example).

>> You cannot do patch-id based filtering with this form, but I see
>> that "single" is often wanted on the list and #git, and people
>> who want it do not care about patch-id based filtering at all.
> 
> The reason it is wanted is because it is _intuitive_.  This is
> what engineers (of different backgrounds) tend to intuitively think
> and assume given git's structure and the nature and meaning of
> what an <sha-1> is in git, only to be surprised later when that
> assumption is completely broken by git-format-patch.

Intuiveness is in the eye of beholder :-)

>> And I do not think it is that "they do not realize how much they
>> would be missing without patch-id filtering", in this case.  So
>> the above command line would probably be Ok.
> 
> I do not think they'd be missing _anything_.  After all,
> "git-format-patch <sha-1>..HEAD" is also intuitive.

And we have <sha-1>.. shortcut. 

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: git-format-patch little gripe

[Junio C Hamano]

Luben Tuikov <ltuikov@yahoo.com> writes:

> Also I doubt how many people actually have coded in their scripts
> "git-format-patch <sha-1>" to mead by default "<sha-1>..HEAD" -- I'd think
> that maintainers always want to know the "end point" and would always
> use the <a>..<b> format, instead of presuming where/what their HEAD is.

Well, the answer would be "do not script around Porcelain".

> Sorry it's my rant.  I know it's not your fault.

I do not even think there is any "fault" there, to be honest, so
there is no need to apologize.  The format-patch command does
what it was designed to do and does it well -- format a sequence
of patches for upstream submission.

If you want a single patch, you can always say "git show", by
the way.

Re: git-format-patch little gripe

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 364 bytes --]

On Fri, 03 Nov 2006 14:26:29 -0800, Junio C Hamano wrote:
> If you want a single patch, you can always say "git show", by
> the way.

Ugh.

I mean, git show is fine for displaying something to just read it, but
if the results are to be fed back into git at some point, then
git-show is pretty useless, (throws away commit message, author,
timestamp, etc.).

-Carl

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: git-format-patch little gripe

[Jeff King]

On Fri, Nov 03, 2006 at 02:35:05PM -0800, Carl Worth wrote:

> I mean, git show is fine for displaying something to just read it, but
> if the results are to be fed back into git at some point, then
> git-show is pretty useless, (throws away commit message, author,
> timestamp, etc.).

Try
  git-show --pretty=email
or even
  git-show --pretty=email --stat -p

Re: git-format-patch little gripe

[Jakub Narebski]

Jeff King wrote:

> On Fri, Nov 03, 2006 at 02:35:05PM -0800, Carl Worth wrote:
> 
>> I mean, git show is fine for displaying something to just read it, but
>> if the results are to be fed back into git at some point, then
>> git-show is pretty useless, (throws away commit message, author,
>> timestamp, etc.).
> 
> Try
>   git-show --pretty=email
> or even
>   git-show --pretty=email --stat -p

YAUO, Yet Another Undocumented Option. ::sigh::

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: git-format-patch little gripe

[Linus Torvalds]

On Sat, 4 Nov 2006, Jakub Narebski wrote:
>
> YAUO, Yet Another Undocumented Option. ::sigh::

The "--pretty=xxx" is certainly not new, and it's not even totally 
undocumented. It's mentioned in several of the examples in various forms.

All the logging commands take the "pretty" argument.

Try

	git log --pretty={oneline,short,medium,full,fuller,raw}

(and the same things work for "git whatchanged" and "git show" etc)

Re: git-format-patch little gripe

[Jakub Narebski]

Linus Torvalds wrote:
> 
> On Sat, 4 Nov 2006, Jakub Narebski wrote:
>>
>> YAUO, Yet Another Undocumented Option. ::sigh::
> 
> The "--pretty=xxx" is certainly not new, and it's not even totally 
> undocumented. It's mentioned in several of the examples in various forms.
> 
> All the logging commands take the "pretty" argument.
> 
> Try
> 
> 	git log --pretty={oneline,short,medium,full,fuller,raw}
> 
> (and the same things work for "git whatchanged" and "git show" etc)

I meant git show --pretty=email option. Sorry for confusion.

-- 
Jakub Narebski

Re: git-format-patch little gripe

[Jeff King]

On Fri, Nov 03, 2006 at 01:07:20AM -0800, Luben Tuikov wrote:

> So I still make the mistake of:
> # git-rev-list --no-merges --pretty=one-line HEAD -- <somepath>
>     I select a commit
> # git-format-patch -o /tmp/ <commit>
> ...
>     Oh, f@#$!  <CTRL-C>
> # git-format-patch -o /tmp/ <commit>^..<commit>

For my own workflow, I don't want to have to pick the commit out of
rev-list (or log) output. I want to find it and hit a button to say "OK,
now mail this patch." So I put _all_ of my patches into an mbox, and
then browse them with mutt. Sort of a poor man's patch browser, but then
I'm ready to jump into mailing them immediately.

I use the following script:

#!/bin/sh
root=${1:-origin}
git-format-patch -s --stdout $root >.mbox
mutt -f .mbox
rm -f .mbox

And then in mutt, I use this macro to bind 'b' to editing the full
message w/ headers:

macro index b ":set edit_headers=yes<enter><resend-message>:set edit_headers=no<enter>"

I know that may be useless if you're not using mutt, but I just wanted
to stimulate some discussion among patch submitters about how they
actually do it. I'm not sure how configurable tig is, but it shouldn't
be too hard to add something like this to it.

Re: git-format-patch little gripe

[Luben Tuikov]

--- Jeff King <peff@peff.net> wrote:
> For my own workflow, I don't want to have to pick the commit out of
> rev-list (or log) output. I want to find it and hit a button to say "OK,
> now mail this patch." So I put _all_ of my patches into an mbox, and
> then browse them with mutt. Sort of a poor man's patch browser, but then
> I'm ready to jump into mailing them immediately.
> 
> I use the following script:
> 
> #!/bin/sh
> root=${1:-origin}
> git-format-patch -s --stdout $root >.mbox
> mutt -f .mbox
> rm -f .mbox
> 
> And then in mutt, I use this macro to bind 'b' to editing the full
> message w/ headers:
> 
> macro index b ":set edit_headers=yes<enter><resend-message>:set edit_headers=no<enter>"
> 
> I know that may be useless if you're not using mutt, but I just wanted
> to stimulate some discussion among patch submitters about how they
> actually do it. I'm not sure how configurable tig is, but it shouldn't
> be too hard to add something like this to it.

That's very cool!

   Luben

Re: git-format-patch little gripe

[Sean]

On Fri, 3 Nov 2006 13:50:26 -0500
Jeff King <peff@peff.net> wrote:

> For my own workflow, I don't want to have to pick the commit out of
> rev-list (or log) output. I want to find it and hit a button to say "OK,
> now mail this patch." So I put _all_ of my patches into an mbox, and
> then browse them with mutt. Sort of a poor man's patch browser, but then
> I'm ready to jump into mailing them immediately.
> 
> I use the following script:
> 
> #!/bin/sh
> root=${1:-origin}
> git-format-patch -s --stdout $root >.mbox
> mutt -f .mbox
> rm -f .mbox

If your mail setup support imap, the patches can be dumped directly into
it rather than having to go through an mbox.  For instance you can have
something like this in your ~/.gitconfig:

[imap]
   Host = imap.server.com
   Folder = "Drafts"
   User = uname
   Pass = password

And then the above command line becomes:

git-format-patch -s --stdout $root | git-imap-send

To move all the patches into your imap drafts folder to be accessed
by whatever email client you use.

Sean

Re: git-format-patch little gripe

[Jeff King]

On Sat, Nov 25, 2006 at 10:30:33AM -0500, Sean wrote:

> If your mail setup support imap, the patches can be dumped directly into
> it rather than having to go through an mbox.  For instance you can have
> something like this in your ~/.gitconfig:

Sure, I could. But if you have a client which supports mboxes, it's
probably better to use the mbox. With your solution, I send the patch to
the imap server, then fire up the mail client and re-download the
patch, possibly mark it up, and then send it out by smtp. The latency
increase can be noticeable, especially for big patches and slow imap
servers.

> To move all the patches into your imap drafts folder to be accessed
> by whatever email client you use.

Oh come on, Real Men just edit the mbox in vi. :)

Re: git-format-patch little gripe

[Sean]

On Sat, 25 Nov 2006 10:39:49 -0500
Jeff King <peff@peff.net> wrote:

> On Sat, Nov 25, 2006 at 10:30:33AM -0500, Sean wrote:
> 
> > If your mail setup support imap, the patches can be dumped directly into
> > it rather than having to go through an mbox.  For instance you can have
> > something like this in your ~/.gitconfig:
> 
> Sure, I could. But if you have a client which supports mboxes, it's
> probably better to use the mbox. With your solution, I send the patch to
> the imap server, then fire up the mail client and re-download the
> patch, possibly mark it up, and then send it out by smtp. The latency
> increase can be noticeable, especially for big patches and slow imap
> servers.

Yeah, I didn't mean it as a better replacement for your use, just as
an option that might suit some people better.  It works out well here.

> > To move all the patches into your imap drafts folder to be accessed
> > by whatever email client you use.
> 
> Oh come on, Real Men just edit the mbox in vi. :)

Damn.. caught in my skirt again.. ;o)