From 72a54fa877e7b459ab4dc19dbb5cc288f219b41c Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Mon, 30 Jul 2018 10:54:15 -0400 Subject: [PATCH] lib/config: Deprecate commit-update-summary option Now that we have `auto-update-summary`, there is no point in having `commit-update-summary`. The latter also only had an effect through the `commit` CLI command, whereas the former is embedded directly in libostree. There is one corner case that slips through: `commit` would update the summary file even if orphan commits were created, which we no longer do here. I can't imagine anyone relying on this, so it seems safe to drop. Closes: #1689 Closes: #1693 Approved by: mwleeds --- man/ostree.repo-config.xml | 20 ++++++++++---------- src/libostree/ostree-repo.c | 20 +++++++++++++++----- src/ostree/ot-builtin-commit.c | 28 ---------------------------- 3 files changed, 25 insertions(+), 43 deletions(-) diff --git a/man/ostree.repo-config.xml b/man/ostree.repo-config.xml index 5424467c..bb406a2e 100644 --- a/man/ostree.repo-config.xml +++ b/man/ostree.repo-config.xml @@ -86,23 +86,23 @@ Boston, MA 02111-1307, USA. Currently, this must be set to 1. - - commit-update-summary - Boolean value controlling whether or not to - automatically update the summary file after a commit. Defaults - to false. - - auto-update-summary Boolean value controlling whether or not to automatically update the summary file after any ref is added, - removed, or updated. This covers a superset of the cases covered by - commit-update-summary, with the exception of orphan commits which - shouldn't affect the summary anyway. Defaults to false. + removed, or updated. Other modifications which may render a + summary file stale (like static deltas, or collection IDs) do + not currently trigger an auto-update. + + commit-update-summary + This option is deprecated. Use + auto-update-summary instead, for which this + option is now an alias. + + fsync Boolean value controlling whether or not to diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 00a6b460..922e8a66 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -5385,8 +5385,7 @@ summary_add_ref_entry (OstreeRepo *self, * regular, setting the `ostree.summary.expires` key in @additional_metadata * will aid clients in working out when to check for updates. * - * It is regenerated automatically after a commit if - * `core/commit-update-summary` is set, and automatically after any ref is + * It is regenerated automatically after any ref is * added, removed, or updated if `core/auto-update-summary` is set. * * If the `core/collection-id` key is set in the configuration, it will be @@ -5593,20 +5592,31 @@ ostree_repo_regenerate_summary (OstreeRepo *self, return TRUE; } -/* Regenerate the summary if `core/auto-update-summary` is set */ +/* Regenerate the summary if `core/auto-update-summary` is set. We default to FALSE for + * this setting because OSTree supports multiple processes committing to the same repo (but + * different refs) concurrently, and in fact gnome-continuous actually does this. In that + * context it's best to update the summary explicitly once at the end of multiple + * transactions instead of automatically here. `auto-update-summary` only updates + * atomically within a transaction. */ gboolean _ostree_repo_maybe_regenerate_summary (OstreeRepo *self, GCancellable *cancellable, GError **error) { gboolean auto_update_summary; - if (!ot_keyfile_get_boolean_with_default (self->config, "core", "auto-update-summary", FALSE, &auto_update_summary, error)) return FALSE; - if (auto_update_summary && + /* Deprecated alias for `auto-update-summary`. */ + gboolean commit_update_summary; + if (!ot_keyfile_get_boolean_with_default (self->config, "core", + "commit-update-summary", FALSE, + &commit_update_summary, error)) + return FALSE; + + if ((auto_update_summary || commit_update_summary) && !ostree_repo_regenerate_summary (self, NULL, cancellable, error)) return FALSE; diff --git a/src/ostree/ot-builtin-commit.c b/src/ostree/ot-builtin-commit.c index 6d295d6b..535239be 100644 --- a/src/ostree/ot-builtin-commit.c +++ b/src/ostree/ot-builtin-commit.c @@ -753,7 +753,6 @@ ostree_builtin_commit (int argc, char **argv, OstreeCommandInvocation *invocatio if (!skip_commit) { guint64 timestamp; - gboolean auto_update_summary; if (!opt_no_bindings) { @@ -823,33 +822,6 @@ ostree_builtin_commit (int argc, char **argv, OstreeCommandInvocation *invocatio if (!ostree_repo_commit_transaction (repo, &stats, cancellable, error)) goto out; - - if (!ot_keyfile_get_boolean_with_default (ostree_repo_get_config (repo), "core", - "auto-update-summary", FALSE, - &auto_update_summary, error)) - goto out; - - /* No need to update it again if we did for each ref change */ - if (opt_orphan || !auto_update_summary) - { - gboolean commit_update_summary; - - /* The default for this option is FALSE, even for archive repos, - * because ostree supports multiple processes committing to the same - * repo (but different refs) concurrently, and in fact gnome-continuous - * actually does this. In that context it's best to update the summary - * explicitly instead of automatically here. */ - if (!ot_keyfile_get_boolean_with_default (ostree_repo_get_config (repo), "core", - "commit-update-summary", FALSE, - &commit_update_summary, error)) - goto out; - - if (commit_update_summary && !ostree_repo_regenerate_summary (repo, - NULL, - cancellable, - error)) - goto out; - } } else {