git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [Bug] git stash generates a different diff then other commands (diff, add, etc) resulting in merge conflicts!
@ 2013-08-08  7:07 Luke San Antonio
  2013-08-08 20:54 ` Phil Hord
  0 siblings, 1 reply; 5+ messages in thread
From: Luke San Antonio @ 2013-08-08  7:07 UTC (permalink / raw)
  To: git

Hi, my name's Luke!

Today, I had a problem merging a stash after immediately creating it.
This is exactly what I did!

git stash save --keep-index
git stash pop

And BAM! Merge conflict! This was especially weird because my file had
this in it (taken directly from my code!)

<<<<<<< Updated upstream
     *
     * It should be used and passed along to member objects by GameStates!
     *
=======
     *
     * It should be used and passed along to member objects by GameStates!
     *
>>>>>>> Stashed changes

They are exactly the same!

Oh, by the way, I should mention that I did not edit any hunks to get
the index the way I wanted it, I have read that doing that causes
merge conflicts similar to this!

Then I got a hunch! I realized that git will refrain from applying a
hunk if it finds it already was applied exactly (that's correct
right?)... So I thought, maybe the patches are similar (represent the
same changes) but aren't *exactly* the same.

I was right!

After saving the stash I take a look at the diff: (git stash show -p)

     /*!
-     * \brief The default font renderer, global to all who have a pointer to
-     * the Game class.
+     * \brief The font renderer implementation, obtained from the config file.
+     *
+     * It should be used and passed along to member objects by GameStates!
      *
-     * It need not be used at all!
+     * \note It can be cached, but not between GameStates, meaning it should be
+     * cached again every time a new GameState is constructed!
      */

After that, I take a look at the diff in my index: (git diff --staged)

     /*!
-     * \brief The default font renderer, global to all who have a pointer to
-     * the Game class.
+     * \brief The font renderer implementation, obtained from the config file.
      *
-     * It need not be used at all!
+     * It should be used and passed along to member objects by GameStates!
+     *
+     * \note It can be cached, but not between GameStates, meaning it should be
+     * cached again every time a new GameState is constructed!
      */

Aha! A difference, a difference so tiny it went unnoticed by me, but not by git!

Now the housekeeping:

What I wanted to do:

Apply a stash on top of a 'kept' index.

What I did:

git stash save --keep-index
git stash pop

What I saw happen:

A merge conflict between the same changes (see above).

What I expected to see:

No merge conflict.

How are these different:

The conflict, which shouldn't happen since the changes introduced were the same!

-------------------------------------------------

It seems to me like the stash command is using a slightly different
diff algorithm...
Can anyone explain to me what's going on under the hood so I can
understand this subtle difference? Does anyone know?

Thanks in advance, I'm sure you all will be very helpful!
- Luke

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

* Re: [Bug] git stash generates a different diff then other commands (diff, add, etc) resulting in merge conflicts!
  2013-08-08  7:07 [Bug] git stash generates a different diff then other commands (diff, add, etc) resulting in merge conflicts! Luke San Antonio
@ 2013-08-08 20:54 ` Phil Hord
  2013-08-12  5:29   ` Luke San Antonio
  0 siblings, 1 reply; 5+ messages in thread
From: Phil Hord @ 2013-08-08 20:54 UTC (permalink / raw)
  To: Luke San Antonio; +Cc: git@vger.kernel.org

On Thu, Aug 8, 2013 at 3:07 AM, Luke San Antonio
<lukesanantonio@gmail.com> wrote:
>
> Hi, my name's Luke!
>
> Today, I had a problem merging a stash after immediately creating it.
> This is exactly what I did!
>
> git stash save --keep-index
> git stash pop
>
> And BAM! Merge conflict! This was especially weird because my file had
> this in it (taken directly from my code!)
>

Luke,

I think the issue is that your working directory receives your cached
file when you say 'git stash --keep-index'.  When you restore the
stash, your previous working directory now conflicts with your new
working directory, but neither is the same as HEAD.

Here's a test script to demonstrate the issue, I think.  Did I get
this right, Luke?

 # cd /tmp && rm -rf foo
 git init foo && cd foo
 echo "foo" > bar &&  git add bar && git commit -mfoo
 echo "bar" > bar &&  git add bar
 echo "baz" > bar
 echo "Before stash  bar: $(cat bar)"
 git stash --keep-index
 echo "After stash  bar: $(cat bar)"
 git stash apply


The output looks like this:

$  git init foo && cd foo
Initialized empty Git repository in /tmp/foo/.git/
$ git commit --allow-empty -mInitialCommit
[master (root-commit) b5ecc7e] InitialCommit
$ echo "Bar" > bar &&  git add bar && git commit -mBar
[master 16d708b] Bar
 1 file changed, 1 insertion(+)
 create mode 100644 bar
$ echo "bar" > bar &&  git add bar
$  echo "baz" > bar
$  echo "Before stash  bar: $(cat bar)"
Before stash  bar: baz
$  git stash --keep-index
Saved working directory and index state WIP on master: 16d708b Bar
HEAD is now at 16d708b Bar
$  echo "After stash  bar: $(cat bar)"
After stash  bar: bar
$  git stash apply
Auto-merging bar
CONFLICT (content): Merge conflict in bar
Recorded preimage for 'bar'
$ cat bar
<<<<<<< Updated upstream
bar
=======
baz
>>>>>>> Stashed changes



Phil

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

* Re: [Bug] git stash generates a different diff then other commands (diff, add, etc) resulting in merge conflicts!
  2013-08-08 20:54 ` Phil Hord
