diff options
author | OpenShift Merge Robot <openshift-merge-robot@users.noreply.github.com> | 2021-01-12 17:34:03 -0500 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-01-12 17:34:03 -0500 |
commit | c216a438b8f9b4b8405bcac35211aa21d5e341a8 (patch) | |
tree | eff9f827ff241d2c996492af032423b6078edfcc | |
parent | bdca64340b9f4046846405148385d934f4912d4b (diff) | |
parent | d7f2955f3717b452b71cb16c5360ff21f79515e7 (diff) | |
download | ostree-c216a438b8f9b4b8405bcac35211aa21d5e341a8.tar.gz |
Merge pull request #2267 from dbnicholson/pull-depth-fixes
Pull depth fixes
-rw-r--r-- | src/libostree/ostree-repo-pull-private.h | 2 | ||||
-rw-r--r-- | src/libostree/ostree-repo-pull.c | 102 | ||||
-rwxr-xr-x | tests/test-local-pull-depth.sh | 34 | ||||
-rwxr-xr-x | tests/test-pull-depth.sh | 44 |
4 files changed, 142 insertions, 40 deletions
diff --git a/src/libostree/ostree-repo-pull-private.h b/src/libostree/ostree-repo-pull-private.h index d4c3e971..59b72e88 100644 --- a/src/libostree/ostree-repo-pull-private.h +++ b/src/libostree/ostree-repo-pull-private.h @@ -88,7 +88,7 @@ typedef struct { GHashTable *ref_keyring_map; /* Maps OstreeCollectionRef to keyring remote name */ GPtrArray *static_delta_superblocks; GHashTable *expected_commit_sizes; /* Maps commit checksum to known size */ - GHashTable *commit_to_depth; /* Maps commit checksum maximum depth */ + GHashTable *commit_to_depth; /* Maps parent commit checksum maximum depth */ GHashTable *scanned_metadata; /* Maps object name to itself */ GHashTable *fetched_detached_metadata; /* Map<checksum,GVariant> */ GHashTable *requested_metadata; /* Maps object name to itself */ diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 7d4b91e2..a95190a5 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -1113,6 +1113,18 @@ on_metadata_written (GObject *object, check_outstanding_requests_handle_error (pull_data, &local_error); } +static gboolean +is_parent_commit (OtPullData *pull_data, + const char *checksum) +{ + /* FIXME: Only parent commits are added to the commit_to_depth table, + * so if the checksum isn't in the table then a new commit chain is + * being started. However, if the desired commit was a parent in a + * previously followed chain, then this will be wrong. + */ + return g_hash_table_contains (pull_data->commit_to_depth, checksum); +} + static void meta_fetch_on_complete (GObject *object, GAsyncResult *result, @@ -1155,10 +1167,13 @@ meta_fetch_on_complete (GObject *object, } /* When traversing parents, do not fail on a missing commit. - * We may be pulling from a partial repository that ends in - * a dangling parent reference. */ + * We may be pulling from a partial repository that ends in a + * dangling parent reference. This logic should match the + * local case in scan_one_metadata_object. + */ else if (objtype == OSTREE_OBJECT_TYPE_COMMIT && - pull_data->maxdepth != 0) + pull_data->maxdepth != 0 && + is_parent_commit (pull_data, checksum)) { g_clear_error (&local_error); /* If the remote repo supports tombstone commits, check if the commit was intentionally @@ -1542,8 +1557,6 @@ scan_commit_object (OtPullData *pull_data, else { depth = pull_data->maxdepth; - g_hash_table_insert (pull_data->commit_to_depth, g_strdup (checksum), - GINT_TO_POINTER (depth)); } #ifndef OSTREE_DISABLE_GPGME @@ -1684,40 +1697,19 @@ scan_commit_object (OtPullData *pull_data, return FALSE; } - if (parent_csum_bytes != NULL && pull_data->maxdepth == -1) - { - queue_scan_one_metadata_object_c (pull_data, parent_csum_bytes, - OSTREE_OBJECT_TYPE_COMMIT, NULL, - recursion_depth + 1, NULL); - } - else if (parent_csum_bytes != NULL && depth > 0) + if (parent_csum_bytes != NULL && (pull_data->maxdepth == -1 || depth > 0)) { char parent_checksum[OSTREE_SHA256_STRING_LEN+1]; - gpointer parent_depthp; - int parent_depth; - ostree_checksum_inplace_from_bytes (parent_csum_bytes, parent_checksum); - if (g_hash_table_lookup_extended (pull_data->commit_to_depth, parent_checksum, - NULL, &parent_depthp)) - { - parent_depth = GPOINTER_TO_INT (parent_depthp); - } - else - { - parent_depth = depth - 1; - } - - if (parent_depth >= 0) - { - g_hash_table_insert (pull_data->commit_to_depth, g_strdup (parent_checksum), - GINT_TO_POINTER (parent_depth)); - queue_scan_one_metadata_object_c (pull_data, parent_csum_bytes, - OSTREE_OBJECT_TYPE_COMMIT, - NULL, - recursion_depth + 1, - NULL); - } + int parent_depth = (depth > 0) ? depth - 1 : -1; + g_hash_table_insert (pull_data->commit_to_depth, g_strdup (parent_checksum), + GINT_TO_POINTER (parent_depth)); + queue_scan_one_metadata_object_c (pull_data, parent_csum_bytes, + OSTREE_OBJECT_TYPE_COMMIT, + NULL, + recursion_depth + 1, + NULL); } /* We only recurse to looking whether we need dirtree/dirmeta @@ -1830,10 +1822,46 @@ scan_one_metadata_object (OtPullData *pull_data, return FALSE; } + g_autoptr(GError) local_error = NULL; if (!_ostree_repo_import_object (pull_data->repo, pull_data->remote_repo_local, objtype, checksum, pull_data->importflags, - cancellable, error)) - return FALSE; + cancellable, &local_error)) + { + /* When traversing parents, do not fail on a missing commit. + * We may be pulling from a partial repository that ends in a + * dangling parent reference. This logic should match the + * remote case in meta_fetch_on_complete. + * + * Note early return. + */ + if (g_error_matches (local_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND) && + objtype == OSTREE_OBJECT_TYPE_COMMIT && + pull_data->maxdepth != 0 && + is_parent_commit (pull_data, checksum)) + { + g_clear_error (&local_error); + + /* If the remote repo supports tombstone commits, check if + * the commit was intentionally deleted. + */ + if (pull_data->has_tombstone_commits) + { + if (!_ostree_repo_import_object (pull_data->repo, pull_data->remote_repo_local, + OSTREE_OBJECT_TYPE_TOMBSTONE_COMMIT, + checksum, pull_data->importflags, + cancellable, error)) + return FALSE; + } + + return TRUE; + } + else + { + g_propagate_error (error, g_steal_pointer (&local_error)); + return FALSE; + } + } + /* The import API will fetch both the commit and detached metadata, so * add it to the hash to avoid re-fetching it below. */ diff --git a/tests/test-local-pull-depth.sh b/tests/test-local-pull-depth.sh index 96b20b9c..80413bc9 100755 --- a/tests/test-local-pull-depth.sh +++ b/tests/test-local-pull-depth.sh @@ -25,7 +25,7 @@ set -euo pipefail setup_test_repository "archive" -echo "1..1" +echo "1..3" cd ${test_tmpdir} mkdir repo2 @@ -62,3 +62,35 @@ find repo2/state -name '*.commitpartial' | wc -l > commitpartialcount assert_file_has_content commitpartialcount "^0$" echo "ok local pull depth" + +# Check that pulling with depth != 0 succeeds with a missing parent +# commit. Prune the remote to truncate the history. +cd ${test_tmpdir} +${CMD_PREFIX} ostree --repo=repo prune --refs-only --depth=0 + +rm -rf repo2/refs/heads/* repo2/refs/remotes/* repo2/objects/*/*.commit +${CMD_PREFIX} ostree --repo=repo2 pull-local --depth=1 repo +find repo/objects -name '*.commit' | wc -l > commitcount +assert_file_has_content commitcount "^1$" +find repo/state -name '*.commitpartial' | wc -l > commitpartialcount +assert_file_has_content commitpartialcount "^0$" + +rm -rf repo2/refs/heads/* repo2/refs/remotes/* repo2/objects/*/*.commit +${CMD_PREFIX} ostree --repo=repo2 pull-local --depth=-1 repo +find repo/objects -name '*.commit' | wc -l > commitcount +assert_file_has_content commitcount "^1$" +find repo/state -name '*.commitpartial' | wc -l > commitpartialcount +assert_file_has_content commitpartialcount "^0$" + +echo "ok local pull depth missing parent" + +# Check that it errors if the ref head commit is missing. +cd ${test_tmpdir} +rm -f repo/objects/*/*.commit + +rm -rf repo2/refs/heads/* repo2/refs/remotes/* repo2/objects/*/*.commit +if ${CMD_PREFIX} ostree --repo=repo2 pull-local --depth=-1 repo; then + fatal "Local pull with depth -1 succeeded with missing HEAD commit" +fi + +echo "ok local pull depth missing HEAD commit" diff --git a/tests/test-pull-depth.sh b/tests/test-pull-depth.sh index 1206c6c4..8fb2f597 100755 --- a/tests/test-pull-depth.sh +++ b/tests/test-pull-depth.sh @@ -25,7 +25,7 @@ set -euo pipefail setup_fake_remote_repo1 "archive" -echo '1..1' +echo '1..3' cd ${test_tmpdir} mkdir repo @@ -35,21 +35,63 @@ ${CMD_PREFIX} ostree --repo=repo remote add --set=gpg-verify=false origin $(cat ${CMD_PREFIX} ostree --repo=repo pull --depth=0 origin main find repo/objects -name '*.commit' | wc -l > commitcount assert_file_has_content commitcount "^1$" +find repo/state -name '*.commitpartial' | wc -l > commitpartialcount +assert_file_has_content commitpartialcount "^0$" ${CMD_PREFIX} ostree --repo=repo pull --depth=0 origin main find repo/objects -name '*.commit' | wc -l > commitcount assert_file_has_content commitcount "^1$" +find repo/state -name '*.commitpartial' | wc -l > commitpartialcount +assert_file_has_content commitpartialcount "^0$" ${CMD_PREFIX} ostree --repo=repo pull --depth=1 origin main find repo/objects -name '*.commit' | wc -l > commitcount assert_file_has_content commitcount "^2$" +find repo/state -name '*.commitpartial' | wc -l > commitpartialcount +assert_file_has_content commitpartialcount "^0$" ${CMD_PREFIX} ostree --repo=repo pull --depth=1 origin main find repo/objects -name '*.commit' | wc -l > commitcount assert_file_has_content commitcount "^2$" +find repo/state -name '*.commitpartial' | wc -l > commitpartialcount +assert_file_has_content commitpartialcount "^0$" ${CMD_PREFIX} ostree --repo=repo pull --depth=-1 origin main find repo/objects -name '*.commit' | wc -l > commitcount assert_file_has_content commitcount "^3$" +find repo/state -name '*.commitpartial' | wc -l > commitpartialcount +assert_file_has_content commitpartialcount "^0$" echo "ok pull depth" + +# Check that pulling with depth != 0 succeeds with a missing parent +# commit. Prune the remote to truncate the history. +cd ${test_tmpdir} +${CMD_PREFIX} ostree --repo=ostree-srv/gnomerepo prune --refs-only --depth=0 + +rm -rf repo/refs/heads/* repo/refs/remotes/* repo/objects/*/*.commit +${CMD_PREFIX} ostree --repo=repo pull --depth=1 origin main +find repo/objects -name '*.commit' | wc -l > commitcount +assert_file_has_content commitcount "^1$" +find repo/state -name '*.commitpartial' | wc -l > commitpartialcount +assert_file_has_content commitpartialcount "^0$" + +rm -rf repo/refs/heads/* repo/refs/remotes/* repo/objects/*/*.commit +${CMD_PREFIX} ostree --repo=repo pull --depth=-1 origin main +find repo/objects -name '*.commit' | wc -l > commitcount +assert_file_has_content commitcount "^1$" +find repo/state -name '*.commitpartial' | wc -l > commitpartialcount +assert_file_has_content commitpartialcount "^0$" + +echo "ok pull depth missing parent" + +# Check that it errors if the ref head commit is missing. +cd ${test_tmpdir} +rm -f ostree-srv/gnomerepo/objects/*/*.commit + +rm -rf repo/refs/heads/* repo/refs/remotes/* repo/objects/*/*.commit +if ${CMD_PREFIX} ostree --repo=repo pull --depth=-1 origin main; then + fatal "Pull with depth -1 succeeded with missing HEAD commit" +fi + +echo "ok pull depth missing HEAD commit" |