From 3c3651417cca082a09d8a86195b0110c08fe3fc2 Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Tue, 17 Nov 2020 10:50:56 +0000 Subject: [PATCH 01/44] configure: post-release version bump --- configure.ac | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index 088acc3d..758fc49e 100644 --- a/configure.ac +++ b/configure.ac @@ -7,10 +7,10 @@ dnl Seed the release notes with `git-shortlog-with-prs ..`. Th dnl `git-evtag` to create the tag and push it. Finally, create a GitHub release and attach dnl the tarball from `make dist`. m4_define([year_version], [2020]) -m4_define([release_version], [8]) +m4_define([release_version], [9]) m4_define([package_version], [year_version.release_version]) AC_INIT([libostree], [package_version], [walters@verbum.org]) -is_release_build=yes +is_release_build=no AC_CONFIG_HEADER([config.h]) AC_CONFIG_MACRO_DIR([buildutil]) AC_CONFIG_AUX_DIR([build-aux]) From a88d2f5f7b788a1b04303ac56a1971bdd5f553e6 Mon Sep 17 00:00:00 2001 From: William Manley Date: Tue, 17 Nov 2020 00:46:10 +0000 Subject: [PATCH 02/44] ostree commit --tree=tar: Import xattrs from tarballs If you specify an `xattr_callback` the xattrs will still be taken from there for now. --- src/libostree/ostree-repo-libarchive.c | 35 +++++++++- tests/test-libarchive-import.c | 89 ++++++++++++++++++++++++++ 2 files changed, 122 insertions(+), 2 deletions(-) diff --git a/src/libostree/ostree-repo-libarchive.c b/src/libostree/ostree-repo-libarchive.c index ef7252e8..96b34c18 100644 --- a/src/libostree/ostree-repo-libarchive.c +++ b/src/libostree/ostree-repo-libarchive.c @@ -458,12 +458,43 @@ aic_get_xattrs (OstreeRepoArchiveImportContext *ctx, g_autoptr(GVariant) xattrs = NULL; const char *cb_path = abspath; + gboolean no_xattrs = ctx->modifier && + ctx->modifier->flags & OSTREE_REPO_COMMIT_MODIFIER_FLAGS_SKIP_XATTRS; + + if (!no_xattrs && archive_entry_xattr_count (ctx->entry) > 0) + { + const char *name; + const void *value; + size_t size; + + g_autoptr(GVariantBuilder) builder = + ot_util_variant_builder_from_variant (xattrs, + G_VARIANT_TYPE ("a(ayay)")); + + archive_entry_xattr_reset (ctx->entry); + while (archive_entry_xattr_next ( + ctx->entry, &name, &value, &size) == ARCHIVE_OK) + { + g_variant_builder_add (builder, "(@ay@ay)", + g_variant_new_bytestring (name), + g_variant_new_fixed_array (G_VARIANT_TYPE("y"), + value, size, 1)); + } + + xattrs = g_variant_builder_end (builder); + g_variant_ref_sink (xattrs); + } + if (ctx->opts->callback_with_entry_pathname) cb_path = archive_entry_pathname (ctx->entry); if (ctx->modifier && ctx->modifier->xattr_callback) - xattrs = ctx->modifier->xattr_callback (ctx->repo, cb_path, file_info, - ctx->modifier->xattr_user_data); + { + if (xattrs) + g_variant_unref (xattrs); + xattrs = ctx->modifier->xattr_callback (ctx->repo, cb_path, file_info, + ctx->modifier->xattr_user_data); + } if (ctx->modifier && ctx->modifier->sepolicy) { diff --git a/tests/test-libarchive-import.c b/tests/test-libarchive-import.c index d3429cd6..75b298bc 100644 --- a/tests/test-libarchive-import.c +++ b/tests/test-libarchive-import.c @@ -95,6 +95,9 @@ test_data_init (TestData *td) archive_entry_set_uid (ae, uid); archive_entry_set_gid (ae, gid); archive_entry_set_size (ae, 4); + archive_entry_xattr_add_entry (ae, "user.a_key", "my value", 8); + archive_entry_xattr_add_entry (ae, "user.key2", "", 0); + archive_entry_xattr_add_entry (ae, "user.b_key", "contains\0nuls", 14); g_assert_cmpint (0, ==, archive_write_header (a, ae)); g_assert_cmpint (4, ==, archive_write_data (a, "bar\n", 4)); archive_entry_free (ae); @@ -453,6 +456,90 @@ test_libarchive_xattr_callback (gconstpointer data) g_assert_no_error (error); } +static void +test_libarchive_xattr_import (gconstpointer data) +{ + TestData *td = (void*)data; + GError *error = NULL; + g_autoptr(OtAutoArchiveRead) a = archive_read_new (); + OstreeRepoImportArchiveOptions opts = { 0 }; + char buf[15] = { 0 }; + + if (skip_if_no_xattr (td)) + goto out; + + if (td->skip_all != NULL) + { + g_test_skip (td->skip_all->message); + goto out; + } + + test_archive_setup (td->fd, a); + + opts.ignore_unsupported_content = TRUE; + + if (!import_write_and_ref (td->repo, &opts, a, "baz", NULL, &error)) + goto out; + + /* check contents */ + if (!spawn_cmdline ("ostree --repo=repo checkout baz import-checkout", NULL)) + goto out; + + spawn_cmdline ("ostree --repo=repo ls -R -X baz", NULL); + spawn_cmdline ("attr -l import-checkout/anotherfile", NULL); + + ssize_t x = getxattr ("import-checkout/anotherfile", "user.a_key", buf, sizeof buf - 1); + g_assert_cmpint (x, ==, 8); + g_assert_cmpstr (buf, ==, "my value"); + + x = getxattr ("import-checkout/anotherfile", "user.key2", buf, sizeof buf - 1); + g_assert_cmpint (x, ==, 0); + + x = getxattr ("import-checkout/anotherfile", "user.b_key", buf, sizeof buf - 1); + g_assert_cmpint (x, ==, 14); + g_assert (memcmp(buf, "contains\0nuls", 14) == 0); + + out: + g_assert_no_error (error); +} + +static void +test_libarchive_xattr_import_skip_xattr (gconstpointer data) +{ + TestData *td = (void*)data; + GError *error = NULL; + g_autoptr(OtAutoArchiveRead) a = archive_read_new (); + OstreeRepoImportArchiveOptions opts = { 0 }; + + if (skip_if_no_xattr (td)) + goto out; + + if (td->skip_all != NULL) + { + g_test_skip (td->skip_all->message); + goto out; + } + + test_archive_setup (td->fd, a); + + opts.ignore_unsupported_content = TRUE; + + g_autoptr(OstreeRepoCommitModifier) modifier = ostree_repo_commit_modifier_new ( + OSTREE_REPO_COMMIT_MODIFIER_FLAGS_SKIP_XATTRS, NULL, NULL, NULL); + if (!import_write_and_ref (td->repo, &opts, a, "baz", modifier, &error)) + goto out; + + /* check contents */ + if (!spawn_cmdline ("ostree --repo=repo checkout baz import-checkout", NULL)) + goto out; + + ssize_t n_attrs = listxattr ("import-checkout/anotherfile", NULL, 0); + g_assert_cmpint (n_attrs, ==, 0); + + out: + g_assert_no_error (error); +} + static GVariant* path_cb (OstreeRepo *repo, const char *path, @@ -610,6 +697,8 @@ int main (int argc, char **argv) g_test_add_data_func ("/libarchive/error-device-file", &td, test_libarchive_error_device_file); g_test_add_data_func ("/libarchive/ignore-device-file", &td, test_libarchive_ignore_device_file); g_test_add_data_func ("/libarchive/ostree-convention", &td, test_libarchive_ostree_convention); + g_test_add_data_func ("/libarchive/xattr-import", &td, test_libarchive_xattr_import); + g_test_add_data_func ("/libarchive/xattr-import-skip-xattr", &td, test_libarchive_xattr_import_skip_xattr); g_test_add_data_func ("/libarchive/xattr-callback", &td, test_libarchive_xattr_callback); g_test_add_data_func ("/libarchive/no-use-entry-pathname", &td, test_libarchive_no_use_entry_pathname); g_test_add_data_func ("/libarchive/use-entry-pathname", &td, test_libarchive_use_entry_pathname); From 9567a0e91c9529b5c95f6264cab8541922fde64b Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Wed, 18 Nov 2020 10:24:19 +0000 Subject: [PATCH 03/44] workflow/release: further refinements This tweaks the release GH workflow further so that it only triggers when the `configure.ac` file (which owns the version) changes. Plus it properly checkouts the PR branch to avoid wrongly looking at a synthetic merge commit. --- .github/workflows/release.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index b2cf3f5a..77a5bda3 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -4,6 +4,8 @@ name: Release on: pull_request: branches: [master] + paths: + - 'configure.ac' jobs: ci-release-build: @@ -14,6 +16,7 @@ jobs: - name: Clone repository uses: actions/checkout@v2 with: + ref: ${{ github.event.pull_request.head.sha }} submodules: 'recursive' fetch-depth: '0' - name: Checkout (HEAD) From 8ece70b20770103a283f2abd2d7af85b31f9c21e Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Wed, 18 Nov 2020 11:14:15 +0000 Subject: [PATCH 04/44] templates: add release-checklist This collects all release steps in a release-checklist template. --- .github/ISSUE_TEMPLATE/release-checklist.md | 62 +++++++++++++++++++++ cfg.mk | 2 +- configure.ac | 8 +-- docs/CONTRIBUTING.md | 6 ++ 4 files changed, 70 insertions(+), 8 deletions(-) create mode 100644 .github/ISSUE_TEMPLATE/release-checklist.md diff --git a/.github/ISSUE_TEMPLATE/release-checklist.md b/.github/ISSUE_TEMPLATE/release-checklist.md new file mode 100644 index 00000000..3b02847f --- /dev/null +++ b/.github/ISSUE_TEMPLATE/release-checklist.md @@ -0,0 +1,62 @@ +# Release process + +The release process follows the usual PR-and-review flow, allowing an external reviewer to have a final check before publishing a release. + +## Requirements + +This guide requires: + + * a web browser (and network connectivity) + * `git` + * GPG setup and personal key for signing + * [git-evtag](https://github.com/cgwalters/git-evtag/) + * write access to the git repository + * upload access to this project on GitHub + +## Release checklist + +- Prepare local environment: + - [ ] `git remote get-url --push origin` + - [ ] validate that the output above points to `git@github.com:ostreedev/ostree.git` + - [ ] `git checkout master && git pull` + - [ ] `git clean -fd` + - [ ] `RELEASE_VER=yyyy.n` (matching `package_version` in `configure.ac`) + - [ ] `git checkout -b release-${RELEASE_VER}` + +- Prepare the release commits: + - [ ] `sed -i -e 's/^is_release_build=no/is_release_build=yes/' configure.ac` + - [ ] move the new-symbols stanza (if any) from `src/libostree/libostree-devel.sym` to `src/libostree/libostree-released.sym` + - [ ] comment the `src/libostree/libostree-devel.sym` include in `Makefile-libostree.am` + - [ ] update `tests/test-symbols.sh` with the new digest from `sha256sum src/libostree/libostree-released.sym` + - [ ] `git commit -a -m "Release ${RELEASE_VER}"` + - [ ] `RELEASE_COMMIT=$(git rev-parse HEAD)` + - [ ] `./autogen.sh && make dist` + - [ ] update `year_version` and `release_version` in `configure.ac` for the next development cycle + - [ ] `sed -i -e 's/^is_release_build=yes/is_release_build=no/' configure.ac` + - [ ] `git commit -a -m 'configure: post-release version bump'` + +- Open a PR to create the release: + - [ ] `git push -u origin release-${RELEASE_VER}` + - [ ] open a web browser and create a PR for the branch above, titled `Release ${RELEASE_VER}` + - [ ] make sure the resulting PR contains two commits + - [ ] in the PR body, write a short summary of relevant changes since last release (using `git shortlog` too) + +- [ ] get the PR reviewed, approved and merged + +- Publish the tag: + - [ ] `git fetch origin && git checkout ${RELEASE_COMMIT}` + - [ ] `git-evtag sign v${RELEASE_VER}` + - [ ] `git push --tags origin v${RELEASE_VER}` + +- Publish the release and artifacts on GitHub: + - [ ] find the new tag in the [GitHub tag list](https://github.com/ostreedev/ostree/tags) and click the triple dots menu, then create a release for it + - [ ] write a short changelog (i.e. re-use the PR content) + - [ ] attach `libostree-{RELEASE_VER}.tar.xz` + - [ ] publish release + +- Clean up: + - [ ] `git clean -fd` + - [ ] `git checkout master` + - [ ] `git pull` + - [ ] `git push origin :release-${RELEASE_VER}` + - [ ] `git branch -d release-${RELEASE_VER}` diff --git a/cfg.mk b/cfg.mk index 5947a141..1f5108b7 100644 --- a/cfg.mk +++ b/cfg.mk @@ -39,4 +39,4 @@ sc_glnx_no_fd_close: show-vc-list-except: @$(VC_LIST_EXCEPT) -VC_LIST_ALWAYS_EXCLUDE_REGEX = ^ABOUT-NLS|cfg.mk|maint.mk|*.gpg|*.sig|.xz|.gz$$ +VC_LIST_ALWAYS_EXCLUDE_REGEX = ^ABOUT-NLS|cfg.mk|maint.mk|*.md|*.gpg|*.sig|.xz|.gz$$ diff --git a/configure.ac b/configure.ac index 758fc49e..8ffdf64b 100644 --- a/configure.ac +++ b/configure.ac @@ -1,11 +1,5 @@ AC_PREREQ([2.63]) -dnl To do a release: follow the instructions to update libostree-released.sym from -dnl libostree-devel.sym, update the checksum in test-symbols.sh, set is_release_build=yes -dnl below. Then make another post-release commit to bump the version and set -dnl is_release_build=no. -dnl Seed the release notes with `git-shortlog-with-prs ..`. Then use -dnl `git-evtag` to create the tag and push it. Finally, create a GitHub release and attach -dnl the tarball from `make dist`. +dnl To perform a release, follow the instructions in `docs/CONTRIBUTING.md`. m4_define([year_version], [2020]) m4_define([release_version], [9]) m4_define([package_version], [year_version.release_version]) diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md index 912ea4a8..1cf6250d 100644 --- a/docs/CONTRIBUTING.md +++ b/docs/CONTRIBUTING.md @@ -177,3 +177,9 @@ Instead do this: ## Contributing Tutorial For a detailed walk-through on building, modifying, and testing, see this [tutorial on how to start contributing to OSTree](contributing-tutorial.md). + +## Release process + +Releases can be performed by [creating a new release ticket][new-release-ticket] and following the steps in the checklist there. + +[new-release-ticket]: https://github.com/ostreedev/ostree/new?labels=kind/release&template=release-checklist.md From 07c4249a3f6e4c30812bdffe7ee18cd67e23a87e Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Sun, 22 Nov 2020 13:17:24 +0000 Subject: [PATCH 05/44] test-pull-summary-sigs: Set timestamps to serve expected files If this is not done, the test can fail when the temporary directory is a tmpfs: for example this happens during build-time testing with /var/tmp on tmpfs or TEST_TMPDIR pointing to a tmpfs, or installed-tests with gnome-desktop-testing-runner allocating the test directory on a tmpfs. In particular, many of Debian's official autobuilders now do the entire build and test procedure in a chroot hosted on a tmpfs, to improve build performance and prevent fsync overhead. In this situation, it appears that overwriting summary.sig with a copy of summary.sig.2 is not sufficient for the web server to tell the libostree client that it needs to be re-downloaded. I'm not completely sure why, because tmpfs does appear to have sub-second-resolution timestamps, but forcing a distinct mtime is certainly enough to resolve it. Resolves: https://github.com/ostreedev/ostree/issues/2245 Bug-Debian: https://bugs.debian.org/975418 Signed-off-by: Simon McVittie --- tests/test-pull-summary-sigs.sh | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/test-pull-summary-sigs.sh b/tests/test-pull-summary-sigs.sh index 401e88c9..3819cbf3 100755 --- a/tests/test-pull-summary-sigs.sh +++ b/tests/test-pull-summary-sigs.sh @@ -175,6 +175,8 @@ cd ${test_tmpdir} # Reset to the old valid summary and pull to cache it cp ${test_tmpdir}/ostree-srv/gnomerepo/summary{.1,} cp ${test_tmpdir}/ostree-srv/gnomerepo/summary.sig{.1,} +touch -t 200101010101 ${test_tmpdir}/ostree-srv/gnomerepo/summary +touch -t 200101010101 ${test_tmpdir}/ostree-srv/gnomerepo/summary.sig repo_reinit ${OSTREE} --repo=repo pull origin main assert_has_file repo/tmp/cache/summaries/origin @@ -186,6 +188,7 @@ cmp repo/tmp/cache/summaries/origin.sig ${test_tmpdir}/ostree-srv/gnomerepo/summ # summary signature since it was generated on the server between the # requests cp ${test_tmpdir}/ostree-srv/gnomerepo/summary.sig{.2,} +touch -t 200202020202 ${test_tmpdir}/ostree-srv/gnomerepo/summary.sig if ${OSTREE} --repo=repo pull origin main 2>err.txt; then assert_not_reached "Successful pull with old summary" fi @@ -197,6 +200,7 @@ cmp repo/tmp/cache/summaries/origin.sig ${test_tmpdir}/ostree-srv/gnomerepo/summ # Publish correct summary and check that subsequent pull succeeds cp ${test_tmpdir}/ostree-srv/gnomerepo/summary{.2,} +touch -t 200202020202 ${test_tmpdir}/ostree-srv/gnomerepo/summary ${OSTREE} --repo=repo pull origin main assert_has_file repo/tmp/cache/summaries/origin assert_has_file repo/tmp/cache/summaries/origin.sig @@ -208,6 +212,8 @@ echo "ok pull with signed summary remote old summary" # Reset to the old valid summary and pull to cache it cp ${test_tmpdir}/ostree-srv/gnomerepo/summary{.1,} cp ${test_tmpdir}/ostree-srv/gnomerepo/summary.sig{.1,} +touch -t 200101010101 ${test_tmpdir}/ostree-srv/gnomerepo/summary +touch -t 200101010101 ${test_tmpdir}/ostree-srv/gnomerepo/summary.sig repo_reinit ${OSTREE} --repo=repo pull origin main assert_has_file repo/tmp/cache/summaries/origin @@ -220,6 +226,7 @@ cmp repo/tmp/cache/summaries/origin.sig ${test_tmpdir}/ostree-srv/gnomerepo/summ # is caching the old signature. This should succeed because the cached # old summary is used. cp ${test_tmpdir}/ostree-srv/gnomerepo/summary{.2,} +touch -t 200202020202 ${test_tmpdir}/ostree-srv/gnomerepo/summary ${OSTREE} --repo=repo pull origin main assert_has_file repo/tmp/cache/summaries/origin assert_has_file repo/tmp/cache/summaries/origin.sig @@ -228,6 +235,7 @@ cmp repo/tmp/cache/summaries/origin.sig ${test_tmpdir}/ostree-srv/gnomerepo/summ # Publish correct signature and check that subsequent pull succeeds cp ${test_tmpdir}/ostree-srv/gnomerepo/summary.sig{.2,} +touch -t 200202020202 ${test_tmpdir}/ostree-srv/gnomerepo/summary.sig ${OSTREE} --repo=repo pull origin main assert_has_file repo/tmp/cache/summaries/origin assert_has_file repo/tmp/cache/summaries/origin.sig @@ -239,6 +247,8 @@ echo "ok pull with signed summary remote old summary signature" # Reset to the old valid summary and pull to cache it cp ${test_tmpdir}/ostree-srv/gnomerepo/summary{.1,} cp ${test_tmpdir}/ostree-srv/gnomerepo/summary.sig{.1,} +touch -t 200101010101 ${test_tmpdir}/ostree-srv/gnomerepo/summary +touch -t 200101010101 ${test_tmpdir}/ostree-srv/gnomerepo/summary.sig repo_reinit ${OSTREE} --repo=repo pull origin main assert_has_file repo/tmp/cache/summaries/origin @@ -273,6 +283,8 @@ cmp repo/tmp/cache/summaries/origin.sig ${test_tmpdir}/ostree-srv/gnomerepo/summ # Publish new signature and check that subsequent pull succeeds cp ${test_tmpdir}/ostree-srv/gnomerepo/summary{.2,} cp ${test_tmpdir}/ostree-srv/gnomerepo/summary.sig{.2,} +touch -t 200202020202 ${test_tmpdir}/ostree-srv/gnomerepo/summary +touch -t 200202020202 ${test_tmpdir}/ostree-srv/gnomerepo/summary.sig ${OSTREE} --repo=repo pull origin main assert_has_file repo/tmp/cache/summaries/origin assert_has_file repo/tmp/cache/summaries/origin.sig From 577b1d21c5c7b794e5681c8bd6e422125abba857 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Jard=C3=B3n?= Date: Tue, 1 Dec 2020 20:25:20 +0000 Subject: [PATCH 06/44] README.md: Add Apertis and GNOME OS --- README.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/README.md b/README.md index be6f6453..dc6d8ea4 100644 --- a/README.md +++ b/README.md @@ -38,6 +38,10 @@ For more information, see the [project documentation](docs/index.md) or the ## Operating systems and distributions using OSTree +[Apertis](https://www.apertis.org/) uses libostree for their host system as +well as fatpak. See [update documentation](https://www.apertis.org/guides/ostree/) and +[apertis-update-manager](https://gitlab.apertis.org/pkg/apertis-update-manager) + [Endless OS](https://endlessos.com/) uses libostree for their host system as well as flatpak. See their [eos-updater](https://github.com/endlessm/eos-updater) @@ -59,6 +63,9 @@ uses rpm-ostree as well. where OSTree was born - as a high performance continuous delivery/testing system for GNOME. +[GNOME OS](https://os.gnome.org/) is a testing OS that uses libostree for +their host system as well as flatpak. + [Liri OS](https://liri.io/download/silverblue/) has the option to install their distribution using ostree. From bb1e9ac0fa362ba2b46e1f09bd34073de547b779 Mon Sep 17 00:00:00 2001 From: Phaedrus Leeds Date: Tue, 1 Dec 2020 17:44:51 -0800 Subject: [PATCH 07/44] README: Fix typos of Flatpak --- README.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index dc6d8ea4..ffbb1c8e 100644 --- a/README.md +++ b/README.md @@ -28,7 +28,7 @@ version of - Binary history on the server side (and client) - Introspectable shared library API for build and deployment systems - Flexible support for multiple branches and repositories, supporting - projects like [flatpak](https://github.com/flatpak/flatpak) which + projects like [Flatpak](https://github.com/flatpak/flatpak) which use libostree for applications, rather than hosts. ## Documentation @@ -39,11 +39,11 @@ For more information, see the [project documentation](docs/index.md) or the ## Operating systems and distributions using OSTree [Apertis](https://www.apertis.org/) uses libostree for their host system as -well as fatpak. See [update documentation](https://www.apertis.org/guides/ostree/) and +well as Flatpak. See [update documentation](https://www.apertis.org/guides/ostree/) and [apertis-update-manager](https://gitlab.apertis.org/pkg/apertis-update-manager) [Endless OS](https://endlessos.com/) uses libostree for their host system as -well as flatpak. See +well as Flatpak. See their [eos-updater](https://github.com/endlessm/eos-updater) and [deb-ostree-builder](https://github.com/dbnicholson/deb-ostree-builder) projects. @@ -64,7 +64,7 @@ where OSTree was born - as a high performance continuous delivery/testing system for GNOME. [GNOME OS](https://os.gnome.org/) is a testing OS that uses libostree for -their host system as well as flatpak. +their host system as well as Flatpak. [Liri OS](https://liri.io/download/silverblue/) has the option to install their distribution using ostree. @@ -96,10 +96,10 @@ model for image and package systems. [eos-updater](https://github.com/endlessm/eos-updater) is a daemon that implements updates on EndlessOS. -[flatpak](https://github.com/flatpak/flatpak) uses libostree for desktop -application containers. Unlike most of the other systems here, flatpak does not +[Flatpak](https://github.com/flatpak/flatpak) uses libostree for desktop +application containers. Unlike most of the other systems here, Flatpak does not use the "libostree host system" aspects (e.g. bootloader management), just the -"git-like hardlink dedup". For example, flatpak supports a per-user OSTree +"git-like hardlink dedup". For example, Flatpak supports a per-user OSTree repository. ## Language bindings From 4db2ba0eb1576cbf7bd5bfc0137af1073d007cd5 Mon Sep 17 00:00:00 2001 From: Dan Nicholson Date: Thu, 17 Dec 2020 14:07:08 -0700 Subject: [PATCH 08/44] pull: Allow disabling commit binding verification In some cases such as backups or mirroring you may want to pull commits from one repo to another even if there commits that have incorrect bindings. Fixing the commits in the source repository to have correct bindings may not be feasible, so provide a pull option to disable verification. For Endless we have several repositories that predate collection IDs and ref bindings. Later these repositories gained collection IDs to support the features they provide and ref bindings as the ostree tooling was upgraded. These repositories contain released commits that were valid to the clients they were targeting at the time. Correcting the bindings is not really an option as it would mean invalidating the repository history. --- bash/ostree | 2 ++ man/ostree-pull-local.xml | 8 ++++++++ man/ostree-pull.xml | 8 ++++++++ src/libostree/ostree-repo-pull-private.h | 1 + src/libostree/ostree-repo-pull.c | 23 ++++++++++++++--------- src/ostree/ot-builtin-pull-local.c | 4 ++++ src/ostree/ot-builtin-pull.c | 4 ++++ tests/test-pull-collections.sh | 12 ++++++++++-- 8 files changed, 51 insertions(+), 11 deletions(-) diff --git a/bash/ostree b/bash/ostree index d00695ef..3cc2e04a 100644 --- a/bash/ostree +++ b/bash/ostree @@ -849,6 +849,7 @@ _ostree_pull_local() { --gpg-verify-summary --require-static-deltas --untrusted + --disable-verify-bindings " local options_with_args=" @@ -904,6 +905,7 @@ _ostree_pull() { --untrusted --bareuseronly-files --dry-run + --disable-verify-bindings " local options_with_args=" diff --git a/man/ostree-pull-local.xml b/man/ostree-pull-local.xml index 2bfb2b0f..8bbf36a9 100644 --- a/man/ostree-pull-local.xml +++ b/man/ostree-pull-local.xml @@ -90,6 +90,14 @@ Boston, MA 02111-1307, USA. Do not trust source, verify checksums and don't hardlink into source. + + + + + + Disable verification of commit metadata bindings. + + diff --git a/man/ostree-pull.xml b/man/ostree-pull.xml index 0606f690..593b2d27 100644 --- a/man/ostree-pull.xml +++ b/man/ostree-pull.xml @@ -137,6 +137,14 @@ Boston, MA 02111-1307, USA. Specifies how many times each download should be retried upon error (default: 5) + + + + + + Disable verification of commit metadata bindings. + + diff --git a/src/libostree/ostree-repo-pull-private.h b/src/libostree/ostree-repo-pull-private.h index a827557a..d4c3e971 100644 --- a/src/libostree/ostree-repo-pull-private.h +++ b/src/libostree/ostree-repo-pull-private.h @@ -70,6 +70,7 @@ typedef struct { gboolean require_static_deltas; gboolean disable_static_deltas; gboolean has_tombstone_commits; + gboolean disable_verify_bindings; GBytes *summary_data; char *summary_etag; diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 758c5054..c9b03312 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -1620,15 +1620,17 @@ scan_commit_object (OtPullData *pull_data, if (!ostree_repo_load_commit (pull_data->repo, checksum, &commit, &commitstate, error)) return FALSE; - /* If ref is non-NULL then the commit we fetched was requested through the - * branch, otherwise we requested a commit checksum without specifying a branch. - */ - g_autofree char *remote_collection_id = NULL; - remote_collection_id = get_remote_repo_collection_id (pull_data); - if (!_ostree_repo_verify_bindings (remote_collection_id, - (ref != NULL) ? ref->ref_name : NULL, - commit, error)) - return glnx_prefix_error (error, "Commit %s", checksum); + if (!pull_data->disable_verify_bindings) { + /* If ref is non-NULL then the commit we fetched was requested through the + * branch, otherwise we requested a commit checksum without specifying a branch. + */ + g_autofree char *remote_collection_id = NULL; + remote_collection_id = get_remote_repo_collection_id (pull_data); + if (!_ostree_repo_verify_bindings (remote_collection_id, + (ref != NULL) ? ref->ref_name : NULL, + commit, error)) + return glnx_prefix_error (error, "Commit %s", checksum); + } guint64 new_ts = ostree_commit_get_timestamp (commit); if (pull_data->timestamp_check) @@ -3670,6 +3672,8 @@ all_requested_refs_have_commit (GHashTable *requested_refs /* (element-type Ostr * specified, the `summary` will be downloaded from the remote. Since: 2020.5 * * `summary-sig-bytes` (`ay`): Contents of the `summary.sig` file. If this * is specified, `summary-bytes` must also be specified. Since: 2020.5 + * * `disable-verify-bindings` (`b`): Disable verification of commit bindings. + * Since: 2020.9 */ gboolean ostree_repo_pull_with_options (OstreeRepo *self, @@ -3771,6 +3775,7 @@ ostree_repo_pull_with_options (OstreeRepo *self, g_variant_lookup (options, "ref-keyring-map", "a(sss)", &ref_keyring_map_iter); (void) g_variant_lookup (options, "summary-bytes", "@ay", &summary_bytes_v); (void) g_variant_lookup (options, "summary-sig-bytes", "@ay", &summary_sig_bytes_v); + (void) g_variant_lookup (options, "disable-verify-bindings", "b", &pull_data->disable_verify_bindings); if (pull_data->remote_refspec_name != NULL) pull_data->remote_name = g_strdup (pull_data->remote_refspec_name); diff --git a/src/ostree/ot-builtin-pull-local.c b/src/ostree/ot-builtin-pull-local.c index 43f4f255..1485b7d4 100644 --- a/src/ostree/ot-builtin-pull-local.c +++ b/src/ostree/ot-builtin-pull-local.c @@ -40,6 +40,7 @@ static gboolean opt_bareuseronly_files; static gboolean opt_require_static_deltas; static gboolean opt_gpg_verify; static gboolean opt_gpg_verify_summary; +static gboolean opt_disable_verify_bindings; static int opt_depth = 0; /* ATTENTION: @@ -57,6 +58,7 @@ static GOptionEntry options[] = { { "require-static-deltas", 0, 0, G_OPTION_ARG_NONE, &opt_require_static_deltas, "Require static deltas", NULL }, { "gpg-verify", 0, 0, G_OPTION_ARG_NONE, &opt_gpg_verify, "GPG verify commits (must specify --remote)", NULL }, { "gpg-verify-summary", 0, 0, G_OPTION_ARG_NONE, &opt_gpg_verify_summary, "GPG verify summary (must specify --remote)", NULL }, + { "disable-verify-bindings", 0, 0, G_OPTION_ARG_NONE, &opt_disable_verify_bindings, "Do not verify commit bindings", NULL }, { "depth", 0, 0, G_OPTION_ARG_INT, &opt_depth, "Traverse DEPTH parents (-1=infinite) (default: 0)", "DEPTH" }, { NULL } }; @@ -181,6 +183,8 @@ ostree_builtin_pull_local (int argc, char **argv, OstreeCommandInvocation *invoc if (opt_gpg_verify_summary) g_variant_builder_add (&builder, "{s@v}", "gpg-verify-summary", g_variant_new_variant (g_variant_new_boolean (TRUE))); + g_variant_builder_add (&builder, "{s@v}", "disable-verify-bindings", + g_variant_new_variant (g_variant_new_boolean (opt_disable_verify_bindings))); g_variant_builder_add (&builder, "{s@v}", "depth", g_variant_new_variant (g_variant_new_int32 (opt_depth))); /* local pulls always disable signapi verification. If you don't want this, use diff --git a/src/ostree/ot-builtin-pull.c b/src/ostree/ot-builtin-pull.c index ed0ec556..df3a8d39 100644 --- a/src/ostree/ot-builtin-pull.c +++ b/src/ostree/ot-builtin-pull.c @@ -38,6 +38,7 @@ static gboolean opt_require_static_deltas; static gboolean opt_untrusted; static gboolean opt_http_trusted; static gboolean opt_timestamp_check; +static gboolean opt_disable_verify_bindings; static char* opt_timestamp_check_from_rev; static gboolean opt_bareuseronly_files; static char** opt_subpaths; @@ -76,6 +77,7 @@ static GOptionEntry options[] = { { "localcache-repo", 'L', 0, G_OPTION_ARG_FILENAME_ARRAY, &opt_localcache_repos, "Add REPO as local cache source for objects during this pull", "REPO" }, { "timestamp-check", 'T', 0, G_OPTION_ARG_NONE, &opt_timestamp_check, "Require fetched commits to have newer timestamps", NULL }, { "timestamp-check-from-rev", 0, 0, G_OPTION_ARG_STRING, &opt_timestamp_check_from_rev, "Require fetched commits to have newer timestamps than given rev", NULL }, + { "disable-verify-bindings", 0, 0, G_OPTION_ARG_NONE, &opt_disable_verify_bindings, "Do not verify commit bindings", NULL }, /* let's leave this hidden for now; we just need it for tests */ { "append-user-agent", 0, G_OPTION_FLAG_HIDDEN, G_OPTION_ARG_STRING, &opt_append_user_agent, "Append string to user agent", NULL }, { NULL } @@ -330,6 +332,8 @@ ostree_builtin_pull (int argc, char **argv, OstreeCommandInvocation *invocation, if (opt_per_object_fsync) g_variant_builder_add (&builder, "{s@v}", "per-object-fsync", g_variant_new_variant (g_variant_new_boolean (TRUE))); + g_variant_builder_add (&builder, "{s@v}", "disable-verify-bindings", + g_variant_new_variant (g_variant_new_boolean (opt_disable_verify_bindings))); if (opt_http_headers) { GVariantBuilder hdr_builder; diff --git a/tests/test-pull-collections.sh b/tests/test-pull-collections.sh index cd60ab21..6882e982 100755 --- a/tests/test-pull-collections.sh +++ b/tests/test-pull-collections.sh @@ -117,7 +117,7 @@ do_pull() { local branch=$3 shift 3 - if ${CMD_PREFIX} ostree "--repo=${repo}" pull "${remote_repo}-remote" "${branch}" + if ${CMD_PREFIX} ostree "--repo=${repo}" pull "$@" "${remote_repo}-remote" "${branch}" then return 0 else return 1 fi @@ -129,7 +129,7 @@ do_local_pull() { local branch=$3 shift 3 - if ${CMD_PREFIX} ostree "--repo=${repo}" pull-local "${remote_repo}" "${branch}" + if ${CMD_PREFIX} ostree "--repo=${repo}" pull-local "$@" "${remote_repo}" "${branch}" then return 0 else return 1 fi @@ -221,19 +221,23 @@ if do_pull local collection-repo badcref1 then assert_not_reached "pulling a commit without collection ID from a repo with collection ID should fail" fi +do_pull local collection-repo badcref1 --disable-verify-bindings if do_pull local collection-repo badcref2 then assert_not_reached "pulling a commit with a mismatched collection ID from a repo with collection ID should fail" fi +do_pull local collection-repo badcref2 --disable-verify-bindings if do_pull local collection-repo badcref3 then assert_not_reached "pulling a commit with empty collection ID from repo with collection ID should fail" fi +do_pull local collection-repo badcref3 --disable-verify-bindings do_pull local collection-repo goodcref1 if do_pull local collection-repo badcref4 then assert_not_reached "pulling a commit that was not requested from repo with collection ID should fail" fi +do_pull local collection-repo badcref4 --disable-verify-bindings echo "ok 5 pull refs from remote repos" @@ -243,19 +247,23 @@ if do_local_pull local collection-local-repo badclref1 then assert_not_reached "pulling a commit without collection ID from a repo with collection ID should fail" fi +do_local_pull local collection-local-repo badclref1 --disable-verify-bindings if do_local_pull local collection-local-repo badclref2 then assert_not_reached "pulling a commit with a mismatched collection ID from a repo with collection ID should fail" fi +do_local_pull local collection-local-repo badclref2 --disable-verify-bindings if do_local_pull local collection-local-repo badclref3 then assert_not_reached "pulling a commit with empty collection ID from repo with collection ID should fail" fi +do_local_pull local collection-local-repo badclref3 --disable-verify-bindings do_local_pull local collection-local-repo goodclref1 if do_local_pull local collection-local-repo badclref4 then assert_not_reached "pulling a commit that was not requested from repo with collection ID should fail" fi +do_local_pull local collection-local-repo badclref4 --disable-verify-bindings echo "ok 6 pull refs from local repos" From 92a484d278b653ea6b1c0fa9c1997bb5faf074a0 Mon Sep 17 00:00:00 2001 From: Dan Nicholson Date: Fri, 18 Dec 2020 09:13:38 -0700 Subject: [PATCH 09/44] pull: Use GNU coding style --- src/libostree/ostree-repo-pull.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index c9b03312..7d4b91e2 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -1620,17 +1620,19 @@ scan_commit_object (OtPullData *pull_data, if (!ostree_repo_load_commit (pull_data->repo, checksum, &commit, &commitstate, error)) return FALSE; - if (!pull_data->disable_verify_bindings) { - /* If ref is non-NULL then the commit we fetched was requested through the - * branch, otherwise we requested a commit checksum without specifying a branch. - */ - g_autofree char *remote_collection_id = NULL; - remote_collection_id = get_remote_repo_collection_id (pull_data); - if (!_ostree_repo_verify_bindings (remote_collection_id, - (ref != NULL) ? ref->ref_name : NULL, - commit, error)) - return glnx_prefix_error (error, "Commit %s", checksum); - } + if (!pull_data->disable_verify_bindings) + { + /* If ref is non-NULL then the commit we fetched was requested through + * the branch, otherwise we requested a commit checksum without + * specifying a branch. + */ + g_autofree char *remote_collection_id = NULL; + remote_collection_id = get_remote_repo_collection_id (pull_data); + if (!_ostree_repo_verify_bindings (remote_collection_id, + (ref != NULL) ? ref->ref_name : NULL, + commit, error)) + return glnx_prefix_error (error, "Commit %s", checksum); + } guint64 new_ts = ostree_commit_get_timestamp (commit); if (pull_data->timestamp_check) From 10556a95b4820d713ca440015be9cb2aca6711e2 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sat, 9 Jan 2021 20:42:43 +0000 Subject: [PATCH 10/44] main: Unconditionally set up mount namespace I was being very conservative initially here, but I think it's really safe to just unconditionally set up the mount namespace. This avoids having to check twice for a read-only `/sysroot` (once in the binary and once in the library). --- src/ostree/ot-main.c | 22 +++------------------- 1 file changed, 3 insertions(+), 19 deletions(-) diff --git a/src/ostree/ot-main.c b/src/ostree/ot-main.c index bffa40c4..d153dcec 100644 --- a/src/ostree/ot-main.c +++ b/src/ostree/ot-main.c @@ -122,26 +122,10 @@ maybe_setup_mount_namespace (gboolean *out_ns, if (errno == ENOENT) return TRUE; - glnx_autofd int sysroot_subdir_fd = glnx_opendirat_with_errno (AT_FDCWD, "/sysroot", TRUE); - if (sysroot_subdir_fd < 0) - { - if (errno != ENOENT) - return glnx_throw_errno_prefix (error, "opendirat"); - /* No /sysroot - nothing to do */ - return TRUE; - } - - struct statvfs stvfs; - if (fstatvfs (sysroot_subdir_fd, &stvfs) < 0) - return glnx_throw_errno_prefix (error, "fstatvfs"); - if (stvfs.f_flag & ST_RDONLY) - { - if (unshare (CLONE_NEWNS) < 0) - return glnx_throw_errno_prefix (error, "preparing writable sysroot: unshare (CLONE_NEWNS)"); - - *out_ns = TRUE; - } + if (unshare (CLONE_NEWNS) < 0) + return glnx_throw_errno_prefix (error, "setting up mount namespace: unshare(CLONE_NEWNS)"); + *out_ns = TRUE; return TRUE; } From a1c0cffeb38fb218d9b0eac50e319fa1c79584c1 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sat, 9 Jan 2021 20:40:55 +0000 Subject: [PATCH 11/44] sysroot: Also maintain canonical boot_fd Just like we hold a fd for `/sysroot`, also do so for `/boot` instead of opening and closing it in a few places. This is a preparatory cleanup for further work. --- src/libostree/ostree-sysroot-deploy.c | 32 ++++++++++++-------------- src/libostree/ostree-sysroot-private.h | 4 ++++ src/libostree/ostree-sysroot.c | 15 ++++++++++++ 3 files changed, 34 insertions(+), 17 deletions(-) diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index 900efe49..ad9b56d1 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -1582,11 +1582,11 @@ full_system_sync (OstreeSysroot *self, out_stats->root_syncfs_msec = (end_msec - start_msec); - start_msec = g_get_monotonic_time () / 1000; - glnx_autofd int boot_dfd = -1; - if (!glnx_opendirat (self->sysroot_fd, "boot", TRUE, &boot_dfd, error)) + if (!_ostree_sysroot_ensure_boot_fd (self, error)) return FALSE; - if (!fsfreeze_thaw_cycle (self, boot_dfd, cancellable, error)) + + start_msec = g_get_monotonic_time () / 1000; + if (!fsfreeze_thaw_cycle (self, self->boot_fd, cancellable, error)) return FALSE; end_msec = g_get_monotonic_time () / 1000; out_stats->boot_syncfs_msec = (end_msec - start_msec); @@ -1770,8 +1770,7 @@ install_deployment_kernel (OstreeSysroot *sysroot, cancellable, error)) return FALSE; - glnx_autofd int boot_dfd = -1; - if (!glnx_opendirat (sysroot->sysroot_fd, "boot", TRUE, &boot_dfd, error)) + if (!_ostree_sysroot_ensure_boot_fd (sysroot, error)) return FALSE; const char *osname = ostree_deployment_get_osname (deployment); @@ -1781,14 +1780,14 @@ install_deployment_kernel (OstreeSysroot *sysroot, g_autofree char *bootconf_name = g_strdup_printf ("ostree-%d-%s.conf", n_deployments - ostree_deployment_get_index (deployment), osname); - if (!glnx_shutil_mkdir_p_at (boot_dfd, bootcsumdir, 0775, cancellable, error)) + if (!glnx_shutil_mkdir_p_at (sysroot->boot_fd, bootcsumdir, 0775, cancellable, error)) return FALSE; glnx_autofd int bootcsum_dfd = -1; - if (!glnx_opendirat (boot_dfd, bootcsumdir, TRUE, &bootcsum_dfd, error)) + if (!glnx_opendirat (sysroot->boot_fd, bootcsumdir, TRUE, &bootcsum_dfd, error)) return FALSE; - if (!glnx_shutil_mkdir_p_at (boot_dfd, bootconfdir, 0775, cancellable, error)) + if (!glnx_shutil_mkdir_p_at (sysroot->boot_fd, bootconfdir, 0775, cancellable, error)) return FALSE; /* Install (hardlink/copy) the kernel into /boot/ostree/osname-${bootcsum} if @@ -1879,18 +1878,18 @@ install_deployment_kernel (OstreeSysroot *sysroot, { overlay_initrds = g_ptr_array_new_with_free_func (g_free); - if (!glnx_shutil_mkdir_p_at (boot_dfd, _OSTREE_SYSROOT_BOOT_INITRAMFS_OVERLAYS, + if (!glnx_shutil_mkdir_p_at (sysroot->boot_fd, _OSTREE_SYSROOT_BOOT_INITRAMFS_OVERLAYS, 0755, cancellable, error)) return FALSE; } - if (!glnx_fstatat_allow_noent (boot_dfd, rel_destpath, NULL, 0, error)) + if (!glnx_fstatat_allow_noent (sysroot->boot_fd, rel_destpath, NULL, 0, error)) return FALSE; if (errno == ENOENT) { g_autofree char *srcpath = g_strdup_printf (_OSTREE_SYSROOT_RUNSTATE_STAGED_INITRDS_DIR "/%s", checksum); - if (!install_into_boot (repo, sepolicy, AT_FDCWD, srcpath, boot_dfd, rel_destpath, + if (!install_into_boot (repo, sepolicy, AT_FDCWD, srcpath, sysroot->boot_fd, rel_destpath, cancellable, error)) return FALSE; } @@ -2019,7 +2018,7 @@ install_deployment_kernel (OstreeSysroot *sysroot, ostree_bootconfig_parser_set (bootconfig, "options", options_key); glnx_autofd int bootconf_dfd = -1; - if (!glnx_opendirat (boot_dfd, bootconfdir, TRUE, &bootconf_dfd, error)) + if (!glnx_opendirat (sysroot->boot_fd, bootconfdir, TRUE, &bootconf_dfd, error)) return FALSE; if (!ostree_bootconfig_parser_write_at (ostree_deployment_get_bootconfig (deployment), @@ -2076,15 +2075,14 @@ swap_bootloader (OstreeSysroot *sysroot, g_assert ((current_bootversion == 0 && new_bootversion == 1) || (current_bootversion == 1 && new_bootversion == 0)); - glnx_autofd int boot_dfd = -1; - if (!glnx_opendirat (sysroot->sysroot_fd, "boot", TRUE, &boot_dfd, error)) + if (!_ostree_sysroot_ensure_boot_fd (sysroot, error)) return FALSE; /* The symlink was already written, and we used syncfs() to ensure * its data is in place. Renaming now should give us atomic semantics; * see https://bugzilla.gnome.org/show_bug.cgi?id=755595 */ - if (!glnx_renameat (boot_dfd, "loader.tmp", boot_dfd, "loader", error)) + if (!glnx_renameat (sysroot->boot_fd, "loader.tmp", sysroot->boot_fd, "loader", error)) return FALSE; /* Now we explicitly fsync this directory, even though it @@ -2096,7 +2094,7 @@ swap_bootloader (OstreeSysroot *sysroot, * for whatever reason, and we wouldn't want to confuse the * admin by going back to the previous session. */ - if (fsync (boot_dfd) != 0) + if (fsync (sysroot->boot_fd) != 0) return glnx_throw_errno_prefix (error, "fsync(boot)"); /* TODO: In the future also execute this automatically via a systemd unit diff --git a/src/libostree/ostree-sysroot-private.h b/src/libostree/ostree-sysroot-private.h index 318b0b19..641f0533 100644 --- a/src/libostree/ostree-sysroot-private.h +++ b/src/libostree/ostree-sysroot-private.h @@ -56,6 +56,7 @@ struct OstreeSysroot { GFile *path; int sysroot_fd; + int boot_fd; GLnxLockFile lock; OstreeSysrootLoadState loadstate; @@ -160,6 +161,9 @@ char * _ostree_sysroot_get_runstate_path (OstreeDeployment *deployment, const ch char *_ostree_sysroot_join_lines (GPtrArray *lines); +gboolean +_ostree_sysroot_ensure_boot_fd (OstreeSysroot *self, GError **error); + gboolean _ostree_sysroot_query_bootloader (OstreeSysroot *sysroot, OstreeBootloader **out_bootloader, GCancellable *cancellable, diff --git a/src/libostree/ostree-sysroot.c b/src/libostree/ostree-sysroot.c index e3d7e425..d04b665d 100644 --- a/src/libostree/ostree-sysroot.c +++ b/src/libostree/ostree-sysroot.c @@ -198,6 +198,7 @@ ostree_sysroot_init (OstreeSysroot *self) keys, G_N_ELEMENTS (keys)); self->sysroot_fd = -1; + self->boot_fd = -1; } /** @@ -278,6 +279,19 @@ ensure_sysroot_fd (OstreeSysroot *self, &self->sysroot_fd, error)) return FALSE; } + + return TRUE; +} + +gboolean +_ostree_sysroot_ensure_boot_fd (OstreeSysroot *self, GError **error) +{ + if (self->boot_fd == -1) + { + if (!glnx_opendirat (self->sysroot_fd, "boot", TRUE, + &self->boot_fd, error)) + return FALSE; + } return TRUE; } @@ -380,6 +394,7 @@ void ostree_sysroot_unload (OstreeSysroot *self) { glnx_close_fd (&self->sysroot_fd); + glnx_close_fd (&self->boot_fd); } /** From 9a526bbaa5ad6a75e3b0d8052d93934df3e7a20f Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sat, 9 Jan 2021 18:34:27 +0000 Subject: [PATCH 12/44] sysroot: Handle ro /boot but rw /sysroot The recent change in https://github.com/coreos/fedora-coreos-config/pull/659 broke some of our tests that do `mount -o remount,rw /sysroot` but leave `/boot` read-only. We had code for having `/boot` read-only before `/sysroot` but in practice we had a file descriptor for `/sysroot` that we opened before the remount that would happen later on. Clean things up here so that in the library, we also remount `/boot` at the same time we remount `/sysroot` if either are readonly. Delete the legacy code for remounting `/boot` rw if we're not in a mount namespace. I am fairly confident most users are either using the `ostree` CLI, or they're using the mount namespace. --- src/libostree/ostree-sysroot-deploy.c | 87 +-------------------------- src/libostree/ostree-sysroot.c | 47 +++++++++++---- src/libotutil/otutil.h | 6 ++ 3 files changed, 44 insertions(+), 96 deletions(-) diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index ad9b56d1..0fff820b 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -56,9 +56,6 @@ #define OSTREE_DEPLOYMENT_FINALIZING_ID SD_ID128_MAKE(e8,64,6c,d6,3d,ff,46,25,b7,79,09,a8,e7,a4,09,94) #endif -static gboolean -is_ro_mount (const char *path); - /* * Like symlinkat() but overwrites (atomically) an existing * symlink. @@ -2225,57 +2222,6 @@ cleanup_legacy_current_symlinks (OstreeSysroot *self, return TRUE; } -/* Detect whether or not @path refers to a read-only mountpoint. This is - * currently just used to handle a potentially read-only /boot by transiently - * remounting it read-write. In the future we might also do this for e.g. - * /sysroot. - */ -static gboolean -is_ro_mount (const char *path) -{ -#ifdef HAVE_LIBMOUNT - /* Dragging in all of this crud is apparently necessary just to determine - * whether something is a mount point. - * - * Systemd has a totally different implementation in - * src/basic/mount-util.c. - */ - struct libmnt_table *tb = mnt_new_table_from_file ("/proc/self/mountinfo"); - struct libmnt_fs *fs; - struct libmnt_cache *cache; - gboolean is_mount = FALSE; - struct statvfs stvfsbuf; - - if (!tb) - return FALSE; - - /* to canonicalize all necessary paths */ - cache = mnt_new_cache (); - mnt_table_set_cache (tb, cache); - - fs = mnt_table_find_target(tb, path, MNT_ITER_BACKWARD); - is_mount = fs && mnt_fs_get_target (fs); -#ifdef HAVE_MNT_UNREF_CACHE - mnt_unref_table (tb); - mnt_unref_cache (cache); -#else - mnt_free_table (tb); - mnt_free_cache (cache); -#endif - - if (!is_mount) - return FALSE; - - /* We *could* parse the options, but it seems more reliable to - * introspect the actual mount at runtime. - */ - if (statvfs (path, &stvfsbuf) == 0) - return (stvfsbuf.f_flag & ST_RDONLY) != 0; - -#endif - return FALSE; -} - /** * ostree_sysroot_write_deployments: * @self: Sysroot @@ -2577,42 +2523,13 @@ ostree_sysroot_write_deployments_with_options (OstreeSysroot *self, } else { - gboolean boot_was_ro_mount = FALSE; - if (self->booted_deployment) - boot_was_ro_mount = is_ro_mount ("/boot"); - - g_debug ("boot is ro: %s", boot_was_ro_mount ? "yes" : "no"); - - if (boot_was_ro_mount) - { - if (mount ("/boot", "/boot", NULL, MS_REMOUNT | MS_SILENT, NULL) < 0) - return glnx_throw_errno_prefix (error, "Remounting /boot read-write"); - } - if (!_ostree_sysroot_query_bootloader (self, &bootloader, cancellable, error)) return FALSE; bootloader_is_atomic = bootloader != NULL && _ostree_bootloader_is_atomic (bootloader); - /* Note equivalent of try/finally here */ - gboolean success = write_deployments_bootswap (self, new_deployments, opts, bootloader, - &syncstats, cancellable, error); - /* Below here don't set GError until the if (!success) check. - * Note we only bother remounting if a mount namespace isn't in use. - * */ - if (boot_was_ro_mount && !self->mount_namespace_in_use) - { - if (mount ("/boot", "/boot", NULL, MS_REMOUNT | MS_RDONLY | MS_SILENT, NULL) < 0) - { - /* Only make this a warning because we don't want to - * completely bomb out if some other process happened to - * jump in and open a file there. See above TODO - * around doing this in a new mount namespace. - */ - g_printerr ("warning: Failed to remount /boot read-only: %s\n", strerror (errno)); - } - } - if (!success) + if (!write_deployments_bootswap (self, new_deployments, opts, bootloader, + &syncstats, cancellable, error)) return FALSE; } diff --git a/src/libostree/ostree-sysroot.c b/src/libostree/ostree-sysroot.c index d04b665d..45baf883 100644 --- a/src/libostree/ostree-sysroot.c +++ b/src/libostree/ostree-sysroot.c @@ -295,6 +295,31 @@ _ostree_sysroot_ensure_boot_fd (OstreeSysroot *self, GError **error) return TRUE; } +static gboolean +remount_writable (const char *path, gboolean *did_remount, GError **error) +{ + *did_remount = FALSE; + struct statvfs stvfsbuf; + if (statvfs (path, &stvfsbuf) < 0) + { + if (errno != ENOENT) + return glnx_throw_errno_prefix (error, "statvfs(%s)", path); + else + return TRUE; + } + + if ((stvfsbuf.f_flag & ST_RDONLY) != 0) + { + /* OK, let's remount writable. */ + if (mount (path, path, NULL, MS_REMOUNT | MS_RELATIME, "") < 0) + return glnx_throw_errno_prefix (error, "Remounting %s read-write", path); + *did_remount = TRUE; + g_debug ("remounted %s writable", path); + } + + return TRUE; +} + /* Remount /sysroot read-write if necessary */ gboolean _ostree_sysroot_ensure_writable (OstreeSysroot *self, @@ -314,19 +339,19 @@ _ostree_sysroot_ensure_writable (OstreeSysroot *self, if (!self->root_is_ostree_booted) return TRUE; - /* Check if /sysroot is a read-only mountpoint */ - struct statvfs stvfsbuf; - if (statvfs ("/sysroot", &stvfsbuf) < 0) - return glnx_throw_errno_prefix (error, "fstatvfs(/sysroot)"); - if ((stvfsbuf.f_flag & ST_RDONLY) == 0) - return TRUE; + /* In these cases we also require /boot */ + if (!_ostree_sysroot_ensure_boot_fd (self, error)) + return FALSE; - /* OK, let's remount writable. */ - if (mount ("/sysroot", "/sysroot", NULL, MS_REMOUNT | MS_RELATIME, "") < 0) - return glnx_throw_errno_prefix (error, "Remounting /sysroot read-write"); + gboolean did_remount_sysroot = FALSE; + if (!remount_writable ("/sysroot", &did_remount_sysroot, error)) + return FALSE; + gboolean did_remount_boot = FALSE; + if (!remount_writable ("/boot", &did_remount_boot, error)) + return FALSE; - /* Reopen our fd */ - glnx_close_fd (&self->sysroot_fd); + /* Now close and reopen our file descriptors */ + ostree_sysroot_unload (self); if (!ensure_sysroot_fd (self, error)) return FALSE; diff --git a/src/libotutil/otutil.h b/src/libotutil/otutil.h index 7db7270d..4cc5cd9f 100644 --- a/src/libotutil/otutil.h +++ b/src/libotutil/otutil.h @@ -34,6 +34,12 @@ #define OT_VARIANT_BUILDER_INITIALIZER {{{0,}}} #endif +static inline const char * +ot_booltostr (int b) +{ + return b ? "true" : "false"; +} + #define ot_gobject_refz(o) (o ? g_object_ref (o) : o) #define ot_transfer_out_value(outp, srcp) G_STMT_START { \ From 441233b51c86c252533923835aedcf2ca1972f78 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 7 Jan 2021 21:17:01 +0000 Subject: [PATCH 13/44] repo: Move fsverity bits to ostree-repo-verity.c This file will get larger when we start doing more with fsverity. --- Makefile-libostree.am | 1 + src/libostree/ostree-repo-commit.c | 110 ------------------ src/libostree/ostree-repo-private.h | 2 + src/libostree/ostree-repo-verity.c | 174 ++++++++++++++++++++++++++++ src/libostree/ostree-repo.c | 27 +---- 5 files changed, 178 insertions(+), 136 deletions(-) create mode 100644 src/libostree/ostree-repo-verity.c diff --git a/Makefile-libostree.am b/Makefile-libostree.am index 02dbaded..7f9d7e67 100644 --- a/Makefile-libostree.am +++ b/Makefile-libostree.am @@ -101,6 +101,7 @@ libostree_1_la_SOURCES = \ src/libostree/ostree-repo-libarchive.c \ src/libostree/ostree-repo-prune.c \ src/libostree/ostree-repo-refs.c \ + src/libostree/ostree-repo-verity.c \ src/libostree/ostree-repo-traverse.c \ src/libostree/ostree-repo-private.h \ src/libostree/ostree-repo-file.c \ diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index 690075e1..9d5c6d3b 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -33,9 +33,6 @@ #include #include #include -#ifdef HAVE_LINUX_FSVERITY_H -#include -#endif #include "otutil.h" #include "ostree.h" @@ -190,113 +187,6 @@ ot_security_smack_reset_fd (int fd) #endif } -/* Wrapper around the fsverity ioctl, compressing the result to - * "success, unsupported or error". This is used for /boot where - * we enable verity if supported. - * */ -gboolean -_ostree_tmpf_fsverity_core (GLnxTmpfile *tmpf, - _OstreeFeatureSupport fsverity_requested, - gboolean *supported, - GError **error) -{ - /* Set this by default to simplify the code below */ - if (supported) - *supported = FALSE; - - if (fsverity_requested == _OSTREE_FEATURE_NO) - return TRUE; - -#ifdef HAVE_LINUX_FSVERITY_H - GLNX_AUTO_PREFIX_ERROR ("fsverity", error); - - /* fs-verity requires a read-only file descriptor */ - if (!glnx_tmpfile_reopen_rdonly (tmpf, error)) - return FALSE; - - struct fsverity_enable_arg arg = { 0, }; - arg.version = 1; - arg.hash_algorithm = FS_VERITY_HASH_ALG_SHA256; /* TODO configurable? */ - arg.block_size = 4096; /* FIXME query */ - arg.salt_size = 0; /* TODO store salt in ostree repo config */ - arg.salt_ptr = 0; - arg.sig_size = 0; /* We don't currently expect use of in-kernel signature verification */ - arg.sig_ptr = 0; - - if (ioctl (tmpf->fd, FS_IOC_ENABLE_VERITY, &arg) < 0) - { - switch (errno) - { - case ENOTTY: - case EOPNOTSUPP: - return TRUE; - default: - return glnx_throw_errno_prefix (error, "ioctl(FS_IOC_ENABLE_VERITY)"); - } - } - - if (supported) - *supported = TRUE; -#endif - return TRUE; -} - -/* Enable verity on a file, respecting the "wanted" and "supported" states. - * The main idea here is to optimize out pointlessly calling the ioctl() - * over and over in cases where it's not supported for the repo's filesystem, - * as well as to support "opportunistic" use (requested and if filesystem supports). - * */ -gboolean -_ostree_tmpf_fsverity (OstreeRepo *self, - GLnxTmpfile *tmpf, - GError **error) -{ -#ifdef HAVE_LINUX_FSVERITY_H - g_mutex_lock (&self->txn_lock); - _OstreeFeatureSupport fsverity_wanted = self->fs_verity_wanted; - _OstreeFeatureSupport fsverity_supported = self->fs_verity_supported; - g_mutex_unlock (&self->txn_lock); - - switch (fsverity_wanted) - { - case _OSTREE_FEATURE_YES: - { - if (fsverity_supported == _OSTREE_FEATURE_NO) - return glnx_throw (error, "fsverity required but filesystem does not support it"); - } - break; - case _OSTREE_FEATURE_MAYBE: - break; - case _OSTREE_FEATURE_NO: - return TRUE; - } - - gboolean supported = FALSE; - if (!_ostree_tmpf_fsverity_core (tmpf, fsverity_wanted, &supported, error)) - return FALSE; - - if (!supported) - { - if (G_UNLIKELY (fsverity_wanted == _OSTREE_FEATURE_YES)) - return glnx_throw (error, "fsverity required but filesystem does not support it"); - - /* If we got here, we must be trying "opportunistic" use of fs-verity */ - g_assert_cmpint (fsverity_wanted, ==, _OSTREE_FEATURE_MAYBE); - g_mutex_lock (&self->txn_lock); - self->fs_verity_supported = _OSTREE_FEATURE_NO; - g_mutex_unlock (&self->txn_lock); - return TRUE; - } - - g_mutex_lock (&self->txn_lock); - self->fs_verity_supported = _OSTREE_FEATURE_YES; - g_mutex_unlock (&self->txn_lock); -#else - g_assert_cmpint (self->fs_verity_wanted, !=, _OSTREE_FEATURE_YES); -#endif - return TRUE; -} - /* Given an O_TMPFILE regular file, link it into place. */ gboolean _ostree_repo_commit_tmpf_final (OstreeRepo *self, diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h index 714fda8b..6c01bc6b 100644 --- a/src/libostree/ostree-repo-private.h +++ b/src/libostree/ostree-repo-private.h @@ -521,6 +521,8 @@ OstreeRepoAutoLock * _ostree_repo_auto_lock_push (OstreeRepo *self, void _ostree_repo_auto_lock_cleanup (OstreeRepoAutoLock *lock); G_DEFINE_AUTOPTR_CLEANUP_FUNC (OstreeRepoAutoLock, _ostree_repo_auto_lock_cleanup) +gboolean _ostree_repo_parse_fsverity_config (OstreeRepo *self, GError **error); + gboolean _ostree_tmpf_fsverity_core (GLnxTmpfile *tmpf, _OstreeFeatureSupport fsverity_requested, diff --git a/src/libostree/ostree-repo-verity.c b/src/libostree/ostree-repo-verity.c new file mode 100644 index 00000000..92b026ee --- /dev/null +++ b/src/libostree/ostree-repo-verity.c @@ -0,0 +1,174 @@ +/* + * Copyright (C) Red Hat, Inc. + * + * SPDX-License-Identifier: LGPL-2.0+ + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the + * Free Software Foundation, Inc., 59 Temple Place - Suite 330, + * Boston, MA 02111-1307, USA. + */ + +#include "config.h" + +#include + +#include "ostree-core-private.h" +#include "ostree-repo-private.h" +#include "otutil.h" +#include "ot-fs-utils.h" +#ifdef HAVE_LINUX_FSVERITY_H +#include +#endif + +gboolean +_ostree_repo_parse_fsverity_config (OstreeRepo *self, GError **error) +{ + /* Currently experimental */ + static const char fsverity_key[] = "ex-fsverity"; + self->fs_verity_wanted = _OSTREE_FEATURE_NO; +#ifdef HAVE_LINUX_FSVERITY_H + self->fs_verity_supported = _OSTREE_FEATURE_MAYBE; +#else + self->fs_verity_supported = _OSTREE_FEATURE_NO; +#endif + gboolean fsverity_required = FALSE; + if (!ot_keyfile_get_boolean_with_default (self->config, fsverity_key, "required", + FALSE, &fsverity_required, error)) + return FALSE; + if (fsverity_required) + { + self->fs_verity_wanted = _OSTREE_FEATURE_YES; + if (self->fs_verity_supported == _OSTREE_FEATURE_NO) + return glnx_throw (error, "fsverity required, but libostree compiled without support"); + } + else + { + gboolean fsverity_opportunistic = FALSE; + if (!ot_keyfile_get_boolean_with_default (self->config, fsverity_key, "opportunistic", + FALSE, &fsverity_opportunistic, error)) + return FALSE; + if (fsverity_opportunistic) + self->fs_verity_wanted = _OSTREE_FEATURE_MAYBE; + } + + return TRUE; +} + + +/* Wrapper around the fsverity ioctl, compressing the result to + * "success, unsupported or error". This is used for /boot where + * we enable verity if supported. + * */ +gboolean +_ostree_tmpf_fsverity_core (GLnxTmpfile *tmpf, + _OstreeFeatureSupport fsverity_requested, + gboolean *supported, + GError **error) +{ + /* Set this by default to simplify the code below */ + if (supported) + *supported = FALSE; + + if (fsverity_requested == _OSTREE_FEATURE_NO) + return TRUE; + +#ifdef HAVE_LINUX_FSVERITY_H + GLNX_AUTO_PREFIX_ERROR ("fsverity", error); + + /* fs-verity requires a read-only file descriptor */ + if (!glnx_tmpfile_reopen_rdonly (tmpf, error)) + return FALSE; + + struct fsverity_enable_arg arg = { 0, }; + arg.version = 1; + arg.hash_algorithm = FS_VERITY_HASH_ALG_SHA256; /* TODO configurable? */ + arg.block_size = 4096; /* FIXME query */ + arg.salt_size = 0; /* TODO store salt in ostree repo config */ + arg.salt_ptr = 0; + arg.sig_size = 0; /* We don't currently expect use of in-kernel signature verification */ + arg.sig_ptr = 0; + + if (ioctl (tmpf->fd, FS_IOC_ENABLE_VERITY, &arg) < 0) + { + switch (errno) + { + case ENOTTY: + case EOPNOTSUPP: + return TRUE; + default: + return glnx_throw_errno_prefix (error, "ioctl(FS_IOC_ENABLE_VERITY)"); + } + } + + if (supported) + *supported = TRUE; +#endif + return TRUE; +} + +/* Enable verity on a file, respecting the "wanted" and "supported" states. + * The main idea here is to optimize out pointlessly calling the ioctl() + * over and over in cases where it's not supported for the repo's filesystem, + * as well as to support "opportunistic" use (requested and if filesystem supports). + * */ +gboolean +_ostree_tmpf_fsverity (OstreeRepo *self, + GLnxTmpfile *tmpf, + GError **error) +{ +#ifdef HAVE_LINUX_FSVERITY_H + g_mutex_lock (&self->txn_lock); + _OstreeFeatureSupport fsverity_wanted = self->fs_verity_wanted; + _OstreeFeatureSupport fsverity_supported = self->fs_verity_supported; + g_mutex_unlock (&self->txn_lock); + + switch (fsverity_wanted) + { + case _OSTREE_FEATURE_YES: + { + if (fsverity_supported == _OSTREE_FEATURE_NO) + return glnx_throw (error, "fsverity required but filesystem does not support it"); + } + break; + case _OSTREE_FEATURE_MAYBE: + break; + case _OSTREE_FEATURE_NO: + return TRUE; + } + + gboolean supported = FALSE; + if (!_ostree_tmpf_fsverity_core (tmpf, fsverity_wanted, &supported, error)) + return FALSE; + + if (!supported) + { + if (G_UNLIKELY (fsverity_wanted == _OSTREE_FEATURE_YES)) + return glnx_throw (error, "fsverity required but filesystem does not support it"); + + /* If we got here, we must be trying "opportunistic" use of fs-verity */ + g_assert_cmpint (fsverity_wanted, ==, _OSTREE_FEATURE_MAYBE); + g_mutex_lock (&self->txn_lock); + self->fs_verity_supported = _OSTREE_FEATURE_NO; + g_mutex_unlock (&self->txn_lock); + return TRUE; + } + + g_mutex_lock (&self->txn_lock); + self->fs_verity_supported = _OSTREE_FEATURE_YES; + g_mutex_unlock (&self->txn_lock); +#else + g_assert_cmpint (self->fs_verity_wanted, !=, _OSTREE_FEATURE_YES); +#endif + return TRUE; +} diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 1c951b12..35b1a2b0 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -3017,33 +3017,8 @@ reload_core_config (OstreeRepo *self, } } - /* Currently experimental */ - static const char fsverity_key[] = "ex-fsverity"; - self->fs_verity_wanted = _OSTREE_FEATURE_NO; -#ifdef HAVE_LINUX_FSVERITY_H - self->fs_verity_supported = _OSTREE_FEATURE_MAYBE; -#else - self->fs_verity_supported = _OSTREE_FEATURE_NO; -#endif - gboolean fsverity_required = FALSE; - if (!ot_keyfile_get_boolean_with_default (self->config, fsverity_key, "required", - FALSE, &fsverity_required, error)) + if (!_ostree_repo_parse_fsverity_config (self, error)) return FALSE; - if (fsverity_required) - { - self->fs_verity_wanted = _OSTREE_FEATURE_YES; - if (self->fs_verity_supported == _OSTREE_FEATURE_NO) - return glnx_throw (error, "fsverity required, but libostree compiled without support"); - } - else - { - gboolean fsverity_opportunistic = FALSE; - if (!ot_keyfile_get_boolean_with_default (self->config, fsverity_key, "opportunistic", - FALSE, &fsverity_opportunistic, error)) - return FALSE; - if (fsverity_opportunistic) - self->fs_verity_wanted = _OSTREE_FEATURE_MAYBE; - } { g_clear_pointer (&self->collection_id, g_free); From b4f06b47a38cea303f678e20d89943a2187b963f Mon Sep 17 00:00:00 2001 From: Dan Nicholson Date: Mon, 11 Jan 2021 16:05:05 -0700 Subject: [PATCH 14/44] tests: Ensure no dangling commit partials on remote depth pull This was already being done on the local depth pull test, so this just adds the matching logic to the remote depth pull test. --- tests/test-pull-depth.sh | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/test-pull-depth.sh b/tests/test-pull-depth.sh index 1206c6c4..998a18f5 100755 --- a/tests/test-pull-depth.sh +++ b/tests/test-pull-depth.sh @@ -35,21 +35,31 @@ ${CMD_PREFIX} ostree --repo=repo remote add --set=gpg-verify=false origin $(cat ${CMD_PREFIX} ostree --repo=repo pull --depth=0 origin main find repo/objects -name '*.commit' | wc -l > commitcount assert_file_has_content commitcount "^1$" +find repo/state -name '*.commitpartial' | wc -l > commitpartialcount +assert_file_has_content commitpartialcount "^0$" ${CMD_PREFIX} ostree --repo=repo pull --depth=0 origin main find repo/objects -name '*.commit' | wc -l > commitcount assert_file_has_content commitcount "^1$" +find repo/state -name '*.commitpartial' | wc -l > commitpartialcount +assert_file_has_content commitpartialcount "^0$" ${CMD_PREFIX} ostree --repo=repo pull --depth=1 origin main find repo/objects -name '*.commit' | wc -l > commitcount assert_file_has_content commitcount "^2$" +find repo/state -name '*.commitpartial' | wc -l > commitpartialcount +assert_file_has_content commitpartialcount "^0$" ${CMD_PREFIX} ostree --repo=repo pull --depth=1 origin main find repo/objects -name '*.commit' | wc -l > commitcount assert_file_has_content commitcount "^2$" +find repo/state -name '*.commitpartial' | wc -l > commitpartialcount +assert_file_has_content commitpartialcount "^0$" ${CMD_PREFIX} ostree --repo=repo pull --depth=-1 origin main find repo/objects -name '*.commit' | wc -l > commitcount assert_file_has_content commitcount "^3$" +find repo/state -name '*.commitpartial' | wc -l > commitpartialcount +assert_file_has_content commitpartialcount "^0$" echo "ok pull depth" From 125c83850a9112ea2f9655216ac11070e28ca6b3 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 12 Jan 2021 01:20:23 +0000 Subject: [PATCH 15/44] repo: Make ostree_repo_create_at take nullable options Hit this when trying to use the Rust bindings. --- src/libostree/ostree-repo.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 1c951b12..2469b52e 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -2584,7 +2584,7 @@ ostree_repo_create (OstreeRepo *self, * @dfd: Directory fd * @path: Path * @mode: The mode to store the repository in - * @options: a{sv}: See below for accepted keys + * @options: (nullable): a{sv}: See below for accepted keys * @cancellable: Cancellable * @error: Error * From 20047ff1fea1b0d9193ee21d6df72d70b086090d Mon Sep 17 00:00:00 2001 From: Dan Nicholson Date: Mon, 11 Jan 2021 12:40:38 -0700 Subject: [PATCH 16/44] pull: Error on depth pull with missing head commit When pulling with depth, missing parent commits are ignored. However, the check was applying to any commit, which means that it would succeed even if the requested commit was missing. This might happen on a corrupted remote repo or when using ref data from a stale summary. To achieve this, the semantics of the `commit_to_depth` hash table is changed slightly to only ever includes parent commits. This makes it easy to detect when a parent commit is being referenced (although there is a minor bug there when multiple refs are being pulled) while keeping references to commits that need their `commitpartial` files cleaned up. It also means that the table is only populated on depth pulls, which saves some memory and processing in the common depth=0 case. Fixes: #2265 --- src/libostree/ostree-repo-pull-private.h | 2 +- src/libostree/ostree-repo-pull.c | 56 ++++++++++-------------- tests/test-pull-depth.sh | 34 +++++++++++++- 3 files changed, 57 insertions(+), 35 deletions(-) diff --git a/src/libostree/ostree-repo-pull-private.h b/src/libostree/ostree-repo-pull-private.h index d4c3e971..59b72e88 100644 --- a/src/libostree/ostree-repo-pull-private.h +++ b/src/libostree/ostree-repo-pull-private.h @@ -88,7 +88,7 @@ typedef struct { GHashTable *ref_keyring_map; /* Maps OstreeCollectionRef to keyring remote name */ GPtrArray *static_delta_superblocks; GHashTable *expected_commit_sizes; /* Maps commit checksum to known size */ - GHashTable *commit_to_depth; /* Maps commit checksum maximum depth */ + GHashTable *commit_to_depth; /* Maps parent commit checksum maximum depth */ GHashTable *scanned_metadata; /* Maps object name to itself */ GHashTable *fetched_detached_metadata; /* Map */ GHashTable *requested_metadata; /* Maps object name to itself */ diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 7d4b91e2..abbb5a0d 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -1113,6 +1113,18 @@ on_metadata_written (GObject *object, check_outstanding_requests_handle_error (pull_data, &local_error); } +static gboolean +is_parent_commit (OtPullData *pull_data, + const char *checksum) +{ + /* FIXME: Only parent commits are added to the commit_to_depth table, + * so if the checksum isn't in the table then a new commit chain is + * being started. However, if the desired commit was a parent in a + * previously followed chain, then this will be wrong. + */ + return g_hash_table_contains (pull_data->commit_to_depth, checksum); +} + static void meta_fetch_on_complete (GObject *object, GAsyncResult *result, @@ -1158,7 +1170,8 @@ meta_fetch_on_complete (GObject *object, * We may be pulling from a partial repository that ends in * a dangling parent reference. */ else if (objtype == OSTREE_OBJECT_TYPE_COMMIT && - pull_data->maxdepth != 0) + pull_data->maxdepth != 0 && + is_parent_commit (pull_data, checksum)) { g_clear_error (&local_error); /* If the remote repo supports tombstone commits, check if the commit was intentionally @@ -1542,8 +1555,6 @@ scan_commit_object (OtPullData *pull_data, else { depth = pull_data->maxdepth; - g_hash_table_insert (pull_data->commit_to_depth, g_strdup (checksum), - GINT_TO_POINTER (depth)); } #ifndef OSTREE_DISABLE_GPGME @@ -1684,40 +1695,19 @@ scan_commit_object (OtPullData *pull_data, return FALSE; } - if (parent_csum_bytes != NULL && pull_data->maxdepth == -1) - { - queue_scan_one_metadata_object_c (pull_data, parent_csum_bytes, - OSTREE_OBJECT_TYPE_COMMIT, NULL, - recursion_depth + 1, NULL); - } - else if (parent_csum_bytes != NULL && depth > 0) + if (parent_csum_bytes != NULL && (pull_data->maxdepth == -1 || depth > 0)) { char parent_checksum[OSTREE_SHA256_STRING_LEN+1]; - gpointer parent_depthp; - int parent_depth; - ostree_checksum_inplace_from_bytes (parent_csum_bytes, parent_checksum); - if (g_hash_table_lookup_extended (pull_data->commit_to_depth, parent_checksum, - NULL, &parent_depthp)) - { - parent_depth = GPOINTER_TO_INT (parent_depthp); - } - else - { - parent_depth = depth - 1; - } - - if (parent_depth >= 0) - { - g_hash_table_insert (pull_data->commit_to_depth, g_strdup (parent_checksum), - GINT_TO_POINTER (parent_depth)); - queue_scan_one_metadata_object_c (pull_data, parent_csum_bytes, - OSTREE_OBJECT_TYPE_COMMIT, - NULL, - recursion_depth + 1, - NULL); - } + int parent_depth = (depth > 0) ? depth - 1 : -1; + g_hash_table_insert (pull_data->commit_to_depth, g_strdup (parent_checksum), + GINT_TO_POINTER (parent_depth)); + queue_scan_one_metadata_object_c (pull_data, parent_csum_bytes, + OSTREE_OBJECT_TYPE_COMMIT, + NULL, + recursion_depth + 1, + NULL); } /* We only recurse to looking whether we need dirtree/dirmeta diff --git a/tests/test-pull-depth.sh b/tests/test-pull-depth.sh index 998a18f5..8fb2f597 100755 --- a/tests/test-pull-depth.sh +++ b/tests/test-pull-depth.sh @@ -25,7 +25,7 @@ set -euo pipefail setup_fake_remote_repo1 "archive" -echo '1..1' +echo '1..3' cd ${test_tmpdir} mkdir repo @@ -63,3 +63,35 @@ find repo/state -name '*.commitpartial' | wc -l > commitpartialcount assert_file_has_content commitpartialcount "^0$" echo "ok pull depth" + +# Check that pulling with depth != 0 succeeds with a missing parent +# commit. Prune the remote to truncate the history. +cd ${test_tmpdir} +${CMD_PREFIX} ostree --repo=ostree-srv/gnomerepo prune --refs-only --depth=0 + +rm -rf repo/refs/heads/* repo/refs/remotes/* repo/objects/*/*.commit +${CMD_PREFIX} ostree --repo=repo pull --depth=1 origin main +find repo/objects -name '*.commit' | wc -l > commitcount +assert_file_has_content commitcount "^1$" +find repo/state -name '*.commitpartial' | wc -l > commitpartialcount +assert_file_has_content commitpartialcount "^0$" + +rm -rf repo/refs/heads/* repo/refs/remotes/* repo/objects/*/*.commit +${CMD_PREFIX} ostree --repo=repo pull --depth=-1 origin main +find repo/objects -name '*.commit' | wc -l > commitcount +assert_file_has_content commitcount "^1$" +find repo/state -name '*.commitpartial' | wc -l > commitpartialcount +assert_file_has_content commitpartialcount "^0$" + +echo "ok pull depth missing parent" + +# Check that it errors if the ref head commit is missing. +cd ${test_tmpdir} +rm -f ostree-srv/gnomerepo/objects/*/*.commit + +rm -rf repo/refs/heads/* repo/refs/remotes/* repo/objects/*/*.commit +if ${CMD_PREFIX} ostree --repo=repo pull --depth=-1 origin main; then + fatal "Pull with depth -1 succeeded with missing HEAD commit" +fi + +echo "ok pull depth missing HEAD commit" From d7f2955f3717b452b71cb16c5360ff21f79515e7 Mon Sep 17 00:00:00 2001 From: Dan Nicholson Date: Mon, 11 Jan 2021 16:52:34 -0700 Subject: [PATCH 17/44] pull: Fix local pull with depth and truncated source history The local pull path was erroring on any missing commit, but that prevents a depth pull where the source repo has truncated history. As in the remote case, this also tries to pull in a tombstone commit if the source repo supports it. Fixes: #2266 --- src/libostree/ostree-repo-pull.c | 46 +++++++++++++++++++++++++++++--- tests/test-local-pull-depth.sh | 34 ++++++++++++++++++++++- 2 files changed, 75 insertions(+), 5 deletions(-) diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index abbb5a0d..a95190a5 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -1167,8 +1167,10 @@ meta_fetch_on_complete (GObject *object, } /* When traversing parents, do not fail on a missing commit. - * We may be pulling from a partial repository that ends in - * a dangling parent reference. */ + * We may be pulling from a partial repository that ends in a + * dangling parent reference. This logic should match the + * local case in scan_one_metadata_object. + */ else if (objtype == OSTREE_OBJECT_TYPE_COMMIT && pull_data->maxdepth != 0 && is_parent_commit (pull_data, checksum)) @@ -1820,10 +1822,46 @@ scan_one_metadata_object (OtPullData *pull_data, return FALSE; } + g_autoptr(GError) local_error = NULL; if (!_ostree_repo_import_object (pull_data->repo, pull_data->remote_repo_local, objtype, checksum, pull_data->importflags, - cancellable, error)) - return FALSE; + cancellable, &local_error)) + { + /* When traversing parents, do not fail on a missing commit. + * We may be pulling from a partial repository that ends in a + * dangling parent reference. This logic should match the + * remote case in meta_fetch_on_complete. + * + * Note early return. + */ + if (g_error_matches (local_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND) && + objtype == OSTREE_OBJECT_TYPE_COMMIT && + pull_data->maxdepth != 0 && + is_parent_commit (pull_data, checksum)) + { + g_clear_error (&local_error); + + /* If the remote repo supports tombstone commits, check if + * the commit was intentionally deleted. + */ + if (pull_data->has_tombstone_commits) + { + if (!_ostree_repo_import_object (pull_data->repo, pull_data->remote_repo_local, + OSTREE_OBJECT_TYPE_TOMBSTONE_COMMIT, + checksum, pull_data->importflags, + cancellable, error)) + return FALSE; + } + + return TRUE; + } + else + { + g_propagate_error (error, g_steal_pointer (&local_error)); + return FALSE; + } + } + /* The import API will fetch both the commit and detached metadata, so * add it to the hash to avoid re-fetching it below. */ diff --git a/tests/test-local-pull-depth.sh b/tests/test-local-pull-depth.sh index 96b20b9c..80413bc9 100755 --- a/tests/test-local-pull-depth.sh +++ b/tests/test-local-pull-depth.sh @@ -25,7 +25,7 @@ set -euo pipefail setup_test_repository "archive" -echo "1..1" +echo "1..3" cd ${test_tmpdir} mkdir repo2 @@ -62,3 +62,35 @@ find repo2/state -name '*.commitpartial' | wc -l > commitpartialcount assert_file_has_content commitpartialcount "^0$" echo "ok local pull depth" + +# Check that pulling with depth != 0 succeeds with a missing parent +# commit. Prune the remote to truncate the history. +cd ${test_tmpdir} +${CMD_PREFIX} ostree --repo=repo prune --refs-only --depth=0 + +rm -rf repo2/refs/heads/* repo2/refs/remotes/* repo2/objects/*/*.commit +${CMD_PREFIX} ostree --repo=repo2 pull-local --depth=1 repo +find repo/objects -name '*.commit' | wc -l > commitcount +assert_file_has_content commitcount "^1$" +find repo/state -name '*.commitpartial' | wc -l > commitpartialcount +assert_file_has_content commitpartialcount "^0$" + +rm -rf repo2/refs/heads/* repo2/refs/remotes/* repo2/objects/*/*.commit +${CMD_PREFIX} ostree --repo=repo2 pull-local --depth=-1 repo +find repo/objects -name '*.commit' | wc -l > commitcount +assert_file_has_content commitcount "^1$" +find repo/state -name '*.commitpartial' | wc -l > commitpartialcount +assert_file_has_content commitpartialcount "^0$" + +echo "ok local pull depth missing parent" + +# Check that it errors if the ref head commit is missing. +cd ${test_tmpdir} +rm -f repo/objects/*/*.commit + +rm -rf repo2/refs/heads/* repo2/refs/remotes/* repo2/objects/*/*.commit +if ${CMD_PREFIX} ostree --repo=repo2 pull-local --depth=-1 repo; then + fatal "Local pull with depth -1 succeeded with missing HEAD commit" +fi + +echo "ok local pull depth missing HEAD commit" From 5d730472ae934ee019ee1b635021ccb055578399 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sun, 31 Jan 2021 16:07:05 +0000 Subject: [PATCH 18/44] README.md: Also link apt2ostree Since the topic of Debian+ostree-for-host comes up fairly often. --- README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/README.md b/README.md index ffbb1c8e..42e197bd 100644 --- a/README.md +++ b/README.md @@ -48,6 +48,9 @@ their [eos-updater](https://github.com/endlessm/eos-updater) and [deb-ostree-builder](https://github.com/dbnicholson/deb-ostree-builder) projects. +For Debian/apt, see also https://github.com/stb-tester/apt2ostree +and the LWN article [Merkle trees and build systems](https://lwn.net/Articles/821367/). + Fedora derivatives use rpm-ostree (noted below); there are 3 variants using OSTree: - [Fedora CoreOS](https://getfedora.org/en/coreos/) From afb032e6938fe0ce7480156f65c3b487a9234dfb Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 2 Feb 2021 21:08:54 +0000 Subject: [PATCH 19/44] ci: Don't install deps if running as non-root This way we run in Prow too. --- ci/installdeps.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/installdeps.sh b/ci/installdeps.sh index 7d7c723e..6880d91d 100755 --- a/ci/installdeps.sh +++ b/ci/installdeps.sh @@ -7,7 +7,7 @@ set -xeuo pipefail # cosa buildroot container # https://github.com/coreos/coreos-assembler/pull/730 # And using `yum` at all means we can flake on fetching rpm metadata -if [ -n "${SKIP_INSTALLDEPS:-}" ]; then +if [ -n "${SKIP_INSTALLDEPS:-}" ] || test "$(id -u)" != 0; then exit 0 fi From 6b5aef761298a5f750b5893c063e8f9e18e5d5c9 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 2 Feb 2021 21:04:18 +0000 Subject: [PATCH 20/44] ci: Add new build-check-sanitized.sh All C/C++ projects should use the sanitizers (and static analysis) in their CI. We had this but lost it in one of our CI shuffles; let's readd it. --- ci/build-check-sanitized.sh | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100755 ci/build-check-sanitized.sh diff --git a/ci/build-check-sanitized.sh b/ci/build-check-sanitized.sh new file mode 100755 index 00000000..39c06f43 --- /dev/null +++ b/ci/build-check-sanitized.sh @@ -0,0 +1,12 @@ +#!/usr/bin/bash +# Build with ASAN and UBSAN + unit tests. + +set -xeuo pipefail + +dn=$(dirname $0) +. ${dn}/libbuild.sh +export CFLAGS='-fsanitize=address -fsanitize=undefined -fsanitize-undefined-trap-on-error' +# We leak global state in a few places, fixing that is hard. +export ASAN_OPTIONS='detect_leaks=0' +${dn}/build.sh +make check From 5a5f54a4590e2e98bb64ac6a3926b56c93e33464 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 2 Feb 2021 21:03:53 +0000 Subject: [PATCH 21/44] deltas: Fix leak of matches Found by ASAN. --- src/libostree/ostree-repo-static-delta-compilation.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libostree/ostree-repo-static-delta-compilation.c b/src/libostree/ostree-repo-static-delta-compilation.c index 893ce2fa..0f0828f7 100644 --- a/src/libostree/ostree-repo-static-delta-compilation.c +++ b/src/libostree/ostree-repo-static-delta-compilation.c @@ -636,7 +636,7 @@ try_content_rollsum (OstreeRepo *repo, if (!get_unpacked_unlinked_content (repo, to, &tmp_to, cancellable, error)) return FALSE; - OstreeRollsumMatches *matches = _ostree_compute_rollsum_matches (tmp_from, tmp_to); + g_autoptr(OstreeRollsumMatches) matches = _ostree_compute_rollsum_matches (tmp_from, tmp_to); const guint match_ratio = (matches->bufmatches*100)/matches->total; From bf2c23ca066b58e9c8104d83c3fa0dbeb9014103 Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Wed, 3 Feb 2021 10:04:10 +0000 Subject: [PATCH 22/44] tests/ext/destructive: enhance test logic This enhances external-tests logic, ensuring that destructive tests have retries and some context to pinpoint failures, and that failed-state services are reset between iterations. --- tests/inst/src/destructive.rs | 2 ++ tests/inst/src/rpmostree.rs | 35 +++++++++++++++++++++++++++-------- 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/tests/inst/src/destructive.rs b/tests/inst/src/destructive.rs index d6977bff..42b42ebf 100644 --- a/tests/inst/src/destructive.rs +++ b/tests/inst/src/destructive.rs @@ -393,6 +393,8 @@ fn impl_transaction_test>( // then we'll exit implicitly via the reboot, and reenter the function // above. loop { + // Make sure previously failed services (if any) can run. + bash!("systemctl reset-failed || true")?; // Save the previous strategy as a string so we can use it in error // messages below let prev_strategy_str = format!("{:?}", live_strategy); diff --git a/tests/inst/src/rpmostree.rs b/tests/inst/src/rpmostree.rs index fee97355..b579c4e6 100644 --- a/tests/inst/src/rpmostree.rs +++ b/tests/inst/src/rpmostree.rs @@ -1,7 +1,6 @@ -use anyhow::Result; +use anyhow::{Context, Result}; use serde_derive::Deserialize; -use serde_json; -use std::process::{Command, Stdio}; +use std::process::Command; #[derive(Deserialize)] #[serde(rename_all = "kebab-case")] @@ -25,9 +24,29 @@ pub(crate) struct Deployment { } pub(crate) fn query_status() -> Result { - let cmd = Command::new("rpm-ostree") - .args(&["status", "--json"]) - .stdout(Stdio::piped()) - .spawn()?; - Ok(serde_json::from_reader(cmd.stdout.unwrap())?) + // Retry on temporary activation failures, see + // https://github.com/coreos/rpm-ostree/issues/2531 + let pause = std::time::Duration::from_secs(1); + let mut retries = 0; + let cmd_res = loop { + retries += 1; + let res = Command::new("rpm-ostree") + .args(&["status", "--json"]) + .output() + .context("failed to spawn 'rpm-ostree status'")?; + + if res.status.success() || retries >= 10 { + break res; + } + std::thread::sleep(pause); + }; + + if !cmd_res.status.success() { + anyhow::bail!( + "running 'rpm-ostree status' failed: {}", + String::from_utf8_lossy(&cmd_res.stderr) + ); + } + + serde_json::from_slice(&cmd_res.stdout).context("failed to parse 'rpm-ostree status' output") } From d49f3291adb4494d28cbbdf9c05c3e6f21936754 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 3 Feb 2021 14:48:49 +0000 Subject: [PATCH 23/44] Add --enable-sanitizers, fix `make check` with it It's cleaner if this is an build option rather than being kludged into the CI layer. Notably we can't use `LD_PRELOAD` anymore with ASAN, so update our tests to check for `ASAN_OPTIONS`. --- ci/build-check-sanitized.sh | 3 +-- configure.ac | 21 +++++++++++---------- tests/libtest.sh | 3 +++ 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/ci/build-check-sanitized.sh b/ci/build-check-sanitized.sh index 39c06f43..f0d98431 100755 --- a/ci/build-check-sanitized.sh +++ b/ci/build-check-sanitized.sh @@ -5,8 +5,7 @@ set -xeuo pipefail dn=$(dirname $0) . ${dn}/libbuild.sh -export CFLAGS='-fsanitize=address -fsanitize=undefined -fsanitize-undefined-trap-on-error' # We leak global state in a few places, fixing that is hard. export ASAN_OPTIONS='detect_leaks=0' -${dn}/build.sh +build --disable-gtk-doc --with-curl --with-openssl --enable-sanitizers make check diff --git a/configure.ac b/configure.ac index 8ffdf64b..3a982f53 100644 --- a/configure.ac +++ b/configure.ac @@ -51,16 +51,17 @@ CC_CHECK_FLAGS_APPEND([WARN_CFLAGS], [CFLAGS], [\ ])]) AC_SUBST(WARN_CFLAGS) -AC_MSG_CHECKING([for -fsanitize=address in CFLAGS]) -if echo $CFLAGS | grep -q -e -fsanitize=address; then - AC_MSG_RESULT([yes]) - using_asan=yes -else - AC_MSG_RESULT([no]) -fi -AM_CONDITIONAL(BUILDOPT_ASAN, [test x$using_asan = xyes]) -AM_COND_IF([BUILDOPT_ASAN], - [AC_DEFINE([BUILDOPT_ASAN], 1, [Define if we are building with -fsanitize=address])]) +AC_ARG_ENABLE(sanitizers, + AS_HELP_STRING([--enable-sanitizers], + [Enable ASAN and UBSAN (default: no)]),, + [enable_sanitizers=no]) +AM_CONDITIONAL(BUILDOPT_ASAN, [test x$enable_sanitizers != xno]) +AM_COND_IF([BUILDOPT_ASAN], [ + sanitizer_flags="-fsanitize=address -fsanitize=undefined -fsanitize-undefined-trap-on-error" + CFLAGS="$CFLAGS ${sanitizer_flags}" + CXXFLAGS="$CXXFLAGS ${sanitizer_flags}" + AC_DEFINE([BUILDOPT_ASAN], 1, [Define if we are building with asan and ubsan]) +]) AC_MSG_CHECKING([for -fsanitize=thread in CFLAGS]) if echo $CFLAGS | grep -q -e -fsanitize=thread; then diff --git a/tests/libtest.sh b/tests/libtest.sh index 7c66a5c6..7b654c52 100755 --- a/tests/libtest.sh +++ b/tests/libtest.sh @@ -147,6 +147,9 @@ fi # This is substituted by the build for installed tests BUILT_WITH_ASAN="" +if test -n "${ASAN_OPTIONS:-}"; then + BUILT_WITH_ASAN=1 +fi if test -n "${OT_TESTS_VALGRIND:-}"; then CMD_PREFIX="env G_SLICE=always-malloc OSTREE_SUPPRESS_SYNCFS=1 valgrind -q --error-exitcode=1 --leak-check=full --num-callers=30 --suppressions=${test_srcdir}/glib.supp --suppressions=${test_srcdir}/ostree.supp" From 2195a6099b254dcc513e2ee0289eb5d07ad17287 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 11 Feb 2021 21:19:18 +0000 Subject: [PATCH 24/44] docs: Describe using scratch/empty deltas for initial fetches Came up with a user hitting ratelimiting from S3. --- docs/repository-management.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/docs/repository-management.md b/docs/repository-management.md index 41b8d2b1..ace150ad 100644 --- a/docs/repository-management.md +++ b/docs/repository-management.md @@ -248,5 +248,26 @@ will have "tombstone markers" added so that you know they were explicitly deleted, but all content in them (that is not referenced by a still retained commit) will be garbage collected. + +## Generating "scratch" deltas for efficient initial downloads + +In general, the happy path for OSTree downloads is via static deltas. +If you are in a situation where you want to download an OSTree +commit from an uninitialized repo (or one with unrelated history), you +can generate "scratch" (aka `--empty` deltas) which bundle all +objects for that commit. + +The tradeoff here is increasing server disk space in return +for many fewer client HTTP requests. + +For example: + +``` +$ ostree --repo=/path/to/repo static-delta generate --empty --to=exampleos/x86_64/testing-devel +$ ostree --repo=/path/to/repo summary -u +``` + +After that, clients fetching that commit will prefer fetching the "scratch" delta if they don't have the original ref. + ###### Licensing for this document: `SPDX-License-Identifier: (CC-BY-SA-3.0 OR GFDL-1.3-or-later)` From bf4bbeca14b4e20c69e780251135346485cfc32c Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 16 Feb 2021 15:48:33 +0000 Subject: [PATCH 25/44] README.md: Fix contributing link, add contact section Came up on #fedora-iot channel. --- README.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 42e197bd..fcde7015 100644 --- a/README.md +++ b/README.md @@ -144,9 +144,15 @@ make make install DESTDIR=/path/to/dest ``` +## Contact and discussion forums + +OSTree has a [mailing list](https://mail.gnome.org/archives/ostree-list/) and +there is also an `#ostree` channel on FreeNode. However, asynchronous+logged +communication is preferred for nontrivial questions. + ## Contributing -See [Contributing](docs/CONTRIBUTING.md). +See [Contributing](CONTRIBUTING.md). ## Licensing From 093c63cd46fa2d1da9aa4c8f56e0aa1f9eb786e9 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 19 Feb 2021 01:10:26 +0000 Subject: [PATCH 26/44] refs: Make ostree_repo_resolve_rev{,_ext}() use (nullable) We have an `allow_noent` boolean that controls this, but were missing the `(nullable)` annotation, so the Rust bindings panic when the ref doesn't exist instead of being `Option`. --- src/libostree/ostree-repo-refs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libostree/ostree-repo-refs.c b/src/libostree/ostree-repo-refs.c index d1d679b2..2fb0f04e 100644 --- a/src/libostree/ostree-repo-refs.c +++ b/src/libostree/ostree-repo-refs.c @@ -449,7 +449,7 @@ _ostree_repo_resolve_rev_internal (OstreeRepo *self, * @self: Repo * @refspec: A refspec * @allow_noent: Do not throw an error if refspec does not exist - * @out_rev: (out) (transfer full): A checksum,or %NULL if @allow_noent is true and it does not exist + * @out_rev: (out) (nullable) (transfer full): A checksum,or %NULL if @allow_noent is true and it does not exist * @error: Error * * Look up the given refspec, returning the checksum it references in @@ -472,7 +472,7 @@ ostree_repo_resolve_rev (OstreeRepo *self, * @refspec: A refspec * @allow_noent: Do not throw an error if refspec does not exist * @flags: Options controlling behavior - * @out_rev: (out) (transfer full): A checksum,or %NULL if @allow_noent is true and it does not exist + * @out_rev: (out) (nullable) (transfer full): A checksum,or %NULL if @allow_noent is true and it does not exist * @error: Error * * Look up the given refspec, returning the checksum it references in From dc10bdfb0c0a594cf276a016f332534502b76a58 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 17 Feb 2021 21:59:15 +0000 Subject: [PATCH 27/44] tests/inst: Switch to rpmostree-client from git See discussion in https://github.com/coreos/rpm-ostree/pull/2569#issuecomment-780569188 Currently pinned to a hash, but after the next stable release let's switch to tags --- tests/inst/Cargo.toml | 3 ++ tests/inst/src/destructive.rs | 7 ++--- tests/inst/src/insttestmain.rs | 1 - tests/inst/src/rpmostree.rs | 52 ---------------------------------- 4 files changed, 6 insertions(+), 57 deletions(-) delete mode 100644 tests/inst/src/rpmostree.rs diff --git a/tests/inst/Cargo.toml b/tests/inst/Cargo.toml index 31b43b4e..0986c0a7 100644 --- a/tests/inst/Cargo.toml +++ b/tests/inst/Cargo.toml @@ -37,6 +37,9 @@ strum_macros = "0.18.0" openat = "0.1.19" openat-ext = "0.1.4" nix = "0.17.0" +# See discussion in https://github.com/coreos/rpm-ostree/pull/2569#issuecomment-780569188 +# Currently pinned to a hash, but after the next stable release let's switch to tags +rpmostree-client = { git = "https://github.com/coreos/rpm-ostree", rev = "170095bd60ec8802afeea42d120fb2e88298648f" } # This one I might publish to crates.io, not sure yet with-procspawn-tempdir = { git = "https://github.com/cgwalters/with-procspawn-tempdir" } diff --git a/tests/inst/src/destructive.rs b/tests/inst/src/destructive.rs index 42b42ebf..459587e4 100644 --- a/tests/inst/src/destructive.rs +++ b/tests/inst/src/destructive.rs @@ -31,7 +31,6 @@ use std::time; use strum::IntoEnumIterator; use strum_macros::EnumIter; -use crate::rpmostree; use crate::test::*; const ORIGREF: &'static str = "orig-booted"; @@ -258,7 +257,7 @@ fn parse_and_validate_reboot_mark>( let mut mark: RebootMark = serde_json::from_str(markstr) .with_context(|| format!("Failed to parse reboot mark {:?}", markstr))?; // The first failed reboot may be into the original booted commit - let status = rpmostree::query_status()?; + let status = rpmostree_client::query_status().map_err(anyhow::Error::msg)?; let firstdeploy = &status.deployments[0]; // The first deployment should not be staged assert!(!firstdeploy.staged.unwrap_or(false)); @@ -318,7 +317,7 @@ fn validate_pending_commit(pending_commit: &str, commitstates: &CommitStates) -> /// In the case where we did a kill -9 of rpm-ostree, check the state fn validate_live_interrupted_upgrade(commitstates: &CommitStates) -> Result { - let status = rpmostree::query_status()?; + let status = rpmostree_client::query_status().map_err(anyhow::Error::msg)?; let firstdeploy = &status.deployments[0]; let pending_commit = firstdeploy.checksum.as_str(); let res = if firstdeploy.staged.unwrap_or(false) { @@ -488,7 +487,7 @@ fn impl_transaction_test>( } else { live_strategy = Some(strategy); } - let status = rpmostree::query_status()?; + let status = rpmostree_client::query_status().map_err(anyhow::Error::msg)?; let firstdeploy = &status.deployments[0]; let pending_commit = firstdeploy.checksum.as_str(); validate_pending_commit(pending_commit, &commitstates) diff --git a/tests/inst/src/insttestmain.rs b/tests/inst/src/insttestmain.rs index 3fdc1be1..162f510c 100644 --- a/tests/inst/src/insttestmain.rs +++ b/tests/inst/src/insttestmain.rs @@ -3,7 +3,6 @@ use structopt::StructOpt; mod destructive; mod repobin; -mod rpmostree; mod sysroot; mod test; mod treegen; diff --git a/tests/inst/src/rpmostree.rs b/tests/inst/src/rpmostree.rs deleted file mode 100644 index b579c4e6..00000000 --- a/tests/inst/src/rpmostree.rs +++ /dev/null @@ -1,52 +0,0 @@ -use anyhow::{Context, Result}; -use serde_derive::Deserialize; -use std::process::Command; - -#[derive(Deserialize)] -#[serde(rename_all = "kebab-case")] -#[allow(unused)] -pub(crate) struct Status { - pub(crate) deployments: Vec, -} - -#[derive(Deserialize)] -#[serde(rename_all = "kebab-case")] -#[allow(unused)] -pub(crate) struct Deployment { - pub(crate) unlocked: Option, - pub(crate) osname: String, - pub(crate) pinned: bool, - pub(crate) checksum: String, - pub(crate) staged: Option, - pub(crate) booted: bool, - pub(crate) serial: u32, - pub(crate) origin: String, -} - -pub(crate) fn query_status() -> Result { - // Retry on temporary activation failures, see - // https://github.com/coreos/rpm-ostree/issues/2531 - let pause = std::time::Duration::from_secs(1); - let mut retries = 0; - let cmd_res = loop { - retries += 1; - let res = Command::new("rpm-ostree") - .args(&["status", "--json"]) - .output() - .context("failed to spawn 'rpm-ostree status'")?; - - if res.status.success() || retries >= 10 { - break res; - } - std::thread::sleep(pause); - }; - - if !cmd_res.status.success() { - anyhow::bail!( - "running 'rpm-ostree status' failed: {}", - String::from_utf8_lossy(&cmd_res.stderr) - ); - } - - serde_json::from_slice(&cmd_res.stdout).context("failed to parse 'rpm-ostree status' output") -} From 975496d241d101ed19826bd6bb732dbea9e85813 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 26 Feb 2021 00:42:58 +0000 Subject: [PATCH 28/44] deploy: Add subbootversion to journal To help debug an issue we've seen where `/boot` isn't in sync with the `/ostree/boot` dir, let's log to the journal what we're doing. --- src/libostree/ostree-sysroot-deploy.c | 18 +++++++++++++----- tests/kolainst/destructive/staged-deploy.sh | 2 +- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index 0fff820b..2a06f166 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -1675,6 +1675,7 @@ static gboolean swap_bootlinks (OstreeSysroot *self, int bootversion, GPtrArray *new_deployments, + char **out_subbootdir, GCancellable *cancellable, GError **error) { @@ -1699,7 +1700,8 @@ swap_bootlinks (OstreeSysroot *self, if (!symlink_at_replace (ostree_subbootdir_name, ostree_dfd, ostree_bootdir_name, cancellable, error)) return FALSE; - + if (out_subbootdir) + *out_subbootdir = g_steal_pointer (&ostree_subbootdir_name); return TRUE; } @@ -2253,6 +2255,7 @@ write_deployments_bootswap (OstreeSysroot *self, OstreeSysrootWriteDeploymentsOpts *opts, OstreeBootloader *bootloader, SyncStats *out_syncstats, + char **out_subbootdir, GCancellable *cancellable, GError **error) { @@ -2300,7 +2303,8 @@ write_deployments_bootswap (OstreeSysroot *self, new_deployments, cancellable, error)) return FALSE; - if (!swap_bootlinks (self, new_bootversion, new_deployments, + g_autofree char *new_subbootdir = NULL; + if (!swap_bootlinks (self, new_bootversion, new_deployments, &new_subbootdir, cancellable, error)) return FALSE; @@ -2326,6 +2330,8 @@ write_deployments_bootswap (OstreeSysroot *self, cancellable, error)) return FALSE; + if (out_subbootdir) + *out_subbootdir = g_steal_pointer (&new_subbootdir); return TRUE; } @@ -2504,6 +2510,7 @@ ostree_sysroot_write_deployments_with_options (OstreeSysroot *self, gboolean bootloader_is_atomic = FALSE; SyncStats syncstats = { 0, }; g_autoptr(OstreeBootloader) bootloader = NULL; + g_autofree char *new_subbootdir = NULL; if (!requires_new_bootversion) { if (!create_new_bootlinks (self, self->bootversion, @@ -2515,7 +2522,7 @@ ostree_sysroot_write_deployments_with_options (OstreeSysroot *self, return FALSE; if (!swap_bootlinks (self, self->bootversion, - new_deployments, + new_deployments, &new_subbootdir, cancellable, error)) return FALSE; @@ -2529,14 +2536,15 @@ ostree_sysroot_write_deployments_with_options (OstreeSysroot *self, bootloader_is_atomic = bootloader != NULL && _ostree_bootloader_is_atomic (bootloader); if (!write_deployments_bootswap (self, new_deployments, opts, bootloader, - &syncstats, cancellable, error)) + &syncstats, &new_subbootdir, cancellable, error)) return FALSE; } { g_autofree char *msg = - g_strdup_printf ("%s; bootconfig swap: %s; deployment count change: %i", + g_strdup_printf ("%s; bootconfig swap: %s; bootversion: %s, deployment count change: %i", (bootloader_is_atomic ? "Transaction complete" : "Bootloader updated"), requires_new_bootversion ? "yes" : "no", + new_subbootdir, new_deployments->len - self->deployments->len); const gchar *bootloader_config = ostree_repo_get_bootloader (ostree_sysroot_repo (self)); ot_journal_send ("MESSAGE_ID=" SD_ID128_FORMAT_STR, SD_ID128_FORMAT_VAL(OSTREE_DEPLOYMENT_COMPLETE_ID), diff --git a/tests/kolainst/destructive/staged-deploy.sh b/tests/kolainst/destructive/staged-deploy.sh index a5da0119..0068ed56 100755 --- a/tests/kolainst/destructive/staged-deploy.sh +++ b/tests/kolainst/destructive/staged-deploy.sh @@ -54,7 +54,7 @@ case "${AUTOPKGTEST_REBOOT_MARK:-}" in rpm-ostree status # Assert that the previous boot had a journal entry for it journalctl -b "-1" -u ostree-finalize-staged.service > svc.txt - assert_file_has_content svc.txt 'Bootloader updated; bootconfig swap: yes; deployment count change: 1' + assert_file_has_content svc.txt 'Bootloader updated; bootconfig swap: yes;.*deployment count change: 1' rm -f svc.txt # And there should not be a staged deployment test '!' -f /run/ostree/staged-deployment From 02b61979245e65ab0324ba0b07ea82b384c721d8 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Tue, 2 Mar 2021 15:24:02 -0500 Subject: [PATCH 29/44] lib/sysroot: Add comments and debug statements around sysroot parsing Was looking at this code more closely today to investigate issues related to bootlink mismatches (#2283). --- src/libostree/ostree-sysroot.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/libostree/ostree-sysroot.c b/src/libostree/ostree-sysroot.c index e3d7e425..3dec0e53 100644 --- a/src/libostree/ostree-sysroot.c +++ b/src/libostree/ostree-sysroot.c @@ -449,6 +449,7 @@ _ostree_sysroot_parse_deploy_path_name (const char *name, return TRUE; } +/* For a given bootversion, get its subbootversion from `/ostree/boot.$bootversion`. */ gboolean _ostree_sysroot_read_current_subbootversion (OstreeSysroot *self, int bootversion, @@ -465,6 +466,7 @@ _ostree_sysroot_read_current_subbootversion (OstreeSysroot *self, return FALSE; if (errno == ENOENT) { + g_debug ("Didn't find $sysroot/ostree/boot.%d symlink; assuming subbootversion 0", bootversion); *out_subbootversion = 0; } else @@ -516,6 +518,7 @@ compare_loader_configs_for_sorting (gconstpointer a_pp, return compare_boot_loader_configs (a, b); } +/* Read all the bootconfigs from `/boot/loader/`. */ gboolean _ostree_sysroot_read_boot_loader_configs (OstreeSysroot *self, int bootversion, @@ -574,6 +577,7 @@ _ostree_sysroot_read_boot_loader_configs (OstreeSysroot *self, return TRUE; } +/* Get the bootversion from the `/boot/loader` symlink. */ static gboolean read_current_bootversion (OstreeSysroot *self, int *out_bootversion, @@ -587,6 +591,7 @@ read_current_bootversion (OstreeSysroot *self, return FALSE; if (errno == ENOENT) { + g_debug ("Didn't find $sysroot/boot/loader symlink; assuming bootversion 0"); ret_bootversion = 0; } else @@ -698,7 +703,7 @@ parse_deployment (OstreeSysroot *self, return FALSE; g_autofree char *errprefix = - g_strdup_printf ("Parsing deployment %i in stateroot '%s'", treebootserial, osname); + g_strdup_printf ("Parsing deployment %s in stateroot '%s'", boot_link, osname); GLNX_AUTO_PREFIX_ERROR(errprefix, error); const char *relative_boot_link = boot_link; @@ -799,6 +804,8 @@ get_ostree_kernel_arg_from_config (OstreeBootconfigParser *config) return NULL; } +/* From a BLS config, use its ostree= karg to find the deployment it points to and add it to + * the inout_deployments array. */ static gboolean list_deployments_process_one_boot_entry (OstreeSysroot *self, OstreeBootconfigParser *config, @@ -1016,6 +1023,9 @@ _ostree_sysroot_reload_staged (OstreeSysroot *self, return TRUE; } +/* Loads the current bootversion, subbootversion, and deplyments, starting from the + * bootloader configs which are the source of truth. + */ static gboolean sysroot_load_from_bootloader_configs (OstreeSysroot *self, GCancellable *cancellable, From 3b935f0d22cac59cb0da6ee56510aa8c15d9918f Mon Sep 17 00:00:00 2001 From: Leonardo Graboski Veiga Date: Thu, 4 Mar 2021 16:55:30 -0300 Subject: [PATCH 30/44] docs: Add Torizon to related projects and OS The Torizon platform, includin the TorizonCore OS, the TorizonCore Builder Tool and the Torizon OTA, use OSTree as a base for update the host OS, while the user focus on application development using Docker. Add TorizonCore to the list of Operating systems and distributions using OSTree. Add Torizon and its components to the list of related projects. --- docs/index.md | 4 ++++ docs/related-projects.md | 27 +++++++++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/docs/index.md b/docs/index.md index 083a69b6..57e9439f 100644 --- a/docs/index.md +++ b/docs/index.md @@ -65,6 +65,10 @@ system for GNOME. [Liri OS](https://liri.io/download/silverblue/) has the option to install their distribution using ostree. +[TorizonCore](https://developer.toradex.com/knowledge-base/torizoncore-overview) +uses libostree and Aktualizr as the base for OTA updates from compatible +platforms, including [Torizon OTA](https://developer.toradex.com/knowledge-base/torizon-update-system). + ## Distribution build tools [meta-updater](https://github.com/advancedtelematic/meta-updater) is diff --git a/docs/related-projects.md b/docs/related-projects.md index 7ddf043f..9367c3f1 100644 --- a/docs/related-projects.md +++ b/docs/related-projects.md @@ -362,5 +362,32 @@ The [Balena](https://github.com/resin-os/balena) project forks Docker and aims to even use Docker/OCI format for the root filesystem, and adds wire deltas using librsync. See also [discussion on libostree-list](https://mail.gnome.org/archives/ostree-list/2017-December/msg00002.html). +## Torizon Platform + +[Torizon](https://www.toradex.com/operating-systems/torizon) is an open-source +software platform that simplifies the development and maintenance of embedded +Linux software. It is designed to be used out-of-the-box on devices requiring +high reliability, allowing you to focus on your application and not on building +and maintaining the operating system. + +### TorizonCore + +The platform OS - [TorizonCore](https://developer.toradex.com/knowledge-base/torizoncore-overview) - +is a minimal OS with a Docker runtime and libostree + Aktualizr. The main goal +of this system is to allow application developers to use containers, while the +maintainers of TorizonCore focus on the base system updates. + +### TorizonCore Builder + +Since the TorizonCore OS is meant as a binary distribution, OS customization is +made easier with [TorizonCore Builder](https://developer.toradex.com/knowledge-base/torizoncore-builder-tool), +as the tool abstracts the handling of OSTree concepts from the final users. + +### Torizon OTA + +[Torizon OTA](https://developer.toradex.com/knowledge-base/torizon-update-system) +is a hosted OTA update system that provides OS updates to TorizonCore using +OSTree and Aktualizr. + ###### Licensing for this document: `SPDX-License-Identifier: (CC-BY-SA-3.0 OR GFDL-1.3-or-later)` From d3e40ca7f6bad76f012efb80b528cfe834b7bc64 Mon Sep 17 00:00:00 2001 From: Phaedrus Leeds Date: Mon, 8 Mar 2021 13:18:31 -0800 Subject: [PATCH 31/44] man: Add missing repo mode in config docs --- man/ostree.repo-config.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/man/ostree.repo-config.xml b/man/ostree.repo-config.xml index 6fead168..335f83e4 100644 --- a/man/ostree.repo-config.xml +++ b/man/ostree.repo-config.xml @@ -78,7 +78,7 @@ Boston, MA 02111-1307, USA. mode - One of bare, bare-user or archive-z2 (note that archive is used everywhere else.) + One of bare, bare-user, bare-user-only, or archive-z2 (note that archive is used everywhere else.) From 60881b75ecb4166c89bb9b3392b5126557aaa6f0 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Tue, 9 Mar 2021 11:47:23 +0000 Subject: [PATCH 32/44] ostree-repo-pull: Fix a leak of the summary data if loading from cache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the `summary_sig_not_modified` branch is taken above, both `signatures` and `summary` are loaded from the cache. This makes the `_ostree_repo_load_cache_summary_if_same_sig()` call below redundant (it checks `signatures` matches the file it was just loaded from, and then loads `summary` again) — but that call also currently overwrites `summary` without clearing the old value. Fix this by only making that call if `signatures` was retrieved, but the server said the local `summary` cache was invalid. Signed-off-by: Philip Withnall --- src/libostree/ostree-repo-pull.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index a95190a5..6c5bffcf 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -6674,7 +6674,7 @@ ostree_repo_remote_fetch_summary_with_options (OstreeRepo *self, return FALSE; } - if (signatures) + if (signatures && !summary) { if (!_ostree_repo_load_cache_summary_if_same_sig (self, name, From 2709da4360391f203d7dd1e0e8f69134e0ddb091 Mon Sep 17 00:00:00 2001 From: Phaedrus Leeds Date: Wed, 10 Mar 2021 10:01:04 -0800 Subject: [PATCH 33/44] pull: Fix some whitespace and a comment --- src/libostree/ostree-repo-pull.c | 33 ++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 6c5bffcf..e8c948aa 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -4104,24 +4104,25 @@ ostree_repo_pull_with_options (OstreeRepo *self, &configured_branches, error)) goto out; - /* TODO reindent later */ - { OstreeFetcherURI *first_uri = pull_data->meta_mirrorlist->pdata[0]; + /* Handle file:// URIs */ + { + OstreeFetcherURI *first_uri = pull_data->meta_mirrorlist->pdata[0]; g_autofree char *first_scheme = _ostree_fetcher_uri_get_scheme (first_uri); - /* NB: we don't support local mirrors in mirrorlists, so if this passes, it - * means that we're not using mirrorlists (see also fetch_mirrorlist()) - * Also, we explicitly disable the "local repo" path if static deltas - * were explicitly requested to be required; this is going to happen - * most often for testing deltas without setting up a HTTP server. - */ - if (g_str_equal (first_scheme, "file") && !pull_data->require_static_deltas) - { - g_autofree char *path = _ostree_fetcher_uri_get_path (first_uri); - g_autoptr(GFile) remote_repo_path = g_file_new_for_path (path); - pull_data->remote_repo_local = ostree_repo_new (remote_repo_path); - if (!ostree_repo_open (pull_data->remote_repo_local, cancellable, error)) - goto out; - } + /* NB: we don't support local mirrors in mirrorlists, so if this passes, it + * means that we're not using mirrorlists (see also fetch_mirrorlist()) + * Also, we explicitly disable the "local repo" path if static deltas + * were explicitly requested to be required; this is going to happen + * most often for testing deltas without setting up a HTTP server. + */ + if (g_str_equal (first_scheme, "file") && !pull_data->require_static_deltas) + { + g_autofree char *path = _ostree_fetcher_uri_get_path (first_uri); + g_autoptr(GFile) remote_repo_path = g_file_new_for_path (path); + pull_data->remote_repo_local = ostree_repo_new (remote_repo_path); + if (!ostree_repo_open (pull_data->remote_repo_local, cancellable, error)) + goto out; + } } /* Change some option defaults if we're actually pulling from a local From 19577522f8eacd868cf25d53e1ac0e7f424e952b Mon Sep 17 00:00:00 2001 From: Phaedrus Leeds Date: Wed, 10 Mar 2021 10:02:14 -0800 Subject: [PATCH 34/44] Fix translation of file:// URIs into paths Currently if a file path contains a special character such as '\', and that character is encoded into a file:// URI that is passed to ostree_repo_pull_with_options(), the percent encoding will remain in the path passed to g_file_new() (in the case of backslash %5C) and the pull will then fail with a file not found error. This is an important edge case to handle because by default on many Linux distributions a filesystem with no label is mounted at a path based on its UUID, and this is then passed to systemd-escape by Flatpak (when --enable-auto-sideloading was used at compile time) to create a symbolic link such as this which contains backslashes: $ ls -l /run/flatpak/sideload-repos/ total 0 lrwxrwxrwx 1 mwleeds mwleeds 55 Mar 9 14:21 'automount-run-media-mwleeds-29419e8f\x2dc680\x2d4e95\x2d9a31\x2d2cc907d421cb' -> /run/media/mwleeds/29419e8f-c680-4e95-9a31-2cc907d421cb And Flatpak then passes libostree a file:// URI containing that path, to implement sideloading (pulling content from the USB drive). This results in an error like: Error: While pulling app/org.videolan.VLC/x86_64/stable from remote flathub: /run/flatpak/sideload-repos/automount-run-media-mwleeds-29419e8f%5Cx2dc680%5Cx2d4e95%5Cx2d9a31%5Cx2d2cc907d421cb/.ostree/repo: opendir(/run/flatpak/sideload-repos/automount-run-media-mwleeds-29419e8f%5Cx2dc680%5Cx2d4e95%5Cx2d9a31%5Cx2d2cc907d421cb/.ostree/repo): No such file or directory This patch avoids such errors by using g_file_new_for_uri() instead of g_file_new_for_path(), so that GLib handles the %-decoding for us. Bug report by user: https://community.endlessos.com/t/can-not-install-vlc-from-usb-drive-3-9-3/16353 --- src/libostree/ostree-repo-pull.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index e8c948aa..c63bdc4a 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -4117,8 +4117,8 @@ ostree_repo_pull_with_options (OstreeRepo *self, */ if (g_str_equal (first_scheme, "file") && !pull_data->require_static_deltas) { - g_autofree char *path = _ostree_fetcher_uri_get_path (first_uri); - g_autoptr(GFile) remote_repo_path = g_file_new_for_path (path); + g_autofree char *uri = _ostree_fetcher_uri_to_string (first_uri); + g_autoptr(GFile) remote_repo_path = g_file_new_for_uri (uri); pull_data->remote_repo_local = ostree_repo_new (remote_repo_path); if (!ostree_repo_open (pull_data->remote_repo_local, cancellable, error)) goto out; From 857587615dc36d2d99e1a9bd36486f4267298971 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 11 Mar 2021 01:36:13 +0000 Subject: [PATCH 35/44] Add an API+CLI to inject metadata for bootable OSTree commits I was doing some rpm-ostree work and I wanted to compare two OSTree commits to see if the kernel has changed. I think this should be a lot more natural. Add `ostree commit --bootable` which calls into a new generic library API `ostree_commit_metadata_for_bootable()` that discovers the kernel version and injects it as an `ostree.linux` metadata key. And for extra clarity, add an `ostree.bootable` key. It's interesting because the "core" OSTree layer is all about generic files, but this is adding special APIs around bootable OSTree commits (as opposed to e.g. flatpak as well as things like rpm-ostree's pkgcache refs). Eventually, I'd like to ensure everyone is using this and hard require this metadata key for the `ostree admin deploy` flow - mainly to prevent accidents. --- Makefile-libostree-defines.am | 1 + Makefile-libostree.am | 9 ++-- apidoc/ostree-sections.txt | 1 + bash/ostree | 1 + man/ostree-commit.xml | 7 +++ src/libostree/libostree-devel.sym | 5 ++ src/libostree/ostree-repo-os.c | 83 +++++++++++++++++++++++++++++++ src/libostree/ostree-repo-os.h | 47 +++++++++++++++++ src/libostree/ostree.h | 1 + src/ostree/ot-builtin-commit.c | 15 +++++- tests/admin-test.sh | 6 +++ tests/basic-test.sh | 9 +++- tests/libtest.sh | 9 ++-- 13 files changed, 185 insertions(+), 9 deletions(-) create mode 100644 src/libostree/ostree-repo-os.c create mode 100644 src/libostree/ostree-repo-os.h diff --git a/Makefile-libostree-defines.am b/Makefile-libostree-defines.am index 43e09281..4d290a88 100644 --- a/Makefile-libostree-defines.am +++ b/Makefile-libostree-defines.am @@ -28,6 +28,7 @@ libostree_public_headers = \ src/libostree/ostree-dummy-enumtypes.h \ src/libostree/ostree-mutable-tree.h \ src/libostree/ostree-repo.h \ + src/libostree/ostree-repo-os.h \ src/libostree/ostree-types.h \ src/libostree/ostree-repo-file.h \ src/libostree/ostree-diff.h \ diff --git a/Makefile-libostree.am b/Makefile-libostree.am index 7f9d7e67..98fbb289 100644 --- a/Makefile-libostree.am +++ b/Makefile-libostree.am @@ -92,6 +92,7 @@ libostree_1_la_SOURCES = \ src/libostree/ostree-ref.c \ src/libostree/ostree-remote.c \ src/libostree/ostree-remote-private.h \ + src/libostree/ostree-repo-os.c \ src/libostree/ostree-repo.c \ src/libostree/ostree-repo-checkout.c \ src/libostree/ostree-repo-commit.c \ @@ -186,10 +187,10 @@ endif # USE_GPGME symbol_files = $(top_srcdir)/src/libostree/libostree-released.sym -## Uncomment this include when adding new development symbols. -#if BUILDOPT_IS_DEVEL_BUILD -#symbol_files += $(top_srcdir)/src/libostree/libostree-devel.sym -#endif +# Uncomment this include when adding new development symbols. +if BUILDOPT_IS_DEVEL_BUILD +symbol_files += $(top_srcdir)/src/libostree/libostree-devel.sym +endif # http://blog.jgc.org/2007/06/escaping-comma-and-space-in-gnu-make.html wl_versionscript_arg = -Wl,--version-script= diff --git a/apidoc/ostree-sections.txt b/apidoc/ostree-sections.txt index 64bc68d2..400eb53a 100644 --- a/apidoc/ostree-sections.txt +++ b/apidoc/ostree-sections.txt @@ -152,6 +152,7 @@ ostree_validate_structureof_dirtree ostree_validate_structureof_dirmeta ostree_commit_get_parent ostree_commit_get_timestamp +ostree_commit_metadata_for_bootable ostree_commit_get_content_checksum ostree_commit_get_object_sizes OstreeCommitSizesEntry diff --git a/bash/ostree b/bash/ostree index 3cc2e04a..d1de8530 100644 --- a/bash/ostree +++ b/bash/ostree @@ -321,6 +321,7 @@ _ostree_commit() { --canonical-permissions --editor -e --generate-sizes + --bootable --link-checkout-speedup --no-xattrs --orphan diff --git a/man/ostree-commit.xml b/man/ostree-commit.xml index ab5d3415..81af7bf2 100644 --- a/man/ostree-commit.xml +++ b/man/ostree-commit.xml @@ -186,6 +186,13 @@ Boston, MA 02111-1307, USA. + + + + Inject standard metadata for a bootable Linux filesystem tree. + + + diff --git a/src/libostree/libostree-devel.sym b/src/libostree/libostree-devel.sym index e2d6efc4..541caaa2 100644 --- a/src/libostree/libostree-devel.sym +++ b/src/libostree/libostree-devel.sym @@ -22,6 +22,11 @@ - uncomment the include in Makefile-libostree.am */ +LIBOSTREE_2021.1 { +global: + ostree_commit_metadata_for_bootable; +} LIBOSTREE_2020.8; + /* Stub section for the stable release *after* this development one; don't * edit this other than to update the year. This is just a copy/paste * source. Replace $LASTSTABLE with the last stable version, and $NEWVERSION diff --git a/src/libostree/ostree-repo-os.c b/src/libostree/ostree-repo-os.c new file mode 100644 index 00000000..96beddda --- /dev/null +++ b/src/libostree/ostree-repo-os.c @@ -0,0 +1,83 @@ +/* + * SPDX-License-Identifier: LGPL-2.0+ + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the + * Free Software Foundation, Inc., 59 Temple Place - Suite 330, + * Boston, MA 02111-1307, USA. + */ + +#include "config.h" + +#include +#include +#include +#include +#include +#include "libglnx.h" +#include "ostree.h" +#include "ostree-core-private.h" +#include "ostree-repo-os.h" +#include "otutil.h" + +/** + * ostree_commit_metadata_for_bootable: + * @root: Root filesystem to be committed + * @dict: Dictionary to update + * + * Update provided @dict with standard metadata for bootable OSTree commits. + * Since: 2021.1 + */ +_OSTREE_PUBLIC +gboolean +ostree_commit_metadata_for_bootable (GFile *root, GVariantDict *dict, GCancellable *cancellable, GError **error) +{ + g_autoptr(GFile) modules = g_file_resolve_relative_path (root, "usr/lib/modules"); + g_autoptr(GFileEnumerator) dir_enum + = g_file_enumerate_children (modules, OSTREE_GIO_FAST_QUERYINFO, + G_FILE_QUERY_INFO_NOFOLLOW_SYMLINKS, + cancellable, error); + if (!dir_enum) + return glnx_prefix_error (error, "Opening usr/lib/modules"); + + g_autofree char *linux_release = NULL; + while (TRUE) + { + GFileInfo *child_info; + GFile *child_path; + if (!g_file_enumerator_iterate (dir_enum, &child_info, &child_path, + cancellable, error)) + return FALSE; + if (child_info == NULL) + break; + if (g_file_info_get_file_type (child_info) != G_FILE_TYPE_DIRECTORY) + continue; + + g_autoptr(GFile) kernel_path = g_file_resolve_relative_path (child_path, "vmlinuz"); + if (!g_file_query_exists (kernel_path, NULL)) + continue; + + if (linux_release != NULL) + return glnx_throw (error, "Multiple kernels found in /usr/lib/modules"); + + linux_release = g_strdup (g_file_info_get_name (child_info)); + } + + if (linux_release) + { + g_variant_dict_insert (dict, OSTREE_METADATA_KEY_BOOTABLE, "b", TRUE); + g_variant_dict_insert (dict, OSTREE_METADATA_KEY_LINUX, "s", linux_release); + return TRUE; + } + return glnx_throw (error, "No kernel found in /usr/lib/modules"); +} diff --git a/src/libostree/ostree-repo-os.h b/src/libostree/ostree-repo-os.h new file mode 100644 index 00000000..b2443768 --- /dev/null +++ b/src/libostree/ostree-repo-os.h @@ -0,0 +1,47 @@ +/* + * SPDX-License-Identifier: LGPL-2.0+ + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the + * Free Software Foundation, Inc., 59 Temple Place - Suite 330, + * Boston, MA 02111-1307, USA. + */ + +#pragma once + +#include +#include +#include + +G_BEGIN_DECLS + +/** + * OSTREE_METADATA_KEY_BOOTABLE: + * + * GVariant type `b`: Set if this commit is intended to be bootable + * Since: 2021.1 + */ +#define OSTREE_METADATA_KEY_BOOTABLE "ostree.bootable" +/** + * OSTREE_METADATA_KEY_LINUX: + * + * GVariant type `s`: Contains the Linux kernel release (i.e. `uname -r`) + * Since: 2021.1 + */ +#define OSTREE_METADATA_KEY_LINUX "ostree.linux" + +_OSTREE_PUBLIC +gboolean +ostree_commit_metadata_for_bootable (GFile *root, GVariantDict *dict, GCancellable *cancellable, GError **error); + +G_END_DECLS diff --git a/src/libostree/ostree.h b/src/libostree/ostree.h index 0308d0ed..e4847dbe 100644 --- a/src/libostree/ostree.h +++ b/src/libostree/ostree.h @@ -24,6 +24,7 @@ #include #include #include +#include #include #include #include diff --git a/src/ostree/ot-builtin-commit.c b/src/ostree/ot-builtin-commit.c index 48fa2928..7a23741e 100644 --- a/src/ostree/ot-builtin-commit.c +++ b/src/ostree/ot-builtin-commit.c @@ -35,6 +35,7 @@ static char *opt_subject; static char *opt_body; +static char *opt_bootable; static char *opt_body_file; static gboolean opt_editor; static char *opt_parent; @@ -112,6 +113,7 @@ static GOptionEntry options[] = { { "owner-uid", 0, 0, G_OPTION_ARG_INT, &opt_owner_uid, "Set file ownership user id", "UID" }, { "owner-gid", 0, 0, G_OPTION_ARG_INT, &opt_owner_gid, "Set file ownership group id", "GID" }, { "canonical-permissions", 0, 0, G_OPTION_ARG_NONE, &opt_canonical_permissions, "Canonicalize permissions in the same way bare-user does for hardlinked files", NULL }, + { "bootable", 0, 0, G_OPTION_ARG_NONE, &opt_bootable, "Flag this commit as a bootable OSTree (e.g. contains a Linux kernel)", NULL }, { "mode-ro-executables", 0, 0, G_OPTION_ARG_NONE, &opt_ro_executables, "Ensure executable files are not writable", NULL }, { "no-xattrs", 0, 0, G_OPTION_ARG_NONE, &opt_no_xattrs, "Do not import extended attributes", NULL }, { "selinux-policy", 0, 0, G_OPTION_ARG_FILENAME, &opt_selinux_policy, "Set SELinux labels based on policy in root filesystem PATH (may be /)", "PATH" }, @@ -501,7 +503,7 @@ ostree_builtin_commit (int argc, char **argv, OstreeCommandInvocation *invocatio goto out; } - if (opt_metadata_strings || opt_metadata_variants || opt_metadata_keep) + if (opt_metadata_strings || opt_metadata_variants || opt_metadata_keep || opt_bootable) { g_autoptr(GVariantBuilder) builder = g_variant_builder_new (G_VARIANT_TYPE ("a{sv}")); @@ -841,6 +843,17 @@ ostree_builtin_commit (int argc, char **argv, OstreeCommandInvocation *invocatio fill_bindings (repo, old_metadata, &metadata); } + if (opt_bootable) + { + g_autoptr(GVariant) old_metadata = g_steal_pointer (&metadata); + g_auto(GVariantDict) bootmeta; + g_variant_dict_init (&bootmeta, old_metadata); + if (!ostree_commit_metadata_for_bootable (root, &bootmeta, cancellable, error)) + goto out; + + metadata = g_variant_ref_sink (g_variant_dict_end (&bootmeta)); + } + if (!opt_timestamp) { if (!ostree_repo_write_commit (repo, parent, opt_subject, commit_body, metadata, diff --git a/tests/admin-test.sh b/tests/admin-test.sh index 3aab74cc..b05893ae 100644 --- a/tests/admin-test.sh +++ b/tests/admin-test.sh @@ -65,6 +65,12 @@ assert_not_file_has_content status.txt "pending" assert_not_file_has_content status.txt "rollback" validate_bootloader +# Test the bootable and linux keys +${CMD_PREFIX} ostree --repo=sysroot/ostree/repo --print-metadata-key=ostree.linux show testos:testos/buildmaster/x86_64-runtime >out.txt +assert_file_has_content_literal out.txt 3.6.0 +${CMD_PREFIX} ostree --repo=sysroot/ostree/repo --print-metadata-key=ostree.bootable show testos:testos/buildmaster/x86_64-runtime >out.txt +assert_file_has_content_literal out.txt true + echo "ok deploy command" ${CMD_PREFIX} ostree admin --print-current-dir > curdir diff --git a/tests/basic-test.sh b/tests/basic-test.sh index 9227b0cd..75333f4d 100644 --- a/tests/basic-test.sh +++ b/tests/basic-test.sh @@ -21,7 +21,7 @@ set -euo pipefail -echo "1..$((86 + ${extra_basic_tests:-0}))" +echo "1..$((87 + ${extra_basic_tests:-0}))" CHECKOUT_U_ARG="" CHECKOUT_H_ARGS="-H" @@ -226,6 +226,13 @@ $OSTREE commit ${COMMIT_ARGS} -b test2-no-parent -s '' --parent=none $test_tmpdi assert_streq $($OSTREE log test2-no-parent |grep '^commit' | wc -l) "1" echo "ok commit no parent" +cd ${test_tmpdir} +if $OSTREE commit ${COMMIT_ARGS} -b test-bootable --bootable $test_tmpdir/checkout-test2-4 2>err.txt; then + fatal "committed non-bootable tree" +fi +assert_file_has_content err.txt "error: .*No such file or directory" +echo "ok commit fails bootable if no kernel" + cd ${test_tmpdir} # Do the --bind-ref=, so we store both branches sorted # in metadata and thus the checksums remain the same. diff --git a/tests/libtest.sh b/tests/libtest.sh index 7b654c52..ffb05c7b 100755 --- a/tests/libtest.sh +++ b/tests/libtest.sh @@ -415,11 +415,14 @@ setup_os_repository () { echo "an hmac file" > ${hmac_path} bootcsum=$(cat ${kernel_path} ${initramfs_path} | sha256sum | cut -f 1 -d ' ') export bootcsum + bootable_flag="" # Add the checksum for legacy dirs (/boot, /usr/lib/ostree-boot), but not # /usr/lib/modules. if [[ $bootdir != usr/lib/modules/* ]]; then mv ${kernel_path}{,-${bootcsum}} mv ${initramfs_path}{,-${bootcsum}} + else + bootable_flag="--bootable" fi echo "an executable" > usr/bin/sh @@ -439,12 +442,12 @@ EOF mkdir -p usr/etc/testdirectory echo "a default daemon file" > usr/etc/testdirectory/test - ${CMD_PREFIX} ostree --repo=${test_tmpdir}/testos-repo commit --add-metadata-string version=1.0.9 -b testos/buildmaster/x86_64-runtime -s "Build" + ${CMD_PREFIX} ostree --repo=${test_tmpdir}/testos-repo commit ${bootable_flag} --add-metadata-string version=1.0.9 -b testos/buildmaster/x86_64-runtime -s "Build" # Ensure these commits have distinct second timestamps sleep 2 echo "a new executable" > usr/bin/sh - ${CMD_PREFIX} ostree --repo=${test_tmpdir}/testos-repo commit --add-metadata-string version=1.0.10 -b testos/buildmaster/x86_64-runtime -s "Build" + ${CMD_PREFIX} ostree --repo=${test_tmpdir}/testos-repo commit ${bootable_flag} --add-metadata-string version=1.0.10 -b testos/buildmaster/x86_64-runtime -s "Build" cd ${test_tmpdir} rm -rf osdata-devel @@ -453,7 +456,7 @@ EOF cd osdata-devel mkdir -p usr/include echo "a development header" > usr/include/foo.h - ${CMD_PREFIX} ostree --repo=${test_tmpdir}/testos-repo commit --add-metadata-string version=1.0.9 -b testos/buildmaster/x86_64-devel -s "Build" + ${CMD_PREFIX} ostree --repo=${test_tmpdir}/testos-repo commit ${bootable_flag} --add-metadata-string version=1.0.9 -b testos/buildmaster/x86_64-devel -s "Build" ${CMD_PREFIX} ostree --repo=${test_tmpdir}/testos-repo fsck -q From d11dd7a37b039ef06643559154cc686517421ad4 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 17 Mar 2021 17:13:52 +0000 Subject: [PATCH 36/44] tests/inst: Fix lots of `cargo clippy` warnings Prep for doing this in CI. --- tests/inst/src/destructive.rs | 47 +++++++++++++++------------------- tests/inst/src/insttestmain.rs | 9 +++---- tests/inst/src/test.rs | 11 ++++---- 3 files changed, 30 insertions(+), 37 deletions(-) diff --git a/tests/inst/src/destructive.rs b/tests/inst/src/destructive.rs index 459587e4..e44704cf 100644 --- a/tests/inst/src/destructive.rs +++ b/tests/inst/src/destructive.rs @@ -33,10 +33,10 @@ use strum_macros::EnumIter; use crate::test::*; -const ORIGREF: &'static str = "orig-booted"; -const TESTREF: &'static str = "testcontent"; -const TDATAPATH: &'static str = "/var/tmp/ostree-test-transaction-data.json"; -const SRVREPO: &'static str = "/var/tmp/ostree-test-srv"; +const ORIGREF: &str = "orig-booted"; +const TESTREF: &str = "testcontent"; +const TDATAPATH: &str = "/var/tmp/ostree-test-transaction-data.json"; +const SRVREPO: &str = "/var/tmp/ostree-test-srv"; // Percentage of ELF files to change per update const TREEGEN_PERCENTAGE: u32 = 15; /// Total number of reboots @@ -111,21 +111,18 @@ impl RebootMark { InterruptStrategy::Polite(t) => self .polite .entry(t.clone()) - .or_insert_with(|| BTreeMap::new()), + .or_insert_with(BTreeMap::new), InterruptStrategy::Force(t) => self .force .entry(t.clone()) - .or_insert_with(|| BTreeMap::new()), + .or_insert_with(BTreeMap::new), } } } impl InterruptStrategy { pub(crate) fn is_noop(&self) -> bool { - match self { - InterruptStrategy::Polite(PoliteInterruptStrategy::None) => true, - _ => false, - } + matches!(self, InterruptStrategy::Polite(PoliteInterruptStrategy::None)) } } @@ -324,18 +321,16 @@ fn validate_live_interrupted_upgrade(commitstates: &CommitStates) -> Result>( live_strategy = Some(strategy); } InterruptStrategy::Force(ForceInterruptStrategy::Reboot) => { - mark.reboot_strategy = Some(strategy.clone()); + mark.reboot_strategy = Some(strategy); prepare_reboot(serde_json::to_string(&mark)?)?; // This is a forced reboot - no syncing of the filesystem. bash!("reboot -ff")?; @@ -516,8 +511,8 @@ fn impl_transaction_test>( anyhow::bail!("Failed to wait for uninterrupted upgrade"); } InterruptStrategy::Polite(PoliteInterruptStrategy::Reboot) => { - mark.reboot_strategy = Some(strategy.clone()); - Err(reboot(serde_json::to_string(&mark)?))?; + mark.reboot_strategy = Some(strategy); + return Err(reboot(serde_json::to_string(&mark)?).into()); // We either rebooted, or failed to reboot } InterruptStrategy::Polite(PoliteInterruptStrategy::Stop) => { @@ -545,7 +540,7 @@ fn transactionality() -> Result<()> { // We need this static across reboots let srvrepo = Path::new(SRVREPO); let firstrun = !srvrepo.exists(); - if let Some(_) = mark.as_ref() { + if mark.as_ref().is_some() { if firstrun { anyhow::bail!("Missing {:?}", srvrepo); } @@ -604,7 +599,7 @@ fn transactionality() -> Result<()> { let end = time::Instant::now(); let cycle_time = end.duration_since(start); let tdata = TransactionalTestInfo { - cycle_time: cycle_time, + cycle_time, }; let mut f = std::io::BufWriter::new(std::fs::File::create(&TDATAPATH)?); serde_json::to_writer(&mut f, &tdata)?; diff --git a/tests/inst/src/insttestmain.rs b/tests/inst/src/insttestmain.rs index 162f510c..0101363e 100644 --- a/tests/inst/src/insttestmain.rs +++ b/tests/inst/src/insttestmain.rs @@ -8,10 +8,11 @@ mod test; mod treegen; // Written by Ignition -const DESTRUCTIVE_TEST_STAMP: &'static str = "/etc/ostree-destructive-test-ok"; +const DESTRUCTIVE_TEST_STAMP: &str = "/etc/ostree-destructive-test-ok"; #[derive(Debug, StructOpt)] #[structopt(rename_all = "kebab-case")] +#[allow(clippy::enum_variant_names)] /// Main options struct enum Opt { /// List the destructive tests @@ -74,13 +75,11 @@ fn main() -> Result<()> { for t in test::DESTRUCTIVE_TESTS.iter() { println!("{}", t.name); } - return Ok(()); + Ok(()) } Opt::NonDestructive(subopt) => { // FIXME add method to parse subargs - let iter = match subopt { - NonDestructiveOpts::Args(subargs) => subargs, - }; + let NonDestructiveOpts::Args(iter) = subopt; let libtestargs = libtest_mimic::Arguments::from_iter(iter); let tests: Vec<_> = test::NONDESTRUCTIVE_TESTS .iter() diff --git a/tests/inst/src/test.rs b/tests/inst/src/test.rs index 9d8e156c..1d9c29d5 100644 --- a/tests/inst/src/test.rs +++ b/tests/inst/src/test.rs @@ -41,7 +41,7 @@ pub(crate) fn cmd_fails_with>(mut c: C, pat: &str) -> Resu if o.status.success() { bail!("Command {:?} unexpectedly succeeded", c); } - if !twoway::find_bytes(&o.stderr, pat.as_bytes()).is_some() { + if twoway::find_bytes(&o.stderr, pat.as_bytes()).is_none() { dbg!(String::from_utf8_lossy(&o.stdout)); dbg!(String::from_utf8_lossy(&o.stderr)); bail!("Command {:?} stderr did not match: {}", c, pat); @@ -56,7 +56,7 @@ pub(crate) fn cmd_has_output>(mut c: C, pat: &str) -> Resu if !o.status.success() { bail!("Command {:?} failed", c); } - if !twoway::find_bytes(&o.stdout, pat.as_bytes()).is_some() { + if twoway::find_bytes(&o.stdout, pat.as_bytes()).is_none() { dbg!(String::from_utf8_lossy(&o.stdout)); bail!("Command {:?} stdout did not match: {}", c, pat); } @@ -77,12 +77,12 @@ pub(crate) struct TestHttpServerOpts { pub(crate) random_delay: Option, } -pub(crate) const TEST_HTTP_BASIC_AUTH: &'static str = "foouser:barpw"; +pub(crate) const TEST_HTTP_BASIC_AUTH: &str = "foouser:barpw"; fn validate_authz(value: &[u8]) -> Result { let buf = std::str::from_utf8(&value)?; if let Some(o) = buf.find("Basic ") { - let (_, buf) = buf.split_at(o + "Basic ".len()); + let (_, buf) = buf.split_at(o + "Basic ".len()); let buf = base64::decode(buf).context("decoding")?; let buf = std::str::from_utf8(&buf)?; Ok(buf == TEST_HTTP_BASIC_AUTH) @@ -136,7 +136,6 @@ pub(crate) async fn http_server>( let make_service = make_service_fn(move |_| { let sv = sv.clone(); - let opts = opts.clone(); future::ok::<_, hyper::Error>(service_fn(move |req| handle_request(req, sv.clone(), opts))) }); let server: hyper::Server<_, _, _> = hyper::Server::bind(&addr).serve(make_service); @@ -161,7 +160,7 @@ where let path = path.as_ref(); let mut rt = Runtime::new()?; rt.block_on(async move { - let addr = http_server(path, opts.clone()).await?; + let addr = http_server(path, *opts).await?; tokio::task::spawn_blocking(move || f(&addr)).await? })?; Ok(()) From c52a2ff52e1d495255c03214e3bb703781e739aa Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 17 Mar 2021 18:45:07 +0000 Subject: [PATCH 37/44] tests/inst: cargo fmt --- tests/inst/src/destructive.rs | 23 +++++++++++------------ tests/inst/src/test.rs | 2 +- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/tests/inst/src/destructive.rs b/tests/inst/src/destructive.rs index e44704cf..59ab59b1 100644 --- a/tests/inst/src/destructive.rs +++ b/tests/inst/src/destructive.rs @@ -108,21 +108,22 @@ impl RebootMark { strategy: &InterruptStrategy, ) -> &mut BTreeMap { match strategy { - InterruptStrategy::Polite(t) => self - .polite - .entry(t.clone()) - .or_insert_with(BTreeMap::new), - InterruptStrategy::Force(t) => self - .force - .entry(t.clone()) - .or_insert_with(BTreeMap::new), + InterruptStrategy::Polite(t) => { + self.polite.entry(t.clone()).or_insert_with(BTreeMap::new) + } + InterruptStrategy::Force(t) => { + self.force.entry(t.clone()).or_insert_with(BTreeMap::new) + } } } } impl InterruptStrategy { pub(crate) fn is_noop(&self) -> bool { - matches!(self, InterruptStrategy::Polite(PoliteInterruptStrategy::None)) + matches!( + self, + InterruptStrategy::Polite(PoliteInterruptStrategy::None) + ) } } @@ -598,9 +599,7 @@ fn transactionality() -> Result<()> { upgrade_and_finalize().context("Firstrun upgrade failed")?; let end = time::Instant::now(); let cycle_time = end.duration_since(start); - let tdata = TransactionalTestInfo { - cycle_time, - }; + let tdata = TransactionalTestInfo { cycle_time }; let mut f = std::io::BufWriter::new(std::fs::File::create(&TDATAPATH)?); serde_json::to_writer(&mut f, &tdata)?; f.flush()?; diff --git a/tests/inst/src/test.rs b/tests/inst/src/test.rs index 1d9c29d5..11d23ab7 100644 --- a/tests/inst/src/test.rs +++ b/tests/inst/src/test.rs @@ -82,7 +82,7 @@ pub(crate) const TEST_HTTP_BASIC_AUTH: &str = "foouser:barpw"; fn validate_authz(value: &[u8]) -> Result { let buf = std::str::from_utf8(&value)?; if let Some(o) = buf.find("Basic ") { - let (_, buf) = buf.split_at(o + "Basic ".len()); + let (_, buf) = buf.split_at(o + "Basic ".len()); let buf = base64::decode(buf).context("decoding")?; let buf = std::str::from_utf8(&buf)?; Ok(buf == TEST_HTTP_BASIC_AUTH) From 26166bb8e360927a0ff3eade91a5a687f1f8deaa Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 17 Mar 2021 17:23:47 +0000 Subject: [PATCH 38/44] ci: Add a Github Action for Rust for tests/inst Let's dip our toes in GH Actions (specifically aiming to migrate away from Travis). --- .github/workflows/tests.yml | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 .github/workflows/tests.yml diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml new file mode 100644 index 00000000..42eaac83 --- /dev/null +++ b/.github/workflows/tests.yml @@ -0,0 +1,27 @@ +--- +name: Rust +on: + push: + branches: [master] + pull_request: + branches: [master] + +env: + CARGO_TERM_COLOR: always + ACTIONS_LINTS_TOOLCHAIN: 1.50.0 + +jobs: + linting: + name: "Lints, pinned toolchain" + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v2 + - name: Install toolchain + uses: actions-rs/toolchain@v1 + with: + toolchain: ${{ env['ACTIONS_LINTS_TOOLCHAIN'] }} + default: true + components: rustfmt, clippy + - name: cargo fmt (check) + run: cd tests/inst && cargo fmt -- --check -l From 1b28e6041c33b3f1508dda458ed7ed97aed4375e Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 17 Mar 2021 14:03:49 +0000 Subject: [PATCH 39/44] sysroot: Add _require_booted_deployment() API This is a common pattern that is replicated both in our code and in rpm-ostree a lot. Let's add a canonical API. --- apidoc/ostree-sections.txt | 1 + src/libostree/libostree-devel.sym | 1 + src/libostree/ostree-sysroot-deploy.c | 4 ++-- src/libostree/ostree-sysroot.c | 21 +++++++++++++++++++++ src/libostree/ostree-sysroot.h | 3 +++ src/ostree/ot-admin-builtin-set-origin.c | 8 ++------ src/ostree/ot-admin-builtin-unlock.c | 8 ++------ tests/admin-test.sh | 2 +- 8 files changed, 33 insertions(+), 15 deletions(-) diff --git a/apidoc/ostree-sections.txt b/apidoc/ostree-sections.txt index 400eb53a..e4954c70 100644 --- a/apidoc/ostree-sections.txt +++ b/apidoc/ostree-sections.txt @@ -533,6 +533,7 @@ ostree_sysroot_get_bootversion ostree_sysroot_get_subbootversion ostree_sysroot_get_deployments ostree_sysroot_get_booted_deployment +ostree_sysroot_require_booted_deployment ostree_sysroot_get_deployment_directory ostree_sysroot_get_deployment_dirpath ostree_sysroot_get_deployment_origin_path diff --git a/src/libostree/libostree-devel.sym b/src/libostree/libostree-devel.sym index 541caaa2..02159cd2 100644 --- a/src/libostree/libostree-devel.sym +++ b/src/libostree/libostree-devel.sym @@ -25,6 +25,7 @@ LIBOSTREE_2021.1 { global: ostree_commit_metadata_for_bootable; + ostree_sysroot_require_booted_deployment; } LIBOSTREE_2020.8; /* 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 2a06f166..32748a62 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -3066,9 +3066,9 @@ ostree_sysroot_stage_tree_with_options (OstreeSysroot *self, if (!_ostree_sysroot_ensure_writable (self, error)) return FALSE; - OstreeDeployment *booted_deployment = ostree_sysroot_get_booted_deployment (self); + OstreeDeployment *booted_deployment = ostree_sysroot_require_booted_deployment (self, error); if (booted_deployment == NULL) - return glnx_throw (error, "Cannot stage a deployment when not currently booted into an OSTree system"); + return glnx_prefix_error (error, "Cannot stage deployment"); /* This is used by the testsuite to exercise the path unit, until it becomes the default * (which is pending on the preset making it everywhere). */ diff --git a/src/libostree/ostree-sysroot.c b/src/libostree/ostree-sysroot.c index 2db47a53..b0ae66cf 100644 --- a/src/libostree/ostree-sysroot.c +++ b/src/libostree/ostree-sysroot.c @@ -1246,6 +1246,27 @@ ostree_sysroot_get_booted_deployment (OstreeSysroot *self) return self->booted_deployment; } + +/** + * ostree_sysroot_require_booted_deployment: + * @self: Sysroot + * + * Find the booted deployment, or return an error if not booted via OSTree. + * + * Returns: (transfer none) (not nullable): The currently booted deployment, or an error + * Since: 2021.1 + */ +OstreeDeployment * +ostree_sysroot_require_booted_deployment (OstreeSysroot *self, GError **error) +{ + g_return_val_if_fail (self->loadstate == OSTREE_SYSROOT_LOAD_STATE_LOADED, NULL); + + if (!self->booted_deployment) + return glnx_null_throw (error, "Not currently booted into an OSTree system"); + return self->booted_deployment; +} + + /** * ostree_sysroot_get_staged_deployment: * @self: Sysroot diff --git a/src/libostree/ostree-sysroot.h b/src/libostree/ostree-sysroot.h index 3a3b6a77..036b81e8 100644 --- a/src/libostree/ostree-sysroot.h +++ b/src/libostree/ostree-sysroot.h @@ -85,6 +85,9 @@ GPtrArray *ostree_sysroot_get_deployments (OstreeSysroot *self); _OSTREE_PUBLIC OstreeDeployment *ostree_sysroot_get_booted_deployment (OstreeSysroot *self); _OSTREE_PUBLIC +OstreeDeployment * +ostree_sysroot_require_booted_deployment (OstreeSysroot *self, GError **error); +_OSTREE_PUBLIC OstreeDeployment *ostree_sysroot_get_staged_deployment (OstreeSysroot *self); _OSTREE_PUBLIC diff --git a/src/ostree/ot-admin-builtin-set-origin.c b/src/ostree/ot-admin-builtin-set-origin.c index 4dc68a00..b133dc58 100644 --- a/src/ostree/ot-admin-builtin-set-origin.c +++ b/src/ostree/ot-admin-builtin-set-origin.c @@ -75,13 +75,9 @@ ot_admin_builtin_set_origin (int argc, char **argv, OstreeCommandInvocation *inv if (opt_index == -1) { - target_deployment = ostree_sysroot_get_booted_deployment (sysroot); + target_deployment = ostree_sysroot_require_booted_deployment (sysroot, error); if (target_deployment == NULL) - { - g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED, - "Not currently booted into an OSTree system"); - goto out; - } + goto out; /* To match the below */ target_deployment = g_object_ref (target_deployment); } diff --git a/src/ostree/ot-admin-builtin-unlock.c b/src/ostree/ot-admin-builtin-unlock.c index 6c265f54..77fe8be3 100644 --- a/src/ostree/ot-admin-builtin-unlock.c +++ b/src/ostree/ot-admin-builtin-unlock.c @@ -61,13 +61,9 @@ ot_admin_builtin_unlock (int argc, char **argv, OstreeCommandInvocation *invocat goto out; } - booted_deployment = ostree_sysroot_get_booted_deployment (sysroot); + booted_deployment = ostree_sysroot_require_booted_deployment (sysroot, error); if (!booted_deployment) - { - g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED, - "Not currently booted into an OSTree system"); - goto out; - } + goto out; if (opt_hotfix && opt_transient) { diff --git a/tests/admin-test.sh b/tests/admin-test.sh index b05893ae..a7016a87 100644 --- a/tests/admin-test.sh +++ b/tests/admin-test.sh @@ -95,7 +95,7 @@ echo "ok layout" if ${CMD_PREFIX} ostree admin deploy --stage --os=testos testos:testos/buildmaster/x86_64-runtime 2>err.txt; then fatal "staged when not booted" fi -assert_file_has_content_literal err.txt "Cannot stage a deployment when not currently booted into an OSTree system" +assert_file_has_content_literal err.txt "Cannot stage deployment: Not currently booted into an OSTree system" echo "ok staging does not work when not booted" orig_mtime=$(stat -c '%.Y' sysroot/ostree/deploy) From cfe313afae7aff392442838420125e2c73102aa9 Mon Sep 17 00:00:00 2001 From: "Kenneth J. Miller" Date: Thu, 18 Mar 2021 13:55:01 +0100 Subject: [PATCH 40/44] Add configure option for unsuffixed GRUB2 commands GRUB starting with version 2.02 permits the use of the linux and initrd commands for both EFI boot in *-efi installations, as well as 32-bit BIOS boot in i386-pc installations. This makes the use of the -16 and -efi suffixes for BIOS and EFI boot obsolete on systems with a modern GRUB installation. The --with-modern-grub configure flag makes ostree use the unsuffixed linux/initrd commands when generating a GRUB configuration, while defaulting to the previous behaviour for users not wanted this option. --- configure.ac | 8 ++++++++ src/libostree/ostree-bootloader-grub2.c | 27 +++++++++++++------------ 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/configure.ac b/configure.ac index 3a982f53..295cd157 100644 --- a/configure.ac +++ b/configure.ac @@ -548,6 +548,14 @@ AM_COND_IF(BUILDOPT_SYSTEMD_AND_LIBMOUNT, AC_DEFINE([BUILDOPT_LIBSYSTEMD_AND_LIBMOUNT], 1, [Define if systemd and libmount])) if test x$with_libsystemd = xyes; then OSTREE_FEATURES="$OSTREE_FEATURES systemd"; fi +AC_ARG_WITH(modern-grub, + AS_HELP_STRING([--with-modern-grub], + [Omit grub linux and initrd suffixes for EFI/BIOS booting on GRUB >2.02 (default: no)]),, + [with_modern_grub=no]) +AS_IF([ test x$with_modern_grub = xyes], [ + AC_DEFINE([WITH_MODERN_GRUB], 1, [Define if we have a GRUB version newer than 2.02]) +]) + AC_ARG_WITH(builtin-grub2-mkconfig, AS_HELP_STRING([--with-builtin-grub2-mkconfig], [Use a builtin minimal grub2-mkconfig to generate a GRUB2 configuration file (default: no)]),, diff --git a/src/libostree/ostree-bootloader-grub2.c b/src/libostree/ostree-bootloader-grub2.c index ceea682e..0ef751dc 100644 --- a/src/libostree/ostree-bootloader-grub2.c +++ b/src/libostree/ostree-bootloader-grub2.c @@ -28,25 +28,26 @@ #include -/* I only did some cursory research here, but it appears - * that we only want to use "linux16" for x86 platforms. - * At least, I got a report that "linux16" is definitely wrong - * for ppc64. See - * http://pkgs.fedoraproject.org/cgit/rpms/grub2.git/tree/0036-Use-linux16-when-appropriate-880840.patch?h=f25 - * https://bugzilla.redhat.com/show_bug.cgi?id=1108296 - * among others. +/* Maintain backwards compatibility with legacy GRUB + * installations that might rely on the -16 suffix + * for real-mode booting. + * Allow us to override this at build time if we're + * using a modern GRUB installation. */ -#if defined(__i386__) || defined(__x86_64__) +#if !defined(WITH_MODERN_GRUB) && ( defined(__i386__) || defined(__x86_64__) ) #define GRUB2_SUFFIX "16" #else #define GRUB2_SUFFIX "" #endif -/* https://github.com/projectatomic/rpm-ostree-toolbox/issues/102#issuecomment-316483554 - * https://github.com/rhboot/grubby/blob/34b1436ccbd56eab8024314cab48f2fc880eef08/grubby.c#L63 - * - * This is true at least on Fedora/Red Hat Enterprise Linux for aarch64. +/* Maintain backwards compatibility with legacy GRUB + * installations that might rely on the -efi suffix + * for RHEL/fedora-based distros. + * Allow us to override this at build time if we're + * using a modern GRUB installation that makes the + * suffix-less linux/initrd commands synonymous for + * both EFI and BIOS booting. */ -#if defined(__aarch64__) +#if defined(WITH_MODERN_GRUB) || defined(__aarch64__) #define GRUB2_EFI_SUFFIX "" #else #define GRUB2_EFI_SUFFIX "efi" From 61184163ea5cb5f1652dd4e9eb8de151a58d9c45 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 18 Mar 2021 21:54:52 +0000 Subject: [PATCH 41/44] Drop minimal rust/ library This was my first experiment with using Rust in this way; I gained a lot of knowledge from it. But, we don't really gain anything from the code as it is today - while it is "bit fiddling" code, the C code is well tested. We have a lot of compile-time options, and trimming them will be helpful. We've also gotten pushback on hard requiring Rust client side. Instead, what I'd like to do is hopefully soon create an `ostree-system` crate that uses the existing `ostree` library and can contain code drained from the rpm-ostree Rust and used by other projects perhaps. So the goal here is really more Rust, but we need to focus our efforts on where it's most valuable. --- .cci.jenkinsfile | 5 -- Makefile-libostree.am | 12 ---- Makefile.am | 24 ------- configure.ac | 34 ---------- rust/.gitignore | 2 - rust/Cargo.toml | 16 ----- rust/cargo-vendor-config | 8 --- rust/src/bupsplit.rs | 137 --------------------------------------- 8 files changed, 238 deletions(-) delete mode 100644 rust/.gitignore delete mode 100644 rust/Cargo.toml delete mode 100644 rust/cargo-vendor-config delete mode 100644 rust/src/bupsplit.rs diff --git a/.cci.jenkinsfile b/.cci.jenkinsfile index ac65b9c8..6a51594a 100644 --- a/.cci.jenkinsfile +++ b/.cci.jenkinsfile @@ -107,11 +107,6 @@ buildopts: { shwrap(""" git submodule update --init - git worktree add build-rust && cd build-rust - env MAKE_JOBS=${n} CONFIGOPTS="--enable-rust" SKIP_INSTALLDEPS=1 ./ci/build.sh - make check TESTS=tests/test-rollsum - cd .. && rm -rf build-rust - git worktree add build-libsoup && cd build-libsoup env MAKE_JOBS=${n} CONFIGOPTS="--without-curl --without-openssl --with-soup" SKIP_INSTALLDEPS=1 ./ci/build.sh make check diff --git a/Makefile-libostree.am b/Makefile-libostree.am index 98fbb289..6dbd00f5 100644 --- a/Makefile-libostree.am +++ b/Makefile-libostree.am @@ -21,18 +21,9 @@ include Makefile-libostree-defines.am - -if ENABLE_RUST -bupsplitpath = @abs_top_builddir@/target/@RUST_TARGET_SUBDIR@/libbupsplit_rs.a -BUPSPLIT_RUST_SRCS = rust/src/bupsplit.rs -EXTRA_DIST += $(BUPSPLIT_RUST_SRCS) -$(bupsplitpath): Makefile $(BUPSPLIT_RUST_SRCS) - cd $(top_srcdir)/rust && CARGO_TARGET_DIR=@abs_top_builddir@/target cargo build --verbose $(CARGO_RELEASE_ARGS) -else bupsplitpath = libbupsplit.la noinst_LTLIBRARIES += libbupsplit.la libbupsplit_la_SOURCES = src/libostree/bupsplit.h src/libostree/bupsplit.c -endif # ENABLE_RUST lib_LTLIBRARIES += libostree-1.la @@ -206,9 +197,6 @@ libostree_1_la_LDFLAGS = -version-number 1:0:0 -Bsymbolic-functions $(addprefix libostree_1_la_LIBADD = libotutil.la libglnx.la libbsdiff.la $(OT_INTERNAL_GIO_UNIX_LIBS) $(OT_INTERNAL_GPGME_LIBS) \ $(OT_DEP_LZMA_LIBS) $(OT_DEP_ZLIB_LIBS) $(OT_DEP_CRYPTO_LIBS) # Some change between rust-1.21.0-1.fc27 and rust-1.22.1-1.fc27.x86_64 -if ENABLE_RUST -libostree_1_la_LIBADD += -ldl -endif libostree_1_la_LIBADD += $(bupsplitpath) EXTRA_libostree_1_la_DEPENDENCIES = $(symbol_files) diff --git a/Makefile.am b/Makefile.am index 87a705cc..b2588ad7 100644 --- a/Makefile.am +++ b/Makefile.am @@ -71,30 +71,6 @@ GIRS = TYPELIBS = $(GIRS:.gir=.typelib) endif -# These bits based on gnome:librsvg/Makefile.am -if ENABLE_RUST -if RUST_DEBUG -CARGO_RELEASE_ARGS= -else -CARGO_RELEASE_ARGS=--release -endif - -check-local: - cd $(srcdir)/rust && CARGO_TARGET_DIR=$(abs_top_builddir)/target cargo test - -clean-local: - cd $(srcdir)/rust && CARGO_TARGET_DIR=$(abs_top_builddir)/target cargo clean - -dist-hook: - (cd $(distdir)/rust && \ - cp $(abs_top_srcdir)/rust/Cargo.lock . && \ - cargo vendor -q && \ - mkdir .cargo && \ - cp cargo-vendor-config .cargo/config) - -EXTRA_DIST += $(srcdir)/rust/Cargo.toml $(srcdir)/rust/cargo-vendor-config -endif # end ENABLE_RUST - libglnx_srcpath := $(srcdir)/libglnx libglnx_cflags := $(OT_DEP_GIO_UNIX_CFLAGS) "-I$(libglnx_srcpath)" libglnx_libs := $(OT_DEP_GIO_UNIX_LIBS) diff --git a/configure.ac b/configure.ac index 3a982f53..f7448a45 100644 --- a/configure.ac +++ b/configure.ac @@ -287,39 +287,6 @@ AS_IF([test "$enable_man" != no], [ ]) AM_CONDITIONAL(ENABLE_MAN, test "$enable_man" != no) -AC_ARG_ENABLE(rust, - [AS_HELP_STRING([--enable-rust], - [Compile Rust code instead of C [default=no]])],, - [enable_rust=no; rust_debug_release=no]) - -AS_IF([test "$enable_rust" = yes], [ - AC_PATH_PROG([cargo], [cargo]) - AS_IF([test -z "$cargo"], [AC_MSG_ERROR([cargo is required for --enable-rust])]) - AC_PATH_PROG([rustc], [rustc]) - AS_IF([test -z "$rustc"], [AC_MSG_ERROR([rustc is required for --enable-rust])]) - - dnl These bits based on gnome:librsvg/configure.ac - - dnl By default, we build in public release mode. - AC_ARG_ENABLE(rust-debug, - AC_HELP_STRING([--enable-rust-debug], - [Build Rust code with debugging information [default=no]]), - [rust_debug_release=$enableval], - [rust_debug_release=release]) - - AC_MSG_CHECKING(whether to build Rust code with debugging information) - if test "x$rust_debug_release" = "xyes" ; then - rust_debug_release=debug - AC_MSG_RESULT(yes) - else - AC_MSG_RESULT(no) - fi - RUST_TARGET_SUBDIR=${rust_debug_release} - AC_SUBST([RUST_TARGET_SUBDIR]) -]) -AM_CONDITIONAL(RUST_DEBUG, [test "x$rust_debug_release" = "xdebug"]) -AM_CONDITIONAL(ENABLE_RUST, [test "$enable_rust" != no]) - AC_ARG_WITH(libarchive, AS_HELP_STRING([--without-libarchive], [Do not use libarchive]), :, with_libarchive=maybe) @@ -628,7 +595,6 @@ echo " introspection: $found_introspection - Rust (internal oxidation): $rust_debug_release rofiles-fuse: $enable_rofiles_fuse HTTP backend: $fetcher_backend \"ostree trivial-httpd\": $enable_trivial_httpd_cmdline diff --git a/rust/.gitignore b/rust/.gitignore deleted file mode 100644 index 1e7caa9e..00000000 --- a/rust/.gitignore +++ /dev/null @@ -1,2 +0,0 @@ -Cargo.lock -target/ diff --git a/rust/Cargo.toml b/rust/Cargo.toml deleted file mode 100644 index 4da5ac32..00000000 --- a/rust/Cargo.toml +++ /dev/null @@ -1,16 +0,0 @@ -[package] -name = "bupsplit" -version = "0.0.1" -authors = ["Colin Walters "] - -[dependencies] -libc = "0.2" - -[lib] -name = "bupsplit_rs" -path = "src/bupsplit.rs" -crate-type = ["staticlib"] - -[profile.release] -panic = "abort" -lto = true diff --git a/rust/cargo-vendor-config b/rust/cargo-vendor-config deleted file mode 100644 index 5407266e..00000000 --- a/rust/cargo-vendor-config +++ /dev/null @@ -1,8 +0,0 @@ -# This is used after `cargo vendor` is run from `make dist` - -[source.crates-io] -registry = 'https://github.com/rust-lang/crates.io-index' -replace-with = 'vendored-sources' - -[source.vendored-sources] -directory = './vendor' diff --git a/rust/src/bupsplit.rs b/rust/src/bupsplit.rs deleted file mode 100644 index 73dc9ee1..00000000 --- a/rust/src/bupsplit.rs +++ /dev/null @@ -1,137 +0,0 @@ -/* - * Copyright 2017 Colin Walters - * Based on original bupsplit.c: - * Copyright 2011 Avery Pennarun. All rights reserved. - * - * (This license applies to bupsplit.c and bupsplit.h only.) - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are - * met: - * - * 1. Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * - * 2. Redistributions in binary form must reproduce the above copyright - * notice, this list of conditions and the following disclaimer in - * the documentation and/or other materials provided with the - * distribution. - * - * THIS SOFTWARE IS PROVIDED BY AVERY PENNARUN ``AS IS'' AND ANY - * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR - * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL OR - * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, - * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, - * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR - * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF - * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING - * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS - * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - */ - -extern crate libc; - -use std::slice; - -// According to librsync/rollsum.h: -// "We should make this something other than zero to improve the -// checksum algorithm: tridge suggests a prime number." -// apenwarr: I unscientifically tried 0 and 7919, and they both ended up -// slightly worse than the librsync value of 31 for my arbitrary test data. -const ROLLSUM_CHAR_OFFSET: u32 = 31; - -// Previously in the header file -const BUP_BLOBBITS: u32 = 13; -const BUP_BLOBSIZE: u32 = 1 << BUP_BLOBBITS; -const BUP_WINDOWBITS: u32 = 7; -const BUP_WINDOWSIZE: u32 = 1 << BUP_WINDOWBITS - 1; - -struct Rollsum { - s1: u32, - s2: u32, - window: [u8; BUP_WINDOWSIZE as usize], - wofs: i32, -} - -impl Rollsum { - pub fn new() -> Rollsum { - Rollsum { - s1: BUP_WINDOWSIZE * ROLLSUM_CHAR_OFFSET, - s2: BUP_WINDOWSIZE * (BUP_WINDOWSIZE - 1) * ROLLSUM_CHAR_OFFSET, - window: [0; 64], - wofs: 0, - } - } - - // These formulas are based on rollsum.h in the librsync project. - pub fn add(&mut self, drop: u8, add: u8) -> () { - let drop_expanded = u32::from(drop); - let add_expanded = u32::from(add); - self.s1 = self - .s1 - .wrapping_add(add_expanded.wrapping_sub(drop_expanded)); - self.s2 = self.s2.wrapping_add( - self.s1 - .wrapping_sub(BUP_WINDOWSIZE * (drop_expanded + ROLLSUM_CHAR_OFFSET)), - ); - } - - pub fn roll(&mut self, ch: u8) -> () { - let wofs = self.wofs as usize; - let dval = self.window[wofs]; - self.add(dval, ch); - self.window[wofs] = ch; - self.wofs = (self.wofs + 1) % (BUP_WINDOWSIZE as i32); - } - - pub fn digest(&self) -> u32 { - (self.s1 << 16) | (self.s2 & 0xFFFF) - } -} - -#[no_mangle] -pub extern "C" fn bupsplit_sum(buf: *const u8, ofs: libc::size_t, len: libc::size_t) -> u32 { - let sbuf = unsafe { - assert!(!buf.is_null()); - slice::from_raw_parts(buf.offset(ofs as isize), (len - ofs) as usize) - }; - - let mut r = Rollsum::new(); - for x in sbuf { - r.roll(*x); - } - r.digest() -} - -#[no_mangle] -pub extern "C" fn bupsplit_find_ofs( - buf: *const u8, - len: libc::size_t, - bits: *mut libc::c_int, -) -> libc::c_int { - if buf.is_null() { - return 0; - } - - let sbuf = unsafe { slice::from_raw_parts(buf, len as usize) }; - let mut r = Rollsum::new(); - for x in sbuf { - r.roll(*x); - if (r.s2 & (BUP_BLOBSIZE - 1)) == ((u32::max_value()) & (BUP_BLOBSIZE - 1)) { - if !bits.is_null() { - let mut sum = r.digest() >> BUP_BLOBBITS; - let mut rbits: libc::c_int = BUP_BLOBBITS as i32; - while sum & 1 != 0 { - sum >>= 1; - rbits += 1; - } - unsafe { - *bits = rbits; - } - } - return len as i32; - } - } - 0 -} From b69a4180b838a5646ef93cb7ef19398f50b1a0cf Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 19 Mar 2021 21:45:54 +0000 Subject: [PATCH 42/44] tests/inst: Patch to use my PR for openat Fixes the build. --- tests/inst/Cargo.toml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/inst/Cargo.toml b/tests/inst/Cargo.toml index 0986c0a7..3301616d 100644 --- a/tests/inst/Cargo.toml +++ b/tests/inst/Cargo.toml @@ -46,3 +46,7 @@ with-procspawn-tempdir = { git = "https://github.com/cgwalters/with-procspawn-te # Internal crate for the test macro itest-macro = { path = "itest-macro" } + +[patch.crates-io] +# PR openat (pun intended) https://github.com/tailhook/openat/pull/36 +openat = { git = 'https://github.com/cgwalters/openat', branch = 'libc-rename-signed' } From 36a543ceb1b2f87eac71f7434400690c9678b011 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Mon, 22 Mar 2021 16:31:06 -0400 Subject: [PATCH 43/44] lib/pull: Add some error-prefixing in dirtree scanning I think these are the paths involved in the error message at https://github.com/coreos/rpm-ostree/issues/2250. --- src/libostree/ostree-repo-pull.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index c63bdc4a..ab47a2a4 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -706,7 +706,7 @@ scan_dirtree_object (OtPullData *pull_data, * before libostree's validation was strengthened. */ if (!ot_util_filename_validate (filename, error)) - return FALSE; + return glnx_prefix_error (error, "File %u in dirtree", i); /* Skip files if we're traversing a request only directory, unless it exactly * matches the path */ @@ -781,7 +781,7 @@ scan_dirtree_object (OtPullData *pull_data, /* See comment above for files */ if (!ot_util_filename_validate (dirname, error)) - return FALSE; + return glnx_prefix_error (error, "Dir %u in dirtree", i); if (!pull_matches_subdir (pull_data, path, dirname, TRUE)) continue; @@ -1934,7 +1934,7 @@ scan_one_metadata_object (OtPullData *pull_data, { if (!scan_dirtree_object (pull_data, checksum, path, recursion_depth, pull_data->cancellable, error)) - return FALSE; + return glnx_prefix_error (error, "Validating dirtree %s (%s)", checksum, path); g_hash_table_add (pull_data->scanned_metadata, g_variant_ref (object)); pull_data->n_scanned_metadata++; From e9e4b9112083228b8c385ad26924b6c4623f4179 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 19 Mar 2021 21:53:37 +0000 Subject: [PATCH 44/44] Release 2021.1 --- Makefile-libostree.am | 6 +++--- configure.ac | 6 +++--- src/libostree/libostree-devel.sym | 6 ------ src/libostree/libostree-released.sym | 6 ++++++ tests/test-symbols.sh | 2 +- 5 files changed, 13 insertions(+), 13 deletions(-) diff --git a/Makefile-libostree.am b/Makefile-libostree.am index 6dbd00f5..ce784aff 100644 --- a/Makefile-libostree.am +++ b/Makefile-libostree.am @@ -179,9 +179,9 @@ endif # USE_GPGME symbol_files = $(top_srcdir)/src/libostree/libostree-released.sym # Uncomment this include when adding new development symbols. -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 27442d2b..26713077 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], [2020]) -m4_define([release_version], [9]) +m4_define([year_version], [2021]) +m4_define([release_version], [1]) 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 02159cd2..e2d6efc4 100644 --- a/src/libostree/libostree-devel.sym +++ b/src/libostree/libostree-devel.sym @@ -22,12 +22,6 @@ - uncomment the include in Makefile-libostree.am */ -LIBOSTREE_2021.1 { -global: - ostree_commit_metadata_for_bootable; - ostree_sysroot_require_booted_deployment; -} LIBOSTREE_2020.8; - /* Stub section for the stable release *after* this development one; don't * edit this other than to update the year. This is just a copy/paste * source. Replace $LASTSTABLE with the last stable version, and $NEWVERSION diff --git a/src/libostree/libostree-released.sym b/src/libostree/libostree-released.sym index 4f80d7fc..f1c773b3 100644 --- a/src/libostree/libostree-released.sym +++ b/src/libostree/libostree-released.sym @@ -638,6 +638,12 @@ global: ostree_repo_gpg_sign_data; } LIBOSTREE_2020.7; +LIBOSTREE_2021.1 { +global: + ostree_commit_metadata_for_bootable; + ostree_sysroot_require_booted_deployment; +} LIBOSTREE_2020.8; + /* 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 3072c212..77f07c4d 100755 --- a/tests/test-symbols.sh +++ b/tests/test-symbols.sh @@ -66,7 +66,7 @@ echo 'ok documented symbols' # ONLY update this checksum in release commits! cat > released-sha256.txt <