@ 2013-08-12  5:29   ` Luke San Antonio
       [not found]     ` <CABURp0rWMAs9vT791vp4BEYS-Y9Jmzjmt4bbuB+po8=vkiqUWQ@mail.gmail.com>
  0 siblings, 1 reply; 5+ messages in thread
From: Luke San Antonio @ 2013-08-12  5:29 UTC (permalink / raw)
  To: Phil Hord; +Cc: git@vger.kernel.org

On 08/08/20130 04:54 PM, Phil Hord wrote:
> Luke,
>
> I think the issue is that your working directory receives your cached
> file when you say 'git stash --keep-index'.  When you restore the
> stash, your previous working directory now conflicts with your new
> working directory, but neither is the same as HEAD.
>
> Here's a test script to demonstrate the issue, I think.  Did I get
> this right, Luke?
>
>   # cd /tmp && rm -rf foo
>   git init foo && cd foo
>   echo "foo" > bar &&  git add bar && git commit -mfoo
>   echo "bar" > bar &&  git add bar
>   echo "baz" > bar
>   echo "Before stash  bar: $(cat bar)"
>   git stash --keep-index
>   echo "After stash  bar: $(cat bar)"
>   git stash apply
Actually no, in your script, the bar file has a modification in the working
tree which is in the same hunk as a change applied to the index. In my
project the changes that were added to the index are not modified further
in theworking tree.

--------

Not only that, but I found out why git was generated different patches!
I realized that when I removed a hunk appearing before the merge conflict
from the working tree and index, the merge conflict disappeared! Turns
out, we can forget about stashing for a minute!
First the hunk in my working tree:

@@ -56,12 +56,14 @@
      bool running_ = true;

      /*!
-     * \brief The default font renderer, global to all who have a 
pointer to
-     * the Game class.
+     * \brief The font renderer implementation, obtained from the 
config file.
       *
-     * It need not be used at all!
+     * It should be used and passed along to member objects by GameStates!
+     *
+     * \note It can be cached, but not between GameStates, meaning it 
should be
+     * cached again every time a new GameState is constructed!
       */
-    std::unique_ptr<FontRenderer> font_renderer_ = nullptr;
+    FontRenderer* font_renderer_ = nullptr;

      int run(int argc, char* argv[]);

