cherry picking changesets instead of commits

cherry picking changesets instead of commits

[Geoffrey Irving]

I have a project stored in a git repository with other versions stored
in both cvs and perforce (sadly).  I want the ability to propagate
changes back and forth between all three repositories without mangling
the git history.  I specifically want to avoid (1) having two copies
of each commit (e.g., an initial version and a version pulled from
cvsimport) and (2) commits full of git-p4 metadata (the [git-p4] lines
in commit messages).

One way to do this would be to have cvs branch imported using "git
cvsimport", and a perforce branch imported via "git-p4", and
communicate between the cvs/p4 branches and the main git branches
entirely through cherry picking.  Unfortunately, "git cherry" doesn't
recognize when commits with potentially different author names or log
messages polluted with "[git-p4]" produce the same changeset, so if I
export a change from the git master branch through cvsexportcommit and
update the cvs branch with cvsimport, "git cherry" will report that
the change doesn't exist on the git master branch.

Has anyone tackled this problem before?  Would it be straightforward
to add an option to "git cherry" to ignore differences in metadata?
The cherry documentation doesn't give its criteria for when commits
match, and I'm not familiar enough with the code yet to figure out
those criteria directly from cmd_cherry in builtin-log.c.

Thanks,
Geoffrey

Re: cherry picking changesets instead of commits

[Junio C Hamano]

"Geoffrey Irving" <irving@naml.us> writes:

> Would it be straightforward
> to add an option to "git cherry" to ignore differences in metadata?
> The cherry documentation doesn't give its criteria for when commits
> match, and I'm not familiar enough with the code yet to figure out
> those criteria directly from cmd_cherry in builtin-log.c.

cherry does not look at metadata at all.  It compares patch-ids.

Re: cherry picking changesets instead of commits

[Geoffrey Irving]

On May 28, 2008, at 11:19 AM, Junio C Hamano <gitster@pobox.com> wrote:

> "Geoffrey Irving" <irving@naml.us> writes:
>
>> Would it be straightforward
>> to add an option to "git cherry" to ignore differences in metadata?
>> The cherry documentation doesn't give its criteria for when commits
>> match, and I'm not familiar enough with the code yet to figure out
>> those criteria directly from cmd_cherry in builtin-log.c.
>
> cherry does not look at metadata at all.  It compares patch-ids.

Can you define what a patch-id is?

Geoffrey

Re: cherry picking changesets instead of commits

[Junio C Hamano]

Geoffrey Irving <geoffrey.irving@gmail.com> writes:

> On May 28, 2008, at 11:19 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
>> "Geoffrey Irving" <irving@naml.us> writes:
>>
>>> Would it be straightforward
>>> to add an option to "git cherry" to ignore differences in metadata?
>>> The cherry documentation doesn't give its criteria for when commits
>>> match, and I'm not familiar enough with the code yet to figure out
>>> those criteria directly from cmd_cherry in builtin-log.c.
>>
>> cherry does not look at metadata at all.  It compares patch-ids.
>
> Can you define what a patch-id is?

$ man git-patch-id

Re: cherry picking changesets instead of commits

[Geoffrey Irving]

On Wed, May 28, 2008 at 11:46 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Geoffrey Irving <geoffrey.irving@gmail.com> writes:
>
>> On May 28, 2008, at 11:19 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>
>>> "Geoffrey Irving" <irving@naml.us> writes:
>>>
>>>> Would it be straightforward
>>>> to add an option to "git cherry" to ignore differences in metadata?
>>>> The cherry documentation doesn't give its criteria for when commits
>>>> match, and I'm not familiar enough with the code yet to figure out
>>>> those criteria directly from cmd_cherry in builtin-log.c.
>>>
>>> cherry does not look at metadata at all.  It compares patch-ids.
>>
>> Can you define what a patch-id is?
>
> $ man git-patch-id

Thanks!  Here's a patch to avoid duplicating this particular ignorant question.

