From 206f1d3a13189e55027ffadd0b8b434768c3ec0f Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Mon, 10 Aug 2020 12:05:06 +0100 Subject: [PATCH 1/3] lib/repo: Add mode and tombstone config options to the summary file Currently, they are set in the `config` file and cause that to be downloaded on every pull. Given that the client is already pulling the `summary` file, it makes sense to avoid an additional network round trip and cache those options in the `summary` file. Signed-off-by: Philip Withnall Helps: #2165 --- src/libostree/ostree-repo-private.h | 2 ++ src/libostree/ostree-repo.c | 18 ++++++++++++++++++ src/ostree/ot-dump.c | 16 ++++++++++++++++ tests/test-summary-view.sh | 2 +- 4 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h index 8c1f5071..a48feca4 100644 --- a/src/libostree/ostree-repo-private.h +++ b/src/libostree/ostree-repo-private.h @@ -55,6 +55,8 @@ G_BEGIN_DECLS #define OSTREE_SUMMARY_EXPIRES "ostree.summary.expires" #define OSTREE_SUMMARY_COLLECTION_ID "ostree.summary.collection-id" #define OSTREE_SUMMARY_COLLECTION_MAP "ostree.summary.collection-map" +#define OSTREE_SUMMARY_MODE "ostree.summary.mode" +#define OSTREE_SUMMARY_TOMBSTONE_COMMITS "ostree.summary.tombstone-commits" #define _OSTREE_PAYLOAD_LINK_PREFIX "../" #define _OSTREE_PAYLOAD_LINK_PREFIX_LEN (sizeof (_OSTREE_PAYLOAD_LINK_PREFIX) - 1) diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 4283f68e..ba3e877f 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -5816,6 +5816,24 @@ ostree_repo_regenerate_summary (OstreeRepo *self, g_variant_new_uint64 (GUINT64_TO_BE (g_get_real_time () / G_USEC_PER_SEC))); } + { + g_autofree char *remote_mode_str = NULL; + if (!ot_keyfile_get_value_with_default (self->config, "core", "mode", "bare", + &remote_mode_str, error)) + return FALSE; + g_variant_dict_insert_value (&additional_metadata_builder, OSTREE_SUMMARY_MODE, + g_variant_new_string (remote_mode_str)); + } + + { + gboolean tombstone_commits = FALSE; + if (!ot_keyfile_get_boolean_with_default (self->config, "core", "tombstone-commits", FALSE, + &tombstone_commits, error)) + return FALSE; + g_variant_dict_insert_value (&additional_metadata_builder, OSTREE_SUMMARY_TOMBSTONE_COMMITS, + g_variant_new_boolean (tombstone_commits)); + } + /* Add refs which have a collection specified, which could be in refs/mirrors, * refs/heads, and/or refs/remotes. */ { diff --git a/src/ostree/ot-dump.c b/src/ostree/ot-dump.c index 225d1845..1f911d4e 100644 --- a/src/ostree/ot-dump.c +++ b/src/ostree/ot-dump.c @@ -361,6 +361,22 @@ ot_dump_summary_bytes (GBytes *summary_bytes, pretty_key = "Collection Map"; value_str = g_strdup ("(printed above)"); } + else if (g_strcmp0 (key, OSTREE_SUMMARY_MODE) == 0) + { + OstreeRepoMode repo_mode; + const char *repo_mode_str = g_variant_get_string (value, NULL); + + pretty_key = "Repository Mode"; + if (!ostree_repo_mode_from_string (repo_mode_str, &repo_mode, NULL)) + value_str = g_strdup_printf ("Invalid (‘%s’)", repo_mode_str); + else + value_str = g_strdup (repo_mode_str); + } + else if (g_strcmp0 (key, OSTREE_SUMMARY_TOMBSTONE_COMMITS) == 0) + { + pretty_key = "Has Tombstone Commits"; + value_str = g_strdup (g_variant_get_boolean (value) ? "Yes" : "No"); + } else { value_str = g_variant_print (value, FALSE); diff --git a/tests/test-summary-view.sh b/tests/test-summary-view.sh index 14de0294..f6278a85 100755 --- a/tests/test-summary-view.sh +++ b/tests/test-summary-view.sh @@ -64,5 +64,5 @@ echo "ok view summary" ${OSTREE} summary --raw > raw-summary.txt assert_file_has_content_literal raw-summary.txt "('main', (" assert_file_has_content_literal raw-summary.txt "('other', (" -assert_file_has_content_literal raw-summary.txt "{'ostree.summary.last-modified': Date: Mon, 10 Aug 2020 12:06:35 +0100 Subject: [PATCH 2/3] lib/pull: Read mode and tombstone options from summary file if possible Otherwise, fall back to downloading and reading them from the `config` file. See the previous commit for details. Signed-off-by: Philip Withnall Fixes: #2165 --- src/libostree/ostree-repo-pull.c | 68 +++++++++++++++++++++----------- 1 file changed, 44 insertions(+), 24 deletions(-) diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index a6401907..f16ccec5 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -2001,6 +2001,8 @@ start_fetch (OtPullData *pull_data, is_meta ? meta_fetch_on_complete : content_fetch_on_complete, fetch); } +/* Deprecated: code should load options from the `summary` file rather than + * downloading the remote’s `config` file, to save on network round trips. */ static gboolean load_remote_repo_config (OtPullData *pull_data, GKeyFile **out_keyfile, @@ -3757,30 +3759,6 @@ ostree_repo_pull_with_options (OstreeRepo *self, if (!ostree_repo_open (pull_data->remote_repo_local, cancellable, error)) goto out; } - else - { - if (!load_remote_repo_config (pull_data, &remote_config, cancellable, error)) - goto out; - - if (!ot_keyfile_get_value_with_default (remote_config, "core", "mode", "bare", - &remote_mode_str, error)) - goto out; - - if (!ostree_repo_mode_from_string (remote_mode_str, &pull_data->remote_mode, error)) - goto out; - - if (!ot_keyfile_get_boolean_with_default (remote_config, "core", "tombstone-commits", FALSE, - &pull_data->has_tombstone_commits, error)) - goto out; - - if (pull_data->remote_mode != OSTREE_REPO_MODE_ARCHIVE) - { - g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, - "Can't pull from archives with mode \"%s\"", - remote_mode_str); - goto out; - } - } } /* Change some option defaults if we're actually pulling from a local @@ -3854,6 +3832,8 @@ ostree_repo_pull_with_options (OstreeRepo *self, g_autoptr(GVariant) deltas = NULL; g_autoptr(GVariant) additional_metadata = NULL; gboolean summary_from_cache = FALSE; + gboolean remote_mode_loaded = FALSE; + gboolean tombstone_commits = FALSE; if (summary_sig_bytes_v) { @@ -4167,6 +4147,46 @@ ostree_repo_pull_with_options (OstreeRepo *self, csum_data); } } + + if (pull_data->summary && + g_variant_lookup (additional_metadata, OSTREE_SUMMARY_MODE, "s", &remote_mode_str) && + g_variant_lookup (additional_metadata, OSTREE_SUMMARY_TOMBSTONE_COMMITS, "b", &tombstone_commits)) + { + if (!ostree_repo_mode_from_string (remote_mode_str, &pull_data->remote_mode, error)) + goto out; + pull_data->has_tombstone_commits = tombstone_commits; + remote_mode_loaded = TRUE; + } + else if (pull_data->remote_repo_local == NULL) + { + /* Fall-back path which loads the necessary config from the remote’s + * `config` file. Doing so is deprecated since it means an + * additional round trip to the remote for each pull. No need to do + * it for local pulls. */ + if (!load_remote_repo_config (pull_data, &remote_config, cancellable, error)) + goto out; + + if (!ot_keyfile_get_value_with_default (remote_config, "core", "mode", "bare", + &remote_mode_str, error)) + goto out; + + if (!ostree_repo_mode_from_string (remote_mode_str, &pull_data->remote_mode, error)) + goto out; + + if (!ot_keyfile_get_boolean_with_default (remote_config, "core", "tombstone-commits", FALSE, + &pull_data->has_tombstone_commits, error)) + goto out; + + remote_mode_loaded = TRUE; + } + + if (remote_mode_loaded && pull_data->remote_repo_local == NULL && pull_data->remote_mode != OSTREE_REPO_MODE_ARCHIVE) + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, + "Can't pull from archives with mode \"%s\"", + remote_mode_str); + goto out; + } } if (pull_data->is_mirror && !refs_to_fetch && !opt_collection_refs_set && !configured_branches) From 23bdc4e5df41a1f53ce387f90eaf6b79c644b2da Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Mon, 10 Aug 2020 12:07:22 +0100 Subject: [PATCH 3/3] ostree/dump: Fix a memory leak Re-using the `refs` variable for the main list of refs, plus the iterated lists, meant that the main list was never freed (although all the iterated ones were freed correctly). Fix this by using two variables rather than reusing the one. Signed-off-by: Philip Withnall --- src/ostree/ot-dump.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/ostree/ot-dump.c b/src/ostree/ot-dump.c index 1f911d4e..a8ed54a2 100644 --- a/src/ostree/ot-dump.c +++ b/src/ostree/ot-dump.c @@ -322,10 +322,11 @@ ot_dump_summary_bytes (GBytes *summary_bytes, collection_map = g_variant_lookup_value (exts, OSTREE_SUMMARY_COLLECTION_MAP, G_VARIANT_TYPE ("a{sa(s(taya{sv}))}")); if (collection_map != NULL) { + g_autoptr(GVariant) collection_refs = NULL; g_variant_iter_init (&iter, collection_map); - while (g_variant_iter_loop (&iter, "{&s@a(s(taya{sv}))}", &collection_id, &refs)) - dump_summary_refs (collection_id, refs); + while (g_variant_iter_loop (&iter, "{&s@a(s(taya{sv}))}", &collection_id, &collection_refs)) + dump_summary_refs (collection_id, collection_refs); } /* Print out the additional metadata. */