Most of this is unimportant, but notice the line number spec:@@ -56,12 
+56,14 @@
The line number of this hunk doesn't change! Then I addeda few lines *above*
this hunk, (around line 30 I think). Here is the diff again:

@@ -56,12 +58,14 @@
      bool running_ = true;

      /*!
-     * \brief The default font renderer, global to all who have a 
pointer to
-     * the Game class.
+     * \brief The font renderer implementation, obtained from the 
config file.
+     *
+     * It should be used and passed along to member objects by GameStates!
       *
-     * It need not be used at all!
+     * \note It can be cached, but not between GameStates, meaning it 
should be
+     * cached again every time a new GameState is constructed!
       */
-    std::unique_ptr<FontRenderer> font_renderer_ = nullptr;
+    FontRenderer* font_renderer_ = nullptr;

      int run(int argc, char* argv[]);

Notice the new line number spec:@@ -56,12 +58,14 @@

It moves two lines down, because I added those two lines before it, 
makes sense!
But also notice that the patches are different, just because of the two 
lines
above it!

I thought I might be able to fix this problem by changing the new 
diff.algorithm
config option to 'patience', but it seems to only affect how patches 
look, not
how they are stored internally... Same problem!

Also, I'm wondering why that line was picked up by git if the patches 
don't match,
shouldn't git give me a conflict with the whole hunk, or is the system 
smarter
than that?

What if merging suppressed the conflict if both possibilities are the 
same? Isn't
that reasonable, or is there some 1% where this could cause (silent but 
deadly)
data loss.

Sorry, I'm just rambling...

Anyway, thanks for your help Phil!
- Luke

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

