From 28ec43c41ad034da3cec695017d69c36c706ec7b Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 5 Jul 2017 16:43:42 -0400 Subject: [PATCH 01/35] build-sys: Post-release version bump Closes: #994 Approved by: jlebon --- configure.ac | 4 ++-- src/libostree/libostree-devel.sym | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index 30ae793b..3fc0fabf 100644 --- a/configure.ac +++ b/configure.ac @@ -4,10 +4,10 @@ dnl update libostree-released.sym from libostree-devel.sym, and update the check dnl in test-symbols.sh, and also set is_release_build=yes below. Then make dnl another post-release commit to bump the version, and set is_release_build=no. m4_define([year_version], [2017]) -m4_define([release_version], [8]) +m4_define([release_version], [9]) m4_define([package_version], [year_version.release_version]) AC_INIT([libostree], [package_version], [walters@verbum.org]) -is_release_build=yes +is_release_build=no AC_CONFIG_HEADER([config.h]) AC_CONFIG_MACRO_DIR([buildutil]) AC_CONFIG_AUX_DIR([build-aux]) diff --git a/src/libostree/libostree-devel.sym b/src/libostree/libostree-devel.sym index 01f182f6..68ed4020 100644 --- a/src/libostree/libostree-devel.sym +++ b/src/libostree/libostree-devel.sym @@ -18,6 +18,8 @@ ***/ /* Add new symbols here. Release commits should copy this section into -released.sym. */ +LIBOSTREE_2017.9 { +}; /* Stub section for the stable release *after* this development one; don't * edit this other than to update the last number. This is just a copy/paste From cf16805a2f70026f24eb0b245666efcd4a7c8e44 Mon Sep 17 00:00:00 2001 From: Krzesimir Nowak Date: Thu, 22 Jun 2017 21:49:22 +0200 Subject: [PATCH 02/35] ostree: Add collection and ref bindings to metadata on commit The collection and ref bindings are stored in the commit metadata under ostree.collection-binding and ostree.ref-binding, respectively. They will be used to verify if the commit really comes from the collection and ref we wanted to pull from. Closes: #972 Approved by: cgwalters --- src/libostree/ostree-repo-private.h | 4 ++ src/ostree/ot-builtin-commit.c | 71 +++++++++++++++++++++++++++++ tests/basic-test.sh | 6 ++- 3 files changed, 79 insertions(+), 2 deletions(-) diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h index 407e2cb3..dc49ed3b 100644 --- a/src/libostree/ostree-repo-private.h +++ b/src/libostree/ostree-repo-private.h @@ -56,6 +56,10 @@ G_BEGIN_DECLS * in a summary file. */ #define OSTREE_COMMIT_TIMESTAMP "ostree.commit.timestamp" +/* Well-known keys for the commit metadata */ +#define OSTREE_REF_BINDING "ostree.ref-binding" +#define OSTREE_COLLECTION_BINDING "ostree.collection-binding" + typedef enum { OSTREE_REPO_TEST_ERROR_PRE_COMMIT = (1 << 0) } OstreeRepoTestErrorFlags; diff --git a/src/ostree/ot-builtin-commit.c b/src/ostree/ot-builtin-commit.c index 8e8b9233..35340a04 100644 --- a/src/ostree/ot-builtin-commit.c +++ b/src/ostree/ot-builtin-commit.c @@ -29,6 +29,7 @@ #include "otutil.h" #include "ot-tool-util.h" #include "parse-datetime.h" +#include "ostree-repo-private.h" static char *opt_subject; static char *opt_body; @@ -36,6 +37,7 @@ static char *opt_body_file; static gboolean opt_editor; static char *opt_parent; static gboolean opt_orphan; +static char **opt_bind_refs; static char *opt_branch; static char *opt_statoverride_file; static char *opt_skiplist_file; @@ -78,6 +80,7 @@ static GOptionEntry options[] = { { "editor", 'e', 0, G_OPTION_ARG_NONE, &opt_editor, "Use an editor to write the commit message", NULL }, { "branch", 'b', 0, G_OPTION_ARG_STRING, &opt_branch, "Branch", "BRANCH" }, { "orphan", 0, 0, G_OPTION_ARG_NONE, &opt_orphan, "Create a commit without writing a ref", NULL }, + { "bind-ref", 0, 0, G_OPTION_ARG_STRING_ARRAY, &opt_bind_refs, "Add a ref to ref binding commit metadata", "BRANCH" }, { "tree", 0, 0, G_OPTION_ARG_STRING_ARRAY, &opt_trees, "Overlay the given argument as a tree", "dir=PATH or tar=TARFILE or ref=COMMIT" }, { "add-metadata-string", 0, 0, G_OPTION_ARG_STRING_ARRAY, &opt_metadata_strings, "Add a key/value pair to metadata", "KEY=VALUE" }, { "add-detached-metadata-string", 0, 0, G_OPTION_ARG_STRING_ARRAY, &opt_detached_metadata_strings, "Add a key/value pair to detached metadata", "KEY=VALUE" }, @@ -303,6 +306,70 @@ parse_keyvalue_strings (char **strings, return TRUE; } +#ifdef OSTREE_ENABLE_EXPERIMENTAL_API +static void +add_collection_binding (OstreeRepo *repo, + GVariantBuilder *metadata_builder) +{ + const char *collection_id = ostree_repo_get_collection_id (repo); + + if (collection_id == NULL) + return; + + g_variant_builder_add (metadata_builder, "{s@v}", OSTREE_COLLECTION_BINDING, + g_variant_new_variant (g_variant_new_string (collection_id))); +} +#endif /* OSTREE_ENABLE_EXPERIMENTAL_API */ + +static int +compare_strings (gconstpointer a, gconstpointer b) +{ + const char **sa = (const char **)a; + const char **sb = (const char **)b; + + return strcmp (*sa, *sb); +} + +static void +add_ref_binding (GVariantBuilder *metadata_builder) +{ + if (opt_orphan) + return; + + g_assert_nonnull (opt_branch); + + g_autoptr(GPtrArray) refs = g_ptr_array_new (); + g_ptr_array_add (refs, opt_branch); + for (char **iter = opt_bind_refs; iter != NULL && *iter != NULL; ++iter) + g_ptr_array_add (refs, *iter); + g_ptr_array_sort (refs, compare_strings); + g_autoptr(GVariant) refs_v = g_variant_new_strv ((const char *const *)refs->pdata, + refs->len); + g_variant_builder_add (metadata_builder, "{s@v}", OSTREE_REF_BINDING, + g_variant_new_variant (g_steal_pointer (&refs_v))); +} + +static void +fill_bindings (OstreeRepo *repo, + GVariant *metadata, + GVariant **out_metadata) +{ + g_autoptr(GVariantBuilder) metadata_builder = + ot_util_variant_builder_from_variant (metadata, G_VARIANT_TYPE_VARDICT); + + add_ref_binding (metadata_builder); + +#ifdef OSTREE_ENABLE_EXPERIMENTAL_API + /* Allow the collection ID to be overridden using + * --add-metadata-string=ostree.collection-binding=blah */ + if (metadata == NULL || + !g_variant_lookup (metadata, OSTREE_COLLECTION_BINDING, "*", NULL)) + add_collection_binding (repo, metadata_builder); +#endif /* OSTREE_ENABLE_EXPERIMENTAL_API */ + + *out_metadata = g_variant_ref_sink (g_variant_builder_end (metadata_builder)); +} + gboolean ostree_builtin_commit (int argc, char **argv, GCancellable *cancellable, GError **error) { @@ -559,6 +626,10 @@ ostree_builtin_commit (int argc, char **argv, GCancellable *cancellable, GError { gboolean update_summary; guint64 timestamp; + g_autoptr(GVariant) old_metadata = g_steal_pointer (&metadata); + + fill_bindings (repo, old_metadata, &metadata); + if (!opt_timestamp) { GDateTime *now = g_date_time_new_now_utc (); diff --git a/tests/basic-test.sh b/tests/basic-test.sh index 07bbeddf..33fdde9d 100644 --- a/tests/basic-test.sh +++ b/tests/basic-test.sh @@ -171,8 +171,10 @@ assert_streq $($OSTREE log test2-no-parent |grep '^commit' | wc -l) "1" echo "ok commit no parent" cd ${test_tmpdir} -empty_rev=$($OSTREE commit ${COMMIT_ARGS} -b test2-no-subject -s '' --timestamp="2005-10-29 12:43:29 +0000" $test_tmpdir/checkout-test2-4) -omitted_rev=$($OSTREE commit ${COMMIT_ARGS} -b test2-no-subject-2 --timestamp="2005-10-29 12:43:29 +0000" $test_tmpdir/checkout-test2-4) +# Do the --bind-ref=, so we store both branches sorted +# in metadata and thus the checksums remain the same. +empty_rev=$($OSTREE commit ${COMMIT_ARGS} -b test2-no-subject --bind-ref=test2-no-subject-2 -s '' --timestamp="2005-10-29 12:43:29 +0000" $test_tmpdir/checkout-test2-4) +omitted_rev=$($OSTREE commit ${COMMIT_ARGS} -b test2-no-subject-2 --bind-ref=test2-no-subject --timestamp="2005-10-29 12:43:29 +0000" $test_tmpdir/checkout-test2-4) assert_streq $empty_rev $omitted_rev echo "ok commit no subject" From d91f6a0f6103fe6b2fcc24dd6fcf24a8aa22d633 Mon Sep 17 00:00:00 2001 From: Krzesimir Nowak Date: Fri, 30 Jun 2017 16:27:33 +0200 Subject: [PATCH 03/35] lib/pull: Pass the ref together with the request We will want to use the requested ref later for the binding verification. Closes: #972 Approved by: cgwalters --- src/libostree/ostree-repo-pull.c | 195 ++++++++++++++++++------------- 1 file changed, 111 insertions(+), 84 deletions(-) diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index b51220bb..49ac91da 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -155,6 +155,8 @@ typedef struct { * whether to fetch the primary object after fetching its * detached metadata (no need if it's already stored). */ gboolean object_is_stored; + + OstreeCollectionRef *requested_ref; /* (nullable) */ } FetchObjectData; typedef struct { @@ -172,31 +174,35 @@ typedef struct { char *path; OstreeObjectType objtype; guint recursion_depth; /* NB: not used anymore, though might be nice to print */ + OstreeCollectionRef *requested_ref; /* (nullable) */ } ScanObjectQueueData; static void start_fetch (OtPullData *pull_data, FetchObjectData *fetch); static void start_fetch_deltapart (OtPullData *pull_data, FetchStaticDeltaData *fetch); static gboolean fetcher_queue_is_full (OtPullData *pull_data); -static void queue_scan_one_metadata_object (OtPullData *pull_data, - const char *csum, - OstreeObjectType objtype, - const char *path, - guint recursion_depth); +static void queue_scan_one_metadata_object (OtPullData *pull_data, + const char *csum, + OstreeObjectType objtype, + const char *path, + guint recursion_depth, + const OstreeCollectionRef *ref); -static void queue_scan_one_metadata_object_c (OtPullData *pull_data, - const guchar *csum, - OstreeObjectType objtype, - const char *path, - guint recursion_depth); +static void queue_scan_one_metadata_object_c (OtPullData *pull_data, + const guchar *csum, + OstreeObjectType objtype, + const char *path, + guint recursion_depth, + const OstreeCollectionRef *ref); -static gboolean scan_one_metadata_object_c (OtPullData *pull_data, - const guchar *csum, - OstreeObjectType objtype, - const char *path, - guint recursion_depth, - GCancellable *cancellable, - GError **error); +static gboolean scan_one_metadata_object_c (OtPullData *pull_data, + const guchar *csum, + OstreeObjectType objtype, + const char *path, + guint recursion_depth, + const OstreeCollectionRef *ref, + GCancellable *cancellable, + GError **error); static gboolean update_progress (gpointer user_data) @@ -422,11 +428,14 @@ idle_worker (gpointer user_data) scan_data->objtype, scan_data->path, scan_data->recursion_depth, + scan_data->requested_ref, pull_data->cancellable, &error); check_outstanding_requests_handle_error (pull_data, &error); g_free (scan_data->path); + if (scan_data->requested_ref != NULL) + ostree_collection_ref_free (scan_data->requested_ref); g_free (scan_data); return G_SOURCE_CONTINUE; } @@ -507,12 +516,13 @@ write_commitpartial_for (OtPullData *pull_data, } static void -enqueue_one_object_request (OtPullData *pull_data, - const char *checksum, - OstreeObjectType objtype, - const char *path, - gboolean is_detached_meta, - gboolean object_is_stored); +enqueue_one_object_request (OtPullData *pull_data, + const char *checksum, + OstreeObjectType objtype, + const char *path, + gboolean is_detached_meta, + gboolean object_is_stored, + const OstreeCollectionRef *ref); static gboolean matches_pull_dir (const char *current_file, @@ -749,7 +759,7 @@ scan_dirtree_object (OtPullData *pull_data, /* Not available locally, queue a HTTP request */ g_hash_table_add (pull_data->requested_content, file_checksum); - enqueue_one_object_request (pull_data, file_checksum, OSTREE_OBJECT_TYPE_FILE, path, FALSE, FALSE); + enqueue_one_object_request (pull_data, file_checksum, OSTREE_OBJECT_TYPE_FILE, path, FALSE, FALSE, NULL); file_checksum = NULL; /* Transfer ownership */ } @@ -779,9 +789,9 @@ scan_dirtree_object (OtPullData *pull_data, g_autofree char *subpath = g_strconcat (path, dirname, "/", NULL); queue_scan_one_metadata_object_c (pull_data, tree_csum_bytes, - OSTREE_OBJECT_TYPE_DIR_TREE, subpath, recursion_depth + 1); + OSTREE_OBJECT_TYPE_DIR_TREE, subpath, recursion_depth + 1, NULL); queue_scan_one_metadata_object_c (pull_data, meta_csum_bytes, - OSTREE_OBJECT_TYPE_DIR_META, subpath, recursion_depth + 1); + OSTREE_OBJECT_TYPE_DIR_META, subpath, recursion_depth + 1, NULL); } return TRUE; @@ -883,6 +893,8 @@ fetch_object_data_free (FetchObjectData *fetch_data) { g_variant_unref (fetch_data->object); g_free (fetch_data->path); + if (fetch_data->requested_ref) + ostree_collection_ref_free (fetch_data->requested_ref); g_free (fetch_data); } @@ -1062,7 +1074,7 @@ on_metadata_written (GObject *object, goto out; } - queue_scan_one_metadata_object_c (pull_data, csum, objtype, fetch_data->path, 0); + queue_scan_one_metadata_object_c (pull_data, csum, objtype, fetch_data->path, 0, fetch_data->requested_ref); out: pull_data->n_outstanding_metadata_write_requests--; @@ -1108,9 +1120,9 @@ meta_fetch_on_complete (GObject *object, g_hash_table_add (pull_data->fetched_detached_metadata, g_strdup (checksum)); if (!fetch_data->object_is_stored) - enqueue_one_object_request (pull_data, checksum, objtype, fetch_data->path, FALSE, FALSE); + enqueue_one_object_request (pull_data, checksum, objtype, fetch_data->path, FALSE, FALSE, fetch_data->requested_ref); else - queue_scan_one_metadata_object (pull_data, checksum, objtype, fetch_data->path, 0); + queue_scan_one_metadata_object (pull_data, checksum, objtype, fetch_data->path, 0, fetch_data->requested_ref); } /* When traversing parents, do not fail on a missing commit. @@ -1125,7 +1137,7 @@ meta_fetch_on_complete (GObject *object, if (pull_data->has_tombstone_commits) { enqueue_one_object_request (pull_data, checksum, OSTREE_OBJECT_TYPE_TOMBSTONE_COMMIT, - fetch_data->path, FALSE, FALSE); + fetch_data->path, FALSE, FALSE, NULL); } } } @@ -1162,9 +1174,9 @@ meta_fetch_on_complete (GObject *object, g_hash_table_add (pull_data->fetched_detached_metadata, g_strdup (checksum)); if (!fetch_data->object_is_stored) - enqueue_one_object_request (pull_data, checksum, objtype, fetch_data->path, FALSE, FALSE); + enqueue_one_object_request (pull_data, checksum, objtype, fetch_data->path, FALSE, FALSE, fetch_data->requested_ref); else - queue_scan_one_metadata_object (pull_data, checksum, objtype, fetch_data->path, 0); + queue_scan_one_metadata_object (pull_data, checksum, objtype, fetch_data->path, 0, fetch_data->requested_ref); } else { @@ -1358,11 +1370,12 @@ commitstate_is_partial (OtPullData *pull_data, } static gboolean -scan_commit_object (OtPullData *pull_data, - const char *checksum, - guint recursion_depth, - GCancellable *cancellable, - GError **error) +scan_commit_object (OtPullData *pull_data, + const char *checksum, + guint recursion_depth, + const OstreeCollectionRef *ref, + GCancellable *cancellable, + GError **error) { gboolean ret = FALSE; /* If we found a legacy transaction flag, assume we have to scan. @@ -1421,7 +1434,7 @@ scan_commit_object (OtPullData *pull_data, { queue_scan_one_metadata_object_c (pull_data, parent_csum_bytes, OSTREE_OBJECT_TYPE_COMMIT, NULL, - recursion_depth + 1); + recursion_depth + 1, NULL); } else if (parent_csum_bytes != NULL && depth > 0) { @@ -1448,7 +1461,8 @@ scan_commit_object (OtPullData *pull_data, queue_scan_one_metadata_object_c (pull_data, parent_csum_bytes, OSTREE_OBJECT_TYPE_COMMIT, NULL, - recursion_depth + 1); + recursion_depth + 1, + NULL); } } @@ -1475,10 +1489,10 @@ scan_commit_object (OtPullData *pull_data, goto out; queue_scan_one_metadata_object_c (pull_data, tree_contents_csum_bytes, - OSTREE_OBJECT_TYPE_DIR_TREE, "/", recursion_depth + 1); + OSTREE_OBJECT_TYPE_DIR_TREE, "/", recursion_depth + 1, NULL); queue_scan_one_metadata_object_c (pull_data, tree_meta_csum_bytes, - OSTREE_OBJECT_TYPE_DIR_META, NULL, recursion_depth + 1); + OSTREE_OBJECT_TYPE_DIR_META, NULL, recursion_depth + 1, NULL); } ret = TRUE; @@ -1487,23 +1501,25 @@ scan_commit_object (OtPullData *pull_data, } static void -queue_scan_one_metadata_object (OtPullData *pull_data, - const char *csum, - OstreeObjectType objtype, - const char *path, - guint recursion_depth) +queue_scan_one_metadata_object (OtPullData *pull_data, + const char *csum, + OstreeObjectType objtype, + const char *path, + guint recursion_depth, + const OstreeCollectionRef *ref) { guchar buf[OSTREE_SHA256_DIGEST_LEN]; ostree_checksum_inplace_to_bytes (csum, buf); - queue_scan_one_metadata_object_c (pull_data, buf, objtype, path, recursion_depth); + queue_scan_one_metadata_object_c (pull_data, buf, objtype, path, recursion_depth, ref); } static void -queue_scan_one_metadata_object_c (OtPullData *pull_data, - const guchar *csum, - OstreeObjectType objtype, - const char *path, - guint recursion_depth) +queue_scan_one_metadata_object_c (OtPullData *pull_data, + const guchar *csum, + OstreeObjectType objtype, + const char *path, + guint recursion_depth, + const OstreeCollectionRef *ref) { ScanObjectQueueData *scan_data = g_new0 (ScanObjectQueueData, 1); @@ -1511,19 +1527,21 @@ queue_scan_one_metadata_object_c (OtPullData *pull_data, scan_data->objtype = objtype; scan_data->path = g_strdup (path); scan_data->recursion_depth = recursion_depth; + scan_data->requested_ref = (ref != NULL) ? ostree_collection_ref_dup (ref) : NULL; g_queue_push_tail (&pull_data->scan_object_queue, scan_data); ensure_idle_queued (pull_data); } static gboolean -scan_one_metadata_object_c (OtPullData *pull_data, - const guchar *csum, - OstreeObjectType objtype, - const char *path, - guint recursion_depth, - GCancellable *cancellable, - GError **error) +scan_one_metadata_object_c (OtPullData *pull_data, + const guchar *csum, + OstreeObjectType objtype, + const char *path, + guint recursion_depth, + const OstreeCollectionRef *ref, + GCancellable *cancellable, + GError **error) { g_autofree char *tmp_checksum = ostree_checksum_from_bytes (csum); g_autoptr(GVariant) object = ostree_object_name_serialize (tmp_checksum, objtype); @@ -1594,7 +1612,7 @@ scan_one_metadata_object_c (OtPullData *pull_data, g_hash_table_add (pull_data->requested_metadata, g_variant_ref (object)); do_fetch_detached = (objtype == OSTREE_OBJECT_TYPE_COMMIT); - enqueue_one_object_request (pull_data, tmp_checksum, objtype, path, do_fetch_detached, FALSE); + enqueue_one_object_request (pull_data, tmp_checksum, objtype, path, do_fetch_detached, FALSE, ref); } else if (is_stored && objtype == OSTREE_OBJECT_TYPE_COMMIT) { @@ -1602,10 +1620,10 @@ scan_one_metadata_object_c (OtPullData *pull_data, * detached metadata before scanning it, in case new signatures appear. * https://github.com/projectatomic/rpm-ostree/issues/630 */ if (!g_hash_table_contains (pull_data->fetched_detached_metadata, tmp_checksum)) - enqueue_one_object_request (pull_data, tmp_checksum, objtype, path, TRUE, TRUE); + enqueue_one_object_request (pull_data, tmp_checksum, objtype, path, TRUE, TRUE, ref); else { - if (!scan_commit_object (pull_data, tmp_checksum, recursion_depth, + if (!scan_commit_object (pull_data, tmp_checksum, recursion_depth, ref, pull_data->cancellable, error)) return FALSE; @@ -1627,12 +1645,13 @@ scan_one_metadata_object_c (OtPullData *pull_data, } static void -enqueue_one_object_request (OtPullData *pull_data, - const char *checksum, - OstreeObjectType objtype, - const char *path, - gboolean is_detached_meta, - gboolean object_is_stored) +enqueue_one_object_request (OtPullData *pull_data, + const char *checksum, + OstreeObjectType objtype, + const char *path, + gboolean is_detached_meta, + gboolean object_is_stored, + const OstreeCollectionRef *ref) { gboolean is_meta; FetchObjectData *fetch_data; @@ -1645,6 +1664,7 @@ enqueue_one_object_request (OtPullData *pull_data, fetch_data->path = g_strdup (path); fetch_data->is_detached_meta = is_detached_meta; fetch_data->object_is_stored = object_is_stored; + fetch_data->requested_ref = (ref != NULL) ? ostree_collection_ref_dup (ref) : NULL; if (is_meta) pull_data->n_requested_metadata++; @@ -1812,7 +1832,7 @@ process_one_static_delta_fallback (OtPullData *pull_data, * for it as logically part of the delta fetch. */ g_hash_table_add (pull_data->requested_fallback_content, g_strdup (checksum)); - enqueue_one_object_request (pull_data, checksum, OSTREE_OBJECT_TYPE_FILE, NULL, FALSE, FALSE); + enqueue_one_object_request (pull_data, checksum, OSTREE_OBJECT_TYPE_FILE, NULL, FALSE, FALSE, NULL); checksum = NULL; /* We transferred ownership to the requested_content hash */ } } @@ -1838,12 +1858,13 @@ start_fetch_deltapart (OtPullData *pull_data, } static gboolean -process_one_static_delta (OtPullData *pull_data, - const char *from_revision, - const char *to_revision, - GVariant *delta_superblock, - GCancellable *cancellable, - GError **error) +process_one_static_delta (OtPullData *pull_data, + const char *from_revision, + const char *to_revision, + GVariant *delta_superblock, + const OstreeCollectionRef *ref, + GCancellable *cancellable, + GError **error) { gboolean ret = FALSE; gboolean delta_byteswap; @@ -1918,6 +1939,7 @@ process_one_static_delta (OtPullData *pull_data, fetch_data->object = ostree_object_name_serialize (to_checksum, OSTREE_OBJECT_TYPE_COMMIT); fetch_data->is_detached_meta = FALSE; fetch_data->object_is_stored = FALSE; + fetch_data->requested_ref = (ref != NULL) ? ostree_collection_ref_dup (ref) : NULL; ostree_repo_write_metadata_async (pull_data->repo, OSTREE_OBJECT_TYPE_COMMIT, to_checksum, to_commit, @@ -2142,6 +2164,7 @@ typedef struct { OtPullData *pull_data; char *from_revision; char *to_revision; + OstreeCollectionRef *requested_ref; /* (nullable) */ } FetchDeltaSuperData; static void @@ -2183,7 +2206,7 @@ on_superblock_fetched (GObject *src, goto out; } - queue_scan_one_metadata_object (pull_data, to_revision, OSTREE_OBJECT_TYPE_COMMIT, NULL, 0); + queue_scan_one_metadata_object (pull_data, to_revision, OSTREE_OBJECT_TYPE_COMMIT, NULL, 0, fdata->requested_ref); } else { @@ -2224,7 +2247,7 @@ on_superblock_fetched (GObject *src, delta_superblock_data, FALSE)); g_ptr_array_add (pull_data->static_delta_superblocks, g_variant_ref (delta_superblock)); - if (!process_one_static_delta (pull_data, from_revision, to_revision, delta_superblock, + if (!process_one_static_delta (pull_data, from_revision, to_revision, delta_superblock, fdata->requested_ref, pull_data->cancellable, error)) goto out; } @@ -2232,6 +2255,8 @@ on_superblock_fetched (GObject *src, out: g_free (fdata->from_revision); g_free (fdata->to_revision); + if (fdata->requested_ref) + ostree_collection_ref_free (fdata->requested_ref); g_free (fdata); g_assert (pull_data->n_outstanding_metadata_fetches > 0); pull_data->n_outstanding_metadata_fetches--; @@ -2774,7 +2799,8 @@ reinitialize_fetcher (OtPullData *pull_data, const char *remote_name, GError **e static void initiate_delta_request (OtPullData *pull_data, const char *from_revision, - const char *to_revision) + const char *to_revision, + const OstreeCollectionRef *ref) { g_autofree char *delta_name = _ostree_get_relative_static_delta_superblock_path (from_revision, to_revision); @@ -2782,6 +2808,7 @@ initiate_delta_request (OtPullData *pull_data, fdata->pull_data = pull_data; fdata->from_revision = g_strdup (from_revision); fdata->to_revision = g_strdup (to_revision); + fdata->requested_ref = (ref != NULL) ? ostree_collection_ref_dup (ref) : NULL; _ostree_fetcher_request_to_membuf (pull_data->fetcher, pull_data->content_mirrorlist, @@ -2815,7 +2842,7 @@ initiate_request (OtPullData *pull_data, /* Are deltas disabled? OK, just start an object fetch and be done */ if (pull_data->disable_static_deltas) { - queue_scan_one_metadata_object (pull_data, to_revision, OSTREE_OBJECT_TYPE_COMMIT, NULL, 0); + queue_scan_one_metadata_object (pull_data, to_revision, OSTREE_OBJECT_TYPE_COMMIT, NULL, 0, ref); return TRUE; } @@ -2831,16 +2858,16 @@ initiate_request (OtPullData *pull_data, return FALSE; if (delta_from_revision) /* Did we find a delta FROM commit? */ - initiate_delta_request (pull_data, delta_from_revision, to_revision); + initiate_delta_request (pull_data, delta_from_revision, to_revision, ref); else if (have_scratch_delta) /* No delta FROM, do we have a scratch? */ - initiate_delta_request (pull_data, NULL, to_revision); + initiate_delta_request (pull_data, NULL, to_revision, ref); else if (pull_data->require_static_deltas) /* No deltas found; are they required? */ { set_required_deltas_error (error, (ref != NULL) ? ref->ref_name : "", to_revision); return FALSE; } else /* No deltas, fall back to object fetches. */ - queue_scan_one_metadata_object (pull_data, to_revision, OSTREE_OBJECT_TYPE_COMMIT, NULL, 0); + queue_scan_one_metadata_object (pull_data, to_revision, OSTREE_OBJECT_TYPE_COMMIT, NULL, 0, ref); } else if (ref != NULL) { @@ -2874,14 +2901,14 @@ initiate_request (OtPullData *pull_data, /* This is similar to the below, except we *might* use the previous * commit, or we might do a scratch delta first. */ - initiate_delta_request (pull_data, delta_from_revision ?: NULL, to_revision); + initiate_delta_request (pull_data, delta_from_revision ?: NULL, to_revision, ref); } else { /* Legacy path without a summary file - let's try a scratch delta, if that * doesn't work, it'll drop down to object requests. */ - initiate_delta_request (pull_data, NULL, to_revision); + initiate_delta_request (pull_data, NULL, to_revision, NULL); } return TRUE; From cc9a0386c46bbeca8f1ce80239eb00ad6d792de8 Mon Sep 17 00:00:00 2001 From: Krzesimir Nowak Date: Thu, 22 Jun 2017 22:42:30 +0200 Subject: [PATCH 04/35] lib/pull: Collection and ref bindings verification This verifies the collection and ref bindings in the commit metadata against the collection ID we have stored in the remote config and ref we want to pull from. For the HEAD commits, we also check if the checksum of the commit we just fetched agrees with the checksum we really wanted to pull from the ref. For commits with explicitly specified checksums and without specified refs, we only verify if the commit has the bindings. We are able to only verify the collection binding, though. Closes: #972 Approved by: cgwalters --- src/libostree/ostree-repo-pull.c | 150 ++++++++++++++++++++++++++++++- 1 file changed, 147 insertions(+), 3 deletions(-) diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 49ac91da..5e0ae6ec 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -1369,6 +1369,142 @@ commitstate_is_partial (OtPullData *pull_data, || (commitstate & OSTREE_REPO_COMMIT_STATE_PARTIAL) > 0; } +#ifdef OSTREE_ENABLE_EXPERIMENTAL_API +/* Reads the collection-id of a given remote from the repo + * configuration. + */ +static char * +get_real_remote_repo_collection_id (OstreeRepo *repo, + const gchar *remote_name) +{ + g_autofree gchar *remote_collection_id = NULL; + if (!ostree_repo_get_remote_option (repo, remote_name, "collection-id", NULL, + &remote_collection_id, NULL) || + (remote_collection_id == NULL) || + (remote_collection_id[0] == '\0')) + return NULL; + + return g_steal_pointer (&remote_collection_id); +} + +/* Reads the collection-id of the remote repo. Where it will be read + * from depends on whether we pull from the "local" remote repo (the + * "file://" URL) or "remote" remote repo (likely the "http(s)://" + * URL). + */ +static char * +get_remote_repo_collection_id (OtPullData *pull_data) +{ + if (pull_data->remote_repo_local != NULL) + { + const char *remote_collection_id = + ostree_repo_get_collection_id (pull_data->remote_repo_local); + if ((remote_collection_id == NULL) || + (remote_collection_id[0] == '\0')) + return NULL; + return g_strdup (remote_collection_id); + } + + return get_real_remote_repo_collection_id (pull_data->repo, + pull_data->remote_name); +} +#endif /* OSTREE_ENABLE_EXPERIMENTAL_API */ + +/* Verify the ref and collection bindings. + * + * The ref binding is verified only if it exists. But if we have the + * collection ID specified in the remote configuration then the ref + * binding must exist, otherwise the verification will fail. Parts of + * the verification can be skipped by passing NULL to the requested_ref + * parameter (in case we requested a checksum directly, without looking it up + * from a ref). + * + * The collection binding is verified only when we have collection ID + * specified in the remote configuration. If it is specified, then the + * binding must exist and must be equal to the remote repository + * collection ID. + */ +static gboolean +verify_bindings (OtPullData *pull_data, + GVariant *commit, + const OstreeCollectionRef *requested_ref, + GError **error) +{ + g_autofree char *remote_collection_id = NULL; +#ifdef OSTREE_ENABLE_EXPERIMENTAL_API + remote_collection_id = get_remote_repo_collection_id (pull_data); +#endif /* OSTREE_ENABLE_EXPERIMENTAL_API */ + g_autoptr(GVariant) metadata = g_variant_get_child_value (commit, 0); + g_autofree const char **refs = NULL; + if (!g_variant_lookup (metadata, + OSTREE_REF_BINDING, + "^a&s", + &refs)) + { + /* Early return here - if the remote collection ID is NULL, then + * we certainly will not verify the collection binding in the + * commit. + */ + if (remote_collection_id == NULL) + return TRUE; + + return glnx_throw (error, + "expected commit metadata to have ref " + "binding information, found none"); + } + + if (requested_ref != NULL) + { + if (!g_strv_contains ((const char *const *) refs, requested_ref->ref_name)) + { + g_autoptr(GString) refs_dump = g_string_new (NULL); + const char *refs_str; + + if (refs != NULL && (*refs) != NULL) + { + for (const char **iter = refs; *iter != NULL; ++iter) + { + const char *ref = *iter; + + if (refs_dump->len > 0) + g_string_append (refs_dump, ", "); + g_string_append_printf (refs_dump, "‘%s’", ref); + } + + refs_str = refs_dump->str; + } + else + { + refs_str = "no refs"; + } + + return glnx_throw (error, "commit has no requested ref ‘%s’ " + "in ref binding metadata (%s)", + requested_ref->ref_name, refs_str); + } + } + + if (remote_collection_id != NULL) + { + const char *collection_id; + if (!g_variant_lookup (metadata, + OSTREE_COLLECTION_BINDING, + "&s", + &collection_id)) + return glnx_throw (error, + "expected commit metadata to have collection ID " + "binding information, found none"); + if (!g_str_equal (collection_id, remote_collection_id)) + return glnx_throw (error, + "commit has collection ID ‘%s’ in collection binding " + "metadata, while the remote it came from has " + "collection ID ‘%s’", + collection_id, remote_collection_id); + } + + return TRUE; +} + static gboolean scan_commit_object (OtPullData *pull_data, const char *checksum, @@ -1418,6 +1554,15 @@ scan_commit_object (OtPullData *pull_data, if (!ostree_repo_load_commit (pull_data->repo, checksum, &commit, &commitstate, error)) goto out; + /* If ref is non-NULL then the commit we fetched was requested through the + * branch, otherwise we requested a commit checksum without specifying a branch. + */ + if (!verify_bindings (pull_data, commit, ref, error)) + { + g_prefix_error (error, "Commit %s: ", checksum); + goto out; + } + /* If we found a legacy transaction flag, assume all commits are partial */ is_partial = commitstate_is_partial (pull_data, commitstate); @@ -5124,9 +5269,8 @@ check_remote_matches_collection_id (OstreeRepo *repo, { g_autofree gchar *remote_collection_id = NULL; - if (!ostree_repo_get_remote_option (repo, remote_name, "collection-id", NULL, - &remote_collection_id, NULL) || - remote_collection_id == NULL) + remote_collection_id = get_real_remote_repo_collection_id (repo, remote_name); + if (remote_collection_id == NULL) return FALSE; return g_str_equal (remote_collection_id, collection_id); From 7fa534ac17370d4dc46c222aaa5d3f1e9e3fdd50 Mon Sep 17 00:00:00 2001 From: Krzesimir Nowak Date: Wed, 28 Jun 2017 12:46:02 +0200 Subject: [PATCH 05/35] tests: New tests for creating commits with bindings and pulling them Closes: #972 Approved by: cgwalters --- Makefile-tests.am | 1 + tests/test-pull-collections.sh | 246 +++++++++++++++++++++++++++++++++ 2 files changed, 247 insertions(+) create mode 100755 tests/test-pull-collections.sh diff --git a/Makefile-tests.am b/Makefile-tests.am index 444b6636..b4ec44d6 100644 --- a/Makefile-tests.am +++ b/Makefile-tests.am @@ -122,6 +122,7 @@ _installed_or_uninstalled_test_scripts += \ tests/test-refs-collections.sh \ tests/test-remote-add-collections.sh \ tests/test-summary-collections.sh \ + tests/test-pull-collections.sh \ $(NULL) endif diff --git a/tests/test-pull-collections.sh b/tests/test-pull-collections.sh new file mode 100755 index 00000000..cc50a3a1 --- /dev/null +++ b/tests/test-pull-collections.sh @@ -0,0 +1,246 @@ +#!/bin/bash +# +# Copyright © 2017 Endless Mobile, Inc. +# +# 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 + +. $(dirname $0)/libtest.sh + +echo '1..6' + +cd ${test_tmpdir} + +do_commit() { + local repo=$1 + local branch=$2 + shift 2 + + mkdir -p files + pushd files + ${CMD_PREFIX} ostree --repo="../${repo}" commit -s "Test ${repo} commit for branch ${branch}" -b "${branch}" --gpg-homedir="${TEST_GPG_KEYHOME}" --gpg-sign="${TEST_GPG_KEYID_1}" "$@" > "../${branch}-checksum" + popd +} + +do_summary() { + local repo=$1 + shift 1 + + ${CMD_PREFIX} ostree "--repo=${repo}" summary --update --gpg-homedir="${TEST_GPG_KEYHOME}" --gpg-sign="${TEST_GPG_KEYID_1}" +} + +do_collection_ref_show() { + local repo=$1 + local branch=$2 + shift 2 + + echo -n "collection ID: " + if ${CMD_PREFIX} ostree "--repo=${repo}" show --print-metadata-key=ostree.collection-binding $(cat "${branch}-checksum") + then : + else return 1 + fi + echo -n "refs: " + if ${CMD_PREFIX} ostree "--repo=${repo}" show --print-metadata-key=ostree.ref-binding $(cat "${branch}-checksum") + then return 0 + else return 1 + fi +} + +ensure_no_collection_ref() { + local repo=$1 + local branch=$2 + local error=$3 + shift 3 + + if do_collection_ref_show "${repo}" "${branch}" >/dev/null + then + assert_not_reached "${error}" + fi +} + +do_join() { + local IFS=', ' + echo "$*" +} + +ensure_collection_ref() { + local repo=$1 + local branch=$2 + local collection_id=$3 + shift 3 + local refs=$(do_join "$@") + + do_collection_ref_show "${repo}" "${branch}" >"${branch}-meta" + assert_file_has_content "${branch}-meta" "^collection ID: '${collection_id}'$" + assert_file_has_content "${branch}-meta" "^refs: \['${refs}'\]$" +} + +do_remote_add() { + local repo=$1 + local remote_repo=$2 + shift 2 + + ${CMD_PREFIX} ostree "--repo=${repo}" remote add "${remote_repo}-remote" "file://$(pwd)/${remote_repo}" "$@" --gpg-import="${test_tmpdir}/gpghome/key1.asc" +} + +do_pull() { + local repo=$1 + local remote_repo=$2 + local branch=$3 + shift 3 + + if ${CMD_PREFIX} ostree "--repo=${repo}" pull "${remote_repo}-remote" "${branch}" + then return 0 + else return 1 + fi +} + +do_local_pull() { + local repo=$1 + local remote_repo=$2 + local branch=$3 + shift 3 + + if ${CMD_PREFIX} ostree "--repo=${repo}" pull-local "${remote_repo}" "${branch}" + then return 0 + else return 1 + fi +} + +# Create a repo without the collection ID. +mkdir no-collection-repo +ostree_repo_init no-collection-repo +do_commit no-collection-repo goodncref1 +do_commit no-collection-repo sortofbadncref1 --add-metadata-string=ostree.collection-binding=org.example.Ignored +do_summary no-collection-repo +ensure_no_collection_ref \ + no-collection-repo \ + goodncref1 \ + "commits in repository without collection ID shouldn't normally contain the ostree.commit.collection metadata information" +ensure_collection_ref no-collection-repo sortofbadncref1 'org.example.Ignored' 'sortofbadncref1' + +echo "ok 1 setup remote repo without collection ID" + +# Create a repo with a collection ID. +mkdir collection-repo +ostree_repo_init collection-repo +do_commit collection-repo badcref1 # has no collection ref +# We set the repo collection ID in this hacky way to get the commit +# without the collection ID. +echo "collection-id=org.example.CollectionRepo" >>collection-repo/config +do_commit collection-repo badcref2 --add-metadata-string=ostree.collection-binding=org.example.Whatever +do_commit collection-repo badcref3 --add-metadata-string=ostree.collection-binding= +do_commit collection-repo goodcref1 +# create a badcref4 ref with a commit that has goodcref1 in its collection ref metadata +${CMD_PREFIX} ostree --repo=collection-repo refs --create=badcref4 $(cat goodcref1-checksum) +do_summary collection-repo +ensure_no_collection_ref \ + collection-repo \ + badcref1 \ + "commit in badcref1 should not have the collection ref metadata information" +ensure_collection_ref collection-repo badcref2 'org.example.Whatever' 'badcref2' +ensure_collection_ref collection-repo badcref3 '' 'badcref3' +ensure_collection_ref collection-repo goodcref1 'org.example.CollectionRepo' 'goodcref1' + +echo "ok 2 setup remote repo with collection ID" + +# Create a local repo without the collection ID. +mkdir no-collection-local-repo +ostree_repo_init no-collection-local-repo +do_commit no-collection-local-repo goodnclref1 +do_commit no-collection-local-repo sortofbadnclref1 --add-metadata-string=ostree.collection-binding=org.example.IgnoredLocal +do_summary no-collection-local-repo +ensure_no_collection_ref \ + no-collection-local-repo \ + goodnclref1 \ + "commits in repository without collection ID shouldn't normally contain the ostree.commit.collection metadata information" +ensure_collection_ref no-collection-local-repo sortofbadnclref1 'org.example.IgnoredLocal' 'sortofbadnclref1' + +echo "ok 3 setup local repo without collection ID" + +# Create a local repo with a collection ID. +mkdir collection-local-repo +ostree_repo_init collection-local-repo +do_commit collection-local-repo badclref1 # has no collection ref +# We set the repo collection ID in this hacky way to get the commit +# without the collection ID. +echo "collection-id=org.example.CollectionRepoLocal" >>collection-local-repo/config +do_commit collection-local-repo badclref2 --add-metadata-string=ostree.collection-binding=org.example.WhateverLocal +do_commit collection-local-repo badclref3 --add-metadata-string=ostree.collection-binding= +do_commit collection-local-repo goodclref1 +# create a badclref4 ref with a commit that has goodclref1 in its collection ref metadata +${CMD_PREFIX} ostree --repo=collection-local-repo refs --create=badclref4 $(cat goodclref1-checksum) +do_summary collection-local-repo +ensure_no_collection_ref \ + collection-local-repo \ + badclref1 \ + "commit in badclref1 should not have the collection ref metadata information" +ensure_collection_ref collection-local-repo badclref2 'org.example.WhateverLocal' 'badclref2' +ensure_collection_ref collection-local-repo badclref3 '' 'badclref3' +ensure_collection_ref collection-local-repo goodclref1 'org.example.CollectionRepoLocal' 'goodclref1' + +echo "ok 4 setup local repo with collection ID" + +# Create a local repository where we pull the branches from the remotes as normal, using GPG. +mkdir local +ostree_repo_init local +do_remote_add local collection-repo --collection-id org.example.CollectionRepo +do_remote_add local no-collection-repo + +do_pull local no-collection-repo goodncref1 +do_pull local no-collection-repo sortofbadncref1 +if do_pull local collection-repo badcref1 +then + assert_not_reached "pulling a commit without collection ID from a repo with collection ID should fail" +fi +if do_pull local collection-repo badcref2 +then + assert_not_reached "pulling a commit with a mismatched collection ID from a repo with collection ID should fail" +fi +if do_pull local collection-repo badcref3 +then + assert_not_reached "pulling a commit with empty collection ID from repo with collection ID should fail" +fi +do_pull local collection-repo goodcref1 +if do_pull local collection-repo badcref4 +then + assert_not_reached "pulling a commit that was not requested from repo with collection ID should fail" +fi + +echo "ok 5 pull refs from remote repos" + +do_local_pull local no-collection-local-repo goodnclref1 +do_local_pull local no-collection-local-repo sortofbadnclref1 +if do_local_pull local collection-local-repo badclref1 +then + assert_not_reached "pulling a commit without collection ID from a repo with collection ID should fail" +fi +if do_local_pull local collection-local-repo badclref2 +then + assert_not_reached "pulling a commit with a mismatched collection ID from a repo with collection ID should fail" +fi +if do_local_pull local collection-local-repo badclref3 +then + assert_not_reached "pulling a commit with empty collection ID from repo with collection ID should fail" +fi +do_local_pull local collection-local-repo goodclref1 +if do_local_pull local collection-local-repo badclref4 +then + assert_not_reached "pulling a commit that was not requested from repo with collection ID should fail" +fi + +echo "ok 6 pull refs from local repos" From d2a05e5a090f446e0738dafd93299177d6d02f79 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 6 Jul 2017 17:04:13 -0400 Subject: [PATCH 06/35] deploy: Port some functions to new style There are a number of simple ports here. Prep for further work in `/etc` merge. I also stripped trailing whitespace globally. Closes: #996 Approved by: jlebon --- src/libostree/ostree-sysroot-deploy.c | 187 +++++++++----------------- 1 file changed, 62 insertions(+), 125 deletions(-) diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index a1584f8c..e3db3691 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -52,34 +52,24 @@ symlink_at_replace (const char *oldpath, GCancellable *cancellable, GError **error) { - gboolean ret = FALSE; - int res; /* Possibly in the future generate a temporary random name here, * would need to move "generate a temporary name" code into * libglnx or glib? */ g_autofree char *temppath = g_strconcat (newpath, ".tmp", NULL); - /* Clean up any stale temporary links */ + /* Clean up any stale temporary links */ (void) unlinkat (parent_dfd, temppath, 0); - /* Create the temp link */ - do - res = symlinkat (oldpath, parent_dfd, temppath); - while (G_UNLIKELY (res == -1 && errno == EINTR)); - if (res == -1) - { - glnx_set_error_from_errno (error); - goto out; - } + /* Create the temp link */ + if (TEMP_FAILURE_RETRY (symlinkat (oldpath, parent_dfd, temppath)) < 0) + return glnx_throw_errno_prefix (error, "symlinkat"); /* Rename it into place */ if (!glnx_renameat (parent_dfd, temppath, parent_dfd, newpath, error)) - goto out; + return FALSE; - ret = TRUE; - out: - return ret; + return TRUE; } static GLnxFileCopyFlags @@ -104,26 +94,17 @@ hardlink_or_copy_at (int src_dfd, GCancellable *cancellable, GError **error) { - gboolean ret = FALSE; - if (linkat (src_dfd, src_subpath, dest_dfd, dest_subpath, 0) != 0) { - if (errno == EMLINK || errno == EXDEV) - { - return glnx_file_copy_at (src_dfd, src_subpath, NULL, dest_dfd, dest_subpath, - sysroot_flags_to_copy_flags (0, flags), - cancellable, error); - } + if (G_IN_SET (errno, EMLINK, EXDEV)) + return glnx_file_copy_at (src_dfd, src_subpath, NULL, dest_dfd, dest_subpath, + sysroot_flags_to_copy_flags (0, flags), + cancellable, error); else - { - glnx_set_error_from_errno (error); - goto out; - } + return glnx_throw_errno_prefix (error, "linkat"); } - ret = TRUE; - out: - return ret; + return TRUE; } static gboolean @@ -135,8 +116,6 @@ dirfd_copy_attributes_and_xattrs (int src_parent_dfd, GCancellable *cancellable, GError **error) { - gboolean ret = FALSE; - struct stat src_stbuf; g_autoptr(GVariant) xattrs = NULL; /* Clone all xattrs first, so we get the SELinux security context @@ -147,31 +126,21 @@ dirfd_copy_attributes_and_xattrs (int src_parent_dfd, { if (!glnx_dfd_name_get_all_xattrs (src_parent_dfd, src_name, &xattrs, cancellable, error)) - goto out; + return FALSE; if (!glnx_fd_set_all_xattrs (dest_dfd, xattrs, cancellable, error)) - goto out; + return FALSE; } - if (fstat (src_dfd, &src_stbuf) != 0) - { - glnx_set_error_from_errno (error); - goto out; - } + struct stat src_stbuf; + if (!glnx_fstat (src_dfd, &src_stbuf, error)) + return FALSE; if (fchown (dest_dfd, src_stbuf.st_uid, src_stbuf.st_gid) != 0) - { - glnx_set_error_from_errno (error); - goto out; - } + return glnx_throw_errno_prefix (error, "fchown"); if (fchmod (dest_dfd, src_stbuf.st_mode) != 0) - { - glnx_set_error_from_errno (error); - goto out; - } + return glnx_throw_errno_prefix (error, "fchmod"); - ret = TRUE; - out: - return ret; + return TRUE; } static gboolean @@ -199,7 +168,7 @@ copy_dir_recurse (int src_parent_dfd, if (!dirfd_copy_attributes_and_xattrs (src_parent_dfd, name, src_dfd_iter.fd, dest_dfd, flags, cancellable, error)) return FALSE; - + while (TRUE) { struct stat child_stbuf; @@ -242,7 +211,6 @@ ensure_directory_from_template (int orig_etc_fd, GCancellable *cancellable, GError **error) { - gboolean ret = FALSE; glnx_fd_close int src_dfd = -1; glnx_fd_close int target_dfd = -1; @@ -250,7 +218,7 @@ ensure_directory_from_template (int orig_etc_fd, g_assert (*path != '/' && *path != '\0'); if (!glnx_opendirat (modified_etc_fd, path, TRUE, &src_dfd, error)) - goto out; + return FALSE; /* Create with mode 0700, we'll fchmod/fchown later */ again: @@ -268,7 +236,7 @@ ensure_directory_from_template (int orig_etc_fd, { if (!ensure_directory_from_template (orig_etc_fd, modified_etc_fd, new_etc_fd, parent_path, NULL, flags, cancellable, error)) - goto out; + return FALSE; /* Loop */ goto again; @@ -280,29 +248,19 @@ ensure_directory_from_template (int orig_etc_fd, } } else - { - glnx_set_error_from_errno (error); - g_prefix_error (error, "mkdirat: "); - goto out; - } + return glnx_throw_errno_prefix (error, "mkdirat"); } if (!glnx_opendirat (new_etc_fd, path, TRUE, &target_dfd, error)) - goto out; + return FALSE; if (!dirfd_copy_attributes_and_xattrs (modified_etc_fd, path, src_dfd, target_dfd, flags, cancellable, error)) - goto out; + return FALSE; - ret = TRUE; if (out_dfd) - { - g_assert (target_dfd != -1); - *out_dfd = target_dfd; - target_dfd = -1; - } - out: - return ret; + *out_dfd = glnx_steal_fd (&target_dfd); + return TRUE; } /** @@ -321,100 +279,79 @@ copy_modified_config_file (int orig_etc_fd, GCancellable *cancellable, GError **error) { - gboolean ret = FALSE; struct stat modified_stbuf; struct stat new_stbuf; + + if (!glnx_fstatat (modified_etc_fd, path, &modified_stbuf, AT_SYMLINK_NOFOLLOW, error)) + return glnx_prefix_error (error, "Reading modified config file"); + glnx_fd_close int dest_parent_dfd = -1; - - if (fstatat (modified_etc_fd, path, &modified_stbuf, AT_SYMLINK_NOFOLLOW) < 0) - { - glnx_set_error_from_errno (error); - g_prefix_error (error, "Failed to read modified config file '%s': ", path); - goto out; - } - if (strchr (path, '/') != NULL) { g_autofree char *parent = g_path_get_dirname (path); if (!ensure_directory_from_template (orig_etc_fd, modified_etc_fd, new_etc_fd, parent, &dest_parent_dfd, flags, cancellable, error)) - goto out; + return FALSE; } else { dest_parent_dfd = dup (new_etc_fd); if (dest_parent_dfd == -1) - { - glnx_set_error_from_errno (error); - goto out; - } + return glnx_throw_errno_prefix (error, "dup"); } g_assert (dest_parent_dfd != -1); if (fstatat (new_etc_fd, path, &new_stbuf, AT_SYMLINK_NOFOLLOW) < 0) { - if (errno == ENOENT) - ; - else - { - glnx_set_error_from_errno (error); - goto out; - } + if (errno != ENOENT) + return glnx_throw_errno_prefix (error, "fstatat"); } else if (S_ISDIR(new_stbuf.st_mode)) { if (!S_ISDIR(modified_stbuf.st_mode)) { - g_set_error (error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND, - "Modified config file newly defaults to directory '%s', cannot merge", - path); - goto out; + return g_set_error (error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND, + "Modified config file newly defaults to directory '%s', cannot merge", + path), FALSE; } else { /* Do nothing here - we assume that we've already * recursively copied the parent directory. */ - ret = TRUE; - goto out; + return TRUE; } } else { - if (unlinkat (new_etc_fd, path, 0) < 0) - { - glnx_set_error_from_errno (error); - goto out; - } + if (!glnx_unlinkat (new_etc_fd, path, 0, error)) + return FALSE; } if (S_ISDIR (modified_stbuf.st_mode)) { if (!copy_dir_recurse (modified_etc_fd, new_etc_fd, path, flags, cancellable, error)) - goto out; + return FALSE; } else if (S_ISLNK (modified_stbuf.st_mode) || S_ISREG (modified_stbuf.st_mode)) { - if (!glnx_file_copy_at (modified_etc_fd, path, &modified_stbuf, + if (!glnx_file_copy_at (modified_etc_fd, path, &modified_stbuf, new_etc_fd, path, sysroot_flags_to_copy_flags (GLNX_FILE_COPY_OVERWRITE, flags), cancellable, error)) - goto out; + return FALSE; } else { - g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, - "Unsupported non-regular/non-symlink file in /etc '%s'", - path); - goto out; + return glnx_throw (error, + "Unsupported non-regular/non-symlink file in /etc '%s'", + path); } - ret = TRUE; - out: - return ret; + return TRUE; } /* @@ -575,7 +512,7 @@ checkout_deployment_tree (OstreeSysroot *sysroot, if (!glnx_opendirat (osdeploy_dfd, checkout_target_name, TRUE, &ret_fd, error)) goto out; - + ret = TRUE; *out_deployment_dfd = ret_fd; out: @@ -652,7 +589,7 @@ relabel_recursively (OstreeSysroot *sysroot, cancellable, error); if (!direnum) goto out; - + while (TRUE) { GFileInfo *file_info; @@ -706,7 +643,7 @@ selinux_relabel_dir (OstreeSysroot *sysroot, cancellable, error); if (!root_info) goto out; - + g_ptr_array_add (path_parts, (char*)prefix); if (!relabel_recursively (sysroot, sepolicy, dir, root_info, path_parts, cancellable, error)) @@ -959,7 +896,7 @@ get_kernel_from_tree (int deployment_dfd, if (!glnx_dirfd_iterator_next_dent (&dfditer, &dent, cancellable, error)) goto out; - + if (dent == NULL) break; @@ -983,7 +920,7 @@ get_kernel_from_tree (int deployment_dfd, ret_initramfs_name = g_strdup (dent->d_name); } } - + if (ret_kernel_name != NULL && ret_initramfs_name != NULL) break; } @@ -1451,7 +1388,7 @@ static GHashTable * assign_bootserials (GPtrArray *deployments) { guint i; - GHashTable *ret = + GHashTable *ret = g_hash_table_new_full (g_str_hash, g_str_equal, NULL, NULL); for (i = 0; i < deployments->len; i++) @@ -1488,7 +1425,7 @@ deployment_bootconfigs_equal (OstreeDeployment *a, __attribute__((cleanup(_ostree_kernel_args_cleanup))) OstreeKernelArgs *b_kargs = NULL; g_autofree char *a_boot_options_without_ostree = NULL; g_autofree char *b_boot_options_without_ostree = NULL; - + /* We checksum the kernel arguments *except* ostree= */ a_kargs = _ostree_kernel_args_from_string (a_boot_options); _ostree_kernel_args_replace (a_kargs, "ostree"); @@ -1662,10 +1599,10 @@ ostree_sysroot_write_deployments_with_options (OstreeSysroot *self, { OstreeDeployment *deployment = new_deployments->pdata[i]; g_autoptr(GFile) deployment_root = NULL; - + if (deployment == self->booted_deployment) found_booted_deployment = TRUE; - + deployment_root = ostree_sysroot_get_deployment_directory (self, deployment); if (!g_file_query_exists (deployment_root, NULL)) { @@ -1693,7 +1630,7 @@ ostree_sysroot_write_deployments_with_options (OstreeSysroot *self, g_prefix_error (error, "Creating new current bootlinks: "); goto out; } - + if (!full_system_sync (self, cancellable, error)) { g_prefix_error (error, "Full sync: "); @@ -1707,7 +1644,7 @@ ostree_sysroot_write_deployments_with_options (OstreeSysroot *self, g_prefix_error (error, "Swapping current bootlinks: "); goto out; } - + bootloader_is_atomic = TRUE; } else @@ -1741,7 +1678,7 @@ ostree_sysroot_write_deployments_with_options (OstreeSysroot *self, if (!glnx_shutil_mkdir_p_at (self->sysroot_fd, new_loader_entries_dir, 0755, cancellable, error)) goto out; - + /* Need the repo to try and extract the versions for deployments. * But this is a "nice-to-have" for the bootloader UI, so failure * here is not fatal to the whole operation. We just gracefully @@ -1819,7 +1756,7 @@ ostree_sysroot_write_deployments_with_options (OstreeSysroot *self, g_prefix_error (error, "Full sync: "); goto out; } - + if (!swap_bootloader (self, self->bootversion, new_bootversion, cancellable, error)) { From 9d941dcebbbbe0cab1f40adf3269a1562760f9ac Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 6 Jul 2017 21:33:59 -0400 Subject: [PATCH 07/35] checkout: Don't set dir mtime to 0 when doing a force copy checkout When we [switched to using checkout + force_copy](https://github.com/ostreedev/ostree/commit/e8efd1c8dcaad8fbd3b05c400972d237406263e7), a side effect that went unnoticed at the time is that we started setting directory mtimes to zero. See the below bug where we long ago set the file times to zero, which got fixed, so let's not regress things by setting the directory times to zero either. (Even though AFAICS GNU tar doesn't complain about those) This semantic is somewhat "overloaded" onto `force_copy`, but it avoids adding yet another boolean; we don't have that many reserved boolean slots left. I can't really think of many good use cases for `force_copy` *other* than the `/etc` merge anyways. https://bugzilla.redhat.com/show_bug.cgi?id=1229160 Closes: https://github.com/ostreedev/ostree/issues/995 Closes: #997 Approved by: jlebon --- src/libostree/ostree-repo-checkout.c | 6 ++++-- tests/installed/itest-deploy-selinux.sh | 6 ++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/libostree/ostree-repo-checkout.c b/src/libostree/ostree-repo-checkout.c index 9ab9a623..7942d607 100644 --- a/src/libostree/ostree-repo-checkout.c +++ b/src/libostree/ostree-repo-checkout.c @@ -770,9 +770,11 @@ checkout_tree_at_recurse (OstreeRepo *self, } /* Set directory mtime to OSTREE_TIMESTAMP, so that it is constant for all checkouts. - * Must be done after setting permissions and creating all children. + * Must be done after setting permissions and creating all children. Note we skip doing + * this for directories that already exist (under the theory we possibly don't own them), + * and we also skip it if doing copying checkouts, which is mostly for /etc. */ - if (!did_exist) + if (!did_exist && !options->force_copy) { const struct timespec times[2] = { { OSTREE_TIMESTAMP, UTIME_OMIT }, { OSTREE_TIMESTAMP, 0} }; if (TEMP_FAILURE_RETRY (futimens (destination_dfd, times)) < 0) diff --git a/tests/installed/itest-deploy-selinux.sh b/tests/installed/itest-deploy-selinux.sh index 7c26aad0..f490b69d 100755 --- a/tests/installed/itest-deploy-selinux.sh +++ b/tests/installed/itest-deploy-selinux.sh @@ -11,6 +11,12 @@ dn=$(dirname $0) ostree admin deploy --karg-proc-cmdline ${host_refspec} new_deployment_path=/ostree/deploy/${host_osname}/deploy/${host_commit}.1 +# Test /etc directory mtime +if ! test ${new_deployment_path}/etc/NetworkManager -nt /etc/NetworkManager; then + ls -al ${new_deployment_path}/etc/NetworkManager /etc/NetworkManager + fatal "/etc directory mtime not newer" +fi + # A set of files that have a variety of security contexts for file in fstab passwd exports hostname sysctl.conf yum.repos.d \ NetworkManager/dispatcher.d/hook-network-manager; do From 0aa20df20e1915c405750bc00995b130cfc3eb75 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sun, 9 Jul 2017 21:46:23 -0400 Subject: [PATCH 08/35] tests: Run pull tests for bare/bare-user We have variants of `test-basic` for all 4 modes, but not for pull-test, which for some reason was named `pull-archive`, but mostly pulls *into* bare repos. The test code was structured like the basic one where it called into a `pull-test.sh`, so let's actually use it for 2/3 bare modes. (I tried to extend it to `bare-user-only` but it failed, going to look at that after this). This is related to https://github.com/ostreedev/ostree/issues/991 Closes: #998 Approved by: jlebon --- Makefile-tests.am | 3 ++- tests/pull-test.sh | 2 +- ...test-pull-archive.sh => test-pull-bare.sh} | 1 + tests/test-pull-bareuser.sh | 27 +++++++++++++++++++ 4 files changed, 31 insertions(+), 2 deletions(-) rename tests/{test-pull-archive.sh => test-pull-bare.sh} (98%) create mode 100755 tests/test-pull-bareuser.sh diff --git a/Makefile-tests.am b/Makefile-tests.am index b4ec44d6..28e0a43a 100644 --- a/Makefile-tests.am +++ b/Makefile-tests.am @@ -67,7 +67,8 @@ _installed_or_uninstalled_test_scripts = \ tests/test-help.sh \ tests/test-libarchive.sh \ tests/test-parent.sh \ - tests/test-pull-archive.sh \ + tests/test-pull-bare.sh \ + tests/test-pull-bareuser.sh \ tests/test-pull-commit-only.sh \ tests/test-pull-depth.sh \ tests/test-pull-mirror-summary.sh \ diff --git a/tests/pull-test.sh b/tests/pull-test.sh index aaba2a7b..37b5e915 100644 --- a/tests/pull-test.sh +++ b/tests/pull-test.sh @@ -23,7 +23,7 @@ function repo_init() { cd ${test_tmpdir} rm repo -rf mkdir repo - ostree_repo_init repo + ostree_repo_init repo --mode=${repo_mode} ${CMD_PREFIX} ostree --repo=repo remote add origin $(cat httpd-address)/ostree/gnomerepo "$@" } diff --git a/tests/test-pull-archive.sh b/tests/test-pull-bare.sh similarity index 98% rename from tests/test-pull-archive.sh rename to tests/test-pull-bare.sh index d8b0f099..18afbe0b 100755 --- a/tests/test-pull-archive.sh +++ b/tests/test-pull-bare.sh @@ -23,4 +23,5 @@ set -euo pipefail setup_fake_remote_repo1 "archive" +repo_mode=bare . ${test_srcdir}/pull-test.sh diff --git a/tests/test-pull-bareuser.sh b/tests/test-pull-bareuser.sh new file mode 100755 index 00000000..0729247f --- /dev/null +++ b/tests/test-pull-bareuser.sh @@ -0,0 +1,27 @@ +#!/bin/bash +# +# Copyright (C) 2011 Colin Walters +# +# 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 + +. $(dirname $0)/libtest.sh + +setup_fake_remote_repo1 "archive" + +repo_mode=bare-user +. ${test_srcdir}/pull-test.sh From 7d57459e831b562b20e1d3efe99f0dab682d1eac Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Mon, 10 Jul 2017 19:48:52 +0100 Subject: [PATCH 09/35] lib/repo-commit: Fix types of content size cache entries Use goffset rather than gsize for file sizes. More importantly, get the unpacked_size from g_file_info_get_size() (goffset) rather than from the splice return value, which has type gssize. This will make a difference on 32-bit systems, where goffset is defined as off64_t, but gsize is 32 bits. Signed-off-by: Philip Withnall Closes: #999 Approved by: cgwalters --- src/libostree/ostree-repo-commit.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index baeef3f9..55d5b16a 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -299,13 +299,13 @@ commit_loose_regfile_object (OstreeRepo *self, typedef struct { - gsize unpacked; - gsize archived; + goffset unpacked; + goffset archived; } OstreeContentSizeCacheEntry; static OstreeContentSizeCacheEntry * -content_size_cache_entry_new (gsize unpacked, - gsize archived) +content_size_cache_entry_new (goffset unpacked, + goffset archived) { OstreeContentSizeCacheEntry *entry = g_slice_new0 (OstreeContentSizeCacheEntry); @@ -325,8 +325,8 @@ content_size_cache_entry_free (gpointer entry) static void repo_store_size_entry (OstreeRepo *self, const gchar *checksum, - gsize unpacked, - gsize archived) + goffset unpacked, + goffset archived) { if (G_UNLIKELY (self->object_sizes == NULL)) self->object_sizes = g_hash_table_new_full (g_str_hash, g_str_equal, @@ -594,7 +594,7 @@ write_content_object (OstreeRepo *self, */ g_auto(OtCleanupUnlinkat) tmp_unlinker = { self->tmp_dir_fd, NULL }; g_auto(GLnxTmpfile) tmpf = { 0, }; - gssize unpacked_size = 0; + goffset unpacked_size = 0; gboolean indexable = FALSE; /* Is it a symlink physically? */ if (phys_object_is_symlink) @@ -643,10 +643,11 @@ write_content_object (OstreeRepo *self, /* Don't close the base; we'll do that later */ g_filter_output_stream_set_close_base_stream ((GFilterOutputStream*)compressed_out_stream, FALSE); - unpacked_size = g_output_stream_splice (compressed_out_stream, file_input, - 0, cancellable, error); - if (unpacked_size < 0) + if (g_output_stream_splice (compressed_out_stream, file_input, + 0, cancellable, error) < 0) return FALSE; + + unpacked_size = g_file_info_get_size (file_info); } if (!g_output_stream_flush (temp_out, cancellable, error)) From acb14648d7fee86932038a815f11e307e3e13052 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Tue, 11 Jul 2017 20:46:03 +0100 Subject: [PATCH 10/35] lib/repo: Add OSTREE_REPO_METADATA_REF as a well-known metadata store MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As discussed in https://github.com/ostreedev/ostree/pull/946, the summary file is becoming an unsigned cache of ref information; any additional metadata for the repository needs to move elsewhere in order to remain signed. Introduce OSTREE_REPO_METADATA_REF as the well-known name of a ref where such metadata can live, as the metadata on contentless commits. Don’t yet update the documentation for summary-related methods to mention this, since it’s still hidden behind the --enable-experimental-api configure option. Signed-off-by: Philip Withnall Closes: #946 Approved by: cgwalters --- apidoc/ostree-experimental-sections.txt | 1 + src/libostree/ostree-repo.h | 26 +++++++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/apidoc/ostree-experimental-sections.txt b/apidoc/ostree-experimental-sections.txt index 4c71fad2..a2c2c295 100644 --- a/apidoc/ostree-experimental-sections.txt +++ b/apidoc/ostree-experimental-sections.txt @@ -28,6 +28,7 @@ ostree_repo_find_remotes_finish ostree_repo_pull_from_remotes_async ostree_repo_pull_from_remotes_finish ostree_repo_resolve_keyring_for_collection +OSTREE_REPO_METADATA_REF
diff --git a/src/libostree/ostree-repo.h b/src/libostree/ostree-repo.h index ea7a7789..f1e964b7 100644 --- a/src/libostree/ostree-repo.h +++ b/src/libostree/ostree-repo.h @@ -1244,6 +1244,32 @@ gboolean ostree_repo_regenerate_summary (OstreeRepo *self, GCancellable *cancellable, GError **error); +#ifdef OSTREE_ENABLE_EXPERIMENTAL_API + +/** + * OSTREE_REPO_METADATA_REF: + * + * The name of a ref which is used to store metadata for the entire repository, + * such as its expected update time (`ostree.summary.expires`), name, or new + * GPG keys. Metadata is stored on contentless commits in the ref, and hence is + * signed with the commits. + * + * This supersedes the additional metadata dictionary in the `summary` file + * (see ostree_repo_regenerate_summary()), as the use of a ref means that the + * metadata for multiple upstream repositories can be included in a single mirror + * repository, disambiguating the refs using collection IDs. In order to support + * peer to peer redistribution of repository metadata, repositories must set a + * collection ID (ostree_repo_set_collection_id()). + * + * Users of OSTree may place arbitrary metadata in commits on this ref, but the + * keys must be namespaced by product or developer. For example, + * `exampleos.end-of-life`. The `ostree.` prefix is reserved. + * + * Since: 2017.8 + */ +#define OSTREE_REPO_METADATA_REF "ostree-metadata" + +#endif /* OSTREE_ENABLE_EXPERIMENTAL_API */ G_END_DECLS From f6ba20d9d6ce00b1fcaf0a877969554b922f4cd0 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 12 Jul 2017 13:58:16 +0100 Subject: [PATCH 11/35] build: Ensure all .sym files are distributed in tarballs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since we’re using a custom variable for listing the .sym files, automake’s magic support for automatically distributing all files in conditionals doesn’t work, and the devel and experimental .sym files were only being distributed if `make dist` was run on a source tree which had been configured with --enable-experimental-api or not a release flag. Fix that by explicitly listing all the .sym files in EXTRA_DIST. Specifically, this fixes the case of trying to compile with --enable-experimental-api from a release tarball which was disted with --disable-experimental-api. Signed-off-by: Philip Withnall Closes: #1001 Approved by: cgwalters --- Makefile-libostree.am | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Makefile-libostree.am b/Makefile-libostree.am index b589f1b2..fab7bd3f 100644 --- a/Makefile-libostree.am +++ b/Makefile-libostree.am @@ -187,7 +187,11 @@ symbol_files += $(top_srcdir)/src/libostree/libostree-experimental.sym endif # http://blog.jgc.org/2007/06/escaping-comma-and-space-in-gnu-make.html wl_versionscript_arg = -Wl,--version-script= -EXTRA_DIST += $(symbol_files) +EXTRA_DIST += \ + $(top_srcdir)/src/libostree/libostree-devel.sym \ + $(top_srcdir)/src/libostree/libostree-experimental.sym \ + $(top_srcdir)/src/libostree/libostree-released.sym \ + $(NULL) libostree_1_la_CFLAGS = $(AM_CFLAGS) -I$(srcdir)/bsdiff -I$(srcdir)/libglnx -I$(srcdir)/src/libotutil -I$(srcdir)/src/libostree -I$(builddir)/src/libostree \ $(OT_INTERNAL_GIO_UNIX_CFLAGS) $(OT_INTERNAL_GPGME_CFLAGS) $(OT_DEP_LZMA_CFLAGS) $(OT_DEP_ZLIB_CFLAGS) $(OT_DEP_OPENSSL_CFLAGS) \ From 82024f161b280b4ef70880ddd5bc83ad812e6f0a Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 12 Jul 2017 15:04:34 +0100 Subject: [PATCH 12/35] build: Ensure all experimental tests are distributed in tarballs As with the previous commit, ensure that tests which are run when configured with --enable-experimental-api, are always distributed; even when running `make dist` from a source tree configured with --disable-experimental-api. Signed-off-by: Philip Withnall Closes: #1002 Approved by: cgwalters --- Makefile-tests.am | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Makefile-tests.am b/Makefile-tests.am index 28e0a43a..7be2ab59 100644 --- a/Makefile-tests.am +++ b/Makefile-tests.am @@ -114,8 +114,7 @@ _installed_or_uninstalled_test_scripts = \ tests/test-summary-view.sh \ $(NULL) -if ENABLE_EXPERIMENTAL_API -_installed_or_uninstalled_test_scripts += \ +experimental_test_scripts = \ tests/test-find-remotes.sh \ tests/test-fsck-collections.sh \ tests/test-init-collections.sh \ @@ -125,6 +124,11 @@ _installed_or_uninstalled_test_scripts += \ tests/test-summary-collections.sh \ tests/test-pull-collections.sh \ $(NULL) + +if ENABLE_EXPERIMENTAL_API +_installed_or_uninstalled_test_scripts += $(experimental_test_scripts) +else +EXTRA_DIST += $(experimental_test_scripts) endif if BUILDOPT_FUSE From 620a90ebfaf98f779b77cd8f97da1d06e146b832 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 5 Jul 2017 16:41:38 -0400 Subject: [PATCH 13/35] lib/pull: Avoid journaling 404s for optional content Currently in Fedora we don't sign summaries, and every use of `rpm-ostree` would emit to the journal an error when we failed to fetch it. Fix this by having `OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT` tell the fetcher not to journal 404 errors. While fixing this, we had a mix of two booleans vs the flags; fix things so we consistently use the flags in the fetcher and pull code. Closes: #1004 Approved by: mbarnes --- src/libostree/ostree-fetcher-curl.c | 7 +++++-- src/libostree/ostree-fetcher-soup.c | 7 +++++-- src/libostree/ostree-fetcher-util.c | 13 +++++-------- src/libostree/ostree-fetcher-util.h | 6 ++---- src/libostree/ostree-fetcher.h | 10 ++++++---- src/libostree/ostree-metalink.c | 16 ++++------------ src/libostree/ostree-repo-pull.c | 18 ++++++++++-------- 7 files changed, 37 insertions(+), 40 deletions(-) diff --git a/src/libostree/ostree-fetcher-curl.c b/src/libostree/ostree-fetcher-curl.c index 77844ec7..ae8bea73 100644 --- a/src/libostree/ostree-fetcher-curl.c +++ b/src/libostree/ostree-fetcher-curl.c @@ -353,7 +353,9 @@ check_multi_info (OstreeFetcher *fetcher) g_autofree char *msg = g_strdup_printf ("Server returned HTTP %lu", response); g_task_return_new_error (task, G_IO_ERROR, giocode, "%s", msg); - if (req->fetcher->remote_name) + if (req->fetcher->remote_name && + !((req->flags & OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT) > 0 && + giocode == G_IO_ERROR_NOT_FOUND)) _ostree_fetcher_journal_failure (req->fetcher->remote_name, eff_url, msg); @@ -859,13 +861,14 @@ void _ostree_fetcher_request_to_tmpfile (OstreeFetcher *self, GPtrArray *mirrorlist, const char *filename, + OstreeFetcherRequestFlags flags, guint64 max_size, int priority, GCancellable *cancellable, GAsyncReadyCallback callback, gpointer user_data) { - _ostree_fetcher_request_async (self, mirrorlist, filename, 0, FALSE, + _ostree_fetcher_request_async (self, mirrorlist, filename, flags, FALSE, max_size, priority, cancellable, callback, user_data); } diff --git a/src/libostree/ostree-fetcher-soup.c b/src/libostree/ostree-fetcher-soup.c index b877d27c..16fda0a3 100644 --- a/src/libostree/ostree-fetcher-soup.c +++ b/src/libostree/ostree-fetcher-soup.c @@ -1128,7 +1128,9 @@ on_request_sent (GObject *object, g_prefix_error (&local_error, "All %u mirrors failed. Last error was: ", pending->mirrorlist->len); - if (pending->thread_closure->remote_name) + if (pending->thread_closure->remote_name && + !((pending->flags & OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT) > 0 && + code == G_IO_ERROR_NOT_FOUND)) _ostree_fetcher_journal_failure (pending->thread_closure->remote_name, uristring, local_error->message); @@ -1238,13 +1240,14 @@ void _ostree_fetcher_request_to_tmpfile (OstreeFetcher *self, GPtrArray *mirrorlist, const char *filename, + OstreeFetcherRequestFlags flags, guint64 max_size, int priority, GCancellable *cancellable, GAsyncReadyCallback callback, gpointer user_data) { - _ostree_fetcher_request_async (self, mirrorlist, filename, 0, FALSE, + _ostree_fetcher_request_async (self, mirrorlist, filename, flags, FALSE, max_size, priority, cancellable, callback, user_data); } diff --git a/src/libostree/ostree-fetcher-util.c b/src/libostree/ostree-fetcher-util.c index 408b8bcb..7a6d94cc 100644 --- a/src/libostree/ostree-fetcher-util.c +++ b/src/libostree/ostree-fetcher-util.c @@ -55,8 +55,7 @@ gboolean _ostree_fetcher_mirrored_request_to_membuf (OstreeFetcher *fetcher, GPtrArray *mirrorlist, const char *filename, - gboolean add_nul, - gboolean allow_noent, + OstreeFetcherRequestFlags flags, GBytes **out_contents, guint64 max_size, GCancellable *cancellable, @@ -79,7 +78,7 @@ _ostree_fetcher_mirrored_request_to_membuf (OstreeFetcher *fetcher, data.error = error; _ostree_fetcher_request_to_membuf (fetcher, mirrorlist, filename, - add_nul ? OSTREE_FETCHER_REQUEST_NUL_TERMINATION : 0, + flags, max_size, OSTREE_FETCHER_DEFAULT_PRIORITY, cancellable, fetch_uri_sync_on_complete, &data); while (!data.done) @@ -87,7 +86,7 @@ _ostree_fetcher_mirrored_request_to_membuf (OstreeFetcher *fetcher, if (!data.result_buf) { - if (allow_noent) + if (flags & OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT) { if (g_error_matches (*error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND)) { @@ -112,8 +111,7 @@ _ostree_fetcher_mirrored_request_to_membuf (OstreeFetcher *fetcher, gboolean _ostree_fetcher_request_uri_to_membuf (OstreeFetcher *fetcher, OstreeFetcherURI *uri, - gboolean add_nul, - gboolean allow_noent, + OstreeFetcherRequestFlags flags, GBytes **out_contents, guint64 max_size, GCancellable *cancellable, @@ -121,8 +119,7 @@ _ostree_fetcher_request_uri_to_membuf (OstreeFetcher *fetcher, { g_autoptr(GPtrArray) mirrorlist = g_ptr_array_new (); g_ptr_array_add (mirrorlist, uri); /* no transfer */ - return _ostree_fetcher_mirrored_request_to_membuf (fetcher, mirrorlist, NULL, - add_nul, allow_noent, + return _ostree_fetcher_mirrored_request_to_membuf (fetcher, mirrorlist, NULL, flags, out_contents, max_size, cancellable, error); } diff --git a/src/libostree/ostree-fetcher-util.h b/src/libostree/ostree-fetcher-util.h index fe0921cd..c099a872 100644 --- a/src/libostree/ostree-fetcher-util.h +++ b/src/libostree/ostree-fetcher-util.h @@ -29,8 +29,7 @@ G_BEGIN_DECLS gboolean _ostree_fetcher_mirrored_request_to_membuf (OstreeFetcher *fetcher, GPtrArray *mirrorlist, const char *filename, - gboolean add_nul, - gboolean allow_noent, + OstreeFetcherRequestFlags flags, GBytes **out_contents, guint64 max_size, GCancellable *cancellable, @@ -38,8 +37,7 @@ gboolean _ostree_fetcher_mirrored_request_to_membuf (OstreeFetcher *fetcher, gboolean _ostree_fetcher_request_uri_to_membuf (OstreeFetcher *fetcher, OstreeFetcherURI *uri, - gboolean add_nul, - gboolean allow_noent, + OstreeFetcherRequestFlags flags, GBytes **out_contents, guint64 max_size, GCancellable *cancellable, diff --git a/src/libostree/ostree-fetcher.h b/src/libostree/ostree-fetcher.h index 8ec5f209..2dbace78 100644 --- a/src/libostree/ostree-fetcher.h +++ b/src/libostree/ostree-fetcher.h @@ -54,6 +54,11 @@ typedef enum { OSTREE_FETCHER_FLAGS_TRANSFER_GZIP = (1 << 1) } OstreeFetcherConfigFlags; +typedef enum { + OSTREE_FETCHER_REQUEST_NUL_TERMINATION = (1 << 0), + OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT = (1 << 1) +} OstreeFetcherRequestFlags; + void _ostree_fetcher_uri_free (OstreeFetcherURI *uri); G_DEFINE_AUTOPTR_CLEANUP_FUNC(OstreeFetcherURI, _ostree_fetcher_uri_free) @@ -111,6 +116,7 @@ guint64 _ostree_fetcher_bytes_transferred (OstreeFetcher *self); void _ostree_fetcher_request_to_tmpfile (OstreeFetcher *self, GPtrArray *mirrorlist, const char *filename, + OstreeFetcherRequestFlags flags, guint64 max_size, int priority, GCancellable *cancellable, @@ -122,10 +128,6 @@ gboolean _ostree_fetcher_request_to_tmpfile_finish (OstreeFetcher *self, char **out_filename, GError **error); -typedef enum { - OSTREE_FETCHER_REQUEST_NUL_TERMINATION = (1 << 0) -} OstreeFetcherRequestFlags; - void _ostree_fetcher_request_to_membuf (OstreeFetcher *self, GPtrArray *mirrorlist, const char *filename, diff --git a/src/libostree/ostree-metalink.c b/src/libostree/ostree-metalink.c index 6afae59e..0b1b14fb 100644 --- a/src/libostree/ostree-metalink.c +++ b/src/libostree/ostree-metalink.c @@ -431,10 +431,7 @@ try_one_url (OstreeMetalinkRequest *self, gssize n_bytes; if (!_ostree_fetcher_request_uri_to_membuf (self->metalink->fetcher, - uri, - FALSE, - FALSE, - &bytes, + uri, 0, &bytes, self->metalink->max_size, self->cancellable, error)) @@ -614,14 +611,9 @@ _ostree_metalink_request_sync (OstreeMetalink *self, request.urls = g_ptr_array_new_with_free_func ((GDestroyNotify) _ostree_fetcher_uri_free); request.parser = g_markup_parse_context_new (&metalink_parser, G_MARKUP_PREFIX_ERROR_POSITION, &request, NULL); - if (!_ostree_fetcher_request_uri_to_membuf (self->fetcher, - self->uri, - FALSE, - FALSE, - &contents, - self->max_size, - cancellable, - error)) + if (!_ostree_fetcher_request_uri_to_membuf (self->fetcher, self->uri, 0, + &contents, self->max_size, + cancellable, error)) goto out; data = g_bytes_get_data (contents, &len); diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 5e0ae6ec..34ea7f9e 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -470,7 +470,7 @@ fetch_mirrored_uri_contents_utf8_sync (OstreeFetcher *fetcher, { g_autoptr(GBytes) bytes = NULL; if (!_ostree_fetcher_mirrored_request_to_membuf (fetcher, mirrorlist, - filename, TRUE, FALSE, + filename, OSTREE_FETCHER_REQUEST_NUL_TERMINATION, &bytes, OSTREE_MAX_METADATA_SIZE, cancellable, error)) @@ -1864,6 +1864,7 @@ start_fetch (OtPullData *pull_data, else pull_data->n_outstanding_content_fetches++; + OstreeFetcherRequestFlags flags = 0; /* Override the path if we're trying to fetch the .commitmeta file first */ if (fetch->is_detached_meta) { @@ -1871,6 +1872,7 @@ start_fetch (OtPullData *pull_data, _ostree_loose_path (buf, expected_checksum, OSTREE_OBJECT_TYPE_COMMIT_META, pull_data->remote_mode); obj_subpath = g_build_filename ("objects", buf, NULL); mirrorlist = pull_data->meta_mirrorlist; + flags |= OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT; } else { @@ -1890,7 +1892,7 @@ start_fetch (OtPullData *pull_data, expected_max_size = 0; _ostree_fetcher_request_to_tmpfile (pull_data->fetcher, mirrorlist, - obj_subpath, expected_max_size, + obj_subpath, flags, expected_max_size, is_meta ? OSTREE_REPO_PULL_METADATA_PRIORITY : OSTREE_REPO_PULL_CONTENT_PRIORITY, pull_data->cancellable, @@ -1995,7 +1997,7 @@ start_fetch_deltapart (OtPullData *pull_data, g_assert_cmpint (pull_data->n_outstanding_deltapart_fetches, <=, _OSTREE_MAX_OUTSTANDING_DELTAPART_REQUESTS); _ostree_fetcher_request_to_tmpfile (pull_data->fetcher, pull_data->content_mirrorlist, - deltapart_path, fetch->size, + deltapart_path, 0, fetch->size, OSTREE_FETCHER_DEFAULT_PRIORITY, pull_data->cancellable, static_deltapart_fetch_on_complete, @@ -2677,7 +2679,8 @@ _ostree_preload_metadata_file (OstreeRepo *self, else { ret = _ostree_fetcher_mirrored_request_to_membuf (fetcher, mirrorlist, - filename, FALSE, TRUE, + filename, + OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT, out_bytes, OSTREE_MAX_METADATA_SIZE, cancellable, error); @@ -3514,7 +3517,7 @@ ostree_repo_pull_with_options (OstreeRepo *self, { if (!_ostree_fetcher_mirrored_request_to_membuf (pull_data->fetcher, pull_data->meta_mirrorlist, - "summary.sig", FALSE, TRUE, + "summary.sig", OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT, &bytes_sig, OSTREE_MAX_METADATA_SIZE, cancellable, error)) @@ -3538,7 +3541,7 @@ ostree_repo_pull_with_options (OstreeRepo *self, { if (!_ostree_fetcher_mirrored_request_to_membuf (pull_data->fetcher, pull_data->meta_mirrorlist, - "summary", FALSE, TRUE, + "summary", OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT, &bytes_summary, OSTREE_MAX_METADATA_SIZE, cancellable, error)) @@ -4776,8 +4779,7 @@ find_remotes_cb (GObject *obj, if (!_ostree_fetcher_mirrored_request_to_membuf (fetcher, mirrorlist, commit_filename, - FALSE, /* don’t add trailing nul */ - TRUE, /* return NULL on ENOENT */ + OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT, &commit_bytes, 0, /* no maximum size */ cancellable, From 47a54bf876023b0cb457bc8a4f4264f9b2ed5438 Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Mon, 17 Jul 2017 15:44:58 +0100 Subject: [PATCH 14/35] Move the include directive to the enum template There is no actual written guarantee in glib-mkenums that the template line specified using --fhead will be added after the templates specified inside the template file. Since the template file is only used once, we can simply move the `#include` directive inside the template, so that it is guaranteed to be in the right place. Closes: #1007 Approved by: cgwalters --- Makefile-libostree.am | 1 - src/libostree/ostree-enumtypes.c.template | 2 ++ 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/Makefile-libostree.am b/Makefile-libostree.am index fab7bd3f..c83569ff 100644 --- a/Makefile-libostree.am +++ b/Makefile-libostree.am @@ -57,7 +57,6 @@ src/libostree/ostree-enumtypes.h: src/libostree/ostree-enumtypes.h.template $(EN src/libostree/ostree-enumtypes.c: src/libostree/ostree-enumtypes.c.template $(ENUM_TYPES) $(AM_V_GEN) $(GLIB_MKENUMS) \ --template $< \ - --fhead "#include \"ostree-enumtypes.h\"" \ $(ENUM_TYPES) > $@.tmp && mv $@.tmp $@ nodist_libostree_1_la_SOURCES = \ diff --git a/src/libostree/ostree-enumtypes.c.template b/src/libostree/ostree-enumtypes.c.template index 52da7de1..f7eecf24 100644 --- a/src/libostree/ostree-enumtypes.c.template +++ b/src/libostree/ostree-enumtypes.c.template @@ -23,6 +23,8 @@ #endif #include +#include "ostree-enumtypes.h" + /*** END file-header ***/ /*** BEGIN file-production ***/ From 96e49a67f9dad9f846351a150059063aaa38a21b Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 18 Jul 2017 05:59:04 -0400 Subject: [PATCH 15/35] ci/papr: Update to F26 In particular F25AH will stop getting updates. Closes: #1012 Approved by: jlebon --- .papr.yml | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/.papr.yml b/.papr.yml index ac7c3278..f13eb1cf 100644 --- a/.papr.yml +++ b/.papr.yml @@ -4,10 +4,10 @@ branches: - try required: true -context: f25-primary +context: f26-primary container: - image: registry.fedoraproject.org/fedora:25 + image: registry.fedoraproject.org/fedora:26 packages: - git @@ -50,10 +50,10 @@ tests: --- -context: f25-rust +context: f26-rust inherit: true container: - image: registry.fedoraproject.org/fedora:25 + image: registry.fedoraproject.org/fedora:26 packages: - cargo env: @@ -67,7 +67,7 @@ tests: inherit: true -context: f25-experimental-api +context: f26-experimental-api env: CONFIGOPTS: '--enable-experimental-api' @@ -79,7 +79,7 @@ tests: inherit: true required: true -context: f25-curl-openssl +context: f26-curl-openssl packages: - pkgconfig(libcurl) @@ -99,21 +99,22 @@ branches: - auto - try -context: f25ah-insttest +context: f26ah-insttest required: false cluster: hosts: - name: vmcheck - distro: fedora/25/atomic + distro: fedora/26/atomic container: - image: registry.fedoraproject.org/fedora:25 + image: registry.fedoraproject.org/fedora:26 # Copy the build from the container to the host; ideally down the line # this is installing an RPM via https://github.com/jlebon/redhat-ci/issues/10 tests: - ci/build.sh - make install DESTDIR=$(pwd)/insttree + - yum -y install rsync - rsync -rl -e 'ssh -o User=root' . vmcheck:ostree/ - ssh root@vmcheck './ostree/tests/installed/fah-prep.sh && ostree admin unlock && rsync -rlv ./ostree/insttree/usr/ /usr/ && ./ostree/tests/installed/run.sh' @@ -125,19 +126,19 @@ branches: - auto - try -context: f25-flatpak +context: f26-flatpak required: false # This test case wants an "unprivileged container with bubblewrap", # which we don't have right now; so just provision a VM and do a # docker --privileged run. host: - distro: fedora/25/atomic + distro: fedora/26/atomic specs: ram: 4096 # build-bundle is a static delta, which needs RAM right now tests: - - docker run --rm --privileged -v $(pwd):/srv/code registry.fedoraproject.org/fedora:25 /bin/sh -c "cd /srv/code && ./ci/flatpak.sh" + - docker run --rm --privileged -v $(pwd):/srv/code registry.fedoraproject.org/fedora:26 /bin/sh -c "cd /srv/code && ./ci/flatpak.sh" artifacts: - test-suite.log From 8b1f1c442810307c938eab5d5aa2eaf6501a6543 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 14 Jul 2017 16:51:21 +0000 Subject: [PATCH 16/35] lib/pull: Do local content imports async too This came up in ; when we added more direct local importing, we did it synchronously. This was actually quite a regression when doing local pulls between different modes; in particular between a bare mode and `archive`, as we were suddenly doing gzip {de,}compression in the main thread. Down the line actually...a simpler fix is probably to change things so that the local path is really only used when we know we can hardlink; everything else would go though the fetcher codepath but with `file://`. But this isn't a lot more code, and the speed/interactivity win is large. Note we're only doing content async with this patch. We could do metadata as well; we have the object already local. But the metadata code path is messier, and metadata objects are smaller. Another area where this comes up is that in e.g. Fedora releng, most operations talk to a NetApp via NFS. So this has the classic network filesystem problem that operations that are normally cheap like `stat()` can actually have nontrivial latency. Doing as much as possible in threads is better there too. Closes: #1006 Approved by: jlebon --- src/libostree/ostree-repo-pull.c | 107 +++++++++++++++++++++++++++---- 1 file changed, 93 insertions(+), 14 deletions(-) diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 34ea7f9e..97e410e7 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -614,18 +614,19 @@ validate_bareuseronly_mode (OtPullData *pull_data, return TRUE; } -/* Import a single content object in the case where - * we have pull_data->remote_repo_local. +/* Synchronously import a single content object; this is used async for content, + * or synchronously for metadata. @src_repo is either + * pull_data->remote_repo_local or one of pull_data->localcache_repos. * * One important special case here is handling the * OSTREE_REPO_PULL_FLAGS_BAREUSERONLY_FILES flag. */ static gboolean -import_one_local_content_object (OtPullData *pull_data, - OstreeRepo *src_repo, - const char *checksum, - GCancellable *cancellable, - GError **error) +import_one_local_content_object_sync (OtPullData *pull_data, + OstreeRepo *src_repo, + const char *checksum, + GCancellable *cancellable, + GError **error) { const gboolean trusted = !pull_data->is_untrusted; if (trusted && !pull_data->is_bareuseronly_files) @@ -674,6 +675,83 @@ import_one_local_content_object (OtPullData *pull_data, return TRUE; } +typedef struct { + OtPullData *pull_data; + OstreeRepo *src_repo; + char checksum[OSTREE_SHA256_STRING_LEN+1]; +} ImportLocalAsyncData; + +static void +async_import_in_thread (GTask *task, + gpointer source, + gpointer task_data, + GCancellable *cancellable) +{ + ImportLocalAsyncData *iataskdata = task_data; + g_autoptr(GError) local_error = NULL; + if (!import_one_local_content_object_sync (iataskdata->pull_data, + iataskdata->src_repo, + iataskdata->checksum, + cancellable, + &local_error)) + g_task_return_error (task, g_steal_pointer (&local_error)); + else + g_task_return_boolean (task, TRUE); +} + +/* Start an async import of a single object; currently used for content objects. + * @src_repo is from pull_data->remote_repo_local or + * pull_data->localcache_repos. + * + * One important special case here is handling the + * OSTREE_REPO_PULL_FLAGS_BAREUSERONLY_FILES flag. + */ +static void +async_import_one_local_content_object (OtPullData *pull_data, + OstreeRepo *src_repo, + const char *checksum, + GCancellable *cancellable, + GAsyncReadyCallback callback, + gpointer user_data) +{ + ImportLocalAsyncData *iataskdata = g_new0 (ImportLocalAsyncData, 1); + iataskdata->pull_data = pull_data; + iataskdata->src_repo = src_repo; + memcpy (iataskdata->checksum, checksum, OSTREE_SHA256_STRING_LEN); + g_autoptr(GTask) task = g_task_new (pull_data->repo, cancellable, callback, user_data); + g_task_set_source_tag (task, async_import_one_local_content_object); + g_task_set_task_data (task, iataskdata, g_free); + pull_data->n_outstanding_content_write_requests++; + g_task_run_in_thread (task, async_import_in_thread); +} + +static gboolean +async_import_one_local_content_object_finish (OtPullData *pull_data, + GAsyncResult *result, + GError **error) +{ + g_return_val_if_fail (g_task_is_valid (result, pull_data->repo), FALSE); + return g_task_propagate_boolean ((GTask*)result, error); +} + +static void +on_local_object_imported (GObject *object, + GAsyncResult *result, + gpointer user_data) +{ + OtPullData *pull_data = user_data; + g_autoptr(GError) local_error = NULL; + GError **error = &local_error; + + if (!async_import_one_local_content_object_finish (pull_data, result, error)) + goto out; + + out: + g_assert_cmpint (pull_data->n_outstanding_content_write_requests, >, 0); + pull_data->n_outstanding_content_write_requests--; + check_outstanding_requests_handle_error (pull_data, &local_error); +} + static gboolean scan_dirtree_object (OtPullData *pull_data, const char *checksum, @@ -724,10 +802,11 @@ scan_dirtree_object (OtPullData *pull_data, /* Is this a local repo? */ if (pull_data->remote_repo_local) { - if (!import_one_local_content_object (pull_data, pull_data->remote_repo_local, - file_checksum, cancellable, error)) - return FALSE; - + async_import_one_local_content_object (pull_data, pull_data->remote_repo_local, + file_checksum, cancellable, + on_local_object_imported, + pull_data); + g_hash_table_add (pull_data->requested_content, g_steal_pointer (&file_checksum)); /* Note early loop continue */ continue; } @@ -746,9 +825,9 @@ scan_dirtree_object (OtPullData *pull_data, return FALSE; if (!localcache_repo_has_obj) continue; - if (!import_one_local_content_object (pull_data, localcache_repo, file_checksum, - cancellable, error)) - return FALSE; + async_import_one_local_content_object (pull_data, localcache_repo, file_checksum, cancellable, + on_local_object_imported, pull_data); + g_hash_table_add (pull_data->requested_content, g_steal_pointer (&file_checksum)); did_import_from_cache_repo = TRUE; pull_data->n_fetched_localcache_content++; break; From e0346c149498a7ec7d5c064a9d5485c9dcb693b1 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 30 May 2017 14:07:13 -0400 Subject: [PATCH 17/35] Add a notion of "physical" sysroot, use for remote writing (Note this PR was reverted in ; this version should be better) Using `${sysroot}` to mean the physical storage root: We don't want to write to `${sysroot}/etc/ostree/remotes.d`, since nothing will read it, and really `${sysroot}` should just have `/ostree` (ideally). Today the Anaconda rpmostree code ends up writing there. Fix this by adding a notion of "physical" sysroot. We determine whether the path is physical by checking for `/sysroot`, which exists in deployment roots (and there shouldn't be a `${sysroot}/sysroot`). In order to unit test this, I added a `--sysroot` argument to `remote add`. However, doing this better would require reworking the command line parsing for the `remote` argument to support specifying `--repo` or `--sysroot`, and I didn't quite want to do that yet in this patch. This second iteration of this patch fixes the bug we hit the first time; embarassingly enough I broke `ostree remote list` finding system remotes. The fix is to have `ostree_repo_open()` figure out whether it's the same as `/ostree/repo` for now. Down the line...we might consider having the `ostree remote` command line itself instatiate an `OstreeSysroot` by default, but this maximizes compatibility; we just have to pay a small cost that `ostree` usage outside of that case like `ostree static-delta` in a releng Jenkins job or whatever will do this `stat()` too. Closes: https://github.com/ostreedev/ostree/issues/892 Closes: #1008 Approved by: mbarnes --- src/libostree/ostree-repo-private.h | 10 +- src/libostree/ostree-repo.c | 118 ++++++++++++++++++------ src/libostree/ostree-sysroot-private.h | 3 +- src/libostree/ostree-sysroot.c | 24 ++++- src/ostree/ot-main.c | 122 ++++++++++++++++++------- src/ostree/ot-main.h | 8 ++ src/ostree/ot-remote-builtin-add.c | 12 ++- src/ostree/ot-remote-builtin-delete.c | 35 ++++--- tests/admin-test.sh | 15 +++ tests/installed/itest-remotes.sh | 16 ++++ tests/test-admin-deploy-grub2.sh | 2 +- tests/test-admin-deploy-syslinux.sh | 2 +- tests/test-admin-deploy-uboot.sh | 2 +- 13 files changed, 283 insertions(+), 86 deletions(-) create mode 100755 tests/installed/itest-remotes.sh diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h index dc49ed3b..da448bbe 100644 --- a/src/libostree/ostree-repo-private.h +++ b/src/libostree/ostree-repo-private.h @@ -80,6 +80,13 @@ struct OstreeRepoCommitModifier { GHashTable *devino_cache; }; +typedef enum { + OSTREE_REPO_SYSROOT_KIND_UNKNOWN, + OSTREE_REPO_SYSROOT_KIND_NO, /* Not a system repo */ + OSTREE_REPO_SYSROOT_KIND_VIA_SYSROOT, /* Constructed via ostree_sysroot_get_repo() */ + OSTREE_REPO_SYSROOT_KIND_IS_SYSROOT_OSTREE, /* We match /ostree/repo */ +} OstreeRepoSysrootKind; + /** * OstreeRepo: * @@ -101,6 +108,7 @@ struct OstreeRepo { int objects_dir_fd; int uncompressed_objects_dir_fd; GFile *sysroot_dir; + GWeakRef sysroot; /* Weak to avoid a circular ref; see also `is_system` */ char *remotes_config_dir; GHashTable *txn_refs; /* (element-type utf8 utf8) */ @@ -118,7 +126,7 @@ struct OstreeRepo { gboolean inited; gboolean writable; - gboolean is_system; /* Was this repo created via ostree_sysroot_get_repo() ? */ + OstreeRepoSysrootKind sysroot_kind; GError *writable_error; gboolean in_transaction; gboolean disable_fsync; diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 815d2d65..48c9a134 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -32,6 +32,7 @@ #include #include "ostree-core-private.h" +#include "ostree-sysroot-private.h" #include "ostree-remote-private.h" #include "ostree-repo-private.h" #include "ostree-repo-file.h" @@ -125,6 +126,10 @@ G_DEFINE_TYPE (OstreeRepo, ostree_repo, G_TYPE_OBJECT) #define SYSCONF_REMOTES SHORTENED_SYSCONFDIR "/ostree/remotes.d" +static GFile * +get_remotes_d_dir (OstreeRepo *self, + GFile *sysroot); + OstreeRemote * _ostree_repo_get_remote (OstreeRepo *self, const char *name, @@ -467,6 +472,7 @@ ostree_repo_finalize (GObject *object) if (self->uncompressed_objects_dir_fd != -1) (void) close (self->uncompressed_objects_dir_fd); g_clear_object (&self->sysroot_dir); + g_weak_ref_clear (&self->sysroot); g_free (self->remotes_config_dir); if (self->loose_object_devino_hash) @@ -547,10 +553,6 @@ ostree_repo_constructed (GObject *object) g_assert (self->repodir != NULL); - /* Ensure the "sysroot-path" property is set. */ - if (self->sysroot_dir == NULL) - self->sysroot_dir = g_object_ref (_ostree_get_default_sysroot_path ()); - G_OBJECT_CLASS (ostree_repo_parent_class)->constructed (object); } @@ -645,6 +647,7 @@ ostree_repo_init (OstreeRepo *self) self->objects_dir_fd = -1; self->uncompressed_objects_dir_fd = -1; self->commit_stagedir_lock = empty_lockfile; + self->sysroot_kind = OSTREE_REPO_SYSROOT_KIND_UNKNOWN; } /** @@ -728,18 +731,20 @@ ostree_repo_new_default (void) gboolean ostree_repo_is_system (OstreeRepo *repo) { - g_autoptr(GFile) default_repo_path = NULL; - g_return_val_if_fail (OSTREE_IS_REPO (repo), FALSE); /* If we were created via ostree_sysroot_get_repo(), we know the answer is yes * without having to compare file paths. */ - if (repo->is_system) + if (repo->sysroot_kind == OSTREE_REPO_SYSROOT_KIND_VIA_SYSROOT || + repo->sysroot_kind == OSTREE_REPO_SYSROOT_KIND_IS_SYSROOT_OSTREE) return TRUE; - default_repo_path = get_default_repo_path (repo->sysroot_dir); + /* No sysroot_dir set? Not a system repo then. */ + if (!repo->sysroot_dir) + return FALSE; + g_autoptr(GFile) default_repo_path = get_default_repo_path (repo->sysroot_dir); return g_file_equal (repo->repodir, default_repo_path); } @@ -916,24 +921,11 @@ impl_repo_remote_add (OstreeRepo *self, remote = ostree_remote_new (name); - /* The OstreeRepo maintains its own internal system root path, - * so we need to not only check if a "sysroot" argument was given - * but also whether it's actually different from OstreeRepo's. - * - * XXX Having API regret about the "sysroot" argument now. - */ - gboolean different_sysroot = FALSE; - if (sysroot != NULL) - different_sysroot = !g_file_equal (sysroot, self->sysroot_dir); - - if (different_sysroot || ostree_repo_is_system (self)) + g_autoptr(GFile) etc_ostree_remotes_d = get_remotes_d_dir (self, sysroot); + if (etc_ostree_remotes_d) { g_autoptr(GError) local_error = NULL; - if (sysroot == NULL) - sysroot = self->sysroot_dir; - - g_autoptr(GFile) etc_ostree_remotes_d = g_file_resolve_relative_path (sysroot, SYSCONF_REMOTES); if (!g_file_make_directory_with_parents (etc_ostree_remotes_d, cancellable, &local_error)) { @@ -1874,14 +1866,55 @@ append_one_remote_config (OstreeRepo *self, } static GFile * -get_remotes_d_dir (OstreeRepo *self) +get_remotes_d_dir (OstreeRepo *self, + GFile *sysroot) { - if (self->remotes_config_dir != NULL) + /* Support explicit override */ + if (self->sysroot_dir != NULL && self->remotes_config_dir != NULL) return g_file_resolve_relative_path (self->sysroot_dir, self->remotes_config_dir); - else if (ostree_repo_is_system (self)) - return g_file_resolve_relative_path (self->sysroot_dir, SYSCONF_REMOTES); - return NULL; + g_autoptr(GFile) sysroot_owned = NULL; + /* Very complicated sysroot logic; this bit breaks the otherwise mostly clean + * layering between OstreeRepo and OstreeSysroot. First, If a sysroot was + * provided, use it. Otherwise, check to see whether we reference + * /ostree/repo, or if not that, see if we have a ref to a sysroot (and it's + * physical). + */ + g_autoptr(OstreeSysroot) sysroot_ref = NULL; + if (sysroot == NULL) + { + /* No explicit sysroot? Let's see if we have a kind */ + switch (self->sysroot_kind) + { + case OSTREE_REPO_SYSROOT_KIND_UNKNOWN: + g_assert_not_reached (); + break; + case OSTREE_REPO_SYSROOT_KIND_NO: + break; + case OSTREE_REPO_SYSROOT_KIND_IS_SYSROOT_OSTREE: + sysroot = sysroot_owned = g_file_new_for_path ("/"); + break; + case OSTREE_REPO_SYSROOT_KIND_VIA_SYSROOT: + sysroot_ref = (OstreeSysroot*)g_weak_ref_get (&self->sysroot); + /* Only write to /etc/ostree/remotes.d if we are pointed at a deployment */ + if (sysroot_ref != NULL && !sysroot_ref->is_physical) + sysroot = ostree_sysroot_get_path (sysroot_ref); + break; + } + } + /* For backwards compat, also fall back to the sysroot-path variable, which we + * don't set anymore internally, and I hope no one else uses. + */ + if (sysroot == NULL && sysroot_ref == NULL) + sysroot = self->sysroot_dir; + + /* Did we find a sysroot? If not, NULL means use the repo config, otherwise + * return the path in /etc. + */ + if (sysroot == NULL) + return NULL; + else + return g_file_resolve_relative_path (sysroot, SYSCONF_REMOTES); } static gboolean @@ -2036,7 +2069,7 @@ reload_remote_config (OstreeRepo *self, if (!add_remotes_from_keyfile (self, self->config, NULL, error)) return FALSE; - g_autoptr(GFile) remotes_d = get_remotes_d_dir (self); + g_autoptr(GFile) remotes_d = get_remotes_d_dir (self, NULL); if (remotes_d == NULL) return TRUE; @@ -2101,6 +2134,7 @@ ostree_repo_open (OstreeRepo *self, GCancellable *cancellable, GError **error) { + struct stat self_stbuf; struct stat stbuf; g_return_val_if_fail (error == NULL || *error == NULL, FALSE); @@ -2137,6 +2171,9 @@ ostree_repo_open (OstreeRepo *self, return FALSE; } + if (!glnx_fstat (self->repo_dir_fd, &self_stbuf, error)) + return FALSE; + if (!glnx_opendirat (self->repo_dir_fd, "objects", TRUE, &self->objects_dir_fd, error)) { @@ -2178,6 +2215,27 @@ ostree_repo_open (OstreeRepo *self, return FALSE; } + /* If we weren't created via ostree_sysroot_get_repo(), for backwards + * compatibility we need to figure out now whether or not we refer to the + * system repo. See also ostree-sysroot.c. + */ + if (self->sysroot_kind == OSTREE_REPO_SYSROOT_KIND_UNKNOWN) + { + struct stat system_stbuf; + /* Ignore any errors if we can't access /ostree/repo */ + if (fstatat (AT_FDCWD, "/ostree/repo", &system_stbuf, 0) == 0) + { + /* Are we the same as /ostree/repo? */ + if (self_stbuf.st_dev == system_stbuf.st_dev && + self_stbuf.st_ino == system_stbuf.st_ino) + self->sysroot_kind = OSTREE_REPO_SYSROOT_KIND_IS_SYSROOT_OSTREE; + else + self->sysroot_kind = OSTREE_REPO_SYSROOT_KIND_NO; + } + else + self->sysroot_kind = OSTREE_REPO_SYSROOT_KIND_NO; + } + if (!ostree_repo_reload_config (self, cancellable, error)) return FALSE; @@ -4144,7 +4202,7 @@ find_keyring (OstreeRepo *self, return g_steal_pointer (&file); } - g_autoptr(GFile) remotes_d = get_remotes_d_dir (self); + g_autoptr(GFile) remotes_d = get_remotes_d_dir (self, NULL); if (remotes_d) { g_autoptr(GFile) file2 = g_file_get_child (remotes_d, remote->keyring); diff --git a/src/libostree/ostree-sysroot-private.h b/src/libostree/ostree-sysroot-private.h index 14ee5cad..82abc8e7 100644 --- a/src/libostree/ostree-sysroot-private.h +++ b/src/libostree/ostree-sysroot-private.h @@ -48,7 +48,8 @@ struct OstreeSysroot { GLnxLockFile lock; gboolean loaded; - + + gboolean is_physical; /* TRUE if we're pointed at physical storage root and not a deployment */ GPtrArray *deployments; int bootversion; int subbootversion; diff --git a/src/libostree/ostree-sysroot.c b/src/libostree/ostree-sysroot.c index 90868aae..d262a903 100644 --- a/src/libostree/ostree-sysroot.c +++ b/src/libostree/ostree-sysroot.c @@ -134,7 +134,9 @@ ostree_sysroot_constructed (GObject *object) repo_path = g_file_resolve_relative_path (self->path, "ostree/repo"); self->repo = ostree_repo_new_for_sysroot_path (repo_path, self->path); - self->repo->is_system = TRUE; + self->repo->sysroot_kind = OSTREE_REPO_SYSROOT_KIND_VIA_SYSROOT; + /* Hold a weak ref for the remote-add handling */ + g_weak_ref_init (&self->repo->sysroot, object); G_OBJECT_CLASS (ostree_sysroot_parent_class)->constructed (object); } @@ -813,6 +815,26 @@ ostree_sysroot_load_if_changed (OstreeSysroot *self, cancellable, error)) return FALSE; + /* Determine whether we're "physical" or not, the first time we initialize */ + if (!self->loaded) + { + /* If we have a booted deployment, the sysroot is / and we're definitely + * not physical. + */ + if (self->booted_deployment) + self->is_physical = FALSE; /* (the default, but explicit for clarity) */ + /* Otherwise - check for /sysroot which should only exist in a deployment, + * not in ${sysroot} (a metavariable for the real physical root). + */ + else if (fstatat (self->sysroot_fd, "sysroot", &stbuf, 0) < 0) + { + if (errno != ENOENT) + return glnx_throw_errno_prefix (error, "fstatat"); + self->is_physical = TRUE; + } + /* Otherwise, the default is FALSE */ + } + self->bootversion = bootversion; self->subbootversion = subbootversion; self->deployments = deployments; diff --git a/src/ostree/ot-main.c b/src/ostree/ot-main.c index 40d77f5f..ca4f1592 100644 --- a/src/ostree/ot-main.c +++ b/src/ostree/ot-main.c @@ -207,6 +207,88 @@ ostree_run (int argc, return 0; } +/* Process a --repo arg; used below, and for the remote builtins */ +static OstreeRepo * +parse_repo_option (GOptionContext *context, + const char *repo_path, + gboolean skip_repo_open, + GCancellable *cancellable, + GError **error) +{ + g_autoptr(OstreeRepo) repo = NULL; + + if (repo_path == NULL) + { + g_autoptr(GError) local_error = NULL; + + repo = ostree_repo_new_default (); + if (!ostree_repo_open (repo, cancellable, &local_error)) + { + if (g_error_matches (local_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND)) + { + g_autofree char *help = NULL; + + g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED, + "Command requires a --repo argument"); + + help = g_option_context_get_help (context, FALSE, NULL); + g_printerr ("%s", help); + } + else + { + g_propagate_error (error, g_steal_pointer (&local_error)); + } + return NULL; + } + } + else + { + g_autoptr(GFile) repo_file = g_file_new_for_path (repo_path); + + repo = ostree_repo_new (repo_file); + if (!skip_repo_open) + { + if (!ostree_repo_open (repo, cancellable, error)) + return NULL; + } + } + + return g_steal_pointer (&repo); +} + +/* Used by the remote builtins which are special in taking --sysroot or --repo */ +gboolean +ostree_parse_sysroot_or_repo_option (GOptionContext *context, + const char *sysroot_path, + const char *repo_path, + OstreeSysroot **out_sysroot, + OstreeRepo **out_repo, + GCancellable *cancellable, + GError **error) +{ + g_autoptr(OstreeSysroot) sysroot = NULL; + g_autoptr(OstreeRepo) repo = NULL; + if (sysroot_path) + { + g_autoptr(GFile) sysroot_file = g_file_new_for_path (sysroot_path); + sysroot = ostree_sysroot_new (sysroot_file); + if (!ostree_sysroot_load (sysroot, cancellable, error)) + return FALSE; + if (!ostree_sysroot_get_repo (sysroot, &repo, cancellable, error)) + return FALSE; + } + else + { + repo = parse_repo_option (context, repo_path, FALSE, cancellable, error); + if (!repo) + return FALSE; + } + + ot_transfer_out_value (out_sysroot, &sysroot); + ot_transfer_out_value (out_repo, &repo); + return TRUE; +} + gboolean ostree_option_context_parse (GOptionContext *context, const GOptionEntry *main_entries, @@ -217,7 +299,7 @@ ostree_option_context_parse (GOptionContext *context, GCancellable *cancellable, GError **error) { - glnx_unref_object OstreeRepo *repo = NULL; + g_autoptr(OstreeRepo) repo = NULL; /* Entries are listed in --help output in the order added. We add the * main entries ourselves so that we can add the --repo entry first. */ @@ -254,40 +336,12 @@ ostree_option_context_parse (GOptionContext *context, if (opt_verbose) g_log_set_handler (G_LOG_DOMAIN, G_LOG_LEVEL_DEBUG, message_handler, NULL); - if (opt_repo == NULL && !(flags & OSTREE_BUILTIN_FLAG_NO_REPO)) + if (!(flags & OSTREE_BUILTIN_FLAG_NO_REPO)) { - g_autoptr(GError) local_error = NULL; - - repo = ostree_repo_new_default (); - if (!ostree_repo_open (repo, cancellable, &local_error)) - { - if (g_error_matches (local_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND)) - { - g_autofree char *help = NULL; - - g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED, - "Command requires a --repo argument"); - - help = g_option_context_get_help (context, FALSE, NULL); - g_printerr ("%s", help); - } - else - { - g_propagate_error (error, g_steal_pointer (&local_error)); - } - return FALSE; - } - } - else if (opt_repo != NULL) - { - g_autoptr(GFile) repo_file = g_file_new_for_path (opt_repo); - - repo = ostree_repo_new (repo_file); - if (!(flags & OSTREE_BUILTIN_FLAG_NO_CHECK)) - { - if (!ostree_repo_open (repo, cancellable, error)) - return FALSE; - } + repo = parse_repo_option (context, opt_repo, (flags & OSTREE_BUILTIN_FLAG_NO_CHECK) > 0, + cancellable, error); + if (!repo) + return FALSE; } if (out_repo) diff --git a/src/ostree/ot-main.h b/src/ostree/ot-main.h index 3bb75243..c7db4a07 100644 --- a/src/ostree/ot-main.h +++ b/src/ostree/ot-main.h @@ -46,6 +46,14 @@ int ostree_run (int argc, char **argv, OstreeCommand *commands, GError **error); int ostree_usage (OstreeCommand *commands, gboolean is_error); +gboolean ostree_parse_sysroot_or_repo_option (GOptionContext *context, + const char *sysroot_path, + const char *repo_path, + OstreeSysroot **out_sysroot, + OstreeRepo **out_repo, + GCancellable *cancellable, + GError **error); + gboolean ostree_option_context_parse (GOptionContext *context, const GOptionEntry *main_entries, int *argc, char ***argv, diff --git a/src/ostree/ot-remote-builtin-add.c b/src/ostree/ot-remote-builtin-add.c index db115efd..c1f48969 100644 --- a/src/ostree/ot-remote-builtin-add.c +++ b/src/ostree/ot-remote-builtin-add.c @@ -34,6 +34,8 @@ static char *opt_contenturl; #ifdef OSTREE_ENABLE_EXPERIMENTAL_API static char *opt_collection_id; #endif /* OSTREE_ENABLE_EXPERIMENTAL_API */ +static char *opt_sysroot; +static char *opt_repo; static GOptionEntry option_entries[] = { { "set", 0, 0, G_OPTION_ARG_STRING_ARRAY, &opt_set, "Set config option KEY=VALUE for remote", "KEY=VALUE" }, @@ -45,6 +47,8 @@ static GOptionEntry option_entries[] = { { "collection-id", 0, 0, G_OPTION_ARG_STRING, &opt_collection_id, "Globally unique ID for this repository as an collection of refs for redistribution to other repositories", "COLLECTION-ID" }, #endif /* OSTREE_ENABLE_EXPERIMENTAL_API */ + { "repo", 0, 0, G_OPTION_ARG_FILENAME, &opt_repo, "Path to OSTree repository (defaults to /sysroot/ostree/repo)", "PATH" }, + { "sysroot", 0, 0, G_OPTION_ARG_FILENAME, &opt_sysroot, "Use sysroot at PATH (overrides --repo)", "PATH" }, { NULL } }; @@ -52,6 +56,7 @@ gboolean ot_remote_builtin_add (int argc, char **argv, GCancellable *cancellable, GError **error) { g_autoptr(GOptionContext) context = NULL; + g_autoptr(OstreeSysroot) sysroot = NULL; g_autoptr(OstreeRepo) repo = NULL; const char *remote_name; const char *remote_url; @@ -63,7 +68,12 @@ ot_remote_builtin_add (int argc, char **argv, GCancellable *cancellable, GError context = g_option_context_new ("NAME [metalink=|mirrorlist=]URL [BRANCH...] - Add a remote repository"); if (!ostree_option_context_parse (context, option_entries, &argc, &argv, - OSTREE_BUILTIN_FLAG_NONE, &repo, cancellable, error)) + OSTREE_BUILTIN_FLAG_NO_REPO, NULL, cancellable, error)) + goto out; + + if (!ostree_parse_sysroot_or_repo_option (context, opt_sysroot, opt_repo, + &sysroot, &repo, + cancellable, error)) goto out; if (argc < 3) diff --git a/src/ostree/ot-remote-builtin-delete.c b/src/ostree/ot-remote-builtin-delete.c index cac1b7ea..65a7ada3 100644 --- a/src/ostree/ot-remote-builtin-delete.c +++ b/src/ostree/ot-remote-builtin-delete.c @@ -25,43 +25,48 @@ #include "ot-main.h" #include "ot-remote-builtins.h" -gboolean opt_if_exists = FALSE; +static gboolean opt_if_exists = FALSE; +static char *opt_sysroot; +static char *opt_repo; static GOptionEntry option_entries[] = { { "if-exists", 0, 0, G_OPTION_ARG_NONE, &opt_if_exists, "Do nothing if the provided remote does not exist", NULL }, + { "repo", 0, 0, G_OPTION_ARG_FILENAME, &opt_repo, "Path to OSTree repository (defaults to /sysroot/ostree/repo)", "PATH" }, + { "sysroot", 0, 0, G_OPTION_ARG_FILENAME, &opt_sysroot, "Use sysroot at PATH (overrides --repo)", "PATH" }, { NULL } }; gboolean ot_remote_builtin_delete (int argc, char **argv, GCancellable *cancellable, GError **error) { - g_autoptr(GOptionContext) context = NULL; - g_autoptr(OstreeRepo) repo = NULL; - const char *remote_name; - gboolean ret = FALSE; - context = g_option_context_new ("NAME - Delete a remote repository"); + g_autoptr(GOptionContext) context = g_option_context_new ("NAME - Delete a remote repository"); if (!ostree_option_context_parse (context, option_entries, &argc, &argv, - OSTREE_BUILTIN_FLAG_NONE, &repo, cancellable, error)) - goto out; + OSTREE_BUILTIN_FLAG_NO_REPO, NULL, cancellable, error)) + return FALSE; + + g_autoptr(OstreeSysroot) sysroot = NULL; + g_autoptr(OstreeRepo) repo = NULL; + if (!ostree_parse_sysroot_or_repo_option (context, opt_sysroot, opt_repo, + &sysroot, &repo, + cancellable, error)) + return FALSE; if (argc < 2) { ot_util_usage_error (context, "NAME must be specified", error); - goto out; + return FALSE; } - remote_name = argv[1]; + const char *remote_name = argv[1]; if (!ostree_repo_remote_change (repo, NULL, - opt_if_exists ? OSTREE_REPO_REMOTE_CHANGE_DELETE_IF_EXISTS : + opt_if_exists ? OSTREE_REPO_REMOTE_CHANGE_DELETE_IF_EXISTS : OSTREE_REPO_REMOTE_CHANGE_DELETE, remote_name, NULL, NULL, cancellable, error)) - goto out; + return FALSE; - ret = TRUE; - out: - return ret; + return TRUE; } diff --git a/tests/admin-test.sh b/tests/admin-test.sh index 671fd905..6001ceea 100644 --- a/tests/admin-test.sh +++ b/tests/admin-test.sh @@ -234,3 +234,18 @@ curr_rev=$(${CMD_PREFIX} ostree rev-parse --repo=sysroot/ostree/repo testos/buil assert_streq ${curr_rev} ${head_rev} echo "ok upgrade with and without override-commit" + +deployment=$(${CMD_PREFIX} ostree admin --sysroot=sysroot --print-current-dir) +${CMD_PREFIX} ostree --sysroot=sysroot remote add --set=gpg-verify=false remote-test-physical file://$(pwd)/testos-repo +assert_not_has_file ${deployment}/etc/ostree/remotes.d/remote-test-physical.conf testos-repo +assert_file_has_content sysroot/ostree/repo/config remote-test-physical +echo "ok remote add physical sysroot" + +# Now a hack...symlink ${deployment}/sysroot to the sysroot in lieu of a bind +# mount which we can't do in unit tests. +ln -sr sysroot ${deployment}/sysroot +ln -s sysroot/ostree ${deployment}/ostree +${CMD_PREFIX} ostree --sysroot=${deployment} remote add --set=gpg-verify=false remote-test-nonphysical file://$(pwd)/testos-repo +assert_not_file_has_content sysroot/ostree/repo/config remote-test-nonphysical +assert_file_has_content ${deployment}/etc/ostree/remotes.d/remote-test-nonphysical.conf testos-repo +echo "ok remote add nonphysical sysroot" diff --git a/tests/installed/itest-remotes.sh b/tests/installed/itest-remotes.sh new file mode 100755 index 00000000..9b48b68f --- /dev/null +++ b/tests/installed/itest-remotes.sh @@ -0,0 +1,16 @@ +#!/bin/bash + +# Test that we didn't regress /etc/ostree/remotes.d handling + +set -xeuo pipefail + +dn=$(dirname $0) +. ${dn}/libinsttest.sh + +test_tmpdir=$(prepare_tmpdir) +trap _tmpdir_cleanup EXIT + +ostree remote list > remotes.txt +if ! test -s remotes.txt; then + assert_not_reached "no ostree remotes" +fi diff --git a/tests/test-admin-deploy-grub2.sh b/tests/test-admin-deploy-grub2.sh index 2b90c286..d7c1c6db 100755 --- a/tests/test-admin-deploy-grub2.sh +++ b/tests/test-admin-deploy-grub2.sh @@ -19,7 +19,7 @@ set -euo pipefail -echo "1..16" +echo "1..18" . $(dirname $0)/libtest.sh diff --git a/tests/test-admin-deploy-syslinux.sh b/tests/test-admin-deploy-syslinux.sh index 70b3b4d3..797836f0 100755 --- a/tests/test-admin-deploy-syslinux.sh +++ b/tests/test-admin-deploy-syslinux.sh @@ -19,7 +19,7 @@ set -euo pipefail -echo "1..16" +echo "1..18" . $(dirname $0)/libtest.sh diff --git a/tests/test-admin-deploy-uboot.sh b/tests/test-admin-deploy-uboot.sh index d4c3a0db..d9104f8c 100755 --- a/tests/test-admin-deploy-uboot.sh +++ b/tests/test-admin-deploy-uboot.sh @@ -20,7 +20,7 @@ set -euo pipefail -echo "1..17" +echo "1..19" . $(dirname $0)/libtest.sh From 9430b8ad75b5a17cdd05ca3e4eeab1a6302c6f63 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 17 Jul 2017 18:05:25 -0400 Subject: [PATCH 18/35] bin/cookies: Drop libsoup code, fix fd-relative issues, new style Prep for `ostree_repo_new_at()`. These commands were directly accessing `repo->repodir`, which it turns out was unnecessary since the the APIs they then used were fd-relative. Except actually there were bugs there, so fix all of the cookie util code to actually use the passed `dfd` and not just hardcode `AT_FDCWD`. Also, libsoup can't handle this (its APIs require fully qualifed paths), and there's not a really good reason to have two implementations now; historically it was useful to cross-check them, but I don't think we need that. While I'm here, port to new style. Closes: #1010 Approved by: jlebon --- src/ostree/ot-remote-builtin-add-cookie.c | 29 ++---- src/ostree/ot-remote-builtin-delete-cookie.c | 25 ++--- src/ostree/ot-remote-builtin-list-cookies.c | 16 +--- src/ostree/ot-remote-cookie-util.c | 96 ++------------------ 4 files changed, 28 insertions(+), 138 deletions(-) diff --git a/src/ostree/ot-remote-builtin-add-cookie.c b/src/ostree/ot-remote-builtin-add-cookie.c index d5ea3da5..68d5590c 100644 --- a/src/ostree/ot-remote-builtin-add-cookie.c +++ b/src/ostree/ot-remote-builtin-add-cookie.c @@ -36,18 +36,8 @@ static GOptionEntry option_entries[] = { gboolean ot_remote_builtin_add_cookie (int argc, char **argv, GCancellable *cancellable, GError **error) { - g_autoptr(GOptionContext) context = NULL; + g_autoptr(GOptionContext) context = g_option_context_new ("NAME DOMAIN PATH COOKIE_NAME VALUE - Add a cookie to remote"); g_autoptr(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; - - 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; @@ -58,16 +48,13 @@ ot_remote_builtin_add_cookie (int argc, char **argv, GCancellable *cancellable, 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 (gs_file_get_path_cached (repo->repodir), cookie_file, NULL); - - if (!ot_add_cookie_at (AT_FDCWD, jar_path, domain, path, cookie_name, value, error)) + const char *remote_name = argv[1]; + const char *domain = argv[2]; + const char *path = argv[3]; + const char *cookie_name = argv[4]; + const char *value = argv[5]; + g_autofree char *cookie_file = g_strdup_printf ("%s.cookies.txt", remote_name); + if (!ot_add_cookie_at (ostree_repo_get_dfd (repo), cookie_file, domain, path, cookie_name, value, error)) return FALSE; return TRUE; diff --git a/src/ostree/ot-remote-builtin-delete-cookie.c b/src/ostree/ot-remote-builtin-delete-cookie.c index cb1177fc..79778f77 100644 --- a/src/ostree/ot-remote-builtin-delete-cookie.c +++ b/src/ostree/ot-remote-builtin-delete-cookie.c @@ -36,16 +36,8 @@ static GOptionEntry option_entries[] = { gboolean ot_remote_builtin_delete_cookie (int argc, char **argv, GCancellable *cancellable, GError **error) { - g_autoptr(GOptionContext) context = NULL; g_autoptr(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; - - context = g_option_context_new ("NAME DOMAIN PATH COOKIE_NAME- Remote one cookie from remote"); + g_autoptr(GOptionContext) 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)) @@ -57,15 +49,12 @@ ot_remote_builtin_delete_cookie (int argc, char **argv, GCancellable *cancellabl 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 (gs_file_get_path_cached (repo->repodir), cookie_file, NULL); - - if (!ot_delete_cookie_at (AT_FDCWD, jar_path, domain, path, cookie_name, error)) + const char *remote_name = argv[1]; + const char *domain = argv[2]; + const char *path = argv[3]; + const char *cookie_name = argv[4]; + g_autofree char *cookie_file = g_strdup_printf ("%s.cookies.txt", remote_name); + if (!ot_delete_cookie_at (ostree_repo_get_dfd (repo), cookie_file, domain, path, cookie_name, error)) return FALSE; return TRUE; diff --git a/src/ostree/ot-remote-builtin-list-cookies.c b/src/ostree/ot-remote-builtin-list-cookies.c index 83d75a57..18c69035 100644 --- a/src/ostree/ot-remote-builtin-list-cookies.c +++ b/src/ostree/ot-remote-builtin-list-cookies.c @@ -35,13 +35,8 @@ static GOptionEntry option_entries[] = { gboolean ot_remote_builtin_list_cookies (int argc, char **argv, GCancellable *cancellable, GError **error) { - g_autoptr(GOptionContext) context = NULL; g_autoptr(OstreeRepo) repo = NULL; - const char *remote_name; - g_autofree char *jar_path = NULL; - g_autofree char *cookie_file = NULL; - - context = g_option_context_new ("NAME - Show remote repository cookies"); + g_autoptr(GOptionContext) 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)) @@ -53,12 +48,9 @@ ot_remote_builtin_list_cookies (int argc, char **argv, GCancellable *cancellable 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); - - if (!ot_list_cookies_at (AT_FDCWD, jar_path, error)) + const char *remote_name = argv[1]; + g_autofree char *cookie_file = g_strdup_printf ("%s.cookies.txt", remote_name); + if (!ot_list_cookies_at (ostree_repo_get_dfd (repo), cookie_file, error)) return FALSE; return TRUE; diff --git a/src/ostree/ot-remote-cookie-util.c b/src/ostree/ot-remote-cookie-util.c index a33b38bf..a20a3465 100644 --- a/src/ostree/ot-remote-cookie-util.c +++ b/src/ostree/ot-remote-cookie-util.c @@ -23,10 +23,6 @@ #include "ot-remote-cookie-util.h" -#ifndef HAVE_LIBCURL -#include -#endif - #include "otutil.h" #include "ot-main.h" #include "ot-remote-builtins.h" @@ -148,23 +144,15 @@ ot_add_cookie_at (int dfd, const char *jar_path, const char *name, const char *value, GError **error) { -#ifdef HAVE_LIBCURL - glnx_fd_close int fd = openat (AT_FDCWD, jar_path, O_WRONLY | O_APPEND | O_CREAT, 0644); - g_autofree char *buf = NULL; - g_autoptr(GDateTime) now = NULL; - g_autoptr(GDateTime) expires = NULL; - + glnx_fd_close int fd = openat (dfd, jar_path, O_WRONLY | O_APPEND | O_CREAT, 0644); if (fd < 0) - { - glnx_set_error_from_errno (error); - return FALSE; - } + return glnx_throw_errno_prefix (error, "open(%s)", jar_path); - now = g_date_time_new_now_utc (); - expires = g_date_time_add_years (now, 25); + g_autoptr(GDateTime) now = g_date_time_new_now_utc (); + g_autoptr(GDateTime) expires = g_date_time_add_years (now, 25); /* Adapted from soup-cookie-jar-text.c:write_cookie() */ - buf = g_strdup_printf ("%s\t%s\t%s\t%s\t%llu\t%s\t%s\n", + g_autofree char *buf = g_strdup_printf ("%s\t%s\t%s\t%s\t%llu\t%s\t%s\n", domain, *domain == '.' ? "TRUE" : "FALSE", path, @@ -173,24 +161,7 @@ ot_add_cookie_at (int dfd, const char *jar_path, name, value); if (glnx_loop_write (fd, buf, strlen (buf)) < 0) - { - glnx_set_error_from_errno (error); - return FALSE; - } -#else - glnx_unref_object SoupCookieJar *jar = NULL; - SoupCookie *cookie; - - 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 (name, value, domain, path, - SOUP_COOKIE_MAX_AGE_ONE_YEAR * 25); - - /* jar takes ownership of cookie */ - soup_cookie_jar_add_cookie (jar, cookie); -#endif + return glnx_throw_errno_prefix (error, "write"); return TRUE; } @@ -201,18 +172,14 @@ ot_delete_cookie_at (int dfd, const char *jar_path, GError **error) { gboolean found = FALSE; -#ifdef HAVE_LIBCURL g_auto(GLnxTmpfile) tmpf = { 0, }; - g_autofree char *dnbuf = NULL; - const char *dn = NULL; g_autoptr(OtCookieParser) parser = NULL; if (!ot_parse_cookies_at (dfd, jar_path, &parser, NULL, error)) return FALSE; - dnbuf = g_strdup (jar_path); - dn = dirname (dnbuf); - if (!glnx_open_tmpfile_linkable_at (AT_FDCWD, dn, O_WRONLY | O_CLOEXEC, + g_assert (!strchr (jar_path, '/')); + if (!glnx_open_tmpfile_linkable_at (dfd, ".", O_WRONLY | O_CLOEXEC, &tmpf, error)) return FALSE; @@ -233,33 +200,9 @@ ot_delete_cookie_at (int dfd, const char *jar_path, } if (!glnx_link_tmpfile_at (&tmpf, GLNX_LINK_TMPFILE_REPLACE, - AT_FDCWD, jar_path, + dfd, jar_path, error)) return FALSE; -#else - GSList *cookies; - glnx_unref_object SoupCookieJar *jar = 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 (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); - } -#endif if (!found) g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED, "Cookie not found in jar"); @@ -271,7 +214,6 @@ ot_delete_cookie_at (int dfd, const char *jar_path, gboolean ot_list_cookies_at (int dfd, const char *jar_path, GError **error) { -#ifdef HAVE_LIBCURL g_autoptr(OtCookieParser) parser = NULL; if (!ot_parse_cookies_at (AT_FDCWD, jar_path, &parser, NULL, error)) @@ -294,26 +236,6 @@ ot_list_cookies_at (int dfd, const char *jar_path, GError **error) g_print ("Expires: %s\n", expires_str); g_print ("Value: %s\n", parser->value); } -#else - glnx_unref_object SoupCookieJar *jar = soup_cookie_jar_text_new (jar_path, TRUE); - GSList *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); - } -#endif return TRUE; } From efd460782a439394cf980b82db7a93a81b887a4b Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 17 Jul 2017 20:50:55 -0400 Subject: [PATCH 19/35] lib/pull: Drop direct use of ->repodir Prep for `ostree_repo_new_at()`. Down the line perhaps we should extend libcurl to accept a file descriptor for cookies, but this works OK for now. Closes: #1010 Approved by: jlebon --- src/libostree/ostree-repo-pull.c | 16 ++++++++-------- src/ostree/ot-remote-cookie-util.c | 14 +++++++------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 97e410e7..60d11414 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -2697,16 +2697,16 @@ _ostree_repo_remote_new_fetcher (OstreeRepo *self, } { - g_autofree char *jar_path = NULL; - g_autofree char *cookie_file = g_strdup_printf ("%s.cookies.txt", - remote_name); + g_autofree char *cookie_file = g_strdup_printf ("%s.cookies.txt", remote_name); + /* TODO; port away from this; a bit hard since both libsoup and libcurl + * expect a file. Doing ot_fdrel_to_gfile() works for now though. + */ + GFile*repo_path = ostree_repo_get_path (self); + g_autofree char *jar_path = + g_build_filename (gs_file_get_path_cached (repo_path), cookie_file, NULL); - jar_path = g_build_filename (gs_file_get_path_cached (self->repodir), cookie_file, - NULL); - - if (g_file_test(jar_path, G_FILE_TEST_IS_REGULAR)) + if (g_file_test (jar_path, G_FILE_TEST_IS_REGULAR)) _ostree_fetcher_set_cookie_jar (fetcher, jar_path); - } success = TRUE; diff --git a/src/ostree/ot-remote-cookie-util.c b/src/ostree/ot-remote-cookie-util.c index a20a3465..914af5af 100644 --- a/src/ostree/ot-remote-cookie-util.c +++ b/src/ostree/ot-remote-cookie-util.c @@ -153,13 +153,13 @@ ot_add_cookie_at (int dfd, const char *jar_path, /* Adapted from soup-cookie-jar-text.c:write_cookie() */ g_autofree char *buf = g_strdup_printf ("%s\t%s\t%s\t%s\t%llu\t%s\t%s\n", - domain, - *domain == '.' ? "TRUE" : "FALSE", - path, - "FALSE", - (long long unsigned)g_date_time_to_unix (expires), - name, - value); + domain, + *domain == '.' ? "TRUE" : "FALSE", + path, + "FALSE", + (long long unsigned)g_date_time_to_unix (expires), + name, + value); if (glnx_loop_write (fd, buf, strlen (buf)) < 0) return glnx_throw_errno_prefix (error, "write"); return TRUE; From 2a9689b76a52bda06a45665d3ad77be06f1b1c2e Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 17 Jul 2017 21:14:04 -0400 Subject: [PATCH 20/35] Update libglnx, port various bits to new API Using the error prefixing in the delta processing allows us to do new code style. Also strip trailing whitespace. Use error prefixing in a few other random places. I didn't hunt for all of them, just testing out the new API. Use `glnx_fchmod()`. Also note I dropped one `fchmod (tmpf, 0600)` which is no longer necessary. Update submodule: libglnx Closes: #1011 Approved by: jlebon --- libglnx | 2 +- src/libostree/ostree-core.c | 12 +- src/libostree/ostree-fetcher-soup.c | 4 +- src/libostree/ostree-gpg-verifier.c | 3 +- src/libostree/ostree-impl-system-generator.c | 4 +- src/libostree/ostree-repo-checkout.c | 4 +- src/libostree/ostree-repo-commit.c | 20 +- src/libostree/ostree-repo-pull.c | 13 +- .../ostree-repo-static-delta-processing.c | 223 +++++++----------- src/libostree/ostree-repo.c | 2 - 10 files changed, 111 insertions(+), 176 deletions(-) diff --git a/libglnx b/libglnx index a37e6727..607f1775 160000 --- a/libglnx +++ b/libglnx @@ -1 +1 @@ -Subproject commit a37e672739197b8a7f3bdfe3f17099fe402f9a98 +Subproject commit 607f1775bb1c626cae1875a957a34802daebe81c diff --git a/src/libostree/ostree-core.c b/src/libostree/ostree-core.c index 6442bd52..d51fa4d5 100644 --- a/src/libostree/ostree-core.c +++ b/src/libostree/ostree-core.c @@ -26,6 +26,7 @@ #include #include #include +#include #include "libglnx.h" #include "ostree.h" #include "ostree-core-private.h" @@ -740,15 +741,16 @@ ostree_content_file_parse_at (gboolean compressed, GCancellable *cancellable, GError **error) { - g_autoptr(GInputStream) file_input = NULL; - if (!ot_openat_read_stream (parent_dfd, path, TRUE, &file_input, - cancellable, error)) - return FALSE; + int glnx_fd_close fd = openat (parent_dfd, path, O_RDONLY | O_CLOEXEC); + if (fd < 0) + return glnx_throw_errno_prefix (error, "open(%s)", path); struct stat stbuf; - if (!glnx_stream_fstat ((GFileDescriptorBased*)file_input, &stbuf, error)) + if (!glnx_fstat (fd, &stbuf, error)) return FALSE; + g_autoptr(GInputStream) file_input = g_unix_input_stream_new (glnx_steal_fd (&fd), TRUE); + g_autoptr(GFileInfo) ret_file_info = NULL; g_autoptr(GVariant) ret_xattrs = NULL; g_autoptr(GInputStream) ret_input = NULL; diff --git a/src/libostree/ostree-fetcher-soup.c b/src/libostree/ostree-fetcher-soup.c index 16fda0a3..f73554a2 100644 --- a/src/libostree/ostree-fetcher-soup.c +++ b/src/libostree/ostree-fetcher-soup.c @@ -1337,8 +1337,10 @@ _ostree_fetcher_bytes_transferred (OstreeFetcher *self) { if (G_IS_FILE_DESCRIPTOR_BASED (stream)) { + int fd = g_file_descriptor_based_get_fd ((GFileDescriptorBased*)stream); struct stat stbuf; - if (glnx_stream_fstat ((GFileDescriptorBased*)stream, &stbuf, NULL)) + + if (glnx_fstat (fd, &stbuf, NULL)) ret += stbuf.st_size; } } diff --git a/src/libostree/ostree-gpg-verifier.c b/src/libostree/ostree-gpg-verifier.c index 59028821..c008bdaf 100644 --- a/src/libostree/ostree-gpg-verifier.c +++ b/src/libostree/ostree-gpg-verifier.c @@ -93,6 +93,7 @@ _ostree_gpg_verifier_check_signature (OstreeGpgVerifier *self, GCancellable *cancellable, GError **error) { + GLNX_AUTO_PREFIX_ERROR("GPG", error); gpgme_error_t gpg_error = 0; ot_auto_gpgme_data gpgme_data_t data_buffer = NULL; ot_auto_gpgme_data gpgme_data_t signature_buffer = NULL; @@ -253,8 +254,6 @@ out: (void) glnx_shutil_rm_rf_at (AT_FDCWD, tmp_dir, NULL, NULL); } - g_prefix_error (error, "GPG: "); - return result; } diff --git a/src/libostree/ostree-impl-system-generator.c b/src/libostree/ostree-impl-system-generator.c index 5630348c..d64c8a27 100644 --- a/src/libostree/ostree-impl-system-generator.c +++ b/src/libostree/ostree-impl-system-generator.c @@ -203,8 +203,8 @@ _ostree_impl_system_generator (const char *ostree_cmdline, return FALSE; g_clear_object (&outstream); /* It should be readable */ - if (fchmod (tmpf.fd, 0644) < 0) - return glnx_throw_errno_prefix (error, "fchmod"); + if (!glnx_fchmod (tmpf.fd, 0644, error)) + return FALSE; /* Error out if somehow it already exists, that'll help us debug conflicts */ if (!glnx_link_tmpfile_at (&tmpf, GLNX_LINK_TMPFILE_NOREPLACE, normal_dir_dfd, "var.mount", error)) diff --git a/src/libostree/ostree-repo-checkout.c b/src/libostree/ostree-repo-checkout.c index 7942d607..15d0394b 100644 --- a/src/libostree/ostree-repo-checkout.c +++ b/src/libostree/ostree-repo-checkout.c @@ -81,8 +81,8 @@ checkout_object_for_uncompressed_cache (OstreeRepo *self, if (!g_output_stream_close (temp_out, cancellable, error)) return FALSE; - if (fchmod (tmpf.fd, file_mode) < 0) - return glnx_throw_errno (error); + if (!glnx_fchmod (tmpf.fd, file_mode, error)) + return FALSE; if (!_ostree_repo_ensure_loose_objdir_at (self->uncompressed_objects_dir_fd, loose_path, diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index 55d5b16a..a89be88b 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -253,8 +253,8 @@ commit_loose_regfile_object (OstreeRepo *self, if (S_ISREG (mode)) { const mode_t content_mode = (mode & (S_IFREG | 0775)) | S_IRUSR; - if (fchmod (tmpf->fd, content_mode) < 0) - return glnx_throw_errno_prefix (error, "fchmod"); + if (!glnx_fchmod (tmpf->fd, content_mode, error)) + return FALSE; } else g_assert (S_ISLNK (mode)); @@ -266,8 +266,8 @@ commit_loose_regfile_object (OstreeRepo *self, return glnx_throw (error, "Invalid mode 0%04o with bits 0%04o in bare-user-only repository", mode, invalid_modebits); - if (fchmod (tmpf->fd, mode) < 0) - return glnx_throw_errno_prefix (error, "fchmod"); + if (!glnx_fchmod (tmpf->fd, mode, error)) + return FALSE; } if (_ostree_repo_mode_is_bare (self->mode)) @@ -495,8 +495,8 @@ create_regular_tmpfile_linkable_with_content (OstreeRepo *self, } } - if (fchmod (tmpf.fd, 0644) < 0) - return glnx_throw_errno_prefix (error, "fchmod"); + if (!glnx_fchmod (tmpf.fd, 0644, error)) + return FALSE; *out_tmpf = tmpf; tmpf.initialized = FALSE; return TRUE; @@ -653,8 +653,8 @@ write_content_object (OstreeRepo *self, if (!g_output_stream_flush (temp_out, cancellable, error)) return FALSE; - if (fchmod (tmpf.fd, 0644) < 0) - return glnx_throw_errno_prefix (error, "fchmod"); + if (!glnx_fchmod (tmpf.fd, 0644, error)) + return FALSE; } const char *actual_checksum = NULL; @@ -852,8 +852,8 @@ write_metadata_object (OstreeRepo *self, return FALSE; if (glnx_loop_write (tmpf.fd, bufp, len) < 0) return glnx_throw_errno_prefix (error, "write()"); - if (fchmod (tmpf.fd, 0644) < 0) - return glnx_throw_errno_prefix (error, "fchmod"); + if (!glnx_fchmod (tmpf.fd, 0644, error)) + return FALSE; /* And commit it into place */ if (!_ostree_repo_commit_tmpf_final (self, actual_checksum, objtype, diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 60d11414..5d31e034 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -1386,11 +1386,10 @@ process_verify_result (OtPullData *pull_data, OstreeGpgVerifyResult *result, GError **error) { + const char *error_prefix = glnx_strjoina ("Commit ", checksum); + GLNX_AUTO_PREFIX_ERROR(error_prefix, error); if (result == NULL) - { - g_prefix_error (error, "Commit %s: ", checksum); - return FALSE; - } + return FALSE; /* Allow callers to output the results immediately. */ g_signal_emit_by_name (pull_data->repo, @@ -1398,10 +1397,8 @@ process_verify_result (OtPullData *pull_data, checksum, result); if (!ostree_gpg_verify_result_require_valid_signature (result, error)) - { - g_prefix_error (error, "Commit %s: ", checksum); - return FALSE; - } + return FALSE; + return TRUE; } diff --git a/src/libostree/ostree-repo-static-delta-processing.c b/src/libostree/ostree-repo-static-delta-processing.c index 028c700d..6b1dc337 100644 --- a/src/libostree/ostree-repo-static-delta-processing.c +++ b/src/libostree/ostree-repo-static-delta-processing.c @@ -50,7 +50,7 @@ typedef struct { GVariant *mode_dict; GVariant *xattr_dict; - + gboolean object_start; gboolean caught_error; GError **async_error; @@ -68,12 +68,12 @@ typedef struct { guint32 gid; guint32 mode; GVariant *xattrs; - + const guint8 *output_target; const guint8 *input_target_csum; const guint8 *payload_data; - guint64 payload_size; + guint64 payload_size; } StaticDeltaExecutionState; typedef struct { @@ -127,26 +127,21 @@ open_output_target (StaticDeltaExecutionState *state, GCancellable *cancellable, GError **error) { - gboolean ret = FALSE; - guint8 *objcsum; - g_assert (state->checksums != NULL); g_assert (state->output_target == NULL); g_assert (state->checksum_index < state->n_checksums); - objcsum = (guint8*)state->checksums + (state->checksum_index * OSTREE_STATIC_DELTA_OBJTYPE_CSUM_LEN); + guint8 *objcsum = (guint8*)state->checksums + (state->checksum_index * OSTREE_STATIC_DELTA_OBJTYPE_CSUM_LEN); if (G_UNLIKELY(!ostree_validate_structureof_objtype (*objcsum, error))) - goto out; + return FALSE; state->output_objtype = (OstreeObjectType) *objcsum; state->output_target = objcsum + 1; ostree_checksum_inplace_from_bytes (state->output_target, state->checksum); - ret = TRUE; - out: - return ret; + return TRUE; } static guint @@ -353,7 +348,7 @@ _ostree_static_delta_part_execute_async (OstreeRepo *repo, gboolean _ostree_static_delta_part_execute_finish (OstreeRepo *repo, GAsyncResult *result, - GError **error) + GError **error) { GSimpleAsyncResult *simple = G_SIMPLE_ASYNC_RESULT (result); @@ -386,7 +381,7 @@ content_out_write (OstreeRepo *repo, StaticDeltaExecutionState *state, const guint8* buf, gsize len, - GCancellable *cancellable, + GCancellable *cancellable, GError **error) { gsize bytes_written; @@ -407,21 +402,19 @@ content_out_write (OstreeRepo *repo, static gboolean do_content_open_generic (OstreeRepo *repo, StaticDeltaExecutionState *state, - GCancellable *cancellable, + GCancellable *cancellable, GError **error) { - gboolean ret = FALSE; - g_autoptr(GVariant) modev = NULL; guint64 mode_offset; guint64 xattr_offset; - guint32 uid, gid, mode; if (!read_varuint64 (state, &mode_offset, error)) - goto out; + return FALSE; if (!read_varuint64 (state, &xattr_offset, error)) - goto out; + return FALSE; - modev = g_variant_get_child_value (state->mode_dict, mode_offset); + g_autoptr(GVariant) modev = g_variant_get_child_value (state->mode_dict, mode_offset); + guint32 uid, gid, mode; g_variant_get (modev, "(uuu)", &uid, &gid, &mode); state->uid = GUINT32_FROM_BE (uid); state->gid = GUINT32_FROM_BE (gid); @@ -429,9 +422,7 @@ do_content_open_generic (OstreeRepo *repo, state->xattrs = g_variant_get_child_value (state->xattr_dict, xattr_offset); - ret = TRUE; - out: - return ret; + return TRUE; } struct bzpatch_opaque_s @@ -460,35 +451,29 @@ dispatch_bspatch (OstreeRepo *repo, GCancellable *cancellable, GError **error) { - gboolean ret = FALSE; guint64 offset, length; - g_autoptr(GMappedFile) input_mfile = NULL; - g_autofree guchar *buf = NULL; - struct bspatch_stream stream; - struct bzpatch_opaque_s opaque; if (!read_varuint64 (state, &offset, error)) - goto out; + return FALSE; if (!read_varuint64 (state, &length, error)) - goto out; + return FALSE; if (state->stats_only) - { - ret = TRUE; - goto out; - } + return TRUE; /* Early return */ if (!state->have_obj) { - input_mfile = g_mapped_file_new_from_fd (state->read_source_fd, FALSE, error); + g_autoptr(GMappedFile) input_mfile = g_mapped_file_new_from_fd (state->read_source_fd, FALSE, error); if (!input_mfile) - goto out; + return FALSE; - buf = g_malloc0 (state->content_size); + g_autofree guchar *buf = g_malloc0 (state->content_size); + struct bzpatch_opaque_s opaque; opaque.state = state; opaque.offset = offset; opaque.length = length; + struct bspatch_stream stream; stream.read = bspatch_read; stream.opaque = &opaque; if (bspatch ((const guint8*)g_mapped_file_get_contents (input_mfile), @@ -496,16 +481,14 @@ dispatch_bspatch (OstreeRepo *repo, buf, state->content_size, &stream) < 0) - goto out; + return FALSE; if (!content_out_write (repo, state, buf, state->content_size, cancellable, error)) - goto out; + return FALSE; } - ret = TRUE; - out: - return ret; + return TRUE; } /* Before, we had a distinction between "trusted" and "untrusted" deltas @@ -534,7 +517,7 @@ handle_untrusted_content_checksum (OstreeRepo *repo, static gboolean dispatch_open_splice_and_close (OstreeRepo *repo, StaticDeltaExecutionState *state, - GCancellable *cancellable, + GCancellable *cancellable, GError **error) { gboolean ret = FALSE; @@ -560,7 +543,7 @@ dispatch_open_splice_and_close (OstreeRepo *repo, ret = TRUE; goto out; } - + metadata = g_variant_new_from_data (ostree_metadata_variant_type (state->output_objtype), state->payload_data + offset, length, TRUE, NULL, NULL); @@ -581,7 +564,7 @@ dispatch_open_splice_and_close (OstreeRepo *repo, guint64 objlen; g_autoptr(GInputStream) object_input = NULL; g_autoptr(GInputStream) memin = NULL; - + if (!do_content_open_generic (repo, state, cancellable, error)) goto out; @@ -591,7 +574,7 @@ dispatch_open_splice_and_close (OstreeRepo *repo, goto out; if (!validate_ofs (state, content_offset, state->content_size, error)) goto out; - + if (state->stats_only) { ret = TRUE; @@ -599,7 +582,7 @@ dispatch_open_splice_and_close (OstreeRepo *repo, } /* Fast path for regular files to bare repositories */ - if (S_ISREG (state->mode) && + if (S_ISREG (state->mode) && (repo->mode == OSTREE_REPO_MODE_BARE || repo->mode == OSTREE_REPO_MODE_BARE_USER)) { @@ -676,10 +659,10 @@ dispatch_open_splice_and_close (OstreeRepo *repo, static gboolean dispatch_open (OstreeRepo *repo, StaticDeltaExecutionState *state, - GCancellable *cancellable, + GCancellable *cancellable, GError **error) { - gboolean ret = FALSE; + GLNX_AUTO_PREFIX_ERROR("opcode open", error); g_assert (state->output_target == NULL); /* FIXME - lift this restriction */ @@ -689,71 +672,58 @@ dispatch_open (OstreeRepo *repo, repo->mode == OSTREE_REPO_MODE_BARE_USER || repo->mode == OSTREE_REPO_MODE_BARE_USER_ONLY); } - + if (!open_output_target (state, cancellable, error)) - goto out; + return FALSE; if (!do_content_open_generic (repo, state, cancellable, error)) - goto out; + return FALSE; if (!read_varuint64 (state, &state->content_size, error)) - goto out; + return FALSE; if (state->stats_only) - { - ret = TRUE; - goto out; - } - + return TRUE; /* Early return */ + if (!_ostree_repo_open_content_bare (repo, state->checksum, state->content_size, &state->tmpf, &state->have_obj, cancellable, error)) - goto out; + return FALSE; if (!state->have_obj) state->content_out = g_unix_output_stream_new (state->tmpf.fd, FALSE); if (!handle_untrusted_content_checksum (repo, state, cancellable, error)) - goto out; + return FALSE; - ret = TRUE; - out: - if (!ret) - g_prefix_error (error, "opcode open: "); - return ret; + return TRUE; } static gboolean dispatch_write (OstreeRepo *repo, StaticDeltaExecutionState *state, - GCancellable *cancellable, + GCancellable *cancellable, GError **error) { - gboolean ret = FALSE; + GLNX_AUTO_PREFIX_ERROR("opcode open-splice-and-close", error); guint64 content_size; guint64 content_offset; - + if (!read_varuint64 (state, &content_size, error)) - goto out; + return FALSE; if (!read_varuint64 (state, &content_offset, error)) - goto out; + return FALSE; if (state->stats_only) - { - ret = TRUE; - goto out; - } + return TRUE; /* Early return */ if (!state->have_obj) { if (state->read_source_fd != -1) { if (lseek (state->read_source_fd, content_offset, SEEK_SET) == -1) - { - glnx_set_error_from_errno (error); - goto out; - } + return glnx_throw_errno_prefix (error, "lseek"); while (content_size > 0) { char buf[4096]; @@ -763,49 +733,38 @@ dispatch_write (OstreeRepo *repo, bytes_read = read (state->read_source_fd, buf, MIN(sizeof(buf), content_size)); while (G_UNLIKELY (bytes_read == -1 && errno == EINTR)); if (bytes_read == -1) - { - glnx_set_error_from_errno (error); - goto out; - } + return glnx_throw_errno_prefix (error, "read"); if (G_UNLIKELY (bytes_read == 0)) - { - g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, - "Unexpected EOF reading object %s", state->read_source_object); - goto out; - } - + return glnx_throw (error, "Unexpected EOF reading object %s", state->read_source_object); + if (!content_out_write (repo, state, (guint8*)buf, bytes_read, cancellable, error)) - goto out; - + return FALSE; + content_size -= bytes_read; } } else { if (!validate_ofs (state, content_offset, content_size, error)) - goto out; + return FALSE; if (!content_out_write (repo, state, state->payload_data + content_offset, content_size, cancellable, error)) - goto out; + return FALSE; } } - - ret = TRUE; - out: - if (!ret) - g_prefix_error (error, "opcode open-splice-and-close: "); - return ret; + + return TRUE; } static gboolean dispatch_set_read_source (OstreeRepo *repo, StaticDeltaExecutionState *state, - GCancellable *cancellable, + GCancellable *cancellable, GError **error) { - gboolean ret = FALSE; + GLNX_AUTO_PREFIX_ERROR("opcode set-read-source", error); guint64 source_offset; if (state->read_source_fd != -1) @@ -815,15 +774,12 @@ dispatch_set_read_source (OstreeRepo *repo, } if (!read_varuint64 (state, &source_offset, error)) - goto out; + return FALSE; if (!validate_ofs (state, source_offset, 32, error)) - goto out; + return FALSE; if (state->stats_only) - { - ret = TRUE; - goto out; - } + return TRUE; /* Early return */ g_free (state->read_source_object); state->read_source_object = ostree_checksum_from_bytes (state->payload_data + source_offset); @@ -832,28 +788,21 @@ dispatch_set_read_source (OstreeRepo *repo, &state->read_source_fd, NULL, NULL, NULL, cancellable, error)) - goto out; + return FALSE; - ret = TRUE; - out: - if (!ret) - g_prefix_error (error, "opcode set-read-source: "); - return ret; + return TRUE; } static gboolean dispatch_unset_read_source (OstreeRepo *repo, StaticDeltaExecutionState *state, - GCancellable *cancellable, + GCancellable *cancellable, GError **error) { - gboolean ret = FALSE; + GLNX_AUTO_PREFIX_ERROR("opcode unset-read-source", error); if (state->stats_only) - { - ret = TRUE; - goto out; - } + return TRUE; /* Early return */ if (state->read_source_fd != -1) { @@ -862,60 +811,48 @@ dispatch_unset_read_source (OstreeRepo *repo, } g_clear_pointer (&state->read_source_object, g_free); - - ret = TRUE; - out: - if (!ret) - g_prefix_error (error, "opcode unset-read-source: "); - return ret; + + return TRUE; } static gboolean dispatch_close (OstreeRepo *repo, StaticDeltaExecutionState *state, - GCancellable *cancellable, + GCancellable *cancellable, GError **error) { - gboolean ret = FALSE; - + GLNX_AUTO_PREFIX_ERROR("opcode open-splice-and-close", error); + if (state->content_out) { if (!g_output_stream_flush (state->content_out, cancellable, error)) - goto out; + return FALSE; if (state->content_checksum) { const char *actual_checksum = g_checksum_get_string (state->content_checksum); if (strcmp (actual_checksum, state->checksum) != 0) - { - g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, - "Corrupted object %s (actual checksum is %s)", - state->checksum, actual_checksum); - goto out; - } + return glnx_throw (error, "Corrupted object %s (actual checksum is %s)", + state->checksum, actual_checksum); } if (!_ostree_repo_commit_trusted_content_bare (repo, state->checksum, &state->tmpf, state->uid, state->gid, state->mode, state->xattrs, cancellable, error)) - goto out; + return FALSE; g_clear_object (&state->content_out); } if (!dispatch_unset_read_source (repo, state, cancellable, error)) - goto out; - + return FALSE; + g_clear_pointer (&state->xattrs, g_variant_unref); g_clear_pointer (&state->content_checksum, g_checksum_free); - + state->checksum_index++; state->output_target = NULL; - ret = TRUE; - out: - if (!ret) - g_prefix_error (error, "opcode open-splice-and-close: "); - return ret; + return TRUE; } diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 48c9a134..3c09079d 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -1753,8 +1753,6 @@ ostree_repo_create (OstreeRepo *self, if (!glnx_open_tmpfile_linkable_at (dfd, ".", O_RDWR|O_CLOEXEC, &tmpf, error)) return FALSE; - if (fchmod (tmpf.fd, 0600) < 0) - return glnx_throw_errno_prefix (error, "fchmod"); if (!_ostree_write_bareuser_metadata (tmpf.fd, 0, 0, 644, NULL, error)) return FALSE; } From bed931c91f4e5471bbb9f0b5f6bfe3e166858da7 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Wed, 21 Jun 2017 10:27:18 +0100 Subject: [PATCH 21/35] build: Don't distribute generated man pages We build them in "make" and clean them in "make clean", so there doesn't seem much point in shipping them pre-generated in the tarball. Signed-off-by: Simon McVittie Closes: #1013 Approved by: cgwalters --- Makefile-man.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile-man.am b/Makefile-man.am index 93779509..b3881526 100644 --- a/Makefile-man.am +++ b/Makefile-man.am @@ -46,7 +46,7 @@ man5_files = ostree.repo.5 ostree.repo-config.5 man1_MANS = $(addprefix man/,$(man1_files)) man5_MANS = $(addprefix man/,$(man5_files)) -EXTRA_DIST += $(man1_MANS) $(man5_MANS) $(man1_MANS:.1=.xml) $(man5_MANS:.5=.xml) +EXTRA_DIST += $(man1_MANS:.1=.xml) $(man5_MANS:.5=.xml) XSLT_STYLESHEET = http://docbook.sourceforge.net/release/xsl/current/manpages/docbook.xsl From c740b7f6d2310105b1a978aa7448d529bacad0c9 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 19 Jul 2017 05:47:33 -0400 Subject: [PATCH 22/35] core: Sanitize error text validating refs (e.g. against HTML) See: https://github.com/projectatomic/rpm-ostree/issues/885 If we get a successful Apache directory listing HTML when fetching what we intend to be a ref, we'd dump the HTML into the error. I did some scanning of the pull code, and this was the only case I saw offhand where we were dumping text out into an error. Which makes sense, since most of our formats are binary, the exeptions I think are just `repo/config` and `repo/refs/`. Closes: #1015 Approved by: mbarnes --- src/libostree/ostree-core.c | 44 +++++++++++++++++++++++++++++++- src/libostree/ostree-repo-pull.c | 5 ++-- tests/pull-test.sh | 13 +++++++++- tests/test-delta.sh | 2 +- 4 files changed, 59 insertions(+), 5 deletions(-) diff --git a/src/libostree/ostree-core.c b/src/libostree/ostree-core.c index d51fa4d5..116f376f 100644 --- a/src/libostree/ostree-core.c +++ b/src/libostree/ostree-core.c @@ -36,6 +36,40 @@ #define ALIGN_VALUE(this, boundary) \ (( ((unsigned long)(this)) + (((unsigned long)(boundary)) -1)) & (~(((unsigned long)(boundary))-1))) +/* Return a copy of @input suitable for addition to + * a GError message; newlines are quashed, the value + * is forced to be UTF-8, is truncated to @maxlen (if maxlen != -1). + */ +static char * +quash_string_for_error_message (const char *input, + ssize_t len, + ssize_t maxlen) +{ + if (len == -1) + len = strlen (input); + if (maxlen != -1 && maxlen < len) + len = maxlen; +#if GLIB_CHECK_VERSION(2, 52, 0) + G_GNUC_BEGIN_IGNORE_DEPRECATIONS + char *buf = g_utf8_make_valid (input, len); + G_GNUC_END_IGNORE_DEPRECATIONS +#else + char *buf = g_strndup (input, len); +#endif + for (char *iter = buf; iter && *iter; iter++) + { + char c = *iter; + if (c == '\n') + *iter = ' '; +#if !GLIB_CHECK_VERSION(2, 52, 0) + /* No g_utf8_make_valid()? OK, let's just brute force this. */ + if (!g_ascii_isprint (c)) + *iter = ' '; +#endif + } + return buf; +} + static gboolean file_header_parse (GVariant *metadata, GFileInfo **out_file_info, @@ -1825,7 +1859,15 @@ ostree_validate_structureof_checksum_string (const char *checksum, size_t len = strlen (checksum); if (len != OSTREE_SHA256_STRING_LEN) - return glnx_throw (error, "Invalid rev '%s'", checksum); + { + /* If we happen to get e.g. an Apache directory listing HTML, don't + * dump it all to the error. + * https://github.com/projectatomic/rpm-ostree/issues/885 + */ + g_autofree char *sanitized = quash_string_for_error_message (checksum, len, + OSTREE_SHA256_STRING_LEN); + return glnx_throw (error, "Invalid rev %s", sanitized); + } for (i = 0; i < len; i++) { diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 5d31e034..33ea489c 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -876,6 +876,7 @@ scan_dirtree_object (OtPullData *pull_data, return TRUE; } +/* Given a @ref, fetch its contents (should be a SHA256 ASCII string) */ static gboolean fetch_ref_contents (OtPullData *pull_data, const char *main_collection_id, @@ -901,7 +902,7 @@ fetch_ref_contents (OtPullData *pull_data, g_strchomp (ret_contents); if (!ostree_validate_checksum_string (ret_contents, error)) - return FALSE; + return glnx_prefix_error (error, "Fetching %s", filename); ot_transfer_out_value (out_contents, &ret_contents); return TRUE; @@ -1992,7 +1993,7 @@ load_remote_repo_config (OtPullData *pull_data, g_autoptr(GKeyFile) ret_keyfile = g_key_file_new (); if (!g_key_file_load_from_data (ret_keyfile, contents, strlen (contents), 0, error)) - return FALSE; + return glnx_prefix_error (error, "Parsing config"); ot_transfer_out_value (out_keyfile, &ret_keyfile); return TRUE; diff --git a/tests/pull-test.sh b/tests/pull-test.sh index 37b5e915..9bbe0fa2 100644 --- 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..26" +echo "1..27" # Try both syntaxes repo_init --no-gpg-verify @@ -413,3 +413,14 @@ rm $localsig ${CMD_PREFIX} ostree --repo=repo pull origin main test -f $localsig echo "ok re-pull signature for stored commit" + +cd ${test_tmpdir} +repo_init --no-gpg-verify +mv ostree-srv/gnomerepo/refs/heads/main{,.orig} +rm ostree-srv/gnomerepo/summary +(for x in $(seq 20); do echo "lots of html here "; done) > ostree-srv/gnomerepo/refs/heads/main +if ${CMD_PREFIX} ostree --repo=repo pull origin main 2>err.txt; then + fatal "pull of invalid ref succeeded" +fi +assert_file_has_content_literal err.txt 'error: Fetching refs/heads/main: Invalid rev lots of html here lots of html here lots of html here lots of' +echo "ok pull got HTML for a ref" diff --git a/tests/test-delta.sh b/tests/test-delta.sh index 8baee723..a59ee8e5 100755 --- a/tests/test-delta.sh +++ b/tests/test-delta.sh @@ -258,6 +258,6 @@ ${CMD_PREFIX} ostree --repo=repo summary -u if ${CMD_PREFIX} ostree --repo=repo static-delta show GARBAGE 2> err.txt; then assert_not_reached "static-delta show GARBAGE unexpectedly succeeded" fi -assert_file_has_content err.txt "Invalid rev 'GARBAGE'" +assert_file_has_content err.txt "Invalid rev GARBAGE" echo 'ok handle bad delta name' From 779f125cbe0a2e9b4c0d75d1ed2b9606424802c3 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 19 Jul 2017 10:35:06 -0400 Subject: [PATCH 23/35] lib/repo: Auto-recreate repo/tmp if it's deleted We can accumulate a lot of space there; let's be nice to people who delete the whole directory. Closes: https://github.com/ostreedev/ostree/issues/1018 Closes: #1020 Approved by: jlebon --- src/libostree/ostree-repo.c | 14 ++++++++++++++ tests/basic-test.sh | 10 +++++++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 3c09079d..a009f1c8 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -2201,6 +2201,20 @@ ostree_repo_open (OstreeRepo *self, self->target_owner_uid = self->target_owner_gid = -1; } + if (self->writable) + { + /* Always try to recreate the tmpdir to be nice to people + * who are looking to free up space. + * + * https://github.com/ostreedev/ostree/issues/1018 + */ + if (mkdirat (self->repo_dir_fd, "tmp", 0755) == -1) + { + if (G_UNLIKELY (errno != EEXIST)) + return glnx_throw_errno_prefix (error, "mkdir(tmp)"); + } + } + if (!glnx_opendirat (self->repo_dir_fd, "tmp", TRUE, &self->tmp_dir_fd, error)) return FALSE; diff --git a/tests/basic-test.sh b/tests/basic-test.sh index 33fdde9d..9ae26f47 100644 --- a/tests/basic-test.sh +++ b/tests/basic-test.sh @@ -19,7 +19,7 @@ set -euo pipefail -echo "1..$((69 + ${extra_basic_tests:-0}))" +echo "1..$((70 + ${extra_basic_tests:-0}))" $CMD_PREFIX ostree --version > version.yaml python -c 'import yaml; yaml.safe_load(open("version.yaml"))' @@ -112,6 +112,14 @@ ostree_repo_init test-repo --mode=bare-user rm test-repo -rf echo "ok repo-init on existing repo" +rm test-repo -rf +ostree_repo_init test-repo --mode=bare-user +${CMD_PREFIX} ostree --repo=test-repo refs +rm -rf test-repo/tmp +${CMD_PREFIX} ostree --repo=test-repo refs +assert_has_dir test-repo/tmp +echo "ok autocreate tmp" + rm checkout-test2 -rf $OSTREE checkout test2 checkout-test2 cd checkout-test2 From ebbd0b3ce6b50b4e5a1cc2d1ec550d05dfeb04f4 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 19 Jul 2017 09:08:41 -0400 Subject: [PATCH 24/35] ci/papr: Switch primary to libcurl, add libsoup context Sometime in the next few releases I think we should make libcurl the default. Prep for more CI work. Closes: #1016 Approved by: jlebon --- .papr.yml | 10 ++++------ ci/build.sh | 2 ++ 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.papr.yml b/.papr.yml index f13eb1cf..32d2b224 100644 --- a/.papr.yml +++ b/.papr.yml @@ -12,6 +12,8 @@ packages: - git env: + # At some point soon will encourage distros to use libcurl + CONFIGOPTS: "--with-curl --with-openssl" # Enable all the sanitizers for this primary build. # We only use -Werror=maybe-uninitialized here with a "fixed" toolchain CFLAGS: '-fsanitize=undefined -fsanitize-undefined-trap-on-error -fsanitize=address -O2 -Wp,-D_FORTIFY_SOURCE=2' @@ -79,14 +81,10 @@ tests: inherit: true required: true -context: f26-curl-openssl - -packages: - - pkgconfig(libcurl) - - pkgconfig(openssl) +context: f26-libsoup env: - CONFIGOPTS: "--with-curl --with-openssl" + CONFIGOPTS: "--without-curl --without-openssl" tests: - ci/build-check.sh diff --git a/ci/build.sh b/ci/build.sh index a9a8ac3f..2d7eb61e 100755 --- a/ci/build.sh +++ b/ci/build.sh @@ -7,6 +7,8 @@ dn=$(dirname $0) . ${dn}/libbuild.sh pkg_install_builddeps ostree +# Until this propagates farther +pkg_install 'pkgconfig(libcurl)' 'pkgconfig(openssl)' pkg_install sudo which attr fuse \ libubsan libasan libtsan PyYAML redhat-rpm-config \ elfutils From f9f7d55e7954ebde075c1afd18256d3c017feacd Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 19 Jul 2017 09:19:16 -0400 Subject: [PATCH 25/35] lib/commit: Fix EBADF with GENERATE_SIZES option for commit Regression from previous tmpfile refactoring; unfortunately the `OSTREE_REPO_COMMIT_MODIFIER_FLAGS_GENERATE_SIZES` option only has coverage via gjs currently. Might expose it via the cmdline in a later option, but in the big picture the idea was that this data is better kept in static deltas. Closes: https://github.com/ostreedev/ostree/issues/1014 Closes: #1016 Approved by: jlebon --- src/libostree/ostree-repo-commit.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index a89be88b..bcd9c2d9 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -734,6 +734,17 @@ write_content_object (OstreeRepo *self, } else { + /* Update size metadata if configured */ + if (indexable && object_file_type == G_FILE_TYPE_REGULAR) + { + struct stat stbuf; + + if (!glnx_fstat (tmpf.fd, &stbuf, error)) + return FALSE; + + repo_store_size_entry (self, actual_checksum, unpacked_size, stbuf.st_size); + } + /* This path is for regular files */ if (!commit_loose_regfile_object (self, actual_checksum, &tmpf, uid, gid, mode, @@ -743,17 +754,6 @@ write_content_object (OstreeRepo *self, ostree_object_type_to_string (OSTREE_OBJECT_TYPE_FILE)); } - /* Update size metadata if configured */ - if (indexable && object_file_type == G_FILE_TYPE_REGULAR) - { - struct stat stbuf; - - if (!glnx_fstat (tmpf.fd, &stbuf, error)) - return FALSE; - - repo_store_size_entry (self, actual_checksum, unpacked_size, stbuf.st_size); - } - /* Update statistics */ g_mutex_lock (&self->txn_stats_lock); self->txn_stats.content_objects_written++; From cf6c15a6c5bf75bc160c77d3bd302c9c02563455 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 19 Jul 2017 09:21:23 -0400 Subject: [PATCH 26/35] ci/papr: Add a suite to run introspection-based tests without ASAN Unfortunately we can't do gobject-introspection based tests while compiling with `-fsanitize=address`, since it needs to hook `malloc` early on. Add a new suite which just runs the introspection-based tests without ASAN. Closes: #1016 Approved by: jlebon --- .papr.yml | 16 ++++++++++++++++ ci/build-check.sh | 2 +- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/.papr.yml b/.papr.yml index 32d2b224..bd4ff47b 100644 --- a/.papr.yml +++ b/.papr.yml @@ -91,6 +91,22 @@ tests: --- +inherit: true +required: true + +context: f26-introspection-tests + +env: + CONFIGOPTS: "--with-curl --with-openssl" + # ASAN conflicts with introspection testing; + # See https://github.com/ostreedev/ostree/issues/1014 + INSTALLED_TESTS_PATTERN: "libostree/test-sizes.js libostree/test-sysroot.js libostree/test-core.js libostree/test-corrupt-repo-ref.js" + +tests: + - ci/build-check.sh + +--- + inherit: false branches: - master diff --git a/ci/build-check.sh b/ci/build-check.sh index 677515b3..0e12000f 100755 --- a/ci/build-check.sh +++ b/ci/build-check.sh @@ -11,7 +11,7 @@ make syntax-check # TODO: do syntax-check under check # And now run the installed tests make install if test -x /usr/bin/gnome-desktop-testing-runner; then - gnome-desktop-testing-runner -p 0 libostree/ + gnome-desktop-testing-runner -p 0 ${INSTALLED_TESTS_PATTERN:-libostree/} fi if test -x /usr/bin/clang; then From 6430207e4764c352ca2bd048be88dc8326853047 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 21 Jul 2017 09:44:12 -0400 Subject: [PATCH 27/35] lib: Add #defines for current well-known metadata keys This came up in https://github.com/projectatomic/rpm-ostree/issues/142 Let's add `#define`s for our metadata keys, with documentation so that, well, they're documented. Closes: #1024 Approved by: peterbaouoft --- src/libostree/ostree-core.h | 22 ++++++++++++++++++++++ src/libostree/ostree-sysroot-deploy.c | 4 ++-- src/libostree/ostree-sysroot-upgrader.c | 2 +- src/ostree/ot-admin-functions.c | 2 +- 4 files changed, 26 insertions(+), 4 deletions(-) diff --git a/src/libostree/ostree-core.h b/src/libostree/ostree-core.h index fa9e5e86..1b0f8913 100644 --- a/src/libostree/ostree-core.h +++ b/src/libostree/ostree-core.h @@ -195,6 +195,28 @@ typedef enum { OSTREE_REPO_MODE_BARE_USER_ONLY, } OstreeRepoMode; +/** + * OSTREE_COMMIT_META_KEY_VERSION: + * + * GVariant type `s`. This metadata key is used for version numbers. A freeform + * string; the intention is that systems using ostree do not interpret this + * semantically as traditional package managers do. + * + * This is the only ostree-defined metadata key that does not start with `ostree.`. + * Since: 2014.9 + */ +#define OSTREE_COMMIT_META_KEY_VERSION "version" +/** + * OSTREE_COMMIT_META_KEY_ENDOFLIFE_REBASE: + * + * GVariant type `s`. Should contain a refspec defining a new target branch; + * `ostree admin upgrade` and `OstreeSysrootUpgrader` will automatically initiate + * a rebase upon encountering this metadata key. + * + * Since: 2017.7 + */ +#define OSTREE_COMMIT_META_KEY_ENDOFLIFE_REBASE "ostree.endoflife-rebase" + _OSTREE_PUBLIC const GVariantType *ostree_metadata_variant_type (OstreeObjectType objtype); diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index e3db3691..bf4424a9 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -1265,7 +1265,7 @@ install_deployment_kernel (OstreeSysroot *sysroot, &variant, NULL)) { metadata = g_variant_get_child_value (variant, 0); - g_variant_lookup (metadata, "version", "s", &deployment_version); + g_variant_lookup (metadata, OSTREE_COMMIT_META_KEY_VERSION, "s", &deployment_version); } } @@ -1292,7 +1292,7 @@ install_deployment_kernel (OstreeSysroot *sysroot, ostree_bootconfig_parser_set (bootconfig, "title", title_key->str); g_autofree char *version_key = g_strdup_printf ("%d", n_deployments - ostree_deployment_get_index (deployment)); - ostree_bootconfig_parser_set (bootconfig, "version", version_key); + ostree_bootconfig_parser_set (bootconfig, OSTREE_COMMIT_META_KEY_VERSION, version_key); g_autofree char * boot_relpath = g_strconcat ("/", bootcsumdir, "/", dest_kernel_name, NULL); ostree_bootconfig_parser_set (bootconfig, "linux", boot_relpath); diff --git a/src/libostree/ostree-sysroot-upgrader.c b/src/libostree/ostree-sysroot-upgrader.c index 45ef90e6..8afea3a6 100644 --- a/src/libostree/ostree-sysroot-upgrader.c +++ b/src/libostree/ostree-sysroot-upgrader.c @@ -559,7 +559,7 @@ ostree_sysroot_upgrader_pull_one_dir (OstreeSysrootUpgrader *self, return FALSE; g_variant_get_child (new_variant, 0, "@a{sv}", &new_metadata); - rebase = g_variant_lookup_value (new_metadata, "ostree.endoflife-rebase", G_VARIANT_TYPE_STRING); + rebase = g_variant_lookup_value (new_metadata, OSTREE_COMMIT_META_KEY_ENDOFLIFE_REBASE, G_VARIANT_TYPE_STRING); if (rebase) { const char *new_ref = g_variant_get_string (rebase, 0); diff --git a/src/ostree/ot-admin-functions.c b/src/ostree/ot-admin-functions.c index 672d384a..a0a66b6a 100644 --- a/src/ostree/ot-admin-functions.c +++ b/src/ostree/ot-admin-functions.c @@ -57,7 +57,7 @@ ot_admin_checksum_version (GVariant *checksum) metadata = g_variant_get_child_value (checksum, 0); - if (!g_variant_lookup (metadata, "version", "&s", &ret)) + if (!g_variant_lookup (metadata, OSTREE_COMMIT_META_KEY_VERSION, "&s", &ret)) return NULL; return g_strdup (ret); From ef6f6bc688f3381a66e6bcb1fa440d5201537fd1 Mon Sep 17 00:00:00 2001 From: Ruixin Date: Fri, 21 Jul 2017 15:20:50 +0000 Subject: [PATCH 28/35] lib: Add #define for endoflife metadata key It is a continuation of https://github.com/ostreedev/ostree/pull/1024 It adds documentation for endoflife metadata key. Closes: #1025 Approved by: cgwalters --- src/libostree/ostree-core.h | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/libostree/ostree-core.h b/src/libostree/ostree-core.h index 1b0f8913..1fef003a 100644 --- a/src/libostree/ostree-core.h +++ b/src/libostree/ostree-core.h @@ -216,6 +216,16 @@ typedef enum { * Since: 2017.7 */ #define OSTREE_COMMIT_META_KEY_ENDOFLIFE_REBASE "ostree.endoflife-rebase" +/** + * OSTREE_COMMIT_META_KEY_ENDOFLIFE: + * + * GVariant type `s`. This metadata key is used to display vendor's message + * when an update stream for a particular branch ends. It usually provides + * update instructions for the users. + * + * Since: 2017.7 + */ +#define OSTREE_COMMIT_META_KEY_ENDOFLIFE "ostree.endoflife" _OSTREE_PUBLIC const GVariantType *ostree_metadata_variant_type (OstreeObjectType objtype); From ed99b4169e06c6a204f92e85b3e323abdb542bbe Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 20 Jul 2017 10:32:44 -0400 Subject: [PATCH 29/35] tests: More fixes for gjs tests Previous to this commit, the gjs tests were installed-only; and our logic for handling the "--enable-installed-tests=exclusive" logic actually also meant they weren't installed. It did work for me locally with `--enable-installed-tests`. However, to make things fully symmetric, let's enable the js tests to also be run under `make check`. Also remove `corrupt-repo-ref.js` from the PAPR invocation since it's not actually a unit test, it's a utility helper. Closes: #1022 Approved by: jlebon --- .papr.yml | 2 +- Makefile-tests.am | 2 +- tests/test-core.js | 4 +++- tests/test-sizes.js | 4 +++- tests/test-sysroot.js | 6 +++++- 5 files changed, 13 insertions(+), 5 deletions(-) mode change 100644 => 100755 tests/test-core.js mode change 100644 => 100755 tests/test-sizes.js mode change 100644 => 100755 tests/test-sysroot.js diff --git a/.papr.yml b/.papr.yml index bd4ff47b..4ec0765c 100644 --- a/.papr.yml +++ b/.papr.yml @@ -100,7 +100,7 @@ env: CONFIGOPTS: "--with-curl --with-openssl" # ASAN conflicts with introspection testing; # See https://github.com/ostreedev/ostree/issues/1014 - INSTALLED_TESTS_PATTERN: "libostree/test-sizes.js libostree/test-sysroot.js libostree/test-core.js libostree/test-corrupt-repo-ref.js" + INSTALLED_TESTS_PATTERN: "libostree/test-sizes.js libostree/test-sysroot.js libostree/test-core.js" tests: - ci/build-check.sh diff --git a/Makefile-tests.am b/Makefile-tests.am index 7be2ab59..82ce7209 100644 --- a/Makefile-tests.am +++ b/Makefile-tests.am @@ -191,7 +191,7 @@ js_installed_tests = \ $(NULL) if BUILDOPT_GJS -dist_installed_test_scripts = $(js_installed_tests) +_installed_or_uninstalled_test_scripts += $(js_installed_tests) else EXTRA_DIST += $(js_installed_tests) endif diff --git a/tests/test-core.js b/tests/test-core.js old mode 100644 new mode 100755 index 64d1b62d..8a06b6fa --- a/tests/test-core.js +++ b/tests/test-core.js @@ -25,6 +25,8 @@ function assertEquals(a, b) { throw new Error("assertion failed " + JSON.stringify(a) + " == " + JSON.stringify(b)); } +print('1..1') + let testDataDir = Gio.File.new_for_path('test-data'); testDataDir.make_directory(null); testDataDir.get_child('some-file').replace_contents("hello world!", null, false, 0, null); @@ -66,4 +68,4 @@ repo.commit_transaction(null, null); [,readCommit] = repo.resolve_rev('someref', true); assertEquals(readCommit, null); -print("test-core complete"); +print("ok test-core"); diff --git a/tests/test-sizes.js b/tests/test-sizes.js old mode 100644 new mode 100755 index 5cf765fc..71e76515 --- a/tests/test-sizes.js +++ b/tests/test-sizes.js @@ -26,6 +26,8 @@ function assertEquals(a, b) { throw new Error("assertion failed " + JSON.stringify(a) + " == " + JSON.stringify(b)); } +print('1..1') + let testDataDir = Gio.File.new_for_path('test-data'); testDataDir.make_directory(null); testDataDir.get_child('some-file').replace_contents("hello world!", null, false, 0, null); @@ -79,4 +81,4 @@ if (expectedUncompressedSizes.length > 0) { throw new Error("Failed to match expectedUncompressedSizes: " + JSON.stringify(expectedUncompressedSizes)); } -print("test-sizes complete"); +print("ok test-sizes"); diff --git a/tests/test-sysroot.js b/tests/test-sysroot.js old mode 100644 new mode 100755 index 7e8fcf7a..d7d3dab3 --- a/tests/test-sysroot.js +++ b/tests/test-sysroot.js @@ -33,11 +33,13 @@ function assertNotEquals(a, b) { function libtestExec(shellCode) { let testdatadir = GLib.getenv("G_TEST_SRCDIR"); - let libtestPath = GLib.build_filenamev([testdatadir, 'libtest.sh']) + let libtestPath = GLib.build_filenamev([testdatadir, 'tests/libtest.sh']) let proc = Gio.Subprocess.new(['bash', '-c', 'set -xeuo pipefail; . ' + GLib.shell_quote(libtestPath) + '; ' + shellCode], 0); proc.wait_check(null); } +print('1..1') + libtestExec('setup_os_repository archive-z2 syslinux'); let upstreamRepo = OSTree.Repo.new(Gio.File.new_for_path('testos-repo')); @@ -145,3 +147,5 @@ newDeployments = [deployment, newDeployment, thirdDeployment]; sysroot.write_deployments(newDeployments, null); deployments = sysroot.get_deployments(); assertEquals(deployments.length, 3); + +print("ok test-sysroot") From 0ded552cf1440a806bd7af684b63f486c03d67c9 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 20 Jul 2017 15:34:15 -0400 Subject: [PATCH 30/35] ci: Enable -Werror for clang I hit an unused-variable warning with `GLNX_AUTO_PREFIX_ERROR` for rpm-ostree and led me to wonder why ostree didn't fail, then I noticed we had lost the special `-Werror=unused-variable` bit. Let's go ahead and use `-Werror` for clang too. Closes: #1023 Approved by: jlebon --- .papr.yml | 1 + ci/build-check.sh | 3 +++ 2 files changed, 4 insertions(+) diff --git a/.papr.yml b/.papr.yml index 4ec0765c..ea378c98 100644 --- a/.papr.yml +++ b/.papr.yml @@ -31,6 +31,7 @@ timeout: 30m artifacts: - test-suite.log + - config.log --- context: c7-primary diff --git a/ci/build-check.sh b/ci/build-check.sh index 0e12000f..7df1f424 100755 --- a/ci/build-check.sh +++ b/ci/build-check.sh @@ -15,6 +15,9 @@ if test -x /usr/bin/gnome-desktop-testing-runner; then fi if test -x /usr/bin/clang; then + # always fail on warnings; https://github.com/ostreedev/ostree/pull/971 + # Except for clang-4.0: error: argument unused during compilation: '-specs=/usr/lib/rpm/redhat/redhat-hardened-cc1' [-Werror,-Wunused-command-line-argument] + export CFLAGS="-Wno-error=unused-command-line-argument -Werror ${CFLAGS:-}" git clean -dfx && git submodule foreach git clean -dfx # And now a clang build to find unused variables because it does a better # job than gcc for vars with cleanups; perhaps in the future these could From 8456fd5057cb3fbef3e27d53d979b8af33cbbbd4 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 20 Jul 2017 15:37:27 -0400 Subject: [PATCH 31/35] build: Turn off default warnings if we find -Werror specified Our CI runs use `-Werror`; there's no point to our default warning set kicking in, it just bloats the command line output. Closes: #1023 Approved by: jlebon --- configure.ac | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 3fc0fabf..9728dee3 100644 --- a/configure.ac +++ b/configure.ac @@ -28,6 +28,7 @@ AC_SUBST([YEAR_VERSION], [year_version]) AC_SUBST([RELEASE_VERSION], [release_version]) AC_SUBST([PACKAGE_VERSION], [package_version]) +AS_IF([echo "$CFLAGS" | grep -q -E -e '-Werror($| )'], [], [ CC_CHECK_FLAGS_APPEND([WARN_CFLAGS], [CFLAGS], [\ -pipe \ -Wall \ @@ -46,7 +47,7 @@ CC_CHECK_FLAGS_APPEND([WARN_CFLAGS], [CFLAGS], [\ -Werror=misleading-indentation \ -Werror=missing-include-dirs -Werror=aggregate-return \ -Werror=unused-result \ -]) +])]) AC_SUBST(WARN_CFLAGS) AC_MSG_CHECKING([for -fsanitize=address in CFLAGS]) From 0985158be7cf1529ddb349f0a1e3153600f187ad Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 24 Jul 2017 12:25:07 -0400 Subject: [PATCH 32/35] Update libglnx, port some uses to newer APIs Mostly for the latest `-Wmaybe-uninitialized` fix, but while here also port some places to newer APIs. Update submodule: libglnx Closes: #1027 Approved by: jlebon --- libglnx | 2 +- src/libostree/ostree-bootloader-grub2.c | 12 +++++------- src/libostree/ostree-core.c | 6 +++--- src/libostree/ostree-gpg-verifier.c | 8 ++------ src/libostree/ostree-repo-commit.c | 12 +++++------- src/libostree/ostree-repo-pull.c | 16 ++++------------ src/libostree/ostree-repo.c | 7 ++++--- src/libostree/ostree-sysroot-deploy.c | 6 +++--- 8 files changed, 27 insertions(+), 42 deletions(-) diff --git a/libglnx b/libglnx index 607f1775..ea6df95f 160000 --- a/libglnx +++ b/libglnx @@ -1 +1 @@ -Subproject commit 607f1775bb1c626cae1875a957a34802daebe81c +Subproject commit ea6df95f22c8f2973714bdbb8b1accc4e37d4d56 diff --git a/src/libostree/ostree-bootloader-grub2.c b/src/libostree/ostree-bootloader-grub2.c index 63e4b968..86970d36 100644 --- a/src/libostree/ostree-bootloader-grub2.c +++ b/src/libostree/ostree-bootloader-grub2.c @@ -420,15 +420,13 @@ _ostree_bootloader_grub2_write_config (OstreeBootloader *bootloader, } /* Now let's fdatasync() for the new file */ - { glnx_fd_close int new_config_fd = open (gs_file_get_path_cached (new_config_path), O_RDONLY | O_CLOEXEC); - if (new_config_fd < 0) - { - glnx_set_prefix_error_from_errno (error, "Opening %s", gs_file_get_path_cached (new_config_path)); - goto out; - } + { glnx_fd_close int new_config_fd = -1; + if (!glnx_openat_rdonly (AT_FDCWD, gs_file_get_path_cached (new_config_path), TRUE, &new_config_fd, error)) + goto out; + if (fdatasync (new_config_fd) < 0) { - glnx_set_error_from_errno (error); + (void)glnx_throw_errno_prefix (error, "fdatasync"); goto out; } } diff --git a/src/libostree/ostree-core.c b/src/libostree/ostree-core.c index 116f376f..c13d2f2e 100644 --- a/src/libostree/ostree-core.c +++ b/src/libostree/ostree-core.c @@ -775,9 +775,9 @@ ostree_content_file_parse_at (gboolean compressed, GCancellable *cancellable, GError **error) { - int glnx_fd_close fd = openat (parent_dfd, path, O_RDONLY | O_CLOEXEC); - if (fd < 0) - return glnx_throw_errno_prefix (error, "open(%s)", path); + glnx_fd_close int fd = -1; + if (!glnx_openat_rdonly (parent_dfd, path, TRUE, &fd, error)) + return FALSE; struct stat stbuf; if (!glnx_fstat (fd, &stbuf, error)) diff --git a/src/libostree/ostree-gpg-verifier.c b/src/libostree/ostree-gpg-verifier.c index c008bdaf..77594d9b 100644 --- a/src/libostree/ostree-gpg-verifier.c +++ b/src/libostree/ostree-gpg-verifier.c @@ -167,12 +167,8 @@ _ostree_gpg_verifier_check_signature (OstreeGpgVerifier *self, glnx_fd_close int fd = -1; ot_auto_gpgme_data gpgme_data_t kdata = NULL; - fd = openat (AT_FDCWD, path, O_RDONLY | O_CLOEXEC) ; - if (fd < 0) - { - glnx_set_prefix_error_from_errno (error, "Opening %s", path); - goto out; - } + if (!glnx_openat_rdonly (AT_FDCWD, path, TRUE, &fd, error)) + goto out; gpg_error = gpgme_data_new_from_fd (&kdata, fd); if (gpg_error != GPG_ERR_NO_ERROR) diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index bcd9c2d9..1d3d6840 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -2824,18 +2824,16 @@ write_dfd_iter_to_mtree_internal (OstreeRepo *self, while (TRUE) { struct dirent *dent; - struct stat stbuf; - g_autoptr(GFileInfo) child_info = NULL; - const char *loose_checksum; if (!glnx_dirfd_iterator_next_dent (src_dfd_iter, &dent, cancellable, error)) return FALSE; if (dent == NULL) break; - if (fstatat (src_dfd_iter->fd, dent->d_name, &stbuf, AT_SYMLINK_NOFOLLOW) == -1) - return glnx_throw_errno (error); + struct stat stbuf; + if (!glnx_fstatat (src_dfd_iter->fd, dent->d_name, &stbuf, AT_SYMLINK_NOFOLLOW, error)) + return FALSE; - loose_checksum = devino_cache_lookup (self, modifier, stbuf.st_dev, stbuf.st_ino); + const char *loose_checksum = devino_cache_lookup (self, modifier, stbuf.st_dev, stbuf.st_ino); if (loose_checksum) { if (!ostree_mutable_tree_replace_file (mtree, dent->d_name, loose_checksum, @@ -2845,7 +2843,7 @@ write_dfd_iter_to_mtree_internal (OstreeRepo *self, continue; } - child_info = _ostree_stbuf_to_gfileinfo (&stbuf); + g_autoptr(GFileInfo) child_info = _ostree_stbuf_to_gfileinfo (&stbuf); g_file_info_set_name (child_info, dent->d_name); if (S_ISREG (stbuf.st_mode)) diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 33ea489c..1c0f79ea 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -1229,12 +1229,8 @@ meta_fetch_on_complete (GObject *object, if (objtype == OSTREE_OBJECT_TYPE_TOMBSTONE_COMMIT) goto out; - fd = openat (_ostree_fetcher_get_dfd (fetcher), tmp_unlinker.path, O_RDONLY | O_CLOEXEC); - if (fd == -1) - { - glnx_set_error_from_errno (error); - goto out; - } + if (!glnx_openat_rdonly (_ostree_fetcher_get_dfd (fetcher), tmp_unlinker.path, TRUE, &fd, error)) + goto out; /* Now delete it, keeping the fd open as the last reference; see comment in * corresponding content fetch path. @@ -1342,12 +1338,8 @@ static_deltapart_fetch_on_complete (GObject *object, if (!_ostree_fetcher_request_to_tmpfile_finish (fetcher, result, &temp_path, error)) goto out; - fd = openat (_ostree_fetcher_get_dfd (fetcher), temp_path, O_RDONLY | O_CLOEXEC); - if (fd == -1) - { - glnx_set_error_from_errno (error); - goto out; - } + if (!glnx_openat_rdonly (_ostree_fetcher_get_dfd (fetcher), temp_path, TRUE, &fd, error)) + goto out; /* From here on, if we fail to apply the delta, we'll re-fetch it */ if (unlinkat (_ostree_fetcher_get_dfd (fetcher), temp_path, 0) < 0) diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index a009f1c8..896c57bc 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -4718,9 +4718,10 @@ ostree_repo_regenerate_summary (OstreeRepo *self, return FALSE; g_autofree char *superblock = _ostree_get_relative_static_delta_superblock_path ((from && from[0]) ? from : NULL, to); - glnx_fd_close int superblock_file_fd = openat (self->repo_dir_fd, superblock, O_RDONLY | O_CLOEXEC); - if (superblock_file_fd == -1) - return glnx_throw_errno (error); + glnx_fd_close int superblock_file_fd = -1; + + if (!glnx_openat_rdonly (self->repo_dir_fd, superblock, TRUE, &superblock_file_fd, error)) + return FALSE; g_autoptr(GInputStream) in_stream = g_unix_input_stream_new (superblock_file_fd, FALSE); if (!in_stream) diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index bf4424a9..b2b46b36 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -178,9 +178,9 @@ copy_dir_recurse (int src_parent_dfd, if (dent == NULL) break; - if (fstatat (src_dfd_iter.fd, dent->d_name, &child_stbuf, - AT_SYMLINK_NOFOLLOW) != 0) - return glnx_throw_errno (error); + if (!glnx_fstatat (src_dfd_iter.fd, dent->d_name, &child_stbuf, + AT_SYMLINK_NOFOLLOW, error)) + return FALSE; if (S_ISDIR (child_stbuf.st_mode)) { From e09fc83ab3f4b84685eff427acc29642d7d09104 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 25 Jul 2017 10:11:57 -0400 Subject: [PATCH 33/35] lib/core: Add #defines for ref/collection binding These were previously private, but since we expect people to use them, let's add `#define`s like we did for some of the other commit metadata. Closes: #1028 Approved by: jlebon --- src/libostree/ostree-core.h | 27 +++++++++++++++++++++++++++ src/libostree/ostree-repo-private.h | 4 ---- src/libostree/ostree-repo-pull.c | 6 ++++-- src/ostree/ot-builtin-commit.c | 7 ++++--- 4 files changed, 35 insertions(+), 9 deletions(-) diff --git a/src/libostree/ostree-core.h b/src/libostree/ostree-core.h index 1fef003a..92b56bb7 100644 --- a/src/libostree/ostree-core.h +++ b/src/libostree/ostree-core.h @@ -226,6 +226,33 @@ typedef enum { * Since: 2017.7 */ #define OSTREE_COMMIT_META_KEY_ENDOFLIFE "ostree.endoflife" +/** + * OSTREE_COMMIT_META_KEY_REF_BINDING: + * + * GVariant type `as`; each element is a branch name. If this is added to a + * commit, `ostree_repo_pull()` will enforce that the commit was retrieved from + * one of the branch names in this array. This prevents "sidegrade" attacks. + * The rationale for having this support multiple branch names is that it helps + * support a "promotion" model of taking a commit and moving it between development + * and production branches. + * + * Since: 2017.9 + */ +#define OSTREE_COMMIT_META_KEY_REF_BINDING "ostree.ref-binding" +/** + * OSTREE_COMMIT_META_KEY_COLLECTION_BINDING: + * + * GVariant type `s`. If this is added to a commit, `ostree_repo_pull()` + * will enforce that the commit was retrieved from a repository which has + * the same collection ID. See `ostree_repo_set_collection_id()`. + * This is most useful in concert with `OSTREE_COMMIT_META_KEY_REF_BINDING`, + * as it more strongly binds the commit to the repository and branch. + * + * Since: 2017.9 + */ +#ifdef OSTREE_ENABLE_EXPERIMENTAL_API +#define OSTREE_COMMIT_META_KEY_COLLECTION_BINDING "ostree.collection-binding" +#endif _OSTREE_PUBLIC const GVariantType *ostree_metadata_variant_type (OstreeObjectType objtype); diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h index da448bbe..5dd0dea7 100644 --- a/src/libostree/ostree-repo-private.h +++ b/src/libostree/ostree-repo-private.h @@ -56,10 +56,6 @@ G_BEGIN_DECLS * in a summary file. */ #define OSTREE_COMMIT_TIMESTAMP "ostree.commit.timestamp" -/* Well-known keys for the commit metadata */ -#define OSTREE_REF_BINDING "ostree.ref-binding" -#define OSTREE_COLLECTION_BINDING "ostree.collection-binding" - typedef enum { OSTREE_REPO_TEST_ERROR_PRE_COMMIT = (1 << 0) } OstreeRepoTestErrorFlags; diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 1c0f79ea..8ba69580 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -1506,7 +1506,7 @@ verify_bindings (OtPullData *pull_data, g_autoptr(GVariant) metadata = g_variant_get_child_value (commit, 0); g_autofree const char **refs = NULL; if (!g_variant_lookup (metadata, - OSTREE_REF_BINDING, + OSTREE_COMMIT_META_KEY_REF_BINDING, "^a&s", &refs)) { @@ -1555,9 +1555,10 @@ verify_bindings (OtPullData *pull_data, if (remote_collection_id != NULL) { +#ifdef OSTREE_ENABLE_EXPERIMENTAL_API const char *collection_id; if (!g_variant_lookup (metadata, - OSTREE_COLLECTION_BINDING, + OSTREE_COMMIT_META_KEY_COLLECTION_BINDING, "&s", &collection_id)) return glnx_throw (error, @@ -1569,6 +1570,7 @@ verify_bindings (OtPullData *pull_data, "metadata, while the remote it came from has " "collection ID ‘%s’", collection_id, remote_collection_id); +#endif } return TRUE; diff --git a/src/ostree/ot-builtin-commit.c b/src/ostree/ot-builtin-commit.c index 35340a04..8f359380 100644 --- a/src/ostree/ot-builtin-commit.c +++ b/src/ostree/ot-builtin-commit.c @@ -316,7 +316,7 @@ add_collection_binding (OstreeRepo *repo, if (collection_id == NULL) return; - g_variant_builder_add (metadata_builder, "{s@v}", OSTREE_COLLECTION_BINDING, + g_variant_builder_add (metadata_builder, "{s@v}", OSTREE_COMMIT_META_KEY_COLLECTION_BINDING, g_variant_new_variant (g_variant_new_string (collection_id))); } #endif /* OSTREE_ENABLE_EXPERIMENTAL_API */ @@ -345,10 +345,11 @@ add_ref_binding (GVariantBuilder *metadata_builder) g_ptr_array_sort (refs, compare_strings); g_autoptr(GVariant) refs_v = g_variant_new_strv ((const char *const *)refs->pdata, refs->len); - g_variant_builder_add (metadata_builder, "{s@v}", OSTREE_REF_BINDING, + g_variant_builder_add (metadata_builder, "{s@v}", OSTREE_COMMIT_META_KEY_REF_BINDING, g_variant_new_variant (g_steal_pointer (&refs_v))); } +/* Note if you're using the API, you currently need to do this yourself */ static void fill_bindings (OstreeRepo *repo, GVariant *metadata, @@ -363,7 +364,7 @@ fill_bindings (OstreeRepo *repo, /* Allow the collection ID to be overridden using * --add-metadata-string=ostree.collection-binding=blah */ if (metadata == NULL || - !g_variant_lookup (metadata, OSTREE_COLLECTION_BINDING, "*", NULL)) + !g_variant_lookup (metadata, OSTREE_COMMIT_META_KEY_COLLECTION_BINDING, "*", NULL)) add_collection_binding (repo, metadata_builder); #endif /* OSTREE_ENABLE_EXPERIMENTAL_API */ From f1f199578e0364e6627002fa8cb24605d9485ed4 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 26 Jul 2017 15:51:57 -0400 Subject: [PATCH 34/35] ci: Enable libcurl by default on Fedora The insttest fell over since its build used libsoup, but that just dropped out of FAH. Closes: #1030 Approved by: jlebon --- .papr.yml | 5 +---- ci/build.sh | 7 +++++++ 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/.papr.yml b/.papr.yml index ea378c98..0a4e04ee 100644 --- a/.papr.yml +++ b/.papr.yml @@ -12,8 +12,6 @@ packages: - git env: - # At some point soon will encourage distros to use libcurl - CONFIGOPTS: "--with-curl --with-openssl" # Enable all the sanitizers for this primary build. # We only use -Werror=maybe-uninitialized here with a "fixed" toolchain CFLAGS: '-fsanitize=undefined -fsanitize-undefined-trap-on-error -fsanitize=address -O2 -Wp,-D_FORTIFY_SOURCE=2' @@ -85,7 +83,7 @@ required: true context: f26-libsoup env: - CONFIGOPTS: "--without-curl --without-openssl" + CONFIGOPTS: "--without-curl --without-openssl --with-libsoup" tests: - ci/build-check.sh @@ -98,7 +96,6 @@ required: true context: f26-introspection-tests env: - CONFIGOPTS: "--with-curl --with-openssl" # ASAN conflicts with introspection testing; # See https://github.com/ostreedev/ostree/issues/1014 INSTALLED_TESTS_PATTERN: "libostree/test-sizes.js libostree/test-sysroot.js libostree/test-core.js" diff --git a/ci/build.sh b/ci/build.sh index 2d7eb61e..22071764 100755 --- a/ci/build.sh +++ b/ci/build.sh @@ -21,4 +21,11 @@ DETECTED_CONFIGOPTS= if test -x /usr/bin/gnome-desktop-testing-runner; then DETECTED_CONFIGOPTS="${DETECTED_CONFIGOPTS} --enable-installed-tests=exclusive" fi +# Default libcurl on by default in fedora unless libsoup is enabled +if sh -c '. /etc/os-release; test "${ID}" = fedora'; then + case "${CONFIGOPTS:-}" in + *--with-soup*) ;; + *) CONFIGOPTS="${CONFIGOPTS:-} --with-curl" + esac +fi build --enable-gtk-doc ${DETECTED_CONFIGOPTS} ${CONFIGOPTS:-} From fa3e07e5d716d0ab5477d3f93e7a6d2b1fa56d61 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 26 Jul 2017 15:11:41 -0400 Subject: [PATCH 35/35] Release 2017.9 Closes: #1029 Approved by: jlebon --- configure.ac | 2 +- src/libostree/libostree-devel.sym | 2 +- src/libostree/libostree-released.sym | 3 +++ tests/test-symbols.sh | 2 +- 4 files changed, 6 insertions(+), 3 deletions(-) diff --git a/configure.ac b/configure.ac index 9728dee3..01e6b534 100644 --- a/configure.ac +++ b/configure.ac @@ -7,7 +7,7 @@ m4_define([year_version], [2017]) m4_define([release_version], [9]) m4_define([package_version], [year_version.release_version]) AC_INIT([libostree], [package_version], [walters@verbum.org]) -is_release_build=no +is_release_build=yes AC_CONFIG_HEADER([config.h]) AC_CONFIG_MACRO_DIR([buildutil]) AC_CONFIG_AUX_DIR([build-aux]) diff --git a/src/libostree/libostree-devel.sym b/src/libostree/libostree-devel.sym index 68ed4020..c93e388a 100644 --- a/src/libostree/libostree-devel.sym +++ b/src/libostree/libostree-devel.sym @@ -18,7 +18,7 @@ ***/ /* Add new symbols here. Release commits should copy this section into -released.sym. */ -LIBOSTREE_2017.9 { +LIBOSTREE_2017.10 { }; /* Stub section for the stable release *after* this development one; don't diff --git a/src/libostree/libostree-released.sym b/src/libostree/libostree-released.sym index b68d6a5e..0a138966 100644 --- a/src/libostree/libostree-released.sym +++ b/src/libostree/libostree-released.sym @@ -413,6 +413,9 @@ global: ostree_validate_remote_name; } LIBOSTREE_2017.7; +LIBOSTREE_2017.9 { +}; + /* NOTE: Only add more content here in release commits! See the * comments at the top of this file. */ diff --git a/tests/test-symbols.sh b/tests/test-symbols.sh index fcc345f0..e6504c93 100755 --- a/tests/test-symbols.sh +++ b/tests/test-symbols.sh @@ -52,7 +52,7 @@ echo 'ok documented symbols' # ONLY update this checksum in release commits! cat > released-sha256.txt <