git-subtree: directory mismatch

Hi,

I've come across this weird behaviour using git-subtree: when updating a directory created by git-subtree, the content of some (almost) random neighbour directory gets replaced, instead of the directory specified by "git-subtree --prefix".

mkdir /tmp/testrepo cd /tmp/testrepo git init . mkdir -p modules/cognac/{manifests,files} mkdir -p modules/cognac/manifests/{classes,definitions} touch modules/cognac/README modules/cognac/manifests/classes/foo.pp modules/cognac/manifests/definitions/bar.pp modules/cognac/files/blabla git add modules/ git commit -m "test"

git remote add mapserver git://github.com/camptocamp/puppet-mapserver.git git remote update git-subtree add --prefix modules/mapserver 97f5985b # an older commit

# So far so good, I have 2 subdirectories in modules/ with completely # different content.

# Then, the content of modules/cognac/ gets completely replaced by the # content of HEAD from git://github.com/camptocamp/puppet-mapserver.git. # modules/mapserver still contains the content of commit 97f5985b.

git-subtree pull --prefix modules/mapserver mapserver master

 * branch            master     -> FETCH_HEAD
Removing modules/cognac/files/blabla
Removing modules/cognac/manifests/classes/foo.pp
Removing modules/cognac/manifests/definitions/bar.pp
Merge made by subtree.
 modules/cognac/files/apt-preferences               |    3 +
 modules/cognac/files/epsg.C2C                      | 5445 ++++++++++++++++++++
 modules/cognac/files/epsg.legacy                   | 5445 ++++++++++++++++++++
 modules/cognac/files/epsg.minimal                  |   14 +
 modules/cognac/files/etc/apt/preferences-20070910  |    3 +
 modules/cognac/files/etc/apt/preferences-20080225  |   35 +
 modules/cognac/files/etc/apt/preferences-v5-2      |   35 +
 modules/cognac/manifests/classes/epsg.pp           |   21 +
 .../cognac/manifests/classes/mapserver-debian.pp   |   82 +
 modules/cognac/manifests/classes/v5.pp             |   46 +
 modules/cognac/manifests/init.pp                   |    8 +
 11 files changed, 11137 insertions(+), 0 deletions(-)
 create mode 100644 modules/cognac/files/apt-preferences
 delete mode 100644 modules/cognac/files/blabla
 create mode 100755 modules/cognac/files/epsg.C2C
 create mode 100755 modules/cognac/files/epsg.legacy
 create mode 100644 modules/cognac/files/epsg.minimal
 create mode 100644 modules/cognac/files/etc/apt/preferences-20070910
 create mode 100644 modules/cognac/files/etc/apt/preferences-20080225
 create mode 100644 modules/cognac/files/etc/apt/preferences-v5-2
 create mode 100644 modules/cognac/manifests/classes/epsg.pp
 delete mode 100644 modules/cognac/manifests/classes/foo.pp
 create mode 100644 modules/cognac/manifests/classes/mapserver-debian.pp
 create mode 100644 modules/cognac/manifests/classes/v5.pp
 delete mode 100644 modules/cognac/manifests/definitions/bar.pp
 create mode 100644 modules/cognac/manifests/init.pp

I was not able to reproduce this bug in any of the following cases:
 - if the README file wasn't empty
 - if modules/cognac is renamed to something else
 - if modules/cognac doesn't have the same sub-directories than
   modules/mapserver

This bug happens using git 1.5.5.6 as well as 1.6.5. It seems to happen when git-subtree calls "git merge -s subtree".

Maybe something is broken in this puppet-mapserver.git repository I use ? I've encountered the same problem with other directories too.

Any idea ?

Thanks ! Marc

On Tue, Nov 24, 2009 at 2:53 PM, Marc Fournier <marc.fournier@camptocamp.com> wrote:

Yup. This is basically a bug in "git merge -s subtree": it guesses which subtree to merge into, rather than actually taking a prefix parameter. I've been meaning to either submit a patch for this, or find a way to work around it.

This doesn't usually happen once your project is relatively mature (ie. doesn't have blank or "default" template files in it) since then the auto-guessing gets more reliable. But there's no good reason to do the auto-guessing, so it would be best to do this "properly."

Sorry that I haven't had time to fix it yet...

Avery

Quoting Avery Pennarun <apenwarr@gmail.com>

Probably you can save time by using what was already done

http://thread.gmane.org/gmane.comp.version-control.git/76650/focus=89021

On Tue, Nov 24, 2009 at 6:08 PM, Nanako Shiraishi <nanako3@lavabit.com> wrote:

Hi Nanako,

I've read through the thread (I do remember skimming it awhile ago) but can't find patches for the syntax actually under discussion. I found a patch that introduces "-s theirs", which was summarily shot down in favour of adding -X support, but I can't find any actual patch for this. Moreover, git-merge seems to have been ported to C since then, so I guess it wouldn't apply anyway. And I can't find any implementation at all for the discussed "-Xsubtree=" option, which I'm guessing didn't actually ever get done.

Am I missing something? It looks fairly easy to throw in anyway, but it's even easier if someone has already done it :)

Thanks,

Avery

Avery Pennarun <apenwarr@gmail.com> writes:

I very much prefer gmane threading when following discussion over all the other mail archives, but this shows one thing I really dislike about it.

It is not easy to find a near-by thread when looking at an old article, and you have to be willing to bisect the page number at the right hand side of the web UI. Often a patch series is posted as a separate thread after a discussion reaches conclusion or identifies an issue to solve, and the real patch series lives in a near-by thread. Very inconvenient.

