From 8d9d3a1d4a5a688b0a7e8dcdc1b8894e853d03e6 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Thu, 11 Aug 2016 16:01:27 -0400 Subject: [PATCH 01/23] pull code: support contenturl setting Allow users to pass a --contenturl during `remote add` and store it in the remote config. Fish out the contenturl setting from the remote config and use it when downloading static deltas and objects (except for commit signatures). The idea here is that items in the trust chain (summary & sigs) can be fetched from a more secure e.g. TLS-pinned location, while objects themselves are fetched from another location. Once mirrorlist support is added, this use-case will become even more advantageous. Closes: #469 Approved by: cgwalters --- src/libostree/ostree-repo-pull.c | 33 +++++++++++++++++++++++++++--- src/ostree/ot-remote-builtin-add.c | 10 +++++++++ 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index ed1c67ef..8589d716 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -47,6 +47,7 @@ typedef struct { OstreeRepoMode remote_mode; OstreeFetcher *fetcher; SoupURI *base_uri; + SoupURI *base_content_uri; OstreeRepo *remote_repo_local; GMainContext *main_context; @@ -1347,7 +1348,7 @@ enqueue_one_object_request (OtPullData *pull_data, else { objpath = _ostree_get_relative_object_path (checksum, objtype, TRUE); - obj_uri = suburi_new (pull_data->base_uri, objpath, NULL); + obj_uri = suburi_new (pull_data->base_content_uri, objpath, NULL); } is_meta = OSTREE_OBJECT_TYPE_IS_META (objtype); @@ -1431,7 +1432,7 @@ request_static_delta_superblock_sync (OtPullData *pull_data, g_autoptr(GVariant) delta_superblock = NULL; SoupURI *target_uri = NULL; - target_uri = suburi_new (pull_data->base_uri, delta_name, NULL); + target_uri = suburi_new (pull_data->base_content_uri, delta_name, NULL); if (!fetch_uri_contents_membuf_sync (pull_data, target_uri, FALSE, TRUE, &delta_superblock_data, @@ -1734,7 +1735,7 @@ process_one_static_delta (OtPullData *pull_data, } else { - target_uri = suburi_new (pull_data->base_uri, deltapart_path, NULL); + target_uri = suburi_new (pull_data->base_content_uri, deltapart_path, NULL); _ostree_fetcher_request_uri_with_partial_async (pull_data->fetcher, target_uri, size, OSTREE_FETCHER_DEFAULT_PRIORITY, pull_data->cancellable, @@ -2382,6 +2383,30 @@ ostree_repo_pull_with_options (OstreeRepo *self, summary_bytes, FALSE); } + { + g_autofree char *contenturl = NULL; + + if (metalink_url_str == NULL && url_override != NULL) + contenturl = g_strdup (url_override); + else if (!ostree_repo_get_remote_option (self, remote_name_or_baseurl, + "contenturl", NULL, + &contenturl, error)) + goto out; + + if (contenturl == NULL) + pull_data->base_content_uri = soup_uri_copy (pull_data->base_uri); + else + { + pull_data->base_content_uri = soup_uri_new (contenturl); + if (!pull_data->base_content_uri) + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, + "Failed to parse contenturl '%s'", contenturl); + goto out; + } + } + } + if (!ostree_repo_get_remote_list_option (self, remote_name_or_baseurl, "branches", &configured_branches, error)) @@ -2901,6 +2926,8 @@ ostree_repo_pull_with_options (OstreeRepo *self, g_free (pull_data->remote_name); if (pull_data->base_uri) soup_uri_free (pull_data->base_uri); + if (pull_data->base_content_uri) + soup_uri_free (pull_data->base_content_uri); g_clear_pointer (&pull_data->summary_data, (GDestroyNotify) g_bytes_unref); g_clear_pointer (&pull_data->summary_data_sig, (GDestroyNotify) g_bytes_unref); g_clear_pointer (&pull_data->summary, (GDestroyNotify) g_variant_unref); diff --git a/src/ostree/ot-remote-builtin-add.c b/src/ostree/ot-remote-builtin-add.c index 461d9099..4e1d5595 100644 --- a/src/ostree/ot-remote-builtin-add.c +++ b/src/ostree/ot-remote-builtin-add.c @@ -30,12 +30,14 @@ static char **opt_set; static gboolean opt_no_gpg_verify; static gboolean opt_if_not_exists; static char *opt_gpg_import; +static char *opt_contenturl; static GOptionEntry option_entries[] = { { "set", 0, 0, G_OPTION_ARG_STRING_ARRAY, &opt_set, "Set config option KEY=VALUE for remote", "KEY=VALUE" }, { "no-gpg-verify", 0, 0, G_OPTION_ARG_NONE, &opt_no_gpg_verify, "Disable GPG verification", NULL }, { "if-not-exists", 0, 0, G_OPTION_ARG_NONE, &opt_if_not_exists, "Do nothing if the provided remote exists", NULL }, { "gpg-import", 0, 0, G_OPTION_ARG_FILENAME, &opt_gpg_import, "Import GPG key from FILE", "FILE" }, + { "contenturl", 0, 0, G_OPTION_ARG_STRING, &opt_contenturl, "Use URL when fetching content", "URL" }, { NULL } }; @@ -83,6 +85,14 @@ ot_remote_builtin_add (int argc, char **argv, GCancellable *cancellable, GError g_variant_new_variant (g_variant_new_strv ((const char*const*)branchesp->pdata, -1))); } + /* We could just make users use --set instead for this since it's a string, + * but e.g. when mirrorlist support is added, it'll be kinda awkward to type: + * --set=contenturl=mirrorlist=... */ + + if (opt_contenturl != NULL) + g_variant_builder_add (optbuilder, "{s@v}", + "contenturl", g_variant_new_variant (g_variant_new_string (opt_contenturl))); + for (iter = opt_set; iter && *iter; iter++) { const char *keyvalue = *iter; From bfa8eaccd30ff9c24a8c8d36c907879e44eb95a3 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Thu, 18 Aug 2016 12:10:20 -0400 Subject: [PATCH 02/23] trivial-httpd: prepend timestamp in log file I've found this useful when monitoring multiple logs at the same time to test the upcoming content & meta URL splitting. Closes: #469 Approved by: cgwalters --- src/ostree/ot-builtin-trivial-httpd.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/ostree/ot-builtin-trivial-httpd.c b/src/ostree/ot-builtin-trivial-httpd.c index 30c593c2..88a1a74b 100644 --- a/src/ostree/ot-builtin-trivial-httpd.c +++ b/src/ostree/ot-builtin-trivial-httpd.c @@ -77,9 +77,15 @@ httpd_log (OtTrivialHttpd *httpd, const gchar *format, ...) if (!httpd->log) return; - str = g_string_new (NULL); + { + g_autoptr(GDateTime) now = g_date_time_new_now_local (); + g_autofree char *timestamp = g_date_time_format (now, "%F %T"); + str = g_string_new (timestamp); + g_string_append_printf (str, ".%06d - ", g_date_time_get_microsecond (now)); + } + va_start (args, format); - g_string_vprintf (str, format, args); + g_string_append_vprintf (str, format, args); va_end (args); g_output_stream_write_all (httpd->log, str->str, str->len, &written, NULL, NULL); From 9546b93382fc5a07ec0f753f66497992f514fd09 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Mon, 22 Aug 2016 16:14:52 -0400 Subject: [PATCH 03/23] pull: drop fetching_sync_uri This made sense back when we used a main loop even when we needed to fetch objects synchronously. Nowadays, we no longer actually update progress before the FETCHING_OBJECTS phase, which is only for async requests. This allows us to get rid of fetch_uri_contents_membuf_sync() and to generalize fetch_uri_contents_utf8_sync() so that it only requires a fetcher. This will be needed later. Closes: #469 Approved by: cgwalters --- src/libostree/ostree-metalink.c | 4 -- src/libostree/ostree-metalink.h | 1 - src/libostree/ostree-repo-pull.c | 93 +++++++++++--------------------- 3 files changed, 31 insertions(+), 67 deletions(-) diff --git a/src/libostree/ostree-metalink.c b/src/libostree/ostree-metalink.c index ad3a6a2a..b850818d 100644 --- a/src/libostree/ostree-metalink.c +++ b/src/libostree/ostree-metalink.c @@ -593,7 +593,6 @@ gboolean _ostree_metalink_request_sync (OstreeMetalink *self, SoupURI **out_target_uri, GBytes **out_data, - SoupURI **fetching_sync_uri, GCancellable *cancellable, GError **error) { @@ -604,9 +603,6 @@ _ostree_metalink_request_sync (OstreeMetalink *self, gsize len; const guint8 *data; - if (fetching_sync_uri != NULL) - *fetching_sync_uri = _ostree_metalink_get_uri (self); - mainctx = g_main_context_new (); g_main_context_push_thread_default (mainctx); diff --git a/src/libostree/ostree-metalink.h b/src/libostree/ostree-metalink.h index abf4c629..8f63d372 100644 --- a/src/libostree/ostree-metalink.h +++ b/src/libostree/ostree-metalink.h @@ -53,7 +53,6 @@ SoupURI *_ostree_metalink_get_uri (OstreeMetalink *self); gboolean _ostree_metalink_request_sync (OstreeMetalink *self, SoupURI **out_target_uri, GBytes **out_data, - SoupURI **fetching_sync_uri, GCancellable *cancellable, GError **error); G_END_DECLS diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 8589d716..37a77cc3 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -62,8 +62,7 @@ typedef struct { OSTREE_PULL_PHASE_FETCHING_OBJECTS } phase; gint n_scanned_metadata; - SoupURI *fetching_sync_uri; - + gboolean gpg_verify; gboolean gpg_verify_summary; gboolean has_tombstone_commits; @@ -246,14 +245,7 @@ update_progress (gpointer user_data) ostree_async_progress_set_uint (pull_data->progress, "outstanding-metadata-fetches", pull_data->n_outstanding_metadata_fetches); ostree_async_progress_set_uint (pull_data->progress, "metadata-fetched", pull_data->n_fetched_metadata); - if (pull_data->fetching_sync_uri) - { - g_autofree char *uri_string = soup_uri_to_string (pull_data->fetching_sync_uri, TRUE); - g_autofree char *status_string = g_strconcat ("Requesting ", uri_string, NULL); - ostree_async_progress_set_status (pull_data->progress, status_string); - } - else - ostree_async_progress_set_status (pull_data->progress, NULL); + ostree_async_progress_set_status (pull_data->progress, NULL); if (pull_data->dry_run) pull_data->dry_run_emitted_progress = TRUE; @@ -274,27 +266,19 @@ pull_termination_condition (OtPullData *pull_data) gboolean current_scan_idle = g_queue_is_empty (&pull_data->scan_object_queue); gboolean current_idle = current_fetch_idle && current_write_idle && current_scan_idle; + /* we only enter the main loop when we're fetching objects */ + g_assert (pull_data->phase == OSTREE_PULL_PHASE_FETCHING_OBJECTS); + if (pull_data->caught_error) return TRUE; if (pull_data->dry_run) return pull_data->dry_run_emitted_progress; - switch (pull_data->phase) - { - case OSTREE_PULL_PHASE_FETCHING_REFS: - if (!pull_data->fetching_sync_uri) - return TRUE; - break; - case OSTREE_PULL_PHASE_FETCHING_OBJECTS: - if (current_idle && !pull_data->fetching_sync_uri) - { - g_debug ("pull: idle, exiting mainloop"); - return TRUE; - } - break; - } - return FALSE; + if (current_idle) + g_debug ("pull: idle, exiting mainloop"); + + return current_idle; } static void @@ -362,30 +346,7 @@ typedef struct { } OstreeFetchUriSyncData; static gboolean -fetch_uri_contents_membuf_sync (OtPullData *pull_data, - SoupURI *uri, - gboolean add_nul, - gboolean allow_noent, - GBytes **out_contents, - GCancellable *cancellable, - GError **error) -{ - gboolean ret; - pull_data->fetching_sync_uri = uri; - ret = _ostree_fetcher_request_uri_to_membuf (pull_data->fetcher, - uri, - add_nul, - allow_noent, - out_contents, - OSTREE_MAX_METADATA_SIZE, - cancellable, - error); - pull_data->fetching_sync_uri = NULL; - return ret; -} - -static gboolean -fetch_uri_contents_utf8_sync (OtPullData *pull_data, +fetch_uri_contents_utf8_sync (OstreeFetcher *fetcher, SoupURI *uri, char **out_contents, GCancellable *cancellable, @@ -396,8 +357,10 @@ fetch_uri_contents_utf8_sync (OtPullData *pull_data, g_autofree char *ret_contents = NULL; gsize len; - if (!fetch_uri_contents_membuf_sync (pull_data, uri, TRUE, FALSE, - &bytes, cancellable, error)) + if (!_ostree_fetcher_request_uri_to_membuf (fetcher, uri, TRUE, + FALSE, &bytes, + OSTREE_MAX_METADATA_SIZE, + cancellable, error)) goto out; ret_contents = g_bytes_unref_to_data (bytes, &len); @@ -586,7 +549,8 @@ fetch_ref_contents (OtPullData *pull_data, target_uri = suburi_new (pull_data->base_uri, "refs", "heads", ref, NULL); - if (!fetch_uri_contents_utf8_sync (pull_data, target_uri, &ret_contents, cancellable, error)) + if (!fetch_uri_contents_utf8_sync (pull_data->fetcher, target_uri, + &ret_contents, cancellable, error)) goto out; g_strchomp (ret_contents); @@ -1398,7 +1362,7 @@ load_remote_repo_config (OtPullData *pull_data, target_uri = suburi_new (pull_data->base_uri, "config", NULL); - if (!fetch_uri_contents_utf8_sync (pull_data, target_uri, &contents, + if (!fetch_uri_contents_utf8_sync (pull_data->fetcher, target_uri, &contents, cancellable, error)) goto out; @@ -1434,9 +1398,11 @@ request_static_delta_superblock_sync (OtPullData *pull_data, target_uri = suburi_new (pull_data->base_content_uri, delta_name, NULL); - if (!fetch_uri_contents_membuf_sync (pull_data, target_uri, FALSE, TRUE, - &delta_superblock_data, - pull_data->cancellable, error)) + if (!_ostree_fetcher_request_uri_to_membuf (pull_data->fetcher, target_uri, + FALSE, TRUE, + &delta_superblock_data, + OSTREE_MAX_METADATA_SIZE, + pull_data->cancellable, error)) goto out; if (delta_superblock_data) @@ -1999,7 +1965,7 @@ _ostree_preload_metadata_file (OstreeRepo *self, OSTREE_MAX_METADATA_SIZE, base_uri); - _ostree_metalink_request_sync (metalink, NULL, out_bytes, NULL, + _ostree_metalink_request_sync (metalink, NULL, out_bytes, cancellable, &local_error); if (g_error_matches (local_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND)) @@ -2368,7 +2334,6 @@ ostree_repo_pull_with_options (OstreeRepo *self, if (! _ostree_metalink_request_sync (metalink, &target_uri, &summary_bytes, - &pull_data->fetching_sync_uri, cancellable, error)) goto out; @@ -2465,8 +2430,10 @@ ostree_repo_pull_with_options (OstreeRepo *self, if (!pull_data->summary_data_sig) { uri = suburi_new (pull_data->base_uri, "summary.sig", NULL); - if (!fetch_uri_contents_membuf_sync (pull_data, uri, FALSE, TRUE, - &bytes_sig, cancellable, error)) + if (!_ostree_fetcher_request_uri_to_membuf (pull_data->fetcher, uri, + FALSE, TRUE, &bytes_sig, + OSTREE_MAX_METADATA_SIZE, + cancellable, error)) goto out; soup_uri_free (uri); } @@ -2487,8 +2454,10 @@ ostree_repo_pull_with_options (OstreeRepo *self, if (!pull_data->summary && !bytes_summary) { uri = suburi_new (pull_data->base_uri, "summary", NULL); - if (!fetch_uri_contents_membuf_sync (pull_data, uri, FALSE, TRUE, - &bytes_summary, cancellable, error)) + if (!_ostree_fetcher_request_uri_to_membuf (pull_data->fetcher, uri, + FALSE, TRUE, &bytes_summary, + OSTREE_MAX_METADATA_SIZE, + cancellable, error)) goto out; soup_uri_free (uri); } From 157d878ce1a500a2b96d079e2259dbdc30419342 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Tue, 23 Aug 2016 15:55:15 -0400 Subject: [PATCH 04/23] pull: add mirrorlist support This commit adds mirrorlist support to the fetcher. Users can now prepend url or/and contenturl by mirrorlist= to interpret the link as a mirrorlist. If an object is not found, the fetcher will automatically try the next mirror in the order given in the list (assuming the order returned by the server is significant). Closes: #469 Approved by: cgwalters --- src/libostree/ostree-fetcher.c | 223 ++++++++++----- src/libostree/ostree-fetcher.h | 33 ++- src/libostree/ostree-repo-pull.c | 443 +++++++++++++++++++---------- src/ostree/ot-remote-builtin-add.c | 2 +- 4 files changed, 461 insertions(+), 240 deletions(-) diff --git a/src/libostree/ostree-fetcher.c b/src/libostree/ostree-fetcher.c index bde6ed9a..09006b71 100644 --- a/src/libostree/ostree-fetcher.c +++ b/src/libostree/ostree-fetcher.c @@ -74,7 +74,9 @@ typedef struct { volatile int ref_count; ThreadClosure *thread_closure; - SoupURI *uri; + GPtrArray *mirrorlist; /* list of base URIs */ + char *filename; /* relative name to fetch or NULL */ + guint mirrorlist_idx; OstreeFetcherState state; @@ -204,7 +206,8 @@ pending_uri_unref (OstreeFetcherPendingURI *pending) g_clear_pointer (&pending->thread_closure, thread_closure_unref); - soup_uri_free (pending->uri); + g_clear_pointer (&pending->mirrorlist, g_ptr_array_unref); + g_free (pending->filename); g_clear_object (&pending->request); g_clear_object (&pending->request_body); g_free (pending->out_tmpfile); @@ -353,6 +356,31 @@ session_thread_process_pending_queue (ThreadClosure *thread_closure) } } +static void +create_pending_soup_request (OstreeFetcherPendingURI *pending, + GError **error) +{ + g_autofree char *uristr = NULL; + SoupURI *next_mirror = NULL; + SoupURI *uri = NULL; + + g_assert (pending->mirrorlist); + g_assert (pending->mirrorlist_idx < pending->mirrorlist->len); + + next_mirror = g_ptr_array_index (pending->mirrorlist, + pending->mirrorlist_idx); + uristr = g_build_filename (soup_uri_get_path (next_mirror), + pending->filename /* may be NULL */, NULL); + uri = soup_uri_copy (next_mirror); + soup_uri_set_path (uri, uristr); + + g_clear_object (&pending->request); + + pending->request = soup_session_request_uri (pending->thread_closure->session, + uri, error); + soup_uri_free (uri); +} + static void session_thread_request_uri (ThreadClosure *thread_closure, gpointer data) @@ -365,10 +393,7 @@ session_thread_request_uri (ThreadClosure *thread_closure, pending = g_task_get_task_data (task); cancellable = g_task_get_cancellable (task); - pending->request = soup_session_request_uri (thread_closure->session, - pending->uri, - &local_error); - + create_pending_soup_request (pending, &local_error); if (local_error != NULL) { g_task_return_error (task, local_error); @@ -384,7 +409,8 @@ session_thread_request_uri (ThreadClosure *thread_closure, } else { - g_autofree char *uristring = soup_uri_to_string (pending->uri, FALSE); + g_autofree char *uristring + = soup_uri_to_string (soup_request_get_uri (pending->request), FALSE); g_autofree char *tmpfile = NULL; struct stat stbuf; gboolean exists; @@ -463,6 +489,8 @@ ostree_fetcher_session_thread (gpointer data) SOUP_SESSION_IDLE_TIMEOUT, 60, NULL); + /* XXX: Now that we have mirrorlist support, we could make this even smarter + * by spreading requests across mirrors. */ g_object_get (closure->session, "max-conns-per-host", &max_conns, NULL); if (max_conns < 8) { @@ -856,7 +884,8 @@ on_stream_read (GObject *object, if (bytes_read > pending->max_size || (bytes_read + pending->current_size) > pending->max_size) { - g_autofree char *uristr = soup_uri_to_string (pending->uri, FALSE); + g_autofree char *uristr = + soup_uri_to_string (soup_request_get_uri (pending->request), FALSE); local_error = g_error_new (G_IO_ERROR, G_IO_ERROR_FAILED, "URI %s exceeded maximum size of %" G_GUINT64_FORMAT " bytes", uristr, pending->max_size); @@ -937,20 +966,43 @@ on_request_sent (GObject *object, } else if (!SOUP_STATUS_IS_SUCCESSFUL (msg->status_code)) { - GIOErrorEnum code; - switch (msg->status_code) + /* is there another mirror we can try? */ + if (pending->mirrorlist_idx + 1 < pending->mirrorlist->len) { - case 404: - case 410: - code = G_IO_ERROR_NOT_FOUND; - break; - default: - code = G_IO_ERROR_FAILED; + pending->mirrorlist_idx++; + create_pending_soup_request (pending, &local_error); + if (local_error != NULL) + goto out; + + (void) g_input_stream_close (pending->request_body, NULL, NULL); + g_queue_insert_sorted (&pending->thread_closure->pending_queue, + g_object_ref (task), pending_task_compare, + NULL); + remove_pending_rerun_queue (pending); + } + else + { + GIOErrorEnum code; + switch (msg->status_code) + { + case 404: + case 410: + code = G_IO_ERROR_NOT_FOUND; + break; + default: + code = G_IO_ERROR_FAILED; + } + + local_error = g_error_new (G_IO_ERROR, code, + "Server returned status %u: %s", + msg->status_code, + soup_status_get_phrase (msg->status_code)); + + if (pending->mirrorlist->len > 1) + g_prefix_error (&local_error, + "All %u mirrors failed. Last error was: ", + pending->mirrorlist->len); } - local_error = g_error_new (G_IO_ERROR, code, - "Server returned status %u: %s", - msg->status_code, - soup_status_get_phrase (msg->status_code)); goto out; } } @@ -1013,27 +1065,30 @@ on_request_sent (GObject *object, } static void -ostree_fetcher_request_uri_internal (OstreeFetcher *self, - SoupURI *uri, - gboolean is_stream, - guint64 max_size, - int priority, - GCancellable *cancellable, - GAsyncReadyCallback callback, - gpointer user_data, - gpointer source_tag) +ostree_fetcher_mirrored_request_internal (OstreeFetcher *self, + GPtrArray *mirrorlist, + const char *filename, + gboolean is_stream, + guint64 max_size, + int priority, + GCancellable *cancellable, + GAsyncReadyCallback callback, + gpointer user_data, + gpointer source_tag) { g_autoptr(GTask) task = NULL; OstreeFetcherPendingURI *pending; g_return_if_fail (OSTREE_IS_FETCHER (self)); - g_return_if_fail (uri != NULL); + g_return_if_fail (mirrorlist != NULL); + g_return_if_fail (mirrorlist->len > 0); /* SoupRequest is created in session thread. */ pending = g_new0 (OstreeFetcherPendingURI, 1); pending->ref_count = 1; pending->thread_closure = thread_closure_ref (self->thread_closure); - pending->uri = soup_uri_copy (uri); + pending->mirrorlist = g_ptr_array_ref (mirrorlist); + pending->filename = g_strdup (filename); pending->max_size = max_size; pending->is_stream = is_stream; @@ -1051,53 +1106,57 @@ ostree_fetcher_request_uri_internal (OstreeFetcher *self, } void -_ostree_fetcher_request_uri_with_partial_async (OstreeFetcher *self, - SoupURI *uri, - guint64 max_size, - int priority, - GCancellable *cancellable, - GAsyncReadyCallback callback, - gpointer user_data) +_ostree_fetcher_mirrored_request_with_partial_async (OstreeFetcher *self, + GPtrArray *mirrorlist, + const char *filename, + guint64 max_size, + int priority, + GCancellable *cancellable, + GAsyncReadyCallback callback, + gpointer user_data) { - ostree_fetcher_request_uri_internal (self, uri, FALSE, max_size, priority, cancellable, - callback, user_data, - _ostree_fetcher_request_uri_with_partial_async); + ostree_fetcher_mirrored_request_internal (self, mirrorlist, filename, FALSE, + max_size, priority, cancellable, + callback, user_data, + _ostree_fetcher_mirrored_request_with_partial_async); } char * -_ostree_fetcher_request_uri_with_partial_finish (OstreeFetcher *self, - GAsyncResult *result, - GError **error) +_ostree_fetcher_mirrored_request_with_partial_finish (OstreeFetcher *self, + GAsyncResult *result, + GError **error) { g_return_val_if_fail (g_task_is_valid (result, self), NULL); g_return_val_if_fail (g_async_result_is_tagged (result, - _ostree_fetcher_request_uri_with_partial_async), NULL); + _ostree_fetcher_mirrored_request_with_partial_async), NULL); return g_task_propagate_pointer (G_TASK (result), error); } static void -ostree_fetcher_stream_uri_async (OstreeFetcher *self, - SoupURI *uri, - guint64 max_size, - int priority, - GCancellable *cancellable, - GAsyncReadyCallback callback, - gpointer user_data) +ostree_fetcher_stream_mirrored_uri_async (OstreeFetcher *self, + GPtrArray *mirrorlist, + const char *filename, + guint64 max_size, + int priority, + GCancellable *cancellable, + GAsyncReadyCallback callback, + gpointer user_data) { - ostree_fetcher_request_uri_internal (self, uri, TRUE, max_size, priority, cancellable, - callback, user_data, - ostree_fetcher_stream_uri_async); + ostree_fetcher_mirrored_request_internal (self, mirrorlist, filename, TRUE, + max_size, priority, cancellable, + callback, user_data, + ostree_fetcher_stream_mirrored_uri_async); } static GInputStream * -ostree_fetcher_stream_uri_finish (OstreeFetcher *self, - GAsyncResult *result, - GError **error) +ostree_fetcher_stream_mirrored_uri_finish (OstreeFetcher *self, + GAsyncResult *result, + GError **error) { g_return_val_if_fail (g_task_is_valid (result, self), NULL); g_return_val_if_fail (g_async_result_is_tagged (result, - ostree_fetcher_stream_uri_async), NULL); + ostree_fetcher_stream_mirrored_uri_async), NULL); return g_task_propagate_pointer (G_TASK (result), error); } @@ -1148,20 +1207,21 @@ fetch_uri_sync_on_complete (GObject *object, { FetchUriSyncData *data = user_data; - data->result_stream = ostree_fetcher_stream_uri_finish ((OstreeFetcher*)object, - result, data->error); + data->result_stream = ostree_fetcher_stream_mirrored_uri_finish ((OstreeFetcher*)object, + result, data->error); data->done = TRUE; } gboolean -_ostree_fetcher_request_uri_to_membuf (OstreeFetcher *fetcher, - SoupURI *uri, - gboolean add_nul, - gboolean allow_noent, - GBytes **out_contents, - guint64 max_size, - GCancellable *cancellable, - GError **error) +_ostree_fetcher_mirrored_request_to_membuf (OstreeFetcher *fetcher, + GPtrArray *mirrorlist, + const char *filename, + gboolean add_nul, + gboolean allow_noent, + GBytes **out_contents, + guint64 max_size, + GCancellable *cancellable, + GError **error) { gboolean ret = FALSE; const guint8 nulchar = 0; @@ -1182,10 +1242,8 @@ _ostree_fetcher_request_uri_to_membuf (OstreeFetcher *fetcher, data.done = FALSE; data.error = error; - ostree_fetcher_stream_uri_async (fetcher, uri, - max_size, - OSTREE_FETCHER_DEFAULT_PRIORITY, - cancellable, + ostree_fetcher_stream_mirrored_uri_async (fetcher, mirrorlist, filename, max_size, + OSTREE_FETCHER_DEFAULT_PRIORITY, cancellable, fetch_uri_sync_on_complete, &data); while (!data.done) g_main_context_iteration (mainctx, TRUE); @@ -1227,3 +1285,22 @@ _ostree_fetcher_request_uri_to_membuf (OstreeFetcher *fetcher, g_clear_object (&(data.result_stream)); return ret; } + +/* Helper for callers who just want to fetch single one-off URIs */ +gboolean +_ostree_fetcher_request_uri_to_membuf (OstreeFetcher *fetcher, + SoupURI *uri, + gboolean add_nul, + gboolean allow_noent, + GBytes **out_contents, + guint64 max_size, + GCancellable *cancellable, + GError **error) +{ + 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, + out_contents, max_size, + cancellable, error); +} diff --git a/src/libostree/ostree-fetcher.h b/src/libostree/ostree-fetcher.h index 60a13755..8cceca51 100644 --- a/src/libostree/ostree-fetcher.h +++ b/src/libostree/ostree-fetcher.h @@ -70,20 +70,31 @@ void _ostree_fetcher_set_tls_database (OstreeFetcher *self, guint64 _ostree_fetcher_bytes_transferred (OstreeFetcher *self); -void _ostree_fetcher_request_uri_with_partial_async (OstreeFetcher *self, - SoupURI *uri, - guint64 max_size, - int priority, - GCancellable *cancellable, - GAsyncReadyCallback callback, - gpointer user_data); +void _ostree_fetcher_mirrored_request_with_partial_async (OstreeFetcher *self, + GPtrArray *mirrorlist, + const char *filename, + guint64 max_size, + int priority, + GCancellable *cancellable, + GAsyncReadyCallback callback, + gpointer user_data); -char *_ostree_fetcher_request_uri_with_partial_finish (OstreeFetcher *self, - GAsyncResult *result, - GError **error); +char *_ostree_fetcher_mirrored_request_with_partial_finish (OstreeFetcher *self, + GAsyncResult *result, + GError **error); + +gboolean _ostree_fetcher_mirrored_request_to_membuf (OstreeFetcher *fetcher, + GPtrArray *mirrorlist, + const char *filename, + gboolean add_nul, + gboolean allow_noent, + GBytes **out_contents, + guint64 max_size, + GCancellable *cancellable, + GError **error); gboolean _ostree_fetcher_request_uri_to_membuf (OstreeFetcher *fetcher, - SoupURI *uri, + SoupURI *uri, gboolean add_nul, gboolean allow_noent, GBytes **out_contents, diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 37a77cc3..2448d51e 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -46,8 +46,8 @@ typedef struct { char *remote_name; OstreeRepoMode remote_mode; OstreeFetcher *fetcher; - SoupURI *base_uri; - SoupURI *base_content_uri; + GPtrArray *meta_mirrorlist; /* List of base URIs for fetching metadata */ + GPtrArray *content_mirrorlist; /* List of base URIs for fetching content */ OstreeRepo *remote_repo_local; GMainContext *main_context; @@ -137,11 +137,6 @@ typedef struct { guint recursion_depth; } ScanObjectQueueData; -static SoupURI * -suburi_new (SoupURI *base, - const char *first, - ...) G_GNUC_NULL_TERMINATED; - static void queue_scan_one_metadata_object (OtPullData *pull_data, const char *csum, OstreeObjectType objtype, @@ -159,39 +154,6 @@ static gboolean scan_one_metadata_object_c (OtPullData *pull_data, GCancellable *cancellable, GError **error); -static SoupURI * -suburi_new (SoupURI *base, - const char *first, - ...) -{ - va_list args; - GPtrArray *arg_array; - const char *arg; - char *subpath; - SoupURI *ret; - - arg_array = g_ptr_array_new (); - g_ptr_array_add (arg_array, (char*)soup_uri_get_path (base)); - g_ptr_array_add (arg_array, (char*)first); - - va_start (args, first); - - while ((arg = va_arg (args, const char *)) != NULL) - g_ptr_array_add (arg_array, (char*)arg); - g_ptr_array_add (arg_array, NULL); - - subpath = g_build_filenamev ((char**)arg_array->pdata); - g_ptr_array_unref (arg_array); - - ret = soup_uri_copy (base); - soup_uri_set_path (ret, subpath); - g_free (subpath); - - va_end (args); - - return ret; -} - static gboolean update_progress (gpointer user_data) { @@ -346,21 +308,23 @@ typedef struct { } OstreeFetchUriSyncData; static gboolean -fetch_uri_contents_utf8_sync (OstreeFetcher *fetcher, - SoupURI *uri, - char **out_contents, - GCancellable *cancellable, - GError **error) +fetch_mirrored_uri_contents_utf8_sync (OstreeFetcher *fetcher, + GPtrArray *mirrorlist, + const char *filename, + char **out_contents, + GCancellable *cancellable, + GError **error) { gboolean ret = FALSE; g_autoptr(GBytes) bytes = NULL; g_autofree char *ret_contents = NULL; gsize len; - if (!_ostree_fetcher_request_uri_to_membuf (fetcher, uri, TRUE, - FALSE, &bytes, - OSTREE_MAX_METADATA_SIZE, - cancellable, error)) + if (!_ostree_fetcher_mirrored_request_to_membuf (fetcher, mirrorlist, + filename, TRUE, FALSE, + &bytes, + OSTREE_MAX_METADATA_SIZE, + cancellable, error)) goto out; ret_contents = g_bytes_unref_to_data (bytes, &len); @@ -379,6 +343,20 @@ fetch_uri_contents_utf8_sync (OstreeFetcher *fetcher, return ret; } +static gboolean +fetch_uri_contents_utf8_sync (OstreeFetcher *fetcher, + SoupURI *uri, + char **out_contents, + GCancellable *cancellable, + GError **error) +{ + g_autoptr(GPtrArray) mirrorlist = g_ptr_array_new (); + g_ptr_array_add (mirrorlist, uri); /* no transfer */ + return fetch_mirrored_uri_contents_utf8_sync (fetcher, mirrorlist, + NULL, out_contents, + cancellable, error); +} + static gboolean write_commitpartial_for (OtPullData *pull_data, const char *checksum, @@ -545,12 +523,14 @@ fetch_ref_contents (OtPullData *pull_data, { gboolean ret = FALSE; g_autofree char *ret_contents = NULL; - SoupURI *target_uri = NULL; + g_autofree char *filename = NULL; - target_uri = suburi_new (pull_data->base_uri, "refs", "heads", ref, NULL); + filename = g_build_filename ("refs", "heads", ref, NULL); - if (!fetch_uri_contents_utf8_sync (pull_data->fetcher, target_uri, - &ret_contents, cancellable, error)) + if (!fetch_mirrored_uri_contents_utf8_sync (pull_data->fetcher, + pull_data->meta_mirrorlist, + filename, &ret_contents, + cancellable, error)) goto out; g_strchomp (ret_contents); @@ -561,8 +541,6 @@ fetch_ref_contents (OtPullData *pull_data, ret = TRUE; ot_transfer_out_value (out_contents, &ret_contents); out: - if (target_uri) - soup_uri_free (target_uri); return ret; } @@ -670,7 +648,7 @@ content_fetch_on_complete (GObject *object, OstreeObjectType objtype; gboolean free_fetch_data = TRUE; - temp_path = _ostree_fetcher_request_uri_with_partial_finish (fetcher, result, error); + temp_path = _ostree_fetcher_mirrored_request_with_partial_finish (fetcher, result, error); if (!temp_path) goto out; @@ -808,7 +786,7 @@ meta_fetch_on_complete (GObject *object, g_debug ("fetch of %s%s complete", checksum_obj, fetch_data->is_detached_meta ? " (detached)" : ""); - temp_path = _ostree_fetcher_request_uri_with_partial_finish (fetcher, result, error); + temp_path = _ostree_fetcher_mirrored_request_with_partial_finish (fetcher, result, error); if (!temp_path) { if (g_error_matches (local_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND)) @@ -953,7 +931,7 @@ static_deltapart_fetch_on_complete (GObject *object, g_debug ("fetch static delta part %s complete", fetch_data->expected_checksum); - temp_path = _ostree_fetcher_request_uri_with_partial_finish (fetcher, result, error); + temp_path = _ostree_fetcher_mirrored_request_with_partial_finish (fetcher, result, error); if (!temp_path) goto out; @@ -1292,12 +1270,12 @@ enqueue_one_object_request (OtPullData *pull_data, gboolean is_detached_meta, gboolean object_is_stored) { - SoupURI *obj_uri = NULL; + g_autofree char *obj_subpath = NULL; gboolean is_meta; FetchObjectData *fetch_data; - g_autofree char *objpath = NULL; guint64 *expected_max_size_p; guint64 expected_max_size; + GPtrArray *mirrorlist = NULL; g_debug ("queuing fetch of %s.%s%s", checksum, ostree_object_type_to_string (objtype), @@ -1307,12 +1285,13 @@ enqueue_one_object_request (OtPullData *pull_data, { char buf[_OSTREE_LOOSE_PATH_MAX]; _ostree_loose_path (buf, checksum, OSTREE_OBJECT_TYPE_COMMIT_META, pull_data->remote_mode); - obj_uri = suburi_new (pull_data->base_uri, "objects", buf, NULL); + obj_subpath = g_build_filename ("objects", buf, NULL); + mirrorlist = pull_data->meta_mirrorlist; } else { - objpath = _ostree_get_relative_object_path (checksum, objtype, TRUE); - obj_uri = suburi_new (pull_data->base_content_uri, objpath, NULL); + obj_subpath = _ostree_get_relative_object_path (checksum, objtype, TRUE); + mirrorlist = pull_data->content_mirrorlist; } is_meta = OSTREE_OBJECT_TYPE_IS_META (objtype); @@ -1340,13 +1319,12 @@ enqueue_one_object_request (OtPullData *pull_data, else expected_max_size = 0; - _ostree_fetcher_request_uri_with_partial_async (pull_data->fetcher, obj_uri, - expected_max_size, - is_meta ? OSTREE_REPO_PULL_METADATA_PRIORITY - : OSTREE_REPO_PULL_CONTENT_PRIORITY, - pull_data->cancellable, - is_meta ? meta_fetch_on_complete : content_fetch_on_complete, fetch_data); - soup_uri_free (obj_uri); + _ostree_fetcher_mirrored_request_with_partial_async (pull_data->fetcher, mirrorlist, + obj_subpath, expected_max_size, + is_meta ? OSTREE_REPO_PULL_METADATA_PRIORITY + : OSTREE_REPO_PULL_CONTENT_PRIORITY, + pull_data->cancellable, + is_meta ? meta_fetch_on_complete : content_fetch_on_complete, fetch_data); } static gboolean @@ -1358,12 +1336,11 @@ load_remote_repo_config (OtPullData *pull_data, gboolean ret = FALSE; g_autofree char *contents = NULL; GKeyFile *ret_keyfile = NULL; - SoupURI *target_uri = NULL; - target_uri = suburi_new (pull_data->base_uri, "config", NULL); - - if (!fetch_uri_contents_utf8_sync (pull_data->fetcher, target_uri, &contents, - cancellable, error)) + if (!fetch_mirrored_uri_contents_utf8_sync (pull_data->fetcher, + pull_data->meta_mirrorlist, + "config", &contents, + cancellable, error)) goto out; ret_keyfile = g_key_file_new (); @@ -1375,7 +1352,6 @@ load_remote_repo_config (OtPullData *pull_data, ot_transfer_out_value (out_keyfile, &ret_keyfile); out: g_clear_pointer (&ret_keyfile, (GDestroyNotify) g_key_file_unref); - g_clear_pointer (&target_uri, (GDestroyNotify) soup_uri_free); return ret; } @@ -1394,17 +1370,15 @@ request_static_delta_superblock_sync (OtPullData *pull_data, g_autoptr(GBytes) delta_superblock_data = NULL; g_autoptr(GBytes) delta_meta_data = NULL; g_autoptr(GVariant) delta_superblock = NULL; - SoupURI *target_uri = NULL; - - target_uri = suburi_new (pull_data->base_content_uri, delta_name, NULL); - - if (!_ostree_fetcher_request_uri_to_membuf (pull_data->fetcher, target_uri, - FALSE, TRUE, - &delta_superblock_data, - OSTREE_MAX_METADATA_SIZE, - pull_data->cancellable, error)) + + if (!_ostree_fetcher_mirrored_request_to_membuf (pull_data->fetcher, + pull_data->content_mirrorlist, + delta_name, FALSE, TRUE, + &delta_superblock_data, + OSTREE_MAX_METADATA_SIZE, + pull_data->cancellable, error)) goto out; - + if (delta_superblock_data) { { @@ -1449,7 +1423,6 @@ request_static_delta_superblock_sync (OtPullData *pull_data, if (out_delta_superblock) *out_delta_superblock = g_steal_pointer (&ret_delta_superblock); out: - g_clear_pointer (&target_uri, (GDestroyNotify) soup_uri_free); return ret; } @@ -1615,7 +1588,6 @@ process_one_static_delta (OtPullData *pull_data, const guchar *csum; g_autoptr(GVariant) header = NULL; gboolean have_all = FALSE; - SoupURI *target_uri = NULL; g_autofree char *deltapart_path = NULL; FetchStaticDeltaData *fetch_data; g_autoptr(GVariant) csum_v = NULL; @@ -1689,7 +1661,7 @@ process_one_static_delta (OtPullData *pull_data, NULL, &inline_delta_part, cancellable, error)) goto out; - + _ostree_static_delta_part_execute_async (pull_data->repo, fetch_data->objects, inline_delta_part, @@ -1701,14 +1673,14 @@ process_one_static_delta (OtPullData *pull_data, } else { - target_uri = suburi_new (pull_data->base_content_uri, deltapart_path, NULL); - _ostree_fetcher_request_uri_with_partial_async (pull_data->fetcher, target_uri, size, - OSTREE_FETCHER_DEFAULT_PRIORITY, - pull_data->cancellable, - static_deltapart_fetch_on_complete, - fetch_data); + _ostree_fetcher_mirrored_request_with_partial_async (pull_data->fetcher, + pull_data->content_mirrorlist, + deltapart_path, size, + OSTREE_FETCHER_DEFAULT_PRIORITY, + pull_data->cancellable, + static_deltapart_fetch_on_complete, + fetch_data); pull_data->n_outstanding_deltapart_fetches++; - soup_uri_free (target_uri); } } @@ -1947,7 +1919,7 @@ out: static gboolean _ostree_preload_metadata_file (OstreeRepo *self, OstreeFetcher *fetcher, - SoupURI *base_uri, + GPtrArray *mirrorlist, const char *filename, gboolean is_metalink, GBytes **out_bytes, @@ -1961,9 +1933,11 @@ _ostree_preload_metadata_file (OstreeRepo *self, glnx_unref_object OstreeMetalink *metalink = NULL; GError *local_error = NULL; + /* the metalink uri is buried in the mirrorlist as the first (and only) + * element */ metalink = _ostree_metalink_new (fetcher, filename, OSTREE_MAX_METADATA_SIZE, - base_uri); + mirrorlist->pdata[0]); _ostree_metalink_request_sync (metalink, NULL, out_bytes, cancellable, &local_error); @@ -1981,20 +1955,11 @@ _ostree_preload_metadata_file (OstreeRepo *self, } else { - SoupURI *uri; - const char *base_path; - g_autofree char *path = NULL; - - base_path = soup_uri_get_path (base_uri); - path = g_build_filename (base_path, filename, NULL); - uri = soup_uri_new_with_base (base_uri, path); - - ret = _ostree_fetcher_request_uri_to_membuf (fetcher, uri, - FALSE, TRUE, - out_bytes, - OSTREE_MAX_METADATA_SIZE, - cancellable, error); - soup_uri_free (uri); + ret = _ostree_fetcher_mirrored_request_to_membuf (fetcher, mirrorlist, + filename, FALSE, TRUE, + out_bytes, + OSTREE_MAX_METADATA_SIZE, + cancellable, error); if (!ret) goto out; @@ -2005,6 +1970,117 @@ out: return ret; } +static gboolean +fetch_mirrorlist (OstreeFetcher *fetcher, + const char *mirrorlist_url, + GPtrArray **out_mirrorlist, + GCancellable *cancellable, + GError **error) +{ + gboolean ret = FALSE; + char **lines = NULL; + g_autofree char *contents = NULL; + SoupURI *mirrorlist = NULL; + g_autoptr(GPtrArray) ret_mirrorlist = + g_ptr_array_new_with_free_func ((GDestroyNotify) soup_uri_free); + + mirrorlist = soup_uri_new (mirrorlist_url); + if (mirrorlist == NULL) + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, + "Failed to parse mirrorlist URL '%s'", mirrorlist_url); + goto out; + } + + if (!fetch_uri_contents_utf8_sync (fetcher, mirrorlist, &contents, + cancellable, error)) + { + g_prefix_error (error, "While fetching mirrorlist '%s': ", + mirrorlist_url); + goto out; + } + + /* go through each mirror in mirrorlist and do a quick sanity check that it + * works so that we don't waste the fetcher's time when it goes through them + * */ + lines = g_strsplit (contents, "\n", -1); + g_debug ("Scanning mirrorlist from '%s'", mirrorlist_url); + for (char **iter = lines; iter && *iter; iter++) + { + const char *mirror_uri_str = *iter; + SoupURI *mirror_uri = NULL; + + /* let's be nice and support empty lines and comments */ + if (*mirror_uri_str == '\0' || *mirror_uri_str == '#') + continue; + + mirror_uri = soup_uri_new (mirror_uri_str); + if (mirror_uri == NULL) + { + g_debug ("Can't parse mirrorlist line '%s'", mirror_uri_str); + continue; + } + else if ((strcmp (soup_uri_get_scheme (mirror_uri), "http") != 0) && + (strcmp (soup_uri_get_scheme (mirror_uri), "https") != 0)) + { + /* let's not support mirrorlists that contain non-http based URIs for + * now (e.g. local URIs) -- we need to think about if and how we want + * to support this since we set up things differently depending on + * whether we're pulling locally or not */ + g_debug ("Ignoring non-http/s mirrorlist entry '%s'", mirror_uri_str); + soup_uri_free (mirror_uri); + continue; + } + + /* We keep sanity checking until we hit a working mirror; there's no need + * to waste resources checking the remaining ones. At the same time, + * guaranteeing that the first mirror in the list works saves the fetcher + * time from always iterating through a few bad first mirrors. */ + if (ret_mirrorlist->len == 0) + { + GError *local_error = NULL; + g_autofree char *config_uri_str = g_build_filename (mirror_uri_str, + "config", NULL); + SoupURI *config_uri = soup_uri_new (config_uri_str); + + if (fetch_uri_contents_utf8_sync (fetcher, config_uri, NULL, + cancellable, &local_error)) + g_ptr_array_add (ret_mirrorlist, g_steal_pointer (&mirror_uri)); + else + { + g_debug ("Failed to fetch config from mirror '%s': %s", + mirror_uri_str, local_error->message); + g_clear_error (&local_error); + } + + soup_uri_free (config_uri); + } + else + { + g_ptr_array_add (ret_mirrorlist, g_steal_pointer (&mirror_uri)); + } + + if (mirror_uri != NULL) + soup_uri_free (mirror_uri); + } + + if (ret_mirrorlist->len == 0) + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, + "No valid mirrors were found in mirrorlist '%s'", + mirrorlist_url); + goto out; + } + + *out_mirrorlist = g_steal_pointer (&ret_mirrorlist); + ret = TRUE; + +out: + if (mirrorlist != NULL) + soup_uri_free (mirrorlist); + return ret; +} + static gboolean repo_remote_fetch_summary (OstreeRepo *self, const char *name, @@ -2018,9 +2094,9 @@ repo_remote_fetch_summary (OstreeRepo *self, glnx_unref_object OstreeFetcher *fetcher = NULL; g_autoptr(GMainContext) mainctx = NULL; gboolean ret = FALSE; - SoupURI *base_uri = NULL; gboolean from_cache = FALSE; g_autofree char *url_override = NULL; + g_autoptr(GPtrArray) mirrorlist = NULL; if (options) (void) g_variant_lookup (options, "override-url", "&s", &url_override); @@ -2041,18 +2117,33 @@ repo_remote_fetch_summary (OstreeRepo *self, else if (!ostree_repo_remote_get_url (self, name, &url_string, error)) goto out; - base_uri = soup_uri_new (url_string); - if (base_uri == NULL) + if (metalink_url_string == NULL && + g_str_has_prefix (url_string, "mirrorlist=")) { - g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, - "Invalid URL '%s'", url_string); - goto out; + if (!fetch_mirrorlist (fetcher, url_string + strlen ("mirrorlist="), + &mirrorlist, cancellable, error)) + goto out; + } + else + { + SoupURI *uri = soup_uri_new (url_string); + + if (uri == NULL) + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, + "Failed to parse url '%s'", url_string); + goto out; + } + + mirrorlist = + g_ptr_array_new_with_free_func ((GDestroyNotify) soup_uri_free); + g_ptr_array_add (mirrorlist, uri /* transfer ownership */ ); } } if (!_ostree_preload_metadata_file (self, fetcher, - base_uri, + mirrorlist, "summary.sig", metalink_url_string ? TRUE : FALSE, out_signatures, @@ -2077,7 +2168,7 @@ repo_remote_fetch_summary (OstreeRepo *self, { if (!_ostree_preload_metadata_file (self, fetcher, - base_uri, + mirrorlist, "summary", metalink_url_string ? TRUE : FALSE, out_summary, @@ -2112,8 +2203,6 @@ repo_remote_fetch_summary (OstreeRepo *self, out: if (mainctx) g_main_context_pop_thread_default (mainctx); - if (base_uri != NULL) - soup_uri_free (base_uri); return ret; } @@ -2181,6 +2270,8 @@ ostree_repo_pull_with_options (OstreeRepo *self, gboolean opt_gpg_verify_set = FALSE; gboolean opt_gpg_verify_summary_set = FALSE; const char *url_override = NULL; + g_autofree char *base_meta_url = NULL; + g_autofree char *base_content_url = NULL; if (options) { @@ -2305,13 +2396,28 @@ ostree_repo_pull_with_options (OstreeRepo *self, else if (!ostree_repo_remote_get_url (self, remote_name_or_baseurl, &baseurl, error)) goto out; - pull_data->base_uri = soup_uri_new (baseurl); - - if (!pull_data->base_uri) + if (g_str_has_prefix (baseurl, "mirrorlist=")) { - g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, - "Failed to parse url '%s'", baseurl); - goto out; + if (!fetch_mirrorlist (pull_data->fetcher, + baseurl + strlen ("mirrorlist="), + &pull_data->meta_mirrorlist, + cancellable, error)) + goto out; + } + else + { + SoupURI *baseuri = soup_uri_new (baseurl); + + if (baseuri == NULL) + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, + "Failed to parse url '%s'", baseurl); + goto out; + } + + pull_data->meta_mirrorlist = + g_ptr_array_new_with_free_func ((GDestroyNotify) soup_uri_free); + g_ptr_array_add (pull_data->meta_mirrorlist, baseuri /* transfer */); } } else @@ -2338,10 +2444,16 @@ ostree_repo_pull_with_options (OstreeRepo *self, error)) goto out; + /* XXX: would be interesting to implement metalink as another source of + * mirrors here since we use it as such anyway (rather than the "usual" + * use case of metalink, which is only for a single target filename) */ { + /* reuse target_uri and take ownership */ g_autofree char *repo_base = g_path_get_dirname (soup_uri_get_path (target_uri)); - pull_data->base_uri = soup_uri_copy (target_uri); - soup_uri_set_path (pull_data->base_uri, repo_base); + soup_uri_set_path (target_uri, repo_base); + pull_data->meta_mirrorlist = + g_ptr_array_new_with_free_func ((GDestroyNotify) soup_uri_free); + g_ptr_array_add (pull_data->meta_mirrorlist, target_uri); } pull_data->summary = g_variant_new_from_bytes (OSTREE_SUMMARY_GVARIANT_FORMAT, @@ -2359,15 +2471,34 @@ ostree_repo_pull_with_options (OstreeRepo *self, goto out; if (contenturl == NULL) - pull_data->base_content_uri = soup_uri_copy (pull_data->base_uri); + /* this is a bit hacky but greatly simplifies coding elsewhere; we take + * care in the out path to not double free if they're the same list */ + pull_data->content_mirrorlist = pull_data->meta_mirrorlist; else { - pull_data->base_content_uri = soup_uri_new (contenturl); - if (!pull_data->base_content_uri) + if (g_str_has_prefix (contenturl, "mirrorlist=")) { - g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, - "Failed to parse contenturl '%s'", contenturl); - goto out; + if (!fetch_mirrorlist (pull_data->fetcher, + contenturl + strlen ("mirrorlist="), + &pull_data->content_mirrorlist, + cancellable, error)) + goto out; + } + else + { + SoupURI *contenturi = soup_uri_new (contenturl); + + if (contenturi == NULL) + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, + "Failed to parse contenturl '%s'", contenturl); + goto out; + } + + pull_data->content_mirrorlist = + g_ptr_array_new_with_free_func ((GDestroyNotify) soup_uri_free); + g_ptr_array_add (pull_data->content_mirrorlist, + contenturi /* transfer */); } } } @@ -2377,9 +2508,12 @@ ostree_repo_pull_with_options (OstreeRepo *self, &configured_branches, error)) goto out; - if (strcmp (soup_uri_get_scheme (pull_data->base_uri), "file") == 0) + /* NB: we don't support local mirrors in mirrorlists, so if this passes, it + * means that we're not using mirrorlists (see also fetch_mirrorlist()) */ + if (strcmp (soup_uri_get_scheme (pull_data->meta_mirrorlist->pdata[0]), "file") == 0) { - g_autoptr(GFile) remote_repo_path = g_file_new_for_path (soup_uri_get_path (pull_data->base_uri)); + g_autoptr(GFile) remote_repo_path = + g_file_new_for_path (soup_uri_get_path (pull_data->meta_mirrorlist->pdata[0])); pull_data->remote_repo_local = ostree_repo_new (remote_repo_path); if (!ostree_repo_open (pull_data->remote_repo_local, cancellable, error)) goto out; @@ -2418,7 +2552,6 @@ ostree_repo_pull_with_options (OstreeRepo *self, pull_data->static_delta_superblocks = g_ptr_array_new_with_free_func ((GDestroyNotify)g_variant_unref); { - SoupURI *uri = NULL; g_autoptr(GBytes) bytes_sig = NULL; g_autofree char *ret_contents = NULL; gsize i, n; @@ -2429,13 +2562,13 @@ ostree_repo_pull_with_options (OstreeRepo *self, if (!pull_data->summary_data_sig) { - uri = suburi_new (pull_data->base_uri, "summary.sig", NULL); - if (!_ostree_fetcher_request_uri_to_membuf (pull_data->fetcher, uri, - FALSE, TRUE, &bytes_sig, - OSTREE_MAX_METADATA_SIZE, - cancellable, error)) + if (!_ostree_fetcher_mirrored_request_to_membuf (pull_data->fetcher, + pull_data->meta_mirrorlist, + "summary.sig", FALSE, TRUE, + &bytes_sig, + OSTREE_MAX_METADATA_SIZE, + cancellable, error)) goto out; - soup_uri_free (uri); } if (bytes_sig && @@ -2453,13 +2586,13 @@ ostree_repo_pull_with_options (OstreeRepo *self, if (!pull_data->summary && !bytes_summary) { - uri = suburi_new (pull_data->base_uri, "summary", NULL); - if (!_ostree_fetcher_request_uri_to_membuf (pull_data->fetcher, uri, - FALSE, TRUE, &bytes_summary, - OSTREE_MAX_METADATA_SIZE, - cancellable, error)) + if (!_ostree_fetcher_mirrored_request_to_membuf (pull_data->fetcher, + pull_data->meta_mirrorlist, + "summary", FALSE, TRUE, + &bytes_summary, + OSTREE_MAX_METADATA_SIZE, + cancellable, error)) goto out; - soup_uri_free (uri); } if (!bytes_summary && pull_data->gpg_verify_summary) @@ -2893,10 +3026,10 @@ ostree_repo_pull_with_options (OstreeRepo *self, g_clear_object (&pull_data->cancellable); g_clear_object (&pull_data->remote_repo_local); g_free (pull_data->remote_name); - if (pull_data->base_uri) - soup_uri_free (pull_data->base_uri); - if (pull_data->base_content_uri) - soup_uri_free (pull_data->base_content_uri); + if (pull_data->content_mirrorlist != pull_data->meta_mirrorlist) + g_clear_pointer (&pull_data->content_mirrorlist, (GDestroyNotify) g_ptr_array_unref); + /* we clear this *after* clearing content_mirrorlist to avoid unref'ing twice */ + g_clear_pointer (&pull_data->meta_mirrorlist, (GDestroyNotify) g_ptr_array_unref); g_clear_pointer (&pull_data->summary_data, (GDestroyNotify) g_bytes_unref); g_clear_pointer (&pull_data->summary_data_sig, (GDestroyNotify) g_bytes_unref); g_clear_pointer (&pull_data->summary, (GDestroyNotify) g_variant_unref); diff --git a/src/ostree/ot-remote-builtin-add.c b/src/ostree/ot-remote-builtin-add.c index 4e1d5595..76b0c75a 100644 --- a/src/ostree/ot-remote-builtin-add.c +++ b/src/ostree/ot-remote-builtin-add.c @@ -54,7 +54,7 @@ ot_remote_builtin_add (int argc, char **argv, GCancellable *cancellable, GError g_autoptr(GVariantBuilder) optbuilder = NULL; gboolean ret = FALSE; - context = g_option_context_new ("NAME URL [BRANCH...] - Add a remote repository"); + 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)) From c4c030cc79728820eee6cd19d58f9826a0eab734 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Thu, 25 Aug 2016 12:14:21 -0400 Subject: [PATCH 05/23] libtest: add has_gpgme() helper function Closes: #469 Approved by: cgwalters --- tests/libtest.sh | 4 ++++ tests/test-commit-sign.sh | 6 +++--- tests/test-gpg-signed-commit.sh | 6 +++--- tests/test-pull-mirror-summary.sh | 2 +- tests/test-pull-summary-sigs.sh | 2 +- 5 files changed, 12 insertions(+), 8 deletions(-) diff --git a/tests/libtest.sh b/tests/libtest.sh index 958f6769..bc114bfa 100755 --- a/tests/libtest.sh +++ b/tests/libtest.sh @@ -420,6 +420,10 @@ skip_without_fuse () { [ -e /etc/mtab ] || skip "no /etc/mtab" } +has_gpgme () { + ${CMD_PREFIX} ostree --version | grep -q -e '\+gpgme' +} + libtest_cleanup_gpg () { gpg-connect-agent --homedir ${test_tmpdir}/gpghome killagent /bye || true } diff --git a/tests/test-commit-sign.sh b/tests/test-commit-sign.sh index e469c0f9..01eb45f8 100755 --- a/tests/test-commit-sign.sh +++ b/tests/test-commit-sign.sh @@ -19,13 +19,13 @@ set -euo pipefail -if ! ostree --version | grep -q -e '\+gpgme'; then +. $(dirname $0)/libtest.sh + +if ! has_gpgme; then echo "1..0 #SKIP no gpg support compiled in" exit 0 fi -. $(dirname $0)/libtest.sh - echo "1..1" keyid="472CDAFA" diff --git a/tests/test-gpg-signed-commit.sh b/tests/test-gpg-signed-commit.sh index fa42d677..ad022b2b 100755 --- a/tests/test-gpg-signed-commit.sh +++ b/tests/test-gpg-signed-commit.sh @@ -20,13 +20,13 @@ set -euo pipefail -if ! ostree --version | grep -q -e '\+gpgme'; then +. $(dirname $0)/libtest.sh + +if ! has_gpgme; then echo "1..0 #SKIP no gpgme support compiled in" exit 0 fi -. $(dirname $0)/libtest.sh - echo "1..1" setup_test_repository "archive-z2" diff --git a/tests/test-pull-mirror-summary.sh b/tests/test-pull-mirror-summary.sh index 49aa91ad..1ef6a09c 100755 --- a/tests/test-pull-mirror-summary.sh +++ b/tests/test-pull-mirror-summary.sh @@ -61,7 +61,7 @@ find repo/objects -name '*.filez' | while read name; do done echo "ok pull mirror summary" -if ! ${CMD_PREFIX} ostree --version | grep -q -e '\+gpgme'; then +if ! has_gpgme; then exit 0; fi diff --git a/tests/test-pull-summary-sigs.sh b/tests/test-pull-summary-sigs.sh index 65822d23..2dcfbd65 100755 --- a/tests/test-pull-summary-sigs.sh +++ b/tests/test-pull-summary-sigs.sh @@ -54,7 +54,7 @@ assert_file_has_content yet-another-copy/yet-another-hello-world "hello world ye ${CMD_PREFIX} ostree --repo=repo fsck echo "ok pull mirror summary" -if ! ${CMD_PREFIX} ostree --version | grep -q -e '\+gpgme'; then +if ! has_gpgme; then exit 0; fi From 661e4636f51823d56412fa09ae75697d9294057b Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Thu, 25 Aug 2016 15:34:34 -0400 Subject: [PATCH 06/23] tests: add tests for contenturl and mirrorlist Closes: #469 Approved by: cgwalters --- Makefile-tests.am | 2 + tests/test-pull-contenturl.sh | 75 ++++++++++++++++++++++++ tests/test-pull-mirrorlist.sh | 106 ++++++++++++++++++++++++++++++++++ 3 files changed, 183 insertions(+) create mode 100755 tests/test-pull-contenturl.sh create mode 100755 tests/test-pull-mirrorlist.sh diff --git a/Makefile-tests.am b/Makefile-tests.am index 80f60add..7d3f4011 100644 --- a/Makefile-tests.am +++ b/Makefile-tests.am @@ -88,6 +88,8 @@ dist_test_scripts = \ tests/test-refs.sh \ tests/test-demo-buildsystem.sh \ tests/test-switchroot.sh \ + tests/test-pull-contenturl.sh \ + tests/test-pull-mirrorlist.sh \ $(NULL) if BUILDOPT_FUSE diff --git a/tests/test-pull-contenturl.sh b/tests/test-pull-contenturl.sh new file mode 100755 index 00000000..16dcbe4f --- /dev/null +++ b/tests/test-pull-contenturl.sh @@ -0,0 +1,75 @@ +#!/bin/bash +# +# Copyright (C) 2016 Red Hat, 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..2" + +COMMIT_SIGN="" +if has_gpgme; then + COMMIT_SIGN="--gpg-homedir=${TEST_GPG_KEYHOME} --gpg-sign=${TEST_GPG_KEYID_1}" +fi + +setup_fake_remote_repo1 "archive-z2" "${COMMIT_SIGN}" + +# create a summary +${CMD_PREFIX} ostree --repo=${test_tmpdir}/ostree-srv/gnomerepo \ + summary -u ${COMMIT_SIGN} + +# Let's bring up an identical server in which meta files are missing +cd ${test_tmpdir} +mkdir httpd-content +cd httpd-content +cp -a ${test_tmpdir}/ostree-srv ostree + +# delete all the meta stuff from here +rm ostree/gnomerepo/summary +if has_gpgme; then + rm ostree/gnomerepo/summary.sig + find ostree/gnomerepo/objects -name '*.commitmeta' | xargs rm +fi + +# delete all the content stuff from there +find ${test_tmpdir}/ostree-srv/gnomerepo/objects \ + ! -name '*.commitmeta' -type f | xargs rm + +${CMD_PREFIX} ostree trivial-httpd --autoexit --daemonize \ + -p ${test_tmpdir}/httpd-content-port +content_port=$(cat ${test_tmpdir}/httpd-content-port) +echo "http://127.0.0.1:${content_port}" > ${test_tmpdir}/httpd-content-address + +cd ${test_tmpdir} +mkdir repo +${CMD_PREFIX} ostree --repo=repo init +if has_gpgme; then VERIFY=true; else VERIFY=false; fi +${CMD_PREFIX} ostree --repo=repo remote add origin \ + --set=gpg-verify=$VERIFY --set=gpg-verify-summary=$VERIFY \ + --contenturl=$(cat httpd-content-address)/ostree/gnomerepo \ + $(cat httpd-address)/ostree/gnomerepo +${CMD_PREFIX} ostree --repo=repo pull origin:main + +echo "ok pull objects from contenturl" + +if ! has_gpgme; then + echo "ok don't pull sigs from contenturl # SKIP not compiled with gpgme" +else + echo "ok don't pull sigs from contenturl" +fi diff --git a/tests/test-pull-mirrorlist.sh b/tests/test-pull-mirrorlist.sh new file mode 100755 index 00000000..454014ca --- /dev/null +++ b/tests/test-pull-mirrorlist.sh @@ -0,0 +1,106 @@ +#!/bin/bash +# +# Copyright (C) 2016 Red Hat, 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..3" + +setup_fake_remote_repo1 "archive-z2" + +setup_mirror () { + name=$1; shift + + cd ${test_tmpdir} + mkdir $name + cd $name + cp -a ${test_tmpdir}/ostree-srv ostree + + ${CMD_PREFIX} ostree trivial-httpd --autoexit --daemonize \ + -p ${test_tmpdir}/${name}-port + port=$(cat ${test_tmpdir}/${name}-port) + echo "http://127.0.0.1:${port}" > ${test_tmpdir}/${name}-address +} + +setup_mirror content_mirror1 +setup_mirror content_mirror2 +setup_mirror content_mirror3 + +# Let's delete a file from 1 so that it falls back on 2 +cd ${test_tmpdir}/content_mirror1/ostree/gnomerepo +filez=$(find objects/ -name '*.filez' | head -n 1) +rm ${filez} + +# Let's delete a file from 1 and 2 so that it falls back on 3 +cd ${test_tmpdir}/content_mirror1/ostree/gnomerepo +filez=$(find objects/ -name '*.filez' | head -n 1) +rm ${filez} +cd ${test_tmpdir}/content_mirror2/ostree/gnomerepo +rm ${filez} + +# OK, let's just shove the mirrorlist in the first httpd +cat > ${test_tmpdir}/ostree-srv/mirrorlist < Date: Wed, 31 Aug 2016 13:16:36 -0400 Subject: [PATCH 07/23] pull code: clean up mirrorlist hack While converting the mirrorlist code from using GSList to GPtrArray, I completely missed the fact that there is now a much cleaner way to do this. Closes: #484 Approved by: cgwalters --- src/libostree/ostree-repo-pull.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 2448d51e..76c29272 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -2471,9 +2471,10 @@ ostree_repo_pull_with_options (OstreeRepo *self, goto out; if (contenturl == NULL) - /* this is a bit hacky but greatly simplifies coding elsewhere; we take - * care in the out path to not double free if they're the same list */ - pull_data->content_mirrorlist = pull_data->meta_mirrorlist; + { + pull_data->content_mirrorlist = + g_ptr_array_ref (pull_data->meta_mirrorlist); + } else { if (g_str_has_prefix (contenturl, "mirrorlist=")) @@ -3026,10 +3027,8 @@ ostree_repo_pull_with_options (OstreeRepo *self, g_clear_object (&pull_data->cancellable); g_clear_object (&pull_data->remote_repo_local); g_free (pull_data->remote_name); - if (pull_data->content_mirrorlist != pull_data->meta_mirrorlist) - g_clear_pointer (&pull_data->content_mirrorlist, (GDestroyNotify) g_ptr_array_unref); - /* we clear this *after* clearing content_mirrorlist to avoid unref'ing twice */ g_clear_pointer (&pull_data->meta_mirrorlist, (GDestroyNotify) g_ptr_array_unref); + g_clear_pointer (&pull_data->content_mirrorlist, (GDestroyNotify) g_ptr_array_unref); g_clear_pointer (&pull_data->summary_data, (GDestroyNotify) g_bytes_unref); g_clear_pointer (&pull_data->summary_data_sig, (GDestroyNotify) g_bytes_unref); g_clear_pointer (&pull_data->summary, (GDestroyNotify) g_variant_unref); From 973ec467ceffeeead31689167df67945e72e7273 Mon Sep 17 00:00:00 2001 From: Dan Nicholson Date: Thu, 1 Sep 2016 11:26:48 -0700 Subject: [PATCH 08/23] build: Set --enable-man during distcheck If xsltproc is not installed, then ENABLE_MAN will be false and the generated man pages won't be distributed. Pass --enable-man to enforce that the man pages will be generated and distributed. Closes: #486 Approved by: cgwalters --- Makefile.am | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Makefile.am b/Makefile.am index 495a5930..2911a0cf 100644 --- a/Makefile.am +++ b/Makefile.am @@ -29,7 +29,11 @@ AM_CPPFLAGS += -DDATADIR='"$(datadir)"' -DLIBEXECDIR='"$(libexecdir)"' \ -DGLIB_VERSION_MIN_REQUIRED=GLIB_VERSION_2_40 -DGLIB_VERSION_MAX_ALLOWED=GLIB_VERSION_2_40 \ -DSOUP_VERSION_MIN_REQUIRED=SOUP_VERSION_2_40 -DSOUP_VERSION_MAX_ALLOWED=SOUP_VERSION_2_48 AM_CFLAGS += -std=gnu99 $(WARN_CFLAGS) -AM_DISTCHECK_CONFIGURE_FLAGS += --enable-gtk-doc --disable-maintainer-mode +AM_DISTCHECK_CONFIGURE_FLAGS += \ + --enable-gtk-doc \ + --enable-man \ + --disable-maintainer-mode \ + $(NULL) GITIGNOREFILES = aclocal.m4 build-aux/ buildutil/*.m4 config.h.in gtk-doc.make From 7adc9435969009b02ec2d6f9e647a1b88fc77d79 Mon Sep 17 00:00:00 2001 From: Dan Nicholson Date: Thu, 1 Sep 2016 11:29:26 -0700 Subject: [PATCH 09/23] build: Distribute man page XML source Without this, the manual pages can't actually be regenerated from a dist tarball, and running make clean will remove all traces of them. Closes: #486 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 ce7e93cd..99d67753 100644 --- a/Makefile-man.am +++ b/Makefile-man.am @@ -40,7 +40,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) +EXTRA_DIST += $(man1_MANS) $(man5_MANS) $(man1_MANS:=.xml) $(man5_MANS:=.xml) XSLT_STYLESHEET = http://docbook.sourceforge.net/release/xsl/current/manpages/docbook.xsl From 39918179d1e1bb8bc8708695f63525e511cfb8f0 Mon Sep 17 00:00:00 2001 From: Dan Nicholson Date: Thu, 1 Sep 2016 11:29:26 -0700 Subject: [PATCH 10/23] build: Actually distribute man page XML source The make substitution pattern was wrong. The source files are "ostree.xml", not "ostree.1.xml", for instance. Closes: #488 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 99d67753..bdc78947 100644 --- a/Makefile-man.am +++ b/Makefile-man.am @@ -40,7 +40,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:=.xml) $(man5_MANS:=.xml) +EXTRA_DIST += $(man1_MANS) $(man5_MANS) $(man1_MANS:.1=.xml) $(man5_MANS:.5=.xml) XSLT_STYLESHEET = http://docbook.sourceforge.net/release/xsl/current/manpages/docbook.xsl From 4d3b93e2ad1041fa36e740cba35284d17752e928 Mon Sep 17 00:00:00 2001 From: William Manley Date: Wed, 31 Aug 2016 17:25:22 +0100 Subject: [PATCH 11/23] switchroot: Fix build on Ubuntu Was failing with error: src/switchroot/ostree-prepare-root.c:30:20: fatal error: config.h: No such file or directory compilation terminated. Reported by and fix provided by @gatispaeglis. Closes: #485 Approved by: cgwalters --- Makefile-switchroot.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile-switchroot.am b/Makefile-switchroot.am index 9c215e44..0b30a965 100644 --- a/Makefile-switchroot.am +++ b/Makefile-switchroot.am @@ -38,7 +38,7 @@ if BUILDOPT_USE_STATIC_COMPILER ostree_boot_SCRIPTS = ostree-prepare-root ostree-prepare-root : $(ostree_prepare_root_SOURCES) - $(STATIC_COMPILER) -o $@ -static $(ostree_prepare_root_SOURCES) $(AM_CPPFLAGS) $(AM_CFLAGS) + $(STATIC_COMPILER) -o $@ -static $(ostree_prepare_root_SOURCES) $(AM_CPPFLAGS) $(AM_CFLAGS) $(DEFAULT_INCLUDES) else ostree_boot_PROGRAMS += ostree-prepare-root From 598e3240ff523ca827d225271b6e68da90a5e572 Mon Sep 17 00:00:00 2001 From: William Manley Date: Wed, 31 Aug 2016 18:14:19 +0100 Subject: [PATCH 12/23] switchroot: Fix test-switchroot now autotools can build static This test previously depended on manually building ostree-prepare-root. Since 42dab85 we've been able to build static binaries with the usual autotools build-system. This change reflects the fact that `ostree-prepare-root` is built into $srcdir rather than `src/switchroot` where I was building manually. This test now passes with `./configure --with-static-compiler=gcc` (glibc) but still fails with `./configure --with-static-compiler=musl-gcc` (musl). Closes: #485 Approved by: cgwalters --- tests/test-switchroot.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test-switchroot.sh b/tests/test-switchroot.sh index 17085140..96c849e5 100755 --- a/tests/test-switchroot.sh +++ b/tests/test-switchroot.sh @@ -6,7 +6,7 @@ setup_bootfs() { mkdir -p "$1/proc" "$1/bin" echo "quiet ostree=/ostree/boot.0 ro" >"$1/proc/cmdline" touch "$1/this_is_bootfs" - cp "$(dirname "$this_script")/../src/switchroot/ostree-prepare-root" "$1/bin" + cp "$(dirname "$this_script")/../ostree-prepare-root" "$1/bin" } setup_rootfs() { From 5424404813a3f9ca65d1d9e7a4b2efbaebef71cf Mon Sep 17 00:00:00 2001 From: William Manley Date: Wed, 31 Aug 2016 17:07:17 +0100 Subject: [PATCH 13/23] ostree-prepare-root: Error if realpath fails I've seen it fail with musl which needs `/proc` to be mounted for it to work. The error messages we're rather confusing before. At least this now points to the right location. Closes: #485 Approved by: cgwalters --- src/switchroot/ostree-prepare-root.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/switchroot/ostree-prepare-root.c b/src/switchroot/ostree-prepare-root.c index d866f88e..ea7333b5 100644 --- a/src/switchroot/ostree-prepare-root.c +++ b/src/switchroot/ostree-prepare-root.c @@ -174,17 +174,19 @@ pivot_root(const char * new_root, const char * put_old) int main(int argc, char *argv[]) { - const char *root_mountpoint = NULL; + const char *root_mountpoint = NULL, *root_arg = NULL; char *deploy_path = NULL; char srcpath[PATH_MAX]; struct stat stbuf; if (argc < 2) - root_mountpoint = "/"; + root_arg = "/"; else - root_mountpoint = argv[1]; + root_arg = argv[1]; - root_mountpoint = realpath (root_mountpoint, NULL); + root_mountpoint = realpath (root_arg, NULL); + if (root_mountpoint == NULL) + err (EXIT_FAILURE, "realpath(\"%s\")", root_arg); deploy_path = resolve_deploy_path (root_mountpoint); /* Work-around for a kernel bug: for some reason the kernel From 2aacc6912b78e7d3158f072cd9c1b3c5d0bddc22 Mon Sep 17 00:00:00 2001 From: William Manley Date: Wed, 31 Aug 2016 17:15:48 +0100 Subject: [PATCH 14/23] ostree-prepare-root: Fix running with musl musl libc's implementation of `realpath` works by opening the path and then doing a lookup in `/proc/self/fd` to find the canonical path. This fails if `/proc` is not mounted. This causes problems for us if `ostree-prepare-root` is `init` as `/proc` won't be mounted. We have to mount `/proc` anyway for `/proc/cmdline` so this fix just expands the scope over which `/proc` is mounted to include both our `realpath` calls. See also: * http://www.openwall.com/lists/musl/2016/06/08/2 and * http://git.musl-libc.org/cgit/musl/tree/src/misc/realpath.c?id=e738b8cbe64b6dd3ed9f47b6d4cd7eb2c422b38d Closes: #485 Approved by: cgwalters --- src/switchroot/ostree-prepare-root.c | 37 +++++++++++++++------------- tests/test-switchroot.sh | 9 ++++++- 2 files changed, 28 insertions(+), 18 deletions(-) diff --git a/src/switchroot/ostree-prepare-root.c b/src/switchroot/ostree-prepare-root.c index ea7333b5..ce48a91e 100644 --- a/src/switchroot/ostree-prepare-root.c +++ b/src/switchroot/ostree-prepare-root.c @@ -80,26 +80,10 @@ parse_ostree_cmdline (void) char *cmdline = NULL; const char *iter; char *ret = NULL; - int tmp_errno; cmdline = read_proc_cmdline (); if (!cmdline) - { - // Mount proc - if (mount ("proc", "/proc", "proc", 0, NULL) < 0) - err (EXIT_FAILURE, "failed to mount proc on /proc"); - - cmdline = read_proc_cmdline (); - tmp_errno = errno; - - /* Leave the filesystem in the state that we found it: */ - if (umount ("/proc")) - err (EXIT_FAILURE, "failed to umount proc from /proc"); - - errno = tmp_errno; - if (!cmdline) - err (EXIT_FAILURE, "failed to read /proc/cmdline"); - } + err (EXIT_FAILURE, "failed to read /proc/cmdline"); iter = cmdline; while (iter != NULL) @@ -178,17 +162,36 @@ main(int argc, char *argv[]) char *deploy_path = NULL; char srcpath[PATH_MAX]; struct stat stbuf; + int we_mounted_proc = 0; if (argc < 2) root_arg = "/"; else root_arg = argv[1]; + if (stat ("/proc/cmdline", &stbuf) < 0) + { + if (errno != ENOENT) + err (EXIT_FAILURE, "stat(\"/proc/cmdline\") failed"); + /* We need /proc mounted for /proc/cmdline and realpath (on musl) to + * work: */ + if (mount ("proc", "/proc", "proc", 0, NULL) < 0) + err (EXIT_FAILURE, "failed to mount proc on /proc"); + we_mounted_proc = 1; + } + root_mountpoint = realpath (root_arg, NULL); if (root_mountpoint == NULL) err (EXIT_FAILURE, "realpath(\"%s\")", root_arg); deploy_path = resolve_deploy_path (root_mountpoint); + if (we_mounted_proc) + { + /* Leave the filesystem in the state that we found it: */ + if (umount ("/proc")) + err (EXIT_FAILURE, "failed to umount proc from /proc"); + } + /* Work-around for a kernel bug: for some reason the kernel * refuses switching root if any file systems are mounted * MS_SHARED. Hence remount them MS_PRIVATE here as a diff --git a/tests/test-switchroot.sh b/tests/test-switchroot.sh index 96c849e5..fd5a30d0 100755 --- a/tests/test-switchroot.sh +++ b/tests/test-switchroot.sh @@ -4,7 +4,14 @@ this_script="${BASH_SOURCE:-$(readlink -f "$0")}" setup_bootfs() { mkdir -p "$1/proc" "$1/bin" - echo "quiet ostree=/ostree/boot.0 ro" >"$1/proc/cmdline" + + # We need the real /proc mounted here so musl's realpath will work, but we + # want to be able to override /proc/cmdline, so bind mount. + mount -t proc proc "$1/proc" + + echo "quiet ostree=/ostree/boot.0 ro" >"$1/override_cmdline" + mount --bind "$1/override_cmdline" "$1/proc/cmdline" + touch "$1/this_is_bootfs" cp "$(dirname "$this_script")/../ostree-prepare-root" "$1/bin" } From f760a4612a234268815c5ed3941c11840d469b3b Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Wed, 7 Sep 2016 12:15:26 +0200 Subject: [PATCH 15/23] gpg: do not segfault when the algorithm name is not known Reported by: Patrick Uiterwijk Signed-off-by: Giuseppe Scrivano Closes: #494 Approved by: cgwalters --- src/libostree/ostree-gpg-verify-result.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libostree/ostree-gpg-verify-result.c b/src/libostree/ostree-gpg-verify-result.c index fa4614d1..73fbfeed 100644 --- a/src/libostree/ostree-gpg-verify-result.c +++ b/src/libostree/ostree-gpg-verify-result.c @@ -370,11 +370,15 @@ ostree_gpg_verify_result_get (OstreeGpgVerifyResult *result, case OSTREE_GPG_SIGNATURE_ATTR_PUBKEY_ALGO_NAME: v_string = gpgme_pubkey_algo_name (signature->pubkey_algo); + if (v_string == NULL) + v_string = "[unknown name]"; child = g_variant_new_string (v_string); break; case OSTREE_GPG_SIGNATURE_ATTR_HASH_ALGO_NAME: v_string = gpgme_hash_algo_name (signature->hash_algo); + if (v_string == NULL) + v_string = "[unknown name]"; child = g_variant_new_string (v_string); break; From 845dc651962b2c106872428336c9d81c2ed12432 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 8 Sep 2016 07:11:52 -0400 Subject: [PATCH 16/23] repo: Revert default timestamp from 1 back to 0 Quoting Dan Nicholson in mtime of 0 has been the semantics of ostree deployments from basically the beginning of the project. We (and others, see flatpak/flatpak@b5204c9) rely on that fact when generating trees. In particular, this affects caches that use the mtime of the associated file or directory to determine if the cache is valid. By arbitrarily changing the mtime of the files to something else, all the caches we setup in the build are now invalidated. Preseeding caches is really important to the user experience as it avoids having the user wait while they're regenerated on first run. Now, we could change our build infrastructure to preset all the mtimes to 1 to match this change, but what does that do for our existing users who are on an ostree that deploys with mtimes of 0? We could just revert this change at Endless (and the associated one in Flatpak), and that would be fine for our users. However, if we point non-Endless users to our apps, they'll have the great experience of waiting 10 seconds the first time they launch it while the fontconfig cache is rebuilt unnecessarily. Closes: #495 Approved by: jlebon --- docs/manual/repo.md | 9 +++++---- src/libostree/ostree-repo-private.h | 2 +- tests/basic-test.sh | 4 ++-- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/docs/manual/repo.md b/docs/manual/repo.md index c23a4453..6e307ba3 100644 --- a/docs/manual/repo.md +++ b/docs/manual/repo.md @@ -58,10 +58,11 @@ comparing timestamps. For Git, the logical choice is to not mess with timestamps, because unnecessary rebuilding is better than a broken tree. However, OSTree has to hardlink files to check them out, and commits are assumed to be internally consistent with no build steps needed. For this reason, OSTree -acts as though all timestamps are set to time_t 1, so that comparisons will be -considered up-to-date. 1 is a better choice than 0 because some programs use 0 -as a special value; for example, GNU Tar warns of an "implausibly old time -stamp" with 0. +acts as though all timestamps are set to time_t 0, so that comparisons will be +considered up-to-date. Note that for a few releases, OSTree used 1 to fix +warnings such as GNU Tar emitting "implausibly old time stamp" with 0; however, +until we have a mechanism to transition cleanly to 1, for compatibilty OSTree +is reverted to use zero again. # Repository types and locations diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h index 92d8a084..e8d5550f 100644 --- a/src/libostree/ostree-repo-private.h +++ b/src/libostree/ostree-repo-private.h @@ -32,7 +32,7 @@ G_BEGIN_DECLS #define _OSTREE_SUMMARY_CACHE_DIR "summaries" #define _OSTREE_CACHE_DIR "cache" -#define OSTREE_TIMESTAMP (1) +#define OSTREE_TIMESTAMP (0) typedef enum { OSTREE_REPO_TEST_ERROR_PRE_COMMIT = (1 << 0) diff --git a/tests/basic-test.sh b/tests/basic-test.sh index c4828d90..9db56e77 100755 --- a/tests/basic-test.sh +++ b/tests/basic-test.sh @@ -392,9 +392,9 @@ else $OSTREE checkout test2 test2-checkout fi stat '--format=%Y' test2-checkout/baz/cow > cow-mtime -assert_file_has_content cow-mtime 1 +assert_file_has_content cow-mtime 0 stat '--format=%Y' test2-checkout/baz/deeper > deeper-mtime -assert_file_has_content deeper-mtime 1 +assert_file_has_content deeper-mtime 0 echo "ok content mtime" cd ${test_tmpdir} From 8dbb104cdcfaadad8a826eb76fec5f7058c5beb4 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 8 Sep 2016 10:29:10 -0400 Subject: [PATCH 17/23] delta: Add missing `goto out` for failure to mmap() This was hit in practice when generating a delta for a flatpak app on ARM it looks like. Closes: #497 Approved by: alexlarsson --- src/libostree/ostree-repo-static-delta-compilation.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libostree/ostree-repo-static-delta-compilation.c b/src/libostree/ostree-repo-static-delta-compilation.c index 188f467d..15b1df8d 100644 --- a/src/libostree/ostree-repo-static-delta-compilation.c +++ b/src/libostree/ostree-repo-static-delta-compilation.c @@ -483,6 +483,8 @@ get_unpacked_unlinked_content (OstreeRepo *repo, goto out; { GMappedFile *mfile = g_mapped_file_new_from_fd (fd, FALSE, error); + if (!mfile) + goto out; ret_content = g_mapped_file_get_bytes (mfile); g_mapped_file_unref (mfile); } From e127070550fd22d129cd19b45e0d8c74c1b8cc79 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 1 Sep 2016 16:09:30 -0400 Subject: [PATCH 18/23] repo: Only use mmap() for metadata > 16k See http://stackoverflow.com/questions/258091/when-should-i-use-mmap-for-file-access and https://lwn.net/Articles/591978/ I didn't really notice much performance difference in some small tests, but I happened to be stracing and realized we were `mmap()`ing even for 50 bytes which is not very useful, so let's not do it. Closes: #489 Approved by: alexlarsson --- src/libostree/ostree-repo.c | 56 ++++++++++++++++++++++--------------- 1 file changed, 34 insertions(+), 22 deletions(-) diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index d33a96ab..59bfbf9e 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -2544,6 +2544,7 @@ load_metadata_internal (OstreeRepo *self, { gboolean ret = FALSE; char loose_path_buf[_OSTREE_LOOSE_PATH_MAX]; + struct stat stbuf; glnx_fd_close int fd = -1; g_autoptr(GInputStream) ret_stream = NULL; g_autoptr(GVariant) ret_variant = NULL; @@ -2565,23 +2566,39 @@ load_metadata_internal (OstreeRepo *self, if (fd != -1) { + if (fstat (fd, &stbuf) < 0) + { + glnx_set_error_from_errno (error); + goto out; + } + if (out_variant) { - GMappedFile *mfile; + /* http://stackoverflow.com/questions/258091/when-should-i-use-mmap-for-file-access */ + if (stbuf.st_size > 16*1024) + { + GMappedFile *mfile; - mfile = g_mapped_file_new_from_fd (fd, FALSE, error); - if (!mfile) - goto out; - ret_variant = g_variant_new_from_data (ostree_metadata_variant_type (objtype), - g_mapped_file_get_contents (mfile), - g_mapped_file_get_length (mfile), - TRUE, - (GDestroyNotify) g_mapped_file_unref, - mfile); - g_variant_ref_sink (ret_variant); - - if (out_size) - *out_size = g_variant_get_size (ret_variant); + mfile = g_mapped_file_new_from_fd (fd, FALSE, error); + if (!mfile) + goto out; + ret_variant = g_variant_new_from_data (ostree_metadata_variant_type (objtype), + g_mapped_file_get_contents (mfile), + g_mapped_file_get_length (mfile), + TRUE, + (GDestroyNotify) g_mapped_file_unref, + mfile); + g_variant_ref_sink (ret_variant); + } + else + { + GBytes *data = glnx_fd_readall_bytes (fd, cancellable, error); + if (!data) + goto out; + ret_variant = g_variant_new_from_bytes (ostree_metadata_variant_type (objtype), + data, TRUE); + g_variant_ref_sink (ret_variant); + } } else if (out_stream) { @@ -2589,15 +2606,10 @@ load_metadata_internal (OstreeRepo *self, if (!ret_stream) goto out; fd = -1; /* Transfer ownership */ - if (out_size) - { - struct stat stbuf; - - if (!glnx_stream_fstat ((GFileDescriptorBased*)ret_stream, &stbuf, error)) - goto out; - *out_size = stbuf.st_size; - } } + + if (out_size) + *out_size = stbuf.st_size; } else if (self->parent_repo) { From 23b4b58d57c7f9f01144a5e59daf26fbe6d5ba4d Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 8 Sep 2016 10:47:33 -0400 Subject: [PATCH 19/23] delta: Unreference files we've processed This should help avoid address space exhaustion on 32 bit systems, and in general is obviously going to improve efficiency. Closes: #498 Approved by: alexlarsson --- .../ostree-repo-static-delta-compilation.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/libostree/ostree-repo-static-delta-compilation.c b/src/libostree/ostree-repo-static-delta-compilation.c index 15b1df8d..acface61 100644 --- a/src/libostree/ostree-repo-static-delta-compilation.c +++ b/src/libostree/ostree-repo-static-delta-compilation.c @@ -429,8 +429,8 @@ content_rollsums_free (ContentRollsum *rollsum) { g_free (rollsum->from_checksum); _ostree_rollsum_matches_free (rollsum->matches); - g_bytes_unref (rollsum->tmp_from); - g_bytes_unref (rollsum->tmp_to); + g_clear_pointer (&rollsum->tmp_from, g_bytes_unref); + g_clear_pointer (&rollsum->tmp_to, g_bytes_unref); g_free (rollsum); } @@ -438,8 +438,8 @@ static void content_bsdiffs_free (ContentBsdiff *bsdiff) { g_free (bsdiff->from_checksum); - g_bytes_unref (bsdiff->tmp_from); - g_bytes_unref (bsdiff->tmp_to); + g_clear_pointer (&bsdiff->tmp_from, g_bytes_unref); + g_clear_pointer (&bsdiff->tmp_to, g_bytes_unref); g_free (bsdiff); } @@ -758,6 +758,9 @@ process_one_rollsum (OstreeRepo *repo, g_string_append_c (current_part->operations, (gchar)OSTREE_STATIC_DELTA_OP_CLOSE); } + g_clear_pointer (&rollsum->tmp_from, g_bytes_unref); + g_clear_pointer (&rollsum->tmp_to, g_bytes_unref); + ret = TRUE; out: return ret; @@ -854,6 +857,9 @@ process_one_bsdiff (OstreeRepo *repo, g_string_append_c (current_part->operations, (gchar)OSTREE_STATIC_DELTA_OP_UNSET_READ_SOURCE); + g_clear_pointer (&bsdiff_content->tmp_from, g_bytes_unref); + g_clear_pointer (&bsdiff_content->tmp_to, g_bytes_unref); + ret = TRUE; out: return ret; From 82b756783be8c0bcbf7b84d2694dcbca9f58cd0e Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 8 Sep 2016 12:39:42 -0400 Subject: [PATCH 20/23] fetcher: Fix another finalization deadlock If the current repo is already up to date (we have no content to fetch), it's possible for the fetcher to not request any URIs. So create and then finalize it quickly. Finalization involves calling `g_main_loop_quit()` + `g_thread_wait()`. However, if `g_main_loop_quit()` is run *before* `g_main_loop_run()`, we'll deadlock because GMainLoop assumes in `_run()` to start things. This is a common trap - ideally, GMainLoop would record if `_quit()` was called before `_run()` or something, but doing that now would likely break people who are expecting quit() -> run() to restart. In general, we've moved in various GLib-consuming apps to an explicit "main context iteration with termination condition" model; see `pull_termination_condition()` in the pull code. This fixes this race condition. I verified that an assertion in `_finalize` that more than zero URIs were requested was hit in multiple test cases, and this patch has survived a while of make check loops. Closes: https://github.com/ostreedev/ostree/issues/496 Closes: #499 Approved by: jlebon --- src/libostree/ostree-fetcher.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/libostree/ostree-fetcher.c b/src/libostree/ostree-fetcher.c index 09006b71..3ddf2389 100644 --- a/src/libostree/ostree-fetcher.c +++ b/src/libostree/ostree-fetcher.c @@ -46,7 +46,7 @@ typedef struct { SoupSession *session; /* not referenced */ GMainContext *main_context; - GMainLoop *main_loop; + volatile gint running; int tmpdir_dfd; char *tmpdir_name; @@ -144,7 +144,6 @@ thread_closure_unref (ThreadClosure *thread_closure) g_assert (thread_closure->session == NULL); g_clear_pointer (&thread_closure->main_context, g_main_context_unref); - g_clear_pointer (&thread_closure->main_loop, g_main_loop_unref); if (thread_closure->tmpdir_dfd != -1) close (thread_closure->tmpdir_dfd); @@ -503,7 +502,12 @@ ostree_fetcher_session_thread (gpointer data) } closure->max_outstanding = 3 * max_conns; - g_main_loop_run (closure->main_loop); + /* This model ensures we don't hit a race using g_main_loop_quit(); + * see also what pull_termination_condition() in ostree-repo-pull.c + * is doing. + */ + while (g_atomic_int_get (&closure->running)) + g_main_context_iteration (closure->main_context, TRUE); /* Since the ThreadClosure may be finalized from any thread we * unreference all data related to the SoupSession ourself to ensure @@ -567,7 +571,8 @@ _ostree_fetcher_finalize (GObject *object) OstreeFetcher *self = OSTREE_FETCHER (object); /* Terminate the session thread. */ - g_main_loop_quit (self->thread_closure->main_loop); + g_atomic_int_set (&self->thread_closure->running, 0); + g_main_context_wakeup (self->thread_closure->main_context); if (self->session_thread) { /* We need to explicitly synchronize to clean up TLS */ @@ -594,7 +599,7 @@ _ostree_fetcher_constructed (GObject *object) self->thread_closure = g_slice_new0 (ThreadClosure); self->thread_closure->ref_count = 1; self->thread_closure->main_context = g_main_context_ref (main_context); - self->thread_closure->main_loop = g_main_loop_new (main_context, FALSE); + self->thread_closure->running = 1; self->thread_closure->tmpdir_dfd = -1; self->thread_closure->tmpdir_lock = empty_lockfile; From 30aaffa9956cfb54183e6ddd9e425b064cb0bd74 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 8 Sep 2016 13:38:08 -0400 Subject: [PATCH 21/23] sysroot: Avoid double cleanup, and ensure no cleanup if specified Since forever, we've been doing two cleanups. In https://github.com/ostreedev/ostree/commit/8ece4d6d51bdbe3e41ab318259276bb83e553aa0 I thought we were doing just one and wanted to go to zero (if specified), but I actually just dropped one cleanup. In https://github.com/projectatomic/rpm-ostree/pull/452 @jlebon pointed out the duplication. Fix this by creating a new internal deploy wrapper that takes cleanup flags. (Since we already had the "piecemeal cleanup" API internally, let's frame it in terms of that, rather than passing down a boolean). Closes: #500 Approved by: jlebon --- src/libostree/ostree-sysroot-deploy.c | 15 ++++++++++++++- src/libostree/ostree-sysroot-private.h | 6 ++++++ src/libostree/ostree-sysroot.c | 10 +++------- 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index a05ca30d..a49428d2 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -1712,6 +1712,18 @@ ostree_sysroot_write_deployments (OstreeSysroot *self, GPtrArray *new_deployments, GCancellable *cancellable, GError **error) +{ + return _ostree_sysroot_write_deployments_internal (self, new_deployments, + OSTREE_SYSROOT_CLEANUP_ALL, + cancellable, error); +} + +gboolean +_ostree_sysroot_write_deployments_internal (OstreeSysroot *self, + GPtrArray *new_deployments, + OstreeSysrootCleanupFlags cleanup_flags, + GCancellable *cancellable, + GError **error) { gboolean ret = FALSE; guint i; @@ -1937,7 +1949,8 @@ ostree_sysroot_write_deployments (OstreeSysroot *self, /* And finally, cleanup of any leftover data. */ - if (!ostree_sysroot_cleanup (self, cancellable, error)) + if (!_ostree_sysroot_piecemeal_cleanup (self, cleanup_flags, + cancellable, error)) { g_prefix_error (error, "Performing final cleanup: "); goto out; diff --git a/src/libostree/ostree-sysroot-private.h b/src/libostree/ostree-sysroot-private.h index 1fa8e83c..b2def7fa 100644 --- a/src/libostree/ostree-sysroot-private.h +++ b/src/libostree/ostree-sysroot-private.h @@ -121,4 +121,10 @@ gboolean _ostree_sysroot_piecemeal_cleanup (OstreeSysroot *sysroot, GCancellable *cancellable, GError **error); +gboolean _ostree_sysroot_write_deployments_internal (OstreeSysroot *self, + GPtrArray *new_deployments, + OstreeSysrootCleanupFlags cleanup_flags, + GCancellable *cancellable, + GError **error); + G_END_DECLS diff --git a/src/libostree/ostree-sysroot.c b/src/libostree/ostree-sysroot.c index 82f864cb..37063e28 100644 --- a/src/libostree/ostree-sysroot.c +++ b/src/libostree/ostree-sysroot.c @@ -1591,15 +1591,11 @@ ostree_sysroot_simple_write_deployment (OstreeSysroot *sysroot, added_new = TRUE; } - if (!ostree_sysroot_write_deployments (sysroot, new_deployments, cancellable, error)) + if (!_ostree_sysroot_write_deployments_internal (sysroot, new_deployments, + postclean ? OSTREE_SYSROOT_CLEANUP_ALL : 0, + cancellable, error)) goto out; - if (postclean) - { - if (!ostree_sysroot_cleanup (sysroot, cancellable, error)) - goto out; - } - ret = TRUE; out: return ret; From 916684ba0d7b53e7cc745fadac41611e2e543ec5 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 8 Sep 2016 16:50:35 -0400 Subject: [PATCH 22/23] core: Make OSTREE_TIMESTAMP public API This way e.g. flatpak can detect which timestamp it should use. See `flatpak/common/flatpak-utils.c:flatpak_zero_mtime`. Closes: #501 Approved by: cgwalters --- src/libostree/ostree-core.h | 9 +++++++++ src/libostree/ostree-repo-private.h | 2 -- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/libostree/ostree-core.h b/src/libostree/ostree-core.h index d1f76cf1..a3419949 100644 --- a/src/libostree/ostree-core.h +++ b/src/libostree/ostree-core.h @@ -165,6 +165,15 @@ typedef enum { #define OSTREE_SUMMARY_SIG_GVARIANT_STRING "a{sv}" #define OSTREE_SUMMARY_SIG_GVARIANT_FORMAT G_VARIANT_TYPE (OSTREE_SUMMARY_SIG_GVARIANT_STRING) +/** + * OSTREE_TIMESTAMP: + * + * The mtime used for stored files. This was originally 0, changed to 1 for + * a few releases, then was reverted due to regressions it introduced from + * users who had been using zero before. + */ +#define OSTREE_TIMESTAMP (0) + /** * OstreeRepoMode: * @OSTREE_REPO_MODE_BARE: Files are stored as themselves; checkouts are hardlinks; can only be written as root diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h index e8d5550f..6b25a6fc 100644 --- a/src/libostree/ostree-repo-private.h +++ b/src/libostree/ostree-repo-private.h @@ -32,8 +32,6 @@ G_BEGIN_DECLS #define _OSTREE_SUMMARY_CACHE_DIR "summaries" #define _OSTREE_CACHE_DIR "cache" -#define OSTREE_TIMESTAMP (0) - typedef enum { OSTREE_REPO_TEST_ERROR_PRE_COMMIT = (1 << 0) } OstreeRepoTestErrorFlags; From 36e8ba124e1f133db47237f3fa873efc9fb42ae1 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 8 Sep 2016 16:54:00 -0400 Subject: [PATCH 23/23] Release 2016.10 Closes: #502 Approved by: cgwalters --- configure.ac | 2 +- src/libostree/libostree.sym | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index 008728dd..b739d2a7 100644 --- a/configure.ac +++ b/configure.ac @@ -1,6 +1,6 @@ AC_PREREQ([2.63]) dnl If incrementing the version here, remember to update libostree.sym too -AC_INIT([ostree], [2016.9], [walters@verbum.org]) +AC_INIT([ostree], [2016.10], [walters@verbum.org]) AC_CONFIG_HEADER([config.h]) AC_CONFIG_MACRO_DIR([buildutil]) AC_CONFIG_AUX_DIR([build-aux]) diff --git a/src/libostree/libostree.sym b/src/libostree/libostree.sym index 705c9eea..5a2d71be 100644 --- a/src/libostree/libostree.sym +++ b/src/libostree/libostree.sym @@ -354,6 +354,7 @@ global: } LIBOSTREE_2016.7; /* No new symbols in 2016.9 */ +/* No new symbols in 2016.10 */ /* NOTE NOTE NOTE * Versions above here are released. Only add symbols below this line. @@ -361,7 +362,7 @@ global: */ /* Remove comment when first new symbol is added -LIBOSTREE_2016.10 +LIBOSTREE_2016.11 global: someostree_symbol_deleteme; } LIBOSTREE_2016.8;