git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* problem with cherry-picking a commit which comes before introducing a new submodule
@ 2011-01-07 17:24 Yaroslav Halchenko
  2011-01-07 18:15 ` Jonathan Nieder
  0 siblings, 1 reply; 10+ messages in thread
From: Yaroslav Halchenko @ 2011-01-07 17:24 UTC (permalink / raw)
  To: git

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

Hi GIT Gurus,

per quick IRC troubleshooting it seems to be some kind of an issue in
git cherry-pick logic which tries to deal with existing .gitmodules
structure?

In our repository we had some submodules, then there was a branch off
(call new branch todonotloose) with a single commit.  In the master we
had some other commits and moved one of the subdirectories into a
submodule.

Later on we decided to cherry pick todonotloose into master but
cherry-pick fails despite the fact that 'git show todonotloose | patch
-p1' applies just fine, ie there were no changes touching any of the
submodules.

See http://pastebin.com/hpqbiB03 for output.

IMHO everything is legit and failure should have not occurred, or am I
missing something?

-- 
Yaroslav O. Halchenko
Postdoctoral Fellow,   Department of Psychological and Brain Sciences
Dartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755
Phone: +1 (603) 646-9834                       Fax: +1 (603) 646-1419
WWW:   http://www.linkedin.com/in/yarik        

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: problem with cherry-picking a commit which comes before introducing a new submodule
  2011-01-07 17:24 problem with cherry-picking a commit which comes before introducing a new submodule Yaroslav Halchenko
@ 2011-01-07 18:15 ` Jonathan Nieder
  2011-01-07 18:32   ` Yaroslav Halchenko
  0 siblings, 1 reply; 10+ messages in thread
From: Jonathan Nieder @ 2011-01-07 18:15 UTC (permalink / raw)
  To: Yaroslav Halchenko; +Cc: git, Elijah Newren

(+cc: Elijah Newren, who has worked on some of this code)

Yaroslav Halchenko wrote:

> In our repository we had some submodules, then there was a branch off
> (call new branch todonotloose) with a single commit.  In the master we
> had some other commits and moved one of the subdirectories into a
> submodule.
> 
> Later on we decided to cherry pick todonotloose into master but
> cherry-pick fails despite the fact that 'git show todonotloose | patch
> -p1' applies just fine, ie there were no changes touching any of the
> submodules.
[...]
> $> git status
> # On branch master
> # Changes to be committed:
> #   (use "git reset HEAD <file>..." to unstage)
> #
> #	new file:   poster-hbm2011_neurodebian/abstract.txt
> #	modified:   poster-hbm2011_neurodebian/jb.txt
> #
> # Unmerged paths:
> #   (use "git reset HEAD <file>..." to unstage)
> #   (use "git add/rm <file>..." as appropriate to mark resolution)
> #
> #	added by us:        frontiers/code
> #

As contrib/examples/git-revert.sh explains, the heart of "git
cherry-pick" is

	base=todonotloose^
	next=todonotloose
	head=HEAD

	git merge-recursive $base -- $head $next

Could you try that, perhaps with GIT_MERGE_VERBOSITY=4 (or some other
number from 1 to 5, larger is louder) in the environment?  For context,

	git ls-files -u;	# after the merge
	git diff-tree todonotloose
	git diff-tree todonotloose^ HEAD

would also be interesting.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: problem with cherry-picking a commit which comes before introducing a new submodule
  2011-01-07 18:15 ` Jonathan Nieder
@ 2011-01-07 18:32   ` Yaroslav Halchenko
  2011-01-07 23:00     ` Jonathan Nieder
  0 siblings, 1 reply; 10+ messages in thread
From: Yaroslav Halchenko @ 2011-01-07 18:32 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: git, Elijah Newren

oy -- I thought that cherry-pick is primarily just application of the
patch (without use of the merge clevernesses).