Geoffrey

commit a1bac5e1aa52f9a78733aa35a0a7e820df618301
Author: Geoffrey Irving <irving@naml.us>
Date:   Wed May 28 12:38:11 2008 -0700

    doc: mention git-patch-id in git-cherry documentation

diff --git a/Documentation/git-cherry.txt b/Documentation/git-cherry.txt
index b0468aa..9817fb5 100644
--- a/Documentation/git-cherry.txt
+++ b/Documentation/git-cherry.txt
@@ -41,6 +41,8 @@ has been applied <upstream> under a different commit
id.  For example,
 this will happen if you're feeding patches <upstream> via email rather
 than pushing or pulling commits directly.

+For details about how patches are compared, see git-patch-id.
+

 OPTIONS
 -------

Re: cherry picking changesets instead of commits

[Junio C Hamano]

"Geoffrey Irving" <irving@naml.us> writes:

> commit a1bac5e1aa52f9a78733aa35a0a7e820df618301
> Author: Geoffrey Irving <irving@naml.us>
> Date:   Wed May 28 12:38:11 2008 -0700
>
>     doc: mention git-patch-id in git-cherry documentation
>
> diff --git a/Documentation/git-cherry.txt b/Documentation/git-cherry.txt
> index b0468aa..9817fb5 100644
> --- a/Documentation/git-cherry.txt
> +++ b/Documentation/git-cherry.txt
> @@ -41,6 +41,8 @@ has been applied <upstream> under a different commit
> id.  For example,
>  this will happen if you're feeding patches <upstream> via email rather
>  than pushing or pulling commits directly.
>
> +For details about how patches are compared, see git-patch-id.
> +

Thanks.

In manual pages, "SEE ALSO" section is a more appropriate place to do this
kind of thing.

Re: cherry picking changesets instead of commits

[Geoffrey Irving]

On Wed, May 28, 2008 at 1:46 PM, Junio C Hamano <gitster@pobox.com> wrote:
> "Geoffrey Irving" <irving@naml.us> writes:
>
>> commit a1bac5e1aa52f9a78733aa35a0a7e820df618301
>> Author: Geoffrey Irving <irving@naml.us>
>> Date:   Wed May 28 12:38:11 2008 -0700
>>
>>     doc: mention git-patch-id in git-cherry documentation
>>
>> diff --git a/Documentation/git-cherry.txt b/Documentation/git-cherry.txt
>> index b0468aa..9817fb5 100644
>> --- a/Documentation/git-cherry.txt
>> +++ b/Documentation/git-cherry.txt
>> @@ -41,6 +41,8 @@ has been applied <upstream> under a different commit
>> id.  For example,
>>  this will happen if you're feeding patches <upstream> via email rather
>>  than pushing or pulling commits directly.
>>
>> +For details about how patches are compared, see git-patch-id.
>> +
>
> Thanks.
>
> In manual pages, "SEE ALSO" section is a more appropriate place to do this
> kind of thing.

Here's an improved patch.

Geoffrey

commit 2417df3ffc04e05c187a93ae58f620f2709ae950
Author: Geoffrey Irving <irving@naml.us>
Date:   Wed May 28 12:38:11 2008 -0700

    doc: reference git-patch-id in git-cherry documentation

diff --git a/Documentation/git-cherry.txt b/Documentation/git-cherry.txt
index b0468aa..dbf8895 100644
--- a/Documentation/git-cherry.txt
+++ b/Documentation/git-cherry.txt
@@ -56,6 +56,11 @@ OPTIONS
 <limit>::
        Do not report commits up to (and including) limit.

+See Also
+--------
+'git-patch-id(1)'::
+       The mechanism git-cherry uses to compare changesets.
+
 Author
 ------
 Written by Junio C Hamano <junkio@cox.net>

Re: cherry picking changesets instead of commits

[Jeff King]

On Wed, May 28, 2008 at 02:14:59PM -0700, Geoffrey Irving wrote:

