From d0e74cf3af73d6855eebe9c52f19105f7c84df0a Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Mon, 24 Oct 2016 11:12:15 +0200 Subject: [PATCH 01/41] Fix pruning of partial commits If we have a partial commit it is not an error for a dirmeta to be missing (in fact, that is likely), so instead of returning a not-found error from ostree_repo_traverse_commit() we ignore the error and continue. In particular, this means we don't stop early at the first missing dirmeta, which previously caused ostree_repo_prune() to thing the dirmetas after that to be unreached and thus purged. Also, we remove the special casing in ostree_repo_prune() to not report errors for commitpartial, because these should not be reported anymore. This fixes https://github.com/ostreedev/ostree/issues/541 Closes: #542 Approved by: cgwalters --- src/libostree/ostree-repo-prune.c | 44 ++------------------ src/libostree/ostree-repo-traverse.c | 60 +++++++++++++++++++++++----- tests/test-pull-subpath.sh | 25 +++++++++++- 3 files changed, 77 insertions(+), 52 deletions(-) diff --git a/src/libostree/ostree-repo-prune.c b/src/libostree/ostree-repo-prune.c index 66170d4d..22bff915 100644 --- a/src/libostree/ostree-repo-prune.c +++ b/src/libostree/ostree-repo-prune.c @@ -311,29 +311,11 @@ ostree_repo_prune (OstreeRepo *self, while (g_hash_table_iter_next (&hash_iter, &key, &value)) { const char *checksum = value; - OstreeRepoCommitState commitstate; - GError *local_error = NULL; - - if (!ostree_repo_load_commit (self, checksum, NULL, &commitstate, - error)) - goto out; g_debug ("Finding objects to keep for commit %s", checksum); if (!ostree_repo_traverse_commit_union (self, checksum, depth, data.reachable, - cancellable, &local_error)) - { - /* Don't fail traversing a partial commit */ - if ((commitstate & OSTREE_REPO_COMMIT_STATE_PARTIAL) > 0 && - g_error_matches (local_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND)) - { - g_clear_error (&local_error); - } - else - { - g_propagate_error (error, local_error); - goto out; - } - } + cancellable, error)) + goto out; } } @@ -349,34 +331,16 @@ ostree_repo_prune (OstreeRepo *self, GVariant *serialized_key = key; const char *checksum; OstreeObjectType objtype; - OstreeRepoCommitState commitstate; - GError *local_error = NULL; ostree_object_name_deserialize (serialized_key, &checksum, &objtype); if (objtype != OSTREE_OBJECT_TYPE_COMMIT) continue; - if (!ostree_repo_load_commit (self, checksum, NULL, &commitstate, - error)) - goto out; - g_debug ("Finding objects to keep for commit %s", checksum); if (!ostree_repo_traverse_commit_union (self, checksum, depth, data.reachable, - cancellable, &local_error)) - { - /* Don't fail traversing a partial commit */ - if ((commitstate & OSTREE_REPO_COMMIT_STATE_PARTIAL) > 0 && - g_error_matches (local_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND)) - { - g_clear_error (&local_error); - } - else - { - g_propagate_error (error, local_error); - goto out; - } - } + cancellable, error)) + goto out; } } diff --git a/src/libostree/ostree-repo-traverse.c b/src/libostree/ostree-repo-traverse.c index 516f39f5..5d994763 100644 --- a/src/libostree/ostree-repo-traverse.c +++ b/src/libostree/ostree-repo-traverse.c @@ -301,6 +301,7 @@ static gboolean traverse_dirtree (OstreeRepo *repo, const char *checksum, GHashTable *inout_reachable, + gboolean ignore_missing_dirs, GCancellable *cancellable, GError **error); @@ -308,6 +309,7 @@ static gboolean traverse_iter (OstreeRepo *repo, OstreeRepoCommitTraverseIter *iter, GHashTable *inout_reachable, + gboolean ignore_missing_dirs, GCancellable *cancellable, GError **error) { @@ -316,11 +318,26 @@ traverse_iter (OstreeRepo *repo, while (TRUE) { g_autoptr(GVariant) key = NULL; + g_autoptr(GError) local_error = NULL; OstreeRepoCommitIterResult iterres = - ostree_repo_commit_traverse_iter_next (iter, cancellable, error); - + ostree_repo_commit_traverse_iter_next (iter, cancellable, &local_error); + if (iterres == OSTREE_REPO_COMMIT_ITER_RESULT_ERROR) - goto out; + { + /* There is only one kind of not-found error, which is + failing to load the dirmeta itself, if so, we ignore that + (and the whole subtree) if told to. */ + if (ignore_missing_dirs && + g_error_matches (local_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND)) + { + g_debug ("Ignoring not-found dirmeta"); + ret = TRUE; + } + else + g_propagate_error (error, g_steal_pointer (&local_error)); + + goto out; + } else if (iterres == OSTREE_REPO_COMMIT_ITER_RESULT_END) break; else if (iterres == OSTREE_REPO_COMMIT_ITER_RESULT_FILE) @@ -357,7 +374,7 @@ traverse_iter (OstreeRepo *repo, key = NULL; if (!traverse_dirtree (repo, content_checksum, inout_reachable, - cancellable, error)) + ignore_missing_dirs, cancellable, error)) goto out; } } @@ -374,6 +391,7 @@ static gboolean traverse_dirtree (OstreeRepo *repo, const char *checksum, GHashTable *inout_reachable, + gboolean ignore_missing_dirs, GCancellable *cancellable, GError **error) { @@ -381,10 +399,22 @@ traverse_dirtree (OstreeRepo *repo, g_autoptr(GVariant) dirtree = NULL; ostree_cleanup_repo_commit_traverse_iter OstreeRepoCommitTraverseIter iter = { 0, }; + g_autoptr(GError) local_error = NULL; if (!ostree_repo_load_variant (repo, OSTREE_OBJECT_TYPE_DIR_TREE, checksum, - &dirtree, error)) - goto out; + &dirtree, &local_error)) + { + if (ignore_missing_dirs && + g_error_matches (local_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND)) + { + g_print ("Ignoring not-found dirmeta %s", checksum); + ret = TRUE; + } + else + g_propagate_error (error, g_steal_pointer (&local_error)); + + goto out; + } g_debug ("Traversing dirtree %s", checksum); if (!ostree_repo_commit_traverse_iter_init_dirtree (&iter, repo, dirtree, @@ -392,7 +422,7 @@ traverse_dirtree (OstreeRepo *repo, error)) goto out; - if (!traverse_iter (repo, &iter, inout_reachable, cancellable, error)) + if (!traverse_iter (repo, &iter, inout_reachable, ignore_missing_dirs, cancellable, error)) goto out; ret = TRUE; @@ -430,6 +460,8 @@ ostree_repo_traverse_commit_union (OstreeRepo *repo, g_autoptr(GVariant) commit = NULL; ostree_cleanup_repo_commit_traverse_iter OstreeRepoCommitTraverseIter iter = { 0, }; + OstreeRepoCommitState commitstate; + gboolean ignore_missing_dirs = FALSE; key = ostree_object_name_serialize (commit_checksum, OSTREE_OBJECT_TYPE_COMMIT); @@ -440,13 +472,21 @@ ostree_repo_traverse_commit_union (OstreeRepo *repo, commit_checksum, &commit, error)) goto out; - + /* Just return if the parent isn't found; we do expect most * people to have partial repositories. */ if (!commit) break; + /* See if the commit is partial, if so it's not an error to lack objects */ + if (!ostree_repo_load_commit (repo, commit_checksum, NULL, &commitstate, + error)) + goto out; + + if ((commitstate & OSTREE_REPO_COMMIT_STATE_PARTIAL) != 0) + ignore_missing_dirs = TRUE; + g_hash_table_add (inout_reachable, key); key = NULL; @@ -456,9 +496,9 @@ ostree_repo_traverse_commit_union (OstreeRepo *repo, error)) goto out; - if (!traverse_iter (repo, &iter, inout_reachable, cancellable, error)) + if (!traverse_iter (repo, &iter, inout_reachable, ignore_missing_dirs, cancellable, error)) goto out; - + if (maxdepth == -1 || maxdepth > 0) { g_free (tmp_checksum); diff --git a/tests/test-pull-subpath.sh b/tests/test-pull-subpath.sh index 09145f09..dba8b495 100755 --- a/tests/test-pull-subpath.sh +++ b/tests/test-pull-subpath.sh @@ -23,7 +23,7 @@ set -euo pipefail setup_fake_remote_repo1 "archive-z2" -echo '1..2' +echo '1..4' repopath=${test_tmpdir}/ostree-srv/gnomerepo cp -a ${repopath} ${repopath}.orig @@ -54,5 +54,26 @@ ${CMD_PREFIX} ostree --repo=repo ls origin:main /firstfile ${CMD_PREFIX} ostree --repo=repo pull origin main assert_not_has_file repo/state/${rev}.commitpartial ${CMD_PREFIX} ostree --repo=repo fsck -echo "ok" +echo "ok subpaths" + +rm -rf repo + +${CMD_PREFIX} ostree --repo=repo init +${CMD_PREFIX} ostree --repo=repo remote add --set=gpg-verify=false origin ${remoteurl} + +# Pull a directory which is not the first in the commit (/baz/another is before) +${CMD_PREFIX} ostree --repo=repo pull --subpath=/baz/deeper origin main + +# Ensure it is there +${CMD_PREFIX} ostree --repo=repo ls origin:main /baz/deeper + +# Now prune, this should not prune the /baz/deeper dirmeta even if the +# /baz/another dirmeta is not in the repo. +${CMD_PREFIX} ostree --repo=repo prune --refs-only + +# Ensure it is still there +${CMD_PREFIX} ostree --repo=repo ls origin:main /baz/deeper + +echo "ok prune with commitpartial" + done From 27f37b55bcd87ca156119dbbf3afb781dc81b430 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 24 Oct 2016 13:54:25 -0400 Subject: [PATCH 02/41] docs: Link to releng-scripts Now that the repo starts to implement some of this stuff. Closes: #544 Approved by: jlebon --- docs/manual/repository-management.md | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/docs/manual/repository-management.md b/docs/manual/repository-management.md index b6da9629..5688a582 100644 --- a/docs/manual/repository-management.md +++ b/docs/manual/repository-management.md @@ -13,11 +13,12 @@ generating commits. In this section, we will describe some high level ideas and methods for managing content in OSTree repositories, mostly independent of any -particular model or tool. That said, a goal is to include at least -some sample scripts and workflows upstream in a potential new -"contrib" git repository. +particular model or tool. That said, there is an associated upstream +project [ostree-releng-scripts](https://github.com/ostreedev/ostree-releng-scripts) +which has some scripts that are intended to implement portions of +this document. -One example of software which can assist in managing OSTree +Another example of software which can assist in managing OSTree repositories today is the [Pulp Project](http://www.pulpproject.org/), which has a [Pulp OSTree plugin](https://pulp-ostree.readthedocs.org/en/latest/). @@ -43,6 +44,9 @@ ostree --repo=repo pull --mirror exampleos:exampleos/x86_64/standard You can use the `--depth=-1` option to retrieve all history, or a positive integer like `3` to retrieve just the last 3 commits. +See also the `rsync-repos` script in +[ostree-releng-scripts](https://github.com/ostreedev/ostree-releng-scripts). + ## Separate development vs release repositories By default, OSTree accumulates server side history. This is actually @@ -94,7 +98,7 @@ want to "promote" that commit. Let's create a new branch called complete system. This might be where human testers get involved, for example. -The build system can "promote" the `buildmaster` commit that passed +A basic way to "promote" the `buildmaster` commit that passed testing like this: ``` @@ -105,6 +109,11 @@ Here we're generating a new commit object (perhaps include in the commit log links to build logs, etc.), but we're reusing the *content* from the `buildmaster` commit `aec070645fe53` that passed the smoketests. +For a more sophisticated implementation of this model, see the +[do-release-tags](https://github.com/ostreedev/ostree-releng-scripts/blob/master/do-release-tags) +script, which includes support for things like propagating version +numbers across commit promotion. + We can easily generalize this model to have an arbitrary number of stages like `exampleos/x86_64/stage-1-pass/standard`, `exampleos/x86_64/stage-2-pass/standard`, etc. depending on business From 82a4f56593207b2f8a5e6620e64d9c3fa65fbfcf Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 25 Oct 2016 10:40:45 -0400 Subject: [PATCH 03/41] tests: Skip libarchive/selinux tests if in container without SELinux I'm doing builds and `make check` inside a Docker container, with selinux on as a build-time option, but no policy in the container. This currently aborts. Let's not do that. (This type of thing is why installed tests are a better model) Closes: #546 Approved by: jlebon --- tests/test-libarchive-import.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/test-libarchive-import.c b/tests/test-libarchive-import.c index 3ef379e3..ad2e4c94 100644 --- a/tests/test-libarchive-import.c +++ b/tests/test-libarchive-import.c @@ -501,12 +501,10 @@ test_libarchive_selinux (gconstpointer data) { glnx_unref_object GFile *root = g_file_new_for_path ("/"); - /* creation should always succeed */ - sepol = ostree_sepolicy_new (root, NULL, &error); - g_assert (sepol != NULL); + sepol = ostree_sepolicy_new (root, NULL, NULL); } - if (ostree_sepolicy_get_name (sepol) == NULL) + if (sepol == NULL || ostree_sepolicy_get_name (sepol) == NULL) { g_test_skip ("SELinux disabled"); goto out; From b77edf24a3afb7a26b40a8adfd81e88ce8d42182 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 25 Oct 2016 13:06:36 -0400 Subject: [PATCH 04/41] tree-wide: Remove unused variables detected by CLang CLang finds these, whereas GCC treats having `__attribute__((cleanup))` as a use. This obsoletes https://github.com/ostreedev/ostree/pull/411 Closes: #548 Approved by: jlebon --- src/libostree/ostree-bootloader-grub2.c | 3 --- src/libostree/ostree-fetcher.c | 1 - src/libostree/ostree-repo-commit.c | 5 ----- src/libostree/ostree-repo-libarchive.c | 2 -- src/libostree/ostree-repo-pull.c | 8 -------- src/libostree/ostree-repo-static-delta-compilation.c | 8 -------- src/libostree/ostree-repo-static-delta-core.c | 2 -- src/libostree/ostree-repo-static-delta-processing.c | 2 -- src/libostree/ostree-repo.c | 2 -- src/libostree/ostree-sysroot-deploy.c | 2 -- src/libostree/ostree-sysroot-upgrader.c | 1 - src/libostree/ostree-sysroot.c | 1 - src/libotutil/ot-gpg-utils.c | 3 --- src/ostree/ot-admin-builtin-deploy.c | 1 - src/ostree/ot-admin-builtin-set-origin.c | 2 -- src/ostree/ot-admin-builtin-switch.c | 5 ----- src/ostree/ot-admin-builtin-undeploy.c | 1 - src/ostree/ot-admin-builtin-unlock.c | 3 --- src/ostree/ot-admin-builtin-upgrade.c | 2 -- src/ostree/ot-admin-instutil-builtin-grub2-generate.c | 4 ---- src/ostree/ot-builtin-pull-local.c | 1 - src/ostree/ot-builtin-static-delta.c | 1 - src/ostree/ot-remote-builtin-add.c | 2 -- tests/test-mutable-tree.c | 3 --- 24 files changed, 65 deletions(-) diff --git a/src/libostree/ostree-bootloader-grub2.c b/src/libostree/ostree-bootloader-grub2.c index f3dc8e16..0fbb098e 100644 --- a/src/libostree/ostree-bootloader-grub2.c +++ b/src/libostree/ostree-bootloader-grub2.c @@ -54,7 +54,6 @@ _ostree_bootloader_grub2_query (OstreeBootloader *bootloader, gboolean ret = FALSE; OstreeBootloaderGrub2 *self = OSTREE_BOOTLOADER_GRUB2 (bootloader); g_autoptr(GFile) efi_basedir = NULL; - g_autoptr(GFileInfo) file_info = NULL; if (g_file_query_exists (self->config_path_bios, NULL)) { @@ -290,8 +289,6 @@ _ostree_bootloader_grub2_write_config (OstreeBootloader *bootloader, { OstreeBootloaderGrub2 *self = OSTREE_BOOTLOADER_GRUB2 (bootloader); gboolean ret = FALSE; - g_autoptr(GFile) efi_new_config_temp = NULL; - g_autoptr(GFile) efi_orig_config = NULL; g_autoptr(GFile) new_config_path = NULL; GSubprocessFlags subp_flags = 0; glnx_unref_object GSubprocessLauncher *launcher = NULL; diff --git a/src/libostree/ostree-fetcher.c b/src/libostree/ostree-fetcher.c index c2dc8ea4..21526813 100644 --- a/src/libostree/ostree-fetcher.c +++ b/src/libostree/ostree-fetcher.c @@ -1282,7 +1282,6 @@ _ostree_fetcher_mirrored_request_to_membuf (OstreeFetcher *fetcher, { gboolean ret = FALSE; const guint8 nulchar = 0; - g_autofree char *ret_contents = NULL; g_autoptr(GMemoryOutputStream) buf = NULL; g_autoptr(GMainContext) mainctx = NULL; FetchUriSyncData data; diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index 42e06400..7d78a19b 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -592,7 +592,6 @@ write_object (OstreeRepo *self, gboolean do_commit; OstreeRepoMode repo_mode; g_autofree char *temp_filename = NULL; - g_autoptr(GFile) stored_path = NULL; g_autofree guchar *ret_csum = NULL; glnx_unref_object OstreeChecksumInputStream *checksum_input = NULL; g_autoptr(GInputStream) file_input = NULL; @@ -1093,9 +1092,6 @@ ostree_repo_prepare_transaction (OstreeRepo *self, { gboolean ret = FALSE; gboolean ret_transaction_resume = FALSE; - g_autofree char *stagedir_name = NULL; - glnx_fd_close int stagedir_fd = -1; - g_auto(GLnxDirFdIterator) dfd_iter = { 0, }; g_return_val_if_fail (self->in_transaction == FALSE, FALSE); @@ -2529,7 +2525,6 @@ write_directory_to_mtree_internal (OstreeRepo *self, gboolean ret = FALSE; OstreeRepoCommitFilterResult filter_result; OstreeRepoFile *repo_dir = NULL; - g_autoptr(GFileEnumerator) dir_enum = NULL; g_autoptr(GFileInfo) child_info = NULL; if (dir) diff --git a/src/libostree/ostree-repo-libarchive.c b/src/libostree/ostree-repo-libarchive.c index d05b34ea..3a58d106 100644 --- a/src/libostree/ostree-repo-libarchive.c +++ b/src/libostree/ostree-repo-libarchive.c @@ -300,7 +300,6 @@ aic_apply_modifier_filter (OstreeRepoArchiveImportContext *ctx, const char *relpath, GFileInfo **out_file_info) { - g_autofree char *hardlink = aic_get_final_entry_hardlink (ctx); g_autoptr(GFileInfo) file_info = NULL; g_autofree char *abspath = NULL; const char *cb_path = NULL; @@ -915,7 +914,6 @@ ostree_repo_write_archive_to_mtree (OstreeRepo *self, #ifdef HAVE_LIBARCHIVE gboolean ret = FALSE; struct archive *a = NULL; - g_autoptr(GFileInfo) tmp_dir_info = NULL; OstreeRepoImportArchiveOptions opts = { 0, }; a = archive_read_new (); diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 52455ed4..9d361d5f 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -608,10 +608,8 @@ lookup_commit_checksum_from_summary (OtPullData *pull_data, g_autoptr(GVariant) refs = g_variant_get_child_value (pull_data->summary, 0); g_autoptr(GVariant) refdata = NULL; g_autoptr(GVariant) reftargetdata = NULL; - g_autoptr(GVariant) commit_data = NULL; guint64 commit_size; g_autoptr(GVariant) commit_csum_v = NULL; - g_autoptr(GBytes) commit_bytes = NULL; int i; if (!ot_variant_bsearch_str (refs, ref, &i)) @@ -973,7 +971,6 @@ static_deltapart_fetch_on_complete (GObject *object, OstreeFetcher *fetcher = (OstreeFetcher *)object; FetchStaticDeltaData *fetch_data = user_data; OtPullData *pull_data = fetch_data->pull_data; - g_autoptr(GVariant) metadata = NULL; g_autofree char *temp_path = NULL; g_autoptr(GInputStream) in = NULL; g_autoptr(GVariant) part = NULL; @@ -1430,8 +1427,6 @@ request_static_delta_superblock_sync (OtPullData *pull_data, g_autofree char *delta_name = _ostree_get_relative_static_delta_superblock_path (from_revision, to_revision); g_autoptr(GBytes) delta_superblock_data = NULL; - g_autoptr(GBytes) delta_meta_data = NULL; - g_autoptr(GVariant) delta_superblock = NULL; if (!_ostree_fetcher_mirrored_request_to_membuf (pull_data->fetcher, pull_data->content_mirrorlist, @@ -2335,8 +2330,6 @@ ostree_repo_pull_with_options (OstreeRepo *self, gboolean opt_gpg_verify_set = FALSE; gboolean opt_gpg_verify_summary_set = FALSE; const char *url_override = NULL; - g_autofree char *base_meta_url = NULL; - g_autofree char *base_content_url = NULL; gboolean mirroring_into_archive; gboolean inherit_transaction = FALSE; int i; @@ -2641,7 +2634,6 @@ ostree_repo_pull_with_options (OstreeRepo *self, { g_autoptr(GBytes) bytes_sig = NULL; - g_autofree char *ret_contents = NULL; gsize i, n; g_autoptr(GVariant) refs = NULL; g_autoptr(GVariant) deltas = NULL; diff --git a/src/libostree/ostree-repo-static-delta-compilation.c b/src/libostree/ostree-repo-static-delta-compilation.c index acface61..d73bdc83 100644 --- a/src/libostree/ostree-repo-static-delta-compilation.c +++ b/src/libostree/ostree-repo-static-delta-compilation.c @@ -506,8 +506,6 @@ try_content_bsdiff (OstreeRepo *repo, GError **error) { gboolean ret = FALSE; - g_autoptr(GHashTable) from_bsdiff = NULL; - g_autoptr(GHashTable) to_bsdiff = NULL; g_autoptr(GBytes) tmp_from = NULL; g_autoptr(GBytes) tmp_to = NULL; g_autoptr(GFileInfo) from_finfo = NULL; @@ -551,8 +549,6 @@ try_content_rollsum (OstreeRepo *repo, GError **error) { gboolean ret = FALSE; - g_autoptr(GHashTable) from_rollsum = NULL; - g_autoptr(GHashTable) to_rollsum = NULL; g_autoptr(GBytes) tmp_from = NULL; g_autoptr(GBytes) tmp_to = NULL; g_autoptr(GFileInfo) from_finfo = NULL; @@ -884,14 +880,12 @@ generate_delta_lowlatency (OstreeRepo *repo, g_autoptr(GVariant) to_commit = NULL; g_autoptr(GHashTable) to_reachable_objects = NULL; g_autoptr(GHashTable) from_reachable_objects = NULL; - g_autoptr(GHashTable) from_regfile_content = NULL; g_autoptr(GHashTable) new_reachable_metadata = NULL; g_autoptr(GHashTable) new_reachable_regfile_content = NULL; g_autoptr(GHashTable) new_reachable_symlink_content = NULL; g_autoptr(GHashTable) modified_regfile_content = NULL; g_autoptr(GHashTable) rollsum_optimized_content_objects = NULL; g_autoptr(GHashTable) bsdiff_optimized_content_objects = NULL; - g_autoptr(GHashTable) content_object_to_size = NULL; if (from != NULL) { @@ -1273,7 +1267,6 @@ ostree_repo_static_delta_generate (OstreeRepo *self, const char *opt_filename; g_autofree char *descriptor_name = NULL; glnx_fd_close int descriptor_dfd = -1; - g_autoptr(GVariant) tmp_metadata = NULL; g_autoptr(GVariant) fallback_headers = NULL; g_autoptr(GVariant) detached = NULL; gboolean inline_parts; @@ -1387,7 +1380,6 @@ ostree_repo_static_delta_generate (OstreeRepo *self, GBytes *payload_b; GBytes *operations_b; g_autofree guchar *part_checksum = NULL; - g_autoptr(GChecksum) checksum = NULL; g_autoptr(GBytes) objtype_checksum_array = NULL; g_autoptr(GBytes) checksum_bytes = NULL; g_autoptr(GOutputStream) part_temp_outstream = NULL; diff --git a/src/libostree/ostree-repo-static-delta-core.c b/src/libostree/ostree-repo-static-delta-core.c index 82c80a2a..77e7939a 100644 --- a/src/libostree/ostree-repo-static-delta-core.c +++ b/src/libostree/ostree-repo-static-delta-core.c @@ -356,7 +356,6 @@ ostree_repo_static_delta_execute_offline (OstreeRepo *self, char checksum[OSTREE_SHA256_STRING_LEN+1]; gboolean have_all; g_autoptr(GInputStream) part_in = NULL; - g_autoptr(GBytes) delta_data = NULL; g_autoptr(GVariant) inline_part_data = NULL; g_autoptr(GVariant) header = NULL; g_autoptr(GVariant) csum_v = NULL; @@ -864,7 +863,6 @@ _ostree_repo_static_delta_dump (OstreeRepo *self, g_autofree char *from = NULL; g_autofree char *to = NULL; g_autofree char *superblock_path = NULL; - glnx_fd_close int superblock_fd = -1; g_autoptr(GVariant) delta_superblock = NULL; guint64 total_size = 0, total_usize = 0; guint64 total_fallback_size = 0, total_fallback_usize = 0; diff --git a/src/libostree/ostree-repo-static-delta-processing.c b/src/libostree/ostree-repo-static-delta-processing.c index 304d7b4a..95f9ddba 100644 --- a/src/libostree/ostree-repo-static-delta-processing.c +++ b/src/libostree/ostree-repo-static-delta-processing.c @@ -188,7 +188,6 @@ _ostree_static_delta_part_execute (OstreeRepo *repo, { gboolean ret = FALSE; guint8 *checksums_data; - g_autoptr(GVariant) checksums = NULL; g_autoptr(GVariant) mode_dict = NULL; g_autoptr(GVariant) xattr_dict = NULL; g_autoptr(GVariant) payload = NULL; @@ -471,7 +470,6 @@ dispatch_bspatch (OstreeRepo *repo, { gboolean ret = FALSE; guint64 offset, length; - g_autoptr(GInputStream) in_stream = NULL; g_autoptr(GMappedFile) input_mfile = NULL; g_autofree guchar *buf = NULL; struct bspatch_stream stream; diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index d4b1f1d6..305588a9 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -4687,7 +4687,6 @@ ostree_repo_regenerate_summary (OstreeRepo *self, guint i; g_autoptr(GPtrArray) delta_names = NULL; g_auto(GVariantDict) deltas_builder = {{0,}}; - g_autoptr(GVariant) deltas = NULL; if (!ostree_repo_list_static_delta_names (self, &delta_names, cancellable, error)) goto out; @@ -4890,7 +4889,6 @@ _ostree_repo_allocate_tmpdir (int tmpdir_dfd, { g_autofree char *tmpdir_name_template = g_strconcat (tmpdir_prefix, "XXXXXX", NULL); glnx_fd_close int new_tmpdir_fd = -1; - g_autoptr(GError) local_error = NULL; /* No existing tmpdir found, create a new */ diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index a14f6005..41a9a8df 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -534,7 +534,6 @@ checkout_deployment_tree (OstreeSysroot *sysroot, const char *csum = ostree_deployment_get_csum (deployment); g_autofree char *checkout_target_name = NULL; g_autofree char *osdeploy_path = NULL; - g_autoptr(GFile) ret_deploy_target_path = NULL; glnx_fd_close int osdeploy_dfd = -1; int ret_fd; @@ -1199,7 +1198,6 @@ swap_bootlinks (OstreeSysroot *self, gboolean ret = FALSE; int old_subbootversion, new_subbootversion; glnx_fd_close int ostree_dfd = -1; - glnx_fd_close int ostree_subbootdir_dfd = -1; g_autofree char *ostree_bootdir_name = NULL; g_autofree char *ostree_subbootdir_name = NULL; diff --git a/src/libostree/ostree-sysroot-upgrader.c b/src/libostree/ostree-sysroot-upgrader.c index 92b8dc83..22710bda 100644 --- a/src/libostree/ostree-sysroot-upgrader.c +++ b/src/libostree/ostree-sysroot-upgrader.c @@ -530,7 +530,6 @@ ostree_sysroot_upgrader_pull_one_dir (OstreeSysrootUpgrader *self, glnx_unref_object OstreeRepo *repo = NULL; char *refs_to_fetch[] = { NULL, NULL }; const char *from_revision = NULL; - g_autofree char *new_revision = NULL; g_autofree char *origin_refspec = NULL; if (self->override_csum != NULL) diff --git a/src/libostree/ostree-sysroot.c b/src/libostree/ostree-sysroot.c index 97f00c4e..608d4cff 100644 --- a/src/libostree/ostree-sysroot.c +++ b/src/libostree/ostree-sysroot.c @@ -689,7 +689,6 @@ parse_deployment (OstreeSysroot *self, glnx_fd_close int deployment_dfd = -1; const char *deploy_basename; g_autofree char *treebootserial_target = NULL; - g_autofree char *deploy_dir = NULL; GKeyFile *origin = NULL; g_autofree char *unlocked_development_path = NULL; struct stat stbuf; diff --git a/src/libotutil/ot-gpg-utils.c b/src/libotutil/ot-gpg-utils.c index 9414088d..9042d88b 100644 --- a/src/libotutil/ot-gpg-utils.c +++ b/src/libotutil/ot-gpg-utils.c @@ -73,9 +73,6 @@ ot_gpgme_ctx_tmp_home_dir (gpgme_ctx_t gpgme_ctx, GCancellable *cancellable, GError **error) { - g_autoptr(GFile) pubring_file = NULL; - g_autoptr(GOutputStream) target_stream = NULL; - g_autofree char *pubring_path = NULL; g_autofree char *tmp_home_dir = NULL; gpgme_error_t gpg_error; gboolean ret = FALSE; diff --git a/src/ostree/ot-admin-builtin-deploy.c b/src/ostree/ot-admin-builtin-deploy.c index c66c9b3f..3039cf0d 100644 --- a/src/ostree/ot-admin-builtin-deploy.c +++ b/src/ostree/ot-admin-builtin-deploy.c @@ -58,7 +58,6 @@ ot_admin_builtin_deploy (int argc, char **argv, GCancellable *cancellable, GErro glnx_unref_object OstreeSysroot *sysroot = NULL; GKeyFile *origin = NULL; glnx_unref_object OstreeRepo *repo = NULL; - g_autoptr(GPtrArray) new_deployments = NULL; glnx_unref_object OstreeDeployment *new_deployment = NULL; glnx_unref_object OstreeDeployment *merge_deployment = NULL; g_autofree char *revision = NULL; diff --git a/src/ostree/ot-admin-builtin-set-origin.c b/src/ostree/ot-admin-builtin-set-origin.c index be88aa6c..8b32eb79 100644 --- a/src/ostree/ot-admin-builtin-set-origin.c +++ b/src/ostree/ot-admin-builtin-set-origin.c @@ -120,7 +120,6 @@ ot_admin_builtin_set_origin (int argc, char **argv, GCancellable *cancellable, G { GKeyFile *old_origin = ostree_deployment_get_origin (target_deployment); g_autofree char *origin_refspec = g_key_file_get_string (old_origin, "origin", "refspec", NULL); - g_autofree char *new_refspec = NULL; g_autofree char *origin_remote = NULL; g_autofree char *origin_ref = NULL; @@ -129,7 +128,6 @@ ot_admin_builtin_set_origin (int argc, char **argv, GCancellable *cancellable, G { g_autofree char *new_refspec = g_strconcat (remotename, ":", branch ? branch : origin_ref, NULL); g_autoptr(GKeyFile) new_origin = NULL; - g_autoptr(GFile) origin_path = NULL; new_origin = ostree_sysroot_origin_new_from_refspec (sysroot, new_refspec); diff --git a/src/ostree/ot-admin-builtin-switch.c b/src/ostree/ot-admin-builtin-switch.c index 895538aa..485e6cd4 100644 --- a/src/ostree/ot-admin-builtin-switch.c +++ b/src/ostree/ot-admin-builtin-switch.c @@ -53,11 +53,6 @@ ot_admin_builtin_switch (int argc, char **argv, GCancellable *cancellable, GErro g_autofree char *new_remote = NULL; g_autofree char *new_ref = NULL; g_autofree char *new_refspec = NULL; - g_autofree char *new_revision = NULL; - g_autoptr(GFile) deployment_path = NULL; - g_autoptr(GFile) deployment_origin_path = NULL; - glnx_unref_object OstreeDeployment *merge_deployment = NULL; - glnx_unref_object OstreeDeployment *new_deployment = NULL; glnx_unref_object OstreeSysrootUpgrader *upgrader = NULL; glnx_unref_object OstreeAsyncProgress *progress = NULL; gboolean changed; diff --git a/src/ostree/ot-admin-builtin-undeploy.c b/src/ostree/ot-admin-builtin-undeploy.c index e048729d..dd41950e 100644 --- a/src/ostree/ot-admin-builtin-undeploy.c +++ b/src/ostree/ot-admin-builtin-undeploy.c @@ -41,7 +41,6 @@ ot_admin_builtin_undeploy (int argc, char **argv, GCancellable *cancellable, GEr const char *deploy_index_str; int deploy_index; g_autoptr(GPtrArray) current_deployments = NULL; - glnx_unref_object OstreeDeployment *booted_deployment = NULL; glnx_unref_object OstreeDeployment *target_deployment = NULL; context = g_option_context_new ("INDEX - Delete deployment INDEX"); diff --git a/src/ostree/ot-admin-builtin-unlock.c b/src/ostree/ot-admin-builtin-unlock.c index 9d265325..a1789377 100644 --- a/src/ostree/ot-admin-builtin-unlock.c +++ b/src/ostree/ot-admin-builtin-unlock.c @@ -44,9 +44,6 @@ ot_admin_builtin_unlock (int argc, char **argv, GCancellable *cancellable, GErro gboolean ret = FALSE; GOptionContext *context; glnx_unref_object OstreeSysroot *sysroot = NULL; - glnx_unref_object OstreeRepo *repo = NULL; - g_autoptr(GPtrArray) new_deployments = NULL; - glnx_unref_object OstreeDeployment *merge_deployment = NULL; OstreeDeployment *booted_deployment = NULL; OstreeDeploymentUnlockedState target_state; diff --git a/src/ostree/ot-admin-builtin-upgrade.c b/src/ostree/ot-admin-builtin-upgrade.c index 81f9bb6f..2ad74fc6 100644 --- a/src/ostree/ot-admin-builtin-upgrade.c +++ b/src/ostree/ot-admin-builtin-upgrade.c @@ -52,8 +52,6 @@ ot_admin_builtin_upgrade (int argc, char **argv, GCancellable *cancellable, GErr GOptionContext *context; glnx_unref_object OstreeSysroot *sysroot = NULL; glnx_unref_object OstreeSysrootUpgrader *upgrader = NULL; - g_autoptr(GFile) deployment_path = NULL; - g_autoptr(GFile) deployment_origin_path = NULL; g_autoptr(GKeyFile) origin = NULL; glnx_unref_object OstreeAsyncProgress *progress = NULL; gboolean changed; diff --git a/src/ostree/ot-admin-instutil-builtin-grub2-generate.c b/src/ostree/ot-admin-instutil-builtin-grub2-generate.c index ee7fe702..7d837ff4 100644 --- a/src/ostree/ot-admin-instutil-builtin-grub2-generate.c +++ b/src/ostree/ot-admin-instutil-builtin-grub2-generate.c @@ -38,12 +38,8 @@ ot_admin_instutil_builtin_grub2_generate (int argc, char **argv, GCancellable *c { gboolean ret = FALSE; guint bootversion; - g_autoptr(GFile) subpath = NULL; - glnx_unref_object OstreeSePolicy *sepolicy = NULL; - g_autoptr(GPtrArray) deployments = NULL; GOptionContext *context = NULL; glnx_unref_object OstreeSysroot *sysroot = NULL; - g_autoptr(GFile) deployment_path = NULL; context = g_option_context_new ("[BOOTVERSION] - generate GRUB2 configuration from given BLS entries"); diff --git a/src/ostree/ot-builtin-pull-local.c b/src/ostree/ot-builtin-pull-local.c index 5401a281..58a653ff 100644 --- a/src/ostree/ot-builtin-pull-local.c +++ b/src/ostree/ot-builtin-pull-local.c @@ -60,7 +60,6 @@ ostree_builtin_pull_local (int argc, char **argv, GCancellable *cancellable, GEr g_autofree char *src_repo_uri = NULL; glnx_unref_object OstreeAsyncProgress *progress = NULL; g_autoptr(GPtrArray) refs_to_fetch = NULL; - g_autoptr(GHashTable) source_objects = NULL; OstreeRepoPullFlags pullflags = 0; context = g_option_context_new ("SRC_REPO [REFS...] - Copy data from SRC_REPO"); diff --git a/src/ostree/ot-builtin-static-delta.c b/src/ostree/ot-builtin-static-delta.c index a1c220bb..ca29911a 100644 --- a/src/ostree/ot-builtin-static-delta.c +++ b/src/ostree/ot-builtin-static-delta.c @@ -399,7 +399,6 @@ ostree_builtin_static_delta (int argc, char **argv, GCancellable *cancellable, G gboolean ret = FALSE; OstreeCommand *command = NULL; const char *cmdname = NULL; - glnx_unref_object OstreeRepo *repo = NULL; int i; gboolean want_help = FALSE; diff --git a/src/ostree/ot-remote-builtin-add.c b/src/ostree/ot-remote-builtin-add.c index 76b0c75a..6e83b2af 100644 --- a/src/ostree/ot-remote-builtin-add.c +++ b/src/ostree/ot-remote-builtin-add.c @@ -49,8 +49,6 @@ ot_remote_builtin_add (int argc, char **argv, GCancellable *cancellable, GError const char *remote_name; const char *remote_url; char **iter; - g_autofree char *target_name = NULL; - g_autoptr(GFile) target_conf = NULL; g_autoptr(GVariantBuilder) optbuilder = NULL; gboolean ret = FALSE; diff --git a/tests/test-mutable-tree.c b/tests/test-mutable-tree.c index fa2f68db..b357f691 100644 --- a/tests/test-mutable-tree.c +++ b/tests/test-mutable-tree.c @@ -110,7 +110,6 @@ test_ensure_dir (void) { glnx_unref_object OstreeMutableTree *tree = ostree_mutable_tree_new (); glnx_unref_object OstreeMutableTree *parent = NULL; - g_autoptr(GPtrArray) split_path = NULL; GError *error = NULL; const char *dirname = "foo"; const char *filename = "bar"; @@ -129,8 +128,6 @@ static void test_replace_file (void) { glnx_unref_object OstreeMutableTree *tree = ostree_mutable_tree_new (); - glnx_unref_object OstreeMutableTree *parent = NULL; - g_autoptr(GPtrArray) split_path = NULL; GError *error = NULL; const char *filename = "bar"; const char *checksum = "01234567890123456789012345678901"; From 21ca60f9875c97a13848bb2298282b006e472b04 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 25 Oct 2016 13:11:12 -0400 Subject: [PATCH 05/41] otutil: Note that ot_log_structured takes a printf format This notably fixes compilation with CLang. Closes: #548 Approved by: jlebon --- src/libotutil/ot-log-utils.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libotutil/ot-log-utils.h b/src/libotutil/ot-log-utils.h index 5e0502ab..8d9786de 100644 --- a/src/libotutil/ot-log-utils.h +++ b/src/libotutil/ot-log-utils.h @@ -26,6 +26,6 @@ G_BEGIN_DECLS void ot_log_structured_print_id_v (const char *message_id, const char *format, - ...); + ...) G_GNUC_PRINTF(2, 3); G_END_DECLS From cbbfb5369fbbab9b0553db16636cfe76b12b1d75 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 25 Oct 2016 13:16:14 -0400 Subject: [PATCH 06/41] parse-datetime: Use labs() for long input value Fixes a compliation warning with CLang, I didn't study it to see whether this was a major issue or not. Closes: #548 Approved by: jlebon --- src/ostree/parse-datetime.y | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ostree/parse-datetime.y b/src/ostree/parse-datetime.y index d05bb55b..f5688d30 100644 --- a/src/ostree/parse-datetime.y +++ b/src/ostree/parse-datetime.y @@ -896,7 +896,7 @@ time_zone_hhmm (parser_control *pc, textint s, long int mm) /* If the absolute number of minutes is larger than 24 hours, arrange to reject it by incrementing pc->zones_seen. Thus, we allow only values in the range UTC-24:00 to UTC+24:00. */ - if (24 * 60 < abs (n_minutes)) + if (24 * 60 < labs (n_minutes)) pc->zones_seen++; return n_minutes; From 835d97d659b06d8e71d325fff458548731b9d78f Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 27 Oct 2016 13:43:39 -0400 Subject: [PATCH 07/41] deploy: Suppress unused variable warning for fscreatecon cleanup Fixes the clang build. Closes: #551 Approved by: jlebon --- src/libostree/ostree-sysroot-deploy.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index 41a9a8df..427e188b 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -2146,7 +2146,10 @@ ostree_sysroot_deploy_tree (OstreeSysroot *self, goto out; } +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wunused-variable" { ostree_cleanup_sepolicy_fscreatecon gpointer dummy = NULL; +#pragma GCC diagnostic pop /* Explicitly override the label for the origin file to ensure * it's system_conf_t. From cde792849695acacf6b69712e2a4f7a721807fec Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Thu, 27 Oct 2016 12:03:04 -0400 Subject: [PATCH 08/41] .redhat-ci.yml: add clang Clang has better detection for unused vars when using auto cleanup functions. We should eventually just fold this back into the first testsuite. But let's just turn it on for now, at least until it's satisfied with the whole codebase. Closes: #549 Approved by: cgwalters --- .redhat-ci.yml | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/.redhat-ci.yml b/.redhat-ci.yml index 1956cb30..da69f97a 100644 --- a/.redhat-ci.yml +++ b/.redhat-ci.yml @@ -23,3 +23,22 @@ timeout: 30m artifacts: - test-suite.log + +--- + +inherit: true + +packages: + - clang + +tests: + - sh autogen.sh + --prefix=/usr + --libdir=/usr/lib64 + --enable-installed-tests + --enable-gtk-doc + - make -j2 CC=clang CFLAGS='-Werror=unused-variable' + +context: Clang + +artifacts: From 7f2960db431496a73bd8e4b2c653a18f37e15151 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 25 Oct 2016 12:07:16 -0400 Subject: [PATCH 09/41] Define an initializer for GVariant{Builder,Dict} So we build warning-free on GLib (< 2.50, >= 2.50). This is a band aid until we hard-require >= 2.50. Closes: #547 Approved by: jlebon --- src/libostree/ostree-repo-static-delta-compilation.c | 6 +++--- src/libostree/ostree-repo.c | 6 +++--- src/libotutil/ot-variant-utils.c | 3 +-- src/libotutil/otutil.h | 7 +++++++ 4 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/libostree/ostree-repo-static-delta-compilation.c b/src/libostree/ostree-repo-static-delta-compilation.c index d73bdc83..fef204c7 100644 --- a/src/libostree/ostree-repo-static-delta-compilation.c +++ b/src/libostree/ostree-repo-static-delta-compilation.c @@ -1255,7 +1255,7 @@ ostree_repo_static_delta_generate (OstreeRepo *self, guint min_fallback_size; guint max_bsdiff_size; guint max_chunk_size; - g_auto(GVariantBuilder) metadata_builder = {{0,}}; + g_auto(GVariantBuilder) metadata_builder = OT_VARIANT_BUILDER_INITIALIZER; DeltaOpts delta_opts = DELTAOPT_FLAG_NONE; guint64 total_compressed_size = 0; guint64 total_uncompressed_size = 0; @@ -1391,8 +1391,8 @@ ostree_repo_static_delta_generate (OstreeRepo *self, g_autoptr(GVariant) delta_part_content = NULL; g_autoptr(GVariant) delta_part = NULL; g_autoptr(GVariant) delta_part_header = NULL; - g_auto(GVariantBuilder) mode_builder = {{0,}}; - g_auto(GVariantBuilder) xattr_builder = {{0,}}; + g_auto(GVariantBuilder) mode_builder = OT_VARIANT_BUILDER_INITIALIZER; + g_auto(GVariantBuilder) xattr_builder = OT_VARIANT_BUILDER_INITIALIZER; guint8 compression_type_char; g_variant_builder_init (&mode_builder, G_VARIANT_TYPE ("a(uuu)")); diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 305588a9..c897f819 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -3164,7 +3164,7 @@ ostree_repo_delete_object (OstreeRepo *self, if (tombstone_commits) { - g_auto(GVariantBuilder) builder = {{0,}}; + g_auto(GVariantBuilder) builder = OT_VARIANT_BUILDER_INITIALIZER; g_autoptr(GVariant) variant = NULL; g_variant_builder_init (&builder, G_VARIANT_TYPE ("a{sv}")); @@ -4653,7 +4653,7 @@ ostree_repo_regenerate_summary (OstreeRepo *self, g_autoptr(GVariant) summary = NULL; GList *ordered_keys = NULL; GList *iter = NULL; - g_auto(GVariantDict) additional_metadata_builder = {{0,}}; + g_auto(GVariantDict) additional_metadata_builder = OT_VARIANT_BUILDER_INITIALIZER; if (!ostree_repo_list_refs (self, NULL, &refs, cancellable, error)) goto out; @@ -4686,7 +4686,7 @@ ostree_repo_regenerate_summary (OstreeRepo *self, { guint i; g_autoptr(GPtrArray) delta_names = NULL; - g_auto(GVariantDict) deltas_builder = {{0,}}; + g_auto(GVariantDict) deltas_builder = OT_VARIANT_BUILDER_INITIALIZER; if (!ostree_repo_list_static_delta_names (self, &delta_names, cancellable, error)) goto out; diff --git a/src/libotutil/ot-variant-utils.c b/src/libotutil/ot-variant-utils.c index 315bbeb2..1c4c5efa 100644 --- a/src/libotutil/ot-variant-utils.c +++ b/src/libotutil/ot-variant-utils.c @@ -33,8 +33,7 @@ GVariant * ot_gvariant_new_empty_string_dict (void) { - g_auto(GVariantBuilder) builder = {{0,}}; - + g_auto(GVariantBuilder) builder = OT_VARIANT_BUILDER_INITIALIZER; g_variant_builder_init (&builder, G_VARIANT_TYPE ("a{sv}")); return g_variant_builder_end (&builder); } diff --git a/src/libotutil/otutil.h b/src/libotutil/otutil.h index ec516f65..c66d5634 100644 --- a/src/libotutil/otutil.h +++ b/src/libotutil/otutil.h @@ -26,6 +26,13 @@ #include /* Yeah...let's just do that here. */ #include +/* https://bugzilla.gnome.org/show_bug.cgi?id=766370 */ +#if !GLIB_CHECK_VERSION(2, 49, 3) +#define OT_VARIANT_BUILDER_INITIALIZER {{0,}} +#else +#define OT_VARIANT_BUILDER_INITIALIZER {{{0,}}} +#endif + #define ot_gobject_refz(o) (o ? g_object_ref (o) : o) #define ot_transfer_out_value(outp, srcp) G_STMT_START { \ From 033326055974c6537c90cd583af1b07248951045 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Thu, 27 Oct 2016 17:12:05 -0400 Subject: [PATCH 10/41] delta: return valid enum member If we can't figure out what endianness a delta is, we should just throw ENDIAN_INVALID. Resolves: #550 Closes: #553 Approved by: cgwalters --- src/libostree/ostree-repo-static-delta-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libostree/ostree-repo-static-delta-core.c b/src/libostree/ostree-repo-static-delta-core.c index 77e7939a..0b0d7067 100644 --- a/src/libostree/ostree-repo-static-delta-core.c +++ b/src/libostree/ostree-repo-static-delta-core.c @@ -764,7 +764,7 @@ _ostree_delta_get_endianness (GVariant *superblock, } } - return G_BYTE_ORDER; + return OSTREE_DELTA_ENDIAN_INVALID; } } From c4c8937b20902398e9720c39f25922df23036484 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Fri, 28 Oct 2016 14:44:09 +0200 Subject: [PATCH 11/41] static-delta: find a similar filename using what is before '.' or '-' Improve the heuristic to use only the part before the first '.' when looking for a similar file in the current directory. last versions of dracut generate reproducible initramfs files, but we still fallback to the full file download if there is any minimal change that causes a different checksum and file name. This change extends that case to deal better with similar files that have a different suffix. This is the difference generating a static delta from fedora-atomic/f24/x86_64/docker-host to fedora-atomic/f24/x86_64/testing/docker-host before the patch: fallback for 111ec866aa7ce3688407fa4a1ae7c9fca93dcee0b851fc9434c59ff947830cc7 (47.0 MB) fallback for c6a898265de22b02c89ea2f35d132628d0ee1c0a058052ed14fee5799c17904c (47.0 MB) fallback for fbce656249ece77260887ed873e445561b9d43bcb28a32e759c0b1bab89e7137 (6.6 MB) fallback for cfdb51457e47e0a0fe0bac38991a21279d2646ff2f019630c7b52a0cd3451397 (6.6 MB) part 0 n:1972 compressed:11239809 uncompressed:33747412 part 1 n:1079 compressed:9683681 uncompressed:55641397 part 2 n:1507 compressed:15050265 uncompressed:44448838 part 3 n:101 compressed:1865881 uncompressed:31896086 part 4 n:278 compressed:2452585 uncompressed:52811323 part 5 n:18 compressed:67621 uncompressed:100220 uncompressed=218645276 compressed=40359842 loose=545102 rollsum=49 objects, 2117254 bytes bsdiff=4067 objects after the patch: part 0 n:843 compressed:19844109 uncompressed:95443178 part 1 n:1223 compressed:11188609 uncompressed:33330401 part 2 n:990 compressed:15762905 uncompressed:61214132 part 3 n:1441 compressed:20614573 uncompressed:31534195 part 4 n:163 compressed:2734997 uncompressed:51356423 part 5 n:285 compressed:2480813 uncompressed:52902904 part 6 n:14 compressed:59125 uncompressed:75341 uncompressed=325856574 compressed=72685131 loose=533283 rollsum=51 objects, 57235332 bytes bsdiff=4073 objects Signed-off-by: Giuseppe Scrivano Closes: #554 Approved by: cgwalters --- ...e-repo-static-delta-compilation-analysis.c | 74 ++++++++++++------- 1 file changed, 49 insertions(+), 25 deletions(-) diff --git a/src/libostree/ostree-repo-static-delta-compilation-analysis.c b/src/libostree/ostree-repo-static-delta-compilation-analysis.c index ae5b8dec..c1990591 100644 --- a/src/libostree/ostree-repo-static-delta-compilation-analysis.c +++ b/src/libostree/ostree-repo-static-delta-compilation-analysis.c @@ -189,18 +189,30 @@ build_content_sizenames_filtered (OstreeRepo *repo, static gboolean string_array_nonempty_intersection (GPtrArray *a, - GPtrArray *b) + GPtrArray *b, + gboolean fuzzy) { guint i; for (i = 0; i < a->len; i++) { guint j; const char *a_str = a->pdata[i]; + const char *a_dot = strchr (a_str, '.'); for (j = 0; j < b->len; j++) { const char *b_str = b->pdata[j]; - if (strcmp (a_str, b_str) == 0) - return TRUE; + const char *b_dot = strchr (b_str, '.'); + /* When doing fuzzy comparison, just compare the part before the '.' if it exists. */ + if (fuzzy && a_dot && b_dot && b_dot - b_str && b_dot - b_str == a_dot - a_str) + { + if (strncmp (a_str, b_str, a_dot - a_str) == 0) + return TRUE; + } + else + { + if (strcmp (a_str, b_str) == 0) + return TRUE; + } } } return FALSE; @@ -258,6 +270,8 @@ _ostree_delta_compute_similar_objects (OstreeRepo *repo, upper = from_sizes->len; for (i = 0; i < to_sizes->len; i++) { + int fuzzy; + gboolean found = FALSE; OstreeDeltaContentSizeNames *to_sizenames = to_sizes->pdata[i]; const guint64 min_threshold = to_sizenames->size * (1.0-similarity_percent_threshold/100.0); @@ -268,31 +282,41 @@ _ostree_delta_compute_similar_objects (OstreeRepo *repo, if (to_sizenames->size == 0) continue; - for (j = lower; j < upper; j++) + for (fuzzy = 0; fuzzy < 2 && !found; fuzzy++) { - OstreeDeltaContentSizeNames *from_sizenames = from_sizes->pdata[j]; - - /* Don't build candidates for the empty object */ - if (from_sizenames->size == 0) - continue; - - if (from_sizenames->size < min_threshold) + for (j = lower; j < upper; j++) { - lower++; - continue; + OstreeDeltaContentSizeNames *from_sizenames = from_sizes->pdata[j]; + + /* Don't build candidates for the empty object */ + if (from_sizenames->size == 0) + { + continue; + } + + if (from_sizenames->size < min_threshold) + { + lower++; + continue; + } + + if (from_sizenames->size > max_threshold) + break; + + if (!string_array_nonempty_intersection (from_sizenames->basenames, + to_sizenames->basenames, + fuzzy == 1)) + { + continue; + } + + /* Only one candidate right now */ + g_hash_table_insert (ret_modified_regfile_content, + g_strdup (to_sizenames->checksum), + g_strdup (from_sizenames->checksum)); + found = TRUE; + break; } - - if (from_sizenames->size > max_threshold) - break; - - if (!string_array_nonempty_intersection (from_sizenames->basenames, to_sizenames->basenames)) - continue; - - /* Only one candidate right now */ - g_hash_table_insert (ret_modified_regfile_content, - g_strdup (to_sizenames->checksum), - g_strdup (from_sizenames->checksum)); - break; } } From 396563e7e912a9d3e34d727fcc814250bb8819b1 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 27 Oct 2016 13:50:46 -0400 Subject: [PATCH 12/41] libglnx: Bump to master (for -fsanitize fixes) Closes: #552 Approved by: jlebon --- libglnx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libglnx b/libglnx index 36396b49..abd37a47 160000 --- a/libglnx +++ b/libglnx @@ -1 +1 @@ -Subproject commit 36396b49ad6636c9959f3dfac5e04d41584b1a92 +Subproject commit abd37a4790f86f53bfb442e6d80e1710f50bff92 From 05dc77d7e5f31e210a7d93edf75e42eb66f1d26d Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 28 Oct 2016 09:32:02 -0400 Subject: [PATCH 13/41] remote-refs: Add NULL terminator to options array Caught by `-fsanitize=undefined`. Closes: #552 Approved by: jlebon --- src/ostree/ot-remote-builtin-refs.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ostree/ot-remote-builtin-refs.c b/src/ostree/ot-remote-builtin-refs.c index da3bcbb9..4317c45f 100644 --- a/src/ostree/ot-remote-builtin-refs.c +++ b/src/ostree/ot-remote-builtin-refs.c @@ -29,6 +29,7 @@ static char* opt_cache_dir; static GOptionEntry option_entries[] = { { "cache-dir", 0, 0, G_OPTION_ARG_STRING, &opt_cache_dir, "Use custom cache dir", NULL }, + { NULL } }; gboolean From a15dc7f1913e0aaceebbba329db13faf836f3745 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 27 Oct 2016 13:51:55 -0400 Subject: [PATCH 14/41] ci: Use -fsanitize=undefined by default It's fast enough to use for CI testing by default, and it can catch a lot of bad things. Closes: #552 Approved by: jlebon --- .redhat-ci.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.redhat-ci.yml b/.redhat-ci.yml index da69f97a..f2a0cfe9 100644 --- a/.redhat-ci.yml +++ b/.redhat-ci.yml @@ -6,12 +6,16 @@ branches: container: image: projectatomic/ostree-tester +packages: + - libubsan + tests: - sh autogen.sh --prefix=/usr --libdir=/usr/lib64 --enable-installed-tests --enable-gtk-doc + CFLAGS='-fsanitize=undefined' - make -j2 - make syntax-check - make check From 7091d288f7e1648314dc984da0c0abb5794834fb Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Sat, 8 Oct 2016 14:19:01 +0100 Subject: [PATCH 15/41] Force C.UTF-8 or C locale for tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Otherwise several tests fail, for example in this build done in a French locale by Debian's reproducible builds initiative, to check whether the resulting binaries are identical to what was produced in an English locale: (test-basic) # error: Cannot write to repository: Permission non accordée ... File 'error-message' doesn't match regexp 'Permission denied' (test-help) # Utilisation : # ostree [OPTION...] COMMAND ... File 'out' doesn't match regexp '[Uu]sage' (test-pull-metalink) # error: Erreur à la ligne 1, caractère 1 : Le document doit commencer avec un élément (par ex. ) ... File 'err.txt' doesn't match regexp 'Document must begin with an element' Signed-off-by: Simon McVittie Closes: #558 Approved by: cgwalters --- tests/libtest.sh | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/libtest.sh b/tests/libtest.sh index bc114bfa..d16aae70 100755 --- a/tests/libtest.sh +++ b/tests/libtest.sh @@ -35,6 +35,15 @@ assert_not_reached () { test_tmpdir=$(pwd) +# Some tests look for specific English strings. Use a UTF-8 version +# of the C (POSIX) locale if we have one, or fall back to POSIX +# (https://sourceware.org/glibc/wiki/Proposals/C.UTF-8) +if locale -a | grep C.UTF-8 >/dev/null; then + export LC_ALL=C.UTF-8 +else + export LC_ALL=C +fi + # Sanity check that we're in a tmpdir that has # just .testtmp (created by tap-driver for `make check`, # or nothing at all (as ginstest-runner does) From 4eb55a687d2f45701ffe511eaa72756903d749f6 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Fri, 7 Oct 2016 23:09:29 +0100 Subject: [PATCH 16/41] Distribute test scripts even if we wouldn't run them This fixes a "make dist" tarball produced on a minimal system and run on a non-minimal system. Automake knows that files that are only conditionally included in dist_whatever_WHATEVER are to be distributed, but it does not do the same for files that are only conditionally included in EXTRA_DIST, which is how glib-tap.mk's various variables like dist_test_scripts work. Signed-off-by: Simon McVittie Closes: #557 Approved by: cgwalters --- Makefile-tests.am | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/Makefile-tests.am b/Makefile-tests.am index 7d3f4011..3cfb1e02 100644 --- a/Makefile-tests.am +++ b/Makefile-tests.am @@ -94,11 +94,16 @@ dist_test_scripts = \ if BUILDOPT_FUSE dist_test_scripts += tests/test-rofiles-fuse.sh +else +EXTRA_DIST += tests/test-rofiles-fuse.sh endif # This one uses corrupt-repo-ref.js +js_tests = tests/test-corruption.sh if BUILDOPT_GJS dist_test_scripts += tests/test-corruption.sh +else +EXTRA_DIST += $(js_tests) endif dist_installed_test_data = tests/archive-test.sh \ @@ -133,11 +138,16 @@ dist_gpgvinsttest_DATA = $(addprefix tests/gpg-verify-data/, \ gpg.conf lgpl2 lgpl2.sig pubring.gpg secring.gpg trustdb.gpg) endif -if BUILDOPT_GJS -dist_installed_test_scripts = tests/test-core.js \ +js_installed_tests = \ + tests/test-core.js \ tests/test-sizes.js \ tests/test-sysroot.js \ $(NULL) + +if BUILDOPT_GJS +dist_installed_test_scripts = $(js_installed_tests) +else +EXTRA_DIST += $(js_installed_tests) endif test_ltlibraries = libreaddir-rand.la From a0e7d411c11c41f81321b463c53722e746334012 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Fri, 7 Oct 2016 23:12:29 +0100 Subject: [PATCH 17/41] Distribute valgrind suppressions in tarballs Signed-off-by: Simon McVittie Closes: #557 Approved by: cgwalters --- Makefile-tests.am | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Makefile-tests.am b/Makefile-tests.am index 3cfb1e02..55381234 100644 --- a/Makefile-tests.am +++ b/Makefile-tests.am @@ -22,6 +22,8 @@ include $(top_srcdir)/buildutil/glib-tap.mk EXTRA_DIST += \ buildutil/tap-driver.sh \ buildutil/tap-test \ + tests/glib.supp \ + tests/ostree.supp \ $(NULL) # We should probably consider flipping the default for DEBUG. Also, From e757f736e7f363f1fd03cb6b198f96888d31ad18 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Sat, 29 Oct 2016 21:31:18 +0100 Subject: [PATCH 18/41] _ostree_kernel_args_replace_take: don't leak when replacing If !existed, then we add arg to kargs->order, where it will be freed by that array's free-function. However, if the kernel argument did already exist, we have to either free arg ourselves (and make sure the old key is what appears in the hash table), or do a linear search on kargs->order to replace the old key with the new. Leak found by valgrind memcheck. Signed-off-by: Simon McVittie Closes: #559 Approved by: cgwalters --- src/libostree/ostree-kernel-args.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/libostree/ostree-kernel-args.c b/src/libostree/ostree-kernel-args.c index 4c9ff147..ec189fc1 100644 --- a/src/libostree/ostree-kernel-args.c +++ b/src/libostree/ostree-kernel-args.c @@ -87,12 +87,21 @@ _ostree_kernel_args_replace_take (OstreeKernelArgs *kargs, gboolean existed; GPtrArray *values = g_ptr_array_new_with_free_func (g_free); const char *value = split_keyeq (arg); + gpointer old_key; - existed = g_hash_table_remove (kargs->table, arg); - if (!existed) - g_ptr_array_add (kargs->order, arg); g_ptr_array_add (values, g_strdup (value)); - g_hash_table_replace (kargs->table, arg, values); + existed = g_hash_table_lookup_extended (kargs->table, arg, &old_key, NULL); + + if (existed) + { + g_hash_table_replace (kargs->table, old_key, values); + g_free (arg); + } + else + { + g_ptr_array_add (kargs->order, arg); + g_hash_table_replace (kargs->table, arg, values); + } } void From 22ed96d23cc1ffadb30e119a0677036348707f52 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Sun, 30 Oct 2016 14:11:23 +0000 Subject: [PATCH 19/41] ot_admin_builtin_set_origin: don't leak options GVariant Found by valgrind memcheck. Signed-off-by: Simon McVittie Closes: #559 Approved by: cgwalters --- src/ostree/ot-admin-builtin-set-origin.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/ostree/ot-admin-builtin-set-origin.c b/src/ostree/ot-admin-builtin-set-origin.c index 8b32eb79..0e79ab5e 100644 --- a/src/ostree/ot-admin-builtin-set-origin.c +++ b/src/ostree/ot-admin-builtin-set-origin.c @@ -96,6 +96,7 @@ ot_admin_builtin_set_origin (int argc, char **argv, GCancellable *cancellable, G { char **iter; g_autoptr(GVariantBuilder) optbuilder = g_variant_builder_new (G_VARIANT_TYPE ("a{sv}")); + g_autoptr(GVariant) options = NULL; for (iter = opt_set; iter && *iter; iter++) { @@ -109,11 +110,13 @@ ot_admin_builtin_set_origin (int argc, char **argv, GCancellable *cancellable, G g_variant_builder_add (optbuilder, "{s@v}", subkey, g_variant_new_variant (g_variant_new_string (subvalue))); } - + + options = g_variant_ref_sink (g_variant_builder_end (optbuilder)); + if (!ostree_repo_remote_change (repo, NULL, OSTREE_REPO_REMOTE_CHANGE_ADD_IF_NOT_EXISTS, remotename, url, - g_variant_builder_end (optbuilder), + options, cancellable, error)) goto out; } From 213d5013cec6abe61c3500c1bffceff1a42d5509 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Sun, 30 Oct 2016 14:12:02 +0000 Subject: [PATCH 20/41] ostree_builtin_pull: consistently set free-function on refs_to_fetch We are relying on the GPtrArray to free its contents, but we only give it a free-function on one code path. Found by valgrind memcheck. Signed-off-by: Simon McVittie Closes: #559 Approved by: cgwalters --- src/ostree/ot-builtin-pull.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ostree/ot-builtin-pull.c b/src/ostree/ot-builtin-pull.c index 9f48c2e0..52a55375 100644 --- a/src/ostree/ot-builtin-pull.c +++ b/src/ostree/ot-builtin-pull.c @@ -198,7 +198,7 @@ ostree_builtin_pull (int argc, char **argv, GCancellable *cancellable, GError ** else { char *ref_to_fetch; - refs_to_fetch = g_ptr_array_new (); + refs_to_fetch = g_ptr_array_new_with_free_func (g_free); if (!ostree_parse_refspec (argv[1], &remote, &ref_to_fetch, error)) goto out; /* Transfer ownership */ From 4c32344b8e71e40034fcdbf3053f46c290fac785 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Sun, 30 Oct 2016 14:14:09 +0000 Subject: [PATCH 21/41] ostree_admin_option_context_parse: explicitly clean up when exiting early The cleanup attribute doesn't clean up before calling a noreturn function like exit(). Explicitly clean up the pointer variables (but don't assume that a simple g_object_unref() would be OK either, in case the behaviour of the cleanup attribute changes). This isn't a real leak since we're about to exit anyway, but if we don't fix it then valgrind memcheck will make the tests fail. Signed-off-by: Simon McVittie Closes: #559 Approved by: cgwalters --- src/ostree/ot-main.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/ostree/ot-main.c b/src/ostree/ot-main.c index 18c13239..5080e8c6 100644 --- a/src/ostree/ot-main.c +++ b/src/ostree/ot-main.c @@ -366,6 +366,13 @@ ostree_admin_option_context_parse (GOptionContext *context, g_print ("%s\n", deployment_path); + /* The g_autoptr, g_autofree etc. don't happen when we explicitly + * exit, making valgrind complain about leaks */ + g_clear_object (&sysroot); + g_clear_object (&sysroot_path); + g_clear_object (&deployment_file); + g_clear_pointer (&deployments, g_ptr_array_unref); + g_clear_pointer (&deployment_path, g_free); exit (EXIT_SUCCESS); } From ff28ac4a3014a5e76577cd68df84e0290cf20f0d Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Sat, 29 Oct 2016 18:37:32 +0100 Subject: [PATCH 22/41] ostree_sysroot_upgrader_finalize: free new_revision Leak found with valgrind memcheck. Signed-off-by: Simon McVittie Closes: #556 Approved by: cgwalters --- src/libostree/ostree-sysroot-upgrader.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libostree/ostree-sysroot-upgrader.c b/src/libostree/ostree-sysroot-upgrader.c index 22710bda..b0061c87 100644 --- a/src/libostree/ostree-sysroot-upgrader.c +++ b/src/libostree/ostree-sysroot-upgrader.c @@ -188,6 +188,7 @@ ostree_sysroot_upgrader_finalize (GObject *object) g_free (self->origin_remote); g_free (self->origin_ref); g_free (self->override_csum); + g_free (self->new_revision); G_OBJECT_CLASS (ostree_sysroot_upgrader_parent_class)->finalize (object); } From c8a6b037ef3eb457c25f2a6e891fc0ef59b6b7d9 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Sat, 29 Oct 2016 18:37:09 +0100 Subject: [PATCH 23/41] _ostree_sysroot_write_deployments_internal: stop leaking hash table It appears the result of assign_bootserials() is never actually used, but I haven't changed it to return void right now. Leak found with valgrind memcheck. Signed-off-by: Simon McVittie Closes: #556 Approved by: cgwalters --- src/libostree/ostree-sysroot-deploy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index 427e188b..cb5a4615 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -1747,7 +1747,7 @@ _ostree_sysroot_write_deployments_internal (OstreeSysroot *self, /* Assign a bootserial to each new deployment. */ - assign_bootserials (new_deployments); + g_hash_table_unref (assign_bootserials (new_deployments)); /* Determine whether or not we need to touch the bootloader * configuration. If we have an equal number of deployments with From 47397097423578a43ce827e663e9e008887eaf54 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Sat, 29 Oct 2016 18:36:19 +0100 Subject: [PATCH 24/41] keyfile_set_from_vardict: free the string array g_variant_get_strv is (transfer container): the caller is expected to free the array, but not the individual strings. Leak found with valgrind memcheck. Signed-off-by: Simon McVittie Closes: #556 Approved by: cgwalters --- src/libostree/ostree-repo.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index c897f819..2e35faf8 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -936,7 +936,7 @@ keyfile_set_from_vardict (GKeyFile *keyfile, else if (g_variant_is_of_type (child, G_VARIANT_TYPE_STRING_ARRAY)) { gsize len; - const char *const*strv_child = g_variant_get_strv (child, &len); + g_autofree const gchar **strv_child = g_variant_get_strv (child, &len); g_key_file_set_string_list (keyfile, section, key, strv_child, len); } else From 1fc2a1202e595a8eb3b2ec8ab2d78c89a1908166 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Thu, 27 Oct 2016 12:32:10 +0100 Subject: [PATCH 25/41] ostree_repo_pull_with_options: clear dirs array Leak found with valgrind memcheck. Signed-off-by: Simon McVittie Closes: #556 Approved by: cgwalters --- src/libostree/ostree-repo-pull.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 9d361d5f..359660bb 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -3123,6 +3123,7 @@ ostree_repo_pull_with_options (OstreeRepo *self, g_clear_pointer (&pull_data->requested_content, (GDestroyNotify) g_hash_table_unref); g_clear_pointer (&pull_data->requested_metadata, (GDestroyNotify) g_hash_table_unref); g_clear_pointer (&pull_data->idle_src, (GDestroyNotify) g_source_destroy); + g_clear_pointer (&pull_data->dirs, (GDestroyNotify) g_ptr_array_unref); g_clear_pointer (&remote_config, (GDestroyNotify) g_key_file_unref); return ret; } From 53f1fabfbe679f123c6cf66053e894a479c57314 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Thu, 27 Oct 2016 12:31:52 +0100 Subject: [PATCH 26/41] ot_remote_builtin_show_url: autofree context Leak found with valgrind memcheck. Signed-off-by: Simon McVittie Closes: #556 Approved by: cgwalters --- src/ostree/ot-remote-builtin-show-url.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ostree/ot-remote-builtin-show-url.c b/src/ostree/ot-remote-builtin-show-url.c index a4b447c8..401dfb0b 100644 --- a/src/ostree/ot-remote-builtin-show-url.c +++ b/src/ostree/ot-remote-builtin-show-url.c @@ -32,7 +32,7 @@ static GOptionEntry option_entries[] = { gboolean ot_remote_builtin_show_url (int argc, char **argv, GCancellable *cancellable, GError **error) { - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeRepo *repo = NULL; const char *remote_name; g_autofree char *remote_url = NULL; From 24af123c533e0ba5fdb68772315436b5bb4e6720 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Sat, 29 Oct 2016 18:47:11 +0100 Subject: [PATCH 27/41] Fix some leaks of floating GVariants ostree_repo_pull_with_options() and ostree_repo_remote_change() don't sink floating GVariant arguments, and doing so now would be an ABI change; so don't rely on them to do so. Leak found with valgrind memcheck. Signed-off-by: Simon McVittie Closes: #556 Approved by: cgwalters --- src/ostree/ot-builtin-pull.c | 5 ++++- src/ostree/ot-remote-builtin-add.c | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/ostree/ot-builtin-pull.c b/src/ostree/ot-builtin-pull.c index 52a55375..7981f18a 100644 --- a/src/ostree/ot-builtin-pull.c +++ b/src/ostree/ot-builtin-pull.c @@ -208,6 +208,7 @@ ostree_builtin_pull (int argc, char **argv, GCancellable *cancellable, GError ** { GVariantBuilder builder; + g_autoptr(GVariant) options = NULL; g_auto(GLnxConsoleRef) console = { 0, }; g_variant_builder_init (&builder, G_VARIANT_TYPE ("a{sv}")); @@ -265,7 +266,9 @@ ostree_builtin_pull (int argc, char **argv, GCancellable *cancellable, GError ** &console); } - if (!ostree_repo_pull_with_options (repo, remote, g_variant_builder_end (&builder), + options = g_variant_ref_sink (g_variant_builder_end (&builder)); + + if (!ostree_repo_pull_with_options (repo, remote, options, progress, cancellable, error)) goto out; diff --git a/src/ostree/ot-remote-builtin-add.c b/src/ostree/ot-remote-builtin-add.c index 6e83b2af..9d275cb5 100644 --- a/src/ostree/ot-remote-builtin-add.c +++ b/src/ostree/ot-remote-builtin-add.c @@ -50,6 +50,7 @@ ot_remote_builtin_add (int argc, char **argv, GCancellable *cancellable, GError const char *remote_url; char **iter; g_autoptr(GVariantBuilder) optbuilder = NULL; + g_autoptr(GVariant) options = NULL; gboolean ret = FALSE; context = g_option_context_new ("NAME [metalink=|mirrorlist=]URL [BRANCH...] - Add a remote repository"); @@ -109,11 +110,13 @@ ot_remote_builtin_add (int argc, char **argv, GCancellable *cancellable, GError "gpg-verify", g_variant_new_variant (g_variant_new_boolean (FALSE))); + options = g_variant_ref_sink (g_variant_builder_end (optbuilder)); + if (!ostree_repo_remote_change (repo, NULL, opt_if_not_exists ? OSTREE_REPO_REMOTE_CHANGE_ADD_IF_NOT_EXISTS : OSTREE_REPO_REMOTE_CHANGE_ADD, remote_name, remote_url, - g_variant_builder_end (optbuilder), + options, cancellable, error)) goto out; From 8ae03d6497db35b8d34be590d770e6d92beec9c6 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Sun, 30 Oct 2016 13:52:51 +0000 Subject: [PATCH 28/41] load_metadata_internal: don't leak GBytes Found by valgrind memcheck. g_variant_new_from_bytes takes a ref to the bytes, so we need to release the original ref. Signed-off-by: Simon McVittie Closes: #556 Approved by: cgwalters --- src/libostree/ostree-repo.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 2e35faf8..1f866bbe 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -2593,7 +2593,7 @@ load_metadata_internal (OstreeRepo *self, } else { - GBytes *data = glnx_fd_readall_bytes (fd, cancellable, error); + g_autoptr(GBytes) data = glnx_fd_readall_bytes (fd, cancellable, error); if (!data) goto out; ret_variant = g_variant_new_from_bytes (ostree_metadata_variant_type (objtype), From f0e493bf2992d752ec3cf517542e60d9ea376be4 Mon Sep 17 00:00:00 2001 From: Sjoerd Simons Date: Sun, 30 Oct 2016 21:06:27 +0100 Subject: [PATCH 29/41] Filter bootloader supplied kernel cmdline options Various bootloader add kernel commandline options dynamically, filter these out when grabbing boot options from /proc/cmdline. Specifically grub adds BOOT_IMAGE and systemd-boot adds initrd. Closes: #560 Approved by: cgwalters --- src/libostree/ostree-kernel-args.c | 43 +++++++++++++++++++++++--- src/libostree/ostree-kernel-args.h | 3 ++ tests/test-admin-deploy-karg.sh | 2 ++ tests/test-admin-instutil-set-kargs.sh | 2 ++ 4 files changed, 46 insertions(+), 4 deletions(-) diff --git a/src/libostree/ostree-kernel-args.c b/src/libostree/ostree-kernel-args.c index ec189fc1..22b5caa7 100644 --- a/src/libostree/ostree-kernel-args.c +++ b/src/libostree/ostree-kernel-args.c @@ -53,6 +53,23 @@ split_keyeq (char *arg) } } +static gboolean +_arg_has_prefix (const char *arg, + char **prefixes) +{ + char **strviter; + + for (strviter = prefixes; strviter && *strviter; strviter++) + { + const char *prefix = *strviter; + + if (g_str_has_prefix (arg, prefix)) + return TRUE; + } + + return FALSE; +} + OstreeKernelArgs * _ostree_kernel_args_new (void) { @@ -154,18 +171,28 @@ _ostree_kernel_args_replace_argv (OstreeKernelArgs *kargs, } void -_ostree_kernel_args_append_argv (OstreeKernelArgs *kargs, - char **argv) +_ostree_kernel_args_append_argv_filtered (OstreeKernelArgs *kargs, + char **argv, + char **prefixes) { char **strviter; for (strviter = argv; strviter && *strviter; strviter++) { const char *arg = *strviter; - _ostree_kernel_args_append (kargs, arg); + + if (!_arg_has_prefix (arg, prefixes)) + _ostree_kernel_args_append (kargs, arg); } } +void +_ostree_kernel_args_append_argv (OstreeKernelArgs *kargs, + char **argv) +{ + _ostree_kernel_args_append_argv_filtered (kargs, argv, NULL); +} + gboolean _ostree_kernel_args_append_proc_cmdline (OstreeKernelArgs *kargs, GCancellable *cancellable, @@ -175,6 +202,13 @@ _ostree_kernel_args_append_proc_cmdline (OstreeKernelArgs *kargs, g_autofree char *proc_cmdline = NULL; gsize proc_cmdline_len = 0; g_auto(GStrv) proc_cmdline_args = NULL; + /* When updating the filter list don't forget to update the list in the tests + * e.g. tests/test-admin-deploy-karg.sh and + * tests/test-admin-instutil-set-kargs.sh + */ + char *filtered_prefixes[] = { "BOOT_IMAGE=", /* GRUB 2 */ + "initrd=", /* sd-boot */ + NULL }; if (!g_file_load_contents (proc_cmdline_path, cancellable, &proc_cmdline, &proc_cmdline_len, @@ -184,7 +218,8 @@ _ostree_kernel_args_append_proc_cmdline (OstreeKernelArgs *kargs, g_strchomp (proc_cmdline); proc_cmdline_args = g_strsplit (proc_cmdline, " ", -1); - _ostree_kernel_args_append_argv (kargs, proc_cmdline_args); + _ostree_kernel_args_append_argv_filtered (kargs, proc_cmdline_args, + filtered_prefixes); return TRUE; } diff --git a/src/libostree/ostree-kernel-args.h b/src/libostree/ostree-kernel-args.h index 18710d78..ceaa1ca7 100644 --- a/src/libostree/ostree-kernel-args.h +++ b/src/libostree/ostree-kernel-args.h @@ -39,6 +39,9 @@ void _ostree_kernel_args_append (OstreeKernelArgs *kargs, const char *key); void _ostree_kernel_args_append_argv (OstreeKernelArgs *kargs, char **argv); +void _ostree_kernel_args_append_argv_filtered (OstreeKernelArgs *kargs, + char **argv, + char **prefixes); gboolean _ostree_kernel_args_append_proc_cmdline (OstreeKernelArgs *kargs, GCancellable *cancellable, diff --git a/tests/test-admin-deploy-karg.sh b/tests/test-admin-deploy-karg.sh index b7305f4c..643aef78 100755 --- a/tests/test-admin-deploy-karg.sh +++ b/tests/test-admin-deploy-karg.sh @@ -46,6 +46,8 @@ ${CMD_PREFIX} ostree admin deploy --karg-proc-cmdline --os=testos testos:testos/ for arg in $(cat /proc/cmdline); do case "$arg" in ostree=*) # Skip ostree arg that gets stripped out + ;; + initrd=*|BOOT_IMAGE=*) # Skip options set by bootloader that gets filtered out ;; *) assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf "options.*$arg" ;; diff --git a/tests/test-admin-instutil-set-kargs.sh b/tests/test-admin-instutil-set-kargs.sh index 40f4b746..132c9336 100755 --- a/tests/test-admin-instutil-set-kargs.sh +++ b/tests/test-admin-instutil-set-kargs.sh @@ -58,6 +58,8 @@ for arg in $(cat /proc/cmdline); do case "$arg" in ostree=*) # Skip ostree arg that gets stripped out ;; + initrd=*|BOOT_IMAGE=*) # Skip options set by bootloader that gets filtered out + ;; *) assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf "options.*$arg" ;; esac From 730f7238699b932c90423a63c6720a12643a76b2 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 1 Nov 2016 14:17:57 -0400 Subject: [PATCH 30/41] repo: Don't put remote refs in the summary file I was doing a chain of mirroring like A -> B -> C And repo B had A as a remote. When I added B as a remote to C, the summary file of B had a ref upstream:foo/bar/baz, which caused all pulls from B to C to fail, since the summary file is only expected to have refs, not refspecs. Closes: https://github.com/ostreedev/ostree/issues/561 Closes: #565 Approved by: jlebon --- src/libostree/ostree-repo.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 1f866bbe..ad629421 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -4668,10 +4668,18 @@ ostree_repo_regenerate_summary (OstreeRepo *self, { const char *ref = iter->data; const char *commit = g_hash_table_lookup (refs, ref); + g_autofree char *remotename = NULL; g_autoptr(GVariant) commit_obj = NULL; g_assert (commit); + if (!ostree_parse_refspec (ref, &remotename, NULL, NULL)) + g_assert_not_reached (); + + /* Don't put remote refs in the summary */ + if (remotename != NULL) + continue; + if (!ostree_repo_load_variant (self, OSTREE_OBJECT_TYPE_COMMIT, commit, &commit_obj, error)) goto out; From 2139f0e437038e05a562c9a566bb4903d3293c63 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 1 Nov 2016 16:39:58 -0400 Subject: [PATCH 31/41] pull: Don't do deltas with --commit-metadata-only We should just download the commit objects directly, as it's obviously a lot more efficient than deltas. I had to generate a summary file in more places in the tests, since once created, it needs to be updated. Closes: https://github.com/ostreedev/ostree/issues/528 Closes: #566 Approved by: jlebon --- src/libostree/ostree-repo-pull.c | 2 +- tests/pull-test.sh | 19 ++++++++++++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 359660bb..1f57b133 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -2926,7 +2926,7 @@ ostree_repo_pull_with_options (OstreeRepo *self, &from_revision, error)) goto out; - if (!disable_static_deltas && !mirroring_into_archive && + if (!(disable_static_deltas || mirroring_into_archive || pull_data->is_commit_only) && (from_revision == NULL || g_strcmp0 (from_revision, to_revision) != 0)) { if (!request_static_delta_superblock_sync (pull_data, from_revision, to_revision, diff --git a/tests/pull-test.sh b/tests/pull-test.sh index b050e111..408d0539 100755 --- a/tests/pull-test.sh +++ b/tests/pull-test.sh @@ -35,7 +35,7 @@ function verify_initial_contents() { assert_file_has_content baz/cow '^moo$' } -echo "1..12" +echo "1..13" # Try both syntaxes repo_init @@ -63,11 +63,27 @@ $OSTREE --repo=ostree-srv/gnomerepo checkout main checkout-origin-main echo moomoo > checkout-origin-main/baz/cow ${CMD_PREFIX} ostree --repo=ostree-srv/gnomerepo commit -b main -s "" --tree=dir=checkout-origin-main ${CMD_PREFIX} ostree --repo=ostree-srv/gnomerepo static-delta generate main +${CMD_PREFIX} ostree --repo=ostree-srv/gnomerepo summary -u ${CMD_PREFIX} ostree --repo=ostree-srv/gnomerepo fsck ${CMD_PREFIX} ostree --repo=mirrorrepo pull --mirror origin main ${CMD_PREFIX} ostree --repo=mirrorrepo fsck echo "ok pull mirror (should not apply deltas)" +cd ${test_tmpdir} +rm mirrorrepo/refs/remotes/* -rf +${CMD_PREFIX} ostree --repo=mirrorrepo prune --refs-only +${CMD_PREFIX} ostree --repo=mirrorrepo pull origin main +rm checkout-origin-main -rf +$OSTREE --repo=ostree-srv/gnomerepo checkout main checkout-origin-main +echo yetmorecontent > checkout-origin-main/baz/cowtest +${CMD_PREFIX} ostree --repo=ostree-srv/gnomerepo commit -b main -s "" --tree=dir=checkout-origin-main +rev=$(${CMD_PREFIX} ostree --repo=ostree-srv/gnomerepo rev-parse main) +${CMD_PREFIX} ostree --repo=ostree-srv/gnomerepo static-delta generate main +${CMD_PREFIX} ostree --repo=ostree-srv/gnomerepo summary -u +${CMD_PREFIX} ostree --repo=mirrorrepo pull --commit-metadata-only origin main +assert_has_file mirrorrepo/state/${rev}.commitpartial +echo "ok pull commit metadata only (should not apply deltas)" + cd ${test_tmpdir} mkdir mirrorrepo-local ${CMD_PREFIX} ostree --repo=mirrorrepo-local init --mode=archive-z2 @@ -79,6 +95,7 @@ echo "ok pull local mirror" cd ${test_tmpdir} ${CMD_PREFIX} ostree --repo=ostree-srv/gnomerepo commit -b main -s "Metadata string" --add-detached-metadata-string=SIGNATURE=HANCOCK --tree=ref=main +${CMD_PREFIX} ostree --repo=ostree-srv/gnomerepo summary -u ${CMD_PREFIX} ostree --repo=repo pull origin main ${CMD_PREFIX} ostree --repo=repo fsck $OSTREE show --print-detached-metadata-key=SIGNATURE main > main-meta From 96356aa1927b68ff6f887cd93c6dd67a18fc3063 Mon Sep 17 00:00:00 2001 From: Sjoerd Simons Date: Mon, 17 Oct 2016 21:39:38 +0200 Subject: [PATCH 32/41] pull: Add per-remote cookie jar Optionally read cookie jars for a remote to be used when downloading data. This can be used for private repositories which require specific cookies to be present, e.g. repositories hosted on Amazon cloudfront using signed cookies. Closes: #531 Approved by: cgwalters --- src/libostree/ostree-fetcher.c | 27 +++++++++++++++++++++++++++ src/libostree/ostree-fetcher.h | 3 +++ src/libostree/ostree-repo-pull.c | 13 +++++++++++++ 3 files changed, 43 insertions(+) diff --git a/src/libostree/ostree-fetcher.c b/src/libostree/ostree-fetcher.c index 21526813..a4d46016 100644 --- a/src/libostree/ostree-fetcher.c +++ b/src/libostree/ostree-fetcher.c @@ -326,6 +326,16 @@ session_thread_set_proxy_cb (ThreadClosure *thread_closure, } } +static void +session_thread_set_cookie_jar_cb (ThreadClosure *thread_closure, + gpointer data) +{ + SoupCookieJar *jar = data; + + soup_session_add_feature (thread_closure->session, + SOUP_SESSION_FEATURE (jar)); +} + #ifdef HAVE_LIBSOUP_CLIENT_CERTS static void session_thread_set_tls_interaction_cb (ThreadClosure *thread_closure, @@ -746,6 +756,23 @@ _ostree_fetcher_set_proxy (OstreeFetcher *self, } } +void +_ostree_fetcher_set_cookie_jar (OstreeFetcher *self, + const char *jar_path) +{ + SoupCookieJar *jar; + + g_return_if_fail (OSTREE_IS_FETCHER (self)); + g_return_if_fail (jar_path != NULL); + + jar = soup_cookie_jar_text_new (jar_path, TRUE); + + session_thread_idle_add (self->thread_closure, + session_thread_set_cookie_jar_cb, + jar, /* takes ownership */ + (GDestroyNotify) g_object_unref); +} + void _ostree_fetcher_set_client_cert (OstreeFetcher *self, GTlsCertificate *cert) diff --git a/src/libostree/ostree-fetcher.h b/src/libostree/ostree-fetcher.h index 8cceca51..ae20edaa 100644 --- a/src/libostree/ostree-fetcher.h +++ b/src/libostree/ostree-fetcher.h @@ -59,6 +59,9 @@ OstreeFetcher *_ostree_fetcher_new (int tmpdir_dfd, int _ostree_fetcher_get_dfd (OstreeFetcher *fetcher); +void _ostree_fetcher_set_cookie_jar (OstreeFetcher *self, + const char *jar_path); + void _ostree_fetcher_set_proxy (OstreeFetcher *fetcher, const char *proxy); diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 1f57b133..8facf8cb 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -1964,6 +1964,19 @@ _ostree_repo_remote_new_fetcher (OstreeRepo *self, _ostree_fetcher_set_proxy (fetcher, http_proxy); } + { + g_autofree char *jar_path = NULL; + g_autofree char *cookie_file = g_strdup_printf ("%s.cookies.txt", + remote_name); + + jar_path = g_build_filename (g_file_get_path (self->repodir), cookie_file, + NULL); + + if (g_file_test(jar_path, G_FILE_TEST_IS_REGULAR)) + _ostree_fetcher_set_cookie_jar (fetcher, jar_path); + + } + success = TRUE; out: From 6b467b9dbcb71759e70c79a246dc5e9a80e1b076 Mon Sep 17 00:00:00 2001 From: Sjoerd Simons Date: Mon, 17 Oct 2016 22:14:14 +0200 Subject: [PATCH 33/41] remote: Add command to list cookies Closes: #531 Approved by: cgwalters --- Makefile-ostree.am | 1 + src/ostree/ot-builtin-remote.c | 1 + src/ostree/ot-remote-builtin-list-cookies.c | 86 +++++++++++++++++++++ src/ostree/ot-remote-builtins.h | 1 + 4 files changed, 89 insertions(+) create mode 100644 src/ostree/ot-remote-builtin-list-cookies.c diff --git a/Makefile-ostree.am b/Makefile-ostree.am index 33875dc4..d2641cb4 100644 --- a/Makefile-ostree.am +++ b/Makefile-ostree.am @@ -83,6 +83,7 @@ ostree_SOURCES += \ src/ostree/ot-remote-builtin-delete.c \ src/ostree/ot-remote-builtin-gpg-import.c \ src/ostree/ot-remote-builtin-list.c \ + src/ostree/ot-remote-builtin-list-cookies.c \ src/ostree/ot-remote-builtin-show-url.c \ src/ostree/ot-remote-builtin-refs.c \ src/ostree/ot-remote-builtin-summary.c \ diff --git a/src/ostree/ot-builtin-remote.c b/src/ostree/ot-builtin-remote.c index 7809dd67..9ac03173 100644 --- a/src/ostree/ot-builtin-remote.c +++ b/src/ostree/ot-builtin-remote.c @@ -36,6 +36,7 @@ static OstreeRemoteCommand remote_subcommands[] = { { "delete", ot_remote_builtin_delete }, { "show-url", ot_remote_builtin_show_url }, { "list", ot_remote_builtin_list }, + { "list-cookies", ot_remote_builtin_list_cookies }, { "gpg-import", ot_remote_builtin_gpg_import }, { "refs", ot_remote_builtin_refs }, { "summary", ot_remote_builtin_summary }, diff --git a/src/ostree/ot-remote-builtin-list-cookies.c b/src/ostree/ot-remote-builtin-list-cookies.c new file mode 100644 index 00000000..5ccc5d8d --- /dev/null +++ b/src/ostree/ot-remote-builtin-list-cookies.c @@ -0,0 +1,86 @@ +/* -*- mode: C; c-file-style: "gnu"; indent-tabs-mode: nil; -*- + * + * Copyright (C) 2015 Red Hat, Inc. + * Copyright (C) 2016 Sjoerd Simons + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the + * Free Software Foundation, Inc., 59 Temple Place - Suite 330, + * Boston, MA 02111-1307, USA. + */ + +#include "config.h" + +#include + +#include "otutil.h" + +#include "ot-main.h" +#include "ot-remote-builtins.h" +#include "ostree-repo-private.h" + + +static GOptionEntry option_entries[] = { + { NULL } +}; + +gboolean +ot_remote_builtin_list_cookies (int argc, char **argv, GCancellable *cancellable, GError **error) +{ + GOptionContext *context; + glnx_unref_object OstreeRepo *repo = NULL; + const char *remote_name; + g_autofree char *jar_path = NULL; + g_autofree char *cookie_file = NULL; + glnx_unref_object SoupCookieJar *jar = NULL; + GSList *cookies; + + context = g_option_context_new ("NAME - Show remote repository cookies"); + + if (!ostree_option_context_parse (context, option_entries, &argc, &argv, + OSTREE_BUILTIN_FLAG_NONE, &repo, cancellable, error)) + return FALSE; + + if (argc < 2) + { + ot_util_usage_error (context, "NAME must be specified", error); + return FALSE; + } + + remote_name = argv[1]; + + cookie_file = g_strdup_printf ("%s.cookies.txt", remote_name); + jar_path = g_build_filename (g_file_get_path (repo->repodir), cookie_file, NULL); + + jar = soup_cookie_jar_text_new (jar_path, TRUE); + cookies = soup_cookie_jar_all_cookies (jar); + + while (cookies != NULL) + { + SoupCookie *cookie = cookies->data; + SoupDate *expiry = soup_cookie_get_expires (cookie); + + g_print ("--\n"); + g_print ("Domain: %s\n", soup_cookie_get_domain (cookie)); + g_print ("Path: %s\n", soup_cookie_get_path (cookie)); + g_print ("Name: %s\n", soup_cookie_get_name (cookie)); + g_print ("Secure: %s\n", soup_cookie_get_secure (cookie) ? "yes" : "no"); + g_print ("Expires: %s\n", soup_date_to_string (expiry, SOUP_DATE_COOKIE)); + g_print ("Value: %s\n", soup_cookie_get_value (cookie)); + + soup_cookie_free (cookie); + cookies = g_slist_delete_link (cookies, cookies); + } + + return TRUE; +} diff --git a/src/ostree/ot-remote-builtins.h b/src/ostree/ot-remote-builtins.h index 0e65092f..a725752c 100644 --- a/src/ostree/ot-remote-builtins.h +++ b/src/ostree/ot-remote-builtins.h @@ -28,6 +28,7 @@ gboolean ot_remote_builtin_add (int argc, char **argv, GCancellable *cancellable gboolean ot_remote_builtin_delete (int argc, char **argv, GCancellable *cancellable, GError **error); gboolean ot_remote_builtin_gpg_import (int argc, char **argv, GCancellable *cancellable, GError **error); gboolean ot_remote_builtin_list (int argc, char **argv, GCancellable *cancellable, GError **error); +gboolean ot_remote_builtin_list_cookies (int argc, char **argv, GCancellable *cancellable, GError **error); gboolean ot_remote_builtin_show_url (int argc, char **argv, GCancellable *cancellable, GError **error); gboolean ot_remote_builtin_refs (int argc, char **argv, GCancellable *cancellable, GError **error); gboolean ot_remote_builtin_summary (int argc, char **argv, GCancellable *cancellable, GError **error); From 0613b4a47920374888f1c731ac4267094a6a6a8e Mon Sep 17 00:00:00 2001 From: Sjoerd Simons Date: Mon, 17 Oct 2016 22:30:14 +0200 Subject: [PATCH 34/41] remote: Add commands to add and remove cookies for a remote Add commands to add and remove cookies to a remotes cookie jar. Closes: #531 Approved by: cgwalters --- Makefile-ostree.am | 2 + src/ostree/ot-builtin-remote.c | 2 + src/ostree/ot-remote-builtin-add-cookie.c | 84 +++++++++++++++++ src/ostree/ot-remote-builtin-delete-cookie.c | 96 ++++++++++++++++++++ src/ostree/ot-remote-builtins.h | 2 + 5 files changed, 186 insertions(+) create mode 100644 src/ostree/ot-remote-builtin-add-cookie.c create mode 100644 src/ostree/ot-remote-builtin-delete-cookie.c diff --git a/Makefile-ostree.am b/Makefile-ostree.am index d2641cb4..9f2119dc 100644 --- a/Makefile-ostree.am +++ b/Makefile-ostree.am @@ -80,7 +80,9 @@ ostree_SOURCES += \ ostree_SOURCES += \ src/ostree/ot-remote-builtins.h \ src/ostree/ot-remote-builtin-add.c \ + src/ostree/ot-remote-builtin-add-cookie.c \ src/ostree/ot-remote-builtin-delete.c \ + src/ostree/ot-remote-builtin-delete-cookie.c \ src/ostree/ot-remote-builtin-gpg-import.c \ src/ostree/ot-remote-builtin-list.c \ src/ostree/ot-remote-builtin-list-cookies.c \ diff --git a/src/ostree/ot-builtin-remote.c b/src/ostree/ot-builtin-remote.c index 9ac03173..31924eb1 100644 --- a/src/ostree/ot-builtin-remote.c +++ b/src/ostree/ot-builtin-remote.c @@ -33,7 +33,9 @@ typedef struct { static OstreeRemoteCommand remote_subcommands[] = { { "add", ot_remote_builtin_add }, + { "add-cookie", ot_remote_builtin_add_cookie }, { "delete", ot_remote_builtin_delete }, + { "delete-cookie", ot_remote_builtin_delete_cookie }, { "show-url", ot_remote_builtin_show_url }, { "list", ot_remote_builtin_list }, { "list-cookies", ot_remote_builtin_list_cookies }, diff --git a/src/ostree/ot-remote-builtin-add-cookie.c b/src/ostree/ot-remote-builtin-add-cookie.c new file mode 100644 index 00000000..439e7503 --- /dev/null +++ b/src/ostree/ot-remote-builtin-add-cookie.c @@ -0,0 +1,84 @@ +/* -*- mode: C; c-file-style: "gnu"; indent-tabs-mode: nil; -*- + * + * Copyright (C) 2015 Red Hat, Inc. + * Copyright (C) 2016 Sjoerd Simons + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the + * Free Software Foundation, Inc., 59 Temple Place - Suite 330, + * Boston, MA 02111-1307, USA. + */ + +#include "config.h" + +#include + +#include "otutil.h" + +#include "ot-main.h" +#include "ot-remote-builtins.h" +#include "ostree-repo-private.h" + + +static GOptionEntry option_entries[] = { + { NULL } +}; + +gboolean +ot_remote_builtin_add_cookie (int argc, char **argv, GCancellable *cancellable, GError **error) +{ + GOptionContext *context; + glnx_unref_object OstreeRepo *repo = NULL; + const char *remote_name; + const char *domain; + const char *path; + const char *cookie_name; + const char *value; + g_autofree char *jar_path = NULL; + g_autofree char *cookie_file = NULL; + glnx_unref_object SoupCookieJar *jar = NULL; + SoupCookie *cookie; + + context = g_option_context_new ("NAME DOMAIN PATH COOKIE_NAME VALUE - Add a cookie to remote"); + + if (!ostree_option_context_parse (context, option_entries, &argc, &argv, + OSTREE_BUILTIN_FLAG_NONE, &repo, cancellable, error)) + return FALSE; + + if (argc < 6) + { + ot_util_usage_error (context, "NAME, DOMAIN, PATH, COOKIE_NAME and VALUE must be specified", error); + return FALSE; + } + + remote_name = argv[1]; + domain = argv[2]; + path = argv[3]; + cookie_name = argv[4]; + value = argv[5]; + + cookie_file = g_strdup_printf ("%s.cookies.txt", remote_name); + jar_path = g_build_filename (g_file_get_path (repo->repodir), cookie_file, NULL); + + jar = soup_cookie_jar_text_new (jar_path, FALSE); + + /* Pick a silly long expire time, we're just storing the cookies in the + * jar and on pull the jar is read-only so expiry has little actual value */ + cookie = soup_cookie_new (cookie_name, value, domain, path, + SOUP_COOKIE_MAX_AGE_ONE_YEAR * 25); + + /* jar takes ownership of cookie */ + soup_cookie_jar_add_cookie (jar, cookie); + + return TRUE; +} diff --git a/src/ostree/ot-remote-builtin-delete-cookie.c b/src/ostree/ot-remote-builtin-delete-cookie.c new file mode 100644 index 00000000..9f05a564 --- /dev/null +++ b/src/ostree/ot-remote-builtin-delete-cookie.c @@ -0,0 +1,96 @@ +/* -*- mode: C; c-file-style: "gnu"; indent-tabs-mode: nil; -*- + * + * Copyright (C) 2015 Red Hat, Inc. + * Copyright (C) 2016 Sjoerd Simons + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the + * Free Software Foundation, Inc., 59 Temple Place - Suite 330, + * Boston, MA 02111-1307, USA. + */ + +#include "config.h" + +#include + +#include "otutil.h" + +#include "ot-main.h" +#include "ot-remote-builtins.h" +#include "ostree-repo-private.h" + + +static GOptionEntry option_entries[] = { + { NULL } +}; + +gboolean +ot_remote_builtin_delete_cookie (int argc, char **argv, GCancellable *cancellable, GError **error) +{ + GOptionContext *context; + glnx_unref_object OstreeRepo *repo = NULL; + const char *remote_name; + const char *domain; + const char *path; + const char *cookie_name; + g_autofree char *jar_path = NULL; + g_autofree char *cookie_file = NULL; + glnx_unref_object SoupCookieJar *jar = NULL; + GSList *cookies; + gboolean found = FALSE; + + context = g_option_context_new ("NAME DOMAIN PATH COOKIE_NAME- Remote one cookie from remote"); + + if (!ostree_option_context_parse (context, option_entries, &argc, &argv, + OSTREE_BUILTIN_FLAG_NONE, &repo, cancellable, error)) + return FALSE; + + if (argc < 5) + { + ot_util_usage_error (context, "NAME, DOMAIN, PATH and COOKIE_NAME must be specified", error); + return FALSE; + } + + remote_name = argv[1]; + domain = argv[2]; + path = argv[3]; + cookie_name = argv[4]; + + cookie_file = g_strdup_printf ("%s.cookies.txt", remote_name); + jar_path = g_build_filename (g_file_get_path (repo->repodir), cookie_file, NULL); + + jar = soup_cookie_jar_text_new (jar_path, FALSE); + cookies = soup_cookie_jar_all_cookies (jar); + + while (cookies != NULL) + { + SoupCookie *cookie = cookies->data; + + if (!strcmp (domain, soup_cookie_get_domain (cookie)) && + !strcmp (path, soup_cookie_get_path (cookie)) && + !strcmp (cookie_name, soup_cookie_get_name (cookie))) + { + soup_cookie_jar_delete_cookie (jar, cookie); + + found = TRUE; + } + + soup_cookie_free (cookie); + cookies = g_slist_delete_link (cookies, cookies); + } + + if (!found) + g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED, "Cookie not found in jar"); + + return found; +} diff --git a/src/ostree/ot-remote-builtins.h b/src/ostree/ot-remote-builtins.h index a725752c..289e2e0d 100644 --- a/src/ostree/ot-remote-builtins.h +++ b/src/ostree/ot-remote-builtins.h @@ -25,7 +25,9 @@ G_BEGIN_DECLS gboolean ot_remote_builtin_add (int argc, char **argv, GCancellable *cancellable, GError **error); +gboolean ot_remote_builtin_add_cookie (int argc, char **argv, GCancellable *cancellable, GError **error); gboolean ot_remote_builtin_delete (int argc, char **argv, GCancellable *cancellable, GError **error); +gboolean ot_remote_builtin_delete_cookie (int argc, char **argv, GCancellable *cancellable, GError **error); gboolean ot_remote_builtin_gpg_import (int argc, char **argv, GCancellable *cancellable, GError **error); gboolean ot_remote_builtin_list (int argc, char **argv, GCancellable *cancellable, GError **error); gboolean ot_remote_builtin_list_cookies (int argc, char **argv, GCancellable *cancellable, GError **error); From be9a3a7a19336ba680bc5f4a7b62add4bbfddb81 Mon Sep 17 00:00:00 2001 From: Sjoerd Simons Date: Mon, 17 Oct 2016 22:30:41 +0200 Subject: [PATCH 35/41] OsreeFetcher: Treat 403 as not found Private Cloudfront instances return 403 for objects which don't exist rather then a 404. Change the fetcher to assume 403 is ok for download that are "optional" rather then erroring out at that step (e.g. trying to download a static delta if the remote repo doesn't have those) Closes: #531 Approved by: cgwalters --- src/libostree/ostree-fetcher.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libostree/ostree-fetcher.c b/src/libostree/ostree-fetcher.c index a4d46016..18794ce1 100644 --- a/src/libostree/ostree-fetcher.c +++ b/src/libostree/ostree-fetcher.c @@ -1057,6 +1057,7 @@ on_request_sent (GObject *object, switch (msg->status_code) { case 404: + case 403: case 410: code = G_IO_ERROR_NOT_FOUND; break; From 6303e2d67b7280fcafbf0c03cf5b09b0a0da72a4 Mon Sep 17 00:00:00 2001 From: Sjoerd Simons Date: Mon, 17 Oct 2016 22:35:40 +0200 Subject: [PATCH 36/41] trivial-httpd: Add support for checking cookies Allow passsing a list of cookie key/values to trivial-httpd which should be provided to allow downloads Closes: #531 Approved by: cgwalters --- src/ostree/ot-builtin-trivial-httpd.c | 39 +++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/src/ostree/ot-builtin-trivial-httpd.c b/src/ostree/ot-builtin-trivial-httpd.c index 2b6bda25..6e6415dd 100644 --- a/src/ostree/ot-builtin-trivial-httpd.c +++ b/src/ostree/ot-builtin-trivial-httpd.c @@ -44,6 +44,8 @@ static int opt_random_500s_percentage; static int opt_random_500s_max = 100; static gint opt_port = 0; +static gchar **opt_expected_cookies; + static guint emitted_random_500s_count = 0; typedef struct { @@ -61,6 +63,7 @@ static GOptionEntry options[] = { { "random-500s", 0, 0, G_OPTION_ARG_INT, &opt_random_500s_percentage, "Generate random HTTP 500 errors approximately for PERCENTAGE requests", "PERCENTAGE" }, { "random-500s-max", 0, 0, G_OPTION_ARG_INT, &opt_random_500s_max, "Limit HTTP 500 errors to MAX (default 100)", "MAX" }, { "log-file", 0, 0, G_OPTION_ARG_FILENAME, &opt_log, "Put logs here", "PATH" }, + { "expected-cookies", 0, 0, G_OPTION_ARG_STRING_ARRAY, &opt_expected_cookies, "Expect given cookies in the http request", "KEY=VALUE" }, { NULL } }; @@ -199,6 +202,42 @@ do_get (OtTrivialHttpd *self, struct stat stbuf; httpd_log (self, "serving %s\n", path); + + if (opt_expected_cookies) + { + GSList *cookies = soup_cookies_from_request (msg); + GSList *l; + int i; + + for (i = 0 ; opt_expected_cookies[i] != NULL; i++) + { + gboolean found = FALSE; + gchar *k = opt_expected_cookies[i]; + gchar *v = strchr (k, '=') + 1; + + for (l = cookies; l != NULL ; l = g_slist_next (l)) + { + SoupCookie *c = l->data; + + if (!strncmp (k, soup_cookie_get_name (c), v - k - 1) && + !strcmp (v, soup_cookie_get_value (c))) + { + found = TRUE; + break; + } + } + + if (!found) + { + httpd_log (self, "Expected cookie not found %s\n", k); + soup_message_set_status (msg, SOUP_STATUS_FORBIDDEN); + soup_cookies_free (cookies); + goto out; + } + } + soup_cookies_free (cookies); + } + if (strstr (path, "../") != NULL) { soup_message_set_status (msg, SOUP_STATUS_FORBIDDEN); From 6af8db6fc47ba7e98d1c5d14b0ddf3a9e3fa54e9 Mon Sep 17 00:00:00 2001 From: Sjoerd Simons Date: Mon, 17 Oct 2016 22:53:32 +0200 Subject: [PATCH 37/41] tests: Add test for the cookie jar handling Closes: #531 Approved by: cgwalters --- Makefile-tests.am | 1 + tests/test-remote-cookies.sh | 68 ++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+) create mode 100755 tests/test-remote-cookies.sh diff --git a/Makefile-tests.am b/Makefile-tests.am index 55381234..6c2301ce 100644 --- a/Makefile-tests.am +++ b/Makefile-tests.am @@ -45,6 +45,7 @@ dist_test_scripts = \ tests/test-pull-subpath.sh \ tests/test-archivez.sh \ tests/test-remote-add.sh \ + tests/test-remote-cookies.sh \ tests/test-remote-gpg-import.sh \ tests/test-commit-sign.sh \ tests/test-export.sh \ diff --git a/tests/test-remote-cookies.sh b/tests/test-remote-cookies.sh new file mode 100755 index 00000000..11c201f1 --- /dev/null +++ b/tests/test-remote-cookies.sh @@ -0,0 +1,68 @@ +#!/bin/bash +# +# Copyright (C) 2013 Jeremy Whiting +# Copyright (C) 2016 Sjoerd Simons +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this library; if not, write to the +# Free Software Foundation, Inc., 59 Temple Place - Suite 330, +# Boston, MA 02111-1307, USA. + +set -euo pipefail + +echo '1..4' + +. $(dirname $0)/libtest.sh + +setup_fake_remote_repo1 "archive-z2" "" \ + "--expected-cookies foo=bar --expected-cookies baz=badger" + +assert_fail (){ + set +e + $@ + if [ $? = 0 ] ; then + echo 1>&2 "$@ did not fail"; exit 1 + fi + set -euo pipefail +} + +cd ${test_tmpdir} +rm repo -rf +mkdir repo +${CMD_PREFIX} ostree --repo=repo init +${CMD_PREFIX} ostree --repo=repo remote add --set=gpg-verify=false origin $(cat httpd-address)/ostree/gnomerepo + +# Sanity check the setup, without cookies the pull should fail +assert_fail ${CMD_PREFIX} ostree --repo=repo pull origin main + +echo "ok, setup done" + +# Add 2 cookies, pull should succeed now +${CMD_PREFIX} ostree --repo=repo remote add-cookie origin 127.0.0.1 / foo bar +${CMD_PREFIX} ostree --repo=repo remote add-cookie origin 127.0.0.1 / baz badger +${CMD_PREFIX} ostree --repo=repo pull origin main + +echo "ok, initial cookie pull succeeded" + +# Delete one cookie, if successful pulls will fail again +${CMD_PREFIX} ostree --repo=repo remote delete-cookie origin 127.0.0.1 / baz badger +assert_fail ${CMD_PREFIX} ostree --repo=repo pull origin main + +echo "ok, delete succeeded" + +# Re-add the removed cooking and things succeed again, verified the removal +# removed exactly one cookie +${CMD_PREFIX} ostree --repo=repo remote add-cookie origin 127.0.0.1 / baz badger +${CMD_PREFIX} ostree --repo=repo pull origin main + +echo "ok, second cookie pull succeeded" From 2b150f52f8961b9e7b3a477d59b68275902ed1a7 Mon Sep 17 00:00:00 2001 From: Sjoerd Simons Date: Mon, 17 Oct 2016 22:47:58 +0200 Subject: [PATCH 38/41] Update documentation for cookie handling commands Closes: #531 Approved by: cgwalters --- man/ostree-remote.xml | 22 ++++++++++++++++++++++ man/ostree.repo-config.xml | 12 ++++++++++++ 2 files changed, 34 insertions(+) diff --git a/man/ostree-remote.xml b/man/ostree-remote.xml index 2e600845..88e61ac0 100644 --- a/man/ostree-remote.xml +++ b/man/ostree-remote.xml @@ -69,6 +69,25 @@ Boston, MA 02111-1307, USA. ostree remote summary OPTIONS NAME + + ostree remote add-cookie + NAME + DOMAIN + PATH + COOKIE_NAME + VALUE + + + ostree remote delete-cookie + NAME + DOMAIN + PATH + COOKIE_NAME + VALUE + + + ostree remote list-cookies NAME + @@ -83,6 +102,9 @@ Boston, MA 02111-1307, USA. The GPG keys to import may be in binary OpenPGP format or ASCII armored. The optional KEY-ID list can restrict which keys are imported from a keyring file or input stream. All keys are imported if this list is omitted. If neither nor options are given, then keys are imported from the user's personal GPG keyring. + + The various cookie related command allow management of a remote specific cookie jar. + diff --git a/man/ostree.repo-config.xml b/man/ostree.repo-config.xml index 0c421ba4..1182aa93 100644 --- a/man/ostree.repo-config.xml +++ b/man/ostree.repo-config.xml @@ -204,6 +204,18 @@ Boston, MA 02111-1307, USA. in the section GPG verification. + + + Per-remote HTTP cookies + + Some content providers may want to control access to remote + repositories via HTTP cookies. The ostree remote + add-cookie and ostree remote + delete-cookie commands will update a per-remote + lookaside cookie jar, named + $remotename.cookies.txt. + + See Also From 24ac4ff190a96de47fab73d81c257f089c0e2020 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 3 Nov 2016 08:32:19 -0400 Subject: [PATCH 39/41] deltas: Only keep one file open at a time during compilation Otherwise it's possible for us to exhaust available file descriptors or (on 32 bit) run up against mmap limits. In the rollsum case, we didn't need to hold open the "from" object at all. And in the bsdiff case, we weren't even looking at either of the files until we started processing. Also, while we have the patient open, switch to using O_TMPFILE if available. Closes: #567 Approved by: giuseppe --- .../ostree-repo-static-delta-compilation.c | 111 +++++++----------- 1 file changed, 45 insertions(+), 66 deletions(-) diff --git a/src/libostree/ostree-repo-static-delta-compilation.c b/src/libostree/ostree-repo-static-delta-compilation.c index fef204c7..286c74e1 100644 --- a/src/libostree/ostree-repo-static-delta-compilation.c +++ b/src/libostree/ostree-repo-static-delta-compilation.c @@ -32,6 +32,7 @@ #include "ostree-diff.h" #include "ostree-rollsum.h" #include "otutil.h" +#include "libglnx.h" #include "ostree-varint.h" #include "bsdiff/bsdiff.h" @@ -413,15 +414,11 @@ process_one_object (OstreeRepo *repo, typedef struct { char *from_checksum; - GBytes *tmp_from; - GBytes *tmp_to; } ContentBsdiff; typedef struct { char *from_checksum; OstreeRollsumMatches *matches; - GBytes *tmp_from; - GBytes *tmp_to; } ContentRollsum; static void @@ -429,8 +426,6 @@ content_rollsums_free (ContentRollsum *rollsum) { g_free (rollsum->from_checksum); _ostree_rollsum_matches_free (rollsum->matches); - g_clear_pointer (&rollsum->tmp_from, g_bytes_unref); - g_clear_pointer (&rollsum->tmp_to, g_bytes_unref); g_free (rollsum); } @@ -438,8 +433,6 @@ static void content_bsdiffs_free (ContentBsdiff *bsdiff) { g_free (bsdiff->from_checksum); - g_clear_pointer (&bsdiff->tmp_from, g_bytes_unref); - g_clear_pointer (&bsdiff->tmp_to, g_bytes_unref); g_free (bsdiff); } @@ -450,50 +443,40 @@ static gboolean get_unpacked_unlinked_content (OstreeRepo *repo, const char *checksum, GBytes **out_content, - GFileInfo **out_finfo, GCancellable *cancellable, GError **error) { - gboolean ret = FALSE; - g_autofree char *tmpname = g_strdup ("/var/tmp/tmpostree-deltaobj-XXXXXX"); + g_autofree char *tmpname = NULL; glnx_fd_close int fd = -1; g_autoptr(GBytes) ret_content = NULL; g_autoptr(GInputStream) istream = NULL; - g_autoptr(GFileInfo) ret_finfo = NULL; g_autoptr(GOutputStream) out = NULL; - fd = g_mkstemp (tmpname); - if (fd == -1) - { - glnx_set_error_from_errno (error); - goto out; - } - /* Doesn't need a name */ - (void) unlink (tmpname); + if (!glnx_open_tmpfile_linkable_at (AT_FDCWD, "/tmp", O_RDWR | O_CLOEXEC, + &fd, &tmpname, error)) + return FALSE; + /* We don't need the file name */ + if (tmpname) + (void) unlinkat (AT_FDCWD, tmpname, 0); - if (!ostree_repo_load_file (repo, checksum, &istream, &ret_finfo, NULL, + if (!ostree_repo_load_file (repo, checksum, &istream, NULL, NULL, cancellable, error)) - goto out; + return FALSE; - g_assert (g_file_info_get_file_type (ret_finfo) == G_FILE_TYPE_REGULAR); - out = g_unix_output_stream_new (fd, FALSE); if (g_output_stream_splice (out, istream, G_OUTPUT_STREAM_SPLICE_CLOSE_TARGET, cancellable, error) < 0) - goto out; - - { GMappedFile *mfile = g_mapped_file_new_from_fd (fd, FALSE, error); + return FALSE; + + { g_autoptr(GMappedFile) mfile = g_mapped_file_new_from_fd (fd, FALSE, error); if (!mfile) - goto out; + return FALSE; ret_content = g_mapped_file_get_bytes (mfile); - g_mapped_file_unref (mfile); } - ret = TRUE; if (out_content) *out_content = g_steal_pointer (&ret_content); - out: - return ret; + return TRUE; } static gboolean @@ -506,22 +489,20 @@ try_content_bsdiff (OstreeRepo *repo, GError **error) { gboolean ret = FALSE; - g_autoptr(GBytes) tmp_from = NULL; - g_autoptr(GBytes) tmp_to = NULL; g_autoptr(GFileInfo) from_finfo = NULL; g_autoptr(GFileInfo) to_finfo = NULL; ContentBsdiff *ret_bsdiff = NULL; *out_bsdiff = NULL; - if (!get_unpacked_unlinked_content (repo, from, &tmp_from, &from_finfo, - cancellable, error)) - goto out; - if (!get_unpacked_unlinked_content (repo, to, &tmp_to, &to_finfo, - cancellable, error)) - goto out; + if (!ostree_repo_load_file (repo, from, NULL, &from_finfo, NULL, + cancellable, error)) + return FALSE; + if (!ostree_repo_load_file (repo, to, NULL, &to_finfo, NULL, + cancellable, error)) + return FALSE; - if (g_bytes_get_size (tmp_to) + g_bytes_get_size (tmp_from) > max_bsdiff_size_bytes) + if (g_file_info_get_size (to_finfo) + g_file_info_get_size (from_finfo) > max_bsdiff_size_bytes) { ret = TRUE; goto out; @@ -529,8 +510,6 @@ try_content_bsdiff (OstreeRepo *repo, ret_bsdiff = g_new0 (ContentBsdiff, 1); ret_bsdiff->from_checksum = g_strdup (from); - ret_bsdiff->tmp_from = tmp_from; tmp_from = NULL; - ret_bsdiff->tmp_to = tmp_to; tmp_to = NULL; ret = TRUE; if (out_bsdiff) @@ -551,8 +530,6 @@ try_content_rollsum (OstreeRepo *repo, gboolean ret = FALSE; g_autoptr(GBytes) tmp_from = NULL; g_autoptr(GBytes) tmp_to = NULL; - g_autoptr(GFileInfo) from_finfo = NULL; - g_autoptr(GFileInfo) to_finfo = NULL; OstreeRollsumMatches *matches = NULL; ContentRollsum *ret_rollsum = NULL; @@ -561,11 +538,9 @@ try_content_rollsum (OstreeRepo *repo, /* Load the content objects, splice them to uncompressed temporary files that * we can just mmap() and seek around in conveniently. */ - if (!get_unpacked_unlinked_content (repo, from, &tmp_from, &from_finfo, - cancellable, error)) + if (!get_unpacked_unlinked_content (repo, from, &tmp_from, cancellable, error)) goto out; - if (!get_unpacked_unlinked_content (repo, to, &tmp_to, &to_finfo, - cancellable, error)) + if (!get_unpacked_unlinked_content (repo, to, &tmp_to, cancellable, error)) goto out; matches = _ostree_compute_rollsum_matches (tmp_from, tmp_to); @@ -592,10 +567,8 @@ try_content_rollsum (OstreeRepo *repo, ret_rollsum = g_new0 (ContentRollsum, 1); ret_rollsum->from_checksum = g_strdup (from); - ret_rollsum->matches = matches; matches = NULL; - ret_rollsum->tmp_from = tmp_from; tmp_from = NULL; - ret_rollsum->tmp_to = tmp_to; tmp_to = NULL; - + ret_rollsum->matches = g_steal_pointer (&matches); + ret = TRUE; if (out_rollsum) *out_rollsum = g_steal_pointer (&ret_rollsum); @@ -651,7 +624,7 @@ process_one_rollsum (OstreeRepo *repo, { gboolean ret = FALSE; guint64 content_size; - g_autoptr(GInputStream) content_stream = NULL; + g_autoptr(GBytes) tmp_to = NULL; g_autoptr(GFileInfo) content_finfo = NULL; g_autoptr(GVariant) content_xattrs = NULL; OstreeStaticDeltaPartBuilder *current_part = *current_part_val; @@ -665,9 +638,13 @@ process_one_rollsum (OstreeRepo *repo, *current_part_val = current_part = allocate_part (builder); } - tmp_to_buf = g_bytes_get_data (rollsum->tmp_to, &tmp_to_len); + if (!get_unpacked_unlinked_content (repo, to_checksum, &tmp_to, + cancellable, error)) + goto out; - if (!ostree_repo_load_file (repo, to_checksum, &content_stream, + tmp_to_buf = g_bytes_get_data (tmp_to, &tmp_to_len); + + if (!ostree_repo_load_file (repo, to_checksum, NULL, &content_finfo, &content_xattrs, cancellable, error)) goto out; @@ -754,9 +731,6 @@ process_one_rollsum (OstreeRepo *repo, g_string_append_c (current_part->operations, (gchar)OSTREE_STATIC_DELTA_OP_CLOSE); } - g_clear_pointer (&rollsum->tmp_from, g_bytes_unref); - g_clear_pointer (&rollsum->tmp_to, g_bytes_unref); - ret = TRUE; out: return ret; @@ -773,10 +747,11 @@ process_one_bsdiff (OstreeRepo *repo, { gboolean ret = FALSE; guint64 content_size; - g_autoptr(GInputStream) content_stream = NULL; g_autoptr(GFileInfo) content_finfo = NULL; g_autoptr(GVariant) content_xattrs = NULL; OstreeStaticDeltaPartBuilder *current_part = *current_part_val; + g_autoptr(GBytes) tmp_from = NULL; + g_autoptr(GBytes) tmp_to = NULL; const guint8 *tmp_to_buf; gsize tmp_to_len; const guint8 *tmp_from_buf; @@ -789,10 +764,17 @@ process_one_bsdiff (OstreeRepo *repo, *current_part_val = current_part = allocate_part (builder); } - tmp_to_buf = g_bytes_get_data (bsdiff_content->tmp_to, &tmp_to_len); - tmp_from_buf = g_bytes_get_data (bsdiff_content->tmp_from, &tmp_from_len); + if (!get_unpacked_unlinked_content (repo, bsdiff_content->from_checksum, &tmp_from, + cancellable, error)) + goto out; + if (!get_unpacked_unlinked_content (repo, to_checksum, &tmp_to, + cancellable, error)) + goto out; - if (!ostree_repo_load_file (repo, to_checksum, &content_stream, + tmp_to_buf = g_bytes_get_data (tmp_to, &tmp_to_len); + tmp_from_buf = g_bytes_get_data (tmp_from, &tmp_from_len); + + if (!ostree_repo_load_file (repo, to_checksum, NULL, &content_finfo, &content_xattrs, cancellable, error)) goto out; @@ -853,9 +835,6 @@ process_one_bsdiff (OstreeRepo *repo, g_string_append_c (current_part->operations, (gchar)OSTREE_STATIC_DELTA_OP_UNSET_READ_SOURCE); - g_clear_pointer (&bsdiff_content->tmp_from, g_bytes_unref); - g_clear_pointer (&bsdiff_content->tmp_to, g_bytes_unref); - ret = TRUE; out: return ret; From 39e7293e3f9a143c42426cffceaf15f576303d5f Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Tue, 8 Nov 2016 21:37:43 -0500 Subject: [PATCH 40/41] .redhat-ci.yml: use new build key This allows us to more concisely separate building from testing, which in turn gives us a nicer inheritance pattern in our case. See also: https://github.com/jlebon/redhat-ci/issues/11 Closes: #569 Approved by: cgwalters --- .redhat-ci.Dockerfile | 2 ++ .redhat-ci.yml | 34 ++++++++++++++++++---------------- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/.redhat-ci.Dockerfile b/.redhat-ci.Dockerfile index 304563ef..f1a41299 100644 --- a/.redhat-ci.Dockerfile +++ b/.redhat-ci.Dockerfile @@ -9,6 +9,8 @@ RUN dnf install -y \ fuse \ gjs \ parallel \ + clang \ + libubsan \ gnome-desktop-testing \ redhat-rpm-config \ elfutils \ diff --git a/.redhat-ci.yml b/.redhat-ci.yml index f2a0cfe9..9994aa10 100644 --- a/.redhat-ci.yml +++ b/.redhat-ci.yml @@ -6,20 +6,24 @@ branches: container: image: projectatomic/ostree-tester +# XXX: we can wipe this off once a newer image is built with +# it already included packages: - libubsan +env: + CFLAGS: '-fsanitize=undefined' + +build: + config-opts: > + --prefix=/usr + --libdir=/usr/lib64 + --enable-installed-tests + --enable-gtk-doc + tests: - - sh autogen.sh - --prefix=/usr - --libdir=/usr/lib64 - --enable-installed-tests - --enable-gtk-doc - CFLAGS='-fsanitize=undefined' - - make -j2 - make syntax-check - make check - - make install - gnome-desktop-testing-runner ostree - sudo --user=testuser gnome-desktop-testing-runner ostree @@ -32,17 +36,15 @@ artifacts: inherit: true +# XXX: ditto packages: - clang -tests: - - sh autogen.sh - --prefix=/usr - --libdir=/usr/lib64 - --enable-installed-tests - --enable-gtk-doc - - make -j2 CC=clang CFLAGS='-Werror=unused-variable' - context: Clang +env: + CC: 'clang' + CFLAGS: '-Werror=unused-variable' + +tests: artifacts: From 36c894687046830cafdab3e247dc5ada8c26da52 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 10 Nov 2016 11:27:43 -0500 Subject: [PATCH 41/41] Release 2016.13 Closes: #570 Approved by: jlebon --- configure.ac | 2 +- src/libostree/libostree.sym | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/configure.ac b/configure.ac index 7a6fdf13..ffa86295 100644 --- a/configure.ac +++ b/configure.ac @@ -1,6 +1,6 @@ AC_PREREQ([2.63]) dnl If incrementing the version here, remember to update libostree.sym too -AC_INIT([ostree], [2016.12], [walters@verbum.org]) +AC_INIT([ostree], [2016.13], [walters@verbum.org]) AC_CONFIG_HEADER([config.h]) AC_CONFIG_MACRO_DIR([buildutil]) AC_CONFIG_AUX_DIR([build-aux]) diff --git a/src/libostree/libostree.sym b/src/libostree/libostree.sym index ddb87e7b..fb7e5848 100644 --- a/src/libostree/libostree.sym +++ b/src/libostree/libostree.sym @@ -357,14 +357,15 @@ global: /* No new symbols in 2016.10 */ /* No new symbols in 2016.11 */ /* No new symbols in 2016.12 */ +/* No new symbols in 2016.13 */ /* NOTE NOTE NOTE * Versions above here are released. Only add symbols below this line. * NOTE NOTE NOTE */ -/* Remove comment when first new symbol is added -LIBOSTREE_2016.12 +/* Remove comment when first new symbol is added, replace XX with new stable version. +LIBOSTREE_2016.XX global: someostree_symbol_deleteme; } LIBOSTREE_2016.8;