Re: [Bug] git stash generates a different diff then other commands (diff, add, etc) resulting in merge conflicts!

On 08/12/2013 12:05 PM, Phil Hord wrote:

On Mon, Aug 12, 2013 at 1:29 AM, Luke San Antonio
[off-list ref] wrote:

quoted

On 08/08/20130 04:54 PM, Phil Hord wrote:

quoted

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.

quoted

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

On Tue, Aug 13, 2013 at 1:31 AM, Luke San Antonio
[off-list ref] 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