> > In manual pages, "SEE ALSO" section is a more appropriate place to do this
> > kind of thing.
> 
> Here's an improved patch.

The usual way to include a cover letter in a patch is either:

  Subject: [PATCH] commit message subject

  commit message

  ---
  your cover letter

  diffstat + diff

or (and I often use this in a reply where it makes sense to read the
cover letter first):

  Subject: Re: your mail

  cover letter

  -- >8 -- (any visual separator that is _not_ "---")
  commit message subject

  commit message

  ---
  diffstat + diff

In the second case, I believe Junio just manually deletes everything
above the separator.

> +See Also
> +--------
> +'git-patch-id(1)'::
> +       The mechanism git-cherry uses to compare changesets.
> +

This should be

  linkgit:git-patch-id[1]::

which will turn it into the text you wrote for a manpage, or a link in
the HTML version. Looks like we already have a few of the style you used
that should probably be fixed, though.

-Peff

Re: cherry picking changesets instead of commits

[Jeff King]

On Wed, May 28, 2008 at 06:38:58PM -0400, Jeff King wrote:

> which will turn it into the text you wrote for a manpage, or a link in
> the HTML version. Looks like we already have a few of the style you used
> that should probably be fixed, though.

Actually, it looks like there is only one; patch is below. There are a
few false positives in gitk.txt, but they point to projects outside of
core git (like qgit, gitview, and tig), and so must be left as straight
text.

-- >8 --
doc/git-gui: make gitk "see also" a link

Even though git-gui and gitk are both maintained separately,
the documentation is part of core git, and thus it makes
sense to use the linkgit macro to create an href in the HTML
documentation.
---
 Documentation/git-gui.txt |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-gui.txt b/Documentation/git-gui.txt
index 6d6cd5d..cf2541f 100644
--- a/Documentation/git-gui.txt
+++ b/Documentation/git-gui.txt
@@ -81,7 +81,7 @@ git gui browser maint::
 
 See Also
 --------
-'gitk(1)'::
+linkgit:gitk[1]::
 	The git repository browser.  Shows branches, commit history
 	and file differences.  gitk is the utility started by
 	git-gui's Repository Visualize actions.
-- 
1.5.6.rc0.143.ge5cbe0

Re: cherry picking changesets instead of commits

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> doc/git-gui: make gitk "see also" a link
>
> Even though git-gui and gitk are both maintained separately,
> the documentation is part of core git, and thus it makes
> sense to use the linkgit macro to create an href in the HTML
> documentation.
> ---
>  Documentation/git-gui.txt |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/Documentation/git-gui.txt b/Documentation/git-gui.txt
> index 6d6cd5d..cf2541f 100644
> --- a/Documentation/git-gui.txt
> +++ b/Documentation/git-gui.txt
> @@ -81,7 +81,7 @@ git gui browser maint::
>  
>  See Also
>  --------
> -'gitk(1)'::
> +linkgit:gitk[1]::
>  	The git repository browser.  Shows branches, commit history
>  	and file differences.  gitk is the utility started by
>  	git-gui's Repository Visualize actions.

Hmm.

First a bit offtopic to avoid duplicated work.  Please nobody waste your
time sending s/See Also/SEE ALSO/ clean-up patch.  I've done that already.

Now, I do not think it is customary to say anything but the name of the
manual page if you refer to one in SEE ALSO section.  I am wondering if we
would want to remove the three-liner for consistency (but haven't done so,
neither the short description of qgit and others in gitk manual page is
gone yet).

 Documentation/git-gui.txt |    7 ++-----
 1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-gui.txt b/Documentation/git-gui.txt
index 6d6cd5d..cea8f8a 100644
--- a/Documentation/git-gui.txt
+++ b/Documentation/git-gui.txt
@@ -79,12 +79,9 @@ git gui browser maint::
 	selected in the browser can be viewed with the internal
 	blame viewer.
 
