libreoffice merge issue ...

libreoffice merge issue ...

[Michael Meeks]

Hi guys,

We are having quite some fun merging git branches with LibreOffice, and
I stumbled over this just now with master git with hash:
00e6ee724640701b32aca27cc930fd6409c87ae2

Setup (some large repos):

	git clone git://anongit.freedesktop.org/libreoffice/libs-core
	git checkout integration/dev300_m98
	git remote add stage git://anongit.freedesktop.org/libreoffice/staging/@REPO@
	git fetch stage

	Test[1]:

	git merge stage/premerge/dev300_m98
	git diff idl/source/cmptools/lex.cxx

	yields:

@@@ -147,11 -147,7 +147,15 @@@ SvToken & SvToken::operator = ( const S
  *************************************************************************/
  void SvTokenStream::InitCtor()
  {
++<<<<<<< HEAD
 +#ifdef DOS
 +    SetCharSet( CHARSET_ANSI );
 +#else
      SetCharSet( gsl_getSystemTextEncoding() );
 +#endif
++=======
++    SetCharSet( gsl_getSystemTextEncoding() );
++>>>>>>> stage/premerge/dev300_m98
      aStrTrue  = "TRUE";
      aStrFalse = "FALSE";
      nLine       = nColumn = 0;

	With the above master hash; whereas with v1.7.3.4 it yields nothing (as
it should IMHO) - we havn't edited things around that chunk in master.

	That is slightly concerning; thoughts much appreciated. Incidentally,
the whole 'make install' installs into ~/bin was extremely unexpected
and yielded 30minutes of pain trying to work out what was installed
where and why, and the interaction with --prefix, and ... now it seems I
should always run rehash; git --version before any command, and sanity
check things ;-)

	Thanks,

		Michael.

[1] - potentially you need:
[merge]
    renamelimit = 20000
in your ~/.gitconfig
-- 
 michael.meeks@novell.com  <><, Pseudo Engineer, itinerant idiot

Re: libreoffice merge issue ...

[Norbert Thiebaud]

On Mon, Feb 14, 2011 at 10:07 AM, Michael Meeks
<michael.meeks@novell.com> wrote:
> Hi guys,
>
> We are having quite some fun merging git branches with LibreOffice, and
> I stumbled over this just now with master git with hash:
> 00e6ee724640701b32aca27cc930fd6409c87ae2
>
> Setup (some large repos):
>
>        git clone git://anongit.freedesktop.org/libreoffice/libs-core
>        git checkout integration/dev300_m98
>        git remote add stage git://anongit.freedesktop.org/libreoffice/staging/@REPO@

correction: this should read
        git remote add stage
git://anongit.freedesktop.org/libreoffice/staging/libs-core

or you should use ./g instead of git for all these 3 git operations

Norbert


>        git fetch stage
>
>        Test[1]:
>
>        git merge stage/premerge/dev300_m98
>        git diff idl/source/cmptools/lex.cxx
>
>        yields:
>
> @@@ -147,11 -147,7 +147,15 @@@ SvToken & SvToken::operator = ( const S
>  *************************************************************************/
>  void SvTokenStream::InitCtor()
>  {
> ++<<<<<<< HEAD
>  +#ifdef DOS
>  +    SetCharSet( CHARSET_ANSI );
>  +#else
>      SetCharSet( gsl_getSystemTextEncoding() );
>  +#endif
> ++=======
> ++    SetCharSet( gsl_getSystemTextEncoding() );
> ++>>>>>>> stage/premerge/dev300_m98
>      aStrTrue  = "TRUE";
>      aStrFalse = "FALSE";
>      nLine       = nColumn = 0;
>
>        With the above master hash; whereas with v1.7.3.4 it yields nothing (as
> it should IMHO) - we havn't edited things around that chunk in master.
>
>        That is slightly concerning; thoughts much appreciated. Incidentally,
> the whole 'make install' installs into ~/bin was extremely unexpected
> and yielded 30minutes of pain trying to work out what was installed
> where and why, and the interaction with --prefix, and ... now it seems I
> should always run rehash; git --version before any command, and sanity
> check things ;-)
>
>        Thanks,
>
>                Michael.
>
> [1] - potentially you need:
> [merge]
>    renamelimit = 20000
> in your ~/.gitconfig
> --
>  michael.meeks@novell.com  <><, Pseudo Engineer, itinerant idiot
>
>
>

Re: libreoffice merge issue ...

[Ferry Huberts]

On 02/14/2011 05:07 PM, Michael Meeks wrote:
> Hi guys,
> 
> We are having quite some fun merging git branches with LibreOffice, and
> I stumbled over this just now with master git with hash:
> 00e6ee724640701b32aca27cc930fd6409c87ae2
> 
> Setup (some large repos):
> 
> 	git clone git://anongit.freedesktop.org/libreoffice/libs-core
> 	git checkout integration/dev300_m98
> 	git remote add stage git://anongit.freedesktop.org/libreoffice/staging/@REPO@
> 	git fetch stage
> 
> 	Test[1]:
> 
> 	git merge stage/premerge/dev300_m98

the merge has detected a conflict and has annotated the conflicted
file(s). at least one of the conflicting files is
idl/source/cmptools/lex.cxx (you might want to do 'git mergetool' after
you have setup your merge tool)

> 	git diff idl/source/cmptools/lex.cxx
> 

you're diffing the 'unmerged, annotated' file against the original file
before the merge.

your merge is not yet complete, you'll have to resolve the conflicts first
and then commit


> 	yields:
> 
> @@@ -147,11 -147,7 +147,15 @@@ SvToken & SvToken::operator = ( const S
>   *************************************************************************/
>   void SvTokenStream::InitCtor()
>   {
> ++<<<<<<< HEAD
>  +#ifdef DOS
>  +    SetCharSet( CHARSET_ANSI );
>  +#else
>       SetCharSet( gsl_getSystemTextEncoding() );
>  +#endif
> ++=======
> ++    SetCharSet( gsl_getSystemTextEncoding() );
> ++>>>>>>> stage/premerge/dev300_m98


this is the annotation of the conflict

grtz

-- 
Ferry Huberts

Re: libreoffice merge issue ...

[Norbert Thiebaud]

On Mon, Feb 14, 2011 at 11:26 AM, Ferry Huberts <mailings@hupie.com> wrote:
>
>
> On 02/14/2011 05:07 PM, Michael Meeks wrote:
>> Hi guys,
>>
>> We are having quite some fun merging git branches with LibreOffice, and
>> I stumbled over this just now with master git with hash:
>> 00e6ee724640701b32aca27cc930fd6409c87ae2
>>
[...]
>>       yields:
>>
>> @@@ -147,11 -147,7 +147,15 @@@ SvToken & SvToken::operator = ( const S
>>   *************************************************************************/
>>   void SvTokenStream::InitCtor()
>>   {
>> ++<<<<<<< HEAD
>>  +#ifdef DOS
>>  +    SetCharSet( CHARSET_ANSI );
>>  +#else
>>       SetCharSet( gsl_getSystemTextEncoding() );
>>  +#endif
>> ++=======
>> ++    SetCharSet( gsl_getSystemTextEncoding() );
>> ++>>>>>>> stage/premerge/dev300_m98
>
>
> this is the annotation of the conflict

The point is that there should not have been a conflict to start with.
git 1.7.3.4 agree that there is no conflict.

Norbert

>
> grtz
>
> --
> Ferry Huberts
>

Re: libreoffice merge issue ...

[Jeff King]

On Mon, Feb 14, 2011 at 04:07:15PM +0000, Michael Meeks wrote:

> Setup (some large repos):
> 
> 	git clone git://anongit.freedesktop.org/libreoffice/libs-core
> 	git checkout integration/dev300_m98
> 	git remote add stage git://anongit.freedesktop.org/libreoffice/staging/@REPO@
> 	git fetch stage
> 
> 	Test[1]:
> 
> 	git merge stage/premerge/dev300_m98
> 	git diff idl/source/cmptools/lex.cxx
> 
> 	yields:
> [a conflict in idl/source/cmptools/lex.cxx]
> 
> 	With the above master hash; whereas with v1.7.3.4 it yields nothing (as
> it should IMHO) - we havn't edited things around that chunk in master.

Interesting. I looked at both sides of the merge and the merge base, and
there definitely should not be a conflict there. The regression bisects
to 83c9031 (unpack_trees(): skip trees that are the same in all input,
2010-12-22). Reverting that commit makes the problem go away.

-Peff

Re: libreoffice merge issue ...

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Interesting. I looked at both sides of the merge and the merge base, and
> there definitely should not be a conflict there. The regression bisects
> to 83c9031 (unpack_trees(): skip trees that are the same in all input,
> 2010-12-22). Reverting that commit makes the problem go away.

Thanks; I was wondering about this myself but you bisected it faster.

Will revert.

Re: libreoffice merge issue ...

[Jeff King]

On Tue, Feb 15, 2011 at 10:46:03AM -0800, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > Interesting. I looked at both sides of the merge and the merge base, and
> > there definitely should not be a conflict there. The regression bisects
> > to 83c9031 (unpack_trees(): skip trees that are the same in all input,
> > 2010-12-22). Reverting that commit makes the problem go away.
> 
> Thanks; I was wondering about this myself but you bisected it faster.
> 
> Will revert.

One other thing I noticed during the bisect: when using a version of git
containing 83c9031, the merge took a lot longer. As in, 13 seconds with
v1.7.3 versus 69 seconds with master.

That may simply be because the bug being demonstrated causes us to
erroneously do more file-level merging than we would otherwise need to.
But I thought it worth mentioning in case you want to delve into fixing
the code.

-Peff

Re: libreoffice merge issue ...

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

>> Thanks; I was wondering about this myself but you bisected it faster.
>> 
>> Will revert.
>
> One other thing I noticed during the bisect: when using a version of git
> containing 83c9031, the merge took a lot longer. As in, 13 seconds with
> v1.7.3 versus 69 seconds with master.
>
> That may simply be because the bug being demonstrated causes us to
> erroneously do more file-level merging than we would otherwise need to.

Yeah, the reverted 83c9031 (unpack_trees(): skip trees that are the same
in all input, 2010-12-22) also seems to have seriously broken intermediate
merge merge-recursive makes.  I actually recall scratching my head when I
made 00e6ee7 (Merge branch 'maint', 2011-02-11) that was causing add/add
conflict when it shouldn't.  It turns out that quite a lot of entries were
missing in contrib/ area from the virtual common ancestry tree synthesized
by merge-recursive that called into the botched unpack_trees()---it of
course would result in add/add conflict if a merge is done using such a
tree as the common.

No, I haven't had a chance nor energy to dig further than what I reported
above.

Re: libreoffice merge issue ...

[Damien Wyart]

Hi,

Sorry to wake up an old thread.

* Junio C Hamano <gitster@pobox.com> [2011-02-16 13:30]:
> Yeah, the reverted 83c9031 (unpack_trees(): skip trees that are the
> same in all input, 2010-12-22) also seems to have seriously broken
> intermediate merge merge-recursive makes. I actually recall scratching
> my head when I made 00e6ee7 (Merge branch 'maint', 2011-02-11) that
> was causing add/add conflict when it shouldn't. It turns out that
> quite a lot of entries were missing in contrib/ area from the virtual
> common ancestry tree synthesized by merge-recursive that called into
> the botched unpack_trees()---it of course would result in add/add
> conflict if a merge is done using such a tree as the common.

> No, I haven't had a chance nor energy to dig further than what
> I reported above.

Out of curiosity, I would like to know if digging further into this
issue is still on your TODO list. I feel understanding exactly what was
wrong in 83c9031 would be interesting ; having just the revert is a bit
frustrating.

The initial optimization in 83c9031 seemed right at first glance, so
I would be interesting in having a more final answer to this.


Many thanks in advance,
-- 
Damien Wyart

Re: libreoffice merge issue ...

[Jeff King]

On Thu, Apr 21, 2011 at 04:01:32PM +0200, Damien Wyart wrote:

> * Junio C Hamano <gitster@pobox.com> [2011-02-16 13:30]:
> > Yeah, the reverted 83c9031 (unpack_trees(): skip trees that are the
> > same in all input, 2010-12-22) also seems to have seriously broken
> > intermediate merge merge-recursive makes. I actually recall scratching
> > my head when I made 00e6ee7 (Merge branch 'maint', 2011-02-11) that
> > was causing add/add conflict when it shouldn't. It turns out that
> > quite a lot of entries were missing in contrib/ area from the virtual
> > common ancestry tree synthesized by merge-recursive that called into
> > the botched unpack_trees()---it of course would result in add/add
> > conflict if a merge is done using such a tree as the common.
> 
> > No, I haven't had a chance nor energy to dig further than what
> > I reported above.
> 
> Out of curiosity, I would like to know if digging further into this
> issue is still on your TODO list. I feel understanding exactly what was
> wrong in 83c9031 would be interesting ; having just the revert is a bit
> frustrating.
> 
> The initial optimization in 83c9031 seemed right at first glance, so
> I would be interesting in having a more final answer to this.

I didn't dig further, but looking at 83c9031 with a fresh set of eyes, I
think that merge-recursive falls under the "exceptions" that Junio
listed in the commit message. That is, he indicates correctly that we
cannot use this optimization for "reset --hard" or for checking out for
the first time.

Similarly, merge-recursive is going to do many 3-way merges between
trees into an empty index (when merging virtual ancestors), and we need
to unpack those subtrees even if they are all the same.  But given the
conditions added by 83c9031 in unpack-trees.c:fast_forward_merge, I
don't see anything preventing the optimization in how merge-recursive
calls into unpack-trees.

We do a discard_cache() right before doing the 3-way merge for virtual
ancestors. So we will see that there are no entries in our current index
for that directory. That may be a clue that the optimization should not
be used.

-Peff