Here is the protocol:
% export GIT_MERGE_VERBOSITY=4
%         base=todonotloose^
%         next=todonotloose
%         head=HEAD
% 
%         git merge-recursive $base -- $head $next
Merging HEAD with todonotloose
Merging:
855981d just placeholders in the abstract
a00c497 Initial draft for HBM abstract.
CONFLICT (file/directory): There is a directory with name frontiers/code in todonotloose. Adding frontiers/code as frontiers/code~HEAD
%         git ls-files -u;        # after the merge
160000 a2b57871d2d79bef06ba6214739d82b9a63772a8 2   frontiers/code
zsh: command not found: #
%         git diff-tree todonotloose
a00c497fa399c00486c97121ed0b8fda72c7ce47
:040000 040000 40427e34a1ff89c458f2a5f262a108d46b4fa004 c7ba91028b1cef63f4f7eef70f0c4054b31e92b6 M  poster-hbm2011_neurodebian
%         git diff-tree todonotloose^ HEAD
:100644 100644 378e1379ec5ebb7abac59fec162b7238b5846525 c39ced763aeb5fd352cecd6fef1bfc40471f2246 M  .gitmodules
:000000 040000 0000000000000000000000000000000000000000 141dbc1bfe1be2eab77f04ca03f6f28feb372cca A  challenge-execpapers
:040000 040000 401fd66867de412b8653dc3a698bbaa45441bec1 ee190f09786f324abdda6e7a36e8278c201a20a0 M  frontiers
:040000 040000 26c884a67efb55bdf96d7453d9acd50cee36ae90 ad3e829d15b302c4342a6b2a9fb5dfede0ed77c9 M  sty


On Fri, 07 Jan 2011, Jonathan Nieder wrote:
> As contrib/examples/git-revert.sh explains, the heart of "git
> cherry-pick" is

> 	base=todonotloose^
> 	next=todonotloose
> 	head=HEAD

> 	git merge-recursive $base -- $head $next

> Could you try that, perhaps with GIT_MERGE_VERBOSITY=4 (or some other
> number from 1 to 5, larger is louder) in the environment?  For context,

> 	git ls-files -u;	# after the merge
> 	git diff-tree todonotloose
> 	git diff-tree todonotloose^ HEAD

> would also be interesting.


-- 
Yaroslav O. Halchenko
Postdoctoral Fellow,   Department of Psychological and Brain Sciences
Dartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755
Phone: +1 (603) 646-9834                       Fax: +1 (603) 646-1419
WWW:   http://www.linkedin.com/in/yarik        

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: problem with cherry-picking a commit which comes before introducing a new submodule
  2011-01-07 18:32   ` Yaroslav Halchenko
@ 2011-01-07 23:00     ` Jonathan Nieder
  2011-01-07 23:48       ` Yaroslav Halchenko
  2011-01-08  0:01       ` Yaroslav Halchenko
  0 siblings, 2 replies; 10+ messages in thread
From: Jonathan Nieder @ 2011-01-07 23:00 UTC (permalink / raw)
  To: Yaroslav Halchenko; +Cc: git, Elijah Newren

Yaroslav Halchenko wrote [abbreviated]:

> Merging HEAD with todonotloose
> Merging:
> 855981d just placeholders in the abstract
> a00c497 Initial draft for HBM abstract.
> CONFLICT (file/directory): There is a directory with name frontiers/code in todonotloose. Adding frontiers/code as
> +frontiers/code~HEAD
> %         git ls-files -u
> 160000 a2b5787 2   frontiers/code
> %         git diff-tree todonotloose
> a00c497
> :040000 040000 40427e34 c7ba910 M	poster-hbm2011_neurodebian
> %         git diff-tree todonotloose^ HEAD
> :100644 100644 378e137 c39ced7 M	.gitmodules
> :000000 040000 0000000 141dbc1 A	challenge-execpapers
> :040000 040000 401fd66 ee190f0 M	frontiers
> :040000 040000 26c884a ad3e829 M	sty

One more piece of protocol: what git version are you using?  The
release notes mention a fix in this area in v1.7.3[1]:

 * "git merge -s recursive" (which is the default) did not handle cases
   where a directory becomes a file (or vice versa) very well.

Hopefully this is that.  In any case, sounds like a bug.

(Hopefully someone else can comment on why cherry-pick uses the
merge machinery to notice conflicts that would not be clear from
the patch alone.)

Thanks again.
Jonathan

