threads / discuss / 32427

recommendation for patch maintenance

Subject: recommendation for patch maintenance

## tl;dr

6 messages between Dec 21, 2012 and Dec 21, 2012.

replies: 5people: 2as markdown or json

Manlio Perillo· Dec 21, 2012, 14:45 UTC · lore
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Hi.

I would like to have advices about some possible workflows to use when maintaining a patch, that can evolve over the time (fixing bugs, and applying advices from reviewers).

In my case I have a single commit to maintain.
The workflow I use now is this:
  1) create a topic branch, e.g. mp/complete-path
  2) write code
  3) commit
  4) format-patch --output=mp/complete-patch master
  5) review the patch
  6) send-email
when I need to update the patch:
  1) modify code
  2) commit --amend
  3) format-patch --subject-prefix="PATCH v<n>" \
     --output=mp/complete-patch master
  4) edit patch to add a list of what was changed
  5) review the patch
  6) send-email

This is far from ideal, since all my local changes are lost. Another problem is that when I found some trivial error in 5), I need to call format-patch again, loosing the "what's changed list".

A possible solution is to:
1) create a "public" topic branch, e.g. mp/complete-patch
2) create the associated "private" topic branch, e.g.
   mp/complete-patch/private
...

Changes are committed to the private branch. When I need to update the patch:

  1) update code
  2) commit new changes; the commit message will contain the
     "what's changed" list to be used for the new version of the patch
  3) checkout <public branch>
  4) merge --squash <private branch>

Now I have my full history, and the "what's changed list" is saved in the private commits.

(not tested)
What is the workflow you usually use?
Thanks   Manlio
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAlDUde4ACgkQscQJ24LbaUSqOwCfZON5f9mdAYkvACim802JGFhP 5W8An1Y7WXgsH/Q/p1/0jVMo1dJ3HwwO =Xydn -----END PGP SIGNATURE-----

Junio C Hamano· Dec 21, 2012, 17:01 UTC · re: Manlio Perillo · lore

Re: recommendation for patch maintenance

Manlio Perillo <manlio.perillo@gmail.com> writes:
Show 27 quoted lines
> I would like to have advices about some possible workflows to use when
> maintaining a patch, that can evolve over the time (fixing bugs, and
> applying advices from reviewers).
>
> In my case I have a single commit to maintain.
>
>
> The workflow I use now is this:
>
>   1) create a topic branch, e.g. mp/complete-path
>   2) write code
>   3) commit
>   4) format-patch --output=mp/complete-patch master
>   5) review the patch
>   6) send-email
>
> when I need to update the patch:
>
>   1) modify code
>   2) commit --amend
>   3) format-patch --subject-prefix="PATCH v<n>" \
>      --output=mp/complete-patch master
>   4) edit patch to add a list of what was changed
>   5) review the patch
>   6) send-email
>
> This is far from ideal, since all my local changes are lost.

Not offering any answer, but it is unclear to me what local changes you are losing here. Care to explain?

Manlio Perillo· Dec 21, 2012, 17:29 UTC · re: Junio C Hamano · lore

Re: recommendation for patch maintenance

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Il 21/12/2012 18:01, Junio C Hamano ha scritto:
Show 21 quoted lines
> Manlio Perillo <manlio.perillo@gmail.com> writes:
> 
>> I would like to have advices about some possible workflows to use when
>> maintaining a patch, that can evolve over the time (fixing bugs, and
>> applying advices from reviewers).
>>
> [...]
>> when I need to update the patch:
>>
>>   1) modify code
>>   2) commit --amend
>>   3) format-patch --subject-prefix="PATCH v<n>" \
>>      --output=mp/complete-patch master
>>   4) edit patch to add a list of what was changed
>>   5) review the patch
>>   6) send-email
>>
>> This is far from ideal, since all my local changes are lost.
> 
> Not offering any answer, but it is unclear to me what local changes
> you are losing here.  Care to explain?

I lose the history of all the changes I have made to produce the final version of a patch.

Since for every new version of a patch I do a commit --amend, I can not see, as an example, the changes I have made between x and y versions of a patch.

Of course the commits are not really lost, but I have to search them using the reflog.

Thanks   Manlio
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAlDUnIAACgkQscQJ24LbaUTf0QCfX9WtA+/GLzVWDJFPbLMCPucJ bKQAnj0HJuQs9SVCPV/TlDXcpGDqIqfD =lhZ5 -----END PGP SIGNATURE-----

Junio C Hamano· Dec 21, 2012, 18:17 UTC · re: Manlio Perillo · lore

Re: recommendation for patch maintenance

Manlio Perillo <manlio.perillo@gmail.com> writes:
Show 9 quoted lines
> I lose the history of all the changes I have made to produce the final
> version of a patch.
>
> Since for every new version of a patch I do a commit --amend, I can not
> see, as an example, the changes I have made between x and y versions of
> a patch.
>
> Of course the commits are not really lost, but I have to search them
> using the reflog.

Yeah, these days with "rebase -i" that does not leave individual steps as separate reflog entries, for an easy series, you can do things like:

    $ git rebase -i ;# work on polishing the series
    $ git show-branch -g ;# view where it diverged
    $ git diff HEAD~4 @{1}~4

Of course you can plan ahead (this is what I usually do when working on a series that is not "how about this" throw-away patch I send to this list all the time) and name the topic "topic-v1", fork the new round "topic-v2", "topic-v3", etc. and do things like

    $ sh -c 'git diff topic-v2~$1 topic-v3~$1' - 4

