From c6c640f3aefb4195432e77478f24bd4d842495e3 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Wed, 25 Sep 2019 09:39:19 -0400 Subject: [PATCH 01/39] Post-release version bump Closes: #1927 Approved by: cgwalters --- configure.ac | 4 ++-- src/libostree/libostree-devel.sym | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index cd3cfd10..807af38e 100644 --- a/configure.ac +++ b/configure.ac @@ -4,10 +4,10 @@ dnl update libostree-released.sym from libostree-devel.sym, and update the check dnl in test-symbols.sh, and also set is_release_build=yes below. Then make dnl another post-release commit to bump the version, and set is_release_build=no. m4_define([year_version], [2019]) -m4_define([release_version], [4]) +m4_define([release_version], [5]) m4_define([package_version], [year_version.release_version]) AC_INIT([libostree], [package_version], [walters@verbum.org]) -is_release_build=yes +is_release_build=no AC_CONFIG_HEADER([config.h]) AC_CONFIG_MACRO_DIR([buildutil]) AC_CONFIG_AUX_DIR([build-aux]) diff --git a/src/libostree/libostree-devel.sym b/src/libostree/libostree-devel.sym index 9339c0fd..0b876f3b 100644 --- a/src/libostree/libostree-devel.sym +++ b/src/libostree/libostree-devel.sym @@ -18,6 +18,8 @@ ***/ /* Add new symbols here. Release commits should copy this section into -released.sym. */ +LIBOSTREE_2019.5 { +} LIBOSTREE_2019.4; /* Stub section for the stable release *after* this development one; don't * edit this other than to update the year. This is just a copy/paste From 5ea85ba5ac9a33d014264e48784d59d81ea61b91 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Wed, 25 Sep 2019 11:27:38 -0400 Subject: [PATCH 02/39] configure.ac: Add more details on how to do a release Closes: #1928 Approved by: cgwalters --- configure.ac | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/configure.ac b/configure.ac index 807af38e..6861afb8 100644 --- a/configure.ac +++ b/configure.ac @@ -1,8 +1,11 @@ AC_PREREQ([2.63]) -dnl If doing a final release, remember to follow the instructions to -dnl update libostree-released.sym from libostree-devel.sym, and update the checksum -dnl in test-symbols.sh, and also set is_release_build=yes below. Then make -dnl another post-release commit to bump the version, and set is_release_build=no. +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`. m4_define([year_version], [2019]) m4_define([release_version], [5]) m4_define([package_version], [year_version.release_version]) From 985a14100295c99d0c6d712bfbee0ec52a3a1601 Mon Sep 17 00:00:00 2001 From: Javier Martinez Canillas Date: Thu, 26 Sep 2019 17:33:57 +0200 Subject: [PATCH 03/39] grub2: Exit gracefully if the configuration has BLS enabled Since Fedora 30 grub2 has support to populate its menu entries from the BootLoaderSpec fragments in /boot/loader/entries, so there's no need to generate menu entries anymore using the /etc/grub.d/15_ostree script. But since ostree doesn't update the bootloader, it may be that the grub2 installed is an old one that doesn't have BLS support. For new installs, GRUB_ENABLE_BLSCFG=true is set in /etc/default/grub to tell the /etc/grub.d/10_linux script if a blscfg command has to be added to the generated grub2 config file. So check if BLS is enabled in /etc/default/grub and only add the entries if that's not the case. Otherwise the menu entries will be duplicated. The approach has the drawback that if a user sets GRUB_ENABLE_BLSCFG=true in /etc/default/grub without updating grub2, they will get an empty menu. Since there won't be any entries created by the 30_ostree script and the blscfg command won't work on the older grub2. Unfortunately there is no way to know if the installed grub2 already has BLS support or not. Related: https://bugzilla.redhat.com/show_bug.cgi?id=1751272#c27 Closes: #1929 Approved by: jlebon --- src/boot/grub2/grub2-15_ostree | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/boot/grub2/grub2-15_ostree b/src/boot/grub2/grub2-15_ostree index 0b9bf930..160ac2ca 100644 --- a/src/boot/grub2/grub2-15_ostree +++ b/src/boot/grub2/grub2-15_ostree @@ -26,6 +26,13 @@ if ! test -d /ostree/repo; then exit 0 fi +# Gracefully exit if the grub2 configuration has BLS enabled, +# since there is no need to create menu entries in that case. +. /etc/default/grub +if test ${GRUB_ENABLE_BLSCFG} = "true"; then + exit 0 +fi + # Make sure we're in the right environment if ! test -n "${GRUB_DEVICE}"; then echo "This script must be run as a child of grub2-mkconfig" 1>&2 From 82699a67dbb6bfcc9452bb969e7872809232a84f Mon Sep 17 00:00:00 2001 From: Alex Kiernan Date: Wed, 4 Sep 2019 17:29:15 +0100 Subject: [PATCH 04/39] Always enable trivial-httpd for tests When running tests we always need ostree-trivial-httpd, so enable it unconditionally Signed-off-by: Alex Kiernan --- Makefile.am | 1 + ci/build.sh | 5 +++++ ci/travis-build.sh | 1 + 3 files changed, 7 insertions(+) diff --git a/Makefile.am b/Makefile.am index cd04a055..673dbf88 100644 --- a/Makefile.am +++ b/Makefile.am @@ -39,6 +39,7 @@ AM_DISTCHECK_CONFIGURE_FLAGS += \ --enable-gtk-doc \ --enable-man \ --disable-maintainer-mode \ + --enable-trivial-httpd-cmdline \ $(NULL) GITIGNOREFILES = aclocal.m4 build-aux/ buildutil/*.m4 config.h.in gtk-doc.make diff --git a/ci/build.sh b/ci/build.sh index 09015074..806af050 100755 --- a/ci/build.sh +++ b/ci/build.sh @@ -22,6 +22,11 @@ case "${CONFIGOPTS:-}" in fi ;; esac +# unless libsoup is disabled, enable trivial-httpd for the tests +case "${CONFIGOPTS:-}" in + *--without-soup*) ;; + *) CONFIGOPTS="${CONFIGOPTS:-} --enable-trivial-httpd-cmdline" ;; +esac # always fail on warnings; https://github.com/ostreedev/ostree/pull/971 # NB: this disables the default set of flags from configure.ac diff --git a/ci/travis-build.sh b/ci/travis-build.sh index 3fd969bd..7c85313a 100755 --- a/ci/travis-build.sh +++ b/ci/travis-build.sh @@ -85,6 +85,7 @@ make="make -j${ci_parallel} V=1 VERBOSE=1" ../configure \ --enable-always-build-tests \ + --enable-trivial-httpd-cmdline \ ${ci_configopts} "$@" From 83d44ac20ae80d74e05d89744fd1fbd4f45b7fba Mon Sep 17 00:00:00 2001 From: Alex Kiernan Date: Thu, 5 Sep 2019 13:22:15 +0100 Subject: [PATCH 05/39] Gate ostree-trivial-httpd on BUILDOPT_TRIVIAL_HTTPD When building without --enable-trivial-httpd-cmdline, don't build or install the ostree-trivial-httpd binary. Signed-off-by: Alex Kiernan --- Makefile-ostree.am | 3 ++- configure.ac | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/Makefile-ostree.am b/Makefile-ostree.am index 76f39cad..7b53cb14 100644 --- a/Makefile-ostree.am +++ b/Makefile-ostree.am @@ -143,12 +143,13 @@ ostree_SOURCES += src/ostree/ot-builtin-pull.c endif if USE_LIBSOUP -# Eventually once we stop things from using this, we should support disabling this +if BUILDOPT_TRIVIAL_HTTPD ostree_SOURCES += src/ostree/ot-builtin-trivial-httpd.c pkglibexec_PROGRAMS += ostree-trivial-httpd ostree_trivial_httpd_SOURCES = src/ostree/ostree-trivial-httpd.c ostree_trivial_httpd_CFLAGS = $(ostree_bin_shared_cflags) $(OT_INTERNAL_SOUP_CFLAGS) ostree_trivial_httpd_LDADD = $(ostree_bin_shared_ldadd) $(OT_INTERNAL_SOUP_LIBS) +endif if !USE_CURL # This is necessary for the cookie jar bits diff --git a/configure.ac b/configure.ac index 6861afb8..e009d6b2 100644 --- a/configure.ac +++ b/configure.ac @@ -195,6 +195,9 @@ AC_ARG_ENABLE(trivial-httpd-cmdline, [Continue to support "ostree trivial-httpd" [default=no]])],, enable_trivial_httpd_cmdline=no) AM_CONDITIONAL(BUILDOPT_TRIVIAL_HTTPD, test x$enable_trivial_httpd_cmdline = xyes) +AS_IF([test x$with_soup = xno && test x$enable_trivial_httpd_cmdline = xyes], [ + AC_MSG_ERROR([trivial-httpd enabled, but libsoup is not; libsoup is needed for trivial-httpd]) +]) AM_COND_IF(BUILDOPT_TRIVIAL_HTTPD, [AC_DEFINE([BUILDOPT_ENABLE_TRIVIAL_HTTPD_CMDLINE], 1, [Define if we are enabling ostree trivial-httpd entrypoint])] ) From 569e09f509a15ac26c93fefdc5acd5620cdd826a Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 11 Oct 2019 20:25:48 +0000 Subject: [PATCH 06/39] ci: Honor ARTIFACTS environment variable This is set by the OpenShift Prow pod-utils: https://github.com/openshift/test-infra/blob/master/prow/pod-utilities.md Prep for having OSTree use that. Closes: #1930 Approved by: jlebon --- ci/build-check.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ci/build-check.sh b/ci/build-check.sh index bd0686bd..2b11b4d3 100755 --- a/ci/build-check.sh +++ b/ci/build-check.sh @@ -35,9 +35,11 @@ fi copy_out_gdtr_artifacts() { # Keep this in sync with papr.yml # TODO; Split the main/clang builds into separate build dirs + local artifactdir + artifactdir=${ARTIFACTS:-${topdir}} for x in test-suite.log config.log gdtr-results; do if test -e ${resultsdir}/${x}; then - mv ${resultsdir}/${x} ${topdir} + mv ${resultsdir}/${x} ${artifactdir} fi done } From 1a134bf7eef43299cfc6a689c2fca57b0be413fc Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 14 Oct 2019 14:29:31 +0000 Subject: [PATCH 07/39] ci: Make ${ARTIFACTS} directory It may not exist in OpenShift Prow by default. --- ci/build-check.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/ci/build-check.sh b/ci/build-check.sh index 2b11b4d3..af276ef0 100755 --- a/ci/build-check.sh +++ b/ci/build-check.sh @@ -37,6 +37,7 @@ copy_out_gdtr_artifacts() { # TODO; Split the main/clang builds into separate build dirs local artifactdir artifactdir=${ARTIFACTS:-${topdir}} + mkdir -p "${artifactdir}" for x in test-suite.log config.log gdtr-results; do if test -e ${resultsdir}/${x}; then mv ${resultsdir}/${x} ${artifactdir} From 58f3753ed22ed7eb984efb26655d3060d1284b40 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 14 Oct 2019 19:47:23 +0000 Subject: [PATCH 08/39] OWNERS: New file I tried to balance reflecting the reality of who works on libostree today with keeping some of the existing committers - particularly committers from multiple organizations. Part of switching libostree over to OpenShift Prow. --- OWNERS | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 OWNERS diff --git a/OWNERS b/OWNERS new file mode 100644 index 00000000..08da5513 --- /dev/null +++ b/OWNERS @@ -0,0 +1,18 @@ +# See the OWNERS docs: https://git.k8s.io/community/contributors/guide/owners.md + +# Keep these lists in alphabetical order (just to avoid ordering implying anything) +# Changes to this list can be done via regular pull requests. A generally good +# approach can be to just start reviewing pull requests - if you do that enough +# and understand the libostree code base, feel free to submit a PR to add +# yourself to one of these lists. +# Use the responsibility to merge carefully. +approvers: + - cgwalters + - jlebon + - pwithnall + # Does not currently have affiliation public + # - wmanley +reviewers: + - lucab + - mwleeds + - rfairley From aa7795d08d0d351a9117f2dc1c44f9f0ad568d05 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 14 Oct 2019 13:25:46 +0000 Subject: [PATCH 09/39] libostree: Add an assert to pacify clang-analyzer Got this error when trying to rebase libostree in RHEL: ``` Error: CLANG_WARNING: [#def1] libostree-2019.2/src/libostree/ostree-repo-checkout.c:375:21: warning: Access to field 'disable_xattrs' results in a dereference of a null pointer (loaded from variable 'repo') ``` I think what's happening is it sees us effectively testing `if (repo == NULL)` via the `while (current_repo)`. Let's tell it we're sure it's non-null right after the loop. --- src/libostree/ostree-repo-checkout.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libostree/ostree-repo-checkout.c b/src/libostree/ostree-repo-checkout.c index 49ec995f..8dd14640 100644 --- a/src/libostree/ostree-repo-checkout.c +++ b/src/libostree/ostree-repo-checkout.c @@ -717,6 +717,8 @@ checkout_one_file_at (OstreeRepo *repo, } current_repo = current_repo->parent_repo; } + /* Pacify clang-analyzer which sees us testing effectively if (repo == NULL) */ + g_assert (repo); need_copy = (hardlink_res == HARDLINK_RESULT_NOT_SUPPORTED); } From 9032182e3cb93695eb4cd63b86bf2b27c17b906e Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 14 Oct 2019 14:17:09 +0000 Subject: [PATCH 10/39] repo: [scan-build] Initialize a variable Another GLib error convention issue; but eh, we might as well be conservative and always initialize variables. --- 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 b654aff2..e2998f73 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -4197,7 +4197,7 @@ ostree_repo_has_object (OstreeRepo *self, GCancellable *cancellable, GError **error) { - gboolean ret_have_object; + gboolean ret_have_object = FALSE; if (!_ostree_repo_has_loose_object (self, checksum, objtype, &ret_have_object, cancellable, error)) From f1fdd885aba2aec4155124c8d294096e230ead13 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 14 Oct 2019 14:20:44 +0000 Subject: [PATCH 11/39] sysroot: [scan-build]: Remove a dead assignment Clarify the conditionals here and remove a dead assignment. --- src/libostree/ostree-sysroot-deploy.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index c342d7e0..6507b2c5 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -753,15 +753,14 @@ prepare_deployment_etc (OstreeSysroot *sysroot, return FALSE; gboolean usretc_exists = (errno == 0); - if (etc_exists && usretc_exists) - return glnx_throw (error, "Tree contains both /etc and /usr/etc"); - else if (etc_exists) + if (etc_exists) { + if (usretc_exists) + return glnx_throw (error, "Tree contains both /etc and /usr/etc"); /* Compatibility hack */ if (!glnx_renameat (deployment_dfd, "etc", deployment_dfd, "usr/etc", error)) return FALSE; usretc_exists = TRUE; - etc_exists = FALSE; } if (usretc_exists) From 51d9aa35a936c447c21bd930d3c1d999e20ec1a3 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 14 Oct 2019 14:22:12 +0000 Subject: [PATCH 12/39] sysroot: [scan-build] Remove a dead assignment Just quieting the scan. --- src/libostree/ostree-sysroot.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/libostree/ostree-sysroot.c b/src/libostree/ostree-sysroot.c index 2c0c0546..e17cc233 100644 --- a/src/libostree/ostree-sysroot.c +++ b/src/libostree/ostree-sysroot.c @@ -1671,10 +1671,7 @@ ostree_sysroot_simple_write_deployment (OstreeSysroot *sysroot, /* add it last if no crossover defined (or it's the first deployment in the sysroot) */ if (!added_new) - { - g_ptr_array_add (new_deployments, g_object_ref (new_deployment)); - added_new = TRUE; - } + g_ptr_array_add (new_deployments, g_object_ref (new_deployment)); OstreeSysrootWriteDeploymentsOpts write_opts = { .do_postclean = postclean }; if (!ostree_sysroot_write_deployments_with_options (sysroot, new_deployments, &write_opts, From 806206fac253a7bcb3e1bd7048d91da6c5dcb70c Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 14 Oct 2019 14:24:18 +0000 Subject: [PATCH 13/39] repo: [scan-build]: Mark a variable used We're just using this to auto-free, quiet the static analysis. --- src/libostree/ostree-repo-commit.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index 0aaebbdb..d057ea34 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -871,7 +871,10 @@ write_content_object (OstreeRepo *self, /* Give a null input if there's no content */ g_autoptr(GInputStream) null_input = NULL; if (!input) - null_input = input = g_memory_input_stream_new_from_data ("", 0, NULL); + { + null_input = input = g_memory_input_stream_new_from_data ("", 0, NULL); + (void) null_input; /* quiet static analysis */ + } checksum_input = ot_checksum_instream_new_with_start (input, G_CHECKSUM_SHA256, buf, len); From 9344de1ce1e8c185e01988277606ba1ed7f9d16b Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Tue, 15 Oct 2019 11:56:34 -0400 Subject: [PATCH 14/39] src/libotutil: Fix strv memory leak We were only freeing the array and not the members. Caught by `clang-analyzer` in: https://github.com/ostreedev/ostree/pull/1931 --- src/libotutil/ot-keyfile-utils.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libotutil/ot-keyfile-utils.c b/src/libotutil/ot-keyfile-utils.c index 9d5903ce..e24f0d29 100644 --- a/src/libotutil/ot-keyfile-utils.c +++ b/src/libotutil/ot-keyfile-utils.c @@ -154,7 +154,7 @@ ot_keyfile_get_string_list_with_separator_choice (GKeyFile *keyfile, guint sep_count = 0; gchar sep = '\0'; g_autofree char *value_str = NULL; - g_autofree char **value_list = NULL; + g_auto(GStrv) value_list = NULL; g_return_val_if_fail (keyfile != NULL, FALSE); g_return_val_if_fail (section != NULL, FALSE); @@ -215,8 +215,8 @@ ot_keyfile_get_string_list_with_default (GKeyFile *keyfile, g_key_file_set_list_separator (keyfile, separator); - g_autofree char **ret_value = g_key_file_get_string_list (keyfile, section, - key, NULL, &temp_error); + g_auto(GStrv) ret_value = g_key_file_get_string_list (keyfile, section, + key, NULL, &temp_error); if (temp_error) { @@ -224,7 +224,7 @@ ot_keyfile_get_string_list_with_default (GKeyFile *keyfile, G_KEY_FILE_ERROR_KEY_NOT_FOUND)) { g_clear_error (&temp_error); - ret_value = default_value; + ret_value = g_strdupv (default_value); } else { From 810f24d8977aff903151379ef344146904967a85 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 14 Oct 2019 13:19:28 +0000 Subject: [PATCH 15/39] libotutil: Port keyfile-utils.c to new style I was trying to fix a clang `scan-build` error that jlebon ended up tracking down in https://github.com/ostreedev/ostree/pull/1939/commits/9344de1ce1e8c185e01988277606ba1ed7f9d16b But in the process of tracing through this I found it way easier to read as "new style" code, so this also ports the code. I added a `g_assert()` in there too to help assert that `g_key_file_get_value` won't leak in the error path. --- src/libotutil/ot-keyfile-utils.c | 64 +++++++++++++------------------- 1 file changed, 25 insertions(+), 39 deletions(-) diff --git a/src/libotutil/ot-keyfile-utils.c b/src/libotutil/ot-keyfile-utils.c index e24f0d29..2050e969 100644 --- a/src/libotutil/ot-keyfile-utils.c +++ b/src/libotutil/ot-keyfile-utils.c @@ -35,15 +35,12 @@ ot_keyfile_get_boolean_with_default (GKeyFile *keyfile, gboolean *out_bool, GError **error) { - gboolean ret = FALSE; + g_return_val_if_fail (keyfile != NULL, FALSE); + g_return_val_if_fail (section != NULL, FALSE); + g_return_val_if_fail (value != NULL, FALSE); + GError *temp_error = NULL; - gboolean ret_bool; - - g_return_val_if_fail (keyfile != NULL, ret); - g_return_val_if_fail (section != NULL, ret); - g_return_val_if_fail (value != NULL, ret); - - ret_bool = g_key_file_get_boolean (keyfile, section, value, &temp_error); + gboolean ret_bool = g_key_file_get_boolean (keyfile, section, value, &temp_error); if (temp_error) { if (g_error_matches (temp_error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_KEY_NOT_FOUND)) @@ -54,14 +51,12 @@ ot_keyfile_get_boolean_with_default (GKeyFile *keyfile, else { g_propagate_error (error, temp_error); - goto out; + return FALSE; } } - ret = TRUE; *out_bool = ret_bool; - out: - return ret; + return TRUE; } gboolean @@ -72,33 +67,29 @@ ot_keyfile_get_value_with_default (GKeyFile *keyfile, char **out_value, GError **error) { - gboolean ret = FALSE; + g_return_val_if_fail (keyfile != NULL, FALSE); + g_return_val_if_fail (section != NULL, FALSE); + g_return_val_if_fail (value != NULL, FALSE); + GError *temp_error = NULL; - g_autofree char *ret_value = NULL; - - g_return_val_if_fail (keyfile != NULL, ret); - g_return_val_if_fail (section != NULL, ret); - g_return_val_if_fail (value != NULL, ret); - - ret_value = g_key_file_get_value (keyfile, section, value, &temp_error); + g_autofree char *ret_value = g_key_file_get_value (keyfile, section, value, &temp_error); if (temp_error) { if (g_error_matches (temp_error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_KEY_NOT_FOUND)) { g_clear_error (&temp_error); + g_assert (ret_value == NULL); ret_value = g_strdup (default_value); } else { g_propagate_error (error, temp_error); - goto out; + return FALSE; } } - ret = TRUE; ot_transfer_out_value(out_value, &ret_value); - out: - return ret; + return TRUE; } gboolean @@ -109,14 +100,12 @@ ot_keyfile_get_value_with_default_group_optional (GKeyFile *keyfile, char **out_value, GError **error) { - gboolean ret = FALSE; + g_return_val_if_fail (keyfile != NULL, FALSE); + g_return_val_if_fail (section != NULL, FALSE); + g_return_val_if_fail (value != NULL, FALSE); + GError *local_error = NULL; g_autofree char *ret_value = NULL; - - g_return_val_if_fail (keyfile != NULL, ret); - g_return_val_if_fail (section != NULL, ret); - g_return_val_if_fail (value != NULL, ret); - if (!ot_keyfile_get_value_with_default (keyfile, section, value, default_value, &ret_value, &local_error)) { if (g_error_matches (local_error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_GROUP_NOT_FOUND)) @@ -127,14 +116,12 @@ ot_keyfile_get_value_with_default_group_optional (GKeyFile *keyfile, else { g_propagate_error (error, local_error); - goto out; + return FALSE; } } - ret = TRUE; ot_transfer_out_value(out_value, &ret_value); - out: - return ret; + return TRUE; } /* Read the value of key as a string. If the value string contains @@ -151,22 +138,21 @@ ot_keyfile_get_string_list_with_separator_choice (GKeyFile *keyfile, char ***out_value, GError **error) { - guint sep_count = 0; - gchar sep = '\0'; - g_autofree char *value_str = NULL; - g_auto(GStrv) value_list = NULL; - g_return_val_if_fail (keyfile != NULL, FALSE); g_return_val_if_fail (section != NULL, FALSE); g_return_val_if_fail (key != NULL, FALSE); g_return_val_if_fail (separators != NULL, FALSE); + g_autofree char *value_str = NULL; if (!ot_keyfile_get_value_with_default (keyfile, section, key, NULL, &value_str, error)) return FALSE; + g_auto(GStrv) value_list = NULL; if (value_str) { + gchar sep = '\0'; + guint sep_count = 0; for (size_t i = 0; i < strlen (separators) && sep_count <= 1; i++) { if (strchr (value_str, separators[i])) From 9defac5b8cd12f9af6f1a7abe4d912bc46995e83 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 16 Oct 2019 13:32:36 +0000 Subject: [PATCH 16/39] ci: Skip all yum operations if SKIP_INSTALLDEPS is set This is used by our OpenShift Prow job; we use the cosa buildroot container: https://github.com/coreos/coreos-assembler/pull/730 And using `yum` at all means we can flake on fetching rpm metadata. --- ci/installdeps.sh | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/ci/installdeps.sh b/ci/installdeps.sh index 29bd4e41..4fc3280f 100755 --- a/ci/installdeps.sh +++ b/ci/installdeps.sh @@ -3,6 +3,14 @@ set -xeuo pipefail +# This is used by our OpenShift Prow job; we use the +# 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 + exit 0 +fi + dn=$(dirname $0) . ${dn}/libpaprci/libbuild.sh From 4a38b1115973a8cd00608dc7bc4df96ae93bb79e Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 16 Oct 2019 13:38:29 +0000 Subject: [PATCH 17/39] commit: [scan-build] Remove a dead assignment The `write_commit()` API defaults to current time, and this assignment became dead in: https://github.com/ostreedev/ostree/commit/8ba90a33410c9707a30a77f808a7ec712d465165 --- src/ostree/ot-builtin-commit.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/ostree/ot-builtin-commit.c b/src/ostree/ot-builtin-commit.c index 43eb18b3..0eea09d7 100644 --- a/src/ostree/ot-builtin-commit.c +++ b/src/ostree/ot-builtin-commit.c @@ -772,8 +772,6 @@ ostree_builtin_commit (int argc, char **argv, OstreeCommandInvocation *invocatio if (!skip_commit) { - guint64 timestamp; - if (!opt_no_bindings) { g_autoptr(GVariant) old_metadata = g_steal_pointer (&metadata); @@ -782,10 +780,6 @@ ostree_builtin_commit (int argc, char **argv, OstreeCommandInvocation *invocatio if (!opt_timestamp) { - GDateTime *now = g_date_time_new_now_utc (); - timestamp = g_date_time_to_unix (now); - g_date_time_unref (now); - if (!ostree_repo_write_commit (repo, parent, opt_subject, commit_body, metadata, OSTREE_REPO_FILE (root), &commit_checksum, cancellable, error)) @@ -800,8 +794,8 @@ ostree_builtin_commit (int argc, char **argv, OstreeCommandInvocation *invocatio "Could not parse '%s'", opt_timestamp); goto out; } - timestamp = ts.tv_sec; + guint64 timestamp = ts.tv_sec; if (!ostree_repo_write_commit_with_time (repo, parent, opt_subject, commit_body, metadata, OSTREE_REPO_FILE (root), timestamp, From a8dc90b02fbbd7b951be59704d4c5969463ab8f2 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 16 Oct 2019 13:44:46 +0000 Subject: [PATCH 18/39] tree-wide: [scan-build]: Add some asserts that pointers are non-NULL More "scan-build doesn't understand GError and our out-param conventions" AKA "these errors would be impossible with Rust's sum type Result<> approach". --- src/libostree/ostree-sysroot-cleanup.c | 1 + src/libostree/ostree-sysroot.c | 1 + src/ostree/ot-builtin-fsck.c | 1 + src/ostree/ot-dump.c | 5 ++++- 4 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/libostree/ostree-sysroot-cleanup.c b/src/libostree/ostree-sysroot-cleanup.c index 7a352e6b..ef95d13c 100644 --- a/src/libostree/ostree-sysroot-cleanup.c +++ b/src/libostree/ostree-sysroot-cleanup.c @@ -313,6 +313,7 @@ cleanup_old_deployments (OstreeSysroot *self, if (!list_all_deployment_directories (self, &all_deployment_dirs, cancellable, error)) return FALSE; + g_assert (all_deployment_dirs); /* Pacify static analysis */ for (guint i = 0; i < all_deployment_dirs->len; i++) { OstreeDeployment *deployment = all_deployment_dirs->pdata[i]; diff --git a/src/libostree/ostree-sysroot.c b/src/libostree/ostree-sysroot.c index e17cc233..1c9dbf37 100644 --- a/src/libostree/ostree-sysroot.c +++ b/src/libostree/ostree-sysroot.c @@ -945,6 +945,7 @@ ostree_sysroot_load_if_changed (OstreeSysroot *self, g_autoptr(GPtrArray) deployments = g_ptr_array_new_with_free_func ((GDestroyNotify)g_object_unref); + g_assert (boot_loader_configs); /* Pacify static analysis */ for (guint i = 0; i < boot_loader_configs->len; i++) { OstreeBootconfigParser *config = boot_loader_configs->pdata[i]; diff --git a/src/ostree/ot-builtin-fsck.c b/src/ostree/ot-builtin-fsck.c index 5ad3bf38..dea03af4 100644 --- a/src/ostree/ot-builtin-fsck.c +++ b/src/ostree/ot-builtin-fsck.c @@ -434,6 +434,7 @@ ostree_builtin_fsck (int argc, char **argv, OstreeCommandInvocation *invocation, if (opt_add_tombstones) { guint i; + g_assert (tombstones); /* Pacify static analysis */ if (tombstones->len) { if (!ot_enable_tombstone_commits (repo, error)) diff --git a/src/ostree/ot-dump.c b/src/ostree/ot-dump.c index 1ef63740..38f3730b 100644 --- a/src/ostree/ot-dump.c +++ b/src/ostree/ot-dump.c @@ -125,7 +125,10 @@ dump_commit (GVariant *variant, timestamp = GUINT64_FROM_BE (timestamp); str = format_timestamp (timestamp, &local_error); if (!str) - errx (1, "Failed to read commit: %s", local_error->message); + { + g_assert (local_error); /* Pacify static analysis */ + errx (1, "Failed to read commit: %s", local_error->message); + } g_autofree char *contents = ostree_commit_get_content_checksum (variant) ?: ""; g_print ("ContentChecksum: %s\n", contents); g_print ("Date: %s\n", str); From 946659aacf08bc238e3ceda4475a855c165b06c5 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 16 Oct 2019 13:48:20 +0000 Subject: [PATCH 19/39] prune: [scan-build] Initialize a variable Another false positive because we only read this if `opt_keep_younger_than` is `TRUE`, but let's initialize variables on general principle. --- src/ostree/ot-builtin-prune.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ostree/ot-builtin-prune.c b/src/ostree/ot-builtin-prune.c index 2f560d14..fd2ab05f 100644 --- a/src/ostree/ot-builtin-prune.c +++ b/src/ostree/ot-builtin-prune.c @@ -205,7 +205,7 @@ ostree_builtin_prune (int argc, char **argv, OstreeCommandInvocation *invocation g_autoptr(GHashTable) all_refs = NULL; g_autoptr(GHashTable) reachable = ostree_repo_traverse_new_reachable (); g_autoptr(GHashTable) retain_branch_depth = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL); - struct timespec keep_younger_than_ts; + struct timespec keep_younger_than_ts = {0, }; GHashTableIter hash_iter; gpointer key, value; From c61234a428ee5daa75eea6524c50b7f11c7788ca Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 14 Oct 2019 19:22:19 +0000 Subject: [PATCH 20/39] bootloader: Add a zipl bootloader backend zipl is a bit special in that it parses the BLS config files directly *but* we need to run the command to update the "boot block". Hence, we're not generating a separate config file like the other backends. Instead, extend the bootloader interface with a `post_bls_sync` method that is run in the same place we swap the `boot/loader` symlink. We write a "stamp file" in `/boot` that says we need to run this command. The reason we use stamp file is to prevent the case where the system is interrupted after BLS file is updated, but before zipl is triggered, then zipl boot records are not updated. This opens the door to making things eventually-consistent/reconcilable by later adding a systemd unit to run `zipl` if we're interrupted via a systemd unit - I think we should eventually take this approach everywhere rather than requiring `/boot/loader` to be a symlink. Author: Colin Walters Tested-by: Tuan Hoang Co-Authored-By: Tuan Hoang --- Makefile-libostree.am | 2 + src/libostree/ostree-bootloader-zipl.c | 152 +++++++++++++++++++++++++ src/libostree/ostree-bootloader-zipl.h | 36 ++++++ src/libostree/ostree-bootloader.c | 13 +++ src/libostree/ostree-bootloader.h | 7 ++ src/libostree/ostree-repo.c | 4 +- src/libostree/ostree-sysroot-deploy.c | 21 +++- 7 files changed, 233 insertions(+), 2 deletions(-) create mode 100644 src/libostree/ostree-bootloader-zipl.c create mode 100644 src/libostree/ostree-bootloader-zipl.h diff --git a/Makefile-libostree.am b/Makefile-libostree.am index 46984a75..4b412fec 100644 --- a/Makefile-libostree.am +++ b/Makefile-libostree.am @@ -117,6 +117,8 @@ libostree_1_la_SOURCES = \ src/libostree/ostree-bootloader.c \ src/libostree/ostree-bootloader-grub2.h \ src/libostree/ostree-bootloader-grub2.c \ + src/libostree/ostree-bootloader-zipl.h \ + src/libostree/ostree-bootloader-zipl.c \ src/libostree/ostree-bootloader-syslinux.h \ src/libostree/ostree-bootloader-syslinux.c \ src/libostree/ostree-bootloader-uboot.h \ diff --git a/src/libostree/ostree-bootloader-zipl.c b/src/libostree/ostree-bootloader-zipl.c new file mode 100644 index 00000000..4ac785d9 --- /dev/null +++ b/src/libostree/ostree-bootloader-zipl.c @@ -0,0 +1,152 @@ +/* + * Copyright (C) 2019 Colin Walters + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published + * by the Free Software Foundation; either version 2 of the licence or (at + * your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General + * Public License along with this library; if not, write to the + * Free Software Foundation, Inc., 59 Temple Place, Suite 330, + * Boston, MA 02111-1307, USA. + */ + +#include "config.h" + +#include "ostree-sysroot-private.h" +#include "ostree-bootloader-zipl.h" +#include "otutil.h" + +#include + +/* This is specific to zipl today, but in the future we could also + * use it for the grub2-mkconfig case. + */ +static const char zipl_requires_execute_path[] = "boot/ostree-bootloader-update.stamp"; + +struct _OstreeBootloaderZipl +{ + GObject parent_instance; + + OstreeSysroot *sysroot; +}; + +typedef GObjectClass OstreeBootloaderZiplClass; + +static void _ostree_bootloader_zipl_bootloader_iface_init (OstreeBootloaderInterface *iface); +G_DEFINE_TYPE_WITH_CODE (OstreeBootloaderZipl, _ostree_bootloader_zipl, G_TYPE_OBJECT, + G_IMPLEMENT_INTERFACE (OSTREE_TYPE_BOOTLOADER, _ostree_bootloader_zipl_bootloader_iface_init)); + +static gboolean +_ostree_bootloader_zipl_query (OstreeBootloader *bootloader, + gboolean *out_is_active, + GCancellable *cancellable, + GError **error) +{ + /* We don't auto-detect this one; should be explicitly chosen right now. + * see also https://github.com/coreos/coreos-assembler/pull/849 + */ + *out_is_active = FALSE; + return TRUE; +} + +static const char * +_ostree_bootloader_zipl_get_name (OstreeBootloader *bootloader) +{ + return "zipl"; +} + +static gboolean +_ostree_bootloader_zipl_write_config (OstreeBootloader *bootloader, + int bootversion, + GPtrArray *new_deployments, + GCancellable *cancellable, + GError **error) +{ + OstreeBootloaderZipl *self = OSTREE_BOOTLOADER_ZIPL (bootloader); + + /* Write our stamp file */ + if (!glnx_file_replace_contents_at (self->sysroot->sysroot_fd, zipl_requires_execute_path, + (guint8*)"", 0, GLNX_FILE_REPLACE_NODATASYNC, + cancellable, error)) + return FALSE; + + return TRUE; +} + +static gboolean +_ostree_bootloader_zipl_post_bls_sync (OstreeBootloader *bootloader, + GCancellable *cancellable, + GError **error) +{ + OstreeBootloaderZipl *self = OSTREE_BOOTLOADER_ZIPL (bootloader); + + /* Note that unlike the grub2-mkconfig backend, we make no attempt to + * chroot(). + */ + g_assert (self->sysroot->booted_deployment); + + if (!glnx_fstatat_allow_noent (self->sysroot->sysroot_fd, zipl_requires_execute_path, NULL, 0, error)) + return FALSE; + + /* If there's no stamp file, nothing to do */ + if (errno == ENOENT) + return TRUE; + + const char *const zipl_argv[] = {"zipl", NULL}; + int estatus; + if (!g_spawn_sync (NULL, (char**)zipl_argv, NULL, G_SPAWN_SEARCH_PATH, + NULL, NULL, NULL, NULL, &estatus, error)) + return FALSE; + if (!g_spawn_check_exit_status (estatus, error)) + return FALSE; + if (!glnx_unlinkat (self->sysroot->sysroot_fd, zipl_requires_execute_path, 0, error)) + return FALSE; + return TRUE; +} + +static void +_ostree_bootloader_zipl_finalize (GObject *object) +{ + OstreeBootloaderZipl *self = OSTREE_BOOTLOADER_ZIPL (object); + + g_clear_object (&self->sysroot); + + G_OBJECT_CLASS (_ostree_bootloader_zipl_parent_class)->finalize (object); +} + +void +_ostree_bootloader_zipl_init (OstreeBootloaderZipl *self) +{ +} + +static void +_ostree_bootloader_zipl_bootloader_iface_init (OstreeBootloaderInterface *iface) +{ + iface->query = _ostree_bootloader_zipl_query; + iface->get_name = _ostree_bootloader_zipl_get_name; + iface->write_config = _ostree_bootloader_zipl_write_config; + iface->post_bls_sync = _ostree_bootloader_zipl_post_bls_sync; +} + +void +_ostree_bootloader_zipl_class_init (OstreeBootloaderZiplClass *class) +{ + GObjectClass *object_class = G_OBJECT_CLASS (class); + + object_class->finalize = _ostree_bootloader_zipl_finalize; +} + +OstreeBootloaderZipl * +_ostree_bootloader_zipl_new (OstreeSysroot *sysroot) +{ + OstreeBootloaderZipl *self = g_object_new (OSTREE_TYPE_BOOTLOADER_ZIPL, NULL); + self->sysroot = g_object_ref (sysroot); + return self; +} diff --git a/src/libostree/ostree-bootloader-zipl.h b/src/libostree/ostree-bootloader-zipl.h new file mode 100644 index 00000000..79e84912 --- /dev/null +++ b/src/libostree/ostree-bootloader-zipl.h @@ -0,0 +1,36 @@ +/* + * Copyright (C) 2019 Colin Walters + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published + * by the Free Software Foundation; either version 2 of the licence or (at + * your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General + * Public License along with this library; if not, write to the + * Free Software Foundation, Inc., 59 Temple Place, Suite 330, + * Boston, MA 02111-1307, USA. + */ + +#pragma once + +#include "ostree-bootloader.h" + +G_BEGIN_DECLS + +#define OSTREE_TYPE_BOOTLOADER_ZIPL (_ostree_bootloader_zipl_get_type ()) +#define OSTREE_BOOTLOADER_ZIPL(inst) (G_TYPE_CHECK_INSTANCE_CAST ((inst), OSTREE_TYPE_BOOTLOADER_ZIPL, OstreeBootloaderZipl)) +#define OSTREE_IS_BOOTLOADER_ZIPL(inst) (G_TYPE_CHECK_INSTANCE_TYPE ((inst), OSTREE_TYPE_BOOTLOADER_ZIPL)) + +typedef struct _OstreeBootloaderZipl OstreeBootloaderZipl; + +GType _ostree_bootloader_zipl_get_type (void) G_GNUC_CONST; + +OstreeBootloaderZipl * _ostree_bootloader_zipl_new (OstreeSysroot *sysroot); + +G_END_DECLS diff --git a/src/libostree/ostree-bootloader.c b/src/libostree/ostree-bootloader.c index 5b8125dc..76b7bb82 100644 --- a/src/libostree/ostree-bootloader.c +++ b/src/libostree/ostree-bootloader.c @@ -65,6 +65,19 @@ _ostree_bootloader_write_config (OstreeBootloader *self, cancellable, error); } +gboolean +_ostree_bootloader_post_bls_sync (OstreeBootloader *self, + GCancellable *cancellable, + GError **error) +{ + g_return_val_if_fail (OSTREE_IS_BOOTLOADER (self), FALSE); + + if (OSTREE_BOOTLOADER_GET_IFACE (self)->post_bls_sync) + return OSTREE_BOOTLOADER_GET_IFACE (self)->post_bls_sync (self, cancellable, error); + + return TRUE; +} + gboolean _ostree_bootloader_is_atomic (OstreeBootloader *self) { diff --git a/src/libostree/ostree-bootloader.h b/src/libostree/ostree-bootloader.h index 5af2dcc8..48a7a9cd 100644 --- a/src/libostree/ostree-bootloader.h +++ b/src/libostree/ostree-bootloader.h @@ -47,6 +47,9 @@ struct _OstreeBootloaderInterface GPtrArray *new_deployments, GCancellable *cancellable, GError **error); + gboolean (* post_bls_sync) (OstreeBootloader *self, + GCancellable *cancellable, + GError **error); gboolean (* is_atomic) (OstreeBootloader *self); }; G_DEFINE_AUTOPTR_CLEANUP_FUNC (OstreeBootloader, g_object_unref) @@ -66,6 +69,10 @@ gboolean _ostree_bootloader_write_config (OstreeBootloader *self, GCancellable *cancellable, GError **error); +gboolean _ostree_bootloader_post_bls_sync (OstreeBootloader *self, + GCancellable *cancellable, + GError **error); + gboolean _ostree_bootloader_is_atomic (OstreeBootloader *self); G_END_DECLS diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index b654aff2..e86489d3 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -3184,8 +3184,10 @@ reload_sysroot_config (OstreeRepo *self, * binary "x" in /usr/lib/ostree/bootloaders/x). See: * https://github.com/ostreedev/ostree/issues/1719 * https://github.com/ostreedev/ostree/issues/1801 + * Also, dedup these strings with the bootloader implementations */ - if (!(g_str_equal (bootloader, "auto") || g_str_equal (bootloader, "none"))) + if (!(g_str_equal (bootloader, "auto") || g_str_equal (bootloader, "none") + || g_str_equal (bootloader, "zipl"))) return glnx_throw (error, "Invalid bootloader configuration: '%s'", bootloader); g_free (self->bootloader); diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index c342d7e0..cc0b5c99 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -43,6 +43,7 @@ #include "ostree.h" #include "ostree-sysroot-private.h" #include "ostree-sepolicy-private.h" +#include "ostree-bootloader-zipl.h" #include "ostree-deployment-private.h" #include "ostree-core-private.h" #include "ostree-linuxfsutil.h" @@ -1830,6 +1831,7 @@ prepare_new_bootloader_link (OstreeSysroot *sysroot, /* Update the /boot/loader symlink to point to /boot/loader.$new_bootversion */ static gboolean swap_bootloader (OstreeSysroot *sysroot, + OstreeBootloader *bootloader, int current_bootversion, int new_bootversion, GCancellable *cancellable, @@ -1863,6 +1865,15 @@ swap_bootloader (OstreeSysroot *sysroot, if (fsync (boot_dfd) != 0) return glnx_throw_errno_prefix (error, "fsync(boot)"); + /* TODO: In the future also execute this automatically via a systemd unit + * if we detect it's necessary. + **/ + if (bootloader) + { + if (!_ostree_bootloader_post_bls_sync (bootloader, cancellable, error)) + return FALSE; + } + return TRUE; } @@ -2129,7 +2140,7 @@ write_deployments_bootswap (OstreeSysroot *self, if (!full_system_sync (self, out_syncstats, cancellable, error)) return FALSE; - if (!swap_bootloader (self, self->bootversion, new_bootversion, + if (!swap_bootloader (self, bootloader, self->bootversion, new_bootversion, cancellable, error)) return FALSE; @@ -2356,6 +2367,14 @@ ostree_sysroot_write_deployments_with_options (OstreeSysroot *self, { /* No bootloader specified; do not query bootloaders to run. */ } + else if (g_str_equal (bootloader_config, "zipl")) + { + /* Because we do not mark zipl as active by default, lets creating one here, + * which is basically the same what _ostree_sysroot_query_bootloader() does + * for other bootloaders if being activated. + * */ + bootloader = (OstreeBootloader*) _ostree_bootloader_zipl_new (self); + } bootloader_is_atomic = bootloader != NULL && _ostree_bootloader_is_atomic (bootloader); From deca9d4c7dec22b9828d3ebb39f805f025140002 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 16 Oct 2019 18:38:37 +0000 Subject: [PATCH 21/39] ci: Trim PAPR config to drop required flag Same as https://github.com/coreos/rpm-ostree/pull/1923 Quoting that rationale: > Since we're not using Homu anymore (and Tide instead looks at > all statuses by default), let's just drop it. This brings down the > number of statuses on PRs by one more (and so one less context to > override when needed). --- .papr.yml | 20 +------------------- 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/.papr.yml b/.papr.yml index 22d84da5..f74985e4 100644 --- a/.papr.yml +++ b/.papr.yml @@ -1,10 +1,6 @@ # This suite skips the RPMs and does the build+unit tests in a container inherit: false -branches: - - master - - auto - - try -required: true + container: image: registry.fedoraproject.org/fedora:29 context: f29-primary @@ -74,7 +70,6 @@ tests: --- inherit: true -required: true context: f29-libsoup @@ -87,7 +82,6 @@ tests: --- inherit: true -required: true context: f29-introspection-tests @@ -103,13 +97,8 @@ tests: # Reset inheritance for non-variant builds inherit: false -branches: - - master - - auto - - try context: f29-flatpak -required: true # This test case wants an "unprivileged container with bubblewrap", # which we don't have right now; so just provision a VM and do a @@ -131,14 +120,7 @@ artifacts: # we share more code. https://github.com/projectatomic/rpm-ostree/issues/662 inherit: false -branches: - - master - - auto - - try - context: f29-rpmostree -# XXX: some issues currently failing that need investigating. -required: false cluster: hosts: From a982dc97ea0b18d58be1db19219bf4d97a898525 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 16 Oct 2019 19:36:31 +0000 Subject: [PATCH 22/39] tree-wide: [scan-build] Fix some dead stores No real issues, just quieting the scanner. --- tests/test-keyfile-utils.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/test-keyfile-utils.c b/tests/test-keyfile-utils.c index 258e7dd8..3e4f8257 100644 --- a/tests/test-keyfile-utils.c +++ b/tests/test-keyfile-utils.c @@ -199,8 +199,6 @@ test_get_value_with_default_group_optional (void) static void test_copy_group (void) { - g_auto(GStrv) keys = NULL; - g_auto(GStrv) keys2 = NULL; gsize length, length2, ii; GKeyFile *tmp = g_key_file_new (); const char *section = "section"; @@ -218,8 +216,8 @@ test_copy_group (void) g_assert_true (ot_keyfile_copy_group (g_keyfile, tmp, section)); - keys = g_key_file_get_keys (g_keyfile, section, &length, NULL); - keys2 = g_key_file_get_keys (tmp, section, &length2, NULL); + g_auto(GStrv) keys = g_key_file_get_keys (g_keyfile, section, &length, NULL); + g_strfreev (g_key_file_get_keys (tmp, section, &length2, NULL)); g_assert_cmpint(length, ==, length2); for (ii = 0; ii < length; ii++) From bc1980ca38b2ac2a8bc1b41dc6bb98755fa724ea Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 18 Oct 2019 14:45:33 +0000 Subject: [PATCH 23/39] lib/repo: [scan-build] Quiet a dead store warning False positive, just add a pacifier. --- src/libostree/ostree-repo-commit.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index d057ea34..8c5d9411 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -1588,7 +1588,6 @@ ostree_repo_prepare_transaction (OstreeRepo *self, GCancellable *cancellable, GError **error) { - g_autoptr(_OstreeRepoAutoTransaction) txn = NULL; guint64 reserved_bytes = 0; g_return_val_if_fail (self->in_transaction == FALSE, FALSE); @@ -1596,7 +1595,8 @@ ostree_repo_prepare_transaction (OstreeRepo *self, g_debug ("Preparing transaction in repository %p", self); /* Set up to abort the transaction if we return early from this function. */ - txn = self; + g_autoptr(_OstreeRepoAutoTransaction) txn = self; + (void) txn; /* Add use to silence static analysis */ memset (&self->txn.stats, 0, sizeof (OstreeRepoTransactionStats)); @@ -1652,7 +1652,7 @@ ostree_repo_prepare_transaction (OstreeRepo *self, return FALSE; /* Success: do not abort the transaction when returning. */ - txn = NULL; + txn = NULL; (void) txn; if (out_transaction_resume) *out_transaction_resume = ret_transaction_resume; From 25c5ae5d08727c4eb689ef7bd4b2a5096662fea2 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 18 Oct 2019 14:48:25 +0000 Subject: [PATCH 24/39] lib/pull: [scan-build] Silence a dead store warning This one was actual duplicate code. --- src/libostree/ostree-repo-pull.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index ba6e7289..bcb8b420 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -2181,7 +2181,6 @@ static void start_fetch (OtPullData *pull_data, FetchObjectData *fetch) { - gboolean is_meta; g_autofree char *obj_subpath = NULL; guint64 *expected_max_size_p; guint64 expected_max_size; @@ -2190,13 +2189,12 @@ start_fetch (OtPullData *pull_data, GPtrArray *mirrorlist = NULL; ostree_object_name_deserialize (fetch->object, &expected_checksum, &objtype); - is_meta = OSTREE_OBJECT_TYPE_IS_META (objtype); g_debug ("starting fetch of %s.%s%s", expected_checksum, ostree_object_type_to_string (objtype), fetch->is_detached_meta ? " (detached)" : ""); - is_meta = OSTREE_OBJECT_TYPE_IS_META (objtype); + gboolean is_meta = OSTREE_OBJECT_TYPE_IS_META (objtype); if (is_meta) pull_data->n_outstanding_metadata_fetches++; else From 0a808ffe20ebae0550399101b42af0bcc5bf7380 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 18 Oct 2019 14:48:44 +0000 Subject: [PATCH 25/39] tests: Port keyfile test to new style Just noticed in passing. --- tests/test-keyfile-utils.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/tests/test-keyfile-utils.c b/tests/test-keyfile-utils.c index 3e4f8257..c580f81c 100644 --- a/tests/test-keyfile-utils.c +++ b/tests/test-keyfile-utils.c @@ -199,14 +199,15 @@ test_get_value_with_default_group_optional (void) static void test_copy_group (void) { - gsize length, length2, ii; - GKeyFile *tmp = g_key_file_new (); + gsize length, length2; const char *section = "section"; GLogLevelFlags always_fatal_mask; /* Avoid that g_return_val_if_fail causes the test to fail. */ always_fatal_mask = g_log_set_always_fatal (0); + g_autoptr(GKeyFile) tmp = g_key_file_new (); + g_assert_false (ot_keyfile_copy_group (NULL, tmp, section)); g_assert_false (ot_keyfile_copy_group (g_keyfile, NULL, section)); g_assert_false (ot_keyfile_copy_group (g_keyfile, tmp, NULL)); @@ -220,17 +221,13 @@ test_copy_group (void) g_strfreev (g_key_file_get_keys (tmp, section, &length2, NULL)); g_assert_cmpint(length, ==, length2); - for (ii = 0; ii < length; ii++) + for (gsize ii = 0; ii < length; ii++) { - g_autofree char *value = NULL; - g_autofree char *value2 = NULL; - - value = g_key_file_get_value (g_keyfile, section, keys[ii], NULL); - value2 = g_key_file_get_value (g_keyfile, section, keys[ii], NULL); + g_autofree char *value = g_key_file_get_value (g_keyfile, section, keys[ii], NULL); + g_autofree char *value2 = g_key_file_get_value (g_keyfile, section, keys[ii], NULL); g_assert_cmpstr (value, ==, value2); } - g_key_file_free (tmp); } static void From 57bb06419a3e4c78c6165b755004ff40b685e4db Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 18 Oct 2019 14:56:51 +0000 Subject: [PATCH 26/39] lib: Port variant-builder.c to new style Seeing `scan-build` warning here, prep for fixing it. --- src/libotutil/ot-variant-builder.c | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/src/libotutil/ot-variant-builder.c b/src/libotutil/ot-variant-builder.c index 5b331e62..6636068e 100644 --- a/src/libotutil/ot-variant-builder.c +++ b/src/libotutil/ot-variant-builder.c @@ -1150,14 +1150,8 @@ ot_variant_builder_end (OtVariantBuilder *builder, GError **error) { OtVariantBuilderInfo *info = builder->head; - gsize total_size; - gsize offset_size; - int i; - g_autofree guchar *offset_table = NULL; - gsize offset_table_size; gboolean add_offset_table = FALSE; gboolean reverse_offset_table = FALSE; - guchar *p; g_return_val_if_fail (info->n_children >= info->min_items, FALSE); @@ -1188,15 +1182,14 @@ ot_variant_builder_end (OtVariantBuilder *builder, if (add_offset_table) { - total_size = gvs_calculate_total_size (info->offset, info->child_ends->len); - offset_size = gvs_get_offset_size (total_size); - - offset_table_size = total_size - info->offset; - offset_table = g_malloc (offset_table_size); - p = offset_table; + const gsize total_size = gvs_calculate_total_size (info->offset, info->child_ends->len); + const gsize offset_size = gvs_get_offset_size (total_size); + const gsize offset_table_size = total_size - info->offset; + g_autofree guchar *offset_table = g_malloc (offset_table_size); + guchar *p = offset_table; if (reverse_offset_table) { - for (i = info->child_ends->len - 1; i >= 0; i--) + for (int i = info->child_ends->len - 1; i >= 0; i--) { gvs_write_unaligned_le (p, g_array_index (info->child_ends, guint64, i), offset_size); p += offset_size; @@ -1204,7 +1197,7 @@ ot_variant_builder_end (OtVariantBuilder *builder, } else { - for (i = 0; i < info->child_ends->len; i++) + for (int i = 0; i < info->child_ends->len; i++) { gvs_write_unaligned_le (p, g_array_index (info->child_ends, guint64, i), offset_size); p += offset_size; From 01a3a655251a8d551259c99fae3dbd33343772bc Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 18 Oct 2019 14:57:47 +0000 Subject: [PATCH 27/39] tests: [scan-build] Initialize a variable False positive. --- tests/libostreetest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/libostreetest.c b/tests/libostreetest.c index 9a2fae09..1efee40e 100644 --- a/tests/libostreetest.c +++ b/tests/libostreetest.c @@ -145,7 +145,7 @@ ot_test_setup_sysroot (GCancellable *cancellable, g_autoptr(GString) buf = g_string_new ("mutable-deployments"); - gboolean can_relabel; + gboolean can_relabel = FALSE; if (!ot_check_relabeling (&can_relabel, error)) return FALSE; if (!can_relabel) From fbed380483525fb0780d07d904e8144b7e86ab0b Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 18 Oct 2019 15:06:51 +0000 Subject: [PATCH 28/39] lib/checksum-utils: Use g_memdup() This is clearer and silences a scan-build warning. --- src/libotutil/ot-checksum-utils.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/libotutil/ot-checksum-utils.c b/src/libotutil/ot-checksum-utils.c index 6eb6fdc0..66767368 100644 --- a/src/libotutil/ot-checksum-utils.c +++ b/src/libotutil/ot-checksum-utils.c @@ -250,9 +250,8 @@ ot_gio_splice_get_checksum (GOutputStream *out, guint8 digest[_OSTREE_SHA256_DIGEST_LEN]; ot_checksum_get_digest (&checksum, digest, sizeof (digest)); - g_autofree guchar *ret_csum = g_malloc (sizeof (digest)); - memcpy (ret_csum, digest, sizeof (digest)); - ot_transfer_out_value (out_csum, &ret_csum); + if (out_csum) + *out_csum = g_memdup (digest, sizeof (digest)); return TRUE; } From 4df90d40121df9ba38b8f379660abbfac0212ef7 Mon Sep 17 00:00:00 2001 From: Dan Nicholson Date: Fri, 18 Oct 2019 11:10:44 -0600 Subject: [PATCH 29/39] repo: Stop using deprecated G_GNUC_FUNCTION In glib 2.62 this has been changed to emitting a warning. Use G_STRFUNC instead, which has been available for a long time and is already used in other places in ostree. --- src/libostree/ostree-repo.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 584037c4..cff70d47 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -1548,8 +1548,8 @@ keyfile_set_from_vardict (GKeyFile *keyfile, g_key_file_set_string_list (keyfile, section, key, strv_child, len); } else - g_critical ("Unhandled type '%s' in " G_GNUC_FUNCTION, - (char*)g_variant_get_type (child)); + g_critical ("Unhandled type '%s' in %s", + (char*)g_variant_get_type (child), G_STRFUNC); } } From 8f0b225d600bdead23b4cf32d092a33b372eae57 Mon Sep 17 00:00:00 2001 From: Alex Kiernan Date: Sat, 19 Oct 2019 22:20:25 +0100 Subject: [PATCH 30/39] Revert "Gate ostree-trivial-httpd on BUILDOPT_TRIVIAL_HTTPD" This reverts commit 83d44ac20ae80d74e05d89744fd1fbd4f45b7fba. --- Makefile-ostree.am | 3 +-- configure.ac | 3 --- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/Makefile-ostree.am b/Makefile-ostree.am index 7b53cb14..76f39cad 100644 --- a/Makefile-ostree.am +++ b/Makefile-ostree.am @@ -143,13 +143,12 @@ ostree_SOURCES += src/ostree/ot-builtin-pull.c endif if USE_LIBSOUP -if BUILDOPT_TRIVIAL_HTTPD +# Eventually once we stop things from using this, we should support disabling this ostree_SOURCES += src/ostree/ot-builtin-trivial-httpd.c pkglibexec_PROGRAMS += ostree-trivial-httpd ostree_trivial_httpd_SOURCES = src/ostree/ostree-trivial-httpd.c ostree_trivial_httpd_CFLAGS = $(ostree_bin_shared_cflags) $(OT_INTERNAL_SOUP_CFLAGS) ostree_trivial_httpd_LDADD = $(ostree_bin_shared_ldadd) $(OT_INTERNAL_SOUP_LIBS) -endif if !USE_CURL # This is necessary for the cookie jar bits diff --git a/configure.ac b/configure.ac index e009d6b2..6861afb8 100644 --- a/configure.ac +++ b/configure.ac @@ -195,9 +195,6 @@ AC_ARG_ENABLE(trivial-httpd-cmdline, [Continue to support "ostree trivial-httpd" [default=no]])],, enable_trivial_httpd_cmdline=no) AM_CONDITIONAL(BUILDOPT_TRIVIAL_HTTPD, test x$enable_trivial_httpd_cmdline = xyes) -AS_IF([test x$with_soup = xno && test x$enable_trivial_httpd_cmdline = xyes], [ - AC_MSG_ERROR([trivial-httpd enabled, but libsoup is not; libsoup is needed for trivial-httpd]) -]) AM_COND_IF(BUILDOPT_TRIVIAL_HTTPD, [AC_DEFINE([BUILDOPT_ENABLE_TRIVIAL_HTTPD_CMDLINE], 1, [Define if we are enabling ostree trivial-httpd entrypoint])] ) From 967ea6692187e43487f939298eff1e8024399775 Mon Sep 17 00:00:00 2001 From: Alex Kiernan Date: Sat, 19 Oct 2019 22:20:27 +0100 Subject: [PATCH 31/39] Revert "Always enable trivial-httpd for tests" This reverts commit 82699a67dbb6bfcc9452bb969e7872809232a84f. --- Makefile.am | 1 - ci/build.sh | 5 ----- ci/travis-build.sh | 1 - 3 files changed, 7 deletions(-) diff --git a/Makefile.am b/Makefile.am index 673dbf88..cd04a055 100644 --- a/Makefile.am +++ b/Makefile.am @@ -39,7 +39,6 @@ AM_DISTCHECK_CONFIGURE_FLAGS += \ --enable-gtk-doc \ --enable-man \ --disable-maintainer-mode \ - --enable-trivial-httpd-cmdline \ $(NULL) GITIGNOREFILES = aclocal.m4 build-aux/ buildutil/*.m4 config.h.in gtk-doc.make diff --git a/ci/build.sh b/ci/build.sh index 806af050..09015074 100755 --- a/ci/build.sh +++ b/ci/build.sh @@ -22,11 +22,6 @@ case "${CONFIGOPTS:-}" in fi ;; esac -# unless libsoup is disabled, enable trivial-httpd for the tests -case "${CONFIGOPTS:-}" in - *--without-soup*) ;; - *) CONFIGOPTS="${CONFIGOPTS:-} --enable-trivial-httpd-cmdline" ;; -esac # always fail on warnings; https://github.com/ostreedev/ostree/pull/971 # NB: this disables the default set of flags from configure.ac diff --git a/ci/travis-build.sh b/ci/travis-build.sh index 7c85313a..3fd969bd 100755 --- a/ci/travis-build.sh +++ b/ci/travis-build.sh @@ -85,7 +85,6 @@ make="make -j${ci_parallel} V=1 VERBOSE=1" ../configure \ --enable-always-build-tests \ - --enable-trivial-httpd-cmdline \ ${ci_configopts} "$@" From ac4e3ab3e6e01b5d46fc16f86a0f0e0285f90d05 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 21 Oct 2019 21:45:20 +0000 Subject: [PATCH 32/39] build-sys: Cleanup handling for trivial-httpd-cmdline This way it's clearer this bit is only about the CLI entrypoint also living in `ostree trivial-httpd`, not the underlying `ostree-trivial-httpd` binary that's separate now. Delete the automake conditional for this, and make the manpage conditional use `if USE_LIBSOUP` the same way the C build does. Suggested-by: Jonathan Lebon --- Makefile-man.am | 2 +- configure.ac | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/Makefile-man.am b/Makefile-man.am index c27a9a55..bc58103b 100644 --- a/Makefile-man.am +++ b/Makefile-man.am @@ -34,7 +34,7 @@ ostree-init.1 ostree-log.1 ostree-ls.1 ostree-prune.1 ostree-pull-local.1 \ ostree-pull.1 ostree-refs.1 ostree-remote.1 ostree-reset.1 \ ostree-rev-parse.1 ostree-show.1 ostree-summary.1 \ ostree-static-delta.1 -if BUILDOPT_TRIVIAL_HTTPD +if USE_LIBSOUP man1_files += ostree-trivial-httpd.1 else # We still want to distribute the source, even if we are not building it diff --git a/configure.ac b/configure.ac index 6861afb8..53994be1 100644 --- a/configure.ac +++ b/configure.ac @@ -194,8 +194,7 @@ AC_ARG_ENABLE(trivial-httpd-cmdline, [AS_HELP_STRING([--enable-trivial-httpd-cmdline], [Continue to support "ostree trivial-httpd" [default=no]])],, enable_trivial_httpd_cmdline=no) -AM_CONDITIONAL(BUILDOPT_TRIVIAL_HTTPD, test x$enable_trivial_httpd_cmdline = xyes) -AM_COND_IF(BUILDOPT_TRIVIAL_HTTPD, +AS_IF([test x$enable_trivial_httpd_cmdline = xyes], [AC_DEFINE([BUILDOPT_ENABLE_TRIVIAL_HTTPD_CMDLINE], 1, [Define if we are enabling ostree trivial-httpd entrypoint])] ) From d4a186e80e139b366c527d6398173bd1a80c7c0b Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Wed, 23 Oct 2019 11:04:00 -0400 Subject: [PATCH 33/39] lib/pull: Avoid calling destroy on unref'ed GSource We're creating the timer source and then passing ownership to the context, but because we didn't free the pointer, we would still call `g_source_destroy` in the exit path. We'd do this right after doing `unref` on the context too, which would have already destroyed and unref'ed the source. Drop that and just restrict the scope of that variable down to make things more obvious. Just noticed this after reviewing #1953. --- src/libostree/ostree-repo-pull.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index bcb8b420..dd4dbff1 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -3573,7 +3573,6 @@ ostree_repo_pull_with_options (OstreeRepo *self, g_autofree char **refs_to_fetch = NULL; g_autoptr(GVariantIter) collection_refs_iter = NULL; g_autofree char **override_commit_ids = NULL; - GSource *update_timeout = NULL; gboolean opt_gpg_verify_set = FALSE; gboolean opt_gpg_verify_summary_set = FALSE; gboolean opt_collection_refs_set = FALSE; @@ -4505,6 +4504,8 @@ ostree_repo_pull_with_options (OstreeRepo *self, if (pull_data->progress) { + g_autoptr(GSource) update_timeout = NULL; + /* Setup a custom frequency if set */ if (update_frequency > 0) update_timeout = g_timeout_source_new (pull_data->dry_run ? 0 : update_frequency); @@ -4514,7 +4515,6 @@ ostree_repo_pull_with_options (OstreeRepo *self, g_source_set_priority (update_timeout, G_PRIORITY_HIGH); g_source_set_callback (update_timeout, update_progress, pull_data, NULL); g_source_attach (update_timeout, pull_data->main_context); - g_source_unref (update_timeout); } /* Now await work completion */ @@ -4732,8 +4732,6 @@ ostree_repo_pull_with_options (OstreeRepo *self, if (!inherit_transaction) ostree_repo_abort_transaction (pull_data->repo, cancellable, NULL); g_main_context_unref (pull_data->main_context); - if (update_timeout) - g_source_destroy (update_timeout); g_strfreev (configured_branches); g_clear_object (&pull_data->fetcher); g_clear_pointer (&pull_data->extra_headers, (GDestroyNotify)g_variant_unref); From 650d6252af4ddd9fa82d41e934a221b82c694233 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 24 Oct 2019 15:21:17 +0000 Subject: [PATCH 34/39] Revert "grub2: Exit gracefully if the configuration has BLS enabled" This reverts commit 985a14100295c99d0c6d712bfbee0ec52a3a1601. It turned out that some people have old bootloaders, and hence get the "no entries" problem. That's much, much much worse than double entries. --- src/boot/grub2/grub2-15_ostree | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/boot/grub2/grub2-15_ostree b/src/boot/grub2/grub2-15_ostree index 160ac2ca..0b9bf930 100644 --- a/src/boot/grub2/grub2-15_ostree +++ b/src/boot/grub2/grub2-15_ostree @@ -26,13 +26,6 @@ if ! test -d /ostree/repo; then exit 0 fi -# Gracefully exit if the grub2 configuration has BLS enabled, -# since there is no need to create menu entries in that case. -. /etc/default/grub -if test ${GRUB_ENABLE_BLSCFG} = "true"; then - exit 0 -fi - # Make sure we're in the right environment if ! test -n "${GRUB_DEVICE}"; then echo "This script must be run as a child of grub2-mkconfig" 1>&2 From e314b31ec90ec4fbd4a2d9c2eca55383e7ed4a9a Mon Sep 17 00:00:00 2001 From: Alex Kiernan Date: Thu, 24 Oct 2019 19:07:30 +0100 Subject: [PATCH 35/39] tests/export: Guard with check for libarchive If we are built without libarchive support, this test fails: error: This version of ostree is not compiled with libarchive support ... ERROR: tests/test-export.sh - too few tests run (expected 5, got 0) ERROR: tests/test-export.sh - exited with status 1 Signed-off-by: Alex Kiernan --- tests/test-export.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/test-export.sh b/tests/test-export.sh index e1a0346c..17f7c024 100755 --- a/tests/test-export.sh +++ b/tests/test-export.sh @@ -21,6 +21,11 @@ set -euo pipefail +if ! ostree --version | grep -q -e '- libarchive'; then + echo "1..0 #SKIP no libarchive support compiled in" + exit 0 +fi + . $(dirname $0)/libtest.sh setup_test_repository "archive" From 78c8c25d6446dfe417c5835269cf6d6db38f9490 Mon Sep 17 00:00:00 2001 From: Umang Jain Date: Fri, 25 Oct 2019 20:58:34 +0530 Subject: [PATCH 36/39] async-progress: Plug memory leak while destroying GSource See https://gitlab.gnome.org/GNOME/glib/commit/71973c722 --- src/libostree/ostree-async-progress.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libostree/ostree-async-progress.c b/src/libostree/ostree-async-progress.c index a8e629ee..64372c27 100644 --- a/src/libostree/ostree-async-progress.c +++ b/src/libostree/ostree-async-progress.c @@ -465,7 +465,7 @@ ostree_async_progress_finish (OstreeAsyncProgress *self) if (self->idle_source) { g_source_destroy (self->idle_source); - self->idle_source = NULL; + g_clear_pointer (&self->idle_source, g_source_unref); emit_changed = TRUE; } } From 74936f98d89514820c4d1ec574269fd0a140f1da Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Mon, 28 Oct 2019 14:04:55 -0400 Subject: [PATCH 37/39] lib/pull: Tweak update_timeout logic again I was hitting `SIGSEGV` when running `cosa build` and narrowed it down to #1954. What's happening here is that because we're using the default context, when we unref it in the out path, it may not actually destroy the `GSource` if it (the context) is still ref'ed elsewhere. So then, we'd still get events from it if subsequent operations iterated the context. This patch is mostly a revert of #1954, except that we still keep a ref on the `GSource`. That way it is always safe to destroy it afterwards. (And I've also added a comment to explain this better.) --- src/libostree/ostree-repo-pull.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index dd4dbff1..586b34fe 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -3573,6 +3573,7 @@ ostree_repo_pull_with_options (OstreeRepo *self, g_autofree char **refs_to_fetch = NULL; g_autoptr(GVariantIter) collection_refs_iter = NULL; g_autofree char **override_commit_ids = NULL; + g_autoptr(GSource) update_timeout = NULL; gboolean opt_gpg_verify_set = FALSE; gboolean opt_gpg_verify_summary_set = FALSE; gboolean opt_collection_refs_set = FALSE; @@ -3674,6 +3675,10 @@ ostree_repo_pull_with_options (OstreeRepo *self, pull_data->async_error = &pull_data->cached_async_error; else pull_data->async_error = NULL; + + /* Note we're using the thread default (or global) context here, so it may outlive the + * OtPullData object if there's another ref on it. Thus, always detach/destroy sources + * local to the `ostree_repo_pull*` operation rather than trying to transfer ownership. */ pull_data->main_context = g_main_context_ref_thread_default (); pull_data->flags = flags; @@ -4504,8 +4509,6 @@ ostree_repo_pull_with_options (OstreeRepo *self, if (pull_data->progress) { - g_autoptr(GSource) update_timeout = NULL; - /* Setup a custom frequency if set */ if (update_frequency > 0) update_timeout = g_timeout_source_new (pull_data->dry_run ? 0 : update_frequency); @@ -4732,6 +4735,8 @@ ostree_repo_pull_with_options (OstreeRepo *self, if (!inherit_transaction) ostree_repo_abort_transaction (pull_data->repo, cancellable, NULL); g_main_context_unref (pull_data->main_context); + if (update_timeout) + g_source_destroy (update_timeout); g_strfreev (configured_branches); g_clear_object (&pull_data->fetcher); g_clear_pointer (&pull_data->extra_headers, (GDestroyNotify)g_variant_unref); From 7ae8da08b9f832bbaf6c9c50737e25116ec7ca9c Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Tue, 29 Oct 2019 16:45:29 -0400 Subject: [PATCH 38/39] lib/deploy: Also install HMAC file into /boot To allow for FIPS mode, we need to also install the HMAC file from `/usr/lib/modules` to `/boot` alongside the kernel image where the `fips` dracut module will find it. For details, see: https://github.com/coreos/fedora-coreos-tracker/issues/302 Note I didn't include the file in the boot checksum since it's itself a checksum of the kernel, so we don't really gain much here other than potentially causing an unnecessary bootcsum bump. --- src/libostree/ostree-sysroot-deploy.c | 28 +++++++++++++++++++++++++++ tests/libtest.sh | 3 +++ tests/test-admin-deploy-none.sh | 1 + 3 files changed, 32 insertions(+) diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index 15db0bf1..a09c354b 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -878,6 +878,8 @@ typedef struct { int boot_dfd; char *kernel_srcpath; char *kernel_namever; + char *kernel_hmac_srcpath; + char *kernel_hmac_namever; char *initramfs_srcpath; char *initramfs_namever; char *devicetree_srcpath; @@ -890,6 +892,8 @@ _ostree_kernel_layout_free (OstreeKernelLayout *layout) glnx_close_fd (&layout->boot_dfd); g_free (layout->kernel_srcpath); g_free (layout->kernel_namever); + g_free (layout->kernel_hmac_srcpath); + g_free (layout->kernel_hmac_namever); g_free (layout->initramfs_srcpath); g_free (layout->initramfs_namever); g_free (layout->devicetree_srcpath); @@ -1032,6 +1036,16 @@ get_kernel_from_tree_usrlib_modules (int deployment_dfd, g_clear_object (&in); glnx_close_fd (&fd); + /* And finally, look for any HMAC file. This is needed for FIPS mode on some distros. */ + if (!glnx_fstatat_allow_noent (ret_layout->boot_dfd, ".vmlinuz.hmac", NULL, 0, error)) + return FALSE; + if (errno == 0) + { + ret_layout->kernel_hmac_srcpath = g_strdup (".vmlinuz.hmac"); + /* Name it as dracut expects it: https://github.com/dracutdevs/dracut/blob/225e4b94cbdb702cf512490dcd2ad9ca5f5b22c1/modules.d/01fips/fips.sh#L129 */ + ret_layout->kernel_hmac_namever = g_strdup_printf (".%s.hmac", ret_layout->kernel_namever); + } + char hexdigest[OSTREE_SHA256_STRING_LEN+1]; ot_checksum_get_hexdigest (&checksum, hexdigest, sizeof (hexdigest)); ret_layout->bootcsum = g_strdup (hexdigest); @@ -1686,6 +1700,20 @@ install_deployment_kernel (OstreeSysroot *sysroot, } } + if (kernel_layout->kernel_hmac_srcpath) + { + if (!glnx_fstatat_allow_noent (bootcsum_dfd, kernel_layout->kernel_hmac_namever, &stbuf, 0, error)) + return FALSE; + if (errno == ENOENT) + { + if (!install_into_boot (sepolicy, kernel_layout->boot_dfd, kernel_layout->kernel_hmac_srcpath, + bootcsum_dfd, kernel_layout->kernel_hmac_namever, + sysroot->debug_flags, + cancellable, error)) + return FALSE; + } + } + g_autofree char *contents = NULL; if (!glnx_fstatat_allow_noent (deployment_dfd, "usr/lib/os-release", &stbuf, 0, error)) return FALSE; diff --git a/tests/libtest.sh b/tests/libtest.sh index 8832e63c..ba00073a 100755 --- a/tests/libtest.sh +++ b/tests/libtest.sh @@ -395,6 +395,8 @@ setup_os_repository () { mkdir -p usr/bin ${bootdir} usr/lib/modules/${kver} usr/share usr/etc kernel_path=${bootdir}/vmlinuz initramfs_path=${bootdir}/initramfs.img + # the HMAC file is only in /usr/lib/modules + hmac_path=usr/lib/modules/${kver}/.vmlinuz.hmac # /usr/lib/modules just uses "vmlinuz", since the version is in the module # directory name. if [[ $bootdir != usr/lib/modules/* ]]; then @@ -403,6 +405,7 @@ setup_os_repository () { fi echo "a kernel" > ${kernel_path} echo "an initramfs" > ${initramfs_path} + echo "an hmac file" > ${hmac_path} bootcsum=$(cat ${kernel_path} ${initramfs_path} | sha256sum | cut -f 1 -d ' ') export bootcsum # Add the checksum for legacy dirs (/boot, /usr/lib/ostree-boot), but not diff --git a/tests/test-admin-deploy-none.sh b/tests/test-admin-deploy-none.sh index 66a1491f..09dee624 100755 --- a/tests/test-admin-deploy-none.sh +++ b/tests/test-admin-deploy-none.sh @@ -43,6 +43,7 @@ ${CMD_PREFIX} ostree admin deploy --karg=root=LABEL=MOO --karg=quiet --os testos assert_file_has_content out.txt "Bootloader updated.*" assert_file_has_content sysroot/boot/loader/entries/ostree-1-testos.conf 'options.* root=LABEL=MOO' assert_file_has_content sysroot/boot/ostree/testos-${bootcsum}/vmlinuz-3.6.0 'a kernel' +assert_file_has_content sysroot/boot/ostree/testos-${bootcsum}/.vmlinuz-3.6.0.hmac 'an hmac file' assert_file_has_content sysroot/boot/ostree/testos-${bootcsum}/initramfs-3.6.0.img 'an initramfs' echo "ok generate bls config on first deployment" From 980ca07b03b3aa7e0012729dd6c84b0878775d93 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 24 Oct 2019 15:18:23 +0000 Subject: [PATCH 39/39] Release 2019.5 --- configure.ac | 2 +- src/libostree/libostree-released.sym | 2 ++ tests/test-symbols.sh | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index 53994be1..4b022e36 100644 --- a/configure.ac +++ b/configure.ac @@ -10,7 +10,7 @@ m4_define([year_version], [2019]) m4_define([release_version], [5]) m4_define([package_version], [year_version.release_version]) AC_INIT([libostree], [package_version], [walters@verbum.org]) -is_release_build=no +is_release_build=yes AC_CONFIG_HEADER([config.h]) AC_CONFIG_MACRO_DIR([buildutil]) AC_CONFIG_AUX_DIR([build-aux]) diff --git a/src/libostree/libostree-released.sym b/src/libostree/libostree-released.sym index ec9eec66..f6a83b77 100644 --- a/src/libostree/libostree-released.sym +++ b/src/libostree/libostree-released.sym @@ -575,6 +575,8 @@ LIBOSTREE_2019.4 { ostree_repo_mark_commit_partial_reason; } LIBOSTREE_2019.3; +/* No new symbols in 2019.5 */ + /* 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 32285bdc..2287710d 100755 --- a/tests/test-symbols.sh +++ b/tests/test-symbols.sh @@ -54,7 +54,7 @@ echo 'ok documented symbols' # ONLY update this checksum in release commits! cat > released-sha256.txt <