commit.c: don't persist substituted parents when unshallowing

Since 37b9dcabfc (shallow.c: use '{commit,rollback}_shallow_file',
2020-04-22), Git knows how to reset stat-validity checks for the
$GIT_DIR/shallow file, allowing it to change between a shallow and
non-shallow state in the same process (e.g., in the case of 'git fetch
--unshallow').

However, when $GIT_DIR/shallow changes, Git does not alter or remove any
grafts (nor substituted parents) in memory.

This comes up in a "git fetch --unshallow" with fetch.writeCommitGraph
set to true. Ordinarily in a shallow repository (and before 37b9dcabfc,
even in this case), commit_graph_compatible() would return false,
indicating that the repository should not be used to write a
commit-graphs (since commit-graph files cannot represent a shallow
history). But since 37b9dcabfc, in an --unshallow operation that check
succeeds.

Thus even though the repository isn't shallow any longer (that is, we
have all of the objects), the in-core representation of those objects
still has munged parents at the shallow boundaries.  When the
commit-graph write proceeds, we use the incorrect parentage, producing
wrong results.

There are two ways for a user to work around this: either (1) set
'fetch.writeCommitGraph' to 'false', or (2) drop the commit-graph after
unshallowing.

One way to fix this would be to reset the parsed object pool entirely
(flushing the cache and thus preventing subsequent reads from modifying
their parents) after unshallowing. That would produce a problem when
callers have a now-stale reference to the old pool, and so this patch
implements a different approach. Instead, attach a new bit to the pool,
'substituted_parent', which indicates if the repository *ever* stored a
commit which had its parents modified (i.e., the shallow boundary
prior to unshallowing).

This bit needs to be sticky because all reads subsequent to modifying a
commit's parents are unreliable when unshallowing. Modify the check in
'commit_graph_compatible' to take this bit into account, and correctly
avoid generating commit-graphs in this case, thus solving the bug.

Helped-by: Derrick Stolee <dstolee@microsoft.com>
Helped-by: Jonathan Nieder <jrnieder@gmail.com>
Reported-by: Jay Conrod <jayconrod@google.com>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/commit-graph.c b/commit-graph.c
index f013a84e294b13b552b62257bbf2fa7e1c353a82..2be0b71cd0c497996c7e6de253f53058a377d0ca 100644 (file)
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -88,7 +88,8 @@ static int commit_graph_compatible(struct repository *r)
        }
 
        prepare_commit_graft(r);
-       if (r->parsed_objects && r->parsed_objects->grafts_nr)
+       if (r->parsed_objects &&
+           (r->parsed_objects->grafts_nr || r->parsed_objects->substituted_parent))
                return 0;
        if (is_repository_shallow(r))
                return 0;

diff --git a/commit.c b/commit.c
index a6cfa41a4e315225c08b8e51c5dd982d9f6273d2..f57d8fbaa3fbad2396a147d32bd22be8179633ca 100644 (file)
--- a/commit.c
+++ b/commit.c
@@ -435,6 +435,8 @@ int parse_commit_buffer(struct repository *r, struct commit *item, const void *b
        pptr = &item->parents;
 
        graft = lookup_commit_graft(r, &item->object.oid);
+       if (graft)
+               r->parsed_objects->substituted_parent = 1;
        while (bufptr + parent_entry_len < tail && !memcmp(bufptr, "parent ", 7)) {
                struct commit *new_parent;
 

diff --git a/object.h b/object.h
index 2dbabfca0ab8185f52cca6d3b7c69658e838ae49..4327ec010f860f6e3e469c9ac17ef8cc8abfffce 100644 (file)
--- a/object.h
+++ b/object.h
@@ -26,6 +26,7 @@ struct parsed_object_pool {
        char *alternate_shallow_file;
 
        int commit_graft_prepared;
+       int substituted_parent;
 
        struct buffer_slab *buffer_slab;
 };

diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh
index 126654817d3186704024e09b5459f52809ec46f0..b4e89fe5eb2652638d9afff8a338363ac219e02e 100755 (executable)
--- a/t/t5537-fetch-shallow.sh
+++ b/t/t5537-fetch-shallow.sh
@@ -81,6 +81,20 @@ test_expect_success 'fetch --unshallow from shallow clone' '
        )
 '
 
+test_expect_success 'fetch --unshallow from a full clone' '
+       git clone --no-local --depth=2 .git shallow3 &&
+       (
+       cd shallow3 &&
+       git log --format=%s >actual &&
+       test_write_lines 4 3 >expect &&
+       test_cmp expect actual &&
+       git -c fetch.writeCommitGraph fetch --unshallow &&
+       git log origin/master --format=%s >actual &&
+       test_write_lines 4 3 2 1 >expect &&
+       test_cmp expect actual
+       )
+'
+
 test_expect_success 'fetch something upstream has but hidden by clients shallow boundaries' '
        # the blob "1" is available in .git but hidden by the
        # shallow2/.git/shallow and it should be resent