Repository data loss in fast-export with a merge of a deleted submodule

Hello.

We had a submodule that we deleted and then added back into the repository at the same location as the former submodule. When running fast-export, the newly 'added' files for the merge commit are listed and then are followed with a:

M ... path/to/submodule/file D path/to/submodule

On fast-import, the resultant repository becomes corrupt due to the Delete instruction above occurring AFTER the file adds/modifications. The new repository does not match the old repository where the fast-export was performed.

I have included a repro script below. I have not been able to test this on Git 1.7.7.1, but I have tested on Git 1.7.7 (msysGit version).

Please compare the differences between the generated main.fe and newmain.fe files. newmain.fe has data loss.

I am not familiar with the fast-export code.  Can anyone help out?

Thanks.

Josh

---------

rm -rf main brokenmain sub main.fenewmain.fe

# Create the submodule. mkdir sub cd sub git init echo file > file git add file git commit -m file cd ..

# Create the main repository. mkdir main cd main git init

# Add the submodule. git submodule add ../sub sub git commit -m "Add submodule"

# Remove the submodule. rm -rf sub git rm sub .gitmodules git commit -m "Remove submodule"

# Add sub/file to the master branch.
mkdir sub
echo file > sub/file
git add sub/file
git commit -m "Add sub/file"
if [ -f sub/file ]; then
     echo "main: master branch: sub/file exists."
fi

# Delete the submodule directory manually, because we know that the incoming merge will need it gone. git checkout -B will-be-broken HEAD^^ rm -rf sub git merge --no-ff master

# sub/file exists within the 'will-be-broken' branch.
if [ -f sub/file ]; then
     echo "main: will-be-broken branch: sub/file exists."
fi

# Export out the main repository. git fast-export --all > ../main.fe

# Create the brokenmain repository. cd .. mkdir brokenmain cd brokenmain git init

# Import in everything from the main repository. git fast-import < ../main.fe

# sub/file exists within the master branch.
git checkout master
if [ -f sub/file ]; then
     echo "brokenmain: master branch: sub/file exists."
fi

# sub/file SHOULD exist within the 'will-be-broken' branch but doesn't.
git checkout will-be-broken
if [ ! -f sub/file ]; then
     echo "brokenmain: will-be-broken branch: sub/file SHOULD exist but 
doesn't."
fi

# Export out the brokenmain repository. git fast-export --all > ../brokenmain.fe

----- Original Message -----
From: Joshua Jensen
Date: 10/27/2011 1:27 PM

Okay, I looked into this further, and I came up with a patch that works for me. Nevertheless, I do not understand exactly what is going on here, so I would like to defer to someone else's patch to fix the issue.

-Josh

--- builtin/fast-export.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-)

----- Original Message -----
From: Joshua Jensen
Date: 11/3/2011 10:05 AM

Hi.

__This is a genuine data loss problem in Git.__

I'm confused at the lack of response to this. I first posted about the issue **2-1/2 weeks ago**, and there have been no responses Does no one care?

In case no one received the messages, you can find them at [1] and [2].

-Josh

[1] http://www.spinics.net/lists/git/msg168295.html [2] http://www.spinics.net/lists/git/msg168691.html

Am 14.11.2011 16:06, schrieb Joshua Jensen:

Maybe no one cares, people didn't read the message (or forgot about it) or they are too busy ... thanks for prodding us again.

While I'm interested in this issue because submodules are affected, I'm very short on Git time these days and can't investigate this issue further (and I have no clue about export/import either). I added the last two people who touched depth_first() in builtin/fast-export.c to the CC, maybe they can tell us more about your patch to solve this issue (found in [2]).

On Mon, Nov 14, 2011 at 08:06:51AM -0700, Joshua Jensen wrote:

Still not much response.

I think the keywords "submodule" and "fast-export" in the subject line hit a lot of people's do-not-care filters.

I read your original two messages. It does seem like a simple ordering problem from your description. I suspect you would get more response to actually post your patch with a commit message explaining the problem, and an accompanying test. And then at the very least, one outcome could be Junio picking up the patch. :)

I think you have all of those components spread across your messages, and just need to polish them and put them in one place.

Regarding your patch itself, your explanation make sense to me and the goal of your patch looks reasonable. Bearing in mind that I know virtually nothing about the innards fast-import/fast-export.

But for the patch text itself:

If you have multiple deleted entries, doesn't this leave them in a random order at the beginning of the list? Does that matter? If they are both 'D', should they be compared as usual? I.e.:

  if (a->status != b->status) {
          if (a->status == 'D')
                  return -1;
          if (b->status == 'D')
                  return 1;
  }

  /* and now we do the rest of the function as usual... */

-Peff