* Re: [Bug] git stash generates a different diff then other commands (diff, add, etc) resulting in merge conflicts!
       [not found]     ` <CABURp0rWMAs9vT791vp4BEYS-Y9Jmzjmt4bbuB+po8=vkiqUWQ@mail.gmail.com>
@ 2013-08-13  5:31       ` Luke San Antonio
  2013-08-16  0:05         ` Phil Hord
  0 siblings, 1 reply; 5+ messages in thread
From: Luke San Antonio @ 2013-08-13  5:31 UTC (permalink / raw)
  To: Phil Hord; +Cc: git@vger.kernel.org

On 08/12/2013 12:05 PM, Phil Hord wrote:
> On Mon, Aug 12, 2013 at 1:29 AM, Luke San Antonio
> <lukesanantonio@gmail.com> wrote:
>> On 08/08/20130 04:54 PM, Phil Hord wrote:
>>> Luke,
>>>
>>> I think the issue is that your working directory receives your cached
>>> file when you say 'git stash --keep-index'.  When you restore the
>>> stash, your previous working directory now conflicts with your new
>>> working directory, but neither is the same as HEAD.
>>>
>>> Here's a test script to demonstrate the issue, I think.  Did I get
>>> this right, Luke?
>>>
>>>    # cd /tmp && rm -rf foo
>>>    git init foo && cd foo
>>>    echo "foo" > bar &&  git add bar && git commit -mfoo
>>>    echo "bar" > bar &&  git add bar
>>>    echo "baz" > bar
>>>    echo "Before stash  bar: $(cat bar)"
>>>    git stash --keep-index
>>>    echo "After stash  bar: $(cat bar)"
>>>    git stash apply
>> Actually no, in your script, the bar file has a modification in the working
>> tree which is in the same hunk as a change applied to the index. In my
>> project the changes that were added to the index are not modified further
>> in theworking tree.
>>
>> --------
>>
>> Not only that, but I found out why git was generated different patches!
>> I realized that when I removed a hunk appearing before the merge conflict
>> from the working tree and index, the merge conflict disappeared! Turns
>> out, we can forget about stashing for a minute!
>> First the hunk in my working tree:
>>
>> @@ -56,12 +56,14 @@
>>       bool running_ = true;
>>
>>
>>       /*!
>> -     * \brief The default font renderer, global to all who have a pointer
>> to
>> -     * the Game class.
>> +     * \brief The font renderer implementation, obtained from the config
>> file.
>>        *
>> -     * It need not be used at all!
>> +     * It should be used and passed along to member objects by GameStates!
>> +     *
>> +     * \note It can be cached, but not between GameStates, meaning it
>> should be
>> +     * cached again every time a new GameState is constructed!
>>        */
>> -    std::unique_ptr<FontRenderer> font_renderer_ = nullptr;
>> +    FontRenderer* font_renderer_ = nullptr;
>>
>>       int run(int argc, char* argv[]);
>>
>> Most of this is unimportant, but notice the line number spec:@@ -56,12
>> +56,14 @@
>> The line number of this hunk doesn't change! Then I addeda few lines *above*
>> this hunk, (around line 30 I think). Here is the diff again:
>>
>> @@ -56,12 +58,14 @@
>>       bool running_ = true;
>>
>>
>>       /*!
>> -     * \brief The default font renderer, global to all who have a pointer
>> to
>> -     * the Game class.
>> +     * \brief The font renderer implementation, obtained from the config
>> file.
>> +     *
>> +     * It should be used and passed along to member objects by GameStates!
>>        *
>> -     * It need not be used at all!
>> +     * \note It can be cached, but not between GameStates, meaning it
>> should be
>> +     * cached again every time a new GameState is constructed!
>>        */
>> -    std::unique_ptr<FontRenderer> font_renderer_ = nullptr;
>> +    FontRenderer* font_renderer_ = nullptr;
>>
>>       int run(int argc, char* argv[]);
>>
>> Notice the new line number spec:@@ -56,12 +58,14 @@
>>
>> It moves two lines down, because I added those two lines before it, makes
>> sense!
>> But also notice that the patches are different, just because of the two
>> lines
>> above it!
>>
>> I thought I might be able to fix this problem by changing the new
>> diff.algorithm
>> config option to 'patience', but it seems to only affect how patches look,
>> not
>> how they are stored internally... Same problem!
>>
>> Also, I'm wondering why that line was picked up by git if the patches don't
>> match,
>> shouldn't git give me a conflict with the whole hunk, or is the system
>> smarter
>> than that?
> Git does not store patches.  Git stores the entire file.  I do not
> think the diff algorithm you choose will have any effect on the
> results of the merge.  But I am pretty clueless about the merge
> engine, so I could be off-base on this last part.
>
>> What if merging suppressed the conflict if both possibilities are the same?
>> Isn't
>> that reasonable, or is there some 1% where this could cause (silent but
>> deadly)
>> data loss.
> I think that is what Git is meant to do.  But I am confused now about
> where the failure is occurring for you.  Can you demonstrate the
> problem by modifying my test script?
>
> Is this more like it?
>
>    cd /tmp && rm -rf foo
>    git init foo && cd foo
>    printf "1\n2 foo\n3\n4\n5\n6\n7\n8 foo\n9\n10\n" > bar &&  git add bar
>    git commit -mfoo
>    printf "1\n2 XXX\n3\n4\n5\n6\n7\n8 foo\n9\n10\n" > bar &&  git add bar
>    printf "1\n2 XXX\n3\n4\n5\n6\n7\n8 XXX\n9\n10\n" > bar
>    echo "Before stash  bar: $(cat bar)"
>    git stash --keep-index
>    echo "After stash  bar: $(cat bar)"
>    git stash apply
>
> Phil
So I found an isolated case, it's very strange...

Here's a script!

   cd /tmp && rm -rf foo;
   git init foo && cd foo;
   git config --local diff.algorithm myers
   printf "\n\n-----------------\n\n\n    /*"'!'"\
\n     * ---------\n     * ^^^^^^^^^\n     *\n     \
* =========\n     */\n    |-----------|\n" > foo;
   git add foo && git commit -m "foo";
   printf "\n-----------------\n\n\n    /*"'!'"\
\n     * #########\n     *\n     * !!!!!!!!!\n     \
*\n     * @@@@@@@@@\n     * &&&&&&&&&\n     */\n    \
|===========|\n" > foo
   printf "s\nn\ny\ny\ny\n" | git add foo --patch > /dev/null
   git stash save --keep-index

Let me start off by apologizing for that! =D

... Copy and paste that into a terminal and you should have a recreated 
version of my repository there! Now that the file is partly stashed and 
partly in the index, check out the difference in diffs:

try:
   git diff --staged
then try:
   git stash show -p

You see the difference? Then pop the stash and you'll see a very 
obfuscated and verbose sample of what I am talking about!

Also, sorry about the typos in my last message, I guess I looked past 
them...

Thanks Phil, for you know, helping me out!
- Luke

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

* Re: [Bug] git stash generates a different diff then other commands (diff, add, etc) resulting in merge conflicts!
  2013-08-13  5:31       ` Luke San Antonio
