From 664c41329982ae5165b2a360530da3d61ba89aae Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 6 Jun 2022 11:06:44 -0400 Subject: [PATCH 01/56] configure: post-release version bump --- configure.ac | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index 615580a8..445c53e0 100644 --- a/configure.ac +++ b/configure.ac @@ -1,10 +1,10 @@ AC_PREREQ([2.63]) dnl To perform a release, follow the instructions in `docs/CONTRIBUTING.md`. m4_define([year_version], [2022]) -m4_define([release_version], [4]) +m4_define([release_version], [5]) 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]) From c2baa6d10b18178643bb3b40447343a22165752a Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 7 Jun 2022 20:30:09 -0400 Subject: [PATCH 02/56] repo: Optimize memory use of `ostree_repo_list_objects()` I was looking at https://github.com/ostreedev/ostree/pull/2632 and confused at the usage of `GVariant *value = g_variant_new ("(b@as)", TRUE, g_variant_new_strv (NULL, 0));` which looked strange - why the empty strv? It turns out that this is a historical legacy of the time when ostree had pack files. And nothing actually cares about the values of these variants; we should have an API that returns a proper set, and not a hash. But...since all of these things have exactly the same value, instead of allocating lots of redundant copies on the heap, just have them all hold a refcount on a shared value. This cuts the heap usage from 20MB to 13MB on a test FCOS repository build. --- src/libostree/ostree-repo.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 994db626..5b1aaae9 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -3895,6 +3895,7 @@ ostree_repo_get_parent (OstreeRepo *self) static gboolean list_loose_objects_at (OstreeRepo *self, + GVariant *dummy_value, GHashTable *inout_objects, int dfd, const char *prefix, @@ -3902,7 +3903,7 @@ list_loose_objects_at (OstreeRepo *self, GCancellable *cancellable, GError **error) { - GVariant *key, *value; + GVariant *key; g_auto(GLnxDirFdIterator) dfd_iter = { 0, }; gboolean exists; @@ -3971,11 +3972,10 @@ list_loose_objects_at (OstreeRepo *self, } key = ostree_object_name_serialize (buf, objtype); - value = g_variant_new ("(b@as)", - TRUE, g_variant_new_strv (NULL, 0)); + /* transfer ownership */ g_hash_table_replace (inout_objects, g_variant_ref_sink (key), - g_variant_ref_sink (value)); + g_variant_ref (dummy_value)); } return TRUE; @@ -3989,6 +3989,9 @@ list_loose_objects (OstreeRepo *self, GError **error) { static const gchar hexchars[] = "0123456789abcdef"; + // For unfortunate historical reasons we emit this dummy value. + g_autoptr(GVariant) dummy_loose_object_variant = + g_variant_ref_sink (g_variant_new ("(b@as)", TRUE, g_variant_new_strv (NULL, 0))); for (guint c = 0; c < 256; c++) { @@ -3996,7 +3999,8 @@ list_loose_objects (OstreeRepo *self, buf[0] = hexchars[c >> 4]; buf[1] = hexchars[c & 0xF]; buf[2] = '\0'; - if (!list_loose_objects_at (self, inout_objects, self->objects_dir_fd, buf, + if (!list_loose_objects_at (self, dummy_loose_object_variant, + inout_objects, self->objects_dir_fd, buf, commit_starting_with, cancellable, error)) return FALSE; From 4806d84f568fcb2faa57b43c8f253b2daba2766b Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 7 Jun 2022 17:59:02 -0400 Subject: [PATCH 03/56] rust: Bump semver, add feature for current release There were some changes to the sys API for introspection fixes. And add a feature for the current release, which is something I'll add to the checklist for releases. --- .github/workflows/rust.yml | 2 +- Cargo.toml | 5 +++-- rust-bindings/sys/Cargo.toml | 6 +++++- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index dff0f517..9e60edca 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -11,7 +11,7 @@ on: env: CARGO_TERM_COLOR: always - CARGO_PROJECT_FEATURES: "v2021_3" + CARGO_PROJECT_FEATURES: "v2021_5" # Minimum supported Rust version (MSRV) ACTION_MSRV_TOOLCHAIN: 1.54.0 # Pinned toolchain for linting diff --git a/Cargo.toml b/Cargo.toml index 7c849bf5..0489bc5f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -8,7 +8,7 @@ license = "MIT" name = "ostree" readme = "README.md" repository = "https://github.com/ostreedev/ostree-rs" -version = "0.13.6" +version = "0.14.0" exclude = [ "rust-bindings/conf/**", @@ -32,7 +32,7 @@ members = [".", "rust-bindings/sys"] bitflags = "1.2.1" cap-std = { version = "0.24", optional = true} io-lifetimes = { version = "0.5", optional = true} -ffi = { package = "ostree-sys", path = "rust-bindings/sys", version = "0.9.1" } +ffi = { package = "ostree-sys", path = "rust-bindings/sys", version = "0.10.0" } gio = "0.14" glib = "0.14.4" hex = "0.4.2" @@ -92,3 +92,4 @@ v2021_2 = ["v2021_1", "ffi/v2021_2"] v2021_3 = ["v2021_2", "ffi/v2021_3"] v2021_4 = ["v2021_3", "ffi/v2021_4"] v2021_5 = ["v2021_4", "ffi/v2021_5"] +v2022_5 = ["v2021_5", "ffi/v2022_5"] diff --git a/rust-bindings/sys/Cargo.toml b/rust-bindings/sys/Cargo.toml index 5bda7ba3..fd453454 100644 --- a/rust-bindings/sys/Cargo.toml +++ b/rust-bindings/sys/Cargo.toml @@ -56,6 +56,7 @@ v2021_3 = ["v2021_2"] v2021_4 = ["v2021_3"] v2021_5 = ["v2021_4"] v2022_2 = ["v2021_5"] +v2022_5 = ["v2022_2"] [lib] name = "ostree_sys" @@ -71,7 +72,7 @@ license = "MIT" links = "ostree-1" name = "ostree-sys" repository = "https://github.com/ostreedev/ostree-rs" -version = "0.9.2" +version = "0.10.0" edition = "2018" [package.metadata.docs.rs] features = ["dox"] @@ -207,3 +208,6 @@ version = "2021.5" [package.metadata.system-deps.ostree_1.v2022_2] version = "2022.2" + +[package.metadata.system-deps.ostree_1.v2022_5] +version = "2022.5" From a71915e4367fa8757dc267d128f006661d00feda Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 7 Jun 2022 21:41:08 -0400 Subject: [PATCH 04/56] repo: Further optimize `ostree_repo_list_objects_set()` In a prior change we discovered that for bad historical reasons libostree was returning a mapping "object type+checksum" => "metadata" but the "metadata" was redundant and pointless. Optimize the prune API to use a (currently internal) object listing API which returns a set, not a map. This allows `GHashTable` to avoid allocating a separate array for the values, neatly cutting memory usage in half (from ~13MB to ~6MB) on my test case of a dry-run prune of a FCOS build. --- src/libostree/ostree-repo-private.h | 6 ++ src/libostree/ostree-repo-prune.c | 14 +--- src/libostree/ostree-repo.c | 110 +++++++++++++++++++--------- 3 files changed, 84 insertions(+), 46 deletions(-) diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h index 96253e77..3858a36e 100644 --- a/src/libostree/ostree-repo-private.h +++ b/src/libostree/ostree-repo-private.h @@ -513,6 +513,12 @@ _ostree_repo_verify_bindings (const char *collection_id, GVariant *commit, GError **error); +GHashTable * +ostree_repo_list_objects_set (OstreeRepo *self, + OstreeRepoListObjectsFlags flags, + GCancellable *cancellable, + GError **error); + /** * OstreeRepoAutoTransaction: * diff --git a/src/libostree/ostree-repo-prune.c b/src/libostree/ostree-repo-prune.c index 0c702dc9..ccd55782 100644 --- a/src/libostree/ostree-repo-prune.c +++ b/src/libostree/ostree-repo-prune.c @@ -280,15 +280,8 @@ repo_prune_internal (OstreeRepo *self, g_autoptr(GHashTable) reachable_owned = g_hash_table_ref (options->reachable); data.reachable = reachable_owned; - GLNX_HASH_TABLE_FOREACH_KV (objects, GVariant*, serialized_key, GVariant*, objdata) + GLNX_HASH_TABLE_FOREACH (objects, GVariant*, serialized_key) { - gboolean is_loose; - - g_variant_get_child (objdata, 0, "b", &is_loose); - - if (!is_loose) - continue; - if (!maybe_prune_loose_object (&data, options->flags, serialized_key, cancellable, error)) return FALSE; @@ -444,8 +437,9 @@ ostree_repo_prune (OstreeRepo *self, return FALSE; } - if (!ostree_repo_list_objects (self, OSTREE_REPO_LIST_OBJECTS_ALL | OSTREE_REPO_LIST_OBJECTS_NO_PARENTS, - &objects, cancellable, error)) + objects = ostree_repo_list_objects_set (self, OSTREE_REPO_LIST_OBJECTS_ALL | OSTREE_REPO_LIST_OBJECTS_NO_PARENTS, + cancellable, error); + if (!objects) return FALSE; if (!refs_only) diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 5b1aaae9..e823e19e 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -3974,8 +3974,11 @@ list_loose_objects_at (OstreeRepo *self, key = ostree_object_name_serialize (buf, objtype); /* transfer ownership */ - g_hash_table_replace (inout_objects, g_variant_ref_sink (key), - g_variant_ref (dummy_value)); + if (dummy_value) + g_hash_table_replace (inout_objects, g_variant_ref_sink (key), + g_variant_ref (dummy_value)); + else + g_hash_table_add (inout_objects, g_variant_ref_sink (key)); } return TRUE; @@ -3983,15 +3986,13 @@ list_loose_objects_at (OstreeRepo *self, static gboolean list_loose_objects (OstreeRepo *self, + GVariant *dummy_value, GHashTable *inout_objects, const char *commit_starting_with, GCancellable *cancellable, GError **error) { static const gchar hexchars[] = "0123456789abcdef"; - // For unfortunate historical reasons we emit this dummy value. - g_autoptr(GVariant) dummy_loose_object_variant = - g_variant_ref_sink (g_variant_new ("(b@as)", TRUE, g_variant_new_strv (NULL, 0))); for (guint c = 0; c < 256; c++) { @@ -3999,7 +4000,7 @@ list_loose_objects (OstreeRepo *self, buf[0] = hexchars[c >> 4]; buf[1] = hexchars[c & 0xF]; buf[2] = '\0'; - if (!list_loose_objects_at (self, dummy_loose_object_variant, + if (!list_loose_objects_at (self, dummy_value, inout_objects, self->objects_dir_fd, buf, commit_starting_with, cancellable, error)) @@ -4905,6 +4906,65 @@ ostree_repo_load_commit (OstreeRepo *self, out_variant, NULL, NULL, out_state, NULL, error); } +static GHashTable * +repo_list_objects_impl (OstreeRepo *self, + OstreeRepoListObjectsFlags flags, + GVariant *dummy_value, + GCancellable *cancellable, + GError **error) +{ + g_assert (error == NULL || *error == NULL); + g_assert (self->inited); + + g_autoptr(GHashTable) ret_objects = + g_hash_table_new_full (ostree_hash_object_name, g_variant_equal, + (GDestroyNotify) g_variant_unref, + dummy_value ? (GDestroyNotify) g_variant_unref : NULL); + + if (flags & OSTREE_REPO_LIST_OBJECTS_ALL) + flags |= (OSTREE_REPO_LIST_OBJECTS_LOOSE | OSTREE_REPO_LIST_OBJECTS_PACKED); + + if (flags & OSTREE_REPO_LIST_OBJECTS_LOOSE) + { + if (!list_loose_objects (self, dummy_value, ret_objects, NULL, cancellable, error)) + return FALSE; + if ((flags & OSTREE_REPO_LIST_OBJECTS_NO_PARENTS) == 0 && self->parent_repo) + { + if (!list_loose_objects (self->parent_repo, dummy_value, ret_objects, NULL, cancellable, error)) + return FALSE; + } + } + + if (flags & OSTREE_REPO_LIST_OBJECTS_PACKED) + { + /* Nothing for now... */ + } + + return g_steal_pointer (&ret_objects); +} + +/* A currently-internal version of ostree_repo_list_objects which returns + * a set, and not a map (with a useless value). + */ +GHashTable * +ostree_repo_list_objects_set (OstreeRepo *self, + OstreeRepoListObjectsFlags flags, + GCancellable *cancellable, + GError **error) +{ + return repo_list_objects_impl (self, flags, NULL, cancellable, error); +} + +/* For unfortunate historical reasons we emit this dummy value. + * It was intended to provide additional information about the object (e.g. "is in a pack file") + * but we ended up not shipping pack files. + */ +static GVariant * +get_dummy_list_objects_variant (void) +{ + return g_variant_ref_sink (g_variant_new ("(b@as)", TRUE, g_variant_new_strv (NULL, 0))); +} + /** * ostree_repo_list_objects: * @self: Repo @@ -4928,34 +4988,11 @@ ostree_repo_list_objects (OstreeRepo *self, GCancellable *cancellable, GError **error) { - g_return_val_if_fail (error == NULL || *error == NULL, FALSE); - g_return_val_if_fail (self->inited, FALSE); - - g_autoptr(GHashTable) ret_objects = - g_hash_table_new_full (ostree_hash_object_name, g_variant_equal, - (GDestroyNotify) g_variant_unref, - (GDestroyNotify) g_variant_unref); - - if (flags & OSTREE_REPO_LIST_OBJECTS_ALL) - flags |= (OSTREE_REPO_LIST_OBJECTS_LOOSE | OSTREE_REPO_LIST_OBJECTS_PACKED); - - if (flags & OSTREE_REPO_LIST_OBJECTS_LOOSE) - { - if (!list_loose_objects (self, ret_objects, NULL, cancellable, error)) - return FALSE; - if ((flags & OSTREE_REPO_LIST_OBJECTS_NO_PARENTS) == 0 && self->parent_repo) - { - if (!list_loose_objects (self->parent_repo, ret_objects, NULL, cancellable, error)) - return FALSE; - } - } - - if (flags & OSTREE_REPO_LIST_OBJECTS_PACKED) - { - /* Nothing for now... */ - } - - ot_transfer_out_value (out_objects, &ret_objects); + g_autoptr(GVariant) dummy_value = get_dummy_list_objects_variant (); + g_autoptr(GHashTable) ret = repo_list_objects_impl (self, flags, dummy_value, cancellable, error); + if (!ret) + return FALSE; + ot_transfer_out_value (out_objects, &ret); return TRUE; } @@ -4987,13 +5024,14 @@ ostree_repo_list_commit_objects_starting_with (OstreeRepo *self g_hash_table_new_full (ostree_hash_object_name, g_variant_equal, (GDestroyNotify) g_variant_unref, (GDestroyNotify) g_variant_unref); + g_autoptr(GVariant) dummy_loose_object_variant = get_dummy_list_objects_variant (); - if (!list_loose_objects (self, ret_commits, start, cancellable, error)) + if (!list_loose_objects (self, dummy_loose_object_variant, ret_commits, start, cancellable, error)) return FALSE; if (self->parent_repo) { - if (!list_loose_objects (self->parent_repo, ret_commits, start, + if (!list_loose_objects (self->parent_repo, dummy_loose_object_variant, ret_commits, start, cancellable, error)) return FALSE; } From 5341a13b33d44284dc7aadf02f7689290d429234 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 8 Jun 2022 09:53:33 -0400 Subject: [PATCH 05/56] ci: Add a flow that does a git libostree + git rust-bindings In https://github.com/ostreedev/ostree/pull/2633 I realized that our CI only builds git of libostree or git of rust-bindings, not git of both. And we definitely want to test the latter too, so e.g. the Rust tests *also* become tests for changes to the C code. --- .github/workflows/rust.yml | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 9e60edca..a830c0e8 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -12,6 +12,8 @@ on: env: CARGO_TERM_COLOR: always CARGO_PROJECT_FEATURES: "v2021_5" + # TODO: Automatically query this from the C side + LATEST_LIBOSTREE: "v2022_5" # Minimum supported Rust version (MSRV) ACTION_MSRV_TOOLCHAIN: 1.54.0 # Pinned toolchain for linting @@ -47,6 +49,24 @@ jobs: uses: Swatinem/rust-cache@ce325b60658c1b38465c06cc965b79baf32c1e72 - name: cargo build run: cargo build --features=${{ env['CARGO_PROJECT_FEATURES'] }} + build-git-libostree: + runs-on: ubuntu-latest + container: quay.io/coreos-assembler/fcos-buildroot:testing-devel + steps: + - uses: actions/checkout@v2 + with: + fetch-depth: 0 + submodules: true + - name: Cache Dependencies + uses: Swatinem/rust-cache@ce325b60658c1b38465c06cc965b79baf32c1e72 + - name: Build libostree + run: ./ci/build.sh + - name: Install libostree + run: make install + - name: Rust build + run: cargo test --no-run --verbose --features=${{ env['LATEST_LIBOSTREE'] }} + - name: Run tests + run: cargo test --verbose --features=${{ env['LATEST_LIBOSTREE'] }} linting: name: "Lints, pinned toolchain" runs-on: ubuntu-latest From fffb1116339933d62f25cfe033ca620a49239ae6 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 8 Jun 2022 16:22:26 -0400 Subject: [PATCH 06/56] prune: Also use object set API in `ostree_repo_prune_from_reachable()` I missed the second prune path when working on https://github.com/ostreedev/ostree/pull/2635 --- src/libostree/ostree-repo-prune.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libostree/ostree-repo-prune.c b/src/libostree/ostree-repo-prune.c index ccd55782..da4f4284 100644 --- a/src/libostree/ostree-repo-prune.c +++ b/src/libostree/ostree-repo-prune.c @@ -508,10 +508,10 @@ ostree_repo_prune_from_reachable (OstreeRepo *self, if (!lock) return FALSE; - g_autoptr(GHashTable) objects = NULL; - - if (!ostree_repo_list_objects (self, OSTREE_REPO_LIST_OBJECTS_ALL | OSTREE_REPO_LIST_OBJECTS_NO_PARENTS, - &objects, cancellable, error)) + g_autoptr(GHashTable) objects = + ostree_repo_list_objects_set (self, OSTREE_REPO_LIST_OBJECTS_ALL | OSTREE_REPO_LIST_OBJECTS_NO_PARENTS, + cancellable, error); + if (!objects) return FALSE; return repo_prune_internal (self, objects, options, out_objects_total, From 145d91d1c96755bc61a468b5da1061547909121e Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 8 Jun 2022 16:27:30 -0400 Subject: [PATCH 07/56] lib: Fix symbol versioning inheritance I messed this up; the last release should inherit from the previous release (N-1) and not the previous to that (N-2). I think (hope) this isn't an ABI break... Just noticed this when I was going to add a new symbol. --- src/libostree/libostree-released.sym | 2 +- tests/test-symbols.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libostree/libostree-released.sym b/src/libostree/libostree-released.sym index 8b8a722d..5afdfd3e 100644 --- a/src/libostree/libostree-released.sym +++ b/src/libostree/libostree-released.sym @@ -681,7 +681,7 @@ LIBOSTREE_2022.4 { global: ostree_fs_get_all_xattrs; ostree_fs_get_all_xattrs_at; -} LIBOSTREE_2021.5; +} LIBOSTREE_2022.2; /* 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 a888ef35..a14849d5 100755 --- a/tests/test-symbols.sh +++ b/tests/test-symbols.sh @@ -54,7 +54,7 @@ echo 'ok documented symbols' # ONLY update this checksum in release commits! cat > released-sha256.txt < Date: Thu, 9 Jun 2022 14:51:45 -0400 Subject: [PATCH 08/56] tests/inst: Bump the version of ostree-ext In the interest of cross-testing and keeping things up to date. Hmm, I think we need to set up dependabot here. --- tests/inst/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/inst/Cargo.toml b/tests/inst/Cargo.toml index cf9c2dcc..aed50ab6 100644 --- a/tests/inst/Cargo.toml +++ b/tests/inst/Cargo.toml @@ -19,7 +19,7 @@ serde_json = "1.0" sh-inline = "0.2.0" anyhow = "1.0" tempfile = "3.1.0" -ostree-ext = { version = "0.6.0" } +ostree-ext = { version = "0.7.0" } libtest-mimic = "0.3.0" twoway = "0.2.1" hyper = { version = "0.14", features = ["runtime", "http1", "http2", "tcp", "server"] } From dc13645299df2ddfa77fde8e4072834fb0025e4f Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 9 Jun 2022 14:53:06 -0400 Subject: [PATCH 09/56] rust-bindings: Fix repository reference Since the repo merge. --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 0489bc5f..da623f1e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,7 +7,7 @@ keywords = ["ostree", "libostree"] license = "MIT" name = "ostree" readme = "README.md" -repository = "https://github.com/ostreedev/ostree-rs" +repository = "https://github.com/ostreedev/ostree" version = "0.14.0" exclude = [ From 61dd54f94091b2493afb1fa66e716acb7cd1aa15 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 9 Jun 2022 14:55:25 -0400 Subject: [PATCH 10/56] rust-bindings: use correct README.md I noticed at https://crates.io/crates/ostree/0.14.0 that the `README.md` was wrong... --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index da623f1e..561b4dec 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -6,7 +6,7 @@ edition = "2018" keywords = ["ostree", "libostree"] license = "MIT" name = "ostree" -readme = "README.md" +readme = "rust-bindings/README.md" repository = "https://github.com/ostreedev/ostree" version = "0.14.0" From d7802c27ddd6ffbb8c18cef459f5882a546499c0 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 9 Jun 2022 14:59:44 -0400 Subject: [PATCH 11/56] rust-bindings: Update cargo package list When we did the merger, it turns out cargo by default is basically going to include all of stuff in the git repository root directory which is "libostree". We just want the stuff in `rust-bindings/`. I initially tried adding `include = "rust-bindings/"` but according to https://doc.rust-lang.org/cargo/reference/manifest.html#the-exclude-and-include-fields specifying `include` means that `exclude` is ignored, which is kind of annoying. Further, doing so *also* turns off the cargo automatic rules for handling e.g. `gitignore`. So for now I went with the approach of adding everything from the C library stuff into `exclude/`. --- Cargo.toml | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 561b4dec..a52c27d3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,11 +11,15 @@ repository = "https://github.com/ostreedev/ostree" version = "0.14.0" exclude = [ - "rust-bindings/conf/**", - "rust-bindings/gir-files/**", - "rust-bindings/sys/**", - ".gitlab-ci.yml", - "LICENSE.LGPL*", + "/*.am", "/apidoc", "/autogen.sh", "/bash", "/bsdiff", + "/build-aux", "/buildutil", "/*.mk", "/ci", "/coccinelle", + "/*.ac", "/docs", "/libglnx", "/man", "/manual-tests", + "/*.yml", "/*.doap", "/src", "/tests", + "/.github", "/.cci.jenkinsfile", "/.dir-locals.el", "/.editorconfig", + "/.vimrc", "/.copr", "/.packit.yaml", "/GNUmakefile", "TODO", + "/rust-bindings/conf/**", + "/rust-bindings/gir-files/**", + "/rust-bindings/sys/**", ] [package.metadata.docs.rs] From 99c122d219002a638624c7c580c946a5b0be904a Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 9 Jun 2022 15:55:41 -0400 Subject: [PATCH 12/56] rust: Switch to 2021 edition No real changes. ``` $ cargo fix --edition note: Switching to Edition 2021 will enable the use of the version 2 feature resolver in Cargo. This may cause some dependencies to be built with fewer features enabled than previously. More information about the resolver changes may be found at https://doc.rust-lang.org/nightly/edition-guide/rust-2021/default-cargo-resolver.html When building the following dependencies, the given features will no longer be used: libc v0.2.126 removed features: extra_traits The following differences only apply when building with dev-dependencies: getrandom v0.2.6 removed features: std ``` which looks OK to me. --- Cargo.toml | 2 +- rust-bindings/sys/Cargo.toml | 2 +- tests/inst/Cargo.toml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 0489bc5f..68b3d2bf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2,7 +2,7 @@ authors = ["Felix Krull"] description = "Rust bindings for libostree" documentation = "https://docs.rs/ostree" -edition = "2018" +edition = "2021" keywords = ["ostree", "libostree"] license = "MIT" name = "ostree" diff --git a/rust-bindings/sys/Cargo.toml b/rust-bindings/sys/Cargo.toml index fd453454..5145ab87 100644 --- a/rust-bindings/sys/Cargo.toml +++ b/rust-bindings/sys/Cargo.toml @@ -73,7 +73,7 @@ links = "ostree-1" name = "ostree-sys" repository = "https://github.com/ostreedev/ostree-rs" version = "0.10.0" -edition = "2018" +edition = "2021" [package.metadata.docs.rs] features = ["dox"] [package.metadata.system-deps.ostree_1] diff --git a/tests/inst/Cargo.toml b/tests/inst/Cargo.toml index cf9c2dcc..cd3515b5 100644 --- a/tests/inst/Cargo.toml +++ b/tests/inst/Cargo.toml @@ -2,7 +2,7 @@ name = "ostree-test" version = "0.1.0" authors = ["Colin Walters "] -edition = "2018" +edition = "2021" [workspace] From 76071a2b1184e5e7db7dc8b2e6971eb8313868d3 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 9 Jun 2022 17:49:18 -0400 Subject: [PATCH 13/56] ci: Bump MSRV To match what's in ostree-rs-ext. --- .github/workflows/rust.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index a830c0e8..b9c972e2 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -15,7 +15,7 @@ env: # TODO: Automatically query this from the C side LATEST_LIBOSTREE: "v2022_5" # Minimum supported Rust version (MSRV) - ACTION_MSRV_TOOLCHAIN: 1.54.0 + ACTION_MSRV_TOOLCHAIN: 1.58.1 # Pinned toolchain for linting ACTION_LINTS_TOOLCHAIN: 1.56.0 From 93e3784b66f53d568cf50831b21dc5652c6b80a6 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 9 Jun 2022 17:51:07 -0400 Subject: [PATCH 14/56] rust: Use inline `format!` variables in a few places Since our MSRV now supports it. --- rust-bindings/src/object_name.rs | 2 +- tests/inst/src/destructive.rs | 4 ++-- tests/inst/src/treegen.rs | 8 ++++---- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/rust-bindings/src/object_name.rs b/rust-bindings/src/object_name.rs index 06ff8045..ed4c1bfe 100644 --- a/rust-bindings/src/object_name.rs +++ b/rust-bindings/src/object_name.rs @@ -86,7 +86,7 @@ mod tests { #[test] fn should_stringify_object_name() { let object_name = ObjectName::new("abcdef123456", ObjectType::DirTree); - let stringified = format!("{}", object_name); + let stringified = format!("{object_name}"); assert_eq!(stringified, "abcdef123456.dirtree"); } diff --git a/tests/inst/src/destructive.rs b/tests/inst/src/destructive.rs index 352ee6d5..6cce4b7a 100644 --- a/tests/inst/src/destructive.rs +++ b/tests/inst/src/destructive.rs @@ -363,7 +363,7 @@ fn impl_transaction_test>( booted: booted_commit.to_string(), orig: sysrepo_obj.resolve_rev(ORIGREF, false)?.unwrap().into(), prev: srvrepo_obj - .resolve_rev(&format!("{}^", TESTREF), false)? + .resolve_rev(&format!("{TESTREF}^"), false)? .unwrap() .into(), target: srvrepo_obj.resolve_rev(TESTREF, false)?.unwrap().into(), @@ -568,7 +568,7 @@ pub(crate) fn itest_transactionality() -> Result<()> { ..Default::default() }; with_webserver_in(&srvrepo, &webserver_opts, move |addr| { - let url = format!("http://{}", addr); + let url = format!("http://{addr}"); bash!( "ostree remote delete --if-exists testrepo ostree remote add --set=gpg-verify=false testrepo ${url}", diff --git a/tests/inst/src/treegen.rs b/tests/inst/src/treegen.rs index b6a3a704..29bdffd2 100644 --- a/tests/inst/src/treegen.rs +++ b/tests/inst/src/treegen.rs @@ -31,9 +31,9 @@ pub(crate) fn mkvroot>(p: P, v: u32) -> Result<()> { std::fs::create_dir_all(p.join(v))?; } let verpath = p.join("etc/.mkrootversion"); - write_file(&verpath, &format!("{}", v))?; - write_file(p.join("usr/bin/somebinary"), &format!("somebinary v{}", v))?; - write_file(p.join("etc/someconf"), &format!("someconf v{}", v))?; + write_file(&verpath, &format!("{v}"))?; + write_file(p.join("usr/bin/somebinary"), &format!("somebinary v{v}"))?; + write_file(p.join("etc/someconf"), &format!("someconf v{v}"))?; write_file(p.join("usr/bin/vmod2"), &format!("somebinary v{}", v % 2))?; write_file(p.join("usr/bin/vmod3"), &format!("somebinary v{}", v % 3))?; Ok(()) @@ -134,7 +134,7 @@ pub(crate) fn update_os_tree>( tempdir.ensure_dir(v, 0o755)?; let dest = tempdir.sub_dir(v)?; mutated += mutate_executables_to(&src, &dest, percentage) - .with_context(|| format!("Replacing binaries in {}", v))?; + .with_context(|| format!("Replacing binaries in {v}"))?; } } } From a13d812368639865bacb59237e33a6a09035b771 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 9 Jun 2022 18:34:25 -0400 Subject: [PATCH 15/56] repo: Document non-obvious way to list all commits I was going to add an API for this and then realized the empty string does it. --- src/libostree/ostree-repo.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index e823e19e..9b29bd3f 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -4999,7 +4999,7 @@ ostree_repo_list_objects (OstreeRepo *self, /** * ostree_repo_list_commit_objects_starting_with: * @self: Repo - * @start: List commits starting with this checksum + * @start: List commits starting with this checksum (empty string for all) * @out_commits: (out) (transfer container) (element-type GVariant GVariant): * Map of serialized commit name to variant data * @cancellable: Cancellable @@ -5008,6 +5008,8 @@ ostree_repo_list_objects (OstreeRepo *self, * This function synchronously enumerates all commit objects starting * with @start, returning data in @out_commits. * + * To list all commit objects, provide the empty string `""` for @start. + * * Returns: %TRUE on success, %FALSE on error, and @error will be set */ gboolean From 6981633f9c91abc8b8f8776813f75b992c7cdf38 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 9 Jun 2022 18:32:09 -0400 Subject: [PATCH 16/56] fsck: Don't load all object names into memory We recently discovered `list_objects()` is inefficient with memory. The more efficient `list_objects_set()` API isn't yet public, but this fsck code actually just skips over non-commit objects, and we already have an API to list just those. --- src/ostree/ot-builtin-fsck.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/ostree/ot-builtin-fsck.c b/src/ostree/ot-builtin-fsck.c index 9e3f77ef..77ad6a4f 100644 --- a/src/ostree/ot-builtin-fsck.c +++ b/src/ostree/ot-builtin-fsck.c @@ -282,17 +282,16 @@ ostree_builtin_fsck (int argc, char **argv, OstreeCommandInvocation *invocation, } if (!opt_quiet) - g_print ("Enumerating objects...\n"); + g_print ("Enumerating commits...\n"); - g_autoptr(GHashTable) objects = NULL; - if (!ostree_repo_list_objects (repo, OSTREE_REPO_LIST_OBJECTS_ALL, - &objects, cancellable, error)) + // Find all commit objects, including partial ones + g_autoptr(GHashTable) all_commits = NULL; + if (!ostree_repo_list_commit_objects_starting_with (repo, "", &all_commits, cancellable, error)) return FALSE; - + // And gather a set of non-partial commits for further analysis g_autoptr(GHashTable) commits = g_hash_table_new_full (ostree_hash_object_name, g_variant_equal, (GDestroyNotify)g_variant_unref, NULL); - g_autoptr(GPtrArray) tombstones = NULL; if (opt_add_tombstones) tombstones = g_ptr_array_new_with_free_func (g_free); @@ -302,8 +301,8 @@ ostree_builtin_fsck (int argc, char **argv, OstreeCommandInvocation *invocation, guint n_partial = 0; guint n_fsck_partial = 0; - g_hash_table_iter_init (&hash_iter, objects); - while (g_hash_table_iter_next (&hash_iter, &key, &value)) + g_hash_table_iter_init (&hash_iter, all_commits); + while (g_hash_table_iter_next (&hash_iter, &key, NULL)) { GVariant *serialized_key = key; const char *checksum; @@ -313,7 +312,9 @@ ostree_builtin_fsck (int argc, char **argv, OstreeCommandInvocation *invocation, ostree_object_name_deserialize (serialized_key, &checksum, &objtype); - if (objtype == OSTREE_OBJECT_TYPE_COMMIT) + g_assert (objtype == OSTREE_OBJECT_TYPE_COMMIT); + + if (TRUE) // Temporarily avoiding the noise of de-indenting this whole thing { if (!ostree_repo_load_commit (repo, checksum, &commit, &commitstate, error)) return FALSE; @@ -420,7 +421,7 @@ ostree_builtin_fsck (int argc, char **argv, OstreeCommandInvocation *invocation, } } - g_clear_pointer (&objects, g_hash_table_unref); + g_clear_pointer (&all_commits, g_hash_table_unref); if (!opt_quiet) g_print ("Verifying content integrity of %u commit objects...\n", From 2fe0ea73955c585f9d3fa4989318e73ad21a4015 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sun, 12 Jun 2022 10:48:39 -0400 Subject: [PATCH 17/56] fsck: De-indent loop Could have done this in the previous change, but wanted to avoid excessive diff noise. --- src/ostree/ot-builtin-fsck.c | 176 +++++++++++++++++------------------ 1 file changed, 83 insertions(+), 93 deletions(-) diff --git a/src/ostree/ot-builtin-fsck.c b/src/ostree/ot-builtin-fsck.c index 77ad6a4f..2c104c9e 100644 --- a/src/ostree/ot-builtin-fsck.c +++ b/src/ostree/ot-builtin-fsck.c @@ -314,112 +314,102 @@ ostree_builtin_fsck (int argc, char **argv, OstreeCommandInvocation *invocation, g_assert (objtype == OSTREE_OBJECT_TYPE_COMMIT); - if (TRUE) // Temporarily avoiding the noise of de-indenting this whole thing + if (!ostree_repo_load_commit (repo, checksum, &commit, &commitstate, error)) + return FALSE; + /* If requested, check that all the refs listed in the ref-bindings + * for this commit resolve back to this commit. */ + if (opt_verify_back_refs) { - if (!ostree_repo_load_commit (repo, checksum, &commit, &commitstate, error)) - return FALSE; - - /* If requested, check that all the refs listed in the ref-bindings - * for this commit resolve back to this commit. */ - if (opt_verify_back_refs) + g_autoptr(GVariant) metadata = g_variant_get_child_value (commit, 0); + const char *collection_id = NULL; + if (!g_variant_lookup (metadata, + OSTREE_COMMIT_META_KEY_COLLECTION_BINDING, + "&s", + &collection_id)) + collection_id = NULL; + g_autofree const char **refs = NULL; + if (g_variant_lookup (metadata, + OSTREE_COMMIT_META_KEY_REF_BINDING, + "^a&s", + &refs)) { - g_autoptr(GVariant) metadata = g_variant_get_child_value (commit, 0); - - const char *collection_id = NULL; - if (!g_variant_lookup (metadata, - OSTREE_COMMIT_META_KEY_COLLECTION_BINDING, - "&s", - &collection_id)) - collection_id = NULL; - - g_autofree const char **refs = NULL; - if (g_variant_lookup (metadata, - OSTREE_COMMIT_META_KEY_REF_BINDING, - "^a&s", - &refs)) + for (const char **iter = refs; *iter != NULL; ++iter) { - for (const char **iter = refs; *iter != NULL; ++iter) + g_autofree char *checksum_for_ref = NULL; + if (collection_id != NULL) + { + const OstreeCollectionRef collection_ref = { (char *) collection_id, (char *) *iter }; + if (!ostree_repo_resolve_collection_ref (repo, &collection_ref, + TRUE, + OSTREE_REPO_RESOLVE_REV_EXT_NONE, + &checksum_for_ref, + cancellable, + error)) + return FALSE; + } + else + { + if (!ostree_repo_resolve_rev (repo, *iter, TRUE, + &checksum_for_ref, error)) + return FALSE; + } + if (checksum_for_ref == NULL) { - g_autofree char *checksum_for_ref = NULL; - if (collection_id != NULL) - { - const OstreeCollectionRef collection_ref = { (char *) collection_id, (char *) *iter }; - if (!ostree_repo_resolve_collection_ref (repo, &collection_ref, - TRUE, - OSTREE_REPO_RESOLVE_REV_EXT_NONE, - &checksum_for_ref, - cancellable, - error)) - return FALSE; - } + return glnx_throw (error, + "Collection–ref (%s, %s) in bindings for commit %s does not exist", + collection_id, *iter, checksum); else - { - if (!ostree_repo_resolve_rev (repo, *iter, TRUE, - &checksum_for_ref, error)) - return FALSE; - } - - if (checksum_for_ref == NULL) - { - if (collection_id != NULL) - return glnx_throw (error, - "Collection–ref (%s, %s) in bindings for commit %s does not exist", - collection_id, *iter, checksum); - else - return glnx_throw (error, - "Ref ‘%s’ in bindings for commit %s does not exist", - *iter, checksum); - } - else if (g_strcmp0 (checksum_for_ref, checksum) != 0) - { - if (collection_id != NULL) - return glnx_throw (error, - "Collection–ref (%s, %s) in bindings for commit %s does not resolve to that commit", - collection_id, *iter, checksum); - else - return glnx_throw (error, - "Ref ‘%s’ in bindings for commit %s does not resolve to that commit", - *iter, checksum); - } + return glnx_throw (error, + "Ref ‘%s’ in bindings for commit %s does not exist", + *iter, checksum); } - } - } - - if (opt_add_tombstones) - { - GError *local_error = NULL; - g_autofree char *parent = ostree_commit_get_parent (commit); - if (parent) - { - g_autoptr(GVariant) parent_commit = NULL; - if (!ostree_repo_load_variant (repo, OSTREE_OBJECT_TYPE_COMMIT, parent, - &parent_commit, &local_error)) + else if (g_strcmp0 (checksum_for_ref, checksum) != 0) { - if (g_error_matches (local_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND)) - { - g_ptr_array_add (tombstones, g_strdup (checksum)); - g_clear_error (&local_error); - } + if (collection_id != NULL) + return glnx_throw (error, + "Collection–ref (%s, %s) in bindings for commit %s does not resolve to that commit", + collection_id, *iter, checksum); else - { - g_propagate_error (error, local_error); - return FALSE; - } + return glnx_throw (error, + "Ref ‘%s’ in bindings for commit %s does not resolve to that commit", + *iter, checksum); } } } - - if (commitstate & OSTREE_REPO_COMMIT_STATE_PARTIAL) - { - n_partial++; - if (commitstate & OSTREE_REPO_COMMIT_STATE_FSCK_PARTIAL) - n_fsck_partial++; - } - else - g_hash_table_add (commits, g_variant_ref (serialized_key)); } - } + if (opt_add_tombstones) + { + GError *local_error = NULL; + g_autofree char *parent = ostree_commit_get_parent (commit); + if (parent) + { + g_autoptr(GVariant) parent_commit = NULL; + if (!ostree_repo_load_variant (repo, OSTREE_OBJECT_TYPE_COMMIT, parent, + &parent_commit, &local_error)) + { + if (g_error_matches (local_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND)) + { + g_ptr_array_add (tombstones, g_strdup (checksum)); + g_clear_error (&local_error); + } + else + { + g_propagate_error (error, local_error); + return FALSE; + } + } + } + } + if (commitstate & OSTREE_REPO_COMMIT_STATE_PARTIAL) + { + n_partial++; + if (commitstate & OSTREE_REPO_COMMIT_STATE_FSCK_PARTIAL) + n_fsck_partial++; + } + else + g_hash_table_add (commits, g_variant_ref (serialized_key)); + } g_clear_pointer (&all_commits, g_hash_table_unref); From bd030a96f26e56731cc26c15682f293a1a0142e9 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sun, 12 Jun 2022 10:53:09 -0400 Subject: [PATCH 18/56] fsck: Move most commit processing into helper function The inner loop was way too long; split out most of the heavy lifting around backrefs and tombstones into a helper function. --- src/ostree/ot-builtin-fsck.c | 183 +++++++++++++++++++---------------- 1 file changed, 98 insertions(+), 85 deletions(-) diff --git a/src/ostree/ot-builtin-fsck.c b/src/ostree/ot-builtin-fsck.c index 2c104c9e..0e95ae9c 100644 --- a/src/ostree/ot-builtin-fsck.c +++ b/src/ostree/ot-builtin-fsck.c @@ -229,6 +229,100 @@ fsck_commit_for_ref (OstreeRepo *repo, return TRUE; } +static gboolean +fsck_one_commit (OstreeRepo *repo, const char *checksum, GVariant *commit, GPtrArray *tombstones, + GCancellable *cancellable, GError **error) +{ + /* If requested, check that all the refs listed in the ref-bindings + * for this commit resolve back to this commit. */ + if (opt_verify_back_refs) + { + g_autoptr(GVariant) metadata = g_variant_get_child_value (commit, 0); + const char *collection_id = NULL; + if (!g_variant_lookup (metadata, + OSTREE_COMMIT_META_KEY_COLLECTION_BINDING, + "&s", + &collection_id)) + collection_id = NULL; + g_autofree const char **refs = NULL; + if (g_variant_lookup (metadata, + OSTREE_COMMIT_META_KEY_REF_BINDING, + "^a&s", + &refs)) + { + for (const char **iter = refs; *iter != NULL; ++iter) + { + g_autofree char *checksum_for_ref = NULL; + if (collection_id != NULL) + { + const OstreeCollectionRef collection_ref = { (char *) collection_id, (char *) *iter }; + if (!ostree_repo_resolve_collection_ref (repo, &collection_ref, + TRUE, + OSTREE_REPO_RESOLVE_REV_EXT_NONE, + &checksum_for_ref, + cancellable, + error)) + return FALSE; + } + else + { + if (!ostree_repo_resolve_rev (repo, *iter, TRUE, + &checksum_for_ref, error)) + return FALSE; + } + if (checksum_for_ref == NULL) + { + if (collection_id != NULL) + return glnx_throw (error, + "Collection–ref (%s, %s) in bindings for commit %s does not exist", + collection_id, *iter, checksum); + else + return glnx_throw (error, + "Ref ‘%s’ in bindings for commit %s does not exist", + *iter, checksum); + } + else if (g_strcmp0 (checksum_for_ref, checksum) != 0) + { + if (collection_id != NULL) + return glnx_throw (error, + "Collection–ref (%s, %s) in bindings for commit %s does not resolve to that commit", + collection_id, *iter, checksum); + else + return glnx_throw (error, + "Ref ‘%s’ in bindings for commit %s does not resolve to that commit", + *iter, checksum); + } + } + } + } + + if (opt_add_tombstones) + { + GError *local_error = NULL; + g_autofree char *parent = ostree_commit_get_parent (commit); + if (parent) + { + g_autoptr(GVariant) parent_commit = NULL; + if (!ostree_repo_load_variant (repo, OSTREE_OBJECT_TYPE_COMMIT, parent, + &parent_commit, &local_error)) + { + if (g_error_matches (local_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND)) + { + g_ptr_array_add (tombstones, g_strdup (checksum)); + g_clear_error (&local_error); + } + else + { + g_propagate_error (error, local_error); + return FALSE; + } + } + } + } + + return TRUE; +} + gboolean ostree_builtin_fsck (int argc, char **argv, OstreeCommandInvocation *invocation, GCancellable *cancellable, GError **error) { @@ -316,91 +410,10 @@ ostree_builtin_fsck (int argc, char **argv, OstreeCommandInvocation *invocation, if (!ostree_repo_load_commit (repo, checksum, &commit, &commitstate, error)) return FALSE; - /* If requested, check that all the refs listed in the ref-bindings - * for this commit resolve back to this commit. */ - if (opt_verify_back_refs) - { - g_autoptr(GVariant) metadata = g_variant_get_child_value (commit, 0); - const char *collection_id = NULL; - if (!g_variant_lookup (metadata, - OSTREE_COMMIT_META_KEY_COLLECTION_BINDING, - "&s", - &collection_id)) - collection_id = NULL; - g_autofree const char **refs = NULL; - if (g_variant_lookup (metadata, - OSTREE_COMMIT_META_KEY_REF_BINDING, - "^a&s", - &refs)) - { - for (const char **iter = refs; *iter != NULL; ++iter) - { - g_autofree char *checksum_for_ref = NULL; - if (collection_id != NULL) - { - const OstreeCollectionRef collection_ref = { (char *) collection_id, (char *) *iter }; - if (!ostree_repo_resolve_collection_ref (repo, &collection_ref, - TRUE, - OSTREE_REPO_RESOLVE_REV_EXT_NONE, - &checksum_for_ref, - cancellable, - error)) - return FALSE; - } - else - { - if (!ostree_repo_resolve_rev (repo, *iter, TRUE, - &checksum_for_ref, error)) - return FALSE; - } - if (checksum_for_ref == NULL) - { - if (collection_id != NULL) - return glnx_throw (error, - "Collection–ref (%s, %s) in bindings for commit %s does not exist", - collection_id, *iter, checksum); - else - return glnx_throw (error, - "Ref ‘%s’ in bindings for commit %s does not exist", - *iter, checksum); - } - else if (g_strcmp0 (checksum_for_ref, checksum) != 0) - { - if (collection_id != NULL) - return glnx_throw (error, - "Collection–ref (%s, %s) in bindings for commit %s does not resolve to that commit", - collection_id, *iter, checksum); - else - return glnx_throw (error, - "Ref ‘%s’ in bindings for commit %s does not resolve to that commit", - *iter, checksum); - } - } - } - } - if (opt_add_tombstones) - { - GError *local_error = NULL; - g_autofree char *parent = ostree_commit_get_parent (commit); - if (parent) - { - g_autoptr(GVariant) parent_commit = NULL; - if (!ostree_repo_load_variant (repo, OSTREE_OBJECT_TYPE_COMMIT, parent, - &parent_commit, &local_error)) - { - if (g_error_matches (local_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND)) - { - g_ptr_array_add (tombstones, g_strdup (checksum)); - g_clear_error (&local_error); - } - else - { - g_propagate_error (error, local_error); - return FALSE; - } - } - } - } + + if (!fsck_one_commit (repo, checksum, commit, tombstones, cancellable, error)) + return FALSE; + if (commitstate & OSTREE_REPO_COMMIT_STATE_PARTIAL) { n_partial++; From 436ff11a008be140828e868ae8ffb6b5ee64f280 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sun, 12 Jun 2022 10:55:14 -0400 Subject: [PATCH 19/56] fsck: Use `load_variant_if_exists` This cleans up error handling here. --- src/ostree/ot-builtin-fsck.c | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/src/ostree/ot-builtin-fsck.c b/src/ostree/ot-builtin-fsck.c index 0e95ae9c..60c78090 100644 --- a/src/ostree/ot-builtin-fsck.c +++ b/src/ostree/ot-builtin-fsck.c @@ -298,25 +298,15 @@ fsck_one_commit (OstreeRepo *repo, const char *checksum, GVariant *commit, GPtrA if (opt_add_tombstones) { - GError *local_error = NULL; g_autofree char *parent = ostree_commit_get_parent (commit); if (parent) { g_autoptr(GVariant) parent_commit = NULL; - if (!ostree_repo_load_variant (repo, OSTREE_OBJECT_TYPE_COMMIT, parent, - &parent_commit, &local_error)) - { - if (g_error_matches (local_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND)) - { - g_ptr_array_add (tombstones, g_strdup (checksum)); - g_clear_error (&local_error); - } - else - { - g_propagate_error (error, local_error); - return FALSE; - } - } + if (!ostree_repo_load_variant_if_exists (repo, OSTREE_OBJECT_TYPE_COMMIT, parent, + &parent_commit, error)) + return FALSE; + if (!parent_commit) + g_ptr_array_add (tombstones, g_strdup (checksum)); } } From eee0eea58b0de7b46c6c5086b041a967efdd330f Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sun, 12 Jun 2022 14:34:02 -0400 Subject: [PATCH 20/56] rust-bindings: Wire up `tests/` Because the source is in a subdirectory, we lose out on cargo target autodiscovery. I noticed this when I edited one of the tests in a way that should have failed, but didn't... --- Cargo.toml | 4 ++++ rust-bindings/tests/util/mod.rs | 5 +---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index a52c27d3..ba61b145 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -29,6 +29,10 @@ features = ["dox"] name = "ostree" path = "rust-bindings/src/lib.rs" +[[test]] +name = "integration" +path = "rust-bindings/tests/tests.rs" + [workspace] members = [".", "rust-bindings/sys"] diff --git a/rust-bindings/tests/util/mod.rs b/rust-bindings/tests/util/mod.rs index 28cca926..c38daeec 100644 --- a/rust-bindings/tests/util/mod.rs +++ b/rust-bindings/tests/util/mod.rs @@ -53,10 +53,7 @@ pub fn create_mtree(repo: &ostree::Repo) -> ostree::MutableTree { assert_eq!(mtree.copy_files().len(), 0); assert_eq!(mtree.copy_subdirs().len(), 0); let file = gio::File::for_path( - Path::new(env!("CARGO_MANIFEST_DIR")) - .join("tests") - .join("data") - .join("test.tar"), + Path::new(env!("CARGO_MANIFEST_DIR")).join("rust-bindings/tests/data/test.tar"), ); repo.write_archive_to_mtree(&file, &mtree, None, true, NONE_CANCELLABLE) .expect("test mtree"); From 46e1db392d59e6956d705037feaf6c64b8a15a16 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 14 Jun 2022 09:50:07 -0400 Subject: [PATCH 21/56] cli/os-init: Port to C99 style General background cleanup; motivated by a recent PR which was using pre-C99 code as a base. --- src/ostree/ot-admin-builtin-os-init.c | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/src/ostree/ot-admin-builtin-os-init.c b/src/ostree/ot-admin-builtin-os-init.c index cb1ec66d..05311532 100644 --- a/src/ostree/ot-admin-builtin-os-init.c +++ b/src/ostree/ot-admin-builtin-os-init.c @@ -35,35 +35,29 @@ static GOptionEntry options[] = { gboolean ot_admin_builtin_os_init (int argc, char **argv, OstreeCommandInvocation *invocation, GCancellable *cancellable, GError **error) { - g_autoptr(GOptionContext) context = NULL; + g_autoptr(GOptionContext) context = g_option_context_new ("OSNAME"); + g_autoptr(OstreeSysroot) sysroot = NULL; - gboolean ret = FALSE; - const char *osname = NULL; - - context = g_option_context_new ("OSNAME"); - if (!ostree_admin_option_context_parse (context, options, &argc, &argv, OSTREE_ADMIN_BUILTIN_FLAG_SUPERUSER | OSTREE_ADMIN_BUILTIN_FLAG_UNLOCKED, invocation, &sysroot, cancellable, error)) - goto out; + return FALSE; if (!ostree_sysroot_ensure_initialized (sysroot, cancellable, error)) - goto out; + return FALSE; if (argc < 2) { ot_util_usage_error (context, "OSNAME must be specified", error); - goto out; + return FALSE; } - osname = argv[1]; + const char *osname = argv[1]; if (!ostree_sysroot_init_osname (sysroot, osname, cancellable, error)) - goto out; + return FALSE; g_print ("ostree/deploy/%s initialized as OSTree root\n", osname); - ret = TRUE; - out: - return ret; + return TRUE; } From 588b07e5542106eb3baef046f7858b9c6f56736f Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 14 Jun 2022 09:50:07 -0400 Subject: [PATCH 22/56] cli/undeploy: Port to C99 style General background cleanup. --- src/ostree/ot-admin-builtin-undeploy.c | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/src/ostree/ot-admin-builtin-undeploy.c b/src/ostree/ot-admin-builtin-undeploy.c index 41155689..c079c83f 100644 --- a/src/ostree/ot-admin-builtin-undeploy.c +++ b/src/ostree/ot-admin-builtin-undeploy.c @@ -34,15 +34,9 @@ static GOptionEntry options[] = { gboolean ot_admin_builtin_undeploy (int argc, char **argv, OstreeCommandInvocation *invocation, GCancellable *cancellable, GError **error) { - g_autoptr(GOptionContext) context = NULL; + g_autoptr(GOptionContext) context = g_option_context_new ("INDEX"); + g_autoptr(OstreeSysroot) sysroot = NULL; - const char *deploy_index_str; - int deploy_index; - g_autoptr(GPtrArray) current_deployments = NULL; - g_autoptr(OstreeDeployment) target_deployment = NULL; - - context = g_option_context_new ("INDEX"); - if (!ostree_admin_option_context_parse (context, options, &argc, &argv, OSTREE_ADMIN_BUILTIN_FLAG_SUPERUSER, invocation, &sysroot, cancellable, error)) @@ -54,12 +48,13 @@ ot_admin_builtin_undeploy (int argc, char **argv, OstreeCommandInvocation *invoc return FALSE; } - current_deployments = ostree_sysroot_get_deployments (sysroot); + g_autoptr(GPtrArray) current_deployments = ostree_sysroot_get_deployments (sysroot); - deploy_index_str = argv[1]; - deploy_index = atoi (deploy_index_str); + const char *deploy_index_str = argv[1]; + int deploy_index = atoi (deploy_index_str); - target_deployment = ot_admin_get_indexed_deployment (sysroot, deploy_index, error); + g_autoptr(OstreeDeployment) target_deployment = + ot_admin_get_indexed_deployment (sysroot, deploy_index, error); if (!target_deployment) return FALSE; From 4e356d0e8fde00ec8b283f0ec946fcdd1b833aa6 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 14 Jun 2022 09:50:07 -0400 Subject: [PATCH 23/56] cli/unlock: Port to C99 style General background cleanup. --- src/ostree/ot-admin-builtin-unlock.c | 29 +++++++++++----------------- 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/src/ostree/ot-admin-builtin-unlock.c b/src/ostree/ot-admin-builtin-unlock.c index 800c0744..f438a13e 100644 --- a/src/ostree/ot-admin-builtin-unlock.c +++ b/src/ostree/ot-admin-builtin-unlock.c @@ -40,33 +40,28 @@ static GOptionEntry options[] = { gboolean ot_admin_builtin_unlock (int argc, char **argv, OstreeCommandInvocation *invocation, GCancellable *cancellable, GError **error) { - gboolean ret = FALSE; - g_autoptr(GOptionContext) context = NULL; + g_autoptr(GOptionContext) context = g_option_context_new (""); + g_autoptr(OstreeSysroot) sysroot = NULL; - OstreeDeployment *booted_deployment = NULL; - OstreeDeploymentUnlockedState target_state; - - context = g_option_context_new (""); - if (!ostree_admin_option_context_parse (context, options, &argc, &argv, OSTREE_ADMIN_BUILTIN_FLAG_SUPERUSER, invocation, &sysroot, cancellable, error)) - goto out; + return FALSE; if (argc > 1) { ot_util_usage_error (context, "This command takes no extra arguments", error); - goto out; + return FALSE; } - booted_deployment = ostree_sysroot_require_booted_deployment (sysroot, error); + OstreeDeployment *booted_deployment = ostree_sysroot_require_booted_deployment (sysroot, error); if (!booted_deployment) - goto out; + return FALSE; + OstreeDeploymentUnlockedState target_state; if (opt_hotfix && opt_transient) { - glnx_throw (error, "Cannot specify both --hotfix and --transient"); - goto out; + return glnx_throw (error, "Cannot specify both --hotfix and --transient"); } else if (opt_hotfix) target_state = OSTREE_DEPLOYMENT_UNLOCKED_HOTFIX; @@ -77,8 +72,8 @@ ot_admin_builtin_unlock (int argc, char **argv, OstreeCommandInvocation *invocat if (!ostree_sysroot_deployment_unlock (sysroot, booted_deployment, target_state, cancellable, error)) - goto out; - + return FALSE; + switch (target_state) { case OSTREE_DEPLOYMENT_UNLOCKED_NONE: @@ -99,7 +94,5 @@ ot_admin_builtin_unlock (int argc, char **argv, OstreeCommandInvocation *invocat break; } - ret = TRUE; - out: - return ret; + return TRUE; } From f8403f46dc19da16f23f93c990a644244801efa0 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 14 Jun 2022 09:50:07 -0400 Subject: [PATCH 24/56] cli/config: Port to C99 style General background cleanup. --- src/ostree/ot-builtin-config.c | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/src/ostree/ot-builtin-config.c b/src/ostree/ot-builtin-config.c index 3c160492..5c1334a5 100644 --- a/src/ostree/ot-builtin-config.c +++ b/src/ostree/ot-builtin-config.c @@ -61,18 +61,10 @@ split_key_string (const char *k, gboolean ostree_builtin_config (int argc, char **argv, OstreeCommandInvocation *invocation, GCancellable *cancellable, GError **error) { - g_autoptr(GOptionContext) context = NULL; + + g_autoptr(GOptionContext) context = g_option_context_new ("(get KEY|set KEY VALUE|unset KEY)"); + g_autoptr(OstreeRepo) repo = NULL; - const char *op; - const char *section_key; - const char *value; - g_autofree char *section = NULL; - g_autofree char *key = NULL; - g_autoptr(GKeyFile) config = NULL; - int correct_argc; - - context = g_option_context_new ("(get KEY|set KEY VALUE|unset KEY)"); - if (!ostree_option_context_parse (context, options, &argc, &argv, invocation, &repo, cancellable, error)) return FALSE; @@ -82,12 +74,11 @@ ostree_builtin_config (int argc, char **argv, OstreeCommandInvocation *invocatio return FALSE; } - op = argv[1]; + const char *op = argv[1]; + int correct_argc = 3; if (!strcmp (op, "set")) correct_argc = 4; - else - correct_argc = 3; if (argc > correct_argc) { @@ -95,6 +86,11 @@ ostree_builtin_config (int argc, char **argv, OstreeCommandInvocation *invocatio return FALSE; } + g_autofree char *section = NULL; + g_autofree char *key = NULL; + g_autoptr(GKeyFile) config = NULL; + const char *section_key; + const char *value; if (!strcmp (op, "set")) { if (opt_group) From 43b712951d8f971454b3a0372cde33487b8c29a6 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 14 Jun 2022 09:50:07 -0400 Subject: [PATCH 25/56] cli/diff: Port to C99 style General background cleanup. --- src/ostree/ot-builtin-diff.c | 62 +++++++++++++++--------------------- 1 file changed, 26 insertions(+), 36 deletions(-) diff --git a/src/ostree/ot-builtin-diff.c b/src/ostree/ot-builtin-diff.c index eecc471b..bd9c357a 100644 --- a/src/ostree/ot-builtin-diff.c +++ b/src/ostree/ot-builtin-diff.c @@ -53,7 +53,6 @@ parse_file_or_commit (OstreeRepo *repo, GCancellable *cancellable, GError **error) { - gboolean ret = FALSE; g_autoptr(GFile) ret_file = NULL; if (g_str_has_prefix (arg, "/") @@ -65,13 +64,11 @@ parse_file_or_commit (OstreeRepo *repo, else { if (!ostree_repo_read_commit (repo, arg, &ret_file, NULL, cancellable, error)) - goto out; + return FALSE; } - ret = TRUE; ot_transfer_out_value (out_file, &ret_file); - out: - return ret; + return TRUE; } static GHashTable * @@ -99,7 +96,6 @@ object_set_total_size (OstreeRepo *repo, GCancellable *cancellable, GError **error) { - gboolean ret = FALSE; GHashTableIter hashiter; gpointer key, value; @@ -116,34 +112,21 @@ object_set_total_size (OstreeRepo *repo, ostree_object_name_deserialize (v, &csum, &objtype); if (!ostree_repo_query_object_storage_size (repo, objtype, csum, &size, cancellable, error)) - goto out; + return FALSE; *out_total += size; } - ret = TRUE; - out: - return ret; + return TRUE; } gboolean ostree_builtin_diff (int argc, char **argv, OstreeCommandInvocation *invocation, GCancellable *cancellable, GError **error) { - gboolean ret = FALSE; - g_autoptr(GOptionContext) context = NULL; + g_autoptr(GOptionContext) context = g_option_context_new ("REV_OR_DIR REV_OR_DIR"); + g_autoptr(OstreeRepo) repo = NULL; - const char *src; - const char *target; - g_autofree char *src_prev = NULL; - g_autoptr(GFile) srcf = NULL; - g_autoptr(GFile) targetf = NULL; - g_autoptr(GPtrArray) modified = NULL; - g_autoptr(GPtrArray) removed = NULL; - g_autoptr(GPtrArray) added = NULL; - - context = g_option_context_new ("REV_OR_DIR REV_OR_DIR"); - if (!ostree_option_context_parse (context, options, &argc, &argv, invocation, &repo, cancellable, error)) - goto out; + return FALSE; if (argc < 2) { @@ -152,9 +135,12 @@ ostree_builtin_diff (int argc, char **argv, OstreeCommandInvocation *invocation, g_free (help); g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED, "REV must be specified"); - goto out; + return FALSE; } + g_autofree char *src_prev = NULL; + const char *src; + const char *target; if (argc == 2) { src_prev = g_strconcat (argv[1], "^", NULL); @@ -170,6 +156,12 @@ ostree_builtin_diff (int argc, char **argv, OstreeCommandInvocation *invocation, if (!opt_stats && !opt_fs_diff) opt_fs_diff = TRUE; + g_autoptr(GFile) srcf = NULL; + g_autoptr(GFile) targetf = NULL; + g_autoptr(GPtrArray) modified = NULL; + g_autoptr(GPtrArray) removed = NULL; + g_autoptr(GPtrArray) added = NULL; + if (opt_fs_diff) { OstreeDiffFlags diff_flags = OSTREE_DIFF_FLAGS_NONE; @@ -178,9 +170,9 @@ ostree_builtin_diff (int argc, char **argv, OstreeCommandInvocation *invocation, diff_flags |= OSTREE_DIFF_FLAGS_IGNORE_XATTRS; if (!parse_file_or_commit (repo, src, &srcf, cancellable, error)) - goto out; + return FALSE; if (!parse_file_or_commit (repo, target, &targetf, cancellable, error)) - goto out; + return FALSE; modified = g_ptr_array_new_with_free_func ((GDestroyNotify)ostree_diff_item_unref); removed = g_ptr_array_new_with_free_func ((GDestroyNotify)g_object_unref); @@ -189,7 +181,7 @@ ostree_builtin_diff (int argc, char **argv, OstreeCommandInvocation *invocation, OstreeDiffDirsOptions diff_opts = { opt_owner_uid, opt_owner_gid }; if (!ostree_diff_dirs_with_options (diff_flags, srcf, targetf, modified, removed, added, &diff_opts, cancellable, error)) - goto out; + return FALSE; ostree_diff_print (srcf, targetf, modified, removed, added); } @@ -207,14 +199,14 @@ ostree_builtin_diff (int argc, char **argv, OstreeCommandInvocation *invocation, guint64 total_common; if (!ostree_repo_resolve_rev (repo, src, FALSE, &rev_a, error)) - goto out; + return FALSE; if (!ostree_repo_resolve_rev (repo, target, FALSE, &rev_b, error)) - goto out; + return FALSE; if (!ostree_repo_traverse_commit (repo, rev_a, 0, &reachable_a, cancellable, error)) - goto out; + return FALSE; if (!ostree_repo_traverse_commit (repo, rev_b, 0, &reachable_b, cancellable, error)) - goto out; + return FALSE; a_size = g_hash_table_size (reachable_a); b_size = g_hash_table_size (reachable_b); @@ -227,12 +219,10 @@ ostree_builtin_diff (int argc, char **argv, OstreeCommandInvocation *invocation, if (!object_set_total_size (repo, reachable_intersection, &total_common, cancellable, error)) - goto out; + return FALSE; size = g_format_size_full (total_common, 0); g_print ("Common Object Size: %s\n", size); } - ret = TRUE; - out: - return ret; + return TRUE; } From a0ae2f9156982a94b2ad163c5bf61184c1ffcb5e Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 14 Jun 2022 09:50:07 -0400 Subject: [PATCH 26/56] cli/gpg-sign: Port to C99 style General background cleanup. --- src/ostree/ot-builtin-gpg-sign.c | 50 +++++++++++++------------------- 1 file changed, 20 insertions(+), 30 deletions(-) diff --git a/src/ostree/ot-builtin-gpg-sign.c b/src/ostree/ot-builtin-gpg-sign.c index bfb1d902..48aa4699 100644 --- a/src/ostree/ot-builtin-gpg-sign.c +++ b/src/ostree/ot-builtin-gpg-sign.c @@ -201,62 +201,52 @@ out: gboolean ostree_builtin_gpg_sign (int argc, char **argv,OstreeCommandInvocation *invocation, GCancellable *cancellable, GError **error) { - g_autoptr(GOptionContext) context = NULL; + + g_autoptr(GOptionContext) context = g_option_context_new ("COMMIT KEY-ID..."); + g_autoptr(OstreeRepo) repo = NULL; - g_autofree char *resolved_commit = NULL; - const char *commit; - char **key_ids; - int n_key_ids, ii; - gboolean ret = FALSE; - - context = g_option_context_new ("COMMIT KEY-ID..."); - if (!ostree_option_context_parse (context, options, &argc, &argv, invocation, &repo, cancellable, error)) - goto out; + return FALSE; if (argc < 2) { usage_error (context, "Need a COMMIT to sign", error); - goto out; + return FALSE; } if (argc < 3) { usage_error (context, "Need at least one GPG KEY-ID to sign with", error); - goto out; + return FALSE; } - commit = argv[1]; - key_ids = argv + 2; - n_key_ids = argc - 2; + const char *commit = argv[1]; + char **key_ids = argv + 2; + int n_key_ids = argc - 2; + g_autofree char *resolved_commit = NULL; if (!ostree_repo_resolve_rev (repo, commit, FALSE, &resolved_commit, error)) - goto out; + return FALSE; if (opt_delete) { guint n_deleted = 0; - if (delete_signatures (repo, resolved_commit, - (const char * const *) key_ids, n_key_ids, - &n_deleted, cancellable, error)) - { - g_print ("Signatures deleted: %u\n", n_deleted); - ret = TRUE; - } + if (!delete_signatures (repo, resolved_commit, + (const char * const *) key_ids, n_key_ids, + &n_deleted, cancellable, error)) + return FALSE; - goto out; + g_print ("Signatures deleted: %u\n", n_deleted); + return TRUE; } - for (ii = 0; ii < n_key_ids; ii++) + for (int ii = 0; ii < n_key_ids; ii++) { if (!ostree_repo_sign_commit (repo, resolved_commit, key_ids[ii], opt_gpg_homedir, cancellable, error)) - goto out; + return FALSE; } - ret = TRUE; - -out: - return ret; + return TRUE; } From 2f1c9a727e598f206eae08412de2a872c0a9dedf Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 14 Jun 2022 09:50:07 -0400 Subject: [PATCH 27/56] cli/remote-list: Port to C99 style General background cleanup. --- src/ostree/ot-remote-builtin-list.c | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/src/ostree/ot-remote-builtin-list.c b/src/ostree/ot-remote-builtin-list.c index 552e5359..b8e875d4 100644 --- a/src/ostree/ot-remote-builtin-list.c +++ b/src/ostree/ot-remote-builtin-list.c @@ -37,45 +37,38 @@ static GOptionEntry option_entries[] = { gboolean ot_remote_builtin_list (int argc, char **argv, OstreeCommandInvocation *invocation, GCancellable *cancellable, GError **error) { - g_autoptr(GOptionContext) context = NULL; + g_autoptr(GOptionContext) context = g_option_context_new (""); + g_autoptr(OstreeRepo) repo = NULL; - g_auto(GStrv) remotes = NULL; - guint ii, n_remotes = 0; - gboolean ret = FALSE; - - context = g_option_context_new (""); - if (!ostree_option_context_parse (context, option_entries, &argc, &argv, invocation, &repo, cancellable, error)) - goto out; + return FALSE; - remotes = ostree_repo_remote_list (repo, &n_remotes); + guint n_remotes = 0; + g_auto(GStrv) remotes = ostree_repo_remote_list (repo, &n_remotes); if (opt_show_urls) { int max_length = 0; - for (ii = 0; ii < n_remotes; ii++) + for (guint ii = 0; ii < n_remotes; ii++) max_length = MAX (max_length, strlen (remotes[ii])); - for (ii = 0; ii < n_remotes; ii++) + for (guint ii = 0; ii < n_remotes; ii++) { g_autofree char *remote_url = NULL; if (!ostree_repo_remote_get_url (repo, remotes[ii], &remote_url, error)) - goto out; + return FALSE; g_print ("%-*s %s\n", max_length, remotes[ii], remote_url); } } else { - for (ii = 0; ii < n_remotes; ii++) + for (guint ii = 0; ii < n_remotes; ii++) g_print ("%s\n", remotes[ii]); } - ret = TRUE; - - out: - return ret; + return TRUE; } From 9bdf3861ad08528563456d8cc2748d8d86a37d16 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 14 Jun 2022 09:50:07 -0400 Subject: [PATCH 28/56] cli/refs: Port to C99 style General background cleanup. --- src/ostree/ot-remote-builtin-refs.c | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/src/ostree/ot-remote-builtin-refs.c b/src/ostree/ot-remote-builtin-refs.c index d778fe60..29c39328 100644 --- a/src/ostree/ot-remote-builtin-refs.c +++ b/src/ostree/ot-remote-builtin-refs.c @@ -39,34 +39,30 @@ static GOptionEntry option_entries[] = { gboolean ot_remote_builtin_refs (int argc, char **argv, OstreeCommandInvocation *invocation, GCancellable *cancellable, GError **error) { - g_autoptr(GOptionContext) context = NULL; + g_autoptr(GOptionContext) context = g_option_context_new ("NAME"); + g_autoptr(OstreeRepo) repo = NULL; - const char *remote_name; - gboolean ret = FALSE; - g_autoptr(GHashTable) refs = NULL; - - context = g_option_context_new ("NAME"); - if (!ostree_option_context_parse (context, option_entries, &argc, &argv, invocation, &repo, cancellable, error)) - goto out; + return FALSE; if (argc < 2) { ot_util_usage_error (context, "NAME must be specified", error); - goto out; + return FALSE; } if (opt_cache_dir) { if (!ostree_repo_set_cache_dir (repo, AT_FDCWD, opt_cache_dir, cancellable, error)) - goto out; + return FALSE; } - remote_name = argv[1]; + const char *remote_name = argv[1]; + g_autoptr(GHashTable) refs = NULL; if (!ostree_repo_remote_list_refs (repo, remote_name, &refs, cancellable, error)) - goto out; + return FALSE; else { g_autoptr(GList) ordered_keys = NULL; @@ -81,7 +77,5 @@ ot_remote_builtin_refs (int argc, char **argv, OstreeCommandInvocation *invocati } } - ret = TRUE; -out: - return ret; + return TRUE; } From ce428c1f60b6ec2a831b817c909819482e56bcb8 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Fri, 17 Jun 2022 14:15:35 +0100 Subject: [PATCH 29/56] test-basic-c: Don't assert that extended attributes are available Not all filesystems support extended attributes. This test uses /var/tmp to try to get an extended-attributes-capable filesystem, but that might not succeed. Signed-off-by: Simon McVittie --- tests/test-basic-c.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/test-basic-c.c b/tests/test-basic-c.c index 1886feb2..fc995298 100644 --- a/tests/test-basic-c.c +++ b/tests/test-basic-c.c @@ -514,8 +514,15 @@ test_read_xattrs (void) g_assert_no_error (local_error); int r = fsetxattr (tmpd.fd, "user.ostreetesting", value, sizeof (value), 0); - g_assert_cmpint (r, ==, 0); - + + if (r != 0) + { + g_autofree gchar *message = g_strdup_printf ("Unable to set extended attributes in /var/tmp: %s", + g_strerror (errno)); + g_test_skip (message); + return; + } + g_autoptr(GVariant) new_xattrs = ostree_fs_get_all_xattrs (tmpd.fd, NULL, error); g_assert_no_error (local_error); From 7ee2d1b1376e009b530ad8560b05e0e52a337f13 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Tue, 21 Jun 2022 15:10:18 -0400 Subject: [PATCH 30/56] Drop `.packit.yaml` This repo is already hooked to the @CoreOS/continuous COPR repo for multi-arch RPM builds of the latest commits. It's possible we will use Packit in the future to help with the release process. But for now, let's just drop this file since it's not needed and the Packit failures are marking git main CI as failed. --- .packit.yaml | 24 ------------------------ 1 file changed, 24 deletions(-) delete mode 100644 .packit.yaml diff --git a/.packit.yaml b/.packit.yaml deleted file mode 100644 index 68a1c724..00000000 --- a/.packit.yaml +++ /dev/null @@ -1,24 +0,0 @@ -# build into f34-coreos-continuous on every commit to main -jobs: - - job: copr_build - trigger: commit - metadata: - branch: main - owner: "@CoreOS" - project: continuous - targets: - - centos-stream-8-aarch64 - - centos-stream-8-x86_64 - - fedora-all-aarch64 - - fedora-all-s390x - - fedora-all -specfile_path: ostree.spec -actions: - create-archive: - - "./ci/make-git-snapshot.sh" - - "bash -c 'ls ostree-*.tar'" - # https://packit.dev/faq/#how-can-i-download-rpm-spec-file-if-it-is-not-part-of-upstream-repository - post-upstream-clone: - - "curl -LO https://src.fedoraproject.org/rpms/ostree/raw/rawhide/f/ostree.spec" - # we don't want any downstream patches - - "sed -ie 's/^Patch/# Patch/g' ostree.spec" From 3bc59a52068ed52e063d99ebe2492eae6f6f92c0 Mon Sep 17 00:00:00 2001 From: Huijing Hei Date: Thu, 2 Jun 2022 15:30:20 +0800 Subject: [PATCH 31/56] RFE: Add a hidden option to `ostree admin kargs edit-in-place` to update all existing deployments in place Example: $ sudo ostree admin kargs edit-in-place --append-if-missing=rw See https://github.com/ostreedev/ostree/issues/2617 This will not add duplicate key, if there is `TESTARG=VAL1` in the kernel arguments, `--append-if-missing=TESTARG=VAL2` will be ignored. --- Makefile-libostree.am | 3 + Makefile-ostree.am | 3 + Makefile-tests.am | 1 + apidoc/ostree-sections.txt | 1 + src/libostree/libostree-devel.sym | 4 + src/libostree/ostree-kernel-args.c | 25 ++++ src/libostree/ostree-kernel-args.h | 4 + src/ostree/ot-admin-builtin-kargs.c | 134 ++++++++++++++++++ src/ostree/ot-admin-builtins.h | 1 + .../ot-admin-kargs-builtin-edit-in-place.c | 80 +++++++++++ src/ostree/ot-admin-kargs-builtins.h | 35 +++++ src/ostree/ot-builtin-admin.c | 3 + tests/test-admin-kargs.sh | 44 ++++++ 13 files changed, 338 insertions(+) create mode 100644 src/ostree/ot-admin-builtin-kargs.c create mode 100644 src/ostree/ot-admin-kargs-builtin-edit-in-place.c create mode 100644 src/ostree/ot-admin-kargs-builtins.h create mode 100755 tests/test-admin-kargs.sh diff --git a/Makefile-libostree.am b/Makefile-libostree.am index f93f712a..dd7ed8df 100644 --- a/Makefile-libostree.am +++ b/Makefile-libostree.am @@ -174,6 +174,9 @@ 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= diff --git a/Makefile-ostree.am b/Makefile-ostree.am index 0fe2c5f8..fb377075 100644 --- a/Makefile-ostree.am +++ b/Makefile-ostree.am @@ -73,6 +73,7 @@ ostree_SOURCES += \ src/ostree/ot-admin-builtin-boot-complete.c \ src/ostree/ot-admin-builtin-undeploy.c \ src/ostree/ot-admin-builtin-instutil.c \ + src/ostree/ot-admin-builtin-kargs.c \ src/ostree/ot-admin-builtin-cleanup.c \ src/ostree/ot-admin-builtin-os-init.c \ src/ostree/ot-admin-builtin-set-origin.c \ @@ -88,6 +89,8 @@ ostree_SOURCES += \ src/ostree/ot-admin-instutil-builtins.h \ src/ostree/ot-admin-functions.h \ src/ostree/ot-admin-functions.c \ + src/ostree/ot-admin-kargs-builtins.h \ + src/ostree/ot-admin-kargs-builtin-edit-in-place.c \ $(NULL) # Remote subcommand diff --git a/Makefile-tests.am b/Makefile-tests.am index 29de6c7a..c87893ee 100644 --- a/Makefile-tests.am +++ b/Makefile-tests.am @@ -114,6 +114,7 @@ _installed_or_uninstalled_test_scripts = \ tests/test-admin-pull-deploy-split.sh \ tests/test-admin-locking.sh \ tests/test-admin-deploy-clean.sh \ + tests/test-admin-kargs.sh \ tests/test-reset-nonlinear.sh \ tests/test-oldstyle-partial.sh \ tests/test-delta.sh \ diff --git a/apidoc/ostree-sections.txt b/apidoc/ostree-sections.txt index adf52557..5af8e687 100644 --- a/apidoc/ostree-sections.txt +++ b/apidoc/ostree-sections.txt @@ -727,6 +727,7 @@ ostree_kernel_args_replace_argv ostree_kernel_args_append ostree_kernel_args_append_argv ostree_kernel_args_append_argv_filtered +ostree_kernel_args_append_if_missing ostree_kernel_args_new_replace ostree_kernel_args_delete ostree_kernel_args_delete_key_entry diff --git a/src/libostree/libostree-devel.sym b/src/libostree/libostree-devel.sym index eef5cba0..54945eca 100644 --- a/src/libostree/libostree-devel.sym +++ b/src/libostree/libostree-devel.sym @@ -20,6 +20,10 @@ - uncomment the include in Makefile-libostree.am */ +LIBOSTREE_2022.5 { +global: + ostree_kernel_args_append_if_missing; +} LIBOSTREE_2022.4; /* 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-kernel-args.c b/src/libostree/ostree-kernel-args.c index 40f11e99..08dcf992 100644 --- a/src/libostree/ostree-kernel-args.c +++ b/src/libostree/ostree-kernel-args.c @@ -804,3 +804,28 @@ ostree_kernel_args_get_last_value (OstreeKernelArgs *kargs, const char *key) const OstreeKernelArgsEntry *e = entries->pdata[entries->len-1]; return _ostree_kernel_args_entry_get_value (e); } + +/** + * ostree_kernel_args_append_if_missing: + * @kargs: a OstreeKernelArgs instance + * @arg: key or key/value pair to be added + * + * Appends @arg which is in the form of key=value pair to the hash table kargs->table + * (appends to the value list if key is not in the hash table) + * and appends key to kargs->order if it is not in the hash table. + * + * Since: 2022.5 + **/ +void +ostree_kernel_args_append_if_missing (OstreeKernelArgs *kargs, + const char *arg) +{ + g_autofree char *key = g_strdup (arg); + split_keyeq (key); + + // Don't insert a duplicate key. + if (g_hash_table_contains (kargs->table, key)) + return; + + ostree_kernel_args_append (kargs, arg); +} diff --git a/src/libostree/ostree-kernel-args.h b/src/libostree/ostree-kernel-args.h index dde0312f..2e1b249a 100644 --- a/src/libostree/ostree-kernel-args.h +++ b/src/libostree/ostree-kernel-args.h @@ -130,4 +130,8 @@ char **ostree_kernel_args_to_strv (OstreeKernelArgs *kargs); _OSTREE_PUBLIC char *ostree_kernel_args_to_string (OstreeKernelArgs *kargs); +_OSTREE_PUBLIC +void ostree_kernel_args_append_if_missing (OstreeKernelArgs *kargs, + const char *arg); + G_END_DECLS diff --git a/src/ostree/ot-admin-builtin-kargs.c b/src/ostree/ot-admin-builtin-kargs.c new file mode 100644 index 00000000..4afef294 --- /dev/null +++ b/src/ostree/ot-admin-builtin-kargs.c @@ -0,0 +1,134 @@ +/* + * Copyright (C) 2015 Colin Walters + * Copyright (C) 2022 Huijing Hei + * + * 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, see . + */ + +#include "config.h" + +#include "ot-main.h" +#include "ot-admin-builtins.h" +#include "ot-admin-functions.h" +#include "ot-admin-kargs-builtins.h" +#include "otutil.h" + +#include + +static OstreeCommand admin_kargs_subcommands[] = { + { "edit-in-place", OSTREE_BUILTIN_FLAG_NO_REPO | OSTREE_BUILTIN_FLAG_HIDDEN, + ot_admin_kargs_builtin_edit_in_place, + "Set new kernel command line arguments in place (applies to all deployments by default)" }, + { NULL, 0, NULL, NULL } +}; + +static GOptionContext * +ostree_admin_kargs_option_context_new_with_commands (void) +{ + OstreeCommand *command = admin_kargs_subcommands; + GOptionContext *context = g_option_context_new ("COMMAND"); + + g_autoptr(GString) summary = g_string_new ("Builtin \"admin kargs\" Commands:"); + + while (command->name != NULL) + { + if ((command->flags & OSTREE_BUILTIN_FLAG_HIDDEN) == 0) + { + g_string_append_printf (summary, "\n %-24s", command->name); + if (command->description != NULL) + g_string_append_printf (summary, "%s", command->description); + } + + command++; + } + + g_option_context_set_summary (context, summary->str); + + return context; +} + +gboolean +ot_admin_builtin_kargs (int argc, char **argv, OstreeCommandInvocation *invocation, GCancellable *cancellable, GError **error) +{ + const char *subcommand_name = NULL; + int in, out; + for (in = 1, out = 1; in < argc; in++, out++) + { + /* The non-option is the command, take it out of the arguments */ + if (argv[in][0] != '-') + { + if (subcommand_name == NULL) + { + subcommand_name = argv[in]; + out--; + continue; + } + } + + else if (g_str_equal (argv[in], "--")) + { + break; + } + + argv[out] = argv[in]; + } + + argc = out; + + OstreeCommand *subcommand = admin_kargs_subcommands; + while (subcommand->name) + { + if (g_strcmp0 (subcommand_name, subcommand->name) == 0) + break; + subcommand++; + } + + if (!subcommand->name) + { + g_autoptr(GOptionContext) context = + ostree_admin_kargs_option_context_new_with_commands (); + + /* This will not return for some options (e.g. --version). */ + if (ostree_admin_option_context_parse (context, NULL, &argc, &argv, + OSTREE_ADMIN_BUILTIN_FLAG_NO_SYSROOT, + invocation, NULL, cancellable, error)) + { + if (subcommand_name == NULL) + { + g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED, + "No \"admin kargs\" subcommand specified"); + } + else + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_NOT_SUPPORTED, + "Unknown \"admin kargs\" subcommand '%s'", subcommand_name); + } + } + + g_autofree char *help = g_option_context_get_help (context, FALSE, NULL); + g_printerr ("%s", help); + return FALSE; + } + + g_autofree char *prgname = g_strdup_printf ("%s %s", g_get_prgname (), subcommand_name); + g_set_prgname (prgname); + + OstreeCommandInvocation sub_invocation = { .command = subcommand }; + if (!subcommand->fn (argc, argv, &sub_invocation, cancellable, error)) + return FALSE; + + return TRUE; +} diff --git a/src/ostree/ot-admin-builtins.h b/src/ostree/ot-admin-builtins.h index 8d9451be..8bac1c56 100644 --- a/src/ostree/ot-admin-builtins.h +++ b/src/ostree/ot-admin-builtins.h @@ -46,6 +46,7 @@ BUILTINPROTO(set_origin); BUILTINPROTO(diff); BUILTINPROTO(switch); BUILTINPROTO(upgrade); +BUILTINPROTO(kargs); #undef BUILTINPROTO diff --git a/src/ostree/ot-admin-kargs-builtin-edit-in-place.c b/src/ostree/ot-admin-kargs-builtin-edit-in-place.c new file mode 100644 index 00000000..40ada02f --- /dev/null +++ b/src/ostree/ot-admin-kargs-builtin-edit-in-place.c @@ -0,0 +1,80 @@ +/* + * Copyright (C) 2014 Colin Walters + * Copyright (C) 2022 Huijing Hei + * + * This program 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 licence 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, see . + */ + +#include "config.h" + +#include "ot-main.h" +#include "ot-admin-kargs-builtins.h" + +#include "ostree.h" +#include "otutil.h" + +static char **opt_kargs_edit_in_place_append; + +static GOptionEntry options[] = { + { "append-if-missing", 0, 0, G_OPTION_ARG_STRING_ARRAY, &opt_kargs_edit_in_place_append, "Append kernel arguments if they do not exist", "NAME=VALUE" }, + { NULL } +}; + +gboolean +ot_admin_kargs_builtin_edit_in_place (int argc, char **argv, OstreeCommandInvocation *invocation, GCancellable *cancellable, GError **error) +{ + g_autoptr(OstreeSysroot) sysroot = NULL; + + g_autoptr(GOptionContext) context = g_option_context_new ("ARGS"); + + if (!ostree_admin_option_context_parse (context, options, &argc, &argv, + OSTREE_ADMIN_BUILTIN_FLAG_SUPERUSER, + invocation, &sysroot, cancellable, error)) + return FALSE; + + g_autoptr(GPtrArray) deployments = ostree_sysroot_get_deployments (sysroot); + if (deployments->len == 0) + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, + "Unable to find a deployment in sysroot"); + return FALSE; + } + + // set kargs for each deployment + for (guint i = 0; i < deployments->len; i++) + { + OstreeDeployment *deployment = deployments->pdata[i]; + OstreeBootconfigParser *bootconfig = ostree_deployment_get_bootconfig (deployment); + g_autoptr(OstreeKernelArgs) kargs = ostree_kernel_args_from_string (ostree_bootconfig_parser_get (bootconfig, "options")); + + if (opt_kargs_edit_in_place_append) + { + for (char **strviter = opt_kargs_edit_in_place_append; strviter && *strviter; strviter++) + { + const char *arg = *strviter; + ostree_kernel_args_append_if_missing (kargs, arg); + } + } + + g_auto(GStrv) kargs_strv = ostree_kernel_args_to_strv (kargs); + + if (!ostree_sysroot_deployment_set_kargs (sysroot, deployment, + kargs_strv, + cancellable, error)) + return FALSE; + + } + + return TRUE; +} diff --git a/src/ostree/ot-admin-kargs-builtins.h b/src/ostree/ot-admin-kargs-builtins.h new file mode 100644 index 00000000..f3837c34 --- /dev/null +++ b/src/ostree/ot-admin-kargs-builtins.h @@ -0,0 +1,35 @@ +/* + * Copyright (C) 2014 Colin Walters + * Copyright (C) 2022 Huijing Hei + * + * 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, see . + */ + +#pragma once + +#include + +G_BEGIN_DECLS + +#define BUILTINPROTO(name) gboolean ot_admin_kargs_builtin_ ## name (int argc, char **argv, \ + OstreeCommandInvocation *invocation, \ + GCancellable *cancellable, GError **error) + +BUILTINPROTO(edit_in_place); + +#undef BUILTINPROTO + +G_END_DECLS diff --git a/src/ostree/ot-builtin-admin.c b/src/ostree/ot-builtin-admin.c index af09a614..503fb9a7 100644 --- a/src/ostree/ot-builtin-admin.c +++ b/src/ostree/ot-builtin-admin.c @@ -76,6 +76,9 @@ static OstreeCommand admin_subcommands[] = { { "upgrade", OSTREE_BUILTIN_FLAG_NO_REPO, ot_admin_builtin_upgrade, "Construct new tree from current origin and deploy it, if it changed" }, + { "kargs", OSTREE_BUILTIN_FLAG_NO_REPO, + ot_admin_builtin_kargs, + "Change kernel arguments" }, { NULL, 0, NULL, NULL } }; diff --git a/tests/test-admin-kargs.sh b/tests/test-admin-kargs.sh new file mode 100755 index 00000000..afcfc05a --- /dev/null +++ b/tests/test-admin-kargs.sh @@ -0,0 +1,44 @@ +#!/bin/bash +# +# Copyright (C) 2011 Colin Walters +# Copyright (C) 2022 Huijing Hei +# +# 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, see . + +set -euo pipefail + +. $(dirname $0)/libtest.sh + +# Exports OSTREE_SYSROOT so --sysroot not needed. +setup_os_repository "archive" "syslinux" + +echo "1..2" + +${CMD_PREFIX} ostree --repo=sysroot/ostree/repo pull-local --remote=testos testos-repo testos/buildmain/x86_64-runtime +${CMD_PREFIX} ostree admin deploy --karg=root=LABEL=MOO --karg=quiet --os=testos testos:testos/buildmain/x86_64-runtime +${CMD_PREFIX} ostree admin kargs edit-in-place --append-if-missing=TESTARG=TESTVALUE --append-if-missing=ARGWITHOUTKEY testos:testos/buildmain/x86_64-runtime + +assert_file_has_content sysroot/boot/loader/entries/ostree-1-testos.conf 'options.*TESTARG=TESTVALUE .*ARGWITHOUTKEY' + +echo "ok kargs edit-in-place (basic)" + +${CMD_PREFIX} ostree admin kargs edit-in-place --append-if-missing=quiet testos:testos/buildmain/x86_64-runtime +assert_not_file_has_content sysroot/boot/loader/entries/ostree-1-testos.conf 'quiet$' + +${CMD_PREFIX} ostree admin kargs edit-in-place --append-if-missing=TESTARG=TESTVALUE testos:testos/buildmain/x86_64-runtime +assert_not_file_has_content sysroot/boot/loader/entries/ostree-1-testos.conf 'TESTARG=TESTVALUE$' + +echo "ok kargs edit-in-place (duplicate)" From ed98a7904c9536984567412daf9cc517555a81e4 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 23 Jun 2022 14:43:55 -0400 Subject: [PATCH 32/56] ci/rust: Enable `cap-std-apis` in default build, add a no-feature build Our CI was missing coverage of `cap-std-apis`. --- .github/workflows/rust.yml | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index b9c972e2..74e395ee 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -11,7 +11,7 @@ on: env: CARGO_TERM_COLOR: always - CARGO_PROJECT_FEATURES: "v2021_5" + CARGO_PROJECT_FEATURES: "v2021_5,cap-std-apis" # TODO: Automatically query this from the C side LATEST_LIBOSTREE: "v2022_5" # Minimum supported Rust version (MSRV) @@ -49,6 +49,17 @@ jobs: uses: Swatinem/rust-cache@ce325b60658c1b38465c06cc965b79baf32c1e72 - name: cargo build run: cargo build --features=${{ env['CARGO_PROJECT_FEATURES'] }} + build-no-features: + runs-on: ubuntu-latest + container: quay.io/coreos-assembler/fcos-buildroot:testing-devel + steps: + - uses: actions/checkout@v2 + - name: Cache Dependencies + uses: Swatinem/rust-cache@ce325b60658c1b38465c06cc965b79baf32c1e72 + - name: Build + run: cargo test --no-run + - name: Run tests + run: cargo test --verbose build-git-libostree: runs-on: ubuntu-latest container: quay.io/coreos-assembler/fcos-buildroot:testing-devel From 001839b35f3c02d8df34a127eb99bd17af30ee15 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 23 Jun 2022 14:44:33 -0400 Subject: [PATCH 33/56] ci/rust: Change MSRV to `cargo check` No reason to codegen just to throw it away. We could test here too, but eh. --- .github/workflows/rust.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 74e395ee..affeb42b 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -47,8 +47,8 @@ jobs: default: true - name: Cache Dependencies uses: Swatinem/rust-cache@ce325b60658c1b38465c06cc965b79baf32c1e72 - - name: cargo build - run: cargo build --features=${{ env['CARGO_PROJECT_FEATURES'] }} + - name: cargo check + run: cargo check --features=${{ env['CARGO_PROJECT_FEATURES'] }} build-no-features: runs-on: ubuntu-latest container: quay.io/coreos-assembler/fcos-buildroot:testing-devel From 37d0ca41b65b31040f736be5a3fdd03b8393dc77 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 23 Jun 2022 14:58:00 -0400 Subject: [PATCH 34/56] Fix clippy lint in cap-std bits --- rust-bindings/src/repo.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-bindings/src/repo.rs b/rust-bindings/src/repo.rs index 5d34f98b..a8ddbca1 100644 --- a/rust-bindings/src/repo.rs +++ b/rust-bindings/src/repo.rs @@ -153,7 +153,7 @@ impl Repo { /// Borrow the directory file descriptor for this repository. #[cfg(feature = "cap-std-apis")] - pub fn dfd_borrow<'a>(&'a self) -> io_lifetimes::BorrowedFd<'a> { + pub fn dfd_borrow(&self) -> io_lifetimes::BorrowedFd { unsafe { io_lifetimes::BorrowedFd::borrow_raw_fd(self.dfd()) } } From b87c8a8e23b77553e7888bb484cddea9fcf5e7b5 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 23 Jun 2022 14:56:30 -0400 Subject: [PATCH 35/56] rust: Bump semver to 0.15 Prep for some breaking changes. --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 5b6cce6a..f4544b39 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -8,7 +8,7 @@ license = "MIT" name = "ostree" readme = "rust-bindings/README.md" repository = "https://github.com/ostreedev/ostree" -version = "0.14.0" +version = "0.15.0" exclude = [ "/*.am", "/apidoc", "/autogen.sh", "/bash", "/bsdiff", From 63499747b9380a85ebb7ebf2315ac2fc7116cf1f Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 23 Jun 2022 14:25:42 -0400 Subject: [PATCH 36/56] Bump to cap-std 0.25 and io-lifetimes 0.7 Prep for bumping ostree-rs-ext, which will help bump rpm-ostree, which will get it out of having two copies of rustix. --- Cargo.toml | 6 +++--- rust-bindings/src/repo.rs | 6 ++++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index f4544b39..4b865f2e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -38,8 +38,8 @@ members = [".", "rust-bindings/sys"] [dependencies] bitflags = "1.2.1" -cap-std = { version = "0.24", optional = true} -io-lifetimes = { version = "0.5", optional = true} +cap-std = { version = "0.25", optional = true} +io-lifetimes = { version = "0.7", optional = true} ffi = { package = "ostree-sys", path = "rust-bindings/sys", version = "0.10.0" } gio = "0.14" glib = "0.14.4" @@ -53,7 +53,7 @@ thiserror = "1.0.20" maplit = "1.0.2" openat = "0.1.19" tempfile = "3" -cap-tempfile = "0.24" +cap-tempfile = "0.25" [features] cap-std-apis = ["cap-std", "io-lifetimes", "v2017_10"] diff --git a/rust-bindings/src/repo.rs b/rust-bindings/src/repo.rs index a8ddbca1..6947f49b 100644 --- a/rust-bindings/src/repo.rs +++ b/rust-bindings/src/repo.rs @@ -154,13 +154,15 @@ impl Repo { /// Borrow the directory file descriptor for this repository. #[cfg(feature = "cap-std-apis")] pub fn dfd_borrow(&self) -> io_lifetimes::BorrowedFd { - unsafe { io_lifetimes::BorrowedFd::borrow_raw_fd(self.dfd()) } + unsafe { io_lifetimes::BorrowedFd::borrow_raw(self.dfd()) } } /// Return a new `cap-std` directory reference for this repository. #[cfg(feature = "cap-std-apis")] pub fn dfd_as_dir(&self) -> std::io::Result { - cap_std::fs::Dir::reopen_dir(&self.dfd_borrow()) + use io_lifetimes::AsFd; + let dfd = self.dfd_borrow(); + cap_std::fs::Dir::reopen_dir(&dfd.as_fd()) } /// Find all objects reachable from a commit. From 7814d9339e7ab62b8b8cdbd866deb25182ce2a42 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Thu, 23 Jun 2022 15:50:39 -0400 Subject: [PATCH 37/56] tests/inst/destructive: stop disabling fedora-coreos-pinger It was removed from FCOS: https://github.com/coreos/fedora-coreos-tracker/issues/770 --- tests/inst/src/destructive.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/inst/src/destructive.rs b/tests/inst/src/destructive.rs index 6cce4b7a..da188bcb 100644 --- a/tests/inst/src/destructive.rs +++ b/tests/inst/src/destructive.rs @@ -576,10 +576,10 @@ pub(crate) fn itest_transactionality() -> Result<()> { )?; if firstrun { - // Also disable some services (like zincati) because we don't want automatic updates + // Also disable zincati because we don't want automatic updates // in our reboots, and it currently fails to start. The less // we have in each reboot, the faster reboots are. - bash!("systemctl disable --now zincati fedora-coreos-pinger")?; + bash!("systemctl disable --now zincati")?; // And prepare for updates bash!("rpm-ostree cleanup -pr")?; generate_update(&commit)?; From be2075eef00640f85d62692f061f4b0fe7a12bfd Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 23 Jun 2022 17:36:46 -0400 Subject: [PATCH 38/56] repo: Metadata return values from `load_file` are not nullable The pattern this API uses in C is to allow the input parameters pointer targets to be `NULL`, and it doesn't return values in that case. A further complexity here is that the API will still return `NULL` for symbolic links. But Rust can't express this pattern as is, so we were always returning values but in `Option` wrappers that the caller needed to unwrap for the metadata. (We really want an even more efficient API here that avoids the glib objects entirely, e.g. no reason not to pass directly back a type that lets Rust directly read from the fd for bare repos, but that can come later) --- rust-bindings/conf/ostree.toml | 5 +++++ rust-bindings/src/auto/repo.rs | 12 ---------- rust-bindings/src/auto/versions.txt | 2 +- rust-bindings/src/repo.rs | 33 ++++++++++++++++++++++++++++ rust-bindings/tests/functions/mod.rs | 4 ++-- 5 files changed, 41 insertions(+), 15 deletions(-) diff --git a/rust-bindings/conf/ostree.toml b/rust-bindings/conf/ostree.toml index 64abc10a..57c2a793 100644 --- a/rust-bindings/conf/ostree.toml +++ b/rust-bindings/conf/ostree.toml @@ -168,6 +168,11 @@ concurrency = "send" name = "destination_path" string_type = "filename" + # Cases where we use nullable output parameters that shouldn't be `Option` + [[object.function]] + pattern = "^(load_file)$" + ignore = true + [[object]] name = "OSTree.RepoFinder" status = "generate" diff --git a/rust-bindings/src/auto/repo.rs b/rust-bindings/src/auto/repo.rs index ba34e1dc..4fd2a29c 100644 --- a/rust-bindings/src/auto/repo.rs +++ b/rust-bindings/src/auto/repo.rs @@ -486,18 +486,6 @@ impl Repo { } } - #[doc(alias = "ostree_repo_load_file")] - pub fn load_file>(&self, checksum: &str, cancellable: Option<&P>) -> Result<(Option, Option, Option), glib::Error> { - unsafe { - let mut out_input = ptr::null_mut(); - let mut out_file_info = ptr::null_mut(); - let mut out_xattrs = ptr::null_mut(); - let mut error = ptr::null_mut(); - let _ = ffi::ostree_repo_load_file(self.to_glib_none().0, checksum.to_glib_none().0, &mut out_input, &mut out_file_info, &mut out_xattrs, cancellable.map(|p| p.as_ref()).to_glib_none().0, &mut error); - if error.is_null() { Ok((from_glib_full(out_input), from_glib_full(out_file_info), from_glib_full(out_xattrs))) } else { Err(from_glib_full(error)) } - } - } - #[doc(alias = "ostree_repo_load_object_stream")] pub fn load_object_stream>(&self, objtype: ObjectType, checksum: &str, cancellable: Option<&P>) -> Result<(gio::InputStream, u64), glib::Error> { unsafe { diff --git a/rust-bindings/src/auto/versions.txt b/rust-bindings/src/auto/versions.txt index 47d43759..3c7edf9e 100644 --- a/rust-bindings/src/auto/versions.txt +++ b/rust-bindings/src/auto/versions.txt @@ -1,2 +1,2 @@ Generated by gir (https://github.com/gtk-rs/gir @ e8f82cf6) -from gir-files (@ 21901c2d) +from gir-files (@ ee5b3c76) diff --git a/rust-bindings/src/repo.rs b/rust-bindings/src/repo.rs index 6947f49b..6aeccff5 100644 --- a/rust-bindings/src/repo.rs +++ b/rust-bindings/src/repo.rs @@ -277,6 +277,39 @@ impl Repo { Ok(self.resolve_rev(refspec, false)?.unwrap()) } + /// Load the contents (for regular files) and metadata for a content object. + #[doc(alias = "ostree_repo_load_file")] + pub fn load_file>( + &self, + checksum: &str, + cancellable: Option<&P>, + ) -> Result<(Option, gio::FileInfo, glib::Variant), glib::Error> { + unsafe { + let mut out_input = ptr::null_mut(); + let mut out_file_info = ptr::null_mut(); + let mut out_xattrs = ptr::null_mut(); + let mut error = ptr::null_mut(); + let _ = ffi::ostree_repo_load_file( + self.to_glib_none().0, + checksum.to_glib_none().0, + &mut out_input, + &mut out_file_info, + &mut out_xattrs, + cancellable.map(|p| p.as_ref()).to_glib_none().0, + &mut error, + ); + if error.is_null() { + Ok(( + from_glib_full(out_input), + from_glib_full(out_file_info), + from_glib_full(out_xattrs), + )) + } else { + Err(from_glib_full(error)) + } + } + } + /// Query metadata for a content object. /// /// This is similar to [`load_file`], but is more efficient if reading the file content is not needed. diff --git a/rust-bindings/tests/functions/mod.rs b/rust-bindings/tests/functions/mod.rs index 9fa3a918..e041ddfb 100644 --- a/rust-bindings/tests/functions/mod.rs +++ b/rust-bindings/tests/functions/mod.rs @@ -59,8 +59,8 @@ fn should_checksum_file_from_input() { .load_file(obj.checksum(), NONE_CANCELLABLE) .expect("load file"); let result = checksum_file_from_input( - file_info.as_ref().unwrap(), - xattrs.as_ref(), + &file_info, + Some(&xattrs), stream.as_ref(), ObjectType::File, NONE_CANCELLABLE, From 14a7c0c74bc080bbe8529c8645b895c118b83d9a Mon Sep 17 00:00:00 2001 From: Nikita Dubrovskii Date: Thu, 23 Jun 2022 15:54:04 +0200 Subject: [PATCH 39/56] s390x: rename sd-boot to sdboot Signed-off-by: Nikita Dubrovskii --- src/libostree/ostree-bootloader-zipl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libostree/ostree-bootloader-zipl.c b/src/libostree/ostree-bootloader-zipl.c index 87b9b67a..0ff350f9 100644 --- a/src/libostree/ostree-bootloader-zipl.c +++ b/src/libostree/ostree-bootloader-zipl.c @@ -28,7 +28,7 @@ #define SECURE_EXECUTION_SYSFS_FLAG "/sys/firmware/uv/prot_virt_guest" #define SECURE_EXECUTION_PARTITION "/dev/disk/by-label/se" #define SECURE_EXECUTION_MOUNTPOINT "/sysroot/se" -#define SECURE_EXECUTION_BOOT_IMAGE SECURE_EXECUTION_MOUNTPOINT "/sd-boot" +#define SECURE_EXECUTION_BOOT_IMAGE SECURE_EXECUTION_MOUNTPOINT "/sdboot" #define SECURE_EXECUTION_HOSTKEY_PATH "/etc/se-hostkeys/" #define SECURE_EXECUTION_HOSTKEY_PREFIX "ibm-z-hostkey" #define SECURE_EXECUTION_LUKS_ROOT_KEY "/etc/luks/root" From 52d6f4e790787392e9e2246f3c552349fce5cca7 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 23 Jun 2022 17:23:45 -0400 Subject: [PATCH 40/56] tests/staged-deploy.sh: Hack around cosa systemd unit check https://github.com/coreos/coreos-assembler/pull/2921 broke this test which is intentionally causing a systemd unit to fail. As they say, necessity is the mother of invention. They don't say though that need always causes particularly *beautiful* things to be invented... --- tests/kolainst/destructive/staged-deploy.sh | 22 +++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/tests/kolainst/destructive/staged-deploy.sh b/tests/kolainst/destructive/staged-deploy.sh index 7e1991bb..af31078a 100755 --- a/tests/kolainst/destructive/staged-deploy.sh +++ b/tests/kolainst/destructive/staged-deploy.sh @@ -150,13 +150,27 @@ EOF # Now finally, try breaking staged updates and verify that ostree-boot-complete fails on the next boot unshare -m /bin/sh -c 'mount -o remount,rw /boot; chattr +i /boot' rpm-ostree kargs --append=foo=bar + + # Hack around https://github.com/coreos/coreos-assembler/pull/2921#issuecomment-1156592723 + # where coreos-assembler/kola check systemd unit status right after ssh. + cat >/etc/systemd/system/hackaround-cosa-systemd-unit-checks.service << 'EOF' +[Unit] +Before=systemd-user-sessions.service + +[Service] +Type=oneshot +ExecStart=/bin/sh -c '(systemctl status ostree-boot-complete.service || true) | tee /run/ostree-boot-complete-status.txt' +ExecStart=/bin/systemctl reset-failed ostree-boot-complete.service + +[Install] +WantedBy=multi-user.target +EOF + systemctl enable hackaround-cosa-systemd-unit-checks.service + /tmp/autopkgtest-reboot "3" ;; "3") - (systemctl status ostree-boot-complete.service || true) | tee out.txt - assert_file_has_content out.txt 'error: ostree-finalize-staged.service failed on previous boot.*Operation not permitted' - systemctl show -p Result ostree-boot-complete.service > out.txt - assert_file_has_content out.txt='Result=exit-code' + assert_file_has_content /run/ostree-boot-complete-status.txt 'error: ostree-finalize-staged.service failed on previous boot.*Operation not permitted' echo "ok boot-complete.service" ;; *) fatal "Unexpected AUTOPKGTEST_REBOOT_MARK=${AUTOPKGTEST_REBOOT_MARK}" ;; From e98988ba176e9cef18c80d5280c40f606511e94b Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 24 Jun 2022 15:28:21 -0400 Subject: [PATCH 41/56] tests/inst: Port to cap-std Part of an ongoing effort. --- tests/inst/Cargo.toml | 3 +- tests/inst/src/treegen.rs | 66 +++++++++++++++++++-------------------- 2 files changed, 33 insertions(+), 36 deletions(-) diff --git a/tests/inst/Cargo.toml b/tests/inst/Cargo.toml index e151c073..e006dda1 100644 --- a/tests/inst/Cargo.toml +++ b/tests/inst/Cargo.toml @@ -11,6 +11,7 @@ name = "ostree-test" path = "src/insttestmain.rs" [dependencies] +cap-std-ext = "0.25" clap = "2.32.0" structopt = "0.3" serde = "1.0.111" @@ -33,8 +34,6 @@ procspawn = "0.8" rand = "0.8" strum = "0.18.0" strum_macros = "0.18.0" -openat = "0.1.19" -openat-ext = "0.1.4" nix = "0.23.0" # See discussion in https://github.com/coreos/rpm-ostree/pull/2569#issuecomment-780569188 rpmostree-client = { git = "https://github.com/coreos/rpm-ostree", tag = "v2021.3" } diff --git a/tests/inst/src/treegen.rs b/tests/inst/src/treegen.rs index 29bdffd2..ab0c5bdb 100644 --- a/tests/inst/src/treegen.rs +++ b/tests/inst/src/treegen.rs @@ -1,5 +1,8 @@ use anyhow::{Context, Result}; -use openat_ext::{FileExt, OpenatDirExt}; +use cap_std::fs::Dir; +use cap_std_ext::cap_std; +use cap_std_ext::dirext::*; +use cap_std_ext::rustix::fs::MetadataExt; use rand::Rng; use sh_inline::bash; use std::fs::File; @@ -52,40 +55,36 @@ pub(crate) fn is_elf(f: &mut File) -> Result { pub(crate) fn mutate_one_executable_to( f: &mut File, name: &std::ffi::OsStr, - dest: &openat::Dir, + dest: &Dir, ) -> Result<()> { - let mut destf = dest - .write_file(name, 0o755) - .context("Failed to open for write")?; - f.copy_to(&destf).context("Failed to copy")?; - // ELF is OK with us just appending some junk - let extra = rand::thread_rng() - .sample_iter(&rand::distributions::Alphanumeric) - .take(10) - .collect::>(); - destf - .write_all(&extra) - .context("Failed to append extra data")?; - Ok(()) + let perms = f.metadata()?.permissions(); + dest.atomic_replace_with(name, |w| { + std::io::copy(f, w)?; + // ELF is OK with us just appending some junk + let extra = rand::thread_rng() + .sample_iter(&rand::distributions::Alphanumeric) + .take(10) + .collect::>(); + w.write_all(&extra).context("Failed to append extra data")?; + w.get_mut() + .as_file_mut() + .set_permissions(cap_std::fs::Permissions::from_std(perms))?; + Ok::<_, anyhow::Error>(()) + }) } /// Find ELF files in the srcdir, write new copies to dest (only percentage) -pub(crate) fn mutate_executables_to( - src: &openat::Dir, - dest: &openat::Dir, - percentage: u32, -) -> Result { +pub(crate) fn mutate_executables_to(src: &Dir, dest: &Dir, percentage: u32) -> Result { use nix::sys::stat::Mode as NixMode; assert!(percentage > 0 && percentage <= 100); let mut mutated = 0; - for entry in src.list_dir(".")? { + for entry in src.entries()? { let entry = entry?; - if src.get_file_type(&entry)? != openat::SimpleType::File { + if entry.file_type()? != cap_std::fs::FileType::file() { continue; } - let meta = src.metadata(entry.file_name())?; - let st = meta.stat(); - let mode = NixMode::from_bits_truncate(st.st_mode); + let meta = entry.metadata()?; + let mode = NixMode::from_bits_truncate(meta.mode()); // Must be executable if !mode.intersects(NixMode::S_IXUSR | NixMode::S_IXGRP | NixMode::S_IXOTH) { continue; @@ -95,17 +94,17 @@ pub(crate) fn mutate_executables_to( continue; } // Greater than 1k in size - if st.st_size < 1024 { + if meta.size() < 1024 { continue; } - let mut f = src.open_file(entry.file_name())?; + let mut f = entry.open()?.into_std(); if !is_elf(&mut f)? { continue; } if !rand::thread_rng().gen_ratio(percentage, 100) { continue; } - mutate_one_executable_to(&mut f, entry.file_name(), dest) + mutate_one_executable_to(&mut f, &entry.file_name(), dest) .with_context(|| format!("Failed updating {:?}", entry.file_name()))?; mutated += 1; } @@ -124,15 +123,14 @@ pub(crate) fn update_os_tree>( let tempdir = tempfile::tempdir_in(repo_path.join("tmp"))?; let mut mutated = 0; { - let tempdir = openat::Dir::open(tempdir.path())?; + let tempdir = Dir::open_ambient_dir(tempdir.path(), cap_std::ambient_authority())?; let binary_dirs = &["usr/bin", "usr/sbin", "usr/lib", "usr/lib64"]; - let rootfs = openat::Dir::open("/")?; + let rootfs = Dir::open_ambient_dir("/", cap_std::ambient_authority())?; for v in binary_dirs { let v = *v; - if let Some(src) = rootfs.sub_dir_optional(v)? { - tempdir.ensure_dir("usr", 0o755)?; - tempdir.ensure_dir(v, 0o755)?; - let dest = tempdir.sub_dir(v)?; + if let Some(src) = rootfs.open_dir_optional(v)? { + tempdir.create_dir_all(v)?; + let dest = tempdir.open_dir(v)?; mutated += mutate_executables_to(&src, &dest, percentage) .with_context(|| format!("Replacing binaries in {v}"))?; } From a9848712377302325b3e4d9803dc6dd2d1d6c533 Mon Sep 17 00:00:00 2001 From: Saqib Ali Date: Mon, 6 Jun 2022 17:46:01 -0400 Subject: [PATCH 42/56] lib/prune: speed up pruning by retrieving only commits After landing the new --commit-only functionality, we still noticed exceedingly long pruning times in large repos. Lets add an optimization that will only retrieve commit objects when --commit-only flag is used. --- src/libostree/ostree-repo-prune.c | 29 ++++++++++++++++++++++++----- src/libostree/ostree-repo.h | 2 +- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/src/libostree/ostree-repo-prune.c b/src/libostree/ostree-repo-prune.c index da4f4284..e53b8163 100644 --- a/src/libostree/ostree-repo-prune.c +++ b/src/libostree/ostree-repo-prune.c @@ -189,7 +189,6 @@ _ostree_repo_prune_tmp (OstreeRepo *self, return TRUE; } - /** * ostree_repo_prune_static_deltas: * @self: Repo @@ -437,8 +436,17 @@ ostree_repo_prune (OstreeRepo *self, return FALSE; } - objects = ostree_repo_list_objects_set (self, OSTREE_REPO_LIST_OBJECTS_ALL | OSTREE_REPO_LIST_OBJECTS_NO_PARENTS, + if (commit_only) + { + if (!ostree_repo_list_commit_objects_starting_with (self, "", &objects, cancellable, error)) + return FALSE; + } + else + { + objects = ostree_repo_list_objects_set (self, OSTREE_REPO_LIST_OBJECTS_ALL | OSTREE_REPO_LIST_OBJECTS_NO_PARENTS, cancellable, error); + } + if (!objects) return FALSE; @@ -508,9 +516,20 @@ ostree_repo_prune_from_reachable (OstreeRepo *self, if (!lock) return FALSE; - g_autoptr(GHashTable) objects = - ostree_repo_list_objects_set (self, OSTREE_REPO_LIST_OBJECTS_ALL | OSTREE_REPO_LIST_OBJECTS_NO_PARENTS, - cancellable, error); + g_autoptr(GHashTable) objects = NULL; + OstreeRepoPruneFlags flags = options->flags; + gboolean commit_only = (flags & OSTREE_REPO_PRUNE_FLAGS_COMMIT_ONLY) > 0; + if (commit_only) + { + if (!ostree_repo_list_commit_objects_starting_with (self, "", &objects, cancellable, error)) + return FALSE; + } + else + { + objects = + ostree_repo_list_objects_set (self, OSTREE_REPO_LIST_OBJECTS_ALL | OSTREE_REPO_LIST_OBJECTS_NO_PARENTS, + cancellable, error); + } if (!objects) return FALSE; diff --git a/src/libostree/ostree-repo.h b/src/libostree/ostree-repo.h index 98571170..b7ed3600 100644 --- a/src/libostree/ostree-repo.h +++ b/src/libostree/ostree-repo.h @@ -1261,7 +1261,7 @@ void ostree_repo_commit_traverse_iter_cleanup (void *p); * OstreeRepoPruneFlags: * @OSTREE_REPO_PRUNE_FLAGS_NONE: No special options for pruning * @OSTREE_REPO_PRUNE_FLAGS_NO_PRUNE: Don't actually delete objects - * @OSTREE_REPO_PRUNE_FLAGS_REFS_ONLY: Do not traverse individual commit objects, only follow refs + * @OSTREE_REPO_PRUNE_FLAGS_REFS_ONLY: Do not traverse individual commit objects, only follow refs for reachability calculations * @OSTREE_REPO_PRUNE_FLAGS_COMMIT_ONLY: Only traverse commit objects. (Since 2022.2) */ typedef enum { From becc18936ff7e225e7723adae558186bcc3e1b36 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 6 Jul 2022 07:46:42 -0400 Subject: [PATCH 43/56] lib: Stop using old `ostree_sysroot_get_repo()` API It's falliable, and in one place we were actually ignoring the error and leaving a `NULL` repo object which is just a trap for people coming along later since it's rarely nullable. Quite a while ago we switched to loading the repo at the same time as the sysroot; convert callers in the library to use this infallible accessor. Prep for another patch which will use the repo object. --- src/libostree/ostree-sysroot-deploy.c | 12 +++--------- src/libostree/ostree-sysroot-upgrader.c | 4 +--- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index 3a4f8d41..c23453cb 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -1807,7 +1807,6 @@ parse_os_release (const char *contents, */ static gboolean install_deployment_kernel (OstreeSysroot *sysroot, - OstreeRepo *repo, int new_bootversion, OstreeDeployment *deployment, guint n_deployments, @@ -1855,6 +1854,8 @@ install_deployment_kernel (OstreeSysroot *sysroot, if (!glnx_shutil_mkdir_p_at (sysroot->boot_fd, bootconfdir, 0775, cancellable, error)) return FALSE; + OstreeRepo *repo = ostree_sysroot_repo (sysroot); + /* Install (hardlink/copy) the kernel into /boot/ostree/osname-${bootcsum} if * it doesn't exist already. */ @@ -2378,13 +2379,6 @@ write_deployments_bootswap (OstreeSysroot *self, cancellable, error)) return FALSE; - /* Need the repo to try and extract the versions for deployments. - * But this is a "nice-to-have" for the bootloader UI, so failure - * here is not fatal to the whole operation. We just gracefully - * fall back to the deployment index. */ - g_autoptr(OstreeRepo) repo = NULL; - (void) ostree_sysroot_get_repo (self, &repo, cancellable, NULL); - /* Only show the osname in bootloader titles if there are multiple * osname's among the new deployments. Check for that here. */ gboolean show_osname = FALSE; @@ -2402,7 +2396,7 @@ write_deployments_bootswap (OstreeSysroot *self, for (guint i = 0; i < new_deployments->len; i++) { OstreeDeployment *deployment = new_deployments->pdata[i]; - if (!install_deployment_kernel (self, repo, new_bootversion, + if (!install_deployment_kernel (self, new_bootversion, deployment, new_deployments->len, show_osname, cancellable, error)) return FALSE; diff --git a/src/libostree/ostree-sysroot-upgrader.c b/src/libostree/ostree-sysroot-upgrader.c index 0b2590ba..58f8ecd0 100644 --- a/src/libostree/ostree-sysroot-upgrader.c +++ b/src/libostree/ostree-sysroot-upgrader.c @@ -492,7 +492,6 @@ ostree_sysroot_upgrader_pull_one_dir (OstreeSysrootUpgrader *self, GCancellable *cancellable, GError **error) { - g_autoptr(OstreeRepo) repo = NULL; char *refs_to_fetch[] = { NULL, NULL }; const char *from_revision = NULL; g_autofree char *origin_refspec = NULL; @@ -506,8 +505,7 @@ ostree_sysroot_upgrader_pull_one_dir (OstreeSysrootUpgrader *self, else refs_to_fetch[0] = self->origin_ref; - if (!ostree_sysroot_get_repo (self->sysroot, &repo, cancellable, error)) - return FALSE; + OstreeRepo *repo = ostree_sysroot_repo (self->sysroot); if (self->origin_remote) origin_refspec = g_strconcat (self->origin_remote, ":", self->origin_ref, NULL); From d7107e3036ffb1798feff74bc3cd03f9068c63da Mon Sep 17 00:00:00 2001 From: Saqib Ali Date: Tue, 5 Jul 2022 12:00:16 -0400 Subject: [PATCH 44/56] ostree-repo: bls-append-except-default followup This PR is followup from https://github.com/coreos/coreos-assembler/pull/2863 Summary of changes: - Moved bls-append-except-default parsing logic to reload_sysroot_config() - Made sure heap allocated memory is being freed --- src/libostree/ostree-repo-private.h | 1 + src/libostree/ostree-repo.c | 34 ++++++++++++++++++++++++-- src/libostree/ostree-sysroot-deploy.c | 35 ++------------------------- 3 files changed, 35 insertions(+), 35 deletions(-) diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h index 3858a36e..0d33f7c2 100644 --- a/src/libostree/ostree-repo-private.h +++ b/src/libostree/ostree-repo-private.h @@ -224,6 +224,7 @@ struct OstreeRepo { gint fs_support_reflink; /* The underlying filesystem has support for ioctl (FICLONE..) */ gchar **repo_finders; OstreeCfgSysrootBootloaderOpt bootloader; /* Configure which bootloader to use. */ + GHashTable *bls_append_values; /* Parsed key-values from bls-append-except-default key in config. */ OstreeRepo *parent_repo; }; diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 9b29bd3f..90cde651 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -1238,6 +1238,7 @@ ostree_repo_finalize (GObject *object) g_mutex_clear (&self->txn_lock); g_free (self->collection_id); g_strfreev (self->repo_finders); + g_clear_pointer (&self->bls_append_values, g_hash_table_unref); g_clear_pointer (&self->remotes, g_hash_table_destroy); g_mutex_clear (&self->remotes_lock); @@ -1412,6 +1413,9 @@ ostree_repo_init (OstreeRepo *self) self->remotes = g_hash_table_new_full (g_str_hash, g_str_equal, (GDestroyNotify) NULL, (GDestroyNotify) ostree_remote_unref); + self->bls_append_values = g_hash_table_new_full (g_str_hash, g_str_equal, + (GDestroyNotify) g_free, + (GDestroyNotify) g_free); g_mutex_init (&self->remotes_lock); self->repo_dir_fd = -1; @@ -3510,16 +3514,42 @@ reload_sysroot_config (OstreeRepo *self, * https://github.com/ostreedev/ostree/issues/1719 * https://github.com/ostreedev/ostree/issues/1801 */ + gboolean valid_bootloader; 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; + valid_bootloader = TRUE; } } + if (!valid_bootloader) + { + return glnx_throw (error, "Invalid bootloader configuration: '%s'", bootloader); + } + /* Parse bls-append-except-default string list. */ + g_auto(GStrv) read_values = NULL; + if (!ot_keyfile_get_string_list_with_default (self->config, "sysroot", "bls-append-except-default", + ';', NULL, &read_values, error)) + return glnx_throw(error, "Unable to parse bls-append-except-default"); + + /* get all key value pairs in bls-append-except-default */ + g_hash_table_remove_all (self->bls_append_values); + for (char **iter = read_values; iter && *iter; iter++) + { + const char *key_value = *iter; + const char *sep = strchr (key_value, '='); + if (sep == NULL) + { + glnx_throw (error, "bls-append-except-default key must be of the form \"key1=value1;key2=value2...\""); + return FALSE; + } + char *key = g_strndup (key_value, sep - key_value); + char *value = g_strdup (sep + 1); + g_hash_table_replace (self->bls_append_values, key, value); + } - return glnx_throw (error, "Invalid bootloader configuration: '%s'", bootloader); + return TRUE; } /** diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index c23453cb..456b0c04 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -2083,25 +2083,6 @@ install_deployment_kernel (OstreeSysroot *sysroot, g_autofree char *options_key = ostree_kernel_args_to_string (kargs); ostree_bootconfig_parser_set (bootconfig, "options", options_key); - g_autoptr(GError) local_error = NULL; - GKeyFile *config = ostree_repo_get_config (repo); - gchar **read_values = g_key_file_get_string_list (config, "sysroot", "bls-append-except-default", NULL, &local_error); - /* We can ignore not found errors */ - if (!read_values) - { - gboolean not_found = g_error_matches (local_error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_KEY_NOT_FOUND) || \ - g_error_matches (local_error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_GROUP_NOT_FOUND); - if (not_found) - { - g_clear_error (&local_error); - } - else - { - g_propagate_error (error, g_steal_pointer (&local_error)); - return FALSE; - } - } - /* Only append to this BLS config if: * - this is not the default deployment */ @@ -2111,20 +2092,8 @@ install_deployment_kernel (OstreeSysroot *sysroot, if (allow_append) { /* get all key value pairs in bls-append */ - for (char **iter = read_values; iter && *iter; iter++) - { - const char *key_value = *iter; - const char *sep = strchr (key_value, '='); - if (sep == NULL) - { - glnx_throw (error, "bls-append-except-default key must be of the form \"key1=value1;key2=value2...\""); - return FALSE; - } - g_autofree char *key = g_strndup (key_value, sep - key_value); - g_autofree char *value = g_strdup (sep + 1); - ostree_bootconfig_parser_set (bootconfig, key, value); - } - + GLNX_HASH_TABLE_FOREACH_KV (repo->bls_append_values, const char *, key, const char *, value) + ostree_bootconfig_parser_set (bootconfig, key, value); } glnx_autofd int bootconf_dfd = -1; From d3762be9b3c83853f2f8be827b7cced9a85c5af6 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 8 Jul 2022 09:33:18 -0400 Subject: [PATCH 45/56] deny: Sync with rpm-ostree This extends the license set basically and ignores private repos (which we don't have any yet). --- deny.toml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/deny.toml b/deny.toml index 75b6ac9b..e810c910 100644 --- a/deny.toml +++ b/deny.toml @@ -1,6 +1,8 @@ [licenses] unlicensed = "deny" -allow = ["Apache-2.0", "Apache-2.0 WITH LLVM-exception", "MIT", "BSD-3-Clause", "BSD-2-Clause"] +copyleft = "allow" +allow = ["Apache-2.0", "Apache-2.0 WITH LLVM-exception", "MIT", "BSD-3-Clause", "BSD-2-Clause", "Unlicense", "CC0-1.0"] +private = { ignore = true } [bans] From 22946e9d963fa48e530faf72acecb1c0ba140c77 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Mon, 11 Jul 2022 08:55:43 +0200 Subject: [PATCH 46/56] Fix link to rust bindings ostree-rs was merged into ostree, so link to the rust bindings within this repository. Signed-off-by: Matthias Beyer --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 0896b244..d8e6cdef 100644 --- a/README.md +++ b/README.md @@ -119,7 +119,7 @@ write higher level manual bindings on top; this is more common for statically compiled languages. Here's a list of such bindings: - [ostree-go](https://github.com/ostreedev/ostree-go/) - - [ostree-rs](https://github.com/ostreedev/ostree-rs/) + - [ostree-rs](./rust-bindings) ## Building From 7b7b6d741bda9f9f92c3632153e7a7a5b39cb06b Mon Sep 17 00:00:00 2001 From: Huijing Hei Date: Fri, 8 Jul 2022 19:37:37 +0800 Subject: [PATCH 47/56] Fix `ostree admin kargs edit-in-place` fails issue Add func to set kernel arguments in place, instead of create new deployment Fix https://github.com/ostreedev/ostree/issues/2664 --- apidoc/ostree-sections.txt | 1 + src/libostree/libostree-devel.sym | 1 + src/libostree/ostree-sysroot-deploy.c | 43 +++++++++++++++++++ src/libostree/ostree-sysroot.h | 7 +++ .../ot-admin-kargs-builtin-edit-in-place.c | 7 ++- 5 files changed, 55 insertions(+), 4 deletions(-) diff --git a/apidoc/ostree-sections.txt b/apidoc/ostree-sections.txt index 5af8e687..900e1704 100644 --- a/apidoc/ostree-sections.txt +++ b/apidoc/ostree-sections.txt @@ -577,6 +577,7 @@ ostree_sysroot_get_repo ostree_sysroot_get_staged_deployment ostree_sysroot_init_osname ostree_sysroot_deployment_set_kargs +ostree_sysroot_deployment_set_kargs_in_place ostree_sysroot_deployment_set_mutable ostree_sysroot_deployment_unlock ostree_sysroot_deployment_set_pinned diff --git a/src/libostree/libostree-devel.sym b/src/libostree/libostree-devel.sym index 54945eca..145ec1ec 100644 --- a/src/libostree/libostree-devel.sym +++ b/src/libostree/libostree-devel.sym @@ -23,6 +23,7 @@ LIBOSTREE_2022.5 { global: ostree_kernel_args_append_if_missing; + ostree_sysroot_deployment_set_kargs_in_place; } LIBOSTREE_2022.4; /* Stub section for the stable release *after* this development one; don't diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index 456b0c04..a9d41258 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -3589,6 +3589,49 @@ ostree_sysroot_deployment_set_kargs (OstreeSysroot *self, return TRUE; } +/** + * ostree_sysroot_deployment_set_kargs_in_place: + * @self: Sysroot + * @deployment: A deployment + * @kargs_str: (allow none): Replace @deployment's kernel arguments + * @cancellable: Cancellable + * @error: Error + * + * Replace the kernel arguments of @deployment with the values in @kargs_str. + */ +gboolean +ostree_sysroot_deployment_set_kargs_in_place (OstreeSysroot *self, + OstreeDeployment *deployment, + char *kargs_str, + GCancellable *cancellable, + GError **error) +{ + if (!_ostree_sysroot_ensure_writable (self, error)) + return FALSE; + + g_assert (!ostree_deployment_is_staged (deployment)); + + OstreeBootconfigParser *new_bootconfig = ostree_deployment_get_bootconfig (deployment); + ostree_bootconfig_parser_set (new_bootconfig, "options", kargs_str); + + g_autofree char *bootconf_name = + g_strdup_printf ("ostree-%d-%s.conf", + self->deployments->len - ostree_deployment_get_index (deployment), + ostree_deployment_get_osname (deployment)); + + g_autofree char *bootconfdir = g_strdup_printf ("loader.%d/entries", self->bootversion); + glnx_autofd int bootconf_dfd = -1; + if (!glnx_opendirat (self->boot_fd, bootconfdir, TRUE, &bootconf_dfd, error)) + return FALSE; + + if (!ostree_bootconfig_parser_write_at (new_bootconfig, + bootconf_dfd, bootconf_name, + cancellable, error)) + return FALSE; + + return TRUE; +} + /** * ostree_sysroot_deployment_set_mutable: * @self: Sysroot diff --git a/src/libostree/ostree-sysroot.h b/src/libostree/ostree-sysroot.h index c240aaa0..0cde9e44 100644 --- a/src/libostree/ostree-sysroot.h +++ b/src/libostree/ostree-sysroot.h @@ -175,6 +175,13 @@ gboolean ostree_sysroot_deployment_set_kargs (OstreeSysroot *self, GCancellable *cancellable, GError **error); +_OSTREE_PUBLIC +gboolean ostree_sysroot_deployment_set_kargs_in_place (OstreeSysroot *self, + OstreeDeployment *deployment, + char *kargs_str, + GCancellable *cancellable, + GError **error); + _OSTREE_PUBLIC gboolean ostree_sysroot_write_deployments (OstreeSysroot *self, GPtrArray *new_deployments, diff --git a/src/ostree/ot-admin-kargs-builtin-edit-in-place.c b/src/ostree/ot-admin-kargs-builtin-edit-in-place.c index 40ada02f..2a16da9c 100644 --- a/src/ostree/ot-admin-kargs-builtin-edit-in-place.c +++ b/src/ostree/ot-admin-kargs-builtin-edit-in-place.c @@ -67,11 +67,10 @@ ot_admin_kargs_builtin_edit_in_place (int argc, char **argv, OstreeCommandInvoca } } - g_auto(GStrv) kargs_strv = ostree_kernel_args_to_strv (kargs); + g_autofree char *new_options = ostree_kernel_args_to_string (kargs); - if (!ostree_sysroot_deployment_set_kargs (sysroot, deployment, - kargs_strv, - cancellable, error)) + if (!ostree_sysroot_deployment_set_kargs_in_place (sysroot, deployment, new_options, + cancellable, error)) return FALSE; } From 8f24e0826ac30fe7538bd60a000d9e43500749eb Mon Sep 17 00:00:00 2001 From: Huijing Hei Date: Tue, 12 Jul 2022 16:27:56 +0800 Subject: [PATCH 48/56] Add test to verify `ostree admin kargs edit-in-place` working --- tests/kolainst/destructive/kargs-edit-in-place.sh | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100755 tests/kolainst/destructive/kargs-edit-in-place.sh diff --git a/tests/kolainst/destructive/kargs-edit-in-place.sh b/tests/kolainst/destructive/kargs-edit-in-place.sh new file mode 100755 index 00000000..6380ff33 --- /dev/null +++ b/tests/kolainst/destructive/kargs-edit-in-place.sh @@ -0,0 +1,12 @@ +#!/bin/bash + +# Verify "ostree admin kargs edit-in-place" works + +set -xeuo pipefail + +. ${KOLA_EXT_DATA}/libinsttest.sh + +sudo ostree admin kargs edit-in-place --append-if-missing=testarg +assert_file_has_content /boot/loader/entries/ostree-* testarg + +echo "ok test `kargs edit-in-place --append-if-missing`" From 2c716552052cd3d03cef5f2968d5945d799f8d90 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 13 Jul 2022 15:32:05 -0400 Subject: [PATCH 49/56] deploy: Ensure sysroot is initialized for kargs in place Even without a mount namespace set up. --- src/libostree/ostree-sysroot-deploy.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index a9d41258..47c70c43 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -3606,6 +3606,10 @@ ostree_sysroot_deployment_set_kargs_in_place (OstreeSysroot *self, GCancellable *cancellable, GError **error) { + if (!ostree_sysroot_initialize (self, error)) + return FALSE; + if (!_ostree_sysroot_ensure_boot_fd (self, error)) + return FALSE; if (!_ostree_sysroot_ensure_writable (self, error)) return FALSE; From 75aa7a22f6d6499ade99f66f883dad2a3ebd413c Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 13 Jul 2022 15:35:00 -0400 Subject: [PATCH 50/56] sysroot: Have `ensure_writable` also always initialize For historical reasons we have a fair bit of distinct sysroot initialization going on. A lot of code is calling *just* the new `ensure_writable()` API, which does basically what you'd expect... except if we're not using a mount namespace. Which is the case in unit tests and legacy setups. Change this API to also ensure the sysroot is fully initialized even in those cases. Specifically we'll have `self->sysroot_fd`. For now, callers that need `/boot` also need to separately call `_ensure_boot_fd()`. --- src/libostree/ostree-sysroot.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/libostree/ostree-sysroot.c b/src/libostree/ostree-sysroot.c index bcb55e55..b15265f5 100644 --- a/src/libostree/ostree-sysroot.c +++ b/src/libostree/ostree-sysroot.c @@ -328,14 +328,13 @@ gboolean _ostree_sysroot_ensure_writable (OstreeSysroot *self, GError **error) { + if (!ostree_sysroot_initialize (self, error)) + return FALSE; + /* Do nothing if no mount namespace is in use */ if (!self->mount_namespace_in_use) return TRUE; - /* If a mount namespace is in use, ensure we're initialized */ - if (!ostree_sysroot_initialize (self, error)) - return FALSE; - /* If we aren't operating on a booted system, then we don't * do anything with mounts. */ From 60853219d5a78f0c62d1ccb4e3f27585ae5c7ffd Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 13 Jul 2022 15:38:59 -0400 Subject: [PATCH 51/56] sysroot: Add a few more assertions about `boot_fd` These places are all safe, but it would catch bugs in the future more clearly to trip an assertion here. --- src/libostree/ostree-sysroot-deploy.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index 456b0c04..3b04267c 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -1614,6 +1614,7 @@ full_system_sync (OstreeSysroot *self, return FALSE; start_msec = g_get_monotonic_time () / 1000; + g_assert_cmpint (self->boot_fd, !=, -1); if (!fsfreeze_thaw_cycle (self, self->boot_fd, cancellable, error)) return FALSE; end_msec = g_get_monotonic_time () / 1000; @@ -3526,6 +3527,7 @@ _ostree_sysroot_boot_complete (OstreeSysroot *self, return FALSE; glnx_autofd int failure_fd = -1; + g_assert_cmpint (self->boot_fd, !=, -1); if (!ot_openat_ignore_enoent (self->boot_fd, _OSTREE_FINALIZE_STAGED_FAILURE_PATH, &failure_fd, error)) return FALSE; // If we didn't find a failure log, then there's nothing to do right now. From 7db2fe8cba2a6b296242dc9f988f4e02b52214aa Mon Sep 17 00:00:00 2001 From: Huijing Hei Date: Thu, 14 Jul 2022 14:24:03 +0800 Subject: [PATCH 52/56] Update doc about adding new function to libostree --- docs/contributing-tutorial.md | 49 ++++++++++++++++++++++++++++++----- 1 file changed, 42 insertions(+), 7 deletions(-) diff --git a/docs/contributing-tutorial.md b/docs/contributing-tutorial.md index a9e8d670..d3db4cf7 100644 --- a/docs/contributing-tutorial.md +++ b/docs/contributing-tutorial.md @@ -302,26 +302,23 @@ This will add a command which prints `Hello OSTree!` when `ostree hello-ostree` gboolean ostree_builtin_hello_ostree (int argc, char **argv, OstreeCommandInvocation *invocation, GCancellable *cancellable, GError **error) { - g_autoptr(GOptionContext) context = NULL; g_autoptr(OstreeRepo) repo = NULL; - gboolean ret = FALSE; // Creates new command context, ready to be parsed. // Input to g_option_context_new shows when running ostree --help - context = g_option_context_new (""); + g_autoptr(GOptionContext) context = g_option_context_new (""); // Parses the command context according to the ostree CLI. if (!ostree_option_context_parse (context, options, &argc, &argv, invocation, &repo, cancellable, error)) - goto out; + return FALSE; g_print("Hello OSTree!\n"); - ret = TRUE; - out: - return ret; + return TRUE; } This defines the functionality for `hello-ostree`. Now we have to make sure the CLI can refer to the execution function, and that autotools knows to build this file. + Note: libostree codebase supports C99 features. 3. Add the following in `src/ostree/main.c`: @@ -349,6 +346,44 @@ This will add a command which prints `Hello OSTree!` when `ostree hello-ostree` $ ostree hello-ostree Hello OSTree! +### Adding a new API function to libostree + +This will add a new API function `ostree_kernel_args_foo()` in `src/libostree/ostree-kernel-args.c`. + +1. Add the following to `src/libostree/ostree-kernel-args.h`: + _OSTREE_PUBLIC + gboolean ostree_kernel_args_foo (const char *arg, GCancellable *cancellable, GError **error); + +2. Add the following to `ostree-kernel-args.c`: + /** + * ostree_kernel_args_foo: + * @arg: Description of the arg + * + * Description of the function + * + * Since: $NEWVERSION //The new libostree version, for example 2022.5 + **/ + gboolean + ostree_kernel_args_foo (const char *arg, GCancellable *cancellable, GError **error) + { + //Add code here + } + +3. Add the following to `src/libostree/libostree-devel.sym`: + LIBOSTREE_$NEWVERSION { // The new libostree version + global: + ostree_kernel_args_foo; // Function name + } LIBOSTREE_$LASTSTABLE; // The last stable libostree version + +4. Add the following to `Makefile-libostree.am`: + if BUILDOPT_IS_DEVEL_BUILD + symbol_files += $(top_srcdir)/src/libostree/libostree-devel.sym + endif + +5. Add function name `ostree_kernel_args_foo` to `apidoc/ostree-sections.txt` under `ostree-kernel-args`. + +6. Call function `ostree_kernel_args_foo()` in your code. + ### OSTree Tests Tests for OSTree are done by shell scripting, by running OSTree commands and examining output. These steps will go through adding a test for `hello-ostree`. From 83e6357186be11fb8f2a6b66fab3730c44ee59dd Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 14 Jul 2022 14:42:19 -0400 Subject: [PATCH 53/56] sign/ed25519: Verify signatures are minimum length The ed25519 signature verification code does not check that the signature is a minimum/correct length. As a result, if the signature is too short, libsodium will end up reading a few bytes out of bounds. Reported-by: Demi Marie Obenour Co-authored-by: Demi Marie Obenour Closes: https://github.com/ostreedev/ostree/security/advisories/GHSA-gqf4-p3gv-g8vw --- src/libostree/ostree-sign-ed25519.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libostree/ostree-sign-ed25519.c b/src/libostree/ostree-sign-ed25519.c index 809ffe87..f271fd49 100644 --- a/src/libostree/ostree-sign-ed25519.c +++ b/src/libostree/ostree-sign-ed25519.c @@ -209,6 +209,9 @@ gboolean ostree_sign_ed25519_data_verify (OstreeSign *self, g_autoptr (GVariant) child = g_variant_get_child_value (signatures, i); g_autoptr (GBytes) signature = g_variant_get_data_as_bytes(child); + if (g_bytes_get_size (signature) != crypto_sign_BYTES) + return glnx_throw (error, "Invalid signature length of %" G_GSIZE_FORMAT " bytes, expected %" G_GSIZE_FORMAT, (gsize) g_bytes_get_size (signature), (gsize) crypto_sign_BYTES); + g_autofree char * hex = g_malloc0 (crypto_sign_PUBLICKEYBYTES*2 + 1); g_debug("Read signature %d: %s", (gint)i, g_variant_print(child, TRUE)); From e0417957ea1578032da505947ec9cac299015446 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 14 Jul 2022 16:48:12 -0400 Subject: [PATCH 54/56] rust: Add a test case for ed25519 Specifically, I verified that *before* the previous patch to the ed25519 C code, the last bit of code would fail with a SIGSEGV when trying to read the empty signature. --- rust-bindings/tests/sign/mod.rs | 64 +++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/rust-bindings/tests/sign/mod.rs b/rust-bindings/tests/sign/mod.rs index 5df49d63..f3c817a3 100644 --- a/rust-bindings/tests/sign/mod.rs +++ b/rust-bindings/tests/sign/mod.rs @@ -1,4 +1,8 @@ +use std::process::Command; + +use ostree::prelude::SignExt; use ostree::prelude::*; +use ostree::Sign; use ostree::{gio, glib}; #[test] @@ -19,3 +23,63 @@ fn sign_api_should_work() { let result = ostree::Sign::by_name("NOPE"); assert!(result.is_err()); } + +fn inner_sign_ed25519(signer: T) { + assert_eq!(signer.name().unwrap(), "ed25519"); + + let td = tempfile::tempdir().unwrap(); + let path = td.path(); + + // Horrible bits to reuse libtest shell script code to generate keys + let pwd = std::env::current_dir().unwrap(); + let cmd = format!( + r#". {:?}/tests/libtest.sh +gen_ed25519_keys +echo $ED25519PUBLIC > ed25519.public +echo $ED25519SEED > ed25519.seed +echo $ED25519SECRET > ed25519.secret +"#, + pwd + ); + let s = Command::new("bash") + .env("G_TEST_SRCDIR", pwd) + .current_dir(path) + .args(["-euo", "pipefail"]) + .args(["-c", cmd.as_str()]) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status() + .unwrap(); + assert!(s.success()); + + let seckey = std::fs::read_to_string(path.join("ed25519.secret")).unwrap(); + let seckey = seckey.to_variant(); + signer.set_sk(&seckey).unwrap(); + let pubkey = std::fs::read_to_string(path.join("ed25519.public")).unwrap(); + let pubkey = pubkey.to_variant(); + signer.add_pk(&pubkey).unwrap(); + + let payload = &glib::Bytes::from_static(b"1234"); + + let signature = signer.data(payload, gio::NONE_CANCELLABLE).unwrap(); + let signatures = [&*signature].to_variant(); + + let msg = signer.data_verify(payload, &signatures).unwrap().unwrap(); + assert!(msg.starts_with("ed25519: Signature verified successfully")); + + assert!(signer + .data_verify(&glib::Bytes::from_static(b""), &signatures) + .is_err()); + + let badsigs = [b"".as_slice()].to_variant(); + + let e = signer.data_verify(payload, &badsigs).err().unwrap(); + assert!(e.to_string().contains("Invalid signature length of 0 bytes")) +} + +#[test] +fn sign_ed25519() { + if let Some(signer) = Sign::by_name("ed25519").ok() { + inner_sign_ed25519(signer) + } +} From 6cb1227177dc06acbe840741e47d3e22a075edf4 Mon Sep 17 00:00:00 2001 From: Chris Mucciolo Date: Fri, 15 Jul 2022 13:24:29 -0400 Subject: [PATCH 55/56] docs add debos to readme distribution build tools --- README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/README.md b/README.md index d8e6cdef..6a51482b 100644 --- a/README.md +++ b/README.md @@ -88,6 +88,9 @@ integration tool supports importing and exporting from libostree repos. Fedora [coreos-assembler](https://github.com/coreos/coreos-assembler) is the build tool used to generate Fedora CoreOS derivatives. +[debos](https://github.com/go-debos/debos) is a tool-chain for simplifying the +process of building a Debian-based OS image. + ## Projects linking to libostree [rpm-ostree](https://github.com/projectatomic/rpm-ostree) is used by the From 15740d042c9c5258a1c082b5e228cf6f115edbb0 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 22 Jul 2022 15:08:25 -0400 Subject: [PATCH 56/56] Release 2022.5 --- Makefile-libostree.am | 3 --- configure.ac | 2 +- src/libostree/libostree-devel.sym | 5 ----- src/libostree/libostree-released.sym | 6 ++++++ tests/test-symbols.sh | 2 +- 5 files changed, 8 insertions(+), 10 deletions(-) diff --git a/Makefile-libostree.am b/Makefile-libostree.am index dd7ed8df..f93f712a 100644 --- a/Makefile-libostree.am +++ b/Makefile-libostree.am @@ -174,9 +174,6 @@ 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= diff --git a/configure.ac b/configure.ac index 445c53e0..964df128 100644 --- a/configure.ac +++ b/configure.ac @@ -4,7 +4,7 @@ m4_define([year_version], [2022]) m4_define([release_version], [5]) 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 145ec1ec..eef5cba0 100644 --- a/src/libostree/libostree-devel.sym +++ b/src/libostree/libostree-devel.sym @@ -20,11 +20,6 @@ - uncomment the include in Makefile-libostree.am */ -LIBOSTREE_2022.5 { -global: - ostree_kernel_args_append_if_missing; - ostree_sysroot_deployment_set_kargs_in_place; -} LIBOSTREE_2022.4; /* 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 5afdfd3e..a5b9bdda 100644 --- a/src/libostree/libostree-released.sym +++ b/src/libostree/libostree-released.sym @@ -683,6 +683,12 @@ global: ostree_fs_get_all_xattrs_at; } LIBOSTREE_2022.2; +LIBOSTREE_2022.5 { +global: + ostree_kernel_args_append_if_missing; + ostree_sysroot_deployment_set_kargs_in_place; +} LIBOSTREE_2022.4; + /* 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 a14849d5..6eeb73ae 100755 --- a/tests/test-symbols.sh +++ b/tests/test-symbols.sh @@ -54,7 +54,7 @@ echo 'ok documented symbols' # ONLY update this checksum in release commits! cat > released-sha256.txt <