(the point being that then you do ^P or whatever shell command history mechanism to recall that line, edit "4" to "3" and rerun the comparison for other corresponding ones).

On a related but different front, I've been thinking about improving the "format-patch --subject-prefix" mechanism to make it even easier to use. With the current interface, when you prepare your reroll, you would do:

    $ git format-patch --cover-letter \
        --subject-prefix='PATCH v4' -o ./+mp master

and end up overwriting the patches from the previous round (if their subject did not change). You will always overwrite the cover because its subject never changes and that is where the filename is taken from.

What if we added another option, say --reroll $n (or -v$n), to let you write:

    $ git format-patch --cover-letter -v4 -o ./+mp master
that produces output files named like:
    ./+mp/v4-0000-cover-letter.txt
    ./+mp/v4-0001-git-completion.bash.add-support-for-pa.txt
with the subject '[PATCH v4]' prefix?

Then you can transplant the cover letter material from the cover letter from the older iteration to the new cover letter in your editor, and sending them out will become

    $ git send-email ... ./+mp/v4-*.txt
Hmm?
Manlio Perillo· Dec 21, 2012, 21:30 UTC · re: Junio C Hamano · lore

Re: recommendation for patch maintenance

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Il 21/12/2012 19:17, Junio C Hamano ha scritto:
Show 12 quoted lines
> [...]
> Of course you can plan ahead (this is what I usually do when working
> on a series that is not "how about this" throw-away patch I send to
> this list all the time) and name the topic "topic-v1", fork the new
> round "topic-v2", "topic-v3", etc. and do things like
> 
>     $ sh -c 'git diff topic-v2~$1 topic-v3~$1' - 4
> 
> (the point being that then you do ^P or whatever shell command
> history mechanism to recall that line, edit "4" to "3" and rerun the
> comparison for other corresponding ones).
> 

Thanks, this seems fine. Maybe topic-v2 --> topic/v2 (it looks better to me).

Show 18 quoted lines
> On a related but different front, I've been thinking about improving
> the "format-patch --subject-prefix" mechanism to make it even easier
> to use.
>
> [...]
>
> What if we added another option, say --reroll $n (or -v$n), to let
> you write:
> 
>     $ git format-patch --cover-letter -v4 -o ./+mp master
> 
> that produces output files named like:
> 
>     ./+mp/v4-0000-cover-letter.txt
>     ./+mp/v4-0001-git-completion.bash.add-support-for-pa.txt
> 
> with the subject '[PATCH v4]' prefix?
> 

I think it is a good idea to reduce the things one have to do by hand. I, too, was thinking something similar, with a -v$n option.

And, from these few days I have started to follow the mailing list, it seems that '[PATCH v$n'] is the standard practice.

By the way, I would also like to be able to set the default value for the --output option in config file; something like:

  [format]
  output = +mp/$(git symbolic-ref --short HEAD)
where the string will be processed by the shell:
   sh -c 'printf "+mp/$(git symbolic-ref --short HEAD)'"

Note that printf is POSIX, and the standard suggests to use it instead of echo: http://pubs.opengroup.org/onlinepubs/9699919799/utilities/echo.html

I have read the affected source code, and it should not be too hard. What do you think?

I should be able to hack the patch in the weekend, but I'm not sure it will be accepted.

Show 7 quoted lines
> Then you can transplant the cover letter material from the cover
> letter from the older iteration to the new cover letter in your
> editor, and sending them out will become
> 
>     $ git send-email ... ./+mp/v4-*.txt
> 
> Hmm?
Seems good.
Regards   Manlio
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAlDU1REACgkQscQJ24LbaUT5dgCgismeh+R3B26otuBIXRf/VUiq +5gAoIR56wVfs8Vw8AAWtim4aor97MXN =DfeG -----END PGP SIGNATURE-----

Junio C Hamano· Dec 21, 2012, 22:17 UTC · re: Manlio Perillo · lore

Re: recommendation for patch maintenance

On Fri, Dec 21, 2012 at 1:30 PM, Manlio Perillo <manlio.perillo@gmail.com> wrote:

Show 10 quoted lines
>
> By the way, I would also like to be able to set the default value for
> the --output option in config file; something like:
>
>   [format]
>   output = +mp/$(git symbolic-ref --short HEAD)
>
> where the string will be processed by the shell:
>
>    sh -c 'printf "+mp/$(git symbolic-ref --short HEAD)'"

My knee-jerk reaction is that, while it might make sense to give an option to allow users to use a dedicated directory per branch, I am not interested in that form; especially the part that lets the users too long a rope to hang themselves with by allowing an arbitrary shell expansions goes against my taste.

But you *can* prove me wrong; read on.
> I should be able to hack the patch in the weekend, but I'm not sure it
> will be accepted.

You got it backwards, just like many other new people who come and go on this list. If something is very useful, you'd work on it even if the result may not appear immediately in the upstream, and you'd keep maintaining a local fork so that you can keep benefiting from it, once you have written such a useful feature (whatever it is). If even the original "itch-holder" does not think that the benefit from his change outweighs the cost of such an effort, why should *I* carry it in my tree?

The goal of a contributor who comes up with an idea, gets "It doesn't sound like a good idea" during the review but disagrees and believes in his idea ought to be to prove the reviewer(s) wrong by polishing the idea and the implementation so much that the upstream comes begging for your change ;-)

The core part of the change to add --reroll $n option should be straight-forward and will involve a few functions in builtin/log.c, but the existing code is so badly structured that it probably needs a couple of "preparatory steps" patch to clean up the API between internal functions on that codepath, before the real change can be made, I think.

← back to recent threads