@ 2013-08-16  0:05         ` Phil Hord
  0 siblings, 0 replies; 5+ messages in thread
From: Phil Hord @ 2013-08-16  0:05 UTC (permalink / raw)
  To: Luke San Antonio
  Cc: git@vger.kernel.org, Junio C Hamano, Johannes Schindelin,
	Jonathan Nieder

On Tue, Aug 13, 2013 at 1:31 AM, Luke San Antonio
<lukesanantonio@gmail.com> wrote:
>
> So I found an isolated case, it's very strange...
>
> Here's a script!
>
>
<deleted>

Thanks for that.  It was hard to read, but it demonstrates the problem well.


> ... Copy and paste that into a terminal and you should have a recreated
> version of my repository there! Now that the file is partly stashed and
> partly in the index, check out the difference in diffs:
>
> try:
>   git diff --staged
> then try:
>   git stash show -p

This one is pretty sneaky.  It is not limited to git-stash.  I can
demonstrate the problem now using 'git merge-file'.

But I can only make this problem show itself when:
   1. The collisions are separated by just one line of common code, and
   2. One of the lines of common code is duplicated in one of the
collisions, and
   3. The first two lines of the file are duplicated, and
   4. One of the first two lines is deleted on one side but not the other.

I have managed to boil the test down to this script:

    #-----------------------------
    cat >base <<base
        1 duplicate
        1 duplicate
        3 unchanged
        4 will change
        5 gets deleted
        7 duplicated
        8 will change
base

    cat >left <<left
        1 duplicate
        1 duplicate
        3 unchanged
        4 changed
        7 duplicated
        6 new line
        7 duplicated
        8 changed
left

    sed -e 1d left > right

    git merge-file -p left base right

    #-----------------------------


The result looks like this, showing the duplicate "collision":

  1 duplicate
  3 unchanged
  4 changed
  <<<<<<< left
  7 duplicated
  6 new line
  7 duplicated
  =======
  7 duplicated
  6 new line
  7 duplicated
  >>>>>>> right
  8 changed


But it should look like this instead:

  1 duplicate
  3 unchanged
  4 changed
  7 duplicated
  6 new line
  7 duplicated
  8 changed


A similar (but different) stupidity shows up if you remove line "3"
from all three files.

I tested this in 1.6.5 and the same thing occurs there, so this is NOT
recent regression.

Phil

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

end of thread, other threads:[~2013-08-16  0:05 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-08  7:07 [Bug] git stash generates a different diff then other commands (diff, add, etc) resulting in merge conflicts! Luke San Antonio
2013-08-08 20:54 ` Phil Hord
2013-08-12  5:29   ` Luke San Antonio
     [not found]     ` <CABURp0rWMAs9vT791vp4BEYS-Y9Jmzjmt4bbuB+po8=vkiqUWQ@mail.gmail.com>
2013-08-13  5:31       ` Luke San Antonio
2013-08-16  0:05         ` Phil Hord

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).