-See Also
+SEE ALSO
 --------
-'gitk(1)'::
-	The git repository browser.  Shows branches, commit history
-	and file differences.  gitk is the utility started by
-	git-gui's Repository Visualize actions.
+linkgit:gitk[1]
 
 Other
 -----

[PATCH] Documentation: git-cherry uses git-patch-id

[Junio C Hamano]

"Geoffrey Irving" <irving@naml.us> writes:

> On Wed, May 28, 2008 at 1:46 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
>> Thanks.
>>
>> In manual pages, "SEE ALSO" section is a more appropriate place to do this
>> kind of thing.
>
> Here's an improved patch.

Thanks, I'll do this instead...

-- >8 --
Geoffrey Irving noticed that git-cherry talks about comparing commits without
hinting how they are compared.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-cherry.txt |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-cherry.txt b/Documentation/git-cherry.txt
index b0468aa..d8e0a5b 100644
--- a/Documentation/git-cherry.txt
+++ b/Documentation/git-cherry.txt
@@ -13,6 +13,8 @@ DESCRIPTION
 -----------
 The changeset (or "diff") of each commit between the fork-point and <head>
 is compared against each commit between the fork-point and <upstream>.
+The commits are compared with their 'patch id', obtained from linkgit:git-patch-id[1]
+program.
 
 Every commit that doesn't exist in the <upstream> branch
 has its id (sha1) reported, prefixed by a symbol.  The ones that have
@@ -56,6 +58,10 @@ OPTIONS
 <limit>::
 	Do not report commits up to (and including) limit.
 
+SEE ALSO
+--------
+linkgit:git-patch-id[1]
+
 Author
 ------
 Written by Junio C Hamano <junkio@cox.net>
-- 
1.5.6.rc0.43.g823ea

Re: [PATCH] Documentation: git-cherry uses git-patch-id

[Geoffrey Irving]

On Wed, May 28, 2008 at 5:04 PM, Junio C Hamano <gitster@pobox.com> wrote:
> "Geoffrey Irving" <irving@naml.us> writes:
>
>> On Wed, May 28, 2008 at 1:46 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>
>>> Thanks.
>>>
>>> In manual pages, "SEE ALSO" section is a more appropriate place to do this
>>> kind of thing.
>>
>> Here's an improved patch.
>
> Thanks, I'll do this instead...

Thanks!

Incidentally, my original source of confusion was not understanding
the order of arguments of git-cherry.  Everything works fine once I
use the correct order.

Geoffrey

Re: cherry picking changesets instead of commits

[Jeff King]

On Wed, May 28, 2008 at 04:56:49PM -0700, Junio C Hamano wrote:

> First a bit offtopic to avoid duplicated work.  Please nobody waste your
> time sending s/See Also/SEE ALSO/ clean-up patch.  I've done that already.

It is much more widespread than that. We are very inconsistent with
section title capitalization. For man pages, it doesn't matter; asciidoc
makes them all upper case. However, they end up inconsistent in the html
versions. So if we care about this, then probably everything should be
fixed.

You can get a rough idea with:

  $ cd Documentation && perl -ne '
      print "$ARGV:$last" if
        /^-+$/ &&
        length == length($last) &&
        $last =~ /[a-z]/;
      $last = $_;
    ' git-*.txt

> Now, I do not think it is customary to say anything but the name of the
> manual page if you refer to one in SEE ALSO section.  I am wondering if we
> would want to remove the three-liner for consistency (but haven't done so,
> neither the short description of qgit and others in gitk manual page is
> gone yet).

It isn't how most of the pages are, but I think it is actually helpful.
And it's not as if "SEE ALSO" contents are supposed to be machine
parsable.

Though if the _reason_ that one needs to "see also" is not obvious,
perhaps it is better as part of the text of a different section (OTOH, I
think Geoffrey's patch was in response to your request to put it in the
"SEE ALSO").

-Peff