[1] There is an updated Debian source package at [2].  Or, probably
faster: one can use the build result in bin-wrappers/ from a git.git
clone in place.
[2] http://mentors.debian.net/debian/pool/main/g/git/git_1.7.4~rc1-0.1.dsc

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: problem with cherry-picking a commit which comes before introducing a new submodule
  2011-01-07 23:00     ` Jonathan Nieder
@ 2011-01-07 23:48       ` Yaroslav Halchenko
  2011-01-08  0:01       ` Yaroslav Halchenko
  1 sibling, 0 replies; 10+ messages in thread
From: Yaroslav Halchenko @ 2011-01-07 23:48 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: git, Elijah Newren

sorry -- lame me... 1.7.2.3 ... I will check with current version as soon as
kids permit ;-)

% apt-cache policy git
git:
  Installed: 1:1.7.2.3-2.2
  Candidate: 1:1.7.2.3-2.2
  Version table:
 *** 1:1.7.2.3-2.2 0
        900 http://debian.lcs.mit.edu/debian/ squeeze/main amd64 Packages
        800 http://debian.lcs.mit.edu/debian/ sid/main amd64 Packages
        100 /var/lib/dpkg/status
% git --version
git version 1.7.2.3


On Fri, 07 Jan 2011, Jonathan Nieder wrote:
> One more piece of protocol: what git version are you using?  The
> release notes mention a fix in this area in v1.7.3[1]:

>  * "git merge -s recursive" (which is the default) did not handle cases
>    where a directory becomes a file (or vice versa) very well.

> Hopefully this is that.  In any case, sounds like a bug.

> (Hopefully someone else can comment on why cherry-pick uses the
> merge machinery to notice conflicts that would not be clear from
> the patch alone.)

> Thanks again.
> Jonathan

> [1] There is an updated Debian source package at [2].  Or, probably
> faster: one can use the build result in bin-wrappers/ from a git.git
> clone in place.
> [2] http://mentors.debian.net/debian/pool/main/g/git/git_1.7.4~rc1-0.1.dsc


-- 
=------------------------------------------------------------------=
Keep in touch                                     www.onerussian.com
Yaroslav Halchenko                 www.ohloh.net/accounts/yarikoptic

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: problem with cherry-picking a commit which comes before introducing a new submodule
  2011-01-07 23:00     ` Jonathan Nieder
  2011-01-07 23:48       ` Yaroslav Halchenko
@ 2011-01-08  0:01       ` Yaroslav Halchenko
  2011-01-11 13:27         ` Jonathan Nieder
  1 sibling, 1 reply; 10+ messages in thread
From: Yaroslav Halchenko @ 2011-01-08  0:01 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: git, Elijah Newren

message is different -- result the same:
I: writing typescript to /home/yoh/.tmp/script.git-cherry-pick.17386.20110107.1857 ...
% git --version
git version 1.7.4.rc1
% git reset --hard
HEAD is now at 855981d just placeholders in the abstract
% export GIT_MERGE_VERBOSITY=5
% git cherry-pick todonotloose
CONFLICT (file/directory): There is a directory with name frontiers/code in a00c497... Initial draft for HBM abstract.. Adding frontiers/code as frontiers/code~HEAD
error: could not apply a00c497... Initial draft for HBM abstract.
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add <paths>' or 'git rm <paths>'
hint: and commit the result with 'git commit -c a00c497'
% git status      
# On branch master
# Changes to be committed:
#   (use "git reset HEAD <file>..." to unstage)
#
#   new file:   poster-hbm2011_neurodebian/abstract.txt
#   modified:   poster-hbm2011_neurodebian/jb.txt
#
# Unmerged paths:
#   (use "git reset HEAD <file>..." to unstage)
#   (use "git add/rm <file>..." as appropriate to mark resolution)
#
#   added by us:        frontiers/code
#
% git reset --hard
HEAD is now at 855981d just placeholders in the abstract
%         base=todonotloose^
%         next=todonotloose
%         head=HEAD
% 
%         git merge-recursive $base -- $head $next
Merging HEAD with todonotloose
Merging:
855981d just placeholders in the abstract
a00c497 Initial draft for HBM abstract.
found 1 common ancestor(s):
4708e24 minor moves around
CONFLICT (file/directory): There is a directory with name frontiers/code in todonotloose. Adding frontiers/code as frontiers/code~HEAD
%         git ls-files -u;        # after the merge
160000 a2b57871d2d79bef06ba6214739d82b9a63772a8 2   frontiers/code
zsh: command not found: #
%         git diff-tree todonotloose
a00c497fa399c00486c97121ed0b8fda72c7ce47
:040000 040000 40427e34a1ff89c458f2a5f262a108d46b4fa004 c7ba91028b1cef63f4f7eef70f0c4054b31e92b6 M  poster-hbm2011_neurodebian
%         git diff-tree todonotloose^ HEAD
:100644 100644 378e1379ec5ebb7abac59fec162b7238b5846525 c39ced763aeb5fd352cecd6fef1bfc40471f2246 M  .gitmodules
:000000 040000 0000000000000000000000000000000000000000 141dbc1bfe1be2eab77f04ca03f6f28feb372cca A  challenge-execpapers
:040000 040000 401fd66867de412b8653dc3a698bbaa45441bec1 ee190f09786f324abdda6e7a36e8278c201a20a0 M  frontiers
:040000 040000 26c884a67efb55bdf96d7453d9acd50cee36ae90 ad3e829d15b302c4342a6b2a9fb5dfede0ed77c9 M  sty



On Fri, 07 Jan 2011, Jonathan Nieder wrote:

> Yaroslav Halchenko wrote [abbreviated]:

> > Merging HEAD with todonotloose
> > Merging:
> > 855981d just placeholders in the abstract
> > a00c497 Initial draft for HBM abstract.
> > CONFLICT (file/directory): There is a directory with name frontiers/code in todonotloose. Adding frontiers/code as
> > +frontiers/code~HEAD
> > %         git ls-files -u
> > 160000 a2b5787 2   frontiers/code
> > %         git diff-tree todonotloose
> > a00c497
> > :040000 040000 40427e34 c7ba910 M	poster-hbm2011_neurodebian
> > %         git diff-tree todonotloose^ HEAD
> > :100644 100644 378e137 c39ced7 M	.gitmodules
> > :000000 040000 0000000 141dbc1 A	challenge-execpapers
> > :040000 040000 401fd66 ee190f0 M	frontiers
> > :040000 040000 26c884a ad3e829 M	sty

> One more piece of protocol: what git version are you using?  The
> release notes mention a fix in this area in v1.7.3[1]:

>  * "git merge -s recursive" (which is the default) did not handle cases
>    where a directory becomes a file (or vice versa) very well.

> Hopefully this is that.  In any case, sounds like a bug.

> (Hopefully someone else can comment on why cherry-pick uses the
> merge machinery to notice conflicts that would not be clear from
> the patch alone.)

> Thanks again.
> Jonathan

> [1] There is an updated Debian source package at [2].  Or, probably
> faster: one can use the build result in bin-wrappers/ from a git.git
> clone in place.
> [2] http://mentors.debian.net/debian/pool/main/g/git/git_1.7.4~rc1-0.1.dsc


-- 
=------------------------------------------------------------------=
Keep in touch                                     www.onerussian.com
Yaroslav Halchenko                 www.ohloh.net/accounts/yarikoptic

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: problem with cherry-picking a commit which comes before introducing a new submodule
  2011-01-08  0:01       ` Yaroslav Halchenko
