From 326b8b13c6340d67b67d0ef398a894461d25c2a8 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 13 Oct 2020 14:33:14 -0400 Subject: [PATCH 01/47] Post-release version bump --- Makefile-libostree.am | 6 +++--- configure.ac | 4 ++-- src/libostree/libostree-devel.sym | 4 ++++ 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/Makefile-libostree.am b/Makefile-libostree.am index 96b9249b..a180e86b 100644 --- a/Makefile-libostree.am +++ b/Makefile-libostree.am @@ -182,9 +182,9 @@ libostree_1_la_SOURCES += \ endif # USE_GPGME symbol_files = $(top_srcdir)/src/libostree/libostree-released.sym -if BUILDOPT_IS_DEVEL_BUILD -symbol_files += $(top_srcdir)/src/libostree/libostree-devel.sym -endif +#if BUILDOPT_IS_DEVEL_BUILD +#symbol_files += $(top_srcdir)/src/libostree/libostree-devel.sym +#endif # http://blog.jgc.org/2007/06/escaping-comma-and-space-in-gnu-make.html wl_versionscript_arg = -Wl,--version-script= EXTRA_DIST += \ diff --git a/configure.ac b/configure.ac index 13f5bef8..7a2f7ab6 100644 --- a/configure.ac +++ b/configure.ac @@ -7,10 +7,10 @@ dnl Seed the release notes with `git-shortlog-with-prs ..`. Th dnl `git-evtag` to create the tag and push it. Finally, create a GitHub release and attach dnl the tarball from `make dist`. m4_define([year_version], [2020]) -m4_define([release_version], [7]) +m4_define([release_version], [8]) 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 2ed658c4..8e8d9c81 100644 --- a/src/libostree/libostree-devel.sym +++ b/src/libostree/libostree-devel.sym @@ -17,6 +17,10 @@ Boston, MA 02111-1307, USA. ***/ +/* Copy the bits below and uncomment the include in Makefile-libostree.am + when adding a symbol. +*/ + /* Stub section for the stable release *after* this development one; don't * edit this other than to update the year. This is just a copy/paste * source. Replace $LASTSTABLE with the last stable version, and $NEWVERSION From b4e7ab014ebcb18ecaf2e79a81559c8e2514f52a Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 13 Oct 2020 17:39:18 -0400 Subject: [PATCH 02/47] deploy: Remove (transfer none) from fd arg GI complains. And in general one needs to assume that file descriptors aren't stolen. --- src/libostree/ostree-sysroot-deploy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index 7b7ba5e9..1a4a6369 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -3046,7 +3046,7 @@ _ostree_sysroot_deserialize_deployment_from_variant (GVariant *v, /** * ostree_sysroot_stage_overlay_initrd: * @self: Sysroot - * @fd: (transfer none): File descriptor to overlay initrd + * @fd: File descriptor to overlay initrd * @out_checksum: (out) (transfer full): Overlay initrd checksum * @cancellable: Cancellable * @error: Error From 53b6bbbdf29face8eaaac2bad7e3cc57e3c96b55 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 15 Oct 2020 09:35:40 -0400 Subject: [PATCH 03/47] travis: Add a 32 bit build In the past we've had 32 bit bugs that were caught by the compiler, let's add this to Travis. --- .travis.yml | 1 + ci/travis-install.sh | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index 92217682..712a81dd 100644 --- a/.travis.yml +++ b/.travis.yml @@ -7,6 +7,7 @@ env: - ci_docker=debian:buster-slim ci_distro=debian ci_suite=stretch ci_configopts="--with-ed25519-libsodium" ci_pkgs="libsodium-dev" - ci_docker=debian:buster-slim ci_distro=debian ci_suite=stretch ci_configopts="--with-curl --with-ed25519-libsodium --without-gpgme" ci_pkgs="libsodium-dev" - ci_docker=ubuntu:xenial ci_distro=ubuntu ci_suite=xenial + - ci_docker=i386/ubuntu:bionic ci_distro=ubuntu ci_suite=bionic - ci_docker=ubuntu:bionic ci_distro=ubuntu ci_suite=bionic script: diff --git a/ci/travis-install.sh b/ci/travis-install.sh index c28a4111..64801741 100755 --- a/ci/travis-install.sh +++ b/ci/travis-install.sh @@ -64,10 +64,10 @@ fi if [ -n "$ci_docker" ]; then sed \ - -e "s/@ci_distro@/${ci_distro}/" \ - -e "s/@ci_docker@/${ci_docker}/" \ - -e "s/@ci_suite@/${ci_suite}/" \ - -e "s/@ci_pkgs@/${ci_pkgs}/" \ + -e "s,@ci_distro@,${ci_distro}," \ + -e "s,@ci_docker@,${ci_docker}," \ + -e "s,@ci_suite@,${ci_suite}," \ + -e "s,@ci_pkgs@,${ci_pkgs}," \ < ci/travis-Dockerfile.in > Dockerfile exec docker build -t ci-image . fi From dec9eab2030a212b3713379af9d74c625ecce137 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Thu, 15 Oct 2020 12:05:34 -0400 Subject: [PATCH 04/47] ostree-prepare-root: print st_dev and st_ino as 64-bit ints This matches what systemd does and should work fine on all platforms. Possibly resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1888436 --- src/switchroot/ostree-prepare-root.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/switchroot/ostree-prepare-root.c b/src/switchroot/ostree-prepare-root.c index 6351babb..6bc2c374 100644 --- a/src/switchroot/ostree-prepare-root.c +++ b/src/switchroot/ostree-prepare-root.c @@ -152,8 +152,8 @@ resolve_deploy_path (const char * root_mountpoint) "MESSAGE_ID=" SD_ID128_FORMAT_STR, SD_ID128_FORMAT_VAL(OSTREE_PREPARE_ROOT_DEPLOYMENT_MSG), "DEPLOYMENT_PATH=%s", resolved_path, - "DEPLOYMENT_DEVICE=%u", stbuf.st_dev, - "DEPLOYMENT_INODE=%u", stbuf.st_ino, + "DEPLOYMENT_DEVICE=%" PRIu64, (uint64_t) stbuf.st_dev, + "DEPLOYMENT_INODE=%" PRIu64, (uint64_t) stbuf.st_ino, NULL); #endif return deploy_path; From 4183b68e700f2144003818840a7958a86d313a07 Mon Sep 17 00:00:00 2001 From: Felix Krull Date: Sat, 17 Oct 2020 22:14:09 +0200 Subject: [PATCH 05/47] lib: fix GI parameter tags --- src/libostree/ostree-repo-static-delta-core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libostree/ostree-repo-static-delta-core.c b/src/libostree/ostree-repo-static-delta-core.c index d83ec8c4..e263e928 100644 --- a/src/libostree/ostree-repo-static-delta-core.c +++ b/src/libostree/ostree-repo-static-delta-core.c @@ -508,7 +508,7 @@ ostree_repo_static_delta_execute_offline_with_signature (OstreeRepo *self, */ delta_open_flags |= OSTREE_STATIC_DELTA_OPEN_FLAGS_SKIP_CHECKSUM; - if (!_ostree_static_delta_part_open (part_in, inline_part_bytes, + if (!_ostree_static_delta_part_open (part_in, inline_part_bytes, delta_open_flags, NULL, &part, @@ -1076,7 +1076,7 @@ _ostree_repo_static_delta_dump (OstreeRepo *self, * @self: Repo * @delta_id: delta path * @sign: Signature engine used to check superblock - * @out_success_message: success message + * @out_success_message: (out) (nullable) (optional): success message * @error: Error * * Verify static delta file signature. From a89d011c0b4ce73f4b48e1563ca7044cdfa9e0eb Mon Sep 17 00:00:00 2001 From: Kelvin Fan Date: Thu, 15 Oct 2020 19:51:44 -0400 Subject: [PATCH 06/47] docs: Fix various typos --- docs/atomic-upgrades.md | 8 ++++---- docs/buildsystem-and-repos.md | 8 ++++---- docs/deployment.md | 2 +- docs/formats.md | 7 +++---- docs/repo.md | 6 +++--- docs/repository-management.md | 3 +-- 6 files changed, 16 insertions(+), 18 deletions(-) diff --git a/docs/atomic-upgrades.md b/docs/atomic-upgrades.md index 3ddd8b40..f4b31aca 100644 --- a/docs/atomic-upgrades.md +++ b/docs/atomic-upgrades.md @@ -30,14 +30,14 @@ the remote server. Suppose we're tracking a ref named which contains a SHA256 checksum. This determines the tree to deploy, and `/etc` will be merged from currently booted tree. -If we do not have this commit, then, then we perform a pull process. +If we do not have this commit, then we perform a pull process. At present (without static deltas), this involves quite simply just fetching each individual object that we do not have, asynchronously. Put in other words, we only download changed files (zlib-compressed). Each object has its checksum validated and is stored in `/ostree/repo/objects/`. -Once the pull is complete, we have all the objects locally -we need to perform a deployment. +Once the pull is complete, we have downloaded all the objects that we need +to perform a deployment. ## Upgrades via external tools (e.g. package managers) @@ -50,7 +50,7 @@ locally, etc. At a practical level, most package managers today (`dpkg` and `rpm`) operate "live" on the currently booted filesystem. The way they could -work with OSTree is instead to take the list of installed packages in +work with OSTree is to, instead, take the list of installed packages in the currently booted tree, and compute a new filesystem from that. A later chapter describes in more details how this could work: [Adapting Existing Systems](adapting-existing.md). diff --git a/docs/buildsystem-and-repos.md b/docs/buildsystem-and-repos.md index 6d506b4e..e265ee7a 100644 --- a/docs/buildsystem-and-repos.md +++ b/docs/buildsystem-and-repos.md @@ -21,10 +21,10 @@ primarily on server-side concerns. ## Build vs buy Therefore, you need to either pick an existing tool for writing -content into an OSTree repository, or to write your own. An example -tool is [rpm-ostree](https://github.com/projectatomic/rpm-ostree) - it -takes as input RPMs, and commits them (currently oriented for a server -side, but aiming to do client side too). +content into an OSTree repository, or write your own. An example +tool is [rpm-ostree](https://github.com/coreos/rpm-ostree) - it +takes as input RPMs, and commits them (currently oriented for +server-side, but aiming to do client-side too). ## Initializing diff --git a/docs/deployment.md b/docs/deployment.md index 1ea7ea46..30323f8c 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -26,7 +26,7 @@ at a time; each deployment is intended to be a target for Each deployment is grouped in exactly one "stateroot" (also known as an "osname"); the former term is preferred. -From above, you can see that an stateroot is physically represented in the +From above, you can see that a stateroot is physically represented in the `/ostree/deploy/$stateroot` directory. For example, OSTree can allow parallel installing Debian in `/ostree/deploy/debian` and Red Hat Enterprise Linux in `/ostree/deploy/rhel` (subject to operating system support, present released diff --git a/docs/formats.md b/docs/formats.md index 36d395bd..0943aafa 100644 --- a/docs/formats.md +++ b/docs/formats.md @@ -103,12 +103,11 @@ Since static deltas may not exist, the client first needs to attempt to locate one. Suppose a client wants to retrieve commit `${new}` while currently running `${current}`. -The first thing to understand is that in order to save space, these -two commits are "modified base64" - the `/` character is replaced with -`_`. +In order to save space, these two commits are "modified base64" - the +`/` character is replaced with `_`. Like the commit objects, a "prefix directory" is used to make -management easier for filesystem tools +management easier for filesystem tools. A delta is named `$(mbase64 $from)-$(mbase64 $to)`, for example `GpTyZaVut2jXFPWnO4LJiKEdRTvOw_mFUCtIKW1NIX0-L8f+VVDkEBKNc1Ncd+mDUrSVR4EyybQGCkuKtkDnTwk`, diff --git a/docs/repo.md b/docs/repo.md index 5cc59bf1..9b254b12 100644 --- a/docs/repo.md +++ b/docs/repo.md @@ -39,7 +39,7 @@ regenerate it from source code. A dirtree contains a sorted array of (filename, checksum) pairs for content objects, and a second sorted array of (filename, dirtree checksum, dirmeta checksum), which are -subdirectories. These type of objects are stored as files +subdirectories. This type of object is stored as files ending with `.dirtree` in the objects directory. ### Dirmeta objects @@ -56,7 +56,7 @@ Unlike the first three object types which are metadata, designed to be `mmap()`ed, the content object has a separate internal header and payload sections. The header contains uid, gid, mode, and symbolic link target (for symlinks), as well as extended attributes. After the -header, for regular files, the content follows. These parts toghether +header, for regular files, the content follows. These parts together form the SHA256 hash for content objects. The content type objects in this format exist only in `archive` OSTree repositories. Today the content part is gzip'ed and the objects are stored as files ending @@ -102,7 +102,7 @@ systems. The `bare-user-only` mode is a variant to the `bare-user` mode. Unlike `bare-user`, neither ownership nor extended attributes are stored. These repos are meant to to be checked out in user mode (with the `-U` flag), where this -information is not applied anyway. Hence this mode may loose metadata. +information is not applied anyway. Hence this mode may lose metadata. The main advantage of `bare-user-only` is that repos can be stored on filesystems which do not support extended attributes, such as tmpfs. diff --git a/docs/repository-management.md b/docs/repository-management.md index 11fe2f40..41b8d2b1 100644 --- a/docs/repository-management.md +++ b/docs/repository-management.md @@ -106,8 +106,7 @@ want to "promote" that commit. Let's create a new branch called complete system. This might be where human testers get involved, for example. -A basic way to "promote" the `buildmaster` commit that passed -testing like this: +This is a basic way to "promote" the `buildmaster` commit that passed testing: ``` ostree commit -b exampleos/x86_64/smoketested/standard -s 'Passed tests' --tree=ref=aec070645fe53... From 407c683524bb362cbc39b8445015e4b06e5296a5 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 24 Sep 2020 17:49:48 +0100 Subject: [PATCH 07/47] libostree: Add support for ETag and Last-Modified headers Add support in the soup and curl fetchers to send the `If-None-Match` and `If-Modified-Since` request headers, and pass on the `ETag` and `Last-Modified` response headers. This currently introduces no functional changes, but once call sites provide the appropriate integration, this will allow HTTP caching to happen with requests (typically with metadata requests, where the data is not immutable due to being content-addressed). That should reduce bandwidth requirements. Signed-off-by: Philip Withnall --- src/libostree/ostree-fetcher-curl.c | 251 +++++++++++++++++++++++++++- src/libostree/ostree-fetcher-soup.c | 81 ++++++++- src/libostree/ostree-fetcher-util.c | 43 ++++- src/libostree/ostree-fetcher-util.h | 10 ++ src/libostree/ostree-fetcher.h | 10 ++ src/libostree/ostree-metalink.c | 5 +- src/libostree/ostree-repo-pull.c | 29 +++- 7 files changed, 410 insertions(+), 19 deletions(-) diff --git a/src/libostree/ostree-fetcher-curl.c b/src/libostree/ostree-fetcher-curl.c index 0ce3ff00..975508eb 100644 --- a/src/libostree/ostree-fetcher-curl.c +++ b/src/libostree/ostree-fetcher-curl.c @@ -99,10 +99,16 @@ struct FetcherRequest { guint64 current_size; guint64 max_size; OstreeFetcherRequestFlags flags; + struct curl_slist *req_headers; + char *if_none_match; /* request ETag */ + guint64 if_modified_since; /* seconds since the epoch */ gboolean is_membuf; GError *caught_write_error; GLnxTmpfile tmpf; GString *output_buf; + gboolean out_not_modified; /* TRUE if the server gave a HTTP 304 Not Modified response, which we don’t propagate as an error */ + char *out_etag; /* response ETag */ + guint64 out_last_modified; /* response Last-Modified, seconds since the epoch */ CURL *easy; char error[CURL_ERROR_SIZE]; @@ -335,7 +341,17 @@ check_multi_info (OstreeFetcher *fetcher) else { curl_easy_getinfo (easy, CURLINFO_RESPONSE_CODE, &response); - if (!is_file && !(response >= 200 && response < 300)) + + if (!is_file && response == 304 && + (req->if_none_match != NULL || req->if_modified_since > 0)) + { + /* Version on the server is unchanged from the version we have + * cached locally; report this as an out-argument, a zero-length + * response buffer, and no error. */ + req->out_not_modified = TRUE; + } + + if (!is_file && !(response >= 200 && response < 300) && response != 304) { GIOErrorEnum giocode = _ostree_fetcher_http_status_code_to_io_error (response); @@ -575,6 +591,175 @@ write_cb (void *ptr, size_t size, size_t nmemb, void *data) return realsize; } +/* @buf must already be known to be long enough */ +static gboolean +parse_uint (const char *buf, + guint n_digits, + guint min, + guint max, + guint *out) +{ + guint64 number; + const char *end_ptr = NULL; + gint saved_errno = 0; + + g_return_val_if_fail (n_digits == 2 || n_digits == 4, FALSE); + g_return_val_if_fail (out != NULL, FALSE); + + errno = 0; + number = g_ascii_strtoull (buf, (gchar **)&end_ptr, 10); + saved_errno = errno; + + if (!g_ascii_isdigit (buf[0]) || + saved_errno != 0 || + end_ptr == NULL || + end_ptr != buf + n_digits || + number < min || + number > max) + return FALSE; + + *out = number; + return TRUE; +} + +/* Locale-independent parsing for RFC 2616 date/times. + * + * Reference: https://tools.ietf.org/html/rfc2616#section-3.3.1 + * + * Syntax: + * , :: GMT + * + * Note that this only accepts the full-year and GMT formats specified by + * RFC 1123. It doesn’t accept RFC 850 or asctime formats. + * + * Example: + * Wed, 21 Oct 2015 07:28:00 GMT + */ +static GDateTime * +parse_rfc2616_date_time (const char *buf, + size_t len) +{ + guint day_int, year_int, hour_int, minute_int, second_int; + const char *day_names[] = + { + "Mon", + "Tue", + "Wed", + "Thu", + "Fri", + "Sat", + "Sun", + }; + size_t day_name_index; + const char *month_names[] = + { + "Jan", + "Feb", + "Mar", + "Apr", + "May", + "Jun", + "Jul", + "Aug", + "Sep", + "Oct", + "Nov", + "Dec", + }; + size_t month_name_index; + + if (len != 29) + return NULL; + + const char *day_name = buf; + const char *day = buf + 5; + const char *month_name = day + 3; + const char *year = month_name + 4; + const char *hour = year + 5; + const char *minute = hour + 3; + const char *second = minute + 3; + const char *tz = second + 3; + + for (day_name_index = 0; day_name_index < G_N_ELEMENTS (day_names); day_name_index++) + { + if (strncmp (day_names[day_name_index], day_name, 3) == 0) + break; + } + if (day_name_index >= G_N_ELEMENTS (day_names)) + return NULL; + /* don’t validate whether the day_name matches the rest of the date */ + if (*(day_name + 3) != ',' || *(day_name + 4) != ' ') + return NULL; + if (!parse_uint (day, 2, 1, 31, &day_int)) + return NULL; + if (*(day + 2) != ' ') + return NULL; + for (month_name_index = 0; month_name_index < G_N_ELEMENTS (month_names); month_name_index++) + { + if (strncmp (month_names[month_name_index], month_name, 3) == 0) + break; + } + if (month_name_index >= G_N_ELEMENTS (month_names)) + return NULL; + if (*(month_name + 3) != ' ') + return NULL; + if (!parse_uint (year, 4, 0, 9999, &year_int)) + return NULL; + if (*(year + 4) != ' ') + return NULL; + if (!parse_uint (hour, 2, 0, 23, &hour_int)) + return NULL; + if (*(hour + 2) != ':') + return NULL; + if (!parse_uint (minute, 2, 0, 59, &minute_int)) + return NULL; + if (*(minute + 2) != ':') + return NULL; + if (!parse_uint (second, 2, 0, 60, &second_int)) /* allow leap seconds */ + return NULL; + if (*(second + 2) != ' ') + return NULL; + if (strncmp (tz, "GMT", 3) != 0) + return NULL; + + return g_date_time_new_utc (year_int, month_name_index + 1, day_int, + hour_int, minute_int, second_int); +} + +/* CURLOPT_HEADERFUNCTION */ +static size_t +response_header_cb (const char *buffer, size_t size, size_t n_items, void *user_data) +{ + const size_t real_size = size * n_items; + GTask *task = G_TASK (user_data); + FetcherRequest *req; + + /* libcurl says that @size is always 1, but let’s check + * See https://curl.haxx.se/libcurl/c/CURLOPT_HEADERFUNCTION.html */ + g_assert (size == 1); + + req = g_task_get_task_data (task); + + const char *etag_header = "ETag: "; + const char *last_modified_header = "Last-Modified: "; + + if (real_size > strlen (etag_header) && + strncasecmp (buffer, etag_header, strlen (etag_header)) == 0) + { + g_clear_pointer (&req->out_etag, g_free); + req->out_etag = g_strstrip (g_strdup (buffer + strlen (etag_header))); + } + else if (real_size > strlen (last_modified_header) && + strncasecmp (buffer, last_modified_header, strlen (last_modified_header)) == 0) + { + g_autofree char *lm_buf = g_strstrip (g_strdup (buffer + strlen (last_modified_header))); + g_autoptr(GDateTime) dt = parse_rfc2616_date_time (lm_buf, strlen (lm_buf)); + req->out_last_modified = (dt != NULL) ? g_date_time_to_unix (dt) : 0; + } + + return real_size; +} + /* CURLOPT_PROGRESSFUNCTION */ static int prog_cb (void *p, double dltotal, double dlnow, double ult, double uln) @@ -600,6 +785,9 @@ request_unref (FetcherRequest *req) glnx_tmpfile_clear (&req->tmpf); if (req->output_buf) g_string_free (req->output_buf, TRUE); + g_free (req->if_none_match); + g_free (req->out_etag); + g_clear_pointer (&req->req_headers, (GDestroyNotify)curl_slist_free_all); curl_easy_cleanup (req->easy); g_free (req); @@ -721,8 +909,29 @@ initiate_next_curl_request (FetcherRequest *req, curl_easy_setopt (req->easy, CURLOPT_USERAGENT, self->custom_user_agent ?: OSTREE_FETCHER_USERAGENT_STRING); - if (self->extra_headers) - curl_easy_setopt (req->easy, CURLOPT_HTTPHEADER, self->extra_headers); + + /* Set caching request headers */ + if (req->if_none_match != NULL) + { + g_autofree char *if_none_match = g_strconcat ("If-None-Match: ", req->if_none_match, NULL); + req->req_headers = curl_slist_append (req->req_headers, if_none_match); + } + + if (req->if_modified_since > 0) + { + g_autoptr(GDateTime) date_time = g_date_time_new_from_unix_utc (req->if_modified_since); + g_autofree char *mod_date = g_date_time_format (date_time, "If-Modified-Since: %a, %d %b %Y %H:%M:%S %Z"); + + req->req_headers = curl_slist_append (req->req_headers, mod_date); + } + + /* Append a copy of @extra_headers to @req_headers, as the former could change + * between requests or while a request is in flight */ + for (const struct curl_slist *l = self->extra_headers; l != NULL; l = l->next) + req->req_headers = curl_slist_append (req->req_headers, l->data); + + if (req->req_headers != NULL) + curl_easy_setopt (req->easy, CURLOPT_HTTPHEADER, req->req_headers); if (self->cookie_jar_path) { @@ -796,6 +1005,7 @@ initiate_next_curl_request (FetcherRequest *req, } curl_easy_setopt (req->easy, CURLOPT_WRITEFUNCTION, write_cb); + curl_easy_setopt (req->easy, CURLOPT_HEADERFUNCTION, response_header_cb); if (g_getenv ("OSTREE_DEBUG_HTTP")) curl_easy_setopt (req->easy, CURLOPT_VERBOSE, 1L); curl_easy_setopt (req->easy, CURLOPT_ERRORBUFFER, req->error); @@ -815,6 +1025,7 @@ initiate_next_curl_request (FetcherRequest *req, /* closure bindings -> task */ curl_easy_setopt (req->easy, CURLOPT_PRIVATE, task); curl_easy_setopt (req->easy, CURLOPT_WRITEDATA, task); + curl_easy_setopt (req->easy, CURLOPT_HEADERDATA, task); curl_easy_setopt (req->easy, CURLOPT_PROGRESSDATA, task); CURLMcode multi_rc = curl_multi_add_handle (self->multi, req->easy); @@ -826,6 +1037,8 @@ _ostree_fetcher_request_async (OstreeFetcher *self, GPtrArray *mirrorlist, const char *filename, OstreeFetcherRequestFlags flags, + const char *if_none_match, + guint64 if_modified_since, gboolean is_membuf, guint64 max_size, int priority, @@ -859,6 +1072,8 @@ _ostree_fetcher_request_async (OstreeFetcher *self, req->filename = g_strdup (filename); req->max_size = max_size; req->flags = flags; + req->if_none_match = g_strdup (if_none_match); + req->if_modified_since = if_modified_since; req->is_membuf = is_membuf; /* We'll allocate the tmpfile on demand, so we handle * file I/O errors just in the write func. @@ -882,13 +1097,16 @@ _ostree_fetcher_request_to_tmpfile (OstreeFetcher *self, GPtrArray *mirrorlist, const char *filename, OstreeFetcherRequestFlags flags, + const char *if_none_match, + guint64 if_modified_since, guint64 max_size, int priority, GCancellable *cancellable, GAsyncReadyCallback callback, gpointer user_data) { - _ostree_fetcher_request_async (self, mirrorlist, filename, flags, FALSE, + _ostree_fetcher_request_async (self, mirrorlist, filename, flags, + if_none_match, if_modified_since, FALSE, max_size, priority, cancellable, callback, user_data); } @@ -897,6 +1115,9 @@ gboolean _ostree_fetcher_request_to_tmpfile_finish (OstreeFetcher *self, GAsyncResult *result, GLnxTmpfile *out_tmpf, + gboolean *out_not_modified, + char **out_etag, + guint64 *out_last_modified, GError **error) { g_return_val_if_fail (g_task_is_valid (result, self), FALSE); @@ -912,6 +1133,13 @@ _ostree_fetcher_request_to_tmpfile_finish (OstreeFetcher *self, *out_tmpf = req->tmpf; req->tmpf.initialized = FALSE; /* Transfer ownership */ + if (out_not_modified != NULL) + *out_not_modified = req->out_not_modified; + if (out_etag != NULL) + *out_etag = g_steal_pointer (&req->out_etag); + if (out_last_modified != NULL) + *out_last_modified = req->out_last_modified; + return TRUE; } @@ -920,13 +1148,16 @@ _ostree_fetcher_request_to_membuf (OstreeFetcher *self, GPtrArray *mirrorlist, const char *filename, OstreeFetcherRequestFlags flags, + const char *if_none_match, + guint64 if_modified_since, guint64 max_size, int priority, GCancellable *cancellable, GAsyncReadyCallback callback, gpointer user_data) { - _ostree_fetcher_request_async (self, mirrorlist, filename, flags, TRUE, + _ostree_fetcher_request_async (self, mirrorlist, filename, flags, + if_none_match, if_modified_since, TRUE, max_size, priority, cancellable, callback, user_data); } @@ -935,6 +1166,9 @@ gboolean _ostree_fetcher_request_to_membuf_finish (OstreeFetcher *self, GAsyncResult *result, GBytes **out_buf, + gboolean *out_not_modified, + char **out_etag, + guint64 *out_last_modified, GError **error) { GTask *task; @@ -955,6 +1189,13 @@ _ostree_fetcher_request_to_membuf_finish (OstreeFetcher *self, g_assert (out_buf); *out_buf = ret; + if (out_not_modified != NULL) + *out_not_modified = req->out_not_modified; + if (out_etag != NULL) + *out_etag = g_steal_pointer (&req->out_etag); + if (out_last_modified != NULL) + *out_last_modified = req->out_last_modified; + return TRUE; } diff --git a/src/libostree/ostree-fetcher-soup.c b/src/libostree/ostree-fetcher-soup.c index 970ac7a4..52fe5fc3 100644 --- a/src/libostree/ostree-fetcher-soup.c +++ b/src/libostree/ostree-fetcher-soup.c @@ -90,9 +90,14 @@ typedef struct { gboolean is_membuf; OstreeFetcherRequestFlags flags; + char *if_none_match; /* request ETag */ + guint64 if_modified_since; /* seconds since the epoch */ GInputStream *request_body; GLnxTmpfile tmpf; GOutputStream *out_stream; + gboolean out_not_modified; /* TRUE if the server gave a HTTP 304 Not Modified response, which we don’t propagate as an error */ + char *out_etag; /* response ETag */ + guint64 out_last_modified; /* response Last-Modified, seconds since the epoch */ guint64 max_size; guint64 current_size; @@ -196,8 +201,10 @@ pending_uri_unref (OstreeFetcherPendingURI *pending) g_free (pending->filename); g_clear_object (&pending->request); g_clear_object (&pending->request_body); + g_free (pending->if_none_match); glnx_tmpfile_clear (&pending->tmpf); g_clear_object (&pending->out_stream); + g_free (pending->out_etag); g_free (pending); } @@ -431,6 +438,22 @@ create_pending_soup_request (OstreeFetcherPendingURI *pending, pending->request = soup_session_request_uri (pending->thread_closure->session, (SoupURI*)(uri ? uri : next_mirror), error); + + /* Add caching headers. */ + if (SOUP_IS_REQUEST_HTTP (pending->request) && pending->if_none_match != NULL) + { + glnx_unref_object SoupMessage *msg = soup_request_http_get_message ((SoupRequestHTTP*) pending->request); + soup_message_headers_append (msg->request_headers, "If-None-Match", pending->if_none_match); + } + + if (SOUP_IS_REQUEST_HTTP (pending->request) && pending->if_modified_since > 0) + { + glnx_unref_object SoupMessage *msg = soup_request_http_get_message ((SoupRequestHTTP*) pending->request); + + g_autoptr(GDateTime) date_time = g_date_time_new_from_unix_utc (pending->if_modified_since); + g_autofree char *mod_date = g_date_time_format (date_time, "%a, %d %b %Y %H:%M:%S %Z"); + soup_message_headers_append (msg->request_headers, "If-Modified-Since", mod_date); + } } static void @@ -1050,7 +1073,14 @@ on_request_sent (GObject *object, if (SOUP_IS_REQUEST_HTTP (object)) { msg = soup_request_http_get_message ((SoupRequestHTTP*) object); - if (!SOUP_STATUS_IS_SUCCESSFUL (msg->status_code)) + if (msg->status_code == SOUP_STATUS_NOT_MODIFIED && + (pending->if_none_match != NULL || pending->if_modified_since > 0)) + { + /* Version on the server is unchanged from the version we have cached locally; + * report this as an out-argument, a zero-length response buffer, and no error */ + pending->out_not_modified = TRUE; + } + else if (!SOUP_STATUS_IS_SUCCESSFUL (msg->status_code)) { /* is there another mirror we can try? */ if (pending->mirrorlist_idx + 1 < pending->mirrorlist->len) @@ -1124,6 +1154,21 @@ on_request_sent (GObject *object, } goto out; } + + /* Grab cache properties from the response */ + pending->out_etag = g_strdup (soup_message_headers_get_one (msg->response_headers, "ETag")); + pending->out_last_modified = 0; + + const char *last_modified_str = soup_message_headers_get_one (msg->response_headers, "Last-Modified"); + if (last_modified_str != NULL) + { + SoupDate *soup_date = soup_date_new_from_string (last_modified_str); + if (soup_date != NULL) + { + pending->out_last_modified = soup_date_to_time_t (soup_date); + soup_date_free (soup_date); + } + } } pending->state = OSTREE_FETCHER_STATE_DOWNLOADING; @@ -1154,6 +1199,8 @@ _ostree_fetcher_request_async (OstreeFetcher *self, GPtrArray *mirrorlist, const char *filename, OstreeFetcherRequestFlags flags, + const char *if_none_match, + guint64 if_modified_since, gboolean is_membuf, guint64 max_size, int priority, @@ -1175,6 +1222,8 @@ _ostree_fetcher_request_async (OstreeFetcher *self, pending->mirrorlist = g_ptr_array_ref (mirrorlist); pending->filename = g_strdup (filename); pending->flags = flags; + pending->if_none_match = g_strdup (if_none_match); + pending->if_modified_since = if_modified_since; pending->max_size = max_size; pending->is_membuf = is_membuf; @@ -1196,13 +1245,16 @@ _ostree_fetcher_request_to_tmpfile (OstreeFetcher *self, GPtrArray *mirrorlist, const char *filename, OstreeFetcherRequestFlags flags, + const char *if_none_match, + guint64 if_modified_since, guint64 max_size, int priority, GCancellable *cancellable, GAsyncReadyCallback callback, gpointer user_data) { - _ostree_fetcher_request_async (self, mirrorlist, filename, flags, FALSE, + _ostree_fetcher_request_async (self, mirrorlist, filename, flags, + if_none_match, if_modified_since, FALSE, max_size, priority, cancellable, callback, user_data); } @@ -1211,6 +1263,9 @@ gboolean _ostree_fetcher_request_to_tmpfile_finish (OstreeFetcher *self, GAsyncResult *result, GLnxTmpfile *out_tmpf, + gboolean *out_not_modified, + char **out_etag, + guint64 *out_last_modified, GError **error) { GTask *task; @@ -1231,6 +1286,13 @@ _ostree_fetcher_request_to_tmpfile_finish (OstreeFetcher *self, *out_tmpf = pending->tmpf; pending->tmpf.initialized = FALSE; /* Transfer ownership */ + if (out_not_modified != NULL) + *out_not_modified = pending->out_not_modified; + if (out_etag != NULL) + *out_etag = g_steal_pointer (&pending->out_etag); + if (out_last_modified != NULL) + *out_last_modified = pending->out_last_modified; + return TRUE; } @@ -1239,13 +1301,16 @@ _ostree_fetcher_request_to_membuf (OstreeFetcher *self, GPtrArray *mirrorlist, const char *filename, OstreeFetcherRequestFlags flags, + const char *if_none_match, + guint64 if_modified_since, guint64 max_size, int priority, GCancellable *cancellable, GAsyncReadyCallback callback, gpointer user_data) { - _ostree_fetcher_request_async (self, mirrorlist, filename, flags, TRUE, + _ostree_fetcher_request_async (self, mirrorlist, filename, flags, + if_none_match, if_modified_since, TRUE, max_size, priority, cancellable, callback, user_data); } @@ -1254,6 +1319,9 @@ gboolean _ostree_fetcher_request_to_membuf_finish (OstreeFetcher *self, GAsyncResult *result, GBytes **out_buf, + gboolean *out_not_modified, + char **out_etag, + guint64 *out_last_modified, GError **error) { GTask *task; @@ -1274,6 +1342,13 @@ _ostree_fetcher_request_to_membuf_finish (OstreeFetcher *self, g_assert (out_buf); *out_buf = ret; + if (out_not_modified != NULL) + *out_not_modified = pending->out_not_modified; + if (out_etag != NULL) + *out_etag = g_steal_pointer (&pending->out_etag); + if (out_last_modified != NULL) + *out_last_modified = pending->out_last_modified; + return TRUE; } diff --git a/src/libostree/ostree-fetcher-util.c b/src/libostree/ostree-fetcher-util.c index 1c3f8104..76f6bba1 100644 --- a/src/libostree/ostree-fetcher-util.c +++ b/src/libostree/ostree-fetcher-util.c @@ -34,6 +34,9 @@ typedef struct { GBytes *result_buf; + gboolean result_not_modified; + char *result_etag; + guint64 result_last_modified; /* second since the epoch */ gboolean done; GError **error; } @@ -48,6 +51,8 @@ fetch_uri_sync_on_complete (GObject *object, (void)_ostree_fetcher_request_to_membuf_finish ((OstreeFetcher*)object, result, &data->result_buf, + &data->result_not_modified, + &data->result_etag, &data->result_last_modified, data->error); data->done = TRUE; } @@ -57,7 +62,12 @@ _ostree_fetcher_mirrored_request_to_membuf_once (OstreeFetcher *fe GPtrArray *mirrorlist, const char *filename, OstreeFetcherRequestFlags flags, + const char *if_none_match, + guint64 if_modified_since, GBytes **out_contents, + gboolean *out_not_modified, + char **out_etag, + guint64 *out_last_modified, guint64 max_size, GCancellable *cancellable, GError **error) @@ -79,7 +89,7 @@ _ostree_fetcher_mirrored_request_to_membuf_once (OstreeFetcher *fe data.error = error; _ostree_fetcher_request_to_membuf (fetcher, mirrorlist, filename, - flags, + flags, if_none_match, if_modified_since, max_size, OSTREE_FETCHER_DEFAULT_PRIORITY, cancellable, fetch_uri_sync_on_complete, &data); while (!data.done) @@ -94,6 +104,12 @@ _ostree_fetcher_mirrored_request_to_membuf_once (OstreeFetcher *fe g_clear_error (error); ret = TRUE; *out_contents = NULL; + if (out_not_modified != NULL) + *out_not_modified = FALSE; + if (out_etag != NULL) + *out_etag = NULL; + if (out_last_modified != NULL) + *out_last_modified = 0; } } goto out; @@ -101,10 +117,17 @@ _ostree_fetcher_mirrored_request_to_membuf_once (OstreeFetcher *fe ret = TRUE; *out_contents = g_steal_pointer (&data.result_buf); + if (out_not_modified != NULL) + *out_not_modified = data.result_not_modified; + if (out_etag != NULL) + *out_etag = g_steal_pointer (&data.result_etag); + if (out_last_modified != NULL) + *out_last_modified = data.result_last_modified; out: if (mainctx) g_main_context_pop_thread_default (mainctx); g_clear_pointer (&data.result_buf, (GDestroyNotify)g_bytes_unref); + g_clear_pointer (&data.result_etag, g_free); return ret; } @@ -113,8 +136,13 @@ _ostree_fetcher_mirrored_request_to_membuf (OstreeFetcher *fetcher GPtrArray *mirrorlist, const char *filename, OstreeFetcherRequestFlags flags, + const char *if_none_match, + guint64 if_modified_since, guint n_network_retries, GBytes **out_contents, + gboolean *out_not_modified, + char **out_etag, + guint64 *out_last_modified, guint64 max_size, GCancellable *cancellable, GError **error) @@ -127,7 +155,9 @@ _ostree_fetcher_mirrored_request_to_membuf (OstreeFetcher *fetcher g_clear_error (&local_error); if (_ostree_fetcher_mirrored_request_to_membuf_once (fetcher, mirrorlist, filename, flags, - out_contents, max_size, + if_none_match, if_modified_since, + out_contents, out_not_modified, out_etag, + out_last_modified, max_size, cancellable, &local_error)) return TRUE; } @@ -143,8 +173,13 @@ gboolean _ostree_fetcher_request_uri_to_membuf (OstreeFetcher *fetcher, OstreeFetcherURI *uri, OstreeFetcherRequestFlags flags, + const char *if_none_match, + guint64 if_modified_since, guint n_network_retries, GBytes **out_contents, + gboolean *out_not_modified, + char **out_etag, + guint64 *out_last_modified, guint64 max_size, GCancellable *cancellable, GError **error) @@ -152,7 +187,9 @@ _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, flags, - n_network_retries, out_contents, max_size, + if_none_match, if_modified_since, + n_network_retries, out_contents, + out_not_modified, out_etag, out_last_modified, max_size, cancellable, error); } diff --git a/src/libostree/ostree-fetcher-util.h b/src/libostree/ostree-fetcher-util.h index 46935402..2cbaf5fa 100644 --- a/src/libostree/ostree-fetcher-util.h +++ b/src/libostree/ostree-fetcher-util.h @@ -56,8 +56,13 @@ gboolean _ostree_fetcher_mirrored_request_to_membuf (OstreeFetcher *fetcher, GPtrArray *mirrorlist, const char *filename, OstreeFetcherRequestFlags flags, + const char *if_none_match, + guint64 if_modified_since, guint n_network_retries, GBytes **out_contents, + gboolean *out_not_modified, + char **out_etag, + guint64 *out_last_modified, guint64 max_size, GCancellable *cancellable, GError **error); @@ -65,8 +70,13 @@ gboolean _ostree_fetcher_mirrored_request_to_membuf (OstreeFetcher *fetcher, gboolean _ostree_fetcher_request_uri_to_membuf (OstreeFetcher *fetcher, OstreeFetcherURI *uri, OstreeFetcherRequestFlags flags, + const char *if_none_match, + guint64 if_modified_since, guint n_network_retries, GBytes **out_contents, + gboolean *out_not_modified, + char **out_etag, + guint64 *out_last_modified, guint64 max_size, GCancellable *cancellable, GError **error); diff --git a/src/libostree/ostree-fetcher.h b/src/libostree/ostree-fetcher.h index 32f3ea1b..2065ee54 100644 --- a/src/libostree/ostree-fetcher.h +++ b/src/libostree/ostree-fetcher.h @@ -123,6 +123,8 @@ void _ostree_fetcher_request_to_tmpfile (OstreeFetcher *self, GPtrArray *mirrorlist, const char *filename, OstreeFetcherRequestFlags flags, + const char *if_none_match, + guint64 if_modified_since, guint64 max_size, int priority, GCancellable *cancellable, @@ -132,12 +134,17 @@ void _ostree_fetcher_request_to_tmpfile (OstreeFetcher *self, gboolean _ostree_fetcher_request_to_tmpfile_finish (OstreeFetcher *self, GAsyncResult *result, GLnxTmpfile *out_tmpf, + gboolean *out_not_modified, + char **out_etag, + guint64 *out_last_modified, GError **error); void _ostree_fetcher_request_to_membuf (OstreeFetcher *self, GPtrArray *mirrorlist, const char *filename, OstreeFetcherRequestFlags flags, + const char *if_none_match, + guint64 if_modified_since, guint64 max_size, int priority, GCancellable *cancellable, @@ -147,6 +154,9 @@ void _ostree_fetcher_request_to_membuf (OstreeFetcher *self, gboolean _ostree_fetcher_request_to_membuf_finish (OstreeFetcher *self, GAsyncResult *result, GBytes **out_buf, + gboolean *out_not_modified, + char **out_etag, + guint64 *out_last_modified, GError **error); diff --git a/src/libostree/ostree-metalink.c b/src/libostree/ostree-metalink.c index cb8a50e3..6381b31e 100644 --- a/src/libostree/ostree-metalink.c +++ b/src/libostree/ostree-metalink.c @@ -436,8 +436,10 @@ try_one_url (OstreeMetalinkRequest *self, if (!_ostree_fetcher_request_uri_to_membuf (self->metalink->fetcher, uri, 0, + NULL, 0, self->metalink->n_network_retries, &bytes, + NULL, NULL, NULL, self->metalink->max_size, self->cancellable, error)) @@ -618,8 +620,9 @@ _ostree_metalink_request_sync (OstreeMetalink *self, 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, 0, + NULL, 0, self->n_network_retries, - &contents, self->max_size, + &contents, NULL, NULL, NULL, self->max_size, cancellable, error)) goto out; diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index f16ccec5..a0d983bb 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -458,8 +458,9 @@ fetch_mirrored_uri_contents_utf8_sync (OstreeFetcher *fetcher, g_autoptr(GBytes) bytes = NULL; if (!_ostree_fetcher_mirrored_request_to_membuf (fetcher, mirrorlist, filename, OSTREE_FETCHER_REQUEST_NUL_TERMINATION, + NULL, 0, n_network_retries, - &bytes, + &bytes, NULL, NULL, NULL, OSTREE_MAX_METADATA_SIZE, cancellable, error)) return FALSE; @@ -965,7 +966,7 @@ content_fetch_on_complete (GObject *object, OstreeObjectType objtype; gboolean free_fetch_data = TRUE; - if (!_ostree_fetcher_request_to_tmpfile_finish (fetcher, result, &tmpf, error)) + if (!_ostree_fetcher_request_to_tmpfile_finish (fetcher, result, &tmpf, NULL, NULL, NULL, error)) goto out; ostree_object_name_deserialize (fetch_data->object, &checksum, &objtype); @@ -1105,7 +1106,7 @@ meta_fetch_on_complete (GObject *object, g_debug ("fetch of %s%s complete", checksum_obj, fetch_data->is_detached_meta ? " (detached)" : ""); - if (!_ostree_fetcher_request_to_tmpfile_finish (fetcher, result, &tmpf, error)) + if (!_ostree_fetcher_request_to_tmpfile_finish (fetcher, result, &tmpf, NULL, NULL, NULL, error)) { if (g_error_matches (local_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND)) { @@ -1282,7 +1283,7 @@ static_deltapart_fetch_on_complete (GObject *object, g_debug ("fetch static delta part %s complete", fetch_data->expected_checksum); - if (!_ostree_fetcher_request_to_tmpfile_finish (fetcher, result, &tmpf, error)) + if (!_ostree_fetcher_request_to_tmpfile_finish (fetcher, result, &tmpf, NULL, NULL, NULL, error)) goto out; /* Transfer ownership of the fd */ @@ -1994,7 +1995,7 @@ start_fetch (OtPullData *pull_data, if (!is_meta && pull_data->trusted_http_direct) flags |= OSTREE_FETCHER_REQUEST_LINKABLE; _ostree_fetcher_request_to_tmpfile (pull_data->fetcher, mirrorlist, - obj_subpath, flags, expected_max_size, + obj_subpath, flags, NULL, 0, expected_max_size, is_meta ? OSTREE_REPO_PULL_METADATA_PRIORITY : OSTREE_REPO_PULL_CONTENT_PRIORITY, pull_data->cancellable, @@ -2119,7 +2120,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, 0, fetch->size, + deltapart_path, 0, NULL, 0, fetch->size, OSTREE_FETCHER_DEFAULT_PRIORITY, pull_data->cancellable, static_deltapart_fetch_on_complete, @@ -2494,6 +2495,7 @@ on_superblock_fetched (GObject *src, if (!_ostree_fetcher_request_to_membuf_finish ((OstreeFetcher*)src, res, &delta_superblock_data, + NULL, NULL, NULL, error)) { if (!g_error_matches (local_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND)) @@ -2570,6 +2572,7 @@ start_fetch_delta_superblock (OtPullData *pull_data, _ostree_fetcher_request_to_membuf (pull_data->fetcher, pull_data->content_mirrorlist, delta_name, OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT, + NULL, 0, OSTREE_MAX_METADATA_SIZE, 0, pull_data->cancellable, on_superblock_fetched, @@ -3000,8 +3003,10 @@ _ostree_preload_metadata_file (OstreeRepo *self, { return _ostree_fetcher_mirrored_request_to_membuf (fetcher, mirrorlist, filename, OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT, + NULL, 0, n_network_retries, - out_bytes, OSTREE_MAX_METADATA_SIZE, + out_bytes, NULL, NULL, NULL, + OSTREE_MAX_METADATA_SIZE, cancellable, error); } } @@ -3858,8 +3863,10 @@ ostree_repo_pull_with_options (OstreeRepo *self, if (!_ostree_fetcher_mirrored_request_to_membuf (pull_data->fetcher, pull_data->meta_mirrorlist, "summary.sig", OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT, + NULL, 0, pull_data->n_network_retries, &bytes_sig, + NULL, NULL, NULL, OSTREE_MAX_METADATA_SIZE, cancellable, error)) goto out; @@ -3887,8 +3894,10 @@ ostree_repo_pull_with_options (OstreeRepo *self, if (!_ostree_fetcher_mirrored_request_to_membuf (pull_data->fetcher, pull_data->meta_mirrorlist, "summary", OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT, + NULL, 0, pull_data->n_network_retries, &bytes_summary, + NULL, NULL, NULL, OSTREE_MAX_METADATA_SIZE, cancellable, error)) goto out; @@ -3949,8 +3958,10 @@ ostree_repo_pull_with_options (OstreeRepo *self, pull_data->meta_mirrorlist, "summary", OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT, + NULL, 0, pull_data->n_network_retries, &bytes_summary, + NULL, NULL, NULL, OSTREE_MAX_METADATA_SIZE, cancellable, error)) goto out; @@ -4012,8 +4023,10 @@ ostree_repo_pull_with_options (OstreeRepo *self, pull_data->meta_mirrorlist, "summary", OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT, + NULL, 0, pull_data->n_network_retries, &bytes_summary, + NULL, NULL, NULL, OSTREE_MAX_METADATA_SIZE, cancellable, error)) goto out; @@ -5499,8 +5512,10 @@ find_remotes_cb (GObject *obj, mirrorlist, commit_filename, OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT, + NULL, 0, data->n_network_retries, &commit_bytes, + NULL, NULL, NULL, 0, /* no maximum size */ cancellable, &error)) From 0e3add8d4057b920422bba6405de208cdead00ec Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Tue, 29 Sep 2020 10:51:26 +0100 Subject: [PATCH 08/47] lib/pull: Hook up HTTP caching headers for summary and summary.sig MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As `summary` and `summary.sig` aren’t immutable, HTTP requests to download them can be optimised by sending the `If-None-Match` and `If-Modified-Since` headers to avoid unnecessarily re-downloading them if they haven’t changed since last being checked. Hook them up to the new support for that in the fetcher. The `ETag` and `Last-Modified` for each file in the cache are stored as the `user.etag` xattr and the mtime, respectively. For flatpak, for example, this affects the cached files in `~/.local/share/flatpak/repo/tmp/cache/summaries`. If xattrs aren’t supported, or if the server doesn’t support the caching headers, the pull behaviour is unchanged from before. Signed-off-by: Philip Withnall --- src/libostree/ostree-repo-pull-private.h | 4 + src/libostree/ostree-repo-pull.c | 270 +++++++++++++++++++++-- 2 files changed, 256 insertions(+), 18 deletions(-) diff --git a/src/libostree/ostree-repo-pull-private.h b/src/libostree/ostree-repo-pull-private.h index 689118be..a7ea85cd 100644 --- a/src/libostree/ostree-repo-pull-private.h +++ b/src/libostree/ostree-repo-pull-private.h @@ -72,7 +72,11 @@ typedef struct { gboolean has_tombstone_commits; GBytes *summary_data; + char *summary_etag; + guint64 summary_last_modified; /* seconds since the epoch */ GBytes *summary_data_sig; + char *summary_sig_etag; + guint64 summary_sig_last_modified; /* seconds since the epoch */ GVariant *summary; GHashTable *summary_deltas_checksums; GHashTable *ref_original_commits; /* Maps checksum to commit, used by timestamp checks */ diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index a0d983bb..ae8ad17f 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -46,6 +46,7 @@ #include #include +#include #ifdef HAVE_LIBSYSTEMD #include #endif @@ -2702,6 +2703,64 @@ _ostree_repo_verify_summary (OstreeRepo *self, return TRUE; } +static void +_ostree_repo_load_cache_summary_properties (OstreeRepo *self, + const char *filename, + const char *extension, + char **out_etag, + guint64 *out_last_modified) +{ + const char *file = glnx_strjoina (_OSTREE_SUMMARY_CACHE_DIR, "/", filename, extension); + glnx_autofd int fd = -1; + + if (self->cache_dir_fd == -1) + return; + + if (!glnx_openat_rdonly (self->cache_dir_fd, file, TRUE, &fd, NULL)) + return; + + if (out_etag != NULL) + { + g_autoptr(GBytes) etag_bytes = glnx_fgetxattr_bytes (fd, "user.etag", NULL); + if (etag_bytes != NULL) + { + const guint8 *buf; + gsize buf_len; + + buf = g_bytes_get_data (etag_bytes, &buf_len); + + /* Loosely validate against https://tools.ietf.org/html/rfc7232#section-2.3 + * by checking there are no embedded nuls. */ + for (gsize i = 0; i < buf_len; i++) + { + if (buf[i] == 0) + { + buf_len = 0; + break; + } + } + + /* Nul-terminate and return */ + if (buf_len > 0) + *out_etag = g_strndup ((const char *) buf, buf_len); + else + *out_etag = NULL; + } + else + *out_etag = NULL; + } + + if (out_last_modified != NULL) + { + struct stat statbuf; + + if (glnx_fstatat (fd, "", &statbuf, AT_EMPTY_PATH, NULL)) + *out_last_modified = statbuf.st_mtim.tv_sec; + else + *out_last_modified = 0; + } +} + static gboolean _ostree_repo_load_cache_summary_file (OstreeRepo *self, const char *filename, @@ -2777,11 +2836,38 @@ _ostree_repo_load_cache_summary_if_same_sig (OstreeRepo *self, return TRUE; } +static void +store_file_cache_properties (int dir_fd, + const char *filename, + const char *etag, + guint64 last_modified) +{ + glnx_autofd int fd = -1; + struct timespec time_vals[] = + { + { .tv_sec = last_modified, .tv_nsec = UTIME_OMIT }, /* access, leave unchanged */ + { .tv_sec = last_modified, .tv_nsec = 0 }, /* modification */ + }; + + if (!glnx_openat_rdonly (dir_fd, filename, TRUE, &fd, NULL)) + return; + + if (etag != NULL) + TEMP_FAILURE_RETRY (fsetxattr (fd, "user.etag", etag, strlen (etag), 0)); + else + TEMP_FAILURE_RETRY (fremovexattr (fd, "user.etag")); + + if (last_modified > 0) + TEMP_FAILURE_RETRY (futimens (fd, time_vals)); +} + static gboolean _ostree_repo_save_cache_summary_file (OstreeRepo *self, const char *filename, const char *extension, GBytes *data, + const char *etag, + guint64 last_modified, GCancellable *cancellable, GError **error) { @@ -2802,6 +2888,9 @@ _ostree_repo_save_cache_summary_file (OstreeRepo *self, cancellable, error)) return FALSE; + /* Store the caching properties. This is non-fatal on failure. */ + store_file_cache_properties (self->cache_dir_fd, file, etag, last_modified); + return TRUE; } @@ -2810,16 +2899,24 @@ static gboolean _ostree_repo_cache_summary (OstreeRepo *self, const char *remote, GBytes *summary, + const char *summary_etag, + guint64 summary_last_modified, GBytes *summary_sig, + const char *summary_sig_etag, + guint64 summary_sig_last_modified, GCancellable *cancellable, GError **error) { if (!_ostree_repo_save_cache_summary_file (self, remote, NULL, - summary, cancellable, error)) + summary, + summary_etag, summary_last_modified, + cancellable, error)) return FALSE; if (!_ostree_repo_save_cache_summary_file (self, remote, ".sig", - summary_sig, cancellable, error)) + summary_sig, + summary_sig_etag, summary_sig_last_modified, + cancellable, error)) return FALSE; return TRUE; @@ -2967,8 +3064,13 @@ _ostree_preload_metadata_file (OstreeRepo *self, GPtrArray *mirrorlist, const char *filename, gboolean is_metalink, + const char *if_none_match, + guint64 if_modified_since, guint n_network_retries, GBytes **out_bytes, + gboolean *out_not_modified, + char **out_etag, + guint64 *out_last_modified, GCancellable *cancellable, GError **error) { @@ -3003,9 +3105,9 @@ _ostree_preload_metadata_file (OstreeRepo *self, { return _ostree_fetcher_mirrored_request_to_membuf (fetcher, mirrorlist, filename, OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT, - NULL, 0, + if_none_match, if_modified_since, n_network_retries, - out_bytes, NULL, NULL, NULL, + out_bytes, out_not_modified, out_etag, out_last_modified, OSTREE_MAX_METADATA_SIZE, cancellable, error); } @@ -3365,6 +3467,9 @@ ostree_repo_pull_with_options (OstreeRepo *self, { gboolean ret = FALSE; g_autoptr(GBytes) bytes_summary = NULL; + gboolean summary_not_modified = FALSE; + g_autofree char *summary_etag = NULL; + guint64 summary_last_modified = 0; g_autofree char *metalink_url_str = NULL; g_autoptr(GHashTable) requested_refs_to_fetch = NULL; /* (element-type OstreeCollectionRef utf8) */ g_autoptr(GHashTable) commits_to_fetch = NULL; @@ -3832,6 +3937,9 @@ ostree_repo_pull_with_options (OstreeRepo *self, { g_autoptr(GBytes) bytes_sig = NULL; + gboolean summary_sig_not_modified = FALSE; + g_autofree char *summary_sig_etag = NULL; + guint64 summary_sig_last_modified = 0; gsize n; g_autoptr(GVariant) refs = NULL; g_autoptr(GVariant) deltas = NULL; @@ -3860,16 +3968,47 @@ ostree_repo_pull_with_options (OstreeRepo *self, if (!bytes_sig) { + g_autofree char *summary_sig_if_none_match = NULL; + guint64 summary_sig_if_modified_since = 0; + + /* Load the summary.sig from the network, but send its ETag and + * Last-Modified from the on-disk cache (if it exists) to reduce the + * download size if nothing’s changed. */ + _ostree_repo_load_cache_summary_properties (self, remote_name_or_baseurl, ".sig", + &summary_sig_if_none_match, &summary_sig_if_modified_since); + + g_clear_pointer (&summary_sig_etag, g_free); + summary_sig_last_modified = 0; if (!_ostree_fetcher_mirrored_request_to_membuf (pull_data->fetcher, pull_data->meta_mirrorlist, "summary.sig", OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT, - NULL, 0, + summary_sig_if_none_match, summary_sig_if_modified_since, pull_data->n_network_retries, &bytes_sig, - NULL, NULL, NULL, + &summary_sig_not_modified, &summary_sig_etag, &summary_sig_last_modified, OSTREE_MAX_METADATA_SIZE, cancellable, error)) goto out; + + /* The server returned HTTP status 304 Not Modified, so we’re clear to + * load summary.sig from the cache. Also load summary, since + * `_ostree_repo_load_cache_summary_if_same_sig()` would just do that anyway. */ + if (summary_sig_not_modified) + { + g_clear_pointer (&bytes_sig, g_bytes_unref); + g_clear_pointer (&bytes_summary, g_bytes_unref); + if (!_ostree_repo_load_cache_summary_file (self, remote_name_or_baseurl, ".sig", + &bytes_sig, + cancellable, error)) + goto out; + + if (!bytes_summary && + !pull_data->remote_repo_local && + !_ostree_repo_load_cache_summary_file (self, remote_name_or_baseurl, NULL, + &bytes_summary, + cancellable, error)) + goto out; + } } if (bytes_sig && @@ -3891,16 +4030,35 @@ ostree_repo_pull_with_options (OstreeRepo *self, if (!pull_data->summary && !bytes_summary) { + g_autofree char *summary_if_none_match = NULL; + guint64 summary_if_modified_since = 0; + + _ostree_repo_load_cache_summary_properties (self, remote_name_or_baseurl, NULL, + &summary_if_none_match, &summary_if_modified_since); + + g_clear_pointer (&summary_etag, g_free); + summary_last_modified = 0; if (!_ostree_fetcher_mirrored_request_to_membuf (pull_data->fetcher, pull_data->meta_mirrorlist, "summary", OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT, - NULL, 0, + summary_if_none_match, summary_if_modified_since, pull_data->n_network_retries, &bytes_summary, - NULL, NULL, NULL, + &summary_not_modified, &summary_etag, &summary_last_modified, OSTREE_MAX_METADATA_SIZE, cancellable, error)) goto out; + + /* The server returned HTTP status 304 Not Modified, so we’re clear to + * load summary from the cache. */ + if (summary_not_modified) + { + g_clear_pointer (&bytes_summary, g_bytes_unref); + if (!_ostree_repo_load_cache_summary_file (self, remote_name_or_baseurl, NULL, + &bytes_summary, + cancellable, error)) + goto out; + } } #ifndef OSTREE_DISABLE_GPGME @@ -3939,7 +4097,9 @@ ostree_repo_pull_with_options (OstreeRepo *self, { if (summary_from_cache) { - /* The cached summary doesn't match, fetch a new one and verify again */ + /* The cached summary doesn't match, fetch a new one and verify again. + * Don’t set the cache headers in the HTTP request, to force a + * full download. */ if ((self->test_error_flags & OSTREE_REPO_TEST_ERROR_INVALID_CACHE) > 0) { g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, @@ -3954,14 +4114,16 @@ ostree_repo_pull_with_options (OstreeRepo *self, summary_from_cache = FALSE; g_clear_pointer (&bytes_summary, (GDestroyNotify)g_bytes_unref); + g_clear_pointer (&summary_etag, g_free); + summary_last_modified = 0; if (!_ostree_fetcher_mirrored_request_to_membuf (pull_data->fetcher, pull_data->meta_mirrorlist, "summary", OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT, - NULL, 0, + NULL, 0, /* no cache headers */ pull_data->n_network_retries, &bytes_summary, - NULL, NULL, NULL, + &summary_not_modified, &summary_etag, &summary_last_modified, OSTREE_MAX_METADATA_SIZE, cancellable, error)) goto out; @@ -4004,7 +4166,9 @@ ostree_repo_pull_with_options (OstreeRepo *self, { if (summary_from_cache) { - /* The cached summary doesn't match, fetch a new one and verify again */ + /* The cached summary doesn't match, fetch a new one and verify again. + * Don’t set the cache headers in the HTTP request, to force a + * full download. */ if ((self->test_error_flags & OSTREE_REPO_TEST_ERROR_INVALID_CACHE) > 0) { g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, @@ -4019,14 +4183,16 @@ ostree_repo_pull_with_options (OstreeRepo *self, summary_from_cache = FALSE; g_clear_pointer (&bytes_summary, (GDestroyNotify)g_bytes_unref); + g_clear_pointer (&summary_etag, g_free); + summary_last_modified = 0; if (!_ostree_fetcher_mirrored_request_to_membuf (pull_data->fetcher, pull_data->meta_mirrorlist, "summary", OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT, - NULL, 0, + NULL, 0, /* no cache headers */ pull_data->n_network_retries, &bytes_summary, - NULL, NULL, NULL, + &summary_not_modified, &summary_etag, &summary_last_modified, OSTREE_MAX_METADATA_SIZE, cancellable, error)) goto out; @@ -4046,6 +4212,8 @@ ostree_repo_pull_with_options (OstreeRepo *self, if (bytes_summary) { pull_data->summary_data = g_bytes_ref (bytes_summary); + pull_data->summary_etag = g_strdup (summary_etag); + pull_data->summary_last_modified = summary_last_modified; pull_data->summary = g_variant_new_from_bytes (OSTREE_SUMMARY_GVARIANT_FORMAT, bytes_summary, FALSE); if (!g_variant_is_normal_form (pull_data->summary)) @@ -4063,7 +4231,11 @@ ostree_repo_pull_with_options (OstreeRepo *self, } if (bytes_sig) - pull_data->summary_data_sig = g_bytes_ref (bytes_sig); + { + pull_data->summary_data_sig = g_bytes_ref (bytes_sig); + pull_data->summary_sig_etag = g_strdup (summary_sig_etag); + pull_data->summary_sig_last_modified = summary_sig_last_modified; + } } if (!summary_from_cache && bytes_summary && bytes_sig) @@ -4072,7 +4244,9 @@ ostree_repo_pull_with_options (OstreeRepo *self, !_ostree_repo_cache_summary (self, remote_name_or_baseurl, bytes_summary, + summary_etag, summary_last_modified, bytes_sig, + summary_sig_etag, summary_sig_last_modified, cancellable, error)) goto out; @@ -4507,6 +4681,9 @@ ostree_repo_pull_with_options (OstreeRepo *self, cancellable, error)) goto out; + store_file_cache_properties (pull_data->repo->repo_dir_fd, "summary", + pull_data->summary_etag, pull_data->summary_last_modified); + if (pull_data->summary_data_sig) { buf = g_bytes_get_data (pull_data->summary_data_sig, &len); @@ -4514,6 +4691,9 @@ ostree_repo_pull_with_options (OstreeRepo *self, buf, len, replaceflag, cancellable, error)) goto out; + + store_file_cache_properties (pull_data->repo->repo_dir_fd, "summary.sig", + pull_data->summary_sig_etag, pull_data->summary_sig_last_modified); } } @@ -4700,7 +4880,9 @@ ostree_repo_pull_with_options (OstreeRepo *self, 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_etag, g_free); g_clear_pointer (&pull_data->summary_data_sig, (GDestroyNotify) g_bytes_unref); + g_clear_pointer (&pull_data->summary_sig_etag, g_free); g_clear_pointer (&pull_data->summary, (GDestroyNotify) g_variant_unref); g_clear_pointer (&pull_data->static_delta_superblocks, (GDestroyNotify) g_ptr_array_unref); g_clear_pointer (&pull_data->commit_to_depth, (GDestroyNotify) g_hash_table_unref); @@ -6129,6 +6311,16 @@ ostree_repo_remote_fetch_summary_with_options (OstreeRepo *self, g_autoptr(GPtrArray) mirrorlist = NULL; const char *append_user_agent = NULL; guint n_network_retries = DEFAULT_N_NETWORK_RETRIES; + gboolean summary_sig_not_modified = FALSE; + g_autofree char *summary_sig_if_none_match = NULL; + g_autofree char *summary_sig_etag = NULL; + gboolean summary_not_modified = FALSE; + g_autofree char *summary_if_none_match = NULL; + g_autofree char *summary_etag = NULL; + guint64 summary_sig_if_modified_since = 0; + guint64 summary_sig_last_modified = 0; + guint64 summary_if_modified_since = 0; + guint64 summary_last_modified = 0; g_return_val_if_fail (OSTREE_REPO (self), FALSE); g_return_val_if_fail (name != NULL, FALSE); @@ -6174,22 +6366,48 @@ ostree_repo_remote_fetch_summary_with_options (OstreeRepo *self, &mirrorlist, cancellable, error)) return FALSE; - /* FIXME: Send the ETag from the cache with the request for summary.sig to + /* Send the ETag from the cache with the request for summary.sig to * avoid downloading summary.sig unnecessarily. This won’t normally provide - * any benefits (but won’t do any harm) since summary.sig is typically 500B - * in size. But if a repository has multiple keys, the signature file will + * much benefit since summary.sig is typically 590B in size (vs a 0B HTTP 304 + * response). But if a repository has multiple keys, the signature file will * grow and this optimisation may be useful. */ + _ostree_repo_load_cache_summary_properties (self, name, ".sig", + &summary_sig_if_none_match, &summary_sig_if_modified_since); + _ostree_repo_load_cache_summary_properties (self, name, NULL, + &summary_if_none_match, &summary_if_modified_since); + if (!_ostree_preload_metadata_file (self, fetcher, mirrorlist, "summary.sig", metalink_url_string ? TRUE : FALSE, + summary_sig_if_none_match, summary_sig_if_modified_since, n_network_retries, &signatures, + &summary_sig_not_modified, &summary_sig_etag, &summary_sig_last_modified, cancellable, error)) return FALSE; + /* The server returned HTTP status 304 Not Modified, so we’re clear to + * load summary.sig from the cache. Also load summary, since + * `_ostree_repo_load_cache_summary_if_same_sig()` would just do that anyway. */ + if (summary_sig_not_modified) + { + g_clear_pointer (&signatures, g_bytes_unref); + g_clear_pointer (&summary, g_bytes_unref); + if (!_ostree_repo_load_cache_summary_file (self, name, ".sig", + &signatures, + cancellable, error)) + return FALSE; + + if (!summary && + !_ostree_repo_load_cache_summary_file (self, name, NULL, + &summary, + cancellable, error)) + return FALSE; + } + if (signatures) { if (!_ostree_repo_load_cache_summary_if_same_sig (self, @@ -6210,11 +6428,25 @@ ostree_repo_remote_fetch_summary_with_options (OstreeRepo *self, mirrorlist, "summary", metalink_url_string ? TRUE : FALSE, + summary_if_none_match, summary_if_modified_since, n_network_retries, &summary, + &summary_not_modified, &summary_etag, &summary_last_modified, cancellable, error)) return FALSE; + + /* The server returned HTTP status 304 Not Modified, so we’re clear to + * load summary.sig from the cache. Also load summary, since + * `_ostree_repo_load_cache_summary_if_same_sig()` would just do that anyway. */ + if (summary_not_modified) + { + g_clear_pointer (&summary, g_bytes_unref); + if (!_ostree_repo_load_cache_summary_file (self, name, NULL, + &summary, + cancellable, error)) + return FALSE; + } } if (!_ostree_repo_verify_summary (self, name, @@ -6230,7 +6462,9 @@ ostree_repo_remote_fetch_summary_with_options (OstreeRepo *self, if (!_ostree_repo_cache_summary (self, name, summary, + summary_etag, summary_last_modified, signatures, + summary_sig_etag, summary_sig_last_modified, cancellable, &temp_error)) { From a522bf76283eeb88fcab99a5ee9e54aa0e13f041 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 9 Oct 2020 18:34:55 +0100 Subject: [PATCH 09/47] tests: Add simple test for summary file caching MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This test would have actually passed before the summary file caching changes (in the previous few commits) were added, as the `summary.sig` essentially acted as the ETag for the summary file, and itself wasn’t updated on disk if it didn’t change when querying the server. Actually testing that the HTTP caching headers are working to reduce HTTP traffic would require test hooks into the pull code or the trivial-httpd server, neither of which I have the time to add at the moment. Signed-off-by: Philip Withnall --- Makefile-tests.am | 1 + tests/test-pull-summary-caching.sh | 66 ++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+) create mode 100755 tests/test-pull-summary-caching.sh diff --git a/Makefile-tests.am b/Makefile-tests.am index 3fbc94bf..570f8389 100644 --- a/Makefile-tests.am +++ b/Makefile-tests.am @@ -82,6 +82,7 @@ _installed_or_uninstalled_test_scripts = \ tests/test-pull-mirror-summary.sh \ tests/test-pull-large-metadata.sh \ tests/test-pull-metalink.sh \ + tests/test-pull-summary-caching.sh \ tests/test-pull-summary-sigs.sh \ tests/test-pull-resume.sh \ tests/test-pull-basicauth.sh \ diff --git a/tests/test-pull-summary-caching.sh b/tests/test-pull-summary-caching.sh new file mode 100755 index 00000000..9671199a --- /dev/null +++ b/tests/test-pull-summary-caching.sh @@ -0,0 +1,66 @@ +#!/bin/bash +# +# Copyright © 2020 Endless OS Foundation LLC +# +# SPDX-License-Identifier: LGPL-2.0+ +# +# 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. +# +# Authors: +# - Philip Withnall + +set -euo pipefail + +. $(dirname $0)/libtest.sh + +if ! has_gpgme; then + echo "1..0 #SKIP no gpg support compiled in" + exit 0 +fi + +COMMIT_SIGN="--gpg-homedir=${TEST_GPG_KEYHOME} --gpg-sign=${TEST_GPG_KEYID_1}" + +echo "1..1" + +setup_fake_remote_repo2 "archive" "${COMMIT_SIGN}" + +# Create a few branches and update the summary file (and sign it) +mkdir ${test_tmpdir}/ostree-srv/other-files +cd ${test_tmpdir}/ostree-srv/other-files +echo 'hello world another object' > hello-world +${CMD_PREFIX} ostree --repo=${test_tmpdir}/ostree-srv/gnomerepo commit ${COMMIT_SIGN} -b other -s "A commit" -m "Another Commit body" + +mkdir ${test_tmpdir}/ostree-srv/yet-other-files +cd ${test_tmpdir}/ostree-srv/yet-other-files +echo 'hello world yet another object' > yet-another-hello-world +${CMD_PREFIX} ostree --repo=${test_tmpdir}/ostree-srv/gnomerepo commit ${COMMIT_SIGN} -b yet-another -s "A commit" -m "Another Commit body" + +${CMD_PREFIX} ostree --repo=${test_tmpdir}/ostree-srv/gnomerepo summary -u ${COMMIT_SIGN} + +# Test that pulling twice in a row doesn’t re-download the summary file or its signature +cd ${test_tmpdir} +rm -rf repo +ostree_repo_init repo --mode=archive +${OSTREE} --repo=repo remote add --set=gpg-verify-summary=true origin $(cat httpd-address)/ostree/gnomerepo +${OSTREE} --repo=repo pull origin other +assert_has_file repo/tmp/cache/summaries/origin +assert_has_file repo/tmp/cache/summaries/origin.sig +summary_inode="$(stat -c '%i' repo/tmp/cache/summaries/origin)" +summary_sig_inode="$(stat -c '%i' repo/tmp/cache/summaries/origin.sig)" +${OSTREE} --repo=repo pull origin other +assert_streq "$(stat -c '%i' repo/tmp/cache/summaries/origin)" "${summary_inode}" +assert_streq "$(stat -c '%i' repo/tmp/cache/summaries/origin.sig)" "${summary_sig_inode}" +echo "ok pull caches the summary files" From 92215025aa0eb65f575700d0f3c286d594397e4f Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 9 Oct 2020 18:46:06 +0100 Subject: [PATCH 10/47] ostree/trivial-httpd: Add Last-Modified/ETag support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is basic support for the Last-Modified/ETag/If-Modified-Since/If-None-Match headers. It’s not high performance, and doesn’t support all of the related caching features (like the If-Match header, etc.). Signed-off-by: Philip Withnall --- src/ostree/ostree-trivial-httpd.c | 98 +++++++++++++++++++++++++------ 1 file changed, 80 insertions(+), 18 deletions(-) diff --git a/src/ostree/ostree-trivial-httpd.c b/src/ostree/ostree-trivial-httpd.c index a38abbea..d8bc0556 100644 --- a/src/ostree/ostree-trivial-httpd.c +++ b/src/ostree/ostree-trivial-httpd.c @@ -206,6 +206,15 @@ close_socket (SoupMessage *msg, gpointer user_data) #endif } +/* Returns the ETag including the surrounding quotes */ +static gchar * +calculate_etag (GMappedFile *mapping) +{ + g_autoptr(GBytes) bytes = g_mapped_file_get_bytes (mapping); + g_autofree gchar *checksum = g_compute_checksum_for_bytes (G_CHECKSUM_SHA256, bytes); + return g_strconcat ("\"", checksum, "\"", NULL); +} + static void do_get (OtTrivialHttpd *self, SoupServer *server, @@ -367,31 +376,41 @@ do_get (OtTrivialHttpd *self, soup_message_set_status (msg, SOUP_STATUS_FORBIDDEN); goto out; } + + glnx_autofd int fd = openat (self->root_dfd, path, O_RDONLY | O_CLOEXEC); + if (fd < 0) + { + soup_message_set_status (msg, SOUP_STATUS_INTERNAL_SERVER_ERROR); + goto out; + } + + g_autoptr(GMappedFile) mapping = g_mapped_file_new_from_fd (fd, FALSE, NULL); + if (!mapping) + { + soup_message_set_status (msg, SOUP_STATUS_INTERNAL_SERVER_ERROR); + goto out; + } + (void) close (fd); fd = -1; + + /* Send caching headers */ + g_autoptr(GDateTime) last_modified = g_date_time_new_from_unix_utc (stbuf.st_mtim.tv_sec); + if (last_modified != NULL) + { + g_autofree gchar *formatted = g_date_time_format (last_modified, "%a, %d %b %Y %H:%M:%S GMT"); + soup_message_headers_append (msg->response_headers, "Last-Modified", formatted); + } + + g_autofree gchar *etag = calculate_etag (mapping); + if (etag != NULL) + soup_message_headers_append (msg->response_headers, "ETag", etag); if (msg->method == SOUP_METHOD_GET) { - glnx_autofd int fd = -1; - g_autoptr(GMappedFile) mapping = NULL; gsize buffer_length, file_size; SoupRange *ranges; int ranges_length; gboolean have_ranges; - fd = openat (self->root_dfd, path, O_RDONLY | O_CLOEXEC); - if (fd < 0) - { - soup_message_set_status (msg, SOUP_STATUS_INTERNAL_SERVER_ERROR); - goto out; - } - - mapping = g_mapped_file_new_from_fd (fd, FALSE, NULL); - if (!mapping) - { - soup_message_set_status (msg, SOUP_STATUS_INTERNAL_SERVER_ERROR); - goto out; - } - (void) close (fd); fd = -1; - file_size = g_mapped_file_get_length (mapping); have_ranges = soup_message_headers_get_ranges(msg->request_headers, file_size, &ranges, &ranges_length); if (opt_force_ranges && !have_ranges && g_strrstr (path, "/objects") != NULL) @@ -447,7 +466,50 @@ do_get (OtTrivialHttpd *self, soup_message_headers_append (msg->response_headers, "Content-Length", length); } - soup_message_set_status (msg, SOUP_STATUS_OK); + + /* Check client’s caching headers. */ + const gchar *if_modified_since = soup_message_headers_get_one (msg->request_headers, + "If-Modified-Since"); + const gchar *if_none_match = soup_message_headers_get_one (msg->request_headers, + "If-None-Match"); + + if (if_none_match != NULL && etag != NULL) + { + if (g_strcmp0 (etag, if_none_match) == 0) + { + soup_message_set_status (msg, SOUP_STATUS_NOT_MODIFIED); + soup_message_body_truncate (msg->response_body); + } + else + { + soup_message_set_status (msg, SOUP_STATUS_OK); + } + } + else if (if_modified_since != NULL && last_modified != NULL) + { + SoupDate *if_modified_since_sd = soup_date_new_from_string (if_modified_since); + g_autoptr(GDateTime) if_modified_since_dt = NULL; + + if (if_modified_since_sd != NULL) + if_modified_since_dt = g_date_time_new_from_unix_utc (soup_date_to_time_t (if_modified_since_sd)); + + if (if_modified_since_dt != NULL && + g_date_time_compare (last_modified, if_modified_since_dt) <= 0) + { + soup_message_set_status (msg, SOUP_STATUS_NOT_MODIFIED); + soup_message_body_truncate (msg->response_body); + } + else + { + soup_message_set_status (msg, SOUP_STATUS_OK); + } + + g_clear_pointer (&if_modified_since_sd, soup_date_free); + } + else + { + soup_message_set_status (msg, SOUP_STATUS_OK); + } } out: { From 0974a7faf174988c93832e0b4693a11e3a42821e Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 16 Oct 2020 17:05:54 +0100 Subject: [PATCH 11/47] tests: Split RFC 2616 date parsing code out and add tests This makes it testable, and increases its test coverage too 100% of lines, as measured by `make coverage`. Signed-off-by: Philip Withnall --- Makefile-libostree.am | 2 + Makefile-tests.am | 9 +- src/libostree/ostree-date-utils-private.h | 38 +++++ src/libostree/ostree-date-utils.c | 166 ++++++++++++++++++++++ src/libostree/ostree-fetcher-curl.c | 138 +----------------- tests/.gitignore | 1 + tests/test-rfc2616-dates.c | 123 ++++++++++++++++ 7 files changed, 340 insertions(+), 137 deletions(-) create mode 100644 src/libostree/ostree-date-utils-private.h create mode 100644 src/libostree/ostree-date-utils.c create mode 100644 tests/test-rfc2616-dates.c diff --git a/Makefile-libostree.am b/Makefile-libostree.am index 96b9249b..eeb0b6c6 100644 --- a/Makefile-libostree.am +++ b/Makefile-libostree.am @@ -68,6 +68,8 @@ libostree_1_la_SOURCES = \ src/libostree/ostree-cmdprivate.c \ src/libostree/ostree-core-private.h \ src/libostree/ostree-core.c \ + src/libostree/ostree-date-utils.c \ + src/libostree/ostree-date-utils-private.h \ src/libostree/ostree-dummy-enumtypes.c \ src/libostree/ostree-checksum-input-stream.c \ src/libostree/ostree-checksum-input-stream.h \ diff --git a/Makefile-tests.am b/Makefile-tests.am index 570f8389..257b4a5d 100644 --- a/Makefile-tests.am +++ b/Makefile-tests.am @@ -273,7 +273,8 @@ endif _installed_or_uninstalled_test_programs = tests/test-varint tests/test-ot-unix-utils tests/test-bsdiff tests/test-mutable-tree \ tests/test-keyfile-utils tests/test-ot-opt-utils tests/test-ot-tool-util \ tests/test-checksum tests/test-lzma tests/test-rollsum \ - tests/test-basic-c tests/test-sysroot-c tests/test-pull-c tests/test-repo tests/test-include-ostree-h tests/test-kargs + tests/test-basic-c tests/test-sysroot-c tests/test-pull-c tests/test-repo tests/test-include-ostree-h tests/test-kargs \ + tests/test-rfc2616-dates if USE_GPGME _installed_or_uninstalled_test_programs += \ @@ -390,6 +391,12 @@ tests_test_lzma_SOURCES = src/libostree/ostree-lzma-common.c src/libostree/ostre tests_test_lzma_CFLAGS = $(TESTS_CFLAGS) $(OT_DEP_LZMA_CFLAGS) tests_test_lzma_LDADD = $(TESTS_LDADD) $(OT_DEP_LZMA_LIBS) +tests_test_rfc2616_dates_SOURCES = \ + src/libostree/ostree-date-utils.c \ + tests/test-rfc2616-dates.c +tests_test_rfc2616_dates_CFLAGS = $(TESTS_CFLAGS) +tests_test_rfc2616_dates_LDADD = $(TESTS_LDADD) + if USE_GPGME tests_test_gpg_verify_result_SOURCES = \ src/libostree/ostree-gpg-verify-result-private.h \ diff --git a/src/libostree/ostree-date-utils-private.h b/src/libostree/ostree-date-utils-private.h new file mode 100644 index 00000000..f9b8b3e0 --- /dev/null +++ b/src/libostree/ostree-date-utils-private.h @@ -0,0 +1,38 @@ +/* + * Copyright © 2020 Endless OS Foundation LLC + * + * SPDX-License-Identifier: LGPL-2.0+ + * + * 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. + * + * Authors: + * - Philip Withnall + */ + +#pragma once + +#ifndef __GI_SCANNER__ + +#include + +G_BEGIN_DECLS + +GDateTime *_ostree_parse_rfc2616_date_time (const char *buf, + size_t len); + +G_END_DECLS + +#endif diff --git a/src/libostree/ostree-date-utils.c b/src/libostree/ostree-date-utils.c new file mode 100644 index 00000000..8076e084 --- /dev/null +++ b/src/libostree/ostree-date-utils.c @@ -0,0 +1,166 @@ +/* + * Copyright © 2020 Endless OS Foundation LLC + * + * SPDX-License-Identifier: LGPL-2.0+ + * + * 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. + * + * Authors: + * - Philip Withnall + */ + +#include "config.h" + +#include +#include +#include + +#include "ostree-date-utils-private.h" + +/* @buf must already be known to be long enough */ +static gboolean +parse_uint (const char *buf, + guint n_digits, + guint min, + guint max, + guint *out) +{ + guint64 number; + const char *end_ptr = NULL; + gint saved_errno = 0; + + g_return_val_if_fail (n_digits == 2 || n_digits == 4, FALSE); + g_return_val_if_fail (out != NULL, FALSE); + + errno = 0; + number = g_ascii_strtoull (buf, (gchar **)&end_ptr, 10); + saved_errno = errno; + + if (!g_ascii_isdigit (buf[0]) || + saved_errno != 0 || + end_ptr == NULL || + end_ptr != buf + n_digits || + number < min || + number > max) + return FALSE; + + *out = number; + return TRUE; +} + +/* Locale-independent parsing for RFC 2616 date/times. + * + * Reference: https://tools.ietf.org/html/rfc2616#section-3.3.1 + * + * Syntax: + * , :: GMT + * + * Note that this only accepts the full-year and GMT formats specified by + * RFC 1123. It doesn’t accept RFC 850 or asctime formats. + * + * Example: + * Wed, 21 Oct 2015 07:28:00 GMT + */ +GDateTime * +_ostree_parse_rfc2616_date_time (const char *buf, + size_t len) +{ + guint day_int, year_int, hour_int, minute_int, second_int; + const char *day_names[] = + { + "Mon", + "Tue", + "Wed", + "Thu", + "Fri", + "Sat", + "Sun", + }; + size_t day_name_index; + const char *month_names[] = + { + "Jan", + "Feb", + "Mar", + "Apr", + "May", + "Jun", + "Jul", + "Aug", + "Sep", + "Oct", + "Nov", + "Dec", + }; + size_t month_name_index; + + if (len != 29) + return NULL; + + const char *day_name = buf; + const char *day = buf + 5; + const char *month_name = day + 3; + const char *year = month_name + 4; + const char *hour = year + 5; + const char *minute = hour + 3; + const char *second = minute + 3; + const char *tz = second + 3; + + for (day_name_index = 0; day_name_index < G_N_ELEMENTS (day_names); day_name_index++) + { + if (strncmp (day_names[day_name_index], day_name, 3) == 0) + break; + } + if (day_name_index >= G_N_ELEMENTS (day_names)) + return NULL; + /* don’t validate whether the day_name matches the rest of the date */ + if (*(day_name + 3) != ',' || *(day_name + 4) != ' ') + return NULL; + if (!parse_uint (day, 2, 1, 31, &day_int)) + return NULL; + if (*(day + 2) != ' ') + return NULL; + for (month_name_index = 0; month_name_index < G_N_ELEMENTS (month_names); month_name_index++) + { + if (strncmp (month_names[month_name_index], month_name, 3) == 0) + break; + } + if (month_name_index >= G_N_ELEMENTS (month_names)) + return NULL; + if (*(month_name + 3) != ' ') + return NULL; + if (!parse_uint (year, 4, 0, 9999, &year_int)) + return NULL; + if (*(year + 4) != ' ') + return NULL; + if (!parse_uint (hour, 2, 0, 23, &hour_int)) + return NULL; + if (*(hour + 2) != ':') + return NULL; + if (!parse_uint (minute, 2, 0, 59, &minute_int)) + return NULL; + if (*(minute + 2) != ':') + return NULL; + if (!parse_uint (second, 2, 0, 60, &second_int)) /* allow leap seconds */ + return NULL; + if (*(second + 2) != ' ') + return NULL; + if (strncmp (tz, "GMT", 3) != 0) + return NULL; + + return g_date_time_new_utc (year_int, month_name_index + 1, day_int, + hour_int, minute_int, second_int); +} diff --git a/src/libostree/ostree-fetcher-curl.c b/src/libostree/ostree-fetcher-curl.c index 975508eb..129e6988 100644 --- a/src/libostree/ostree-fetcher-curl.c +++ b/src/libostree/ostree-fetcher-curl.c @@ -45,6 +45,7 @@ #define CURLPIPE_MULTIPLEX 0 #endif +#include "ostree-date-utils-private.h" #include "ostree-fetcher.h" #include "ostree-fetcher-util.h" #include "ostree-enumtypes.h" @@ -591,141 +592,6 @@ write_cb (void *ptr, size_t size, size_t nmemb, void *data) return realsize; } -/* @buf must already be known to be long enough */ -static gboolean -parse_uint (const char *buf, - guint n_digits, - guint min, - guint max, - guint *out) -{ - guint64 number; - const char *end_ptr = NULL; - gint saved_errno = 0; - - g_return_val_if_fail (n_digits == 2 || n_digits == 4, FALSE); - g_return_val_if_fail (out != NULL, FALSE); - - errno = 0; - number = g_ascii_strtoull (buf, (gchar **)&end_ptr, 10); - saved_errno = errno; - - if (!g_ascii_isdigit (buf[0]) || - saved_errno != 0 || - end_ptr == NULL || - end_ptr != buf + n_digits || - number < min || - number > max) - return FALSE; - - *out = number; - return TRUE; -} - -/* Locale-independent parsing for RFC 2616 date/times. - * - * Reference: https://tools.ietf.org/html/rfc2616#section-3.3.1 - * - * Syntax: - * , :: GMT - * - * Note that this only accepts the full-year and GMT formats specified by - * RFC 1123. It doesn’t accept RFC 850 or asctime formats. - * - * Example: - * Wed, 21 Oct 2015 07:28:00 GMT - */ -static GDateTime * -parse_rfc2616_date_time (const char *buf, - size_t len) -{ - guint day_int, year_int, hour_int, minute_int, second_int; - const char *day_names[] = - { - "Mon", - "Tue", - "Wed", - "Thu", - "Fri", - "Sat", - "Sun", - }; - size_t day_name_index; - const char *month_names[] = - { - "Jan", - "Feb", - "Mar", - "Apr", - "May", - "Jun", - "Jul", - "Aug", - "Sep", - "Oct", - "Nov", - "Dec", - }; - size_t month_name_index; - - if (len != 29) - return NULL; - - const char *day_name = buf; - const char *day = buf + 5; - const char *month_name = day + 3; - const char *year = month_name + 4; - const char *hour = year + 5; - const char *minute = hour + 3; - const char *second = minute + 3; - const char *tz = second + 3; - - for (day_name_index = 0; day_name_index < G_N_ELEMENTS (day_names); day_name_index++) - { - if (strncmp (day_names[day_name_index], day_name, 3) == 0) - break; - } - if (day_name_index >= G_N_ELEMENTS (day_names)) - return NULL; - /* don’t validate whether the day_name matches the rest of the date */ - if (*(day_name + 3) != ',' || *(day_name + 4) != ' ') - return NULL; - if (!parse_uint (day, 2, 1, 31, &day_int)) - return NULL; - if (*(day + 2) != ' ') - return NULL; - for (month_name_index = 0; month_name_index < G_N_ELEMENTS (month_names); month_name_index++) - { - if (strncmp (month_names[month_name_index], month_name, 3) == 0) - break; - } - if (month_name_index >= G_N_ELEMENTS (month_names)) - return NULL; - if (*(month_name + 3) != ' ') - return NULL; - if (!parse_uint (year, 4, 0, 9999, &year_int)) - return NULL; - if (*(year + 4) != ' ') - return NULL; - if (!parse_uint (hour, 2, 0, 23, &hour_int)) - return NULL; - if (*(hour + 2) != ':') - return NULL; - if (!parse_uint (minute, 2, 0, 59, &minute_int)) - return NULL; - if (*(minute + 2) != ':') - return NULL; - if (!parse_uint (second, 2, 0, 60, &second_int)) /* allow leap seconds */ - return NULL; - if (*(second + 2) != ' ') - return NULL; - if (strncmp (tz, "GMT", 3) != 0) - return NULL; - - return g_date_time_new_utc (year_int, month_name_index + 1, day_int, - hour_int, minute_int, second_int); -} - /* CURLOPT_HEADERFUNCTION */ static size_t response_header_cb (const char *buffer, size_t size, size_t n_items, void *user_data) @@ -753,7 +619,7 @@ response_header_cb (const char *buffer, size_t size, size_t n_items, void *user_ strncasecmp (buffer, last_modified_header, strlen (last_modified_header)) == 0) { g_autofree char *lm_buf = g_strstrip (g_strdup (buffer + strlen (last_modified_header))); - g_autoptr(GDateTime) dt = parse_rfc2616_date_time (lm_buf, strlen (lm_buf)); + g_autoptr(GDateTime) dt = _ostree_parse_rfc2616_date_time (lm_buf, strlen (lm_buf)); req->out_last_modified = (dt != NULL) ? g_date_time_to_unix (dt) : 0; } diff --git a/tests/.gitignore b/tests/.gitignore index f5e95e49..938c169f 100644 --- a/tests/.gitignore +++ b/tests/.gitignore @@ -21,5 +21,6 @@ test-repo test-repo-finder-avahi test-repo-finder-config test-repo-finder-mount +test-rfc2616-dates test-rollsum-cli test-kargs diff --git a/tests/test-rfc2616-dates.c b/tests/test-rfc2616-dates.c new file mode 100644 index 00000000..d3f2073e --- /dev/null +++ b/tests/test-rfc2616-dates.c @@ -0,0 +1,123 @@ +/* + * Copyright © 2020 Endless OS Foundation LLC + * + * SPDX-License-Identifier: LGPL-2.0+ + * + * 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. + * + * Authors: + * - Philip Withnall + */ + +#include "config.h" + +#include + +#include "ostree-date-utils-private.h" + +static void +test_ostree_parse_rfc2616_date_time (void) +{ +#if GLIB_CHECK_VERSION(2, 62, 0) +G_GNUC_BEGIN_IGNORE_DEPRECATIONS + const struct + { + const char *rfc2616; + const char *expected_iso8601; /* (nullable) if parsing is expected to fail */ + } + tests[] = + { + { "Wed, 21 Oct 2015 07:28:00 GMT", "2015-10-21T07:28:00Z" }, + { "Wed, 21 Oct 2015 07:28:00", NULL }, /* too short */ + { "Wed, 21 Oct 2015 07:28:00 CEST", NULL }, /* too long; not GMT */ + { "Cat, 21 Oct 2015 07:28:00 GMT", NULL }, /* invalid day */ + { "Wed 21 Oct 2015 07:28:00 GMT", NULL }, /* no comma */ + { "Wed,21 Oct 2015 07:28:00 GMT ", NULL }, /* missing space */ + { "Wed, xx Oct 2015 07:28:00 GMT", NULL }, /* no day-of-month */ + { "Wed, 011Oct 2015 07:28:00 GMT", NULL }, /* overlong day-of-month */ + { "Wed, 00 Oct 2015 07:28:00 GMT", NULL }, /* day-of-month underflow */ + { "Wed, 32 Oct 2015 07:28:00 GMT", NULL }, /* day-of-month overflow */ + { "Wed, 21,Oct 2015 07:28:00 GMT", NULL }, /* missing space */ + { "Wed, 21 Cat 2015 07:28:00 GMT", NULL }, /* invalid month */ + { "Wed, 21 Oct,2015 07:28:00 GMT", NULL }, /* missing space */ + { "Wed, 21 Oct xxxx 07:28:00 GMT", NULL }, /* no year */ + { "Wed, 21 Oct 0201507:28:00 GMT", NULL }, /* overlong year */ + { "Wed, 21 Oct 0000 07:28:00 GMT", NULL }, /* year underflow */ + { "Wed, 21 Oct 10000 07:28:00 GM", NULL }, /* year overflow */ + { "Wed, 21 Oct 2015,07:28:00 GMT", NULL }, /* missing space */ + { "Wed, 21 Oct 2015 07 28:00 GMT", NULL }, /* missing colon */ + { "Wed, 21 Oct 2015 007:28:00 GM", NULL }, /* overlong hour */ + { "Wed, 21 Oct 2015 xx:28:00 GMT", NULL }, /* missing hour */ + { "Wed, 21 Oct 2015 -1:28:00 GMT", NULL }, /* hour underflow */ + { "Wed, 21 Oct 2015 24:28:00 GMT", NULL }, /* hour overflow */ + { "Wed, 21 Oct 2015 07:28 00 GMT", NULL }, /* missing colon */ + { "Wed, 21 Oct 2015 07:028:00 GM", NULL }, /* overlong minute */ + { "Wed, 21 Oct 2015 07:xx:00 GMT", NULL }, /* missing minute */ + { "Wed, 21 Oct 2015 07:-1:00 GMT", NULL }, /* minute underflow */ + { "Wed, 21 Oct 2015 07:60:00 GMT", NULL }, /* minute overflow */ + { "Wed, 21 Oct 2015 07:28:00CEST", NULL }, /* missing space */ + { "Wed, 21 Oct 2015 07:28:000 GM", NULL }, /* overlong second */ + { "Wed, 21 Oct 2015 07:28:xx GMT", NULL }, /* missing second */ + { "Wed, 21 Oct 2015 07:28:-1 GMT", NULL }, /* seconds underflow */ + { "Wed, 21 Oct 2015 07:28:61 GMT", NULL }, /* seconds overflow */ + { "Wed, 21 Oct 2015 07:28:00 UTC", NULL }, /* invalid timezone (only GMT is allowed) */ + { "Thu, 01 Jan 1970 00:00:00 GMT", "1970-01-01T00:00:00Z" }, /* extreme but valid date */ + { "Mon, 31 Dec 9999 23:59:59 GMT", "9999-12-31T23:59:59Z" }, /* extreme but valid date */ + }; + + for (gsize i = 0; i < G_N_ELEMENTS (tests); i++) + { + g_test_message ("Test %" G_GSIZE_FORMAT ": %s", i, tests[i].rfc2616); + + /* Parse once with a trailing nul */ + g_autoptr(GDateTime) dt1 = _ostree_parse_rfc2616_date_time (tests[i].rfc2616, strlen (tests[i].rfc2616)); + if (tests[i].expected_iso8601 == NULL) + g_assert_null (dt1); + else + { + g_assert_nonnull (dt1); + g_autofree char *iso8601 = g_date_time_format_iso8601 (dt1); + g_assert_cmpstr (iso8601, ==, tests[i].expected_iso8601); + } + + /* And parse again with no trailing nul */ + g_autofree char *rfc2616_no_nul = g_malloc (strlen (tests[i].rfc2616)); + memcpy (rfc2616_no_nul, tests[i].rfc2616, strlen (tests[i].rfc2616)); + g_autoptr(GDateTime) dt2 = _ostree_parse_rfc2616_date_time (rfc2616_no_nul, strlen (tests[i].rfc2616)); + if (tests[i].expected_iso8601 == NULL) + g_assert_null (dt2); + else + { + g_assert_nonnull (dt2); + g_autofree char *iso8601 = g_date_time_format_iso8601 (dt2); + g_assert_cmpstr (iso8601, ==, tests[i].expected_iso8601); + } + } +G_GNUC_END_IGNORE_DEPRECATIONS +#else + /* GLib 2.62 is needed for g_date_time_format_iso8601(). */ + g_test_skip ("RFC 2616 date parsing test needs GLib ≥ 2.62.0"); +#endif +} + +int +main (int argc, + char **argv) +{ + g_test_init (&argc, &argv, NULL); + g_test_add_func ("/ostree_parse_rfc2616_date_time", test_ostree_parse_rfc2616_date_time); + return g_test_run (); +} From 87e564eeaa569adde786c4c39a2626ca3d687436 Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Tue, 1 Sep 2020 12:00:32 +0200 Subject: [PATCH 12/47] deltas: Add _ostree_get_relative_static_delta_index_path() This gets the subpath for a delta index file, which is of the form "delta-indexes/$commit.index", that contains all the deltas going to the particular commit. --- src/libostree/ostree-core-private.h | 3 +++ src/libostree/ostree-core.c | 30 ++++++++++++++++++++++++----- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/libostree/ostree-core-private.h b/src/libostree/ostree-core-private.h index 5d9d948a..89d95ad9 100644 --- a/src/libostree/ostree-core-private.h +++ b/src/libostree/ostree-core-private.h @@ -135,6 +135,9 @@ _ostree_get_relative_static_delta_part_path (const char *from, const char *to, guint i); +char * +_ostree_get_relative_static_delta_index_path (const char *to); + static inline char * _ostree_get_commitpartial_path (const char *checksum) { return g_strconcat ("state/", checksum, ".commitpartial", NULL); diff --git a/src/libostree/ostree-core.c b/src/libostree/ostree-core.c index 29528fa5..f822101a 100644 --- a/src/libostree/ostree-core.c +++ b/src/libostree/ostree-core.c @@ -1814,15 +1814,15 @@ _ostree_get_relative_object_path (const char *checksum, return g_string_free (path, FALSE); } -char * -_ostree_get_relative_static_delta_path (const char *from, - const char *to, - const char *target) +static GString * +static_delta_path_base (const char *dir, + const char *from, + const char *to) { guint8 csum_to[OSTREE_SHA256_DIGEST_LEN]; char to_b64[44]; guint8 csum_to_copy[OSTREE_SHA256_DIGEST_LEN]; - GString *ret = g_string_new ("deltas/"); + GString *ret = g_string_new (dir); ostree_checksum_inplace_to_bytes (to, csum_to); ostree_checksum_b64_inplace_from_bytes (csum_to, to_b64); @@ -1851,6 +1851,16 @@ _ostree_get_relative_static_delta_path (const char *from, g_string_append_c (ret, '/'); g_string_append (ret, to_b64 + 2); + return ret; +} + +char * +_ostree_get_relative_static_delta_path (const char *from, + const char *to, + const char *target) +{ + GString *ret = static_delta_path_base ("deltas/", from, to); + if (target != NULL) { g_string_append_c (ret, '/'); @@ -1883,6 +1893,16 @@ _ostree_get_relative_static_delta_part_path (const char *from, return _ostree_get_relative_static_delta_path (from, to, partstr); } +char * +_ostree_get_relative_static_delta_index_path (const char *to) +{ + GString *ret = static_delta_path_base ("delta-indexes/", NULL, to); + + g_string_append (ret, ".index"); + + return g_string_free (ret, FALSE); +} + gboolean _ostree_parse_delta_name (const char *delta_name, char **out_from, From 8e1f199dd4d035b5ff501aa37c6f1229ac3c0e61 Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Tue, 1 Sep 2020 12:03:40 +0200 Subject: [PATCH 13/47] deltas: Add ostree_repo_list_static_delta_indexes() function This lists all the available delta indexes. --- Makefile-libostree.am | 6 +- apidoc/ostree-sections.txt | 1 + src/libostree/libostree-devel.sym | 7 +- src/libostree/ostree-repo-static-delta-core.c | 87 +++++++++++++++++++ src/libostree/ostree-repo.h | 6 ++ 5 files changed, 101 insertions(+), 6 deletions(-) diff --git a/Makefile-libostree.am b/Makefile-libostree.am index bdc47122..eeb0b6c6 100644 --- a/Makefile-libostree.am +++ b/Makefile-libostree.am @@ -184,9 +184,9 @@ libostree_1_la_SOURCES += \ endif # USE_GPGME symbol_files = $(top_srcdir)/src/libostree/libostree-released.sym -#if BUILDOPT_IS_DEVEL_BUILD -#symbol_files += $(top_srcdir)/src/libostree/libostree-devel.sym -#endif +if BUILDOPT_IS_DEVEL_BUILD +symbol_files += $(top_srcdir)/src/libostree/libostree-devel.sym +endif # http://blog.jgc.org/2007/06/escaping-comma-and-space-in-gnu-make.html wl_versionscript_arg = -Wl,--version-script= EXTRA_DIST += \ diff --git a/apidoc/ostree-sections.txt b/apidoc/ostree-sections.txt index c1d6b35e..ea37823b 100644 --- a/apidoc/ostree-sections.txt +++ b/apidoc/ostree-sections.txt @@ -412,6 +412,7 @@ OSTREE_REPO_LIST_OBJECTS_VARIANT_TYPE ostree_repo_list_objects ostree_repo_list_commit_objects_starting_with ostree_repo_list_static_delta_names +ostree_repo_list_static_delta_indexes OstreeStaticDeltaGenerateOpt ostree_repo_static_delta_generate ostree_repo_static_delta_execute_offline_with_signature diff --git a/src/libostree/libostree-devel.sym b/src/libostree/libostree-devel.sym index 8e8d9c81..1350341c 100644 --- a/src/libostree/libostree-devel.sym +++ b/src/libostree/libostree-devel.sym @@ -17,9 +17,10 @@ Boston, MA 02111-1307, USA. ***/ -/* Copy the bits below and uncomment the include in Makefile-libostree.am - when adding a symbol. -*/ +LIBOSTREE_2020.8 { +global: + ostree_repo_list_static_delta_indexes; +} LIBOSTREE_2020.7; /* Stub section for the stable release *after* this development one; don't * edit this other than to update the year. This is just a copy/paste diff --git a/src/libostree/ostree-repo-static-delta-core.c b/src/libostree/ostree-repo-static-delta-core.c index e263e928..e3432aa5 100644 --- a/src/libostree/ostree-repo-static-delta-core.c +++ b/src/libostree/ostree-repo-static-delta-core.c @@ -168,6 +168,93 @@ ostree_repo_list_static_delta_names (OstreeRepo *self, return TRUE; } +/** + * ostree_repo_list_static_delta_indexes: + * @self: Repo + * @out_indexes: (out) (element-type utf8) (transfer container): String name of delta indexes (checksum) + * @cancellable: Cancellable + * @error: Error + * + * This function synchronously enumerates all static delta indexes in the + * repository, returning its result in @out_indexes. + * + * Since: 2020.7 + */ +gboolean +ostree_repo_list_static_delta_indexes (OstreeRepo *self, + GPtrArray **out_indexes, + GCancellable *cancellable, + GError **error) +{ + g_autoptr(GPtrArray) ret_indexes = g_ptr_array_new_with_free_func (g_free); + + g_auto(GLnxDirFdIterator) dfd_iter = { 0, }; + gboolean exists; + if (!ot_dfd_iter_init_allow_noent (self->repo_dir_fd, "delta-indexes", &dfd_iter, + &exists, error)) + return FALSE; + if (!exists) + { + /* Note early return */ + ot_transfer_out_value (out_indexes, &ret_indexes); + return TRUE; + } + + while (TRUE) + { + g_auto(GLnxDirFdIterator) sub_dfd_iter = { 0, }; + struct dirent *dent; + + if (!glnx_dirfd_iterator_next_dent_ensure_dtype (&dfd_iter, &dent, cancellable, error)) + return FALSE; + if (dent == NULL) + break; + if (dent->d_type != DT_DIR) + continue; + if (strlen (dent->d_name) != 2) + continue; + + if (!glnx_dirfd_iterator_init_at (dfd_iter.fd, dent->d_name, FALSE, + &sub_dfd_iter, error)) + return FALSE; + + while (TRUE) + { + struct dirent *sub_dent; + + if (!glnx_dirfd_iterator_next_dent_ensure_dtype (&sub_dfd_iter, &sub_dent, + cancellable, error)) + return FALSE; + if (sub_dent == NULL) + break; + if (sub_dent->d_type != DT_REG) + continue; + + const char *name1 = dent->d_name; + const char *name2 = sub_dent->d_name; + + /* base64 len is 43, but 2 chars are in the parent dir name */ + if (strlen (name2) != 41 + strlen(".index") || + !g_str_has_suffix (name2, ".index")) + continue; + + g_autoptr(GString) out = g_string_new (name1); + g_string_append_len (out, name2, 41); + + char checksum[OSTREE_SHA256_STRING_LEN+1]; + guchar csum[OSTREE_SHA256_DIGEST_LEN]; + + ostree_checksum_b64_inplace_to_bytes (out->str, csum); + ostree_checksum_inplace_from_bytes (csum, checksum); + + g_ptr_array_add (ret_indexes, g_strdup (checksum)); + } + } + + ot_transfer_out_value (out_indexes, &ret_indexes); + return TRUE; +} + gboolean _ostree_repo_static_delta_part_have_all_objects (OstreeRepo *repo, GVariant *checksum_array, diff --git a/src/libostree/ostree-repo.h b/src/libostree/ostree-repo.h index d52fc9c3..a3c5dc77 100644 --- a/src/libostree/ostree-repo.h +++ b/src/libostree/ostree-repo.h @@ -1046,6 +1046,12 @@ gboolean ostree_repo_list_static_delta_names (OstreeRepo *self, GCancellable *cancellable, GError **error); +_OSTREE_PUBLIC +gboolean ostree_repo_list_static_delta_indexes (OstreeRepo *self, + GPtrArray **out_indexes, + GCancellable *cancellable, + GError **error); + /** * OstreeStaticDeltaGenerateOpt: * @OSTREE_STATIC_DELTA_GENERATE_OPT_LOWLATENCY: Optimize for speed of delta creation over space From effde3d513de1570488ddf13dac43c6413a44c68 Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Tue, 1 Sep 2020 12:05:36 +0200 Subject: [PATCH 14/47] deltas: Update delta indexes when updating summary When we update the summary file (and its list of deltas) we also update all delta indexes. The index format is a single `a{sv}` variant identical to the metadata-part of the summary with (currently) only the `ostree.static-deltas` key. Since we expect most delta indexes to change rarely, we avoid unnecessary writes when reindexing. New indexes are compared to existing ones and only the changed ones are written to disk. This avoids unnecessary write load and mtime changes on the repo server. --- src/libostree/ostree-repo-static-delta-core.c | 165 ++++++++++++++++++ .../ostree-repo-static-delta-private.h | 5 + src/libostree/ostree-repo.c | 3 + 3 files changed, 173 insertions(+) diff --git a/src/libostree/ostree-repo-static-delta-core.c b/src/libostree/ostree-repo-static-delta-core.c index e3432aa5..c58e3683 100644 --- a/src/libostree/ostree-repo-static-delta-core.c +++ b/src/libostree/ostree-repo-static-delta-core.c @@ -1205,3 +1205,168 @@ ostree_repo_static_delta_verify_signature (OstreeRepo *self, return _ostree_repo_static_delta_verify_signature (self, delta_fd, sign, out_success_message, error); } + +static void +null_or_ptr_array_unref (GPtrArray *array) +{ + if (array != NULL) + g_ptr_array_unref (array); +} + +static gboolean +file_has_content (OstreeRepo *repo, + const char *subpath, + GBytes *data, + GCancellable *cancellable) +{ + struct stat stbuf; + glnx_autofd int existing_fd = -1; + + if (!glnx_fstatat (repo->repo_dir_fd, subpath, &stbuf, 0, NULL)) + return FALSE; + + if (stbuf.st_size != g_bytes_get_size (data)) + return FALSE; + + if (!glnx_openat_rdonly (repo->repo_dir_fd, subpath, TRUE, &existing_fd, NULL)) + return FALSE; + + g_autoptr(GBytes) existing_data = glnx_fd_readall_bytes (existing_fd, cancellable, NULL); + if (existing_data == NULL) + return FALSE; + + return g_bytes_equal (existing_data, data); +} + +gboolean +_ostree_repo_static_delta_reindex (OstreeRepo *repo, + const char *opt_to_commit, + GCancellable *cancellable, + GError **error) +{ + g_autoptr(GPtrArray) all_deltas = NULL; + g_autoptr(GHashTable) deltas_to_commit_ht = NULL; /* map: to checksum -> ptrarray of from checksums (or NULL) */ + + deltas_to_commit_ht = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, (GDestroyNotify)null_or_ptr_array_unref); + + if (opt_to_commit == NULL) + { + g_autoptr(GPtrArray) old_indexes = NULL; + + /* To ensure all old index files either is regenerated, or + * removed, we initialize all existing indexes to NULL in the + * hashtable. */ + if (!ostree_repo_list_static_delta_indexes (repo, &old_indexes, cancellable, error)) + return FALSE; + + for (int i = 0; i < old_indexes->len; i++) + { + const char *old_index = g_ptr_array_index (old_indexes, i); + g_hash_table_insert (deltas_to_commit_ht, g_strdup (old_index), NULL); + } + } + else + { + if (!ostree_validate_checksum_string (opt_to_commit, error)) + return FALSE; + + /* We ensure the specific old index either is regenerated, or removed */ + g_hash_table_insert (deltas_to_commit_ht, g_strdup (opt_to_commit), NULL); + } + + if (!ostree_repo_list_static_delta_names (repo, &all_deltas, cancellable, error)) + return FALSE; + + for (int i = 0; i < all_deltas->len; i++) + { + const char *delta_name = g_ptr_array_index (all_deltas, i); + g_autofree char *from = NULL; + g_autofree char *to = NULL; + GPtrArray *deltas_to_commit = NULL; + + if (!_ostree_parse_delta_name (delta_name, &from, &to, error)) + return FALSE; + + if (opt_to_commit != NULL && strcmp (to, opt_to_commit) != 0) + continue; + + deltas_to_commit = g_hash_table_lookup (deltas_to_commit_ht, to); + if (deltas_to_commit == NULL) + { + deltas_to_commit = g_ptr_array_new_with_free_func (g_free); + g_hash_table_insert (deltas_to_commit_ht, g_steal_pointer (&to), deltas_to_commit); + } + + g_ptr_array_add (deltas_to_commit, g_steal_pointer (&from)); + } + + GLNX_HASH_TABLE_FOREACH_KV (deltas_to_commit_ht, const char*, to, GPtrArray*, froms) + { + g_autofree char *index_path = _ostree_get_relative_static_delta_index_path (to); + + if (froms == NULL) + { + /* No index to this checksum seen, delete if it exists */ + + g_debug ("Removing delta index for %s", to); + if (!ot_ensure_unlinked_at (repo->repo_dir_fd, index_path, error)) + return FALSE; + } + else + { + g_auto(GVariantDict) index_builder = OT_VARIANT_BUILDER_INITIALIZER; + g_auto(GVariantDict) deltas_builder = OT_VARIANT_BUILDER_INITIALIZER; + g_autoptr(GVariant) index_variant = NULL; + g_autoptr(GBytes) index = NULL; + + /* We sort on from here so that the index file is reproducible */ + g_ptr_array_sort (froms, (GCompareFunc)g_strcmp0); + + g_variant_dict_init (&deltas_builder, NULL); + + for (int i = 0; i < froms->len; i++) + { + const char *from = g_ptr_array_index (froms, i); + g_autofree char *delta_name = NULL; + GVariant *digest; + + digest = _ostree_repo_static_delta_superblock_digest (repo, from, to, cancellable, error); + if (digest == NULL) + return FALSE; + + if (from != NULL) + delta_name = g_strconcat (from, "-", to, NULL); + else + delta_name = g_strdup (to); + + g_variant_dict_insert_value (&deltas_builder, delta_name, digest); + } + + /* The toplevel of the index is an a{sv} for extensibility, and we use same key name (and format) as when + * storing deltas in the summary. */ + g_variant_dict_init (&index_builder, NULL); + + g_variant_dict_insert_value (&index_builder, OSTREE_SUMMARY_STATIC_DELTAS, g_variant_dict_end (&deltas_builder)); + + index_variant = g_variant_ref_sink (g_variant_dict_end (&index_builder)); + index = g_variant_get_data_as_bytes (index_variant); + + g_autofree char *index_dirname = g_path_get_dirname (index_path); + if (!glnx_shutil_mkdir_p_at (repo->repo_dir_fd, index_dirname, DEFAULT_DIRECTORY_MODE, cancellable, error)) + return FALSE; + + /* delta indexes are generally small and static, so reading it back and comparing is cheap, and it will + lower the write load (and particular sync-load) on the disk during reindexing (i.e. summary updates), */ + if (file_has_content (repo, index_path, index, cancellable)) + continue; + + g_debug ("Updating delta index for %s", to); + if (!glnx_file_replace_contents_at (repo->repo_dir_fd, index_path, + g_bytes_get_data (index, NULL), g_bytes_get_size (index), + 0, cancellable, error)) + return FALSE; + } + } + + return TRUE; +} diff --git a/src/libostree/ostree-repo-static-delta-private.h b/src/libostree/ostree-repo-static-delta-private.h index 5a2e6879..d6c706da 100644 --- a/src/libostree/ostree-repo-static-delta-private.h +++ b/src/libostree/ostree-repo-static-delta-private.h @@ -227,6 +227,11 @@ _ostree_repo_static_delta_delete (OstreeRepo *repo, const char *delta_id, GCancellable *cancellable, GError **error); +gboolean +_ostree_repo_static_delta_reindex (OstreeRepo *repo, + const char *opt_to_commit, + GCancellable *cancellable, + GError **error); /* Used for static deltas which due to a historical mistake are * inconsistent endian. diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index ba3e877f..3a331c90 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -5927,6 +5927,9 @@ ostree_repo_regenerate_summary (OstreeRepo *self, g_variant_ref_sink (summary); } + if (!_ostree_repo_static_delta_reindex (self, NULL, cancellable, error)) + return FALSE; + if (!_ostree_repo_file_replace_contents (self, self->repo_dir_fd, "summary", From 024ef1d756b2580ab99d0f69f451444be59c9ecc Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Tue, 1 Sep 2020 12:26:17 +0200 Subject: [PATCH 15/47] deltas: Add and document no-deltas-in-summary config option By default this is FALSE to keep existing clients working. --- man/ostree.repo-config.xml | 14 ++++++++++ src/libostree/ostree-repo.c | 54 +++++++++++++++++++++---------------- 2 files changed, 45 insertions(+), 23 deletions(-) diff --git a/man/ostree.repo-config.xml b/man/ostree.repo-config.xml index 7a01fc01..e4984430 100644 --- a/man/ostree.repo-config.xml +++ b/man/ostree.repo-config.xml @@ -249,6 +249,20 @@ Boston, MA 02111-1307, USA. costly). + + + no-deltas-in-summary + Boolean value controlling whether OSTree should skip + putting an index of available deltas in the summary file. Defaults to false. + + + Since 2020.7 OSTree can use delta indexes outside the summary file, + making the summary file smaller (especially for larger repositories). However + by default we still create the index in the summary file to make older clients + work. If you know all clients will be 2020.7 later you can enable this to + save network bandwidth. + + diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 3a331c90..9d9146b3 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -5749,6 +5749,8 @@ ostree_repo_regenerate_summary (OstreeRepo *self, * commits from working. */ g_autoptr(OstreeRepoAutoLock) lock = NULL; + gboolean no_deltas_in_summary = FALSE; + lock = _ostree_repo_auto_lock_push (self, OSTREE_REPO_LOCK_EXCLUSIVE, cancellable, error); if (!lock) @@ -5781,35 +5783,41 @@ ostree_repo_regenerate_summary (OstreeRepo *self, } } - { - g_autoptr(GPtrArray) delta_names = NULL; - g_auto(GVariantDict) deltas_builder = OT_VARIANT_BUILDER_INITIALIZER; + if (!ot_keyfile_get_boolean_with_default (self->config, "core", + "no-deltas-in-summary", FALSE, + &no_deltas_in_summary, error)) + return FALSE; - if (!ostree_repo_list_static_delta_names (self, &delta_names, cancellable, error)) - return FALSE; + if (!no_deltas_in_summary) + { + g_autoptr(GPtrArray) delta_names = NULL; + g_auto(GVariantDict) deltas_builder = OT_VARIANT_BUILDER_INITIALIZER; - g_variant_dict_init (&deltas_builder, NULL); - for (guint i = 0; i < delta_names->len; i++) - { - g_autofree char *from = NULL; - g_autofree char *to = NULL; - GVariant *digest; + if (!ostree_repo_list_static_delta_names (self, &delta_names, cancellable, error)) + return FALSE; - if (!_ostree_parse_delta_name (delta_names->pdata[i], &from, &to, error)) - return FALSE; + g_variant_dict_init (&deltas_builder, NULL); + for (guint i = 0; i < delta_names->len; i++) + { + g_autofree char *from = NULL; + g_autofree char *to = NULL; + GVariant *digest; - digest = _ostree_repo_static_delta_superblock_digest (self, - (from && from[0]) ? from : NULL, - to, cancellable, error); - if (digest == NULL) - return FALSE; + if (!_ostree_parse_delta_name (delta_names->pdata[i], &from, &to, error)) + return FALSE; - g_variant_dict_insert_value (&deltas_builder, delta_names->pdata[i], digest); - } + digest = _ostree_repo_static_delta_superblock_digest (self, + (from && from[0]) ? from : NULL, + to, cancellable, error); + if (digest == NULL) + return FALSE; - if (delta_names->len > 0) - g_variant_dict_insert_value (&additional_metadata_builder, OSTREE_SUMMARY_STATIC_DELTAS, g_variant_dict_end (&deltas_builder)); - } + g_variant_dict_insert_value (&deltas_builder, delta_names->pdata[i], digest); + } + + if (delta_names->len > 0) + g_variant_dict_insert_value (&additional_metadata_builder, OSTREE_SUMMARY_STATIC_DELTAS, g_variant_dict_end (&deltas_builder)); + } { g_variant_dict_insert_value (&additional_metadata_builder, OSTREE_SUMMARY_LAST_MODIFIED, From c304703e1d88265f1fd76744b1abaa2ddf415a9e Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Mon, 12 Oct 2020 15:57:53 +0200 Subject: [PATCH 16/47] deltas: Make ostree_repo_static_delta_reindex() public It is useful to be able to trigger this without having to regenerate the summary. For example, if you are not using summaries, or ar generating the summaries yourself. --- apidoc/ostree-sections.txt | 1 + src/libostree/libostree-devel.sym | 1 + src/libostree/ostree-repo-static-delta-core.c | 26 ++++++++++++++++--- src/libostree/ostree-repo.c | 2 +- src/libostree/ostree-repo.h | 17 ++++++++++++ 5 files changed, 42 insertions(+), 5 deletions(-) diff --git a/apidoc/ostree-sections.txt b/apidoc/ostree-sections.txt index ea37823b..81dc8890 100644 --- a/apidoc/ostree-sections.txt +++ b/apidoc/ostree-sections.txt @@ -413,6 +413,7 @@ ostree_repo_list_objects ostree_repo_list_commit_objects_starting_with ostree_repo_list_static_delta_names ostree_repo_list_static_delta_indexes +ostree_repo_static_delta_reindex OstreeStaticDeltaGenerateOpt ostree_repo_static_delta_generate ostree_repo_static_delta_execute_offline_with_signature diff --git a/src/libostree/libostree-devel.sym b/src/libostree/libostree-devel.sym index 1350341c..82d6a9b6 100644 --- a/src/libostree/libostree-devel.sym +++ b/src/libostree/libostree-devel.sym @@ -20,6 +20,7 @@ LIBOSTREE_2020.8 { global: ostree_repo_list_static_delta_indexes; + ostree_repo_static_delta_reindex; } LIBOSTREE_2020.7; /* Stub section for the stable release *after* this development one; don't diff --git a/src/libostree/ostree-repo-static-delta-core.c b/src/libostree/ostree-repo-static-delta-core.c index c58e3683..670a10a5 100644 --- a/src/libostree/ostree-repo-static-delta-core.c +++ b/src/libostree/ostree-repo-static-delta-core.c @@ -1238,11 +1238,29 @@ file_has_content (OstreeRepo *repo, return g_bytes_equal (existing_data, data); } +/** + * ostree_repo_static_delta_reindex: + * @repo: Repo + * @flags: Flags affecting the indexing operation + * @opt_to_commit: ASCII SHA256 checksum of target commit, or %NULL to index all targets + * @cancellable: Cancellable + * @error: Error + * + * The delta index for a particular commit lists all the existing deltas that can be used + * when downloading that commit. This operation regenerates these indexes, either for + * a particular commit (if @opt_to_commit is non-%NULL), or for all commits that + * are reachable by an existing delta (if @opt_to_commit is %NULL). + * + * This is normally called automatically when the summary is updated in ostree_repo_regenerate_summary(). + * + * Locking: shared + */ gboolean -_ostree_repo_static_delta_reindex (OstreeRepo *repo, - const char *opt_to_commit, - GCancellable *cancellable, - GError **error) +ostree_repo_static_delta_reindex (OstreeRepo *repo, + OstreeStaticDeltaIndexFlags flags, + const char *opt_to_commit, + GCancellable *cancellable, + GError **error) { g_autoptr(GPtrArray) all_deltas = NULL; g_autoptr(GHashTable) deltas_to_commit_ht = NULL; /* map: to checksum -> ptrarray of from checksums (or NULL) */ diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 9d9146b3..c22a6666 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -5935,7 +5935,7 @@ ostree_repo_regenerate_summary (OstreeRepo *self, g_variant_ref_sink (summary); } - if (!_ostree_repo_static_delta_reindex (self, NULL, cancellable, error)) + if (!ostree_repo_static_delta_reindex (self, 0, NULL, cancellable, error)) return FALSE; if (!_ostree_repo_file_replace_contents (self, diff --git a/src/libostree/ostree-repo.h b/src/libostree/ostree-repo.h index a3c5dc77..6201e7b3 100644 --- a/src/libostree/ostree-repo.h +++ b/src/libostree/ostree-repo.h @@ -1074,6 +1074,23 @@ gboolean ostree_repo_static_delta_generate (OstreeRepo *self, GCancellable *cancellable, GError **error); +/** + * OstreeStaticDeltaIndexFlags: + * @OSTREE_STATIC_DELTA_INDEX_FLAGS_NONE: No special flags + * + * Flags controlling static delta index generation. + */ +typedef enum { + OSTREE_STATIC_DELTA_INDEX_FLAGS_NONE = 0, +} OstreeStaticDeltaIndexFlags; + +_OSTREE_PUBLIC +gboolean ostree_repo_static_delta_reindex (OstreeRepo *repo, + OstreeStaticDeltaIndexFlags flags, + const char *opt_to_commit, + GCancellable *cancellable, + GError **error); + _OSTREE_PUBLIC gboolean ostree_repo_static_delta_execute_offline_with_signature (OstreeRepo *self, GFile *dir_or_file, From 625606a7eccffd246b359a12215f1b5bad22b702 Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Tue, 1 Sep 2020 12:28:17 +0200 Subject: [PATCH 17/47] deltas: Add CLI ops to list and reindex delta-indexes --- src/ostree/ot-builtin-static-delta.c | 57 ++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/src/ostree/ot-builtin-static-delta.c b/src/ostree/ot-builtin-static-delta.c index 3e0af5bd..ff31b574 100644 --- a/src/ostree/ot-builtin-static-delta.c +++ b/src/ostree/ot-builtin-static-delta.c @@ -53,6 +53,8 @@ BUILTINPROTO(delete); BUILTINPROTO(generate); BUILTINPROTO(apply_offline); BUILTINPROTO(verify); +BUILTINPROTO(indexes); +BUILTINPROTO(reindex); #undef BUILTINPROTO @@ -75,6 +77,12 @@ static OstreeCommand static_delta_subcommands[] = { { "verify", OSTREE_BUILTIN_FLAG_NONE, ot_static_delta_builtin_verify, "Verify static delta signatures" }, + { "indexes", OSTREE_BUILTIN_FLAG_NONE, + ot_static_delta_builtin_indexes, + "List static delta indexes" }, + { "reindex", OSTREE_BUILTIN_FLAG_NONE, + ot_static_delta_builtin_reindex, + "Regenerate static delta indexes" }, { NULL, 0, NULL, NULL } }; @@ -126,6 +134,15 @@ static GOptionEntry verify_options[] = { { NULL } }; +static GOptionEntry indexes_options[] = { + { NULL } +}; + +static GOptionEntry reindex_options[] = { + { "to", 0, 0, G_OPTION_ARG_STRING, &opt_to_rev, "Only update delta index to revision REV", "REV" }, + { NULL } +}; + static void static_delta_usage (char **argv, gboolean is_error) @@ -176,6 +193,46 @@ ot_static_delta_builtin_list (int argc, char **argv, OstreeCommandInvocation *in return TRUE; } +static gboolean +ot_static_delta_builtin_indexes (int argc, char **argv, OstreeCommandInvocation *invocation, GCancellable *cancellable, GError **error) +{ + g_autoptr(OstreeRepo) repo = NULL; + g_autoptr(GOptionContext) context = g_option_context_new (""); + if (!ostree_option_context_parse (context, indexes_options, &argc, &argv, + invocation, &repo, cancellable, error)) + return FALSE; + + g_autoptr(GPtrArray) indexes = NULL; + if (!ostree_repo_list_static_delta_indexes (repo, &indexes, cancellable, error)) + return FALSE; + + if (indexes->len == 0) + g_print ("(No static deltas indexes)\n"); + else + { + for (guint i = 0; i < indexes->len; i++) + g_print ("%s\n", (char*)indexes->pdata[i]); + } + + return TRUE; +} + +static gboolean +ot_static_delta_builtin_reindex (int argc, char **argv, OstreeCommandInvocation *invocation, GCancellable *cancellable, GError **error) +{ + g_autoptr(GOptionContext) context = g_option_context_new (""); + + g_autoptr(OstreeRepo) repo = NULL; + if (!ostree_option_context_parse (context, reindex_options, &argc, &argv, invocation, &repo, cancellable, error)) + return FALSE; + + if (!ostree_repo_static_delta_reindex (repo, 0, opt_to_rev, cancellable, error)) + return FALSE; + + return TRUE; +} + + static gboolean ot_static_delta_builtin_show (int argc, char **argv, OstreeCommandInvocation *invocation, GCancellable *cancellable, GError **error) { From df7f07fc6c68717ba486e3869f508a28a857341b Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Tue, 1 Sep 2020 14:52:49 +0200 Subject: [PATCH 18/47] deltas: Use delta indexes when pulling If there is no delta index in the summary, try to fetch the delta index for the commit we're going to and use that to find the delta (if any). --- src/libostree/ostree-repo-pull-private.h | 4 +- src/libostree/ostree-repo-pull.c | 314 ++++++++++++++++++----- 2 files changed, 252 insertions(+), 66 deletions(-) diff --git a/src/libostree/ostree-repo-pull-private.h b/src/libostree/ostree-repo-pull-private.h index a7ea85cd..d1c5fd14 100644 --- a/src/libostree/ostree-repo-pull-private.h +++ b/src/libostree/ostree-repo-pull-private.h @@ -78,7 +78,8 @@ typedef struct { char *summary_sig_etag; guint64 summary_sig_last_modified; /* seconds since the epoch */ GVariant *summary; - GHashTable *summary_deltas_checksums; + GHashTable *summary_deltas_checksums; /* Filled from summary and delta indexes */ + gboolean summary_has_deltas; /* True if the summary existed and had a delta index */ GHashTable *ref_original_commits; /* Maps checksum to commit, used by timestamp checks */ GHashTable *verified_commits; /* Set of commits that have been verified */ GHashTable *signapi_verified_commits; /* Map of commits that have been signapi verified */ @@ -93,6 +94,7 @@ typedef struct { GHashTable *requested_fallback_content; /* Maps checksum to itself */ GHashTable *pending_fetch_metadata; /* Map */ GHashTable *pending_fetch_content; /* Map */ + GHashTable *pending_fetch_delta_indexes; /* Set */ GHashTable *pending_fetch_delta_superblocks; /* Set */ GHashTable *pending_fetch_deltaparts; /* Set */ guint n_outstanding_metadata_fetches; diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index bb143136..70cbeca2 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -104,6 +104,14 @@ typedef struct { guint n_retries_remaining; } FetchDeltaSuperData; +typedef struct { + OtPullData *pull_data; + char *from_revision; + char *to_revision; + OstreeCollectionRef *requested_ref; /* (nullable) */ + guint n_retries_remaining; +} FetchDeltaIndexData; + static void variant_or_null_unref (gpointer data) { @@ -116,6 +124,8 @@ static void start_fetch_deltapart (OtPullData *pull_data, FetchStaticDeltaData *fetch); static void start_fetch_delta_superblock (OtPullData *pull_data, FetchDeltaSuperData *fetch_data); +static void start_fetch_delta_index (OtPullData *pull_data, + FetchDeltaIndexData *fetch_data); static gboolean fetcher_queue_is_full (OtPullData *pull_data); static void queue_scan_one_metadata_object (OtPullData *pull_data, const char *csum, @@ -135,6 +145,8 @@ static void queue_scan_one_metadata_object_c (OtPullData *pull_da static void enqueue_one_object_request_s (OtPullData *pull_data, FetchObjectData *fetch_data); +static void enqueue_one_static_delta_index_request_s (OtPullData *pull_data, + FetchDeltaIndexData *fetch_data); static void enqueue_one_static_delta_superblock_request_s (OtPullData *pull_data, FetchDeltaSuperData *fetch_data); static void enqueue_one_static_delta_part_request_s (OtPullData *pull_data, @@ -149,6 +161,11 @@ static gboolean scan_one_metadata_object (OtPullData *pull_data, GCancellable *cancellable, GError **error); static void scan_object_queue_data_free (ScanObjectQueueData *scan_data); +static gboolean initiate_delta_request (OtPullData *pull_data, + const OstreeCollectionRef *ref, + const char *to_revision, + const char *delta_from_revision, + GError **error); static gboolean update_progress (gpointer user_data) @@ -287,6 +304,7 @@ check_outstanding_requests_handle_error (OtPullData *pull_data, g_queue_foreach (&pull_data->scan_object_queue, (GFunc) scan_object_queue_data_free, NULL); g_queue_clear (&pull_data->scan_object_queue); g_hash_table_remove_all (pull_data->pending_fetch_metadata); + g_hash_table_remove_all (pull_data->pending_fetch_delta_indexes); g_hash_table_remove_all (pull_data->pending_fetch_delta_superblocks); g_hash_table_remove_all (pull_data->pending_fetch_deltaparts); g_hash_table_remove_all (pull_data->pending_fetch_content); @@ -320,6 +338,16 @@ check_outstanding_requests_handle_error (OtPullData *pull_data, g_variant_unref (objname); } + /* Next, process delta index requests */ + g_hash_table_iter_init (&hiter, pull_data->pending_fetch_delta_indexes); + while (!fetcher_queue_is_full (pull_data) && + g_hash_table_iter_next (&hiter, &key, &value)) + { + FetchDeltaIndexData *fetch = key; + g_hash_table_iter_steal (&hiter); + start_fetch_delta_index (pull_data, g_steal_pointer (&fetch)); + } + /* Next, process delta superblock requests */ g_hash_table_iter_init (&hiter, pull_data->pending_fetch_delta_superblocks); while (!fetcher_queue_is_full (pull_data) && @@ -2469,6 +2497,16 @@ fetch_delta_super_data_free (FetchDeltaSuperData *fetch_data) g_free (fetch_data); } +static void +fetch_delta_index_data_free (FetchDeltaIndexData *fetch_data) +{ + g_free (fetch_data->from_revision); + g_free (fetch_data->to_revision); + if (fetch_data->requested_ref) + ostree_collection_ref_free (fetch_data->requested_ref); + g_free (fetch_data); +} + static void set_required_deltas_error (GError **error, const char *from_revision, @@ -2629,6 +2667,146 @@ validate_variant_is_csum (GVariant *csum, return ostree_validate_structureof_csum_v (csum, error); } +static gboolean +collect_available_deltas_for_pull (OtPullData *pull_data, + GVariant *deltas, + GError **error) +{ + gsize n; + + n = deltas ? g_variant_n_children (deltas) : 0; + for (gsize i = 0; i < n; i++) + { + const char *delta; + g_autoptr(GVariant) csum_v = NULL; + g_autoptr(GVariant) ref = g_variant_get_child_value (deltas, i); + + g_variant_get_child (ref, 0, "&s", &delta); + g_variant_get_child (ref, 1, "v", &csum_v); + + if (!validate_variant_is_csum (csum_v, error)) + return FALSE; + + guchar *csum_data = g_malloc (OSTREE_SHA256_DIGEST_LEN); + memcpy (csum_data, ostree_checksum_bytes_peek (csum_v), 32); + g_hash_table_insert (pull_data->summary_deltas_checksums, + g_strdup (delta), + csum_data); + } + + return TRUE; +} + +static void +on_delta_index_fetched (GObject *src, + GAsyncResult *res, + gpointer data) + +{ + FetchDeltaIndexData *fetch_data = data; + OtPullData *pull_data = fetch_data->pull_data; + g_autoptr(GError) local_error = NULL; + GError **error = &local_error; + g_autoptr(GBytes) delta_index_data = NULL; + const char *from_revision = fetch_data->from_revision; + const char *to_revision = fetch_data->to_revision; + + if (!_ostree_fetcher_request_to_membuf_finish ((OstreeFetcher*)src, + res, + &delta_index_data, + NULL, NULL, NULL, + error)) + { + if (!g_error_matches (local_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND)) + goto out; + g_clear_error (&local_error); + + /* below call to initiate_delta_request() will fail finding the delta and fall back to commit */ + } + else + { + g_autoptr(GVariant) delta_index = g_variant_ref_sink (g_variant_new_from_bytes (G_VARIANT_TYPE_VARDICT, delta_index_data, FALSE)); + g_autoptr(GVariant) deltas = g_variant_lookup_value (delta_index, OSTREE_SUMMARY_STATIC_DELTAS, G_VARIANT_TYPE ("a{sv}")); + + if (!collect_available_deltas_for_pull (pull_data, deltas, error)) + goto out; + } + + if (!initiate_delta_request (pull_data, + fetch_data->requested_ref, + to_revision, + from_revision, + &local_error)) + goto out; + + out: + g_assert (pull_data->n_outstanding_metadata_fetches > 0); + pull_data->n_outstanding_metadata_fetches--; + + if (local_error == NULL) + pull_data->n_fetched_metadata++; + + if (_ostree_fetcher_should_retry_request (local_error, fetch_data->n_retries_remaining--)) + enqueue_one_static_delta_index_request_s (pull_data, g_steal_pointer (&fetch_data)); + else + check_outstanding_requests_handle_error (pull_data, &local_error); + + g_clear_pointer (&fetch_data, fetch_delta_index_data_free); +} + +static void +start_fetch_delta_index (OtPullData *pull_data, + FetchDeltaIndexData *fetch_data) +{ + g_autofree char *delta_name = + _ostree_get_relative_static_delta_index_path (fetch_data->to_revision); + _ostree_fetcher_request_to_membuf (pull_data->fetcher, + pull_data->content_mirrorlist, + delta_name, OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT, + NULL, 0, + OSTREE_MAX_METADATA_SIZE, + 0, pull_data->cancellable, + on_delta_index_fetched, + g_steal_pointer (&fetch_data)); + pull_data->n_outstanding_metadata_fetches++; + pull_data->n_requested_metadata++; +} + +static void +enqueue_one_static_delta_index_request_s (OtPullData *pull_data, + FetchDeltaIndexData *fetch_data) +{ + if (fetcher_queue_is_full (pull_data)) + { + g_debug ("queuing fetch of static delta index to %s", + fetch_data->to_revision); + + g_hash_table_add (pull_data->pending_fetch_delta_indexes, + g_steal_pointer (&fetch_data)); + } + else + { + start_fetch_delta_index (pull_data, g_steal_pointer (&fetch_data)); + } +} + +/* Start a request for a static delta index */ +static void +enqueue_one_static_delta_index_request (OtPullData *pull_data, + const char *to_revision, + const char *from_revision, + const OstreeCollectionRef *ref) +{ + FetchDeltaIndexData *fdata = g_new0(FetchDeltaIndexData, 1); + 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; + fdata->n_retries_remaining = pull_data->n_network_retries; + + enqueue_one_static_delta_index_request_s (pull_data, g_steal_pointer (&fdata)); +} + static gboolean _ostree_repo_verify_summary (OstreeRepo *self, const char *name, @@ -3259,6 +3437,65 @@ reinitialize_fetcher (OtPullData *pull_data, const char *remote_name, return TRUE; } +static gboolean +initiate_delta_request (OtPullData *pull_data, + const OstreeCollectionRef *ref, + const char *to_revision, + const char *delta_from_revision, + GError **error) +{ + DeltaSearchResult deltares; + + /* Look for a delta to @to_revision in the summary data */ + if (!get_best_static_delta_start_for (pull_data, to_revision, &deltares, + pull_data->cancellable, error)) + return FALSE; + + switch (deltares.result) + { + case DELTA_SEARCH_RESULT_NO_MATCH: + { + 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, ref); + } + break; + case DELTA_SEARCH_RESULT_FROM: + enqueue_one_static_delta_superblock_request (pull_data, deltares.from_revision, to_revision, ref); + break; + case DELTA_SEARCH_RESULT_SCRATCH: + { + /* If a from-scratch delta is available, we don’t want to use it if + * the ref already exists locally, since we are likely only a few + * commits out of date; so doing an object pull is likely more + * bandwidth efficient. */ + if (delta_from_revision != NULL) + queue_scan_one_metadata_object (pull_data, to_revision, OSTREE_OBJECT_TYPE_COMMIT, NULL, 0, ref); + else + enqueue_one_static_delta_superblock_request (pull_data, NULL, to_revision, ref); + } + break; + case DELTA_SEARCH_RESULT_UNCHANGED: + { + /* If we already have the commit, here things get a little special; we've historically + * fetched detached metadata, so let's keep doing that. But in the --require-static-deltas + * path, we don't, under the assumption the user wants as little network traffic as + * possible. + */ + if (pull_data->require_static_deltas) + break; + else + queue_scan_one_metadata_object (pull_data, to_revision, OSTREE_OBJECT_TYPE_COMMIT, NULL, 0, ref); + } + } + + return TRUE; +} + /* * initiate_request: * @ref: Optional ref name and collection ID @@ -3301,53 +3538,14 @@ initiate_request (OtPullData *pull_data, /* If we have a summary, we can use the newer logic */ if (pull_data->summary) { - DeltaSearchResult deltares; - - /* Look for a delta to @to_revision in the summary data */ - if (!get_best_static_delta_start_for (pull_data, to_revision, &deltares, - pull_data->cancellable, error)) - return FALSE; - - switch (deltares.result) + if (!pull_data->summary_has_deltas) { - case DELTA_SEARCH_RESULT_NO_MATCH: - { - 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, ref); - } - break; - case DELTA_SEARCH_RESULT_FROM: - enqueue_one_static_delta_superblock_request (pull_data, deltares.from_revision, to_revision, ref); - break; - case DELTA_SEARCH_RESULT_SCRATCH: - { - /* If a from-scratch delta is available, we don’t want to use it if - * the ref already exists locally, since we are likely only a few - * commits out of date; so doing an object pull is likely more - * bandwidth efficient. */ - if (delta_from_revision != NULL) - queue_scan_one_metadata_object (pull_data, to_revision, OSTREE_OBJECT_TYPE_COMMIT, NULL, 0, ref); - else - enqueue_one_static_delta_superblock_request (pull_data, NULL, to_revision, ref); - } - break; - case DELTA_SEARCH_RESULT_UNCHANGED: - { - /* If we already have the commit, here things get a little special; we've historically - * fetched detached metadata, so let's keep doing that. But in the --require-static-deltas - * path, we don't, under the assumption the user wants as little network traffic as - * possible. - */ - if (pull_data->require_static_deltas) - break; - else - queue_scan_one_metadata_object (pull_data, to_revision, OSTREE_OBJECT_TYPE_COMMIT, NULL, 0, ref); - } + enqueue_one_static_delta_index_request (pull_data, to_revision, delta_from_revision, ref); + } + else + { + if (!initiate_delta_request (pull_data, ref, to_revision, delta_from_revision, error)) + return FALSE; } } else if (ref != NULL) @@ -3657,6 +3855,7 @@ ostree_repo_pull_with_options (OstreeRepo *self, pull_data->pending_fetch_metadata = g_hash_table_new_full (ostree_hash_object_name, g_variant_equal, (GDestroyNotify)g_variant_unref, (GDestroyNotify)fetch_object_data_free); + pull_data->pending_fetch_delta_indexes = g_hash_table_new_full (NULL, NULL, (GDestroyNotify) fetch_delta_index_data_free, NULL); pull_data->pending_fetch_delta_superblocks = g_hash_table_new_full (NULL, NULL, (GDestroyNotify) fetch_delta_super_data_free, NULL); pull_data->pending_fetch_deltaparts = g_hash_table_new_full (NULL, NULL, (GDestroyNotify)fetch_static_delta_data_free, NULL); @@ -4314,25 +4513,9 @@ ostree_repo_pull_with_options (OstreeRepo *self, } deltas = g_variant_lookup_value (additional_metadata, OSTREE_SUMMARY_STATIC_DELTAS, G_VARIANT_TYPE ("a{sv}")); - n = deltas ? g_variant_n_children (deltas) : 0; - for (i = 0; i < n; i++) - { - const char *delta; - g_autoptr(GVariant) csum_v = NULL; - g_autoptr(GVariant) ref = g_variant_get_child_value (deltas, i); - - g_variant_get_child (ref, 0, "&s", &delta); - g_variant_get_child (ref, 1, "v", &csum_v); - - if (!validate_variant_is_csum (csum_v, error)) - goto out; - - guchar *csum_data = g_malloc (OSTREE_SHA256_DIGEST_LEN); - memcpy (csum_data, ostree_checksum_bytes_peek (csum_v), 32); - g_hash_table_insert (pull_data->summary_deltas_checksums, - g_strdup (delta), - csum_data); - } + pull_data->summary_has_deltas = deltas != NULL && g_variant_n_children (deltas) > 0; + if (!collect_available_deltas_for_pull (pull_data, deltas, error)) + goto out; } if (pull_data->summary && @@ -4900,6 +5083,7 @@ ostree_repo_pull_with_options (OstreeRepo *self, g_clear_pointer (&pull_data->requested_metadata, (GDestroyNotify) g_hash_table_unref); g_clear_pointer (&pull_data->pending_fetch_content, (GDestroyNotify) g_hash_table_unref); g_clear_pointer (&pull_data->pending_fetch_metadata, (GDestroyNotify) g_hash_table_unref); + g_clear_pointer (&pull_data->pending_fetch_delta_indexes, (GDestroyNotify) g_hash_table_unref); g_clear_pointer (&pull_data->pending_fetch_delta_superblocks, (GDestroyNotify) g_hash_table_unref); g_clear_pointer (&pull_data->pending_fetch_deltaparts, (GDestroyNotify) g_hash_table_unref); g_queue_foreach (&pull_data->scan_object_queue, (GFunc) scan_object_queue_data_free, NULL); From e8a7485458fe744afb168e64a2e1422c578ac38b Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Tue, 1 Sep 2020 16:00:08 +0200 Subject: [PATCH 19/47] deltas: Add tests for delta indexes This tests generation of the index as well as using it when pulling --- tests/test-delta.sh | 48 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/tests/test-delta.sh b/tests/test-delta.sh index 557447bc..bfdec593 100755 --- a/tests/test-delta.sh +++ b/tests/test-delta.sh @@ -28,7 +28,7 @@ skip_without_user_xattrs bindatafiles="bash true ostree" morebindatafiles="false ls" -echo '1..12' +echo '1..13' mkdir repo ostree_repo_init repo --mode=archive @@ -90,6 +90,11 @@ ${CMD_PREFIX} ostree --repo=repo static-delta list | grep ${origrev} || exit 1 ${CMD_PREFIX} ostree --repo=repo prune ${CMD_PREFIX} ostree --repo=repo static-delta list | grep ${origrev} || exit 1 +${CMD_PREFIX} ostree --repo=repo static-delta reindex +${CMD_PREFIX} ostree --repo=repo static-delta indexes | wc -l > indexcount +assert_file_has_content indexcount "^1$" +${CMD_PREFIX} ostree --repo=repo static-delta indexes | grep ${origrev} || exit 1 + permuteDirectory 1 files ${CMD_PREFIX} ostree --repo=repo commit -b test -s test --tree=dir=files @@ -119,6 +124,12 @@ ${CMD_PREFIX} ostree --repo=repo static-delta generate --max-bsdiff-size=10000 - ${CMD_PREFIX} ostree --repo=repo static-delta list | grep ${origrev}-${newrev} || exit 1 +${CMD_PREFIX} ostree --repo=repo static-delta reindex +${CMD_PREFIX} ostree --repo=repo static-delta indexes | wc -l > indexcount +assert_file_has_content indexcount "^2$" +${CMD_PREFIX} ostree --repo=repo static-delta indexes | grep ${origrev} || exit 1 +${CMD_PREFIX} ostree --repo=repo static-delta indexes | grep ${newrev} || exit 1 + if ${CMD_PREFIX} ostree --repo=repo static-delta generate --from=${origrev} --to=${newrev} --empty 2>>err.txt; then assert_not_reached "static-delta generate --from=${origrev} --empty unexpectedly succeeded" fi @@ -249,6 +260,41 @@ ${CMD_PREFIX} ostree --repo=repo2 ls ${samerev} >/dev/null echo 'ok pull empty delta part' +rm -rf repo/delta-indexes +${CMD_PREFIX} ostree --repo=repo summary -u +${CMD_PREFIX} ostree summary -v --raw --repo=repo > summary.txt +assert_file_has_content summary.txt "ostree\.static\-deltas" + +${CMD_PREFIX} ostree --repo=repo config set core.no-deltas-in-summary true +${CMD_PREFIX} ostree --repo=repo summary -u + +${CMD_PREFIX} ostree summary -v --raw --repo=repo > summary.txt +assert_not_file_has_content summary.txt "ostree\.static\-deltas" + +rm -rf repo2 +mkdir repo2 && ostree_repo_init repo2 --mode=bare-user +${CMD_PREFIX} ostree --repo=repo2 pull-local repo ${newrev} + +rm -rf repo/delta-indexes +if ${CMD_PREFIX} ostree --repo=repo2 pull-local --require-static-deltas repo ${samerev} &> no-delta.txt; then + assert_not_reached "failing pull with --require-static-deltas unexpectedly succeeded" +fi +assert_file_has_content no-delta.txt "Static deltas required, but none found for" + +${CMD_PREFIX} ostree --repo=repo static-delta reindex +${CMD_PREFIX} ostree --repo=repo2 pull-local --require-static-deltas repo ${samerev} + +${CMD_PREFIX} ostree --repo=repo2 fsck +${CMD_PREFIX} ostree --repo=repo2 ls ${samerev} >/dev/null + +${CMD_PREFIX} ostree --repo=repo config set core.no-deltas-in-summary false +${CMD_PREFIX} ostree --repo=repo summary -u + +${CMD_PREFIX} ostree summary -v --raw --repo=repo > summary.txt +assert_file_has_content summary.txt "ostree\.static\-deltas" + +echo 'ok pull delta part with delta index' + # Make a new branch to test "rebase deltas" echo otherbranch-content > files/otherbranch-content ${CMD_PREFIX} ostree --repo=repo commit -b otherbranch --tree=dir=files From 0984ff8471cb758f66e8958813670c8ee99e7358 Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Fri, 9 Oct 2020 10:15:42 +0200 Subject: [PATCH 20/47] deltas: Take a shared repo lock while reindexing deltas This ensures we're not racing with a prune operation that can be removing the delta indexes we're relying on. --- src/libostree/ostree-repo-static-delta-core.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/libostree/ostree-repo-static-delta-core.c b/src/libostree/ostree-repo-static-delta-core.c index 670a10a5..5370d152 100644 --- a/src/libostree/ostree-repo-static-delta-core.c +++ b/src/libostree/ostree-repo-static-delta-core.c @@ -1265,6 +1265,12 @@ ostree_repo_static_delta_reindex (OstreeRepo *repo, g_autoptr(GPtrArray) all_deltas = NULL; g_autoptr(GHashTable) deltas_to_commit_ht = NULL; /* map: to checksum -> ptrarray of from checksums (or NULL) */ + /* Protect against parallel prune operation */ + g_autoptr(OstreeRepoAutoLock) lock = + _ostree_repo_auto_lock_push (repo, OSTREE_REPO_LOCK_SHARED, cancellable, error); + if (!lock) + return FALSE; + deltas_to_commit_ht = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, (GDestroyNotify)null_or_ptr_array_unref); if (opt_to_commit == NULL) From 6c8e6539e2487c5d30a18c9b89dd8d6cf4455bb7 Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Fri, 9 Oct 2020 10:55:52 +0200 Subject: [PATCH 21/47] deltas: Set `indexed-deltas` key in the config and summary Clients can use these during pull and avoid downloading the summary if needed, or use the indexed-deltas instead of relying on the ones in the summary which may be left out. --- src/libostree/ostree-repo-private.h | 1 + src/libostree/ostree-repo-static-delta-core.c | 15 +++++++++++++++ src/libostree/ostree-repo.c | 3 +++ 3 files changed, 19 insertions(+) diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h index a48feca4..cbbe6971 100644 --- a/src/libostree/ostree-repo-private.h +++ b/src/libostree/ostree-repo-private.h @@ -57,6 +57,7 @@ G_BEGIN_DECLS #define OSTREE_SUMMARY_COLLECTION_MAP "ostree.summary.collection-map" #define OSTREE_SUMMARY_MODE "ostree.summary.mode" #define OSTREE_SUMMARY_TOMBSTONE_COMMITS "ostree.summary.tombstone-commits" +#define OSTREE_SUMMARY_INDEXED_DELTAS "ostree.summary.indexed-deltas" #define _OSTREE_PAYLOAD_LINK_PREFIX "../" #define _OSTREE_PAYLOAD_LINK_PREFIX_LEN (sizeof (_OSTREE_PAYLOAD_LINK_PREFIX) - 1) diff --git a/src/libostree/ostree-repo-static-delta-core.c b/src/libostree/ostree-repo-static-delta-core.c index 5370d152..c2536724 100644 --- a/src/libostree/ostree-repo-static-delta-core.c +++ b/src/libostree/ostree-repo-static-delta-core.c @@ -1264,6 +1264,7 @@ ostree_repo_static_delta_reindex (OstreeRepo *repo, { g_autoptr(GPtrArray) all_deltas = NULL; g_autoptr(GHashTable) deltas_to_commit_ht = NULL; /* map: to checksum -> ptrarray of from checksums (or NULL) */ + gboolean opt_indexed_deltas; /* Protect against parallel prune operation */ g_autoptr(OstreeRepoAutoLock) lock = @@ -1271,6 +1272,20 @@ ostree_repo_static_delta_reindex (OstreeRepo *repo, if (!lock) return FALSE; + /* Enusre that the "indexed-deltas" option is set on the config, so we know this when pulling */ + if (!ot_keyfile_get_boolean_with_default (repo->config, "core", + "indexed-deltas", FALSE, + &opt_indexed_deltas, error)) + return FALSE; + + if (!opt_indexed_deltas) + { + g_autoptr(GKeyFile) config = ostree_repo_copy_config (repo); + g_key_file_set_boolean (config, "core", "indexed-deltas", TRUE); + if (!ostree_repo_write_config (repo, config, error)) + return FALSE; + } + deltas_to_commit_ht = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, (GDestroyNotify)null_or_ptr_array_unref); if (opt_to_commit == NULL) diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index c22a6666..82f8db44 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -5842,6 +5842,9 @@ ostree_repo_regenerate_summary (OstreeRepo *self, g_variant_new_boolean (tombstone_commits)); } + g_variant_dict_insert_value (&additional_metadata_builder, OSTREE_SUMMARY_INDEXED_DELTAS, + g_variant_new_boolean (TRUE)); + /* Add refs which have a collection specified, which could be in refs/mirrors, * refs/heads, and/or refs/remotes. */ { From 125ed2b199153768f92049e48eb2cd40010f87ca Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Fri, 23 Oct 2020 13:05:25 +0200 Subject: [PATCH 22/47] pull: Only download summary if we need it for the pull operation If we have a commit id for all the refs we're pulling, and if we don't need the summary to list all the refs when mirroring then the only reason to download the summary is for the list of deltas. With the new "indexed-deltas" property in the config file (and mirrored to the summary file) we can detect when we don't need the summary for deltas and completely avoid downloading it then. --- src/libostree/ostree-repo-pull-private.h | 1 + src/libostree/ostree-repo-pull.c | 932 ++++++++++++----------- 2 files changed, 489 insertions(+), 444 deletions(-) diff --git a/src/libostree/ostree-repo-pull-private.h b/src/libostree/ostree-repo-pull-private.h index d1c5fd14..a827557a 100644 --- a/src/libostree/ostree-repo-pull-private.h +++ b/src/libostree/ostree-repo-pull-private.h @@ -80,6 +80,7 @@ typedef struct { GVariant *summary; GHashTable *summary_deltas_checksums; /* Filled from summary and delta indexes */ gboolean summary_has_deltas; /* True if the summary existed and had a delta index */ + gboolean has_indexed_deltas; GHashTable *ref_original_commits; /* Maps checksum to commit, used by timestamp checks */ GHashTable *verified_commits; /* Set of commits that have been verified */ GHashTable *signapi_verified_commits; /* Map of commits that have been signapi verified */ diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 70cbeca2..81956d3d 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -3535,18 +3535,17 @@ initiate_request (OtPullData *pull_data, return FALSE; } - /* If we have a summary, we can use the newer logic */ - if (pull_data->summary) + /* If we have a summary or delta index, we can use the newer logic. + * We prefer the index as it might have more deltas than the summary + * (i.e. leave some deltas out of summary to make it smaller). */ + if (pull_data->has_indexed_deltas) { - if (!pull_data->summary_has_deltas) - { - enqueue_one_static_delta_index_request (pull_data, to_revision, delta_from_revision, ref); - } - else - { - if (!initiate_delta_request (pull_data, ref, to_revision, delta_from_revision, error)) - return FALSE; - } + enqueue_one_static_delta_index_request (pull_data, to_revision, delta_from_revision, ref); + } + else if (pull_data->summary_has_deltas) + { + if (!initiate_delta_request (pull_data, ref, to_revision, delta_from_revision, error)) + return FALSE; } else if (ref != NULL) { @@ -3590,6 +3589,20 @@ initiate_request (OtPullData *pull_data, return TRUE; } +static gboolean +all_requested_refs_have_commit (GHashTable *requested_refs /* (element-type OstreeCollectionRef utf8) */) +{ + GLNX_HASH_TABLE_FOREACH_KV (requested_refs, const OstreeCollectionRef*, ref, + const char*, override_commitid) + { + /* Note: "" override means whatever is latest */ + if (override_commitid == NULL || *override_commitid == 0) + return FALSE; + } + + return TRUE; +} + /* ------------------------------------------------------------------------------------------ * Below is the libsoup-invariant API; these should match * the stub functions in the #else clause @@ -3695,9 +3708,11 @@ ostree_repo_pull_with_options (OstreeRepo *self, gboolean opt_ref_keyring_map_set = FALSE; gboolean disable_sign_verify = FALSE; gboolean disable_sign_verify_summary = FALSE; + gboolean need_summary = FALSE; const char *main_collection_id = NULL; const char *url_override = NULL; gboolean inherit_transaction = FALSE; + gboolean require_summary_for_mirror = FALSE; g_autoptr(GHashTable) updated_requested_refs_to_fetch = NULL; /* (element-type OstreeCollectionRef utf8) */ gsize i; g_autofree char **opt_localcache_repos = NULL; @@ -3709,6 +3724,7 @@ ostree_repo_pull_with_options (OstreeRepo *self, */ const char *the_ref_to_fetch = NULL; OstreeRepoTransactionStats tstats = { 0, }; + gboolean remote_mode_loaded = FALSE; /* Default */ pull_data->max_metadata_size = OSTREE_MAX_METADATA_SIZE; @@ -4132,442 +4148,11 @@ ostree_repo_pull_with_options (OstreeRepo *self, if (pull_data->is_commit_only) pull_data->disable_static_deltas = TRUE; - pull_data->static_delta_superblocks = g_ptr_array_new_with_free_func ((GDestroyNotify)g_variant_unref); - - { - g_autoptr(GBytes) bytes_sig = NULL; - gboolean summary_sig_not_modified = FALSE; - g_autofree char *summary_sig_etag = NULL; - guint64 summary_sig_last_modified = 0; - gsize n; - g_autoptr(GVariant) refs = NULL; - g_autoptr(GVariant) deltas = NULL; - g_autoptr(GVariant) additional_metadata = NULL; - gboolean summary_from_cache = FALSE; - gboolean remote_mode_loaded = FALSE; - gboolean tombstone_commits = FALSE; - - if (summary_sig_bytes_v) - { - /* Must both be specified */ - g_assert (summary_bytes_v); - - bytes_sig = g_variant_get_data_as_bytes (summary_sig_bytes_v); - bytes_summary = g_variant_get_data_as_bytes (summary_bytes_v); - - if (!bytes_sig || !bytes_summary) - { - g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, - "summary-bytes or summary-sig-bytes set to invalid value"); - goto out; - } - - g_debug ("Loaded %s summary from options", remote_name_or_baseurl); - } - - if (!bytes_sig) - { - g_autofree char *summary_sig_if_none_match = NULL; - guint64 summary_sig_if_modified_since = 0; - - /* Load the summary.sig from the network, but send its ETag and - * Last-Modified from the on-disk cache (if it exists) to reduce the - * download size if nothing’s changed. */ - _ostree_repo_load_cache_summary_properties (self, remote_name_or_baseurl, ".sig", - &summary_sig_if_none_match, &summary_sig_if_modified_since); - - g_clear_pointer (&summary_sig_etag, g_free); - summary_sig_last_modified = 0; - if (!_ostree_fetcher_mirrored_request_to_membuf (pull_data->fetcher, - pull_data->meta_mirrorlist, - "summary.sig", OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT, - summary_sig_if_none_match, summary_sig_if_modified_since, - pull_data->n_network_retries, - &bytes_sig, - &summary_sig_not_modified, &summary_sig_etag, &summary_sig_last_modified, - OSTREE_MAX_METADATA_SIZE, - cancellable, error)) - goto out; - - /* The server returned HTTP status 304 Not Modified, so we’re clear to - * load summary.sig from the cache. Also load summary, since - * `_ostree_repo_load_cache_summary_if_same_sig()` would just do that anyway. */ - if (summary_sig_not_modified) - { - g_clear_pointer (&bytes_sig, g_bytes_unref); - g_clear_pointer (&bytes_summary, g_bytes_unref); - if (!_ostree_repo_load_cache_summary_file (self, remote_name_or_baseurl, ".sig", - &bytes_sig, - cancellable, error)) - goto out; - - if (!bytes_summary && - !pull_data->remote_repo_local && - !_ostree_repo_load_cache_summary_file (self, remote_name_or_baseurl, NULL, - &bytes_summary, - cancellable, error)) - goto out; - } - } - - if (bytes_sig && - !bytes_summary && - !pull_data->remote_repo_local && - !_ostree_repo_load_cache_summary_if_same_sig (self, - remote_name_or_baseurl, - bytes_sig, - &bytes_summary, - cancellable, - error)) - goto out; - - if (bytes_summary && !summary_bytes_v) - { - g_debug ("Loaded %s summary from cache", remote_name_or_baseurl); - summary_from_cache = TRUE; - } - - if (!pull_data->summary && !bytes_summary) - { - g_autofree char *summary_if_none_match = NULL; - guint64 summary_if_modified_since = 0; - - _ostree_repo_load_cache_summary_properties (self, remote_name_or_baseurl, NULL, - &summary_if_none_match, &summary_if_modified_since); - - g_clear_pointer (&summary_etag, g_free); - summary_last_modified = 0; - if (!_ostree_fetcher_mirrored_request_to_membuf (pull_data->fetcher, - pull_data->meta_mirrorlist, - "summary", OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT, - summary_if_none_match, summary_if_modified_since, - pull_data->n_network_retries, - &bytes_summary, - &summary_not_modified, &summary_etag, &summary_last_modified, - OSTREE_MAX_METADATA_SIZE, - cancellable, error)) - goto out; - - /* The server returned HTTP status 304 Not Modified, so we’re clear to - * load summary from the cache. */ - if (summary_not_modified) - { - g_clear_pointer (&bytes_summary, g_bytes_unref); - if (!_ostree_repo_load_cache_summary_file (self, remote_name_or_baseurl, NULL, - &bytes_summary, - cancellable, error)) - goto out; - } - } - -#ifndef OSTREE_DISABLE_GPGME - if (!bytes_summary && pull_data->gpg_verify_summary) - { - g_set_error (error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND, - "GPG verification enabled, but no summary found (use gpg-verify-summary=false in remote config to disable)"); - goto out; - } -#endif /* OSTREE_DISABLE_GPGME */ - - if (!bytes_summary && pull_data->require_static_deltas) - { - g_set_error (error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND, - "Fetch configured to require static deltas, but no summary found"); - goto out; - } - -#ifndef OSTREE_DISABLE_GPGME - if (!bytes_sig && pull_data->gpg_verify_summary) - { - g_set_error (error, OSTREE_GPG_ERROR, OSTREE_GPG_ERROR_NO_SIGNATURE, - "GPG verification enabled, but no summary.sig found (use gpg-verify-summary=false in remote config to disable)"); - goto out; - } - - if (pull_data->gpg_verify_summary && bytes_summary && bytes_sig) - { - g_autoptr(OstreeGpgVerifyResult) result = NULL; - g_autoptr(GError) temp_error = NULL; - - result = ostree_repo_verify_summary (self, pull_data->remote_name, - bytes_summary, bytes_sig, - cancellable, &temp_error); - if (!ostree_gpg_verify_result_require_valid_signature (result, &temp_error)) - { - if (summary_from_cache) - { - /* The cached summary doesn't match, fetch a new one and verify again. - * Don’t set the cache headers in the HTTP request, to force a - * full download. */ - if ((self->test_error_flags & OSTREE_REPO_TEST_ERROR_INVALID_CACHE) > 0) - { - g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, - "Remote %s cached summary invalid and " - "OSTREE_REPO_TEST_ERROR_INVALID_CACHE specified", - pull_data->remote_name); - goto out; - } - else - g_debug ("Remote %s cached summary invalid, pulling new version", - pull_data->remote_name); - - summary_from_cache = FALSE; - g_clear_pointer (&bytes_summary, (GDestroyNotify)g_bytes_unref); - g_clear_pointer (&summary_etag, g_free); - summary_last_modified = 0; - if (!_ostree_fetcher_mirrored_request_to_membuf (pull_data->fetcher, - pull_data->meta_mirrorlist, - "summary", - OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT, - NULL, 0, /* no cache headers */ - pull_data->n_network_retries, - &bytes_summary, - &summary_not_modified, &summary_etag, &summary_last_modified, - OSTREE_MAX_METADATA_SIZE, - cancellable, error)) - goto out; - - g_autoptr(OstreeGpgVerifyResult) retry = - ostree_repo_verify_summary (self, pull_data->remote_name, - bytes_summary, bytes_sig, - cancellable, error); - if (!ostree_gpg_verify_result_require_valid_signature (retry, error)) - goto out; - } - else - { - g_propagate_error (error, g_steal_pointer (&temp_error)); - goto out; - } - } - } -#endif /* OSTREE_DISABLE_GPGME */ - - if (pull_data->signapi_summary_verifiers) - { - if (!bytes_sig && pull_data->signapi_summary_verifiers) - { - g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, - "Signatures verification enabled, but no summary.sig found (use sign-verify-summary=false in remote config to disable)"); - goto out; - } - if (bytes_summary && bytes_sig) - { - g_autoptr(GVariant) signatures = NULL; - g_autoptr(GError) temp_error = NULL; - - signatures = g_variant_new_from_bytes (OSTREE_SUMMARY_SIG_GVARIANT_FORMAT, - bytes_sig, FALSE); - - - g_assert (pull_data->signapi_summary_verifiers); - if (!_sign_verify_for_remote (pull_data->signapi_summary_verifiers, bytes_summary, signatures, NULL, &temp_error)) - { - if (summary_from_cache) - { - /* The cached summary doesn't match, fetch a new one and verify again. - * Don’t set the cache headers in the HTTP request, to force a - * full download. */ - if ((self->test_error_flags & OSTREE_REPO_TEST_ERROR_INVALID_CACHE) > 0) - { - g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, - "Remote %s cached summary invalid and " - "OSTREE_REPO_TEST_ERROR_INVALID_CACHE specified", - pull_data->remote_name); - goto out; - } - else - g_debug ("Remote %s cached summary invalid, pulling new version", - pull_data->remote_name); - - summary_from_cache = FALSE; - g_clear_pointer (&bytes_summary, (GDestroyNotify)g_bytes_unref); - g_clear_pointer (&summary_etag, g_free); - summary_last_modified = 0; - if (!_ostree_fetcher_mirrored_request_to_membuf (pull_data->fetcher, - pull_data->meta_mirrorlist, - "summary", - OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT, - NULL, 0, /* no cache headers */ - pull_data->n_network_retries, - &bytes_summary, - &summary_not_modified, &summary_etag, &summary_last_modified, - OSTREE_MAX_METADATA_SIZE, - cancellable, error)) - goto out; - - if (!_sign_verify_for_remote (pull_data->signapi_summary_verifiers, bytes_summary, signatures, NULL, error)) - goto out; - } - else - { - g_propagate_error (error, g_steal_pointer (&temp_error)); - goto out; - } - } - } - } - - if (bytes_summary) - { - pull_data->summary_data = g_bytes_ref (bytes_summary); - pull_data->summary_etag = g_strdup (summary_etag); - pull_data->summary_last_modified = summary_last_modified; - pull_data->summary = g_variant_new_from_bytes (OSTREE_SUMMARY_GVARIANT_FORMAT, bytes_summary, FALSE); - - if (!g_variant_is_normal_form (pull_data->summary)) - { - g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED, - "Not normal form"); - goto out; - } - if (!g_variant_is_of_type (pull_data->summary, OSTREE_SUMMARY_GVARIANT_FORMAT)) - { - g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, - "Doesn't match variant type '%s'", - (char *)OSTREE_SUMMARY_GVARIANT_FORMAT); - goto out; - } - - if (bytes_sig) - { - pull_data->summary_data_sig = g_bytes_ref (bytes_sig); - pull_data->summary_sig_etag = g_strdup (summary_sig_etag); - pull_data->summary_sig_last_modified = summary_sig_last_modified; - } - } - - if (!summary_from_cache && bytes_summary && bytes_sig) - { - if (!pull_data->remote_repo_local && - !_ostree_repo_cache_summary (self, - remote_name_or_baseurl, - bytes_summary, - summary_etag, summary_last_modified, - bytes_sig, - summary_sig_etag, summary_sig_last_modified, - cancellable, - error)) - goto out; - } - - if (pull_data->summary) - { - additional_metadata = g_variant_get_child_value (pull_data->summary, 1); - - if (!g_variant_lookup (additional_metadata, OSTREE_SUMMARY_COLLECTION_ID, "&s", &main_collection_id)) - main_collection_id = NULL; - else if (!ostree_validate_collection_id (main_collection_id, error)) - goto out; - - refs = g_variant_get_child_value (pull_data->summary, 0); - for (i = 0, n = g_variant_n_children (refs); i < n; i++) - { - const char *refname; - g_autoptr(GVariant) ref = g_variant_get_child_value (refs, i); - - g_variant_get_child (ref, 0, "&s", &refname); - - if (!ostree_validate_rev (refname, error)) - goto out; - - if (pull_data->is_mirror && !refs_to_fetch && !opt_collection_refs_set) - { - g_hash_table_insert (requested_refs_to_fetch, - ostree_collection_ref_new (main_collection_id, refname), NULL); - } - } - - g_autoptr(GVariant) collection_map = NULL; - collection_map = g_variant_lookup_value (additional_metadata, OSTREE_SUMMARY_COLLECTION_MAP, G_VARIANT_TYPE ("a{sa(s(taya{sv}))}")); - if (collection_map != NULL) - { - GVariantIter collection_map_iter; - const char *collection_id; - g_autoptr(GVariant) collection_refs = NULL; - - g_variant_iter_init (&collection_map_iter, collection_map); - - while (g_variant_iter_loop (&collection_map_iter, "{&s@a(s(taya{sv}))}", &collection_id, &collection_refs)) - { - if (!ostree_validate_collection_id (collection_id, error)) - goto out; - - for (i = 0, n = g_variant_n_children (collection_refs); i < n; i++) - { - const char *refname; - g_autoptr(GVariant) ref = g_variant_get_child_value (collection_refs, i); - - g_variant_get_child (ref, 0, "&s", &refname); - - if (!ostree_validate_rev (refname, error)) - goto out; - - if (pull_data->is_mirror && !refs_to_fetch && !opt_collection_refs_set) - { - g_hash_table_insert (requested_refs_to_fetch, - ostree_collection_ref_new (collection_id, refname), NULL); - } - } - } - } - - deltas = g_variant_lookup_value (additional_metadata, OSTREE_SUMMARY_STATIC_DELTAS, G_VARIANT_TYPE ("a{sv}")); - pull_data->summary_has_deltas = deltas != NULL && g_variant_n_children (deltas) > 0; - if (!collect_available_deltas_for_pull (pull_data, deltas, error)) - goto out; - } - - if (pull_data->summary && - g_variant_lookup (additional_metadata, OSTREE_SUMMARY_MODE, "s", &remote_mode_str) && - g_variant_lookup (additional_metadata, OSTREE_SUMMARY_TOMBSTONE_COMMITS, "b", &tombstone_commits)) - { - if (!ostree_repo_mode_from_string (remote_mode_str, &pull_data->remote_mode, error)) - goto out; - pull_data->has_tombstone_commits = tombstone_commits; - remote_mode_loaded = TRUE; - } - else if (pull_data->remote_repo_local == NULL) - { - /* Fall-back path which loads the necessary config from the remote’s - * `config` file. Doing so is deprecated since it means an - * additional round trip to the remote for each pull. No need to do - * it for local pulls. */ - if (!load_remote_repo_config (pull_data, &remote_config, cancellable, error)) - goto out; - - if (!ot_keyfile_get_value_with_default (remote_config, "core", "mode", "bare", - &remote_mode_str, error)) - goto out; - - if (!ostree_repo_mode_from_string (remote_mode_str, &pull_data->remote_mode, error)) - goto out; - - if (!ot_keyfile_get_boolean_with_default (remote_config, "core", "tombstone-commits", FALSE, - &pull_data->has_tombstone_commits, error)) - goto out; - - remote_mode_loaded = TRUE; - } - - if (remote_mode_loaded && pull_data->remote_repo_local == NULL && pull_data->remote_mode != OSTREE_REPO_MODE_ARCHIVE) - { - g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, - "Can't pull from archives with mode \"%s\"", - remote_mode_str); - goto out; - } - } + /* Compute the set of collection-refs (and optional commit id) to fetch */ if (pull_data->is_mirror && !refs_to_fetch && !opt_collection_refs_set && !configured_branches) { - if (!bytes_summary) - { - g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, - "Fetching all refs was requested in mirror mode, but remote repository does not have a summary"); - goto out; - } - + require_summary_for_mirror = TRUE; } else if (opt_collection_refs_set) { @@ -4631,6 +4216,465 @@ ostree_repo_pull_with_options (OstreeRepo *self, } } + /* Deltas are necessary when mirroring or resolving a requested ref to a commit. + * We try to avoid loading the potentially large summary if it is not needed. */ + need_summary = require_summary_for_mirror || !all_requested_refs_have_commit (requested_refs_to_fetch) || summary_sig_bytes_v != NULL; + + /* If we don't have indexed deltas, we need the summary for deltas, so check + * the config file for support. + * NOTE: Avoid download if we don't need deltas */ + if (!need_summary && !pull_data->disable_static_deltas) + { + if (!load_remote_repo_config (pull_data, &remote_config, cancellable, error)) + goto out; + + /* Check if remote has delta indexes outside summary */ + if (!ot_keyfile_get_boolean_with_default (remote_config, "core", "indexed-deltas", FALSE, + &pull_data->has_indexed_deltas, error)) + goto out; + + if (!pull_data->has_indexed_deltas) + need_summary = TRUE; + } + + pull_data->static_delta_superblocks = g_ptr_array_new_with_free_func ((GDestroyNotify)g_variant_unref); + + if (need_summary) + { + g_autoptr(GBytes) bytes_sig = NULL; + gboolean summary_sig_not_modified = FALSE; + g_autofree char *summary_sig_etag = NULL; + guint64 summary_sig_last_modified = 0; + gsize n; + g_autoptr(GVariant) refs = NULL; + g_autoptr(GVariant) deltas = NULL; + g_autoptr(GVariant) additional_metadata = NULL; + gboolean summary_from_cache = FALSE; + gboolean tombstone_commits = FALSE; + + if (summary_sig_bytes_v) + { + /* Must both be specified */ + g_assert (summary_bytes_v); + + bytes_sig = g_variant_get_data_as_bytes (summary_sig_bytes_v); + bytes_summary = g_variant_get_data_as_bytes (summary_bytes_v); + + if (!bytes_sig || !bytes_summary) + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, + "summary-bytes or summary-sig-bytes set to invalid value"); + goto out; + } + + g_debug ("Loaded %s summary from options", remote_name_or_baseurl); + } + + if (!bytes_sig) + { + g_autofree char *summary_sig_if_none_match = NULL; + guint64 summary_sig_if_modified_since = 0; + + /* Load the summary.sig from the network, but send its ETag and + * Last-Modified from the on-disk cache (if it exists) to reduce the + * download size if nothing’s changed. */ + _ostree_repo_load_cache_summary_properties (self, remote_name_or_baseurl, ".sig", + &summary_sig_if_none_match, &summary_sig_if_modified_since); + + g_clear_pointer (&summary_sig_etag, g_free); + summary_sig_last_modified = 0; + if (!_ostree_fetcher_mirrored_request_to_membuf (pull_data->fetcher, + pull_data->meta_mirrorlist, + "summary.sig", OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT, + summary_sig_if_none_match, summary_sig_if_modified_since, + pull_data->n_network_retries, + &bytes_sig, + &summary_sig_not_modified, &summary_sig_etag, &summary_sig_last_modified, + OSTREE_MAX_METADATA_SIZE, + cancellable, error)) + goto out; + + /* The server returned HTTP status 304 Not Modified, so we’re clear to + * load summary.sig from the cache. Also load summary, since + * `_ostree_repo_load_cache_summary_if_same_sig()` would just do that anyway. */ + if (summary_sig_not_modified) + { + g_clear_pointer (&bytes_sig, g_bytes_unref); + g_clear_pointer (&bytes_summary, g_bytes_unref); + if (!_ostree_repo_load_cache_summary_file (self, remote_name_or_baseurl, ".sig", + &bytes_sig, + cancellable, error)) + goto out; + + if (!bytes_summary && + !pull_data->remote_repo_local && + !_ostree_repo_load_cache_summary_file (self, remote_name_or_baseurl, NULL, + &bytes_summary, + cancellable, error)) + goto out; + } + } + + if (bytes_sig && + !bytes_summary && + !pull_data->remote_repo_local && + !_ostree_repo_load_cache_summary_if_same_sig (self, + remote_name_or_baseurl, + bytes_sig, + &bytes_summary, + cancellable, + error)) + goto out; + + if (bytes_summary && !summary_bytes_v) + { + g_debug ("Loaded %s summary from cache", remote_name_or_baseurl); + summary_from_cache = TRUE; + } + + if (!pull_data->summary && !bytes_summary) + { + g_autofree char *summary_if_none_match = NULL; + guint64 summary_if_modified_since = 0; + + _ostree_repo_load_cache_summary_properties (self, remote_name_or_baseurl, NULL, + &summary_if_none_match, &summary_if_modified_since); + + g_clear_pointer (&summary_etag, g_free); + summary_last_modified = 0; + + if (!_ostree_fetcher_mirrored_request_to_membuf (pull_data->fetcher, + pull_data->meta_mirrorlist, + "summary", OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT, + summary_if_none_match, summary_if_modified_since, + pull_data->n_network_retries, + &bytes_summary, + &summary_not_modified, &summary_etag, &summary_last_modified, + OSTREE_MAX_METADATA_SIZE, + cancellable, error)) + goto out; + + /* The server returned HTTP status 304 Not Modified, so we’re clear to + * load summary from the cache. */ + if (summary_not_modified) + { + g_clear_pointer (&bytes_summary, g_bytes_unref); + if (!_ostree_repo_load_cache_summary_file (self, remote_name_or_baseurl, NULL, + &bytes_summary, + cancellable, error)) + goto out; + } + } + +#ifndef OSTREE_DISABLE_GPGME + if (!bytes_summary && pull_data->gpg_verify_summary) + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND, + "GPG verification enabled, but no summary found (use gpg-verify-summary=false in remote config to disable)"); + goto out; + } +#endif /* OSTREE_DISABLE_GPGME */ + + if (!bytes_summary && require_summary_for_mirror) + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, + "Fetching all refs was requested in mirror mode, but remote repository does not have a summary"); + goto out; + } + +#ifndef OSTREE_DISABLE_GPGME + if (!bytes_sig && pull_data->gpg_verify_summary) + { + g_set_error (error, OSTREE_GPG_ERROR, OSTREE_GPG_ERROR_NO_SIGNATURE, + "GPG verification enabled, but no summary.sig found (use gpg-verify-summary=false in remote config to disable)"); + goto out; + } + + if (pull_data->gpg_verify_summary && bytes_summary && bytes_sig) + { + g_autoptr(OstreeGpgVerifyResult) result = NULL; + g_autoptr(GError) temp_error = NULL; + + result = ostree_repo_verify_summary (self, pull_data->remote_name, + bytes_summary, bytes_sig, + cancellable, &temp_error); + if (!ostree_gpg_verify_result_require_valid_signature (result, &temp_error)) + { + if (summary_from_cache) + { + /* The cached summary doesn't match, fetch a new one and verify again. + * Don’t set the cache headers in the HTTP request, to force a + * full download. */ + if ((self->test_error_flags & OSTREE_REPO_TEST_ERROR_INVALID_CACHE) > 0) + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, + "Remote %s cached summary invalid and " + "OSTREE_REPO_TEST_ERROR_INVALID_CACHE specified", + pull_data->remote_name); + goto out; + } + else + g_debug ("Remote %s cached summary invalid, pulling new version", + pull_data->remote_name); + + summary_from_cache = FALSE; + g_clear_pointer (&bytes_summary, (GDestroyNotify)g_bytes_unref); + g_clear_pointer (&summary_etag, g_free); + summary_last_modified = 0; + if (!_ostree_fetcher_mirrored_request_to_membuf (pull_data->fetcher, + pull_data->meta_mirrorlist, + "summary", + OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT, + NULL, 0, /* no cache headers */ + pull_data->n_network_retries, + &bytes_summary, + &summary_not_modified, &summary_etag, &summary_last_modified, + OSTREE_MAX_METADATA_SIZE, + cancellable, error)) + goto out; + + g_autoptr(OstreeGpgVerifyResult) retry = + ostree_repo_verify_summary (self, pull_data->remote_name, + bytes_summary, bytes_sig, + cancellable, error); + if (!ostree_gpg_verify_result_require_valid_signature (retry, error)) + goto out; + } + else + { + g_propagate_error (error, g_steal_pointer (&temp_error)); + goto out; + } + } + } +#endif /* OSTREE_DISABLE_GPGME */ + + if (pull_data->signapi_summary_verifiers) + { + if (!bytes_sig && pull_data->signapi_summary_verifiers) + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, + "Signatures verification enabled, but no summary.sig found (use sign-verify-summary=false in remote config to disable)"); + goto out; + } + if (bytes_summary && bytes_sig) + { + g_autoptr(GVariant) signatures = NULL; + g_autoptr(GError) temp_error = NULL; + + signatures = g_variant_new_from_bytes (OSTREE_SUMMARY_SIG_GVARIANT_FORMAT, + bytes_sig, FALSE); + + g_assert (pull_data->signapi_summary_verifiers); + if (!_sign_verify_for_remote (pull_data->signapi_summary_verifiers, bytes_summary, signatures, NULL, &temp_error)) + { + if (summary_from_cache) + { + /* The cached summary doesn't match, fetch a new one and verify again. + * Don’t set the cache headers in the HTTP request, to force a + * full download. */ + if ((self->test_error_flags & OSTREE_REPO_TEST_ERROR_INVALID_CACHE) > 0) + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, + "Remote %s cached summary invalid and " + "OSTREE_REPO_TEST_ERROR_INVALID_CACHE specified", + pull_data->remote_name); + goto out; + } + else + g_debug ("Remote %s cached summary invalid, pulling new version", + pull_data->remote_name); + + summary_from_cache = FALSE; + g_clear_pointer (&bytes_summary, (GDestroyNotify)g_bytes_unref); + g_clear_pointer (&summary_etag, g_free); + summary_last_modified = 0; + if (!_ostree_fetcher_mirrored_request_to_membuf (pull_data->fetcher, + pull_data->meta_mirrorlist, + "summary", + OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT, + NULL, 0, /* no cache headers */ + pull_data->n_network_retries, + &bytes_summary, + &summary_not_modified, &summary_etag, &summary_last_modified, + OSTREE_MAX_METADATA_SIZE, + cancellable, error)) + goto out; + + if (!_sign_verify_for_remote (pull_data->signapi_summary_verifiers, bytes_summary, signatures, NULL, error)) + goto out; + } + else + { + g_propagate_error (error, g_steal_pointer (&temp_error)); + goto out; + } + } + } + } + + if (bytes_summary) + { + pull_data->summary_data = g_bytes_ref (bytes_summary); + pull_data->summary_etag = g_strdup (summary_etag); + pull_data->summary_last_modified = summary_last_modified; + pull_data->summary = g_variant_new_from_bytes (OSTREE_SUMMARY_GVARIANT_FORMAT, bytes_summary, FALSE); + + if (!g_variant_is_normal_form (pull_data->summary)) + { + g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED, + "Not normal form"); + goto out; + } + if (!g_variant_is_of_type (pull_data->summary, OSTREE_SUMMARY_GVARIANT_FORMAT)) + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, + "Doesn't match variant type '%s'", + (char *)OSTREE_SUMMARY_GVARIANT_FORMAT); + goto out; + } + + if (bytes_sig) + { + pull_data->summary_data_sig = g_bytes_ref (bytes_sig); + pull_data->summary_sig_etag = g_strdup (summary_sig_etag); + pull_data->summary_sig_last_modified = summary_sig_last_modified; + } + } + + if (!summary_from_cache && bytes_summary && bytes_sig) + { + if (!pull_data->remote_repo_local && + !_ostree_repo_cache_summary (self, + remote_name_or_baseurl, + bytes_summary, + summary_etag, summary_last_modified, + bytes_sig, + summary_sig_etag, summary_sig_last_modified, + cancellable, + error)) + goto out; + } + + if (pull_data->summary) + { + additional_metadata = g_variant_get_child_value (pull_data->summary, 1); + + if (!g_variant_lookup (additional_metadata, OSTREE_SUMMARY_COLLECTION_ID, "&s", &main_collection_id)) + main_collection_id = NULL; + else if (!ostree_validate_collection_id (main_collection_id, error)) + goto out; + + refs = g_variant_get_child_value (pull_data->summary, 0); + for (i = 0, n = g_variant_n_children (refs); i < n; i++) + { + const char *refname; + g_autoptr(GVariant) ref = g_variant_get_child_value (refs, i); + + g_variant_get_child (ref, 0, "&s", &refname); + + if (!ostree_validate_rev (refname, error)) + goto out; + + if (pull_data->is_mirror && !refs_to_fetch && !opt_collection_refs_set) + { + g_hash_table_insert (requested_refs_to_fetch, + ostree_collection_ref_new (main_collection_id, refname), NULL); + } + } + + g_autoptr(GVariant) collection_map = NULL; + collection_map = g_variant_lookup_value (additional_metadata, OSTREE_SUMMARY_COLLECTION_MAP, G_VARIANT_TYPE ("a{sa(s(taya{sv}))}")); + if (collection_map != NULL) + { + GVariantIter collection_map_iter; + const char *collection_id; + g_autoptr(GVariant) collection_refs = NULL; + + g_variant_iter_init (&collection_map_iter, collection_map); + + while (g_variant_iter_loop (&collection_map_iter, "{&s@a(s(taya{sv}))}", &collection_id, &collection_refs)) + { + if (!ostree_validate_collection_id (collection_id, error)) + goto out; + + for (i = 0, n = g_variant_n_children (collection_refs); i < n; i++) + { + const char *refname; + g_autoptr(GVariant) ref = g_variant_get_child_value (collection_refs, i); + + g_variant_get_child (ref, 0, "&s", &refname); + + if (!ostree_validate_rev (refname, error)) + goto out; + + if (pull_data->is_mirror && !refs_to_fetch && !opt_collection_refs_set) + { + g_hash_table_insert (requested_refs_to_fetch, + ostree_collection_ref_new (collection_id, refname), NULL); + } + } + } + } + + deltas = g_variant_lookup_value (additional_metadata, OSTREE_SUMMARY_STATIC_DELTAS, G_VARIANT_TYPE ("a{sv}")); + pull_data->summary_has_deltas = deltas != NULL && g_variant_n_children (deltas) > 0; + if (!collect_available_deltas_for_pull (pull_data, deltas, error)) + goto out; + + g_variant_lookup (additional_metadata, OSTREE_SUMMARY_INDEXED_DELTAS, "b", &pull_data->has_indexed_deltas); + } + + if (pull_data->summary && + g_variant_lookup (additional_metadata, OSTREE_SUMMARY_MODE, "s", &remote_mode_str) && + g_variant_lookup (additional_metadata, OSTREE_SUMMARY_TOMBSTONE_COMMITS, "b", &tombstone_commits)) + { + if (!ostree_repo_mode_from_string (remote_mode_str, &pull_data->remote_mode, error)) + goto out; + pull_data->has_tombstone_commits = tombstone_commits; + remote_mode_loaded = TRUE; + } + } + + if (pull_data->require_static_deltas && !pull_data->has_indexed_deltas && !pull_data->summary_has_deltas) + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND, + "Fetch configured to require static deltas, but no summary deltas or delta index found"); + goto out; + } + + if (remote_mode_loaded && pull_data->remote_repo_local == NULL) + { + /* Fall-back path which loads the necessary config from the remote’s + * `config` file (unless we already read it above). Doing so is deprecated since it means an + * additional round trip to the remote for each pull. No need to do + * it for local pulls. */ + if (remote_config == NULL && + !load_remote_repo_config (pull_data, &remote_config, cancellable, error)) + goto out; + + if (!ot_keyfile_get_value_with_default (remote_config, "core", "mode", "bare", + &remote_mode_str, error)) + goto out; + + if (!ostree_repo_mode_from_string (remote_mode_str, &pull_data->remote_mode, error)) + goto out; + + if (!ot_keyfile_get_boolean_with_default (remote_config, "core", "tombstone-commits", FALSE, + &pull_data->has_tombstone_commits, error)) + goto out; + + remote_mode_loaded = TRUE; + } + + if (remote_mode_loaded && pull_data->remote_repo_local == NULL && pull_data->remote_mode != OSTREE_REPO_MODE_ARCHIVE) + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, + "Can't pull from archives with mode \"%s\"", + remote_mode_str); + goto out; + } + /* Resolve the checksum for each ref. This has to be done into a new hash table, * since we can’t modify the keys of @requested_refs_to_fetch while iterating * over it, and we need to ensure the collection IDs are resolved too. */ From bc924ff8709845ab1add41367d51a972d89a7488 Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Fri, 9 Oct 2020 16:30:29 +0200 Subject: [PATCH 23/47] tests: Add a testcase to ensure we're not using the summary if we don't need it With deltas outside the summary, if a commit is specified when pulling we don't download the summary. Verify this. --- tests/pull-test.sh | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/tests/pull-test.sh b/tests/pull-test.sh index 082ed315..08921838 100644 --- a/tests/pull-test.sh +++ b/tests/pull-test.sh @@ -55,10 +55,10 @@ function verify_initial_contents() { } if has_gpgme; then - echo "1..35" + echo "1..36" else # 3 tests needs GPG support - echo "1..32" + echo "1..33" fi # Try both syntaxes @@ -381,6 +381,25 @@ assert_file_has_content err.txt "Upgrade.*is chronologically older" ${CMD_PREFIX} ostree --repo=repo pull --timestamp-check-from-rev=${oldrev} origin main@${middlerev} echo "ok pull timestamp checking" +# test pull without override commit use summary, but with doesn't use summary +# We temporarily replace summary with broken one to detect if it is used +mv ostree-srv/gnomerepo/summary ostree-srv/gnomerepo/summary.backup +echo "broken" > ostree-srv/gnomerepo/summary + +repo_init --no-sign-verify +rev=$(ostree --repo=ostree-srv/gnomerepo rev-parse main) +# This will need summary, so will fail +if ${CMD_PREFIX} ostree --repo=repo -v pull origin main; then + assert_not_reached "Should have failed with broken summary" +fi +# This won't need summary so will not fail +${CMD_PREFIX} ostree --repo=repo pull origin main@${rev} + +# Restore summary +mv ostree-srv/gnomerepo/summary.backup ostree-srv/gnomerepo/summary + +echo "ok pull with override id doesn't use summary" + cd ${test_tmpdir} repo_init --no-sign-verify ${CMD_PREFIX} ostree --repo=repo pull origin main From 8cd796f3f1d0e82ea57b30229e692c31f9cb2e03 Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Tue, 20 Oct 2020 08:37:35 +0200 Subject: [PATCH 24/47] Add ostree_repo_gpg_sign_data() This is similar to ostree_sign_data() but for the old gpg code. Flatpak will need this to reproduce a signed summary. --- apidoc/ostree-sections.txt | 1 + src/libostree/libostree-devel.sym | 1 + src/libostree/ostree-repo.c | 61 +++++++++++++++++++++++++++++++ src/libostree/ostree-repo.h | 10 +++++ 4 files changed, 73 insertions(+) diff --git a/apidoc/ostree-sections.txt b/apidoc/ostree-sections.txt index 81dc8890..64bc68d2 100644 --- a/apidoc/ostree-sections.txt +++ b/apidoc/ostree-sections.txt @@ -447,6 +447,7 @@ ostree_repo_pull_default_console_progress_changed ostree_repo_sign_commit ostree_repo_append_gpg_signature ostree_repo_add_gpg_signature_summary +ostree_repo_gpg_sign_data ostree_repo_gpg_verify_data ostree_repo_verify_commit ostree_repo_verify_commit_ext diff --git a/src/libostree/libostree-devel.sym b/src/libostree/libostree-devel.sym index 82d6a9b6..435be190 100644 --- a/src/libostree/libostree-devel.sym +++ b/src/libostree/libostree-devel.sym @@ -21,6 +21,7 @@ LIBOSTREE_2020.8 { global: ostree_repo_list_static_delta_indexes; ostree_repo_static_delta_reindex; + ostree_repo_gpg_sign_data; } LIBOSTREE_2020.7; /* Stub section for the stable release *after* this development one; don't diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 82f8db44..3bbf5ea0 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -5222,6 +5222,67 @@ ostree_repo_add_gpg_signature_summary (OstreeRepo *self, #endif /* OSTREE_DISABLE_GPGME */ } + +/** + * ostree_repo_gpg_sign_data: + * @self: Self + * @data: Data as a #GBytes + * @old_signatures: Existing signatures to append to (or %NULL) + * @key_id: (array zero-terminated=1) (element-type utf8): NULL-terminated array of GPG keys. + * @homedir: (allow-none): GPG home directory, or %NULL + * @out_signature: (out): in case of success will contain signature + * @cancellable: A #GCancellable + * @error: a #GError + * + * Sign the given @data with the specified keys in @key_id. Similar to + * ostree_repo_add_gpg_signature_summary() but can be used on any + * data. + * + * You can use ostree_repo_gpg_verify_data() to verify the signatures. + * + * Returns: @TRUE if @data has been signed successfully, + * @FALSE in case of error (@error will contain the reason). + * + * Since: 2020.8 + */ +gboolean +ostree_repo_gpg_sign_data (OstreeRepo *self, + GBytes *data, + GBytes *old_signatures, + const gchar **key_id, + const gchar *homedir, + GBytes **out_signatures, + GCancellable *cancellable, + GError **error) +{ +#ifndef OSTREE_DISABLE_GPGME + g_autoptr(GVariant) metadata = NULL; + g_autoptr(GVariant) res = NULL; + + if (old_signatures) + metadata = g_variant_ref_sink (g_variant_new_from_bytes (G_VARIANT_TYPE (OSTREE_SUMMARY_SIG_GVARIANT_STRING), old_signatures, FALSE)); + + for (guint i = 0; key_id[i]; i++) + { + g_autoptr(GBytes) signature_data = NULL; + if (!sign_data (self, data, key_id[i], homedir, + &signature_data, + cancellable, error)) + return FALSE; + + g_autoptr(GVariant) old_metadata = g_steal_pointer (&metadata); + metadata = _ostree_detached_metadata_append_gpg_sig (old_metadata, signature_data); + } + + res = g_variant_get_normal_form (metadata); + *out_signatures = g_variant_get_data_as_bytes (res); + return TRUE; +#else + return glnx_throw (error, "GPG feature is disabled in a build time"); +#endif /* OSTREE_DISABLE_GPGME */ +} + + #ifndef OSTREE_DISABLE_GPGME /* Special remote for _ostree_repo_gpg_verify_with_metadata() */ static const char *OSTREE_ALL_REMOTES = "__OSTREE_ALL_REMOTES__"; diff --git a/src/libostree/ostree-repo.h b/src/libostree/ostree-repo.h index 6201e7b3..e64c3230 100644 --- a/src/libostree/ostree-repo.h +++ b/src/libostree/ostree-repo.h @@ -1416,6 +1416,16 @@ gboolean ostree_repo_append_gpg_signature (OstreeRepo *self, GCancellable *cancellable, GError **error); +_OSTREE_PUBLIC +gboolean ostree_repo_gpg_sign_data (OstreeRepo *self, + GBytes *data, + GBytes *old_signatures, + const gchar **key_id, + const gchar *homedir, + GBytes **out_signatures, + GCancellable *cancellable, + GError **error); + _OSTREE_PUBLIC OstreeGpgVerifyResult * ostree_repo_verify_commit_ext (OstreeRepo *self, const gchar *commit_checksum, From 654f3d959a4563ccf7d36182f4ee7a25a70fa016 Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Tue, 20 Oct 2020 15:51:08 +0200 Subject: [PATCH 25/47] ostree pull: Add more g_debug spew around fetching deltas This is useful to debug what is happening when downloading via deltas. --- src/libostree/ostree-repo-pull.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 81956d3d..da421db3 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -2145,6 +2145,7 @@ start_fetch_deltapart (OtPullData *pull_data, FetchStaticDeltaData *fetch) { g_autofree char *deltapart_path = _ostree_get_relative_static_delta_part_path (fetch->from_revision, fetch->to_revision, fetch->i); + g_debug ("starting fetch of deltapart %s", deltapart_path); pull_data->n_outstanding_deltapart_fetches++; g_assert_cmpint (pull_data->n_outstanding_deltapart_fetches, <=, _OSTREE_MAX_OUTSTANDING_DELTAPART_REQUESTS); _ostree_fetcher_request_to_tmpfile (pull_data->fetcher, @@ -2608,6 +2609,7 @@ start_fetch_delta_superblock (OtPullData *pull_data, g_autofree char *delta_name = _ostree_get_relative_static_delta_superblock_path (fetch_data->from_revision, fetch_data->to_revision); + g_debug ("starting fetch of delta superblock %s", delta_name); _ostree_fetcher_request_to_membuf (pull_data->fetcher, pull_data->content_mirrorlist, delta_name, OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT, @@ -2760,6 +2762,7 @@ start_fetch_delta_index (OtPullData *pull_data, { g_autofree char *delta_name = _ostree_get_relative_static_delta_index_path (fetch_data->to_revision); + g_debug ("starting fetch of delta index %s", delta_name); _ostree_fetcher_request_to_membuf (pull_data->fetcher, pull_data->content_mirrorlist, delta_name, OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT, From 5e223f29627ce63772c27b2a6eb44f55aa9cf56a Mon Sep 17 00:00:00 2001 From: William Manley Date: Mon, 6 Jul 2020 14:58:35 +0100 Subject: [PATCH 26/47] ostree_repo_get_bootloader: Document transfer none I think this may affect bindings too. --- src/libostree/ostree-repo.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 3bbf5ea0..d3066e6a 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -6323,7 +6323,7 @@ ostree_repo_get_default_repo_finders (OstreeRepo *self) * Get the bootloader configured. See the documentation for the * "sysroot.bootloader" config key. * - * Returns: bootloader configuration for the sysroot + * Returns: (transfer none): bootloader configuration for the sysroot * Since: 2019.2 */ const gchar * From 062df6ee8111be538321ee624e219f5d61d5e7dc Mon Sep 17 00:00:00 2001 From: William Manley Date: Mon, 6 Jul 2020 14:54:31 +0100 Subject: [PATCH 27/47] Refactor: Centralise choosing the appropriate bootloader In preparation for enhancing `_ostree_sysroot_query_bootloader` --- src/libostree/ostree-sysroot-deploy.c | 28 ++----------- src/libostree/ostree-sysroot.c | 59 +++++++++++++++++++-------- 2 files changed, 44 insertions(+), 43 deletions(-) diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index 1a4a6369..900efe49 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -44,7 +44,6 @@ #include "ostree-repo-private.h" #include "ostree-sysroot-private.h" #include "ostree-sepolicy-private.h" -#include "ostree-bootloader-zipl.h" #include "ostree-deployment-private.h" #include "ostree-core-private.h" #include "ostree-linuxfsutil.h" @@ -2561,7 +2560,6 @@ ostree_sysroot_write_deployments_with_options (OstreeSysroot *self, gboolean bootloader_is_atomic = FALSE; SyncStats syncstats = { 0, }; g_autoptr(OstreeBootloader) bootloader = NULL; - const char *bootloader_config = NULL; if (!requires_new_bootversion) { if (!create_new_bootlinks (self, self->bootversion, @@ -2593,29 +2591,8 @@ ostree_sysroot_write_deployments_with_options (OstreeSysroot *self, return glnx_throw_errno_prefix (error, "Remounting /boot read-write"); } - OstreeRepo *repo = ostree_sysroot_repo (self); - - bootloader_config = ostree_repo_get_bootloader (repo); - - g_debug ("Using bootloader configuration: %s", bootloader_config); - - if (g_str_equal (bootloader_config, "auto")) - { - if (!_ostree_sysroot_query_bootloader (self, &bootloader, cancellable, error)) - return FALSE; - } - else if (g_str_equal (bootloader_config, "none")) - { - /* No bootloader specified; do not query bootloaders to run. */ - } - else if (g_str_equal (bootloader_config, "zipl")) - { - /* Because we do not mark zipl as active by default, lets creating one here, - * which is basically the same what _ostree_sysroot_query_bootloader() does - * for other bootloaders if being activated. - * */ - bootloader = (OstreeBootloader*) _ostree_bootloader_zipl_new (self); - } + if (!_ostree_sysroot_query_bootloader (self, &bootloader, cancellable, error)) + return FALSE; bootloader_is_atomic = bootloader != NULL && _ostree_bootloader_is_atomic (bootloader); @@ -2646,6 +2623,7 @@ ostree_sysroot_write_deployments_with_options (OstreeSysroot *self, (bootloader_is_atomic ? "Transaction complete" : "Bootloader updated"), requires_new_bootversion ? "yes" : "no", new_deployments->len - self->deployments->len); + const gchar *bootloader_config = ostree_repo_get_bootloader (ostree_sysroot_repo (self)); ot_journal_send ("MESSAGE_ID=" SD_ID128_FORMAT_STR, SD_ID128_FORMAT_VAL(OSTREE_DEPLOYMENT_COMPLETE_ID), "MESSAGE=%s", msg, "OSTREE_BOOTLOADER=%s", bootloader ? _ostree_bootloader_get_name (bootloader) : "none", diff --git a/src/libostree/ostree-sysroot.c b/src/libostree/ostree-sysroot.c index e0813b55..f183edd1 100644 --- a/src/libostree/ostree-sysroot.c +++ b/src/libostree/ostree-sysroot.c @@ -36,6 +36,7 @@ #include "ostree-bootloader-uboot.h" #include "ostree-bootloader-syslinux.h" #include "ostree-bootloader-grub2.h" +#include "ostree-bootloader-zipl.h" /** * SECTION:ostree-sysroot @@ -1328,6 +1329,7 @@ ostree_sysroot_repo (OstreeSysroot *self) * ostree_sysroot_query_bootloader: * @sysroot: Sysroot * @out_bootloader: (out) (transfer full) (allow-none): Return location for bootloader, may be %NULL + * @out_bootloader_config: (out) (transfer none) (allow-none): Return location for value of ostree repo config variable sysroot.bootloader, may be %NULL * @cancellable: Cancellable * @error: Error */ @@ -1337,30 +1339,51 @@ _ostree_sysroot_query_bootloader (OstreeSysroot *sysroot, GCancellable *cancellable, GError **error) { - gboolean is_active; - g_autoptr(OstreeBootloader) ret_loader = - (OstreeBootloader*)_ostree_bootloader_syslinux_new (sysroot); - if (!_ostree_bootloader_query (ret_loader, &is_active, - cancellable, error)) - return FALSE; + OstreeRepo *repo = ostree_sysroot_repo (sysroot); + g_autofree gchar *bootloader_config = ostree_repo_get_bootloader (repo); - if (!is_active) + g_debug ("Using bootloader configuration: %s", bootloader_config); + + g_autoptr(OstreeBootloader) ret_loader = NULL; + if (g_str_equal (bootloader_config, "none")) { - g_object_unref (ret_loader); - ret_loader = (OstreeBootloader*)_ostree_bootloader_grub2_new (sysroot); + /* No bootloader specified; do not query bootloaders to run. */ + ret_loader = NULL; + } + else if (g_str_equal (bootloader_config, "zipl")) + { + /* We never consider zipl as active by default, so it can only be created + * if it's explicitly requested in the config */ + ret_loader = (OstreeBootloader*) _ostree_bootloader_zipl_new (sysroot); + } + else if (g_str_equal (bootloader_config, "auto")) + { + gboolean is_active; + ret_loader = (OstreeBootloader*)_ostree_bootloader_syslinux_new (sysroot); if (!_ostree_bootloader_query (ret_loader, &is_active, cancellable, error)) return FALSE; + + if (!is_active) + { + g_object_unref (ret_loader); + ret_loader = (OstreeBootloader*)_ostree_bootloader_grub2_new (sysroot); + if (!_ostree_bootloader_query (ret_loader, &is_active, + cancellable, error)) + return FALSE; + } + if (!is_active) + { + g_object_unref (ret_loader); + ret_loader = (OstreeBootloader*)_ostree_bootloader_uboot_new (sysroot); + if (!_ostree_bootloader_query (ret_loader, &is_active, cancellable, error)) + return FALSE; + } + if (!is_active) + g_clear_object (&ret_loader); } - if (!is_active) - { - g_object_unref (ret_loader); - ret_loader = (OstreeBootloader*)_ostree_bootloader_uboot_new (sysroot); - if (!_ostree_bootloader_query (ret_loader, &is_active, cancellable, error)) - return FALSE; - } - if (!is_active) - g_clear_object (&ret_loader); + else + g_assert_not_reached (); ot_transfer_out_value(out_bootloader, &ret_loader); return TRUE; From 9482ecfe5ad355a405d17272564c8b15c9f1d84b Mon Sep 17 00:00:00 2001 From: William Manley Date: Mon, 6 Jul 2020 15:50:28 +0100 Subject: [PATCH 28/47] Refactor: sysroot.bootloader: Store enum value rather than string It's easier to extend and it centralises the config parsing. In other places we will no longer need to use `g_str_equal` to match these values, a `switch` statement will be sufficient. --- src/libostree/ostree-repo-private.h | 18 +++++++- src/libostree/ostree-repo.c | 41 +++++++++-------- src/libostree/ostree-sysroot.c | 69 +++++++++++++++-------------- 3 files changed, 73 insertions(+), 55 deletions(-) diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h index cbbe6971..14e2f753 100644 --- a/src/libostree/ostree-repo-private.h +++ b/src/libostree/ostree-repo-private.h @@ -110,6 +110,22 @@ typedef enum { _OSTREE_FEATURE_YES, } _OstreeFeatureSupport; +/* Possible values for the sysroot.bootloader configuration variable */ +typedef enum { + CFG_SYSROOT_BOOTLOADER_OPT_AUTO = 0, + CFG_SYSROOT_BOOTLOADER_OPT_NONE, + CFG_SYSROOT_BOOTLOADER_OPT_ZIPL, + /* Non-exhaustive */ +} OstreeCfgSysrootBootloaderOpt; + +static const char* const CFG_SYSROOT_BOOTLOADER_OPTS_STR[] = { + /* This must be kept in the same order as the enum */ + "auto", + "none", + "zipl", + NULL, +}; + /** * OstreeRepo: * @@ -193,7 +209,7 @@ struct OstreeRepo { guint64 payload_link_threshold; gint fs_support_reflink; /* The underlying filesystem has support for ioctl (FICLONE..) */ gchar **repo_finders; - gchar *bootloader; /* Configure which bootloader to use. */ + OstreeCfgSysrootBootloaderOpt bootloader; /* Configure which bootloader to use. */ OstreeRepo *parent_repo; }; diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index d3066e6a..11a209a4 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -1048,7 +1048,6 @@ ostree_repo_finalize (GObject *object) g_mutex_clear (&self->txn_lock); g_free (self->collection_id); g_strfreev (self->repo_finders); - g_free (self->bootloader); g_clear_pointer (&self->remotes, g_hash_table_destroy); g_mutex_clear (&self->remotes_lock); @@ -3186,28 +3185,28 @@ reload_sysroot_config (OstreeRepo *self, GCancellable *cancellable, GError **error) { - { g_autofree char *bootloader = NULL; + g_autofree char *bootloader = NULL; - if (!ot_keyfile_get_value_with_default_group_optional (self->config, "sysroot", - "bootloader", "auto", - &bootloader, error)) - return FALSE; + if (!ot_keyfile_get_value_with_default_group_optional (self->config, "sysroot", + "bootloader", "auto", + &bootloader, error)) + return FALSE; - /* TODO: possibly later add support for specifying a generic bootloader - * binary "x" in /usr/lib/ostree/bootloaders/x). See: - * https://github.com/ostreedev/ostree/issues/1719 - * https://github.com/ostreedev/ostree/issues/1801 - * Also, dedup these strings with the bootloader implementations - */ - if (!(g_str_equal (bootloader, "auto") || g_str_equal (bootloader, "none") - || g_str_equal (bootloader, "zipl"))) - return glnx_throw (error, "Invalid bootloader configuration: '%s'", bootloader); + /* TODO: possibly later add support for specifying a generic bootloader + * binary "x" in /usr/lib/ostree/bootloaders/x). See: + * https://github.com/ostreedev/ostree/issues/1719 + * https://github.com/ostreedev/ostree/issues/1801 + */ + for (int i = 0; CFG_SYSROOT_BOOTLOADER_OPTS_STR[i]; i++) + { + if (g_str_equal (bootloader, CFG_SYSROOT_BOOTLOADER_OPTS_STR[i])) + { + self->bootloader = (OstreeCfgSysrootBootloaderOpt) i; + return TRUE; + } + } - g_free (self->bootloader); - self->bootloader = g_steal_pointer (&bootloader); - } - - return TRUE; + return glnx_throw (error, "Invalid bootloader configuration: '%s'", bootloader); } /** @@ -6331,7 +6330,7 @@ ostree_repo_get_bootloader (OstreeRepo *self) { g_return_val_if_fail (OSTREE_IS_REPO (self), NULL); - return self->bootloader; + return CFG_SYSROOT_BOOTLOADER_OPTS_STR[self->bootloader]; } diff --git a/src/libostree/ostree-sysroot.c b/src/libostree/ostree-sysroot.c index f183edd1..63065fb9 100644 --- a/src/libostree/ostree-sysroot.c +++ b/src/libostree/ostree-sysroot.c @@ -1340,50 +1340,53 @@ _ostree_sysroot_query_bootloader (OstreeSysroot *sysroot, GError **error) { OstreeRepo *repo = ostree_sysroot_repo (sysroot); - g_autofree gchar *bootloader_config = ostree_repo_get_bootloader (repo); + OstreeCfgSysrootBootloaderOpt bootloader_config = repo->bootloader; - g_debug ("Using bootloader configuration: %s", bootloader_config); + g_debug ("Using bootloader configuration: %s", + CFG_SYSROOT_BOOTLOADER_OPTS_STR[bootloader_config]); g_autoptr(OstreeBootloader) ret_loader = NULL; - if (g_str_equal (bootloader_config, "none")) + switch (repo->bootloader) { + case CFG_SYSROOT_BOOTLOADER_OPT_NONE: /* No bootloader specified; do not query bootloaders to run. */ ret_loader = NULL; - } - else if (g_str_equal (bootloader_config, "zipl")) - { + break; + case CFG_SYSROOT_BOOTLOADER_OPT_ZIPL: /* We never consider zipl as active by default, so it can only be created * if it's explicitly requested in the config */ ret_loader = (OstreeBootloader*) _ostree_bootloader_zipl_new (sysroot); - } - else if (g_str_equal (bootloader_config, "auto")) - { - gboolean is_active; - ret_loader = (OstreeBootloader*)_ostree_bootloader_syslinux_new (sysroot); - if (!_ostree_bootloader_query (ret_loader, &is_active, - cancellable, error)) - return FALSE; + break; + case CFG_SYSROOT_BOOTLOADER_OPT_AUTO: + { + gboolean is_active; + ret_loader = (OstreeBootloader*)_ostree_bootloader_syslinux_new (sysroot); + if (!_ostree_bootloader_query (ret_loader, &is_active, + cancellable, error)) + return FALSE; - if (!is_active) - { - g_object_unref (ret_loader); - ret_loader = (OstreeBootloader*)_ostree_bootloader_grub2_new (sysroot); - if (!_ostree_bootloader_query (ret_loader, &is_active, - cancellable, error)) - return FALSE; - } - if (!is_active) - { - g_object_unref (ret_loader); - ret_loader = (OstreeBootloader*)_ostree_bootloader_uboot_new (sysroot); - if (!_ostree_bootloader_query (ret_loader, &is_active, cancellable, error)) - return FALSE; - } - if (!is_active) - g_clear_object (&ret_loader); + if (!is_active) + { + g_object_unref (ret_loader); + ret_loader = (OstreeBootloader*)_ostree_bootloader_grub2_new (sysroot); + if (!_ostree_bootloader_query (ret_loader, &is_active, + cancellable, error)) + return FALSE; + } + if (!is_active) + { + g_object_unref (ret_loader); + ret_loader = (OstreeBootloader*)_ostree_bootloader_uboot_new (sysroot); + if (!_ostree_bootloader_query (ret_loader, &is_active, cancellable, error)) + return FALSE; + } + if (!is_active) + g_clear_object (&ret_loader); + break; + } + default: + g_assert_not_reached (); } - else - g_assert_not_reached (); ot_transfer_out_value(out_bootloader, &ret_loader); return TRUE; From 31acd2ef99acd43c165b3678cfc4a5076a4fc5c0 Mon Sep 17 00:00:00 2001 From: William Manley Date: Mon, 6 Jul 2020 16:12:29 +0100 Subject: [PATCH 29/47] Add support for explicitly requesting any specific bootloader type ...with the `sysroot.bootloader` configuration option. This can be useful when converting a system to use `ostree` which doesn't currently have a bootloader configuration that `ostree` can automatically detect, and is also useful in combination with the `--sysroot` option when provisioning a rootfs for systems other than the one you're running `ostree admin deploy` on. --- man/ostree.repo-config.xml | 9 ++++++++- src/libostree/ostree-repo-private.h | 6 ++++++ src/libostree/ostree-sysroot.c | 9 +++++++++ 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/man/ostree.repo-config.xml b/man/ostree.repo-config.xml index e4984430..6fead168 100644 --- a/man/ostree.repo-config.xml +++ b/man/ostree.repo-config.xml @@ -373,7 +373,9 @@ Boston, MA 02111-1307, USA. bootloader Configure the bootloader that OSTree uses when deploying the sysroot. This may take the values - bootloader=none or bootloader=auto. + bootloader=none, bootloader=auto, + bootloader=grub2, bootloader=syslinux, + bootloader=uboot or bootloader=zipl. Default is auto. @@ -388,6 +390,11 @@ Boston, MA 02111-1307, USA. then OSTree will generate a config for the bootloader found. For example, grub2-mkconfig is run for the grub2 case. + + A specific bootloader type may also be explicitly requested by choosing + grub2, syslinux, uboot or + zipl. + diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h index 14e2f753..628f2b46 100644 --- a/src/libostree/ostree-repo-private.h +++ b/src/libostree/ostree-repo-private.h @@ -114,6 +114,9 @@ typedef enum { typedef enum { CFG_SYSROOT_BOOTLOADER_OPT_AUTO = 0, CFG_SYSROOT_BOOTLOADER_OPT_NONE, + CFG_SYSROOT_BOOTLOADER_OPT_GRUB2, + CFG_SYSROOT_BOOTLOADER_OPT_SYSLINUX, + CFG_SYSROOT_BOOTLOADER_OPT_UBOOT, CFG_SYSROOT_BOOTLOADER_OPT_ZIPL, /* Non-exhaustive */ } OstreeCfgSysrootBootloaderOpt; @@ -122,6 +125,9 @@ static const char* const CFG_SYSROOT_BOOTLOADER_OPTS_STR[] = { /* This must be kept in the same order as the enum */ "auto", "none", + "grub2", + "syslinux", + "uboot", "zipl", NULL, }; diff --git a/src/libostree/ostree-sysroot.c b/src/libostree/ostree-sysroot.c index 63065fb9..265d39d1 100644 --- a/src/libostree/ostree-sysroot.c +++ b/src/libostree/ostree-sysroot.c @@ -1352,6 +1352,15 @@ _ostree_sysroot_query_bootloader (OstreeSysroot *sysroot, /* No bootloader specified; do not query bootloaders to run. */ ret_loader = NULL; break; + case CFG_SYSROOT_BOOTLOADER_OPT_GRUB2: + ret_loader = (OstreeBootloader*) _ostree_bootloader_grub2_new (sysroot); + break; + case CFG_SYSROOT_BOOTLOADER_OPT_SYSLINUX: + ret_loader = (OstreeBootloader*) _ostree_bootloader_syslinux_new (sysroot); + break; + case CFG_SYSROOT_BOOTLOADER_OPT_UBOOT: + ret_loader = (OstreeBootloader*) _ostree_bootloader_uboot_new (sysroot); + break; case CFG_SYSROOT_BOOTLOADER_OPT_ZIPL: /* We never consider zipl as active by default, so it can only be created * if it's explicitly requested in the config */ From a8dce46b5fb7d19e554ecaec994f51f4269de9ea Mon Sep 17 00:00:00 2001 From: William Manley Date: Tue, 14 Jul 2020 12:53:17 +0100 Subject: [PATCH 30/47] Refactor `ostree_sysroot_query_bootloader` This is more regular, so will make it easier to add more bootloader types in the future. --- src/libostree/ostree-sysroot.c | 96 +++++++++++++++++----------------- 1 file changed, 47 insertions(+), 49 deletions(-) diff --git a/src/libostree/ostree-sysroot.c b/src/libostree/ostree-sysroot.c index 265d39d1..a69984b7 100644 --- a/src/libostree/ostree-sysroot.c +++ b/src/libostree/ostree-sysroot.c @@ -1325,6 +1325,31 @@ ostree_sysroot_repo (OstreeSysroot *self) return self->repo; } +static OstreeBootloader* +_ostree_sysroot_new_bootloader_by_type ( + OstreeSysroot *sysroot, + OstreeCfgSysrootBootloaderOpt bl_type) +{ + switch (bl_type) + { + case CFG_SYSROOT_BOOTLOADER_OPT_NONE: + /* No bootloader specified; do not query bootloaders to run. */ + return NULL; + case CFG_SYSROOT_BOOTLOADER_OPT_GRUB2: + return (OstreeBootloader*) _ostree_bootloader_grub2_new (sysroot); + case CFG_SYSROOT_BOOTLOADER_OPT_SYSLINUX: + return (OstreeBootloader*) _ostree_bootloader_syslinux_new (sysroot); + case CFG_SYSROOT_BOOTLOADER_OPT_UBOOT: + return (OstreeBootloader*) _ostree_bootloader_uboot_new (sysroot); + case CFG_SYSROOT_BOOTLOADER_OPT_ZIPL: + /* We never consider zipl as active by default, so it can only be created + * if it's explicitly requested in the config */ + return (OstreeBootloader*) _ostree_bootloader_zipl_new (sysroot); + default: + g_assert_not_reached (); + } +} + /** * ostree_sysroot_query_bootloader: * @sysroot: Sysroot @@ -1346,58 +1371,31 @@ _ostree_sysroot_query_bootloader (OstreeSysroot *sysroot, CFG_SYSROOT_BOOTLOADER_OPTS_STR[bootloader_config]); g_autoptr(OstreeBootloader) ret_loader = NULL; - switch (repo->bootloader) + if (bootloader_config == CFG_SYSROOT_BOOTLOADER_OPT_AUTO) { - case CFG_SYSROOT_BOOTLOADER_OPT_NONE: - /* No bootloader specified; do not query bootloaders to run. */ - ret_loader = NULL; - break; - case CFG_SYSROOT_BOOTLOADER_OPT_GRUB2: - ret_loader = (OstreeBootloader*) _ostree_bootloader_grub2_new (sysroot); - break; - case CFG_SYSROOT_BOOTLOADER_OPT_SYSLINUX: - ret_loader = (OstreeBootloader*) _ostree_bootloader_syslinux_new (sysroot); - break; - case CFG_SYSROOT_BOOTLOADER_OPT_UBOOT: - ret_loader = (OstreeBootloader*) _ostree_bootloader_uboot_new (sysroot); - break; - case CFG_SYSROOT_BOOTLOADER_OPT_ZIPL: - /* We never consider zipl as active by default, so it can only be created - * if it's explicitly requested in the config */ - ret_loader = (OstreeBootloader*) _ostree_bootloader_zipl_new (sysroot); - break; - case CFG_SYSROOT_BOOTLOADER_OPT_AUTO: - { - gboolean is_active; - ret_loader = (OstreeBootloader*)_ostree_bootloader_syslinux_new (sysroot); - if (!_ostree_bootloader_query (ret_loader, &is_active, - cancellable, error)) - return FALSE; - - if (!is_active) - { - g_object_unref (ret_loader); - ret_loader = (OstreeBootloader*)_ostree_bootloader_grub2_new (sysroot); - if (!_ostree_bootloader_query (ret_loader, &is_active, - cancellable, error)) - return FALSE; - } - if (!is_active) - { - g_object_unref (ret_loader); - ret_loader = (OstreeBootloader*)_ostree_bootloader_uboot_new (sysroot); - if (!_ostree_bootloader_query (ret_loader, &is_active, cancellable, error)) - return FALSE; - } - if (!is_active) - g_clear_object (&ret_loader); - break; - } - default: - g_assert_not_reached (); + OstreeCfgSysrootBootloaderOpt probe[] = { + CFG_SYSROOT_BOOTLOADER_OPT_SYSLINUX, + CFG_SYSROOT_BOOTLOADER_OPT_GRUB2, + CFG_SYSROOT_BOOTLOADER_OPT_UBOOT, + }; + for (int i = 0; i < G_N_ELEMENTS (probe); i++) + { + g_autoptr(OstreeBootloader) bl = _ostree_sysroot_new_bootloader_by_type ( + sysroot, probe[i]); + gboolean is_active = FALSE; + if (!_ostree_bootloader_query (bl, &is_active, cancellable, error)) + return FALSE; + if (is_active) + { + ret_loader = g_steal_pointer (&bl); + break; + } + } } + else + ret_loader = _ostree_sysroot_new_bootloader_by_type (sysroot, bootloader_config); - ot_transfer_out_value(out_bootloader, &ret_loader); + ot_transfer_out_value (out_bootloader, &ret_loader) return TRUE; } From 2a6c0b21db072cf8f66d27ca6ca8a01a7c908b17 Mon Sep 17 00:00:00 2001 From: William Manley Date: Tue, 14 Jul 2020 13:32:30 +0100 Subject: [PATCH 31/47] Tests: Refactor bootloader-entries-crosscheck I've made this use functions to make it easier to add support for more bootloaders. Seeing as there will be a big diff anyway I've also adjusted the formatting to make it pep8 compliant. --- tests/bootloader-entries-crosscheck.py | 177 +++++++++++++------------ 1 file changed, 91 insertions(+), 86 deletions(-) diff --git a/tests/bootloader-entries-crosscheck.py b/tests/bootloader-entries-crosscheck.py index 605bd080..b5a02066 100755 --- a/tests/bootloader-entries-crosscheck.py +++ b/tests/bootloader-entries-crosscheck.py @@ -20,113 +20,118 @@ import os import sys -if len(sys.argv) == 1: - sysroot = '' -else: - sysroot = sys.argv[1] -bootloader = sys.argv[2] -loaderpath = sysroot + '/boot/loader/entries' -syslinuxpath = sysroot + '/boot/syslinux/syslinux.cfg' +def main(argv): + _, sysroot, bootloader = argv + + if bootloader == "grub2": + sys.stdout.write('GRUB2 configuration validation not implemented.\n') + return 0 + else: + return validate_syslinux(sysroot) -if bootloader == "grub2": - sys.stdout.write('GRUB2 configuration validation not implemented.\n') - sys.exit(0) def fatal(msg): sys.stderr.write(msg) sys.stderr.write('\n') sys.exit(1) -def entry_get_version(entry): - return int(entry['version']) def get_ostree_option(optionstring): for o in optionstring.split(): if o.startswith('ostree='): return o[8:] - raise ValueError('ostree= not found') - -entries = [] -syslinux_entries = [] + raise ValueError('ostree= not found in %r' % (optionstring,)) -# Parse loader configs -for fname in os.listdir(loaderpath): - path = os.path.join(loaderpath, fname) - with open(path) as f: + +def parse_loader_configs(sysroot): + loaderpath = sysroot + '/boot/loader/entries' + entries = [] + + # Parse loader configs + for fname in os.listdir(loaderpath): + path = os.path.join(loaderpath, fname) entry = {} - for line in f: - line = line.strip() - if (line == '' or line.startswith('#')): - continue - s = line.find(' ') - assert s > 0 - k = line[0:s] - v = line[s+1:] - entry[k] = v + with open(path) as f: + for line in f: + line = line.strip() + if (line == '' or line.startswith('#')): + continue + k, v = line.split(' ', 1) + entry[k] = v entries.append(entry) - entries.sort(key=entry_get_version, reverse=True) + entries.sort(key=lambda e: int(e['version']), reverse=True) + return entries -# Parse SYSLINUX config -with open(syslinuxpath) as f: - in_ostree_config = False - syslinux_entry = None - syslinux_default = None - for line in f: - try: - k, v = line.strip().split(" ", 1) - except ValueError: - continue - if k == 'DEFAULT': - if syslinux_entry is not None: - syslinux_default = v - elif k == 'LABEL': - if syslinux_entry is not None: - syslinux_entries.append(syslinux_entry) - syslinux_entry = {} - syslinux_entry['title'] = v - elif k == 'KERNEL': - syslinux_entry['linux'] = v - elif k == 'INITRD': - syslinux_entry['initrd'] = v - elif k == 'APPEND': - syslinux_entry['options'] = v - if syslinux_entry is not None: - syslinux_entries.append(syslinux_entry) -if len(entries) != len(syslinux_entries): - fatal("Found {0} loader entries, but {1} SYSLINUX entries\n".format(len(entries), len(syslinux_entries))) +def validate_syslinux(sysroot): + syslinuxpath = sysroot + '/boot/syslinux/syslinux.cfg' + + entries = parse_loader_configs(sysroot) + syslinux_entries = [] + + # Parse SYSLINUX config + with open(syslinuxpath) as f: + syslinux_entry = None + for line in f: + try: + k, v = line.strip().split(" ", 1) + except ValueError: + continue + if k == 'DEFAULT': + if syslinux_entry is not None: + syslinux_default = v + elif k == 'LABEL': + if syslinux_entry is not None: + syslinux_entries.append(syslinux_entry) + syslinux_entry = {} + syslinux_entry['title'] = v + elif k == 'KERNEL': + syslinux_entry['linux'] = v + elif k == 'INITRD': + syslinux_entry['initrd'] = v + elif k == 'APPEND': + syslinux_entry['options'] = v + if syslinux_entry is not None: + syslinux_entries.append(syslinux_entry) + + if len(entries) != len(syslinux_entries): + fatal("Found {0} loader entries, but {1} SYSLINUX entries\n".format( + len(entries), len(syslinux_entries))) + + def assert_key_same_file(a, b, key): + aval = a[key] + bval = b[key] + sys.stderr.write("aval: %r\nbval: %r\n" % (aval, bval)) + + # Paths in entries are always relative to /boot + entry = os.stat(sysroot + "/boot" + aval) + + # Syslinux entries can be relative to /boot (if it's on another filesystem) + # or relative to / if /boot is on /. + s1 = os.stat(sysroot + bval) + s2 = os.stat(sysroot + "/boot" + bval) + + # A symlink ensures that no matter what they point at the same file + assert_eq(entry, s1) + assert_eq(entry, s2) + + for i, (entry, syslinuxentry) in enumerate(zip(entries, syslinux_entries)): + assert_key_same_file(entry, syslinuxentry, 'linux') + assert_key_same_file(entry, syslinuxentry, 'initrd') + entry_ostree = get_ostree_option(entry['options']) + syslinux_ostree = get_ostree_option(syslinuxentry['options']) + if entry_ostree != syslinux_ostree: + fatal("Mismatch on ostree option: {0} != {1}".format( + entry_ostree, syslinux_ostree)) + + sys.stdout.write('SYSLINUX configuration validated\n') + return 0 def assert_eq(a, b): assert a == b, "%r == %r" % (a, b) -def assert_key_same_file(a, b, key): - aval = a[key] - bval = b[key] - sys.stderr.write("aval: %r\nbval: %r\n" % (aval, bval)) - - # Paths in entries are always relative to /boot - entry = os.stat(sysroot + "/boot" + aval) - - # Syslinux entries can be relative to /boot (if it's on another filesystem) - # or relative to / if /boot is on /. - s1 = os.stat(sysroot + bval) - s2 = os.stat(sysroot + "/boot" + bval) - - # A symlink ensures that no matter what they point at the same file - assert_eq(entry, s1) - assert_eq(entry, s2) - - -for i,(entry,syslinuxentry) in enumerate(zip(entries, syslinux_entries)): - assert_key_same_file(entry, syslinuxentry, 'linux') - assert_key_same_file(entry, syslinuxentry, 'initrd') - entry_ostree = get_ostree_option(entry['options']) - syslinux_ostree = get_ostree_option(syslinuxentry['options']) - if entry_ostree != syslinux_ostree: - fatal("Mismatch on ostree option: {0} != {1}".format(entry_ostree, syslinux_ostree)) - -sys.stdout.write('SYSLINUX configuration validated\n') -sys.exit(0) +if __name__ == '__main__': + sys.exit(main(sys.argv)) From 631528c87bd0aee100443c31e3f2fa652cb94310 Mon Sep 17 00:00:00 2001 From: William Manley Date: Tue, 27 Oct 2020 12:35:29 +0000 Subject: [PATCH 32/47] fixup! Refactor: Centralise choosing the appropriate bootloader --- src/libostree/ostree-sysroot.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libostree/ostree-sysroot.c b/src/libostree/ostree-sysroot.c index a69984b7..15cbfafb 100644 --- a/src/libostree/ostree-sysroot.c +++ b/src/libostree/ostree-sysroot.c @@ -1354,7 +1354,6 @@ _ostree_sysroot_new_bootloader_by_type ( * ostree_sysroot_query_bootloader: * @sysroot: Sysroot * @out_bootloader: (out) (transfer full) (allow-none): Return location for bootloader, may be %NULL - * @out_bootloader_config: (out) (transfer none) (allow-none): Return location for value of ostree repo config variable sysroot.bootloader, may be %NULL * @cancellable: Cancellable * @error: Error */ From 663c5b41a3b75850fb19a62ad9bba61848d88e9f Mon Sep 17 00:00:00 2001 From: William Manley Date: Tue, 27 Oct 2020 13:24:46 +0000 Subject: [PATCH 33/47] fixup! Refactor `ostree_sysroot_query_bootloader` --- src/libostree/ostree-sysroot.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libostree/ostree-sysroot.c b/src/libostree/ostree-sysroot.c index 15cbfafb..7062a218 100644 --- a/src/libostree/ostree-sysroot.c +++ b/src/libostree/ostree-sysroot.c @@ -1345,6 +1345,9 @@ _ostree_sysroot_new_bootloader_by_type ( /* We never consider zipl as active by default, so it can only be created * if it's explicitly requested in the config */ return (OstreeBootloader*) _ostree_bootloader_zipl_new (sysroot); + case CFG_SYSROOT_BOOTLOADER_OPT_AUTO: + /* "auto" is handled by ostree_sysroot_query_bootloader so we should + * never get here: Fallthrough */ default: g_assert_not_reached (); } From 40edc33ef3dc623c7ef699b4065fd38614f68910 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Tue, 27 Oct 2020 11:57:00 -0400 Subject: [PATCH 34/47] lib/fetcher-curl: Use G_SOURCE_REMOVE instead of FALSE They're equivalent, though I prefer the former because it's more descriptive and it makes it really obvious that it's a `GSource` callback. --- src/libostree/ostree-fetcher-curl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libostree/ostree-fetcher-curl.c b/src/libostree/ostree-fetcher-curl.c index 129e6988..178cb5cd 100644 --- a/src/libostree/ostree-fetcher-curl.c +++ b/src/libostree/ostree-fetcher-curl.c @@ -433,7 +433,7 @@ timer_cb (gpointer data) if (fetcher->timer_event == orig_src) fetcher->timer_event = NULL; - return FALSE; + return G_SOURCE_REMOVE; } /* Update the event timer after curl_multi library calls */ From 8717608c7e2edc998f370bac8912837a9f15c4b3 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Tue, 27 Oct 2020 11:58:14 -0400 Subject: [PATCH 35/47] lib/fetch-curl: Unref timeout source The timeout timer should always be one-shot, so let's just always destroy it in the callback. The main context has its own ref on it, so it won't be freed behind its back. This *should* fix a leak that was brought up in https://bugzilla.redhat.com/show_bug.cgi?id=1891761. Reported-by: Milan Crha --- src/libostree/ostree-fetcher-curl.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/libostree/ostree-fetcher-curl.c b/src/libostree/ostree-fetcher-curl.c index 178cb5cd..d6534b46 100644 --- a/src/libostree/ostree-fetcher-curl.c +++ b/src/libostree/ostree-fetcher-curl.c @@ -426,12 +426,9 @@ static gboolean timer_cb (gpointer data) { OstreeFetcher *fetcher = data; - GSource *orig_src = fetcher->timer_event; - + g_clear_pointer (&fetcher->timer_event, (GDestroyNotify)destroy_and_unref_source); (void)curl_multi_socket_action (fetcher->multi, CURL_SOCKET_TIMEOUT, 0, &fetcher->curl_running); check_multi_info (fetcher); - if (fetcher->timer_event == orig_src) - fetcher->timer_event = NULL; return G_SOURCE_REMOVE; } From 2f78441bea41fa28033f4a9b8a216913384dd623 Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Wed, 28 Oct 2020 15:53:18 +0100 Subject: [PATCH 36/47] ostree_repo_gpg_sign_data: Fix API doc argument name I got: src/libostree/ostree-repo.c:5232: Warning: OSTree: ostree_repo_gpg_sign_data: unknown parameter 'out_signature' in documentation comment, should be 'out_signatures' --- src/libostree/ostree-repo.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 3bbf5ea0..c91930d9 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -5230,7 +5230,7 @@ ostree_repo_add_gpg_signature_summary (OstreeRepo *self, * @old_signatures: Existing signatures to append to (or %NULL) * @key_id: (array zero-terminated=1) (element-type utf8): NULL-terminated array of GPG keys. * @homedir: (allow-none): GPG home directory, or %NULL - * @out_signature: (out): in case of success will contain signature + * @out_signatures: (out): in case of success will contain signature * @cancellable: A #GCancellable * @error: a #GError * From f895cf4fd264b2cf96c70c9f3991b47cf116d22f Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Mon, 2 Nov 2020 14:53:26 -0500 Subject: [PATCH 37/47] Drop use of `volatile` As detailed in gitlab.gnome.org/GNOME/glib/-/issues/600#note_877282, volatile isn't actually needed in these contexts because the atomic operations already give us strong enough guarantees. In GCC 11, this triggers a diagnostic due to the volatile qualifier getting dropped anyway. There is a WIP to do the same in glib: https://gitlab.gnome.org/GNOME/glib/-/merge_requests/1719 This obsoletes this downstream patch: https://src.fedoraproject.org/rpms/ostree/c/b8c5a6fb --- src/libostree/ostree-diff.h | 2 +- src/libostree/ostree-enumtypes.c.template | 8 ++++---- src/libostree/ostree-fetcher-soup.c | 4 ++-- src/libostree/ostree-remote-private.h | 2 +- src/libostree/ostree-repo-private.h | 2 +- src/libostree/ostree-sysroot-upgrader.c | 8 ++++---- 6 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/libostree/ostree-diff.h b/src/libostree/ostree-diff.h index 1c0024a2..b53125ac 100644 --- a/src/libostree/ostree-diff.h +++ b/src/libostree/ostree-diff.h @@ -42,7 +42,7 @@ typedef enum { typedef struct _OstreeDiffItem OstreeDiffItem; struct _OstreeDiffItem { - volatile gint refcount; + gint refcount; /* atomic */ GFile *src; GFile *target; diff --git a/src/libostree/ostree-enumtypes.c.template b/src/libostree/ostree-enumtypes.c.template index 751c458a..827ad911 100644 --- a/src/libostree/ostree-enumtypes.c.template +++ b/src/libostree/ostree-enumtypes.c.template @@ -36,9 +36,9 @@ GType _@enum_name@_get_type (void) { - static volatile gsize the_type__volatile = 0; + static gsize static_the_type = 0; - if (g_once_init_enter (&the_type__volatile)) + if (g_once_init_enter (&static_the_type)) { static const G@Type@Value values[] = { /*** END value-header ***/ @@ -57,10 +57,10 @@ _@enum_name@_get_type (void) g_intern_static_string ("@EnumName@"), values); - g_once_init_leave (&the_type__volatile, the_type); + g_once_init_leave (&static_the_type, the_type); } - return the_type__volatile; + return static_the_type; } /*** END value-tail ***/ diff --git a/src/libostree/ostree-fetcher-soup.c b/src/libostree/ostree-fetcher-soup.c index 52fe5fc3..e1e5002e 100644 --- a/src/libostree/ostree-fetcher-soup.c +++ b/src/libostree/ostree-fetcher-soup.c @@ -49,7 +49,7 @@ typedef enum { } OstreeFetcherState; typedef struct { - volatile int ref_count; + int ref_count; /* atomic */ SoupSession *session; /* not referenced */ GMainContext *main_context; @@ -77,7 +77,7 @@ typedef struct { } ThreadClosure; typedef struct { - volatile int ref_count; + int ref_count; /* atomic */ ThreadClosure *thread_closure; GPtrArray *mirrorlist; /* list of base URIs */ diff --git a/src/libostree/ostree-remote-private.h b/src/libostree/ostree-remote-private.h index 061412de..bf74f613 100644 --- a/src/libostree/ostree-remote-private.h +++ b/src/libostree/ostree-remote-private.h @@ -41,7 +41,7 @@ G_BEGIN_DECLS * remote which this one inherits from, and is what should be used in refspecs * for pulls from this remote. If it’s %NULL, @name should be used instead. */ struct OstreeRemote { - volatile int ref_count; + int ref_count; /* atomic */ char *name; /* (not nullable) */ char *refspec_name; /* (nullable) */ char *group; /* group name in options (not nullable) */ diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h index 628f2b46..714fda8b 100644 --- a/src/libostree/ostree-repo-private.h +++ b/src/libostree/ostree-repo-private.h @@ -72,7 +72,7 @@ typedef enum { } OstreeRepoTestErrorFlags; struct OstreeRepoCommitModifier { - volatile gint refcount; + gint refcount; /* atomic */ OstreeRepoCommitModifierFlags flags; OstreeRepoCommitFilter filter; diff --git a/src/libostree/ostree-sysroot-upgrader.c b/src/libostree/ostree-sysroot-upgrader.c index 46813358..85e67340 100644 --- a/src/libostree/ostree-sysroot-upgrader.c +++ b/src/libostree/ostree-sysroot-upgrader.c @@ -682,9 +682,9 @@ ostree_sysroot_upgrader_deploy (OstreeSysrootUpgrader *self, GType ostree_sysroot_upgrader_flags_get_type (void) { - static volatile gsize g_define_type_id__volatile = 0; + static gsize static_g_define_type_id = 0; - if (g_once_init_enter (&g_define_type_id__volatile)) + if (g_once_init_enter (&static_g_define_type_id)) { static const GFlagsValue values[] = { { OSTREE_SYSROOT_UPGRADER_FLAGS_IGNORE_UNCONFIGURED, "OSTREE_SYSROOT_UPGRADER_FLAGS_IGNORE_UNCONFIGURED", "ignore-unconfigured" }, @@ -692,8 +692,8 @@ ostree_sysroot_upgrader_flags_get_type (void) }; GType g_define_type_id = g_flags_register_static (g_intern_static_string ("OstreeSysrootUpgraderFlags"), values); - g_once_init_leave (&g_define_type_id__volatile, g_define_type_id); + g_once_init_leave (&static_g_define_type_id, g_define_type_id); } - return g_define_type_id__volatile; + return static_g_define_type_id; } From 3e527d94473aeae5fbd64158cec47ebb4de41722 Mon Sep 17 00:00:00 2001 From: Dan Nicholson Date: Mon, 2 Nov 2020 16:42:30 -0700 Subject: [PATCH 38/47] lib/deltas: Annotate from checksum as nullable Without this you can't create a scratch delta from GI. While here, switch the deprecated allow-none annotations to nullable. --- src/libostree/ostree-repo-static-delta-compilation.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libostree/ostree-repo-static-delta-compilation.c b/src/libostree/ostree-repo-static-delta-compilation.c index 753f8aee..893ce2fa 100644 --- a/src/libostree/ostree-repo-static-delta-compilation.c +++ b/src/libostree/ostree-repo-static-delta-compilation.c @@ -1313,10 +1313,10 @@ get_fallback_headers (OstreeRepo *self, * ostree_repo_static_delta_generate: * @self: Repo * @opt: High level optimization choice - * @from: ASCII SHA256 checksum of origin, or %NULL + * @from: (nullable): ASCII SHA256 checksum of origin, or %NULL * @to: ASCII SHA256 checksum of target - * @metadata: (allow-none): Optional metadata - * @params: (allow-none): Parameters, see below + * @metadata: (nullable): Optional metadata + * @params: (nullable): Parameters, see below * @cancellable: Cancellable * @error: Error * From 52463686afcf6ebca446c40dd32b3a7aa1e9ad48 Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Tue, 3 Nov 2020 11:48:33 +0100 Subject: [PATCH 39/47] pull: Don't save into cache passed in GByte summaries The cache shouldn't be affected by the user passing in some other summary as it may not be the "official one". I ran into this in flatpak where the passed summary was correct, but the re-saving of the cache updated the mtime of the cached file which led to later http If-Modified-Since calls failing to update. --- src/libostree/ostree-repo-pull.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index da421db3..758c5054 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -4545,7 +4545,7 @@ ostree_repo_pull_with_options (OstreeRepo *self, } } - if (!summary_from_cache && bytes_summary && bytes_sig) + if (!summary_from_cache && bytes_summary && bytes_sig && summary_sig_bytes_v == NULL) { if (!pull_data->remote_repo_local && !_ostree_repo_cache_summary (self, From bf8c4c7e325bb01cb5779c213f4ca59d91afc37c Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 6 Nov 2020 20:04:54 +0000 Subject: [PATCH 40/47] sysroot: Fix up some GI nullable annotations Hit `ostree_sysroot_repo()` shouldn't be nullable while using the ostree Rust bindings. --- src/libostree/ostree-sysroot.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/libostree/ostree-sysroot.c b/src/libostree/ostree-sysroot.c index 7062a218..95c19816 100644 --- a/src/libostree/ostree-sysroot.c +++ b/src/libostree/ostree-sysroot.c @@ -257,9 +257,9 @@ ostree_sysroot_set_mount_namespace_in_use (OstreeSysroot *self) /** * ostree_sysroot_get_path: - * @self: + * @self: Sysroot * - * Returns: (transfer none): Path to rootfs + * Returns: (transfer none) (not nullable): Path to rootfs */ GFile * ostree_sysroot_get_path (OstreeSysroot *self) @@ -1186,7 +1186,7 @@ ostree_sysroot_get_subbootversion (OstreeSysroot *self) * ostree_sysroot_get_booted_deployment: * @self: Sysroot * - * Returns: (transfer none): The currently booted deployment, or %NULL if none + * Returns: (transfer none) (nullable): The currently booted deployment, or %NULL if none */ OstreeDeployment * ostree_sysroot_get_booted_deployment (OstreeSysroot *self) @@ -1200,7 +1200,7 @@ ostree_sysroot_get_booted_deployment (OstreeSysroot *self) * ostree_sysroot_get_staged_deployment: * @self: Sysroot * - * Returns: (transfer none): The currently staged deployment, or %NULL if none + * Returns: (transfer none) (nullable): The currently staged deployment, or %NULL if none * * Since: 2018.5 */ @@ -1238,7 +1238,7 @@ ostree_sysroot_get_deployments (OstreeSysroot *self) * to access, it, you must either use fd-relative api such as openat(), * or concatenate it with the full ostree_sysroot_get_path(). * - * Returns: (transfer full): Path to deployment root directory, relative to sysroot + * Returns: (transfer full) (not nullable): Path to deployment root directory, relative to sysroot */ char * ostree_sysroot_get_deployment_dirpath (OstreeSysroot *self, @@ -1313,7 +1313,7 @@ ostree_sysroot_get_repo (OstreeSysroot *self, * returns a cached repository. Can only be called after ostree_sysroot_initialize() * or ostree_sysroot_load() has been invoked successfully. * - * Returns: (transfer none): The OSTree repository in sysroot @self. + * Returns: (transfer none) (not nullable): The OSTree repository in sysroot @self. * * Since: 2017.7 */ @@ -1356,7 +1356,7 @@ _ostree_sysroot_new_bootloader_by_type ( /** * ostree_sysroot_query_bootloader: * @sysroot: Sysroot - * @out_bootloader: (out) (transfer full) (allow-none): Return location for bootloader, may be %NULL + * @out_bootloader: (out) (transfer full) (optional) (nullable): Return location for bootloader, may be %NULL * @cancellable: Cancellable * @error: Error */ @@ -1489,7 +1489,7 @@ ostree_sysroot_query_deployments_for (OstreeSysroot *self, * Find the deployment to use as a configuration merge source; this is * the first one in the current deployment list which matches osname. * - * Returns: (transfer full): Configuration merge deployment + * Returns: (transfer full) (nullable): Configuration merge deployment */ OstreeDeployment * ostree_sysroot_get_merge_deployment (OstreeSysroot *self, @@ -1520,7 +1520,7 @@ ostree_sysroot_get_merge_deployment (OstreeSysroot *self, * @self: Sysroot * @refspec: A refspec * - * Returns: (transfer full): A new config file which sets @refspec as an origin + * Returns: (transfer full) (not nullable): A new config file which sets @refspec as an origin */ GKeyFile * ostree_sysroot_origin_new_from_refspec (OstreeSysroot *self, From 1c65498d7797fb9f69118e4b7cb6f0140cdcda86 Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Tue, 10 Nov 2020 13:06:34 +0000 Subject: [PATCH 41/47] ci/travis: move to newer base distro This removes the old pinned distro (Ubuntu Trusty 14.04) from Travis, moving to the newer default distro (Ubuntu Xenial 16.04). --- .travis.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 712a81dd..be741cc9 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,5 +1,4 @@ language: c -dist: trusty sudo: required env: From f7be2a3e4afd208722321852fef5df4ac90269a3 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 9 Nov 2020 19:03:12 +0000 Subject: [PATCH 42/47] bin/checkout: Port some to new style I was reading this code for unrelated reasons and noticed it was still old style; port most (but not all) to new style. --- src/ostree/ot-builtin-checkout.c | 100 ++++++++++--------------------- 1 file changed, 31 insertions(+), 69 deletions(-) diff --git a/src/ostree/ot-builtin-checkout.c b/src/ostree/ot-builtin-checkout.c index fe9558c8..ed9ee1e5 100644 --- a/src/ostree/ot-builtin-checkout.c +++ b/src/ostree/ot-builtin-checkout.c @@ -123,8 +123,6 @@ process_one_checkout (OstreeRepo *repo, GCancellable *cancellable, GError **error) { - gboolean ret = FALSE; - /* This strange code structure is to preserve testing * coverage of both `ostree_repo_checkout_tree` and * `ostree_repo_checkout_at` until such time as we have a more @@ -145,33 +143,15 @@ process_one_checkout (OstreeRepo *repo, checkout_options.mode = OSTREE_REPO_CHECKOUT_MODE_USER; /* Can't union these */ if (opt_union && opt_union_add) - { - g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, - "Cannot specify both --union and --union-add"); - goto out; - } + return glnx_throw (error, "Cannot specify both --union and --union-add"); if (opt_union && opt_union_identical) - { - g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, - "Cannot specify both --union and --union-identical"); - goto out; - } + return glnx_throw (error, "Cannot specify both --union and --union-identical"); if (opt_union_add && opt_union_identical) - { - g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, - "Cannot specify both --union-add and --union-identical "); - goto out; - } + return glnx_throw (error, "Cannot specify both --union-add and --union-identical"); if (opt_require_hardlinks && opt_force_copy) - { - glnx_throw (error, "Cannot specify both --require-hardlinks and --force-copy"); - goto out; - } + return glnx_throw (error, "Cannot specify both --require-hardlinks and --force-copy"); if (opt_selinux_prefix && !opt_selinux_policy) - { - glnx_throw (error, "Cannot specify --selinux-prefix without --selinux-policy"); - goto out; - } + return glnx_throw (error, "Cannot specify --selinux-prefix without --selinux-policy"); else if (opt_union) checkout_options.overwrite_mode = OSTREE_REPO_CHECKOUT_OVERWRITE_UNION_FILES; else if (opt_union_add) @@ -179,11 +159,7 @@ process_one_checkout (OstreeRepo *repo, else if (opt_union_identical) { if (!opt_require_hardlinks) - { - g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, - "--union-identical requires --require-hardlinks"); - goto out; - } + return glnx_throw (error, "--union-identical requires --require-hardlinks"); checkout_options.overwrite_mode = OSTREE_REPO_CHECKOUT_OVERWRITE_UNION_IDENTICAL; } if (opt_whiteouts) @@ -196,13 +172,10 @@ process_one_checkout (OstreeRepo *repo, { glnx_autofd int rootfs_dfd = -1; if (!glnx_opendirat (AT_FDCWD, opt_selinux_policy, TRUE, &rootfs_dfd, error)) - { - g_prefix_error (error, "selinux-policy: "); - goto out; - } + return glnx_prefix_error (error, "selinux-policy: "); policy = ostree_sepolicy_new_at (rootfs_dfd, cancellable, error); if (!policy) - goto out; + return FALSE; checkout_options.sepolicy = policy; checkout_options.sepolicy_prefix = opt_selinux_prefix; } @@ -213,7 +186,7 @@ process_one_checkout (OstreeRepo *repo, { if (!ot_parse_file_by_line (opt_skiplist_file, handle_skiplist_line, skip_list, cancellable, error)) - goto out; + return FALSE; checkout_options.filter = checkout_filter; checkout_options.filter_user_data = skip_list; } @@ -227,25 +200,25 @@ process_one_checkout (OstreeRepo *repo, AT_FDCWD, destination, resolved_commit, cancellable, error)) - goto out; + return FALSE; } else { GError *tmp_error = NULL; g_autoptr(GFile) root = NULL; - g_autoptr(GFile) subtree = NULL; - g_autoptr(GFileInfo) file_info = NULL; g_autoptr(GFile) destination_file = g_file_new_for_path (destination); if (!ostree_repo_read_commit (repo, resolved_commit, &root, NULL, cancellable, error)) - goto out; + return FALSE; + g_autoptr(GFile) subtree = NULL; if (subpath) subtree = g_file_resolve_relative_path (root, subpath); else subtree = g_object_ref (root); - file_info = g_file_query_info (subtree, OSTREE_GIO_FAST_QUERYINFO, + g_autoptr(GFileInfo) file_info + = g_file_query_info (subtree, OSTREE_GIO_FAST_QUERYINFO, G_FILE_QUERY_INFO_NOFOLLOW_SYMLINKS, cancellable, &tmp_error); if (!file_info) @@ -254,13 +227,14 @@ process_one_checkout (OstreeRepo *repo, && g_error_matches (tmp_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND)) { g_clear_error (&tmp_error); - ret = TRUE; + /* Note early return */ + return TRUE; } else { g_propagate_error (error, tmp_error); + return FALSE; } - goto out; } if (!ostree_repo_checkout_tree (repo, opt_user_mode ? OSTREE_REPO_CHECKOUT_MODE_USER : 0, @@ -268,12 +242,10 @@ process_one_checkout (OstreeRepo *repo, destination_file, OSTREE_REPO_FILE (subtree), file_info, cancellable, error)) - goto out; + return FALSE; } - ret = TRUE; - out: - return ret; + return TRUE; } static gboolean @@ -352,56 +324,46 @@ process_many_checkouts (OstreeRepo *repo, gboolean ostree_builtin_checkout (int argc, char **argv, OstreeCommandInvocation *invocation, GCancellable *cancellable, GError **error) { - g_autoptr(GOptionContext) context = NULL; + g_autoptr(GOptionContext) context = g_option_context_new ("COMMIT [DESTINATION]"); g_autoptr(OstreeRepo) repo = NULL; - gboolean ret = FALSE; - const char *commit; - const char *destination; - g_autofree char *resolved_commit = NULL; - - context = g_option_context_new ("COMMIT [DESTINATION]"); - if (!ostree_option_context_parse (context, options, &argc, &argv, invocation, &repo, cancellable, error)) - goto out; + return FALSE; if (opt_disable_fsync) ostree_repo_set_disable_fsync (repo, TRUE); if (argc < 2) { - gchar *help = g_option_context_get_help (context, TRUE, NULL); + g_autofree char *help = g_option_context_get_help (context, TRUE, NULL); g_printerr ("%s\n", help); - g_free (help); - g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED, - "COMMIT must be specified"); - goto out; + return glnx_throw (error, "COMMIT must be specified"); } if (opt_from_stdin || opt_from_file) { - destination = argv[1]; + const char *destination = argv[1]; if (!process_many_checkouts (repo, destination, cancellable, error)) - goto out; + return FALSE; } else { - commit = argv[1]; + const char *commit = argv[1]; + const char *destination; if (argc < 3) destination = commit; else destination = argv[2]; + g_autofree char *resolved_commit = NULL; if (!ostree_repo_resolve_rev (repo, commit, FALSE, &resolved_commit, error)) - goto out; + return FALSE; if (!process_one_checkout (repo, resolved_commit, opt_subpath, destination, cancellable, error)) - goto out; + return FALSE; } - ret = TRUE; - out: - return ret; + return TRUE; } From 43913178a7eb23f615555dc0962844fce1f82570 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 6 Nov 2020 19:38:40 +0000 Subject: [PATCH 43/47] deployment: Add a bunch of docs and fix annotations We were missing docs for these, also add some nullability annotations. Motivated by using these from the Rust bindings. --- src/libostree/ostree-deployment.c | 85 +++++++++++++++++++++++++++++-- 1 file changed, 81 insertions(+), 4 deletions(-) diff --git a/src/libostree/ostree-deployment.c b/src/libostree/ostree-deployment.c index 182bceea..558434de 100644 --- a/src/libostree/ostree-deployment.c +++ b/src/libostree/ostree-deployment.c @@ -27,12 +27,24 @@ typedef GObjectClass OstreeDeploymentClass; G_DEFINE_TYPE (OstreeDeployment, ostree_deployment, G_TYPE_OBJECT) +/* + * ostree_deployment_get_csum: + * @self: Deployment + * + * Returns: (not nullable): The OSTree commit used for this deployment. + */ const char * ostree_deployment_get_csum (OstreeDeployment *self) { return self->csum; } +/* + * ostree_deployment_get_bootcsum: + * @self: Deployment + * + * Returns: (not nullable): The "boot checksum" for content installed in `/boot/ostree`. + */ const char * ostree_deployment_get_bootcsum (OstreeDeployment *self) { @@ -41,9 +53,9 @@ ostree_deployment_get_bootcsum (OstreeDeployment *self) /* * ostree_deployment_get_osname: - * @self: Deployemnt + * @self: Deployment * - * Returns: The "stateroot" name, also known as an "osname" + * Returns: (not nullable): The "stateroot" name, also known as an "osname" */ const char * ostree_deployment_get_osname (OstreeDeployment *self) @@ -51,12 +63,24 @@ ostree_deployment_get_osname (OstreeDeployment *self) return self->osname; } +/* + * ostree_deployment_get_deployserial: + * @self: Deployment + * + * Returns: An integer counter used to ensure multiple deployments of a commit are unique + */ int ostree_deployment_get_deployserial (OstreeDeployment *self) { return self->deployserial; } +/* + * ostree_deployment_get_bootserial: + * @self: Deployment + * + * Returns: An integer counter to index from shared kernels into deployments + */ int ostree_deployment_get_bootserial (OstreeDeployment *self) { @@ -87,24 +111,51 @@ ostree_deployment_get_origin (OstreeDeployment *self) return self->origin; } +/** + * ostree_deployment_get_index: + * @self: Deployment + * + * Returns: The global index into the bootloader ordering + */ int ostree_deployment_get_index (OstreeDeployment *self) { return self->index; } +/** + * ostree_deployment_set_index: + * @self: Deployment + * @index: Index into bootloader ordering + * + * Sets the global index into the bootloader ordering. + */ void ostree_deployment_set_index (OstreeDeployment *self, int index) { self->index = index; } +/** + * ostree_deployment_set_bootserial: + * @self: Deployment + * @index: Don't use this + * + * Should never have been made public API; don't use this. + */ void ostree_deployment_set_bootserial (OstreeDeployment *self, int index) { self->bootserial = index; } +/** + * ostree_deployment_set_bootconfig: + * @self: Deployment + * @bootconfig: (nullable): Bootloader configuration object + * + * Set or clear the bootloader configuration. + */ void ostree_deployment_set_bootconfig (OstreeDeployment *self, OstreeBootconfigParser *bootconfig) { @@ -113,6 +164,14 @@ ostree_deployment_set_bootconfig (OstreeDeployment *self, OstreeBootconfigParser self->bootconfig = g_object_ref (bootconfig); } +/** + * ostree_deployment_set_origin: + * @self: Deployment + * @origin: (nullable): Set the origin for this deployment + * + * Replace the "origin", which is a description of the source + * of the deployment and how to update to the next version. + */ void ostree_deployment_set_origin (OstreeDeployment *self, GKeyFile *origin) { @@ -190,7 +249,7 @@ _ostree_deployment_get_overlay_initrds (OstreeDeployment *self) * ostree_deployment_clone: * @self: Deployment * - * Returns: (transfer full): New deep copy of @self + * Returns: (not nullable) (transfer full): New deep copy of @self */ OstreeDeployment * ostree_deployment_clone (OstreeDeployment *self) @@ -224,6 +283,12 @@ ostree_deployment_clone (OstreeDeployment *self) return ret; } +/** + * ostree_deployment_hash: + * @v: (type OstreeDeployment): Deployment + * + * Returns: An integer suitable for use in a `GHashTable` + */ guint ostree_deployment_hash (gconstpointer v) { @@ -287,6 +352,17 @@ ostree_deployment_class_init (OstreeDeploymentClass *class) object_class->finalize = ostree_deployment_finalize; } +/** + * ostree_deployment_new: + * @index: Global index into the bootloader entries + * @osname: "stateroot" for this deployment + * @csum: OSTree commit that will be deployed + * @deployserial: Unique counter + * @bootcsum: (nullable): Kernel/initrd checksum + * @bootserial: Unique index + * + * Returns: (transfer full) (not nullable): New deployment + */ OstreeDeployment * ostree_deployment_new (int index, const char *osname, @@ -323,7 +399,7 @@ ostree_deployment_new (int index, * access, it, you must either use fd-relative api such as openat(), * or concatenate it with the full ostree_sysroot_get_path(). * - * Returns: (transfer full): Path to deployment root directory, relative to sysroot + * Returns: (not nullable) (transfer full): Path to deployment root directory, relative to sysroot */ char * ostree_deployment_get_origin_relpath (OstreeDeployment *self) @@ -337,6 +413,7 @@ ostree_deployment_get_origin_relpath (OstreeDeployment *self) /** * ostree_deployment_unlocked_state_to_string: * + * Returns: (not nullable): Description of state * Since: 2016.4 */ const char * From 8fbf2c5b807b2e582986492d6abf18eb750b95aa Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 6 Nov 2020 19:24:58 +0000 Subject: [PATCH 44/47] deployment: Ensure query_deployments_for returns nullable values Since that's a common case; hit this while working on rpm-ostree code using the ostree-rs bindings. --- src/libostree/ostree-sysroot.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libostree/ostree-sysroot.c b/src/libostree/ostree-sysroot.c index 95c19816..e3d7e425 100644 --- a/src/libostree/ostree-sysroot.c +++ b/src/libostree/ostree-sysroot.c @@ -1428,8 +1428,8 @@ _ostree_sysroot_join_lines (GPtrArray *lines) * ostree_sysroot_query_deployments_for: * @self: Sysroot * @osname: (allow-none): "stateroot" name - * @out_pending: (out) (allow-none) (transfer full): The pending deployment - * @out_rollback: (out) (allow-none) (transfer full): The rollback deployment + * @out_pending: (out) (nullable) (optional) (transfer full): The pending deployment + * @out_rollback: (out) (nullable) (optional) (transfer full): The rollback deployment * * Find the pending and rollback deployments for @osname. Pass %NULL for @osname * to use the booted deployment's osname. By default, pending deployment is the From 0d0eb4715bf8e57ee60ee14f534bca11ffe6ca02 Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Mon, 16 Nov 2020 11:05:52 +0000 Subject: [PATCH 45/47] ci: run ci-release-build.sh on GitHub This adds a GitHub action in order to run ci-release-build.sh on release PRs (detected via the `kind/release` label). --- .github/workflows/release.yml | 27 +++++++++++++++++++++++++++ ci/ci-release-build.sh | 3 ++- 2 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/release.yml diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml new file mode 100644 index 00000000..7189a976 --- /dev/null +++ b/.github/workflows/release.yml @@ -0,0 +1,27 @@ +--- +name: Release sanity + +on: + pull_request: + branches: [master] + types: [labeled] + +jobs: + ci-release-build: + name: "Sanity check release commits" + if: ${{ github.event.label.name == 'kind/release' }} + runs-on: ubuntu-latest + steps: + - name: Clone repository + uses: actions/checkout@v2 + with: + submodules: 'recursive' + fetch-depth: '0' + - name: Checkout (HEAD) + run: git checkout HEAD + - name: Check release sanity (HEAD) + run: ci/ci-release-build.sh + - name: Checkout (HEAD^) + run: git checkout HEAD^ + - name: Check release sanity (HEAD^) + run: ci/ci-release-build.sh diff --git a/ci/ci-release-build.sh b/ci/ci-release-build.sh index 157eb030..e7e7f111 100755 --- a/ci/ci-release-build.sh +++ b/ci/ci-release-build.sh @@ -1,4 +1,4 @@ -#!/bin/bash +#!/usr/bin/env bash set -euo pipefail # Makes sure that is_release_build is only set to yes in a release commit. A @@ -10,6 +10,7 @@ set -euo pipefail HEAD=${PAPR_COMMIT:-HEAD} git log --format=%B -n 1 $HEAD > log.txt +trap "rm -f log.txt" EXIT if grep -q ^is_release_build=yes configure.ac; then echo "*** is_release_build is set to yes ***" From ee57fe2821b4c0c5aedfa9a47d5ef89d6c0e0350 Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Mon, 16 Nov 2020 17:08:52 +0000 Subject: [PATCH 46/47] workflows/release: pattern-match on PR title This adds an additional condition in order to run sanity check all PRs starting with `Release` (case-insensitive). --- .github/workflows/release.yml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 7189a976..b2cf3f5a 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -1,15 +1,14 @@ --- -name: Release sanity +name: Release on: pull_request: branches: [master] - types: [labeled] jobs: ci-release-build: name: "Sanity check release commits" - if: ${{ github.event.label.name == 'kind/release' }} + if: ${{ github.event.label.name == 'kind/release' || startsWith(github.event.pull_request.title, 'Release') }} runs-on: ubuntu-latest steps: - name: Clone repository From 3e289b19345e30d2da193cd208e109f7d7aaa2a1 Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Tue, 17 Nov 2020 10:14:16 +0000 Subject: [PATCH 47/47] Release 2020.8 --- Makefile-libostree.am | 9 ++++++--- configure.ac | 2 +- src/libostree/libostree-devel.sym | 12 +++++------- src/libostree/libostree-released.sym | 12 +++++++----- tests/test-symbols.sh | 2 +- 5 files changed, 20 insertions(+), 17 deletions(-) diff --git a/Makefile-libostree.am b/Makefile-libostree.am index eeb0b6c6..02dbaded 100644 --- a/Makefile-libostree.am +++ b/Makefile-libostree.am @@ -184,9 +184,12 @@ libostree_1_la_SOURCES += \ endif # USE_GPGME symbol_files = $(top_srcdir)/src/libostree/libostree-released.sym -if BUILDOPT_IS_DEVEL_BUILD -symbol_files += $(top_srcdir)/src/libostree/libostree-devel.sym -endif + +## Uncomment this include when adding new development symbols. +#if BUILDOPT_IS_DEVEL_BUILD +#symbol_files += $(top_srcdir)/src/libostree/libostree-devel.sym +#endif + # http://blog.jgc.org/2007/06/escaping-comma-and-space-in-gnu-make.html wl_versionscript_arg = -Wl,--version-script= EXTRA_DIST += \ diff --git a/configure.ac b/configure.ac index 7a2f7ab6..088acc3d 100644 --- a/configure.ac +++ b/configure.ac @@ -10,7 +10,7 @@ m4_define([year_version], [2020]) m4_define([release_version], [8]) 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 435be190..e2d6efc4 100644 --- a/src/libostree/libostree-devel.sym +++ b/src/libostree/libostree-devel.sym @@ -15,14 +15,12 @@ License along with this library; if not, write to the Free Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. -***/ -LIBOSTREE_2020.8 { -global: - ostree_repo_list_static_delta_indexes; - ostree_repo_static_delta_reindex; - ostree_repo_gpg_sign_data; -} LIBOSTREE_2020.7; + How to introduce the first new symbol for a development version: + - copy the stub content below to a new entry + - replace templated value as noted + - uncomment the include in Makefile-libostree.am +*/ /* Stub section for the stable release *after* this development one; don't * edit this other than to update the year. This is just a copy/paste diff --git a/src/libostree/libostree-released.sym b/src/libostree/libostree-released.sym index c154d8c1..4f80d7fc 100644 --- a/src/libostree/libostree-released.sym +++ b/src/libostree/libostree-released.sym @@ -596,7 +596,6 @@ global: /* No new symbols in 2020.2 */ /* No new symbols in 2020.3 */ -/* Add new symbols here. Release commits should copy this section into -released.sym. */ LIBOSTREE_2020.4 { global: ostree_repo_commit_modifier_set_sepolicy_from_commit; @@ -619,14 +618,10 @@ global: } LIBOSTREE_2020.1; /* No new symbols in 2020.5 */ - /* No new symbols in 2020.6 */ LIBOSTREE_2020.7 { global: - /* Add symbols here, and uncomment the bits in - * Makefile-libostree.am to enable this too. - */ ostree_repo_static_delta_execute_offline_with_signature; ostree_repo_static_delta_verify_signature; ostree_bootconfig_parser_get_overlay_initrds; @@ -636,6 +631,13 @@ global: ostree_sysroot_stage_overlay_initrd; } LIBOSTREE_2020.4; +LIBOSTREE_2020.8 { +global: + ostree_repo_list_static_delta_indexes; + ostree_repo_static_delta_reindex; + ostree_repo_gpg_sign_data; +} LIBOSTREE_2020.7; + /* 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 e4a0a943..3072c212 100755 --- a/tests/test-symbols.sh +++ b/tests/test-symbols.sh @@ -66,7 +66,7 @@ echo 'ok documented symbols' # ONLY update this checksum in release commits! cat > released-sha256.txt <