I don't know how Nana digs up older discussion; maybe she knows better ways.

In my primary repository, I have an archive of mothballed branches kept with this alias:

    [alias]
    hold = "!sh -c 'git update-ref refs/hold/$1 refs/heads/$1 && git branch -D $1' -"

and found this series in there. It applies to v1.6.0-rc0~245 (no, I won't be rebasing this myself---I don't have time for that while preparing for the pre-release feature freeze).

    f7713ce Document that merge strategies can now take their own options
    7eda236 Make "subtree" part more orthogonal to the rest of merge-recursive.
    e416d61 Teach git-pull to pass -X<option> to git-merge
    09f7d22 Teach git-merge to pass -X<option> to the backend strategy module
    904288c git-merge-recursive-{ours,theirs}
    e0aafb4 git-merge-file --ours, --theirs
    -d3e977b Merge branch 'maint'

Look at http://github.com/gitster/git/commits/jc/merge-theirs/

On Wed, Nov 25, 2009 at 2:31 PM, Junio C Hamano <gitster@pobox.com> wrote:

[...]

I also tried simply searching for things like 'git "-xsubtree"' in google, with no luck. But thanks for the link.

I did a test merge and it looks like a ton of conflicts, but they seem to be pretty understandable ones, at least. I don't mind doing the rebase and resubmitting the patches, since it's sure less work than figuring out how to do it from scratch myself. Some questions though:

- What was the reason this never got merged?  What changes are needed
to rectify that?

- Considering the earlier discussion, do we want to leave out the
actual -Xtheirs feature and just have -Xours and -Xsubtree?

- If I rebase them and the changes turn out to be minimal, do they
still need a signed-off-by Junio?  (He obviously owns part of the
copyright and has previously signed off, but he also won't have signed
off the rebased patches verbatim, so I'm confused.)

Thanks,

Avery

Avery Pennarun <apenwarr@gmail.com> writes:

I didn't _find_ the link ;-) I just pushed it out a few minutes ago, after looking for strings that appear in messages of these commits. The series was done over a few weeks, and would have been very painful to find from the gmane archive.

I do not remember there was any real reason. I do remember some people didn't like -X<option> syntax but I don't think there was any solid counterproposal to achieve a similar goal to satisfy the need to pass arbitrary parameters to the merge strategy backends.

Both -Xtheirs and -Xours have the same degree of badness in the context of "source code management", but there was a real-world use case that would have benefitted from -Xours recently.

  cf. http://thread.gmane.org/gmane.comp.version-control.git/131902/focus=132920

If -Xours goes in, so should -Xtheirs, I think, because Peter's "web tree" example could merge in both ways (i.e. he could be pulling from web tree into his private area and then cleaning things up before pushing the result back).

"minimal" by definition means that you ased your work on mine and I still have the copyright to the change as a co-author together with you. We both need to certify that the change is made under DCO.

Hi,

On Wed, 25 Nov 2009, Junio C Hamano wrote:

I took exception to the awkward way to specify the option. A strong hint just how awkward -X<option> is: it is hard to implement using parse-options.

There is no real reason to do it properly, i.e. with an option --merge-option <option> or --backend-option <option> or -X <option> (even if "-X" is not really an expressive name, to be real honest).

BTW the same goes for -S<string>, but unfortunately, we have to keep it to maintain backwards compatibility.

Ciao, Dscho

On Wed, Nov 25, 2009 at 6:17 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

I read the earlier thread and I still don't quite understand this point. What makes it difficult? X seems just like any other option that takes a string parameter (which is easily supported by getopt at least; I haven't played with parse_options but I certainly hope it's as good as getopt).

The string parameter (say, $x) is translated directly into "--$x" and passed as exactly one argument of the argv array to the subprogram. Easy, right? Plus Junio already wrote a patch to do it (which I'm in the process of forward-porting) and it seems pretty straightforward.

Perhaps I'm missing something.

Thanks,

Avery

On 2009.11.25 18:20:45 -0500, Avery Pennarun wrote:

I guess it's -Xfoo vs. -X foo (note the space). Same deal with log -S<string>. "git log -S foo" would look for an empty string (I guess) being added/removed in some commit in foo's history, while "git log -Sfoo" looks for "foo" being added/remove in HEAD's history.

Björn

2009/11/25 Björn Steinbrink <B.Steinbrink@gmx.de>:

Oh. Ouch. Isn't it standard for options-that-take-string-parameters to always *require* that parameter? This seems to be how getopt works. Basically "-X foo" is the same as "-Xfoo" in that system.

If parse_options doesn't already do this, should I try to add it somehow?

Avery

On Tue, Nov 24, 2009 at 04:48:26PM -0500, Avery Pennarun wrote:

I see. What sort of criteria does git-merge use to do this auto-guessing ?

I'm hoping to find a way to workaround this problem. I have a couple of subtrees I'm completely unable to update. The best I found is: "git rm -fr ..." followed by "git subtree add ..." again.

The thing is I use dozens of different "modules" in my project, all merged into using git-subtree. All these modules have the same structure, and most of them have quite minimal content and won't really evolve more. In my case, evolution will mean adding even more modules.

If I understand the thread Nanako Shiraishi pointed us to, a patch implementing a prefix to "git merge -s subtree" is available somewhere but hasn't been added in any official release yet ?

Marc