@ 2011-01-11 13:27         ` Jonathan Nieder
  2011-01-18 16:02           ` Yaroslav Halchenko
  0 siblings, 1 reply; 10+ messages in thread
From: Jonathan Nieder @ 2011-01-11 13:27 UTC (permalink / raw)
  To: Yaroslav Halchenko; +Cc: git, Elijah Newren

Yaroslav Halchenko wrote [abbreviated]:

> CONFLICT (file/directory): There is a directory with name frontiers/code in todonotloose. Adding frontiers/code as frontiers/code~HEAD
> % git ls-files -u
> 160000 a2b5787 2   frontiers/code
> % git diff-tree todonotloose
> a00c497
> :040000 040000 40427e3 c7ba910 M  poster-hbm2011_neurodebian
> % git diff-tree todonotloose^ HEAD
> :100644 100644 378e137 c39ced7 M  .gitmodules
> :000000 040000 0000000 141dbc1 A  challenge-execpapers
> :040000 040000 401fd66 ee190f0 M  frontiers
> :040000 040000 26c884a ad3e829 M  sty

Here is what happens.

In the heart of merge_trees:

	/*
	 * If there are D/F conflicts, and the paths currently exist
	 * in the working copy as a file, remove them to make room
	 * for the corresponding directory.  Such paths will later be
	 * processed in process_df_entry() at the end.
	 *
	 * If the corresponding directory ends up being removed by the
	 * merge, then the file will be reinstated at that time;
	 * otherwise, if the file is not supposed to be removed by the
	 * merge, the contents of the file will be placed in another
	 * unique filename.
	 */
	make_room_for_directories_of_df_conflicts(o, entries);

In this case I suppose it is rather a directory/submodule conflict; in
any case, there are no regular files involved, so this logic does not
kick in and the directory is left alone.

Next comes rename handling, which is irrelevant for our purposes.

Next comes the per entry merge.

	/*
	 * Per entry merge.  D/F conflicts are deferred so files
	 * contained in such a directory can be resolved first.
	 */
	for (i = 0; i < entries->nr; i++) {
		const char *path = entries->items[i].string;
		struct stage_data *e = entries->items[i].util;
		if (!e->processed
			&& !process_entry(o, path, e))
			clean = 0;
	}

