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

From: Jeff King <peff@peff.net>
Date: Nov 30, 2011, 07:15 UTC
Message-ID: <20111130071519.GF5317@sigill.intra.peff.net>
In-Reply-To: <4EC12E8B.3050909@workspacewhiz.com>

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

Show 5 quoted lines

> __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?

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:

Show 14 quoted lines

> @@ -161,6 +161,14 @@ static int depth_first(const void *a_, const void *b_)
>                name_a = a->one ? a->one->path : a->two->path;
>                name_b = b->one ? b->one->path : b->two->path;
> +             /*
> +             * Move 'D'elete entries first.
> +             */
> +             if (a->status == 'D')
> +                             return -1;
> +             else if (b->status == 'D')
> +                             return 1;
> +
>                len_a = strlen(name_a);
>                len_b = strlen(name_b);
>                len = (len_a < len_b) ? len_a : len_b;

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