This is case B: "added in one" (like all directories, the
frontiers/code directory does not have an index entry, while the
submodule does have one).  Since that path is in the current directory
set, it is deferred for later processing.

Next comes the per entry merge for D/F conflicts (process_df_entry in
merge-recursive.c).  This is the case "directory -> (directory,
file)".  Unfortunately the check that the old and new directories
match is not implemented.  Even worse, git checks for a directory
(which was not moved out of the way before) and does not realize that
a submodule might be another reason for a directory in the worktree.
In any event, we get a spurious conflict.

Thanks, that was interesting (no patch yet, alas).

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: problem with cherry-picking a commit which comes before introducing a new submodule
  2011-01-11 13:27         ` Jonathan Nieder
@ 2011-01-18 16:02           ` Yaroslav Halchenko
  2011-01-18 16:08             ` Andreas Ericsson
  0 siblings, 1 reply; 10+ messages in thread
From: Yaroslav Halchenko @ 2011-01-18 16:02 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: git, Elijah Newren


On Tue, 11 Jan 2011, Jonathan Nieder wrote:
> ...
> a submodule might be another reason for a directory in the worktree.
> In any event, we get a spurious conflict.
> Thanks, that was interesting (no patch yet, alas).

is there a way to memorize this issue somewhere (bug tracking/TODO/etc)
where this issue could be recorded so it doesn't get forgotten? ;)

-- 
Yaroslav O. Halchenko
Postdoctoral Fellow,   Department of Psychological and Brain Sciences
Dartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755
Phone: +1 (603) 646-9834                       Fax: +1 (603) 646-1419
WWW:   http://www.linkedin.com/in/yarik        

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: problem with cherry-picking a commit which comes before introducing a new submodule
  2011-01-18 16:02           ` Yaroslav Halchenko
@ 2011-01-18 16:08             ` Andreas Ericsson
  2011-01-18 16:20               ` Yaroslav Halchenko
  0 siblings, 1 reply; 10+ messages in thread
From: Andreas Ericsson @ 2011-01-18 16:08 UTC (permalink / raw)
  To: Yaroslav Halchenko; +Cc: Jonathan Nieder, git, Elijah Newren

On 01/18/2011 05:02 PM, Yaroslav Halchenko wrote:
> 
> On Tue, 11 Jan 2011, Jonathan Nieder wrote:
>> ...
>> a submodule might be another reason for a directory in the worktree.
>> In any event, we get a spurious conflict.
>> Thanks, that was interesting (no patch yet, alas).
> 
> is there a way to memorize this issue somewhere (bug tracking/TODO/etc)
> where this issue could be recorded so it doesn't get forgotten? ;)
> 

It will be stored in the hive-mind of the mailing list participants.
It's quite a bit better than it sounds actually.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Considering the successes of the wars on alcohol, poverty, drugs and
terror, I think we should give some serious thought to declaring war
on peace.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: problem with cherry-picking a commit which comes before introducing a new submodule
  2011-01-18 16:08             ` Andreas Ericsson
@ 2011-01-18 16:20               ` Yaroslav Halchenko
  0 siblings, 0 replies; 10+ messages in thread
From: Yaroslav Halchenko @ 2011-01-18 16:20 UTC (permalink / raw)
  To: Andreas Ericsson; +Cc: Jonathan Nieder, git, Elijah Newren


On Tue, 18 Jan 2011, Andreas Ericsson wrote:
> > is there a way to memorize this issue somewhere (bug tracking/TODO/etc)
> > where this issue could be recorded so it doesn't get forgotten? ;)
> It will be stored in the hive-mind of the mailing list participants.
> It's quite a bit better than it sounds actually.

ok -- offload completed then, I am cleaning up my memory bank

-- 
=------------------------------------------------------------------=
Keep in touch                                     www.onerussian.com
Yaroslav Halchenko                 www.ohloh.net/accounts/yarikoptic

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2011-01-18 16:20 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-01-07 17:24 problem with cherry-picking a commit which comes before introducing a new submodule Yaroslav Halchenko
2011-01-07 18:15 ` Jonathan Nieder
2011-01-07 18:32   ` Yaroslav Halchenko
2011-01-07 23:00     ` Jonathan Nieder
2011-01-07 23:48       ` Yaroslav Halchenko
2011-01-08  0:01       ` Yaroslav Halchenko
2011-01-11 13:27         ` Jonathan Nieder
2011-01-18 16:02           ` Yaroslav Halchenko
2011-01-18 16:08             ` Andreas Ericsson
2011-01-18 16:20               ` Yaroslav Halchenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).