From 0134c621575fcb43bb9d994251831845dac8dbd1 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 20 Jun 2016 18:13:58 -0400 Subject: [PATCH 01/39] libostree.sym: Fix test-symbols The test isn't smart enough to ignore comments, so change the prefix. Closes: #356 Approved by: jlebon --- src/libostree/libostree.sym | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libostree/libostree.sym b/src/libostree/libostree.sym index e8cca9d8..087f9879 100644 --- a/src/libostree/libostree.sym +++ b/src/libostree/libostree.sym @@ -350,6 +350,6 @@ global: /* LIBOSTREE_2016.7 { global: - ostree_some_new_symbol; + someostree_some_new_symbol; } LIBOSTREE_2016.6; */ From 4819b44189cbdde189a2693e081be39483838e78 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 14 Jun 2016 10:46:15 -0400 Subject: [PATCH 02/39] libglnx porting: Use of GSDirFdIterator This one was pretty simple. One of the uses in `repo.c` was just a leftover variable. Closes: #341 Approved by: jlebon --- src/libostree/ostree-repo-commit.c | 47 ++++++++++-------------------- src/libostree/ostree-repo.c | 1 - 2 files changed, 16 insertions(+), 32 deletions(-) diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index 685eadd0..cec19e4a 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -1173,43 +1173,31 @@ rename_pending_loose_objects (OstreeRepo *self, GError **error) { gboolean ret = FALSE; - gs_dirfd_iterator_cleanup GSDirFdIterator dfd_iter = { 0, }; + g_auto(GLnxDirFdIterator) dfd_iter = { 0, }; - if (!gs_dirfd_iterator_init_at (self->commit_stagedir_fd, ".", FALSE, &dfd_iter, error)) + if (!glnx_dirfd_iterator_init_at (self->commit_stagedir_fd, ".", FALSE, &dfd_iter, error)) goto out; /* Iterate over the outer checksum dir */ while (TRUE) { struct dirent *dent; - gs_dirfd_iterator_cleanup GSDirFdIterator child_dfd_iter = { 0, }; - struct stat stbuf; - int res; + g_auto(GLnxDirFdIterator) child_dfd_iter = { 0, }; - if (!gs_dirfd_iterator_next_dent (&dfd_iter, &dent, cancellable, error)) + if (!glnx_dirfd_iterator_next_dent_ensure_dtype (&dfd_iter, &dent, cancellable, error)) goto out; - if (dent == NULL) break; - do - res = fstatat (dfd_iter.fd, dent->d_name, &stbuf, AT_SYMLINK_NOFOLLOW); - while (G_UNLIKELY (res == -1 && errno == EINTR)); - if (res == -1) - { - glnx_set_error_from_errno (error); - goto out; - } - - if (!S_ISDIR (stbuf.st_mode)) + if (dent->d_type != DT_DIR) continue; /* All object directories only have two character entries */ if (strlen (dent->d_name) != 2) continue; - if (!gs_dirfd_iterator_init_at (dfd_iter.fd, dent->d_name, FALSE, - &child_dfd_iter, error)) + if (!glnx_dirfd_iterator_init_at (dfd_iter.fd, dent->d_name, FALSE, + &child_dfd_iter, error)) goto out; /* Iterate over inner checksum dir */ @@ -1218,9 +1206,8 @@ rename_pending_loose_objects (OstreeRepo *self, struct dirent *child_dent; char loose_objpath[_OSTREE_LOOSE_PATH_MAX]; - if (!gs_dirfd_iterator_next_dent (&child_dfd_iter, &child_dent, cancellable, error)) + if (!glnx_dirfd_iterator_next_dent (&child_dfd_iter, &child_dent, cancellable, error)) goto out; - if (child_dent == NULL) break; @@ -2412,7 +2399,7 @@ write_directory_to_mtree_internal (OstreeRepo *self, GError **error); static gboolean write_dfd_iter_to_mtree_internal (OstreeRepo *self, - GSDirFdIterator *src_dfd_iter, + GLnxDirFdIterator *src_dfd_iter, OstreeMutableTree *mtree, OstreeRepoCommitModifier *modifier, GPtrArray *path, @@ -2423,7 +2410,7 @@ static gboolean write_directory_content_to_mtree_internal (OstreeRepo *self, OstreeRepoFile *repo_dir, GFileEnumerator *dir_enum, - GSDirFdIterator *dfd_iter, + GLnxDirFdIterator *dfd_iter, GFileInfo *child_info, OstreeMutableTree *mtree, OstreeRepoCommitModifier *modifier, @@ -2488,9 +2475,9 @@ write_directory_content_to_mtree_internal (OstreeRepo *self, } else { - gs_dirfd_iterator_cleanup GSDirFdIterator child_dfd_iter = { 0, }; + g_auto(GLnxDirFdIterator) child_dfd_iter = { 0, }; - if (!gs_dirfd_iterator_init_at (dfd_iter->fd, name, FALSE, &child_dfd_iter, error)) + if (!glnx_dirfd_iterator_init_at (dfd_iter->fd, name, FALSE, &child_dfd_iter, error)) goto out; if (!write_dfd_iter_to_mtree_internal (self, &child_dfd_iter, child_mtree, @@ -2693,7 +2680,7 @@ write_directory_to_mtree_internal (OstreeRepo *self, static gboolean write_dfd_iter_to_mtree_internal (OstreeRepo *self, - GSDirFdIterator *src_dfd_iter, + GLnxDirFdIterator *src_dfd_iter, OstreeMutableTree *mtree, OstreeRepoCommitModifier *modifier, GPtrArray *path, @@ -2760,9 +2747,8 @@ write_dfd_iter_to_mtree_internal (OstreeRepo *self, g_autoptr(GFileInfo) child_info = NULL; const char *loose_checksum; - if (!gs_dirfd_iterator_next_dent (src_dfd_iter, &dent, cancellable, error)) + if (!glnx_dirfd_iterator_next_dent (src_dfd_iter, &dent, cancellable, error)) goto out; - if (dent == NULL) break; @@ -2888,15 +2874,14 @@ ostree_repo_write_dfd_to_mtree (OstreeRepo *self, { gboolean ret = FALSE; g_autoptr(GPtrArray) pathbuilder = NULL; - gs_dirfd_iterator_cleanup GSDirFdIterator dfd_iter = { 0, }; + g_auto(GLnxDirFdIterator) dfd_iter = { 0, }; if (modifier && modifier->flags & OSTREE_REPO_COMMIT_MODIFIER_FLAGS_GENERATE_SIZES) self->generate_sizes = TRUE; pathbuilder = g_ptr_array_new (); - if (!gs_dirfd_iterator_init_at (dfd, path, FALSE, - &dfd_iter, error)) + if (!glnx_dirfd_iterator_init_at (dfd, path, FALSE, &dfd_iter, error)) goto out; if (!write_dfd_iter_to_mtree_internal (self, &dfd_iter, mtree, modifier, pathbuilder, diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index e86685bf..0b918ed8 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -4825,7 +4825,6 @@ _ostree_repo_allocate_tmpdir (int tmpdir_dfd, while (tmpdir_name == NULL) { - gs_dirfd_iterator_cleanup GSDirFdIterator child_dfd_iter = { 0, }; struct dirent *dent; glnx_fd_close int existing_tmpdir_fd = -1; g_autoptr(GError) local_error = NULL; From 9e2763106be01044caa541314d3fe185cc73361d Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 14 Jun 2016 13:12:21 -0400 Subject: [PATCH 03/39] lib: Use sd_journal directly (optionally) This was the last caller of libgsystem that isn't `gs_file_get_path_cached()`. I think the use case ostree has where the same code can be called via command line and via a shared library *and* via a daemon is rather unusual, so let's just copy the code for logging from libgsystem into here. For example rpm-ostree hard depends on a daemon mode, so it'll just use `sd_journal` directly. Closes: #341 Approved by: jlebon --- Makefile-libostree.am | 5 + Makefile-ostree.am | 2 +- Makefile-otutil.am | 6 +- configure.ac | 12 +- src/libostree/ostree-sysroot-deploy.c | 6 +- src/libotutil/ot-log-utils.c | 163 ++++++++++++++++++++++++++ src/libotutil/ot-log-utils.h | 31 +++++ src/libotutil/otutil.h | 1 + 8 files changed, 216 insertions(+), 10 deletions(-) create mode 100644 src/libotutil/ot-log-utils.c create mode 100644 src/libotutil/ot-log-utils.h diff --git a/Makefile-libostree.am b/Makefile-libostree.am index d6b83528..8c2fd297 100644 --- a/Makefile-libostree.am +++ b/Makefile-libostree.am @@ -156,6 +156,11 @@ libostree_1_la_CFLAGS += $(OT_DEP_LIBARCHIVE_CFLAGS) libostree_1_la_LIBADD += $(OT_DEP_LIBARCHIVE_LIBS) endif +if BUILDOPT_LIBSYSTEMD +libostree_1_la_CFLAGS += $(LIBSYSTEMD_CFLAGS) +libostree_1_la_LIBADD += $(LIBSYSTEMD_LIBS) +endif + if USE_LIBSOUP libostree_1_la_SOURCES += \ src/libostree/ostree-fetcher.h \ diff --git a/Makefile-ostree.am b/Makefile-ostree.am index 0ef5c4ee..33875dc4 100644 --- a/Makefile-ostree.am +++ b/Makefile-ostree.am @@ -99,7 +99,7 @@ ostree_bin_shared_cflags = $(AM_CFLAGS) -I$(srcdir)/src/libotutil -I$(srcdir)/sr ostree_bin_shared_ldadd = libglnx.la libbsdiff.la libotutil.la libostree-kernel-args.la libostree-1.la ostree_CFLAGS = $(ostree_bin_shared_cflags) $(OT_INTERNAL_GIO_UNIX_CFLAGS) -I$(srcdir)/libglnx -ostree_LDADD = $(ostree_bin_shared_ldadd) $(OT_INTERNAL_GIO_UNIX_LIBS) +ostree_LDADD = $(ostree_bin_shared_ldadd) $(OT_INTERNAL_GIO_UNIX_LIBS) $(LIBSYSTEMD_LIBS) if USE_LIBSOUP ostree_SOURCES += \ diff --git a/Makefile-otutil.am b/Makefile-otutil.am index efcb04b7..ee892a70 100644 --- a/Makefile-otutil.am +++ b/Makefile-otutil.am @@ -36,6 +36,8 @@ libotutil_la_SOURCES = \ src/libotutil/ot-variant-utils.h \ src/libotutil/ot-gio-utils.c \ src/libotutil/ot-gio-utils.h \ + src/libotutil/ot-log-utils.c \ + src/libotutil/ot-log-utils.h \ src/libotutil/ot-gpg-utils.c \ src/libotutil/ot-gpg-utils.h \ src/libotutil/otutil.c \ @@ -43,5 +45,5 @@ libotutil_la_SOURCES = \ src/libotutil/ot-tool-util.c \ src/libotutil/ot-tool-util.h \ $(NULL) -libotutil_la_CFLAGS = $(AM_CFLAGS) -I$(srcdir)/libglnx -I$(srcdir)/src/libotutil -DLOCALEDIR=\"$(datadir)/locale\" $(OT_INTERNAL_GIO_UNIX_CFLAGS) $(OT_INTERNAL_GPGME_CFLAGS) -libotutil_la_LIBADD = $(OT_INTERNAL_GIO_UNIX_LIBS) $(OT_INTERNAL_GPGME_LIBS) +libotutil_la_CFLAGS = $(AM_CFLAGS) -I$(srcdir)/libglnx -I$(srcdir)/src/libotutil -DLOCALEDIR=\"$(datadir)/locale\" $(OT_INTERNAL_GIO_UNIX_CFLAGS) $(OT_INTERNAL_GPGME_CFLAGS) $(LIBSYSTEMD_CFLAGS) +libotutil_la_LIBADD = $(OT_INTERNAL_GIO_UNIX_LIBS) $(OT_INTERNAL_GPGME_LIBS) $(LIBSYSTEMD_LIBS) diff --git a/configure.ac b/configure.ac index 18f9f277..d28c410a 100644 --- a/configure.ac +++ b/configure.ac @@ -251,7 +251,13 @@ AC_ARG_WITH(mkinitcpio, [with_mkinitcpio=no]) AM_CONDITIONAL(BUILDOPT_MKINITCPIO, test x$with_mkinitcpio = xyes) -AS_IF([test "x$with_dracut" = "xyes" || test "x$with_dracut" = "xyesbutnoconf" || test "x$with_mkinitcpio" = "xyes"], [ +dnl We have separate checks for libsystemd and the unit dir for historical reasons +PKG_CHECK_MODULES([LIBSYSTEMD], [libsystemd], [have_libsystemd=yes], [have_libsystemd=no]) +AM_CONDITIONAL(BUILDOPT_LIBSYSTEMD, test x$have_libsystemd = xyes) +AM_COND_IF(BUILDOPT_LIBSYSTEMD, + AC_DEFINE([HAVE_LIBSYSTEMD], 1, [Define if we have libsystemd])) + +AS_IF([test "x$have_libsystemd" = "xyes"], [ with_systemd=yes AC_ARG_WITH([systemdsystemunitdir], AS_HELP_STRING([--with-systemdsystemunitdir=DIR], [Directory for systemd service files]), @@ -309,6 +315,7 @@ echo " libsoup (retrieve remote HTTP repositories): $with_soup libsoup TLS client certs: $have_libsoup_client_certs SELinux: $with_selinux + systemd: $have_libsystemd libmount: $with_libmount libarchive (parse tar files directly): $with_libarchive static deltas: yes (always enabled now) @@ -322,7 +329,4 @@ AS_IF([test x$with_builtin_grub2_mkconfig = xyes], [ ], [ echo " grub2-mkconfig path: $GRUB2_MKCONFIG" ]) -AS_IF([test "x$with_systemd" = "xyes"], [ - echo " systemd unit dir: $with_systemdsystemunitdir" -]) echo "" diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index 4616bab1..cc1a1faa 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -473,7 +473,7 @@ merge_etc_changes (GFile *orig_etc, goto out; } - gs_log_structured_print_id_v (OSTREE_CONFIGMERGE_ID, + ot_log_structured_print_id_v (OSTREE_CONFIGMERGE_ID, "Copying /etc changes: %u modified, %u removed, %u added", modified->len, removed->len, @@ -773,7 +773,7 @@ selinux_relabel_var_if_needed (OstreeSysroot *sysroot, if (!g_file_query_exists (deployment_var_labeled, NULL)) { - gs_log_structured_print_id_v (OSTREE_VARRELABEL_ID, + ot_log_structured_print_id_v (OSTREE_VARRELABEL_ID, "Relabeling /var (no stamp file '%s' found)", gs_file_get_path_cached (deployment_var_labeled)); @@ -1927,7 +1927,7 @@ ostree_sysroot_write_deployments (OstreeSysroot *self, } } - gs_log_structured_print_id_v (OSTREE_DEPLOYMENT_COMPLETE_ID, + ot_log_structured_print_id_v (OSTREE_DEPLOYMENT_COMPLETE_ID, "%s; bootconfig swap: %s deployment count change: %i", (bootloader_is_atomic ? "Transaction complete" : "Bootloader updated"), requires_new_bootversion ? "yes" : "no", diff --git a/src/libotutil/ot-log-utils.c b/src/libotutil/ot-log-utils.c new file mode 100644 index 00000000..e1ce1690 --- /dev/null +++ b/src/libotutil/ot-log-utils.c @@ -0,0 +1,163 @@ +/* -*- mode: C; c-file-style: "gnu"; indent-tabs-mode: nil; -*- + * + * Copyright (C) 2016 Colin Walters + * + * 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. + * + * Author: Colin Walters + */ + +#include "config.h" + +#include + +#include + +#include "otutil.h" +#include "libglnx.h" + +#ifdef HAVE_LIBSYSTEMD +#define SD_JOURNAL_SUPPRESS_LOCATION +#include +#endif +#include + +/** + * ot_log_structured: + * @message: Text message to send + * @keys: (allow-none) (array zero-terminated=1) (element-type utf8): Optional structured data + * + * Log structured data in an operating-system specific fashion. The + * parameter @opts should be an array of UTF-8 KEY=VALUE strings. + * This function does not support binary data. See + * http://www.freedesktop.org/software/systemd/man/systemd.journal-fields.html + * for more information about fields that can be used on a systemd + * system. + */ +static void +ot_log_structured (const char *message, + const char *const *keys) +{ +#ifdef HAVE_LIBSYSTEMD + const char *const*iter; + g_autofree char *msgkey = NULL; + guint i, n_opts; + struct iovec *iovs; + + for (n_opts = 0, iter = keys; *iter; iter++, n_opts++) + ; + + n_opts++; /* Add one for MESSAGE= */ + iovs = g_alloca (sizeof (struct iovec) * n_opts); + + for (i = 0, iter = keys; *iter; iter++, i++) { + iovs[i].iov_base = (char*)keys[i]; + iovs[i].iov_len = strlen (keys[i]); + } + g_assert(i == n_opts-1); + msgkey = g_strconcat ("MESSAGE=", message, NULL); + iovs[i].iov_base = msgkey; + iovs[i].iov_len = strlen (msgkey); + + // The code location isn't useful since we're wrapping + sd_journal_sendv (iovs, n_opts); +#else + g_print ("%s\n", message); +#endif +} + +/** + * ot_stdout_is_journal: + * + * Use this function when you want your code to behave differently + * depeneding on whether your program was started as a systemd unit, + * or e.g. interactively at a terminal. + * + * Returns: %TRUE if stdout is (probably) connnected to the systemd journal + */ +static gboolean +ot_stdout_is_journal (void) +{ + static gsize initialized; + static gboolean stdout_is_socket; + + if (g_once_init_enter (&initialized)) + { + guint64 pid = (guint64) getpid (); + g_autofree char *fdpath = g_strdup_printf ("/proc/%" G_GUINT64_FORMAT "/fd/1", pid); + char buf[1024]; + ssize_t bytes_read; + + if ((bytes_read = readlink (fdpath, buf, sizeof(buf) - 1)) != -1) + { + buf[bytes_read] = '\0'; + stdout_is_socket = g_str_has_prefix (buf, "socket:"); + } + else + stdout_is_socket = FALSE; + + g_once_init_leave (&initialized, TRUE); + } + + return stdout_is_socket; +} + +/** + * gs_log_structured_print: + * @message: A message to log + * @keys: (allow-none) (array zero-terminated=1) (element-type utf8): Optional structured data + * + * Like gs_log_structured(), but also print to standard output (if it + * is not already connected to the system log). + */ +static void +ot_log_structured_print (const char *message, + const char *const *keys) +{ + ot_log_structured (message, keys); + +#ifdef HAVE_LIBSYSTEMD + if (!ot_stdout_is_journal ()) + g_print ("%s\n", message); +#endif +} + +/** + * ot_log_structured_print_id_v: + * @message_id: A unique MESSAGE_ID + * @format: A format string + * + * The provided @message_id is a unique MESSAGE_ID (see for more information). + * + * This function otherwise acts as ot_log_structured_print(), taking + * @format as a format string. + */ +void +ot_log_structured_print_id_v (const char *message_id, + const char *format, + ...) +{ + const char *key0 = glnx_strjoina ("MESSAGE_ID=", message_id); + const char *keys[] = { key0, NULL }; + g_autofree char *msg = NULL; + va_list args; + + va_start (args, format); + msg = g_strdup_vprintf (format, args); + va_end (args); + + ot_log_structured_print (msg, (const char *const *)keys); +} diff --git a/src/libotutil/ot-log-utils.h b/src/libotutil/ot-log-utils.h new file mode 100644 index 00000000..5e0502ab --- /dev/null +++ b/src/libotutil/ot-log-utils.h @@ -0,0 +1,31 @@ +/* -*- mode: C; c-file-style: "gnu"; indent-tabs-mode: nil; -*- + * + * Copyright (C) 2016 Colin Walters . + * + * 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 "ot-unix-utils.h" + +G_BEGIN_DECLS + +void ot_log_structured_print_id_v (const char *message_id, + const char *format, + ...); + +G_END_DECLS diff --git a/src/libotutil/otutil.h b/src/libotutil/otutil.h index 754f9253..6790d8ff 100644 --- a/src/libotutil/otutil.h +++ b/src/libotutil/otutil.h @@ -47,5 +47,6 @@ #include #include #include +#include void ot_ptrarray_add_many (GPtrArray *a, ...) G_GNUC_NULL_TERMINATED; From 90b9a06277c2d91ee331aae225031e5d6a1d4553 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 14 Jun 2016 21:51:10 -0400 Subject: [PATCH 04/39] lib: Use g_file_enumerator_iterate() if available, with fallback Import `gs_file_enumerator_iterate()` for the next six months or so...after RHEL 7.3 is released I'm strongly considering hard requiring 2.46 or so. Likely at some point we should figure out how to share more "glib backport" code with NetworkManager at least. Closes: #341 Approved by: jlebon --- src/libostree/ostree-bootloader-grub2.c | 4 +- src/libostree/ostree-gpg-verifier.c | 4 +- src/libostree/ostree-repo-checkout.c | 4 +- src/libostree/ostree-repo-commit.c | 4 +- src/libostree/ostree-repo-libarchive.c | 4 +- src/libostree/ostree-repo-static-delta-core.c | 8 +-- src/libostree/ostree-repo.c | 4 +- src/libostree/ostree-sysroot-cleanup.c | 12 ++--- src/libotutil/ot-gio-utils.c | 52 +++++++++++++++++++ src/libotutil/ot-gio-utils.h | 22 ++++++++ ...-instutil-builtin-selinux-ensure-labeled.c | 4 +- 11 files changed, 98 insertions(+), 24 deletions(-) diff --git a/src/libostree/ostree-bootloader-grub2.c b/src/libostree/ostree-bootloader-grub2.c index c970e662..6aa551ae 100644 --- a/src/libostree/ostree-bootloader-grub2.c +++ b/src/libostree/ostree-bootloader-grub2.c @@ -83,8 +83,8 @@ _ostree_bootloader_grub2_query (OstreeBootloader *bootloader, const char *fname; g_autofree char *subdir_grub_cfg = NULL; - if (!gs_file_enumerator_iterate (direnum, &file_info, NULL, - cancellable, error)) + if (!g_file_enumerator_iterate (direnum, &file_info, NULL, + cancellable, error)) goto out; if (file_info == NULL) break; diff --git a/src/libostree/ostree-gpg-verifier.c b/src/libostree/ostree-gpg-verifier.c index b9a03a11..0ec0b515 100644 --- a/src/libostree/ostree-gpg-verifier.c +++ b/src/libostree/ostree-gpg-verifier.c @@ -253,8 +253,8 @@ _ostree_gpg_verifier_add_keyring_dir (OstreeGpgVerifier *self, GFile *path; const char *name; - if (!gs_file_enumerator_iterate (enumerator, &file_info, &path, - cancellable, error)) + if (!g_file_enumerator_iterate (enumerator, &file_info, &path, + cancellable, error)) goto out; if (file_info == NULL) break; diff --git a/src/libostree/ostree-repo-checkout.c b/src/libostree/ostree-repo-checkout.c index 5f7cf0bd..59c8517a 100644 --- a/src/libostree/ostree-repo-checkout.c +++ b/src/libostree/ostree-repo-checkout.c @@ -696,8 +696,8 @@ checkout_tree_at (OstreeRepo *self, GFile *src_child; const char *name; - if (!gs_file_enumerator_iterate (dir_enum, &file_info, &src_child, - cancellable, error)) + if (!g_file_enumerator_iterate (dir_enum, &file_info, &src_child, + cancellable, error)) goto out; if (file_info == NULL) break; diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index cec19e4a..0d45dd47 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -2659,8 +2659,8 @@ write_directory_to_mtree_internal (OstreeRepo *self, { GFileInfo *child_info; - if (!gs_file_enumerator_iterate (dir_enum, &child_info, NULL, - cancellable, error)) + if (!g_file_enumerator_iterate (dir_enum, &child_info, NULL, + cancellable, error)) goto out; if (child_info == NULL) break; diff --git a/src/libostree/ostree-repo-libarchive.c b/src/libostree/ostree-repo-libarchive.c index 45427ef7..cfecd6d9 100644 --- a/src/libostree/ostree-repo-libarchive.c +++ b/src/libostree/ostree-repo-libarchive.c @@ -1075,8 +1075,8 @@ write_directory_to_libarchive_recurse (OstreeRepo *self, GFileInfo *file_info; GFile *path; - if (!gs_file_enumerator_iterate (dir_enum, &file_info, &path, - cancellable, error)) + if (!g_file_enumerator_iterate (dir_enum, &file_info, &path, + cancellable, error)) goto out; if (file_info == NULL) break; diff --git a/src/libostree/ostree-repo-static-delta-core.c b/src/libostree/ostree-repo-static-delta-core.c index de9e6c7a..d4f8a573 100644 --- a/src/libostree/ostree-repo-static-delta-core.c +++ b/src/libostree/ostree-repo-static-delta-core.c @@ -94,8 +94,8 @@ ostree_repo_list_static_delta_names (OstreeRepo *self, GFileInfo *file_info; GFile *child; - if (!gs_file_enumerator_iterate (dir_enum, &file_info, &child, - NULL, error)) + if (!g_file_enumerator_iterate (dir_enum, &file_info, &child, + NULL, error)) goto out; if (file_info == NULL) break; @@ -117,8 +117,8 @@ ostree_repo_list_static_delta_names (OstreeRepo *self, const char *name1; const char *name2; - if (!gs_file_enumerator_iterate (dir_enum2, &file_info2, &child2, - NULL, error)) + if (!g_file_enumerator_iterate (dir_enum2, &file_info2, &child2, + NULL, error)) goto out; if (file_info2 == NULL) break; diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 0b918ed8..81fc719f 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -2070,8 +2070,8 @@ append_remotes_d (OstreeRepo *self, const char *name; guint32 type; - if (!gs_file_enumerator_iterate (direnum, &file_info, &path, - NULL, error)) + if (!g_file_enumerator_iterate (direnum, &file_info, &path, + NULL, error)) goto out; if (file_info == NULL) break; diff --git a/src/libostree/ostree-sysroot-cleanup.c b/src/libostree/ostree-sysroot-cleanup.c index 64c1389e..862d8154 100644 --- a/src/libostree/ostree-sysroot-cleanup.c +++ b/src/libostree/ostree-sysroot-cleanup.c @@ -65,8 +65,8 @@ _ostree_sysroot_list_deployment_dirs_for_os (GFile *osdir, g_autofree char *csum = NULL; gint deployserial; - if (!gs_file_enumerator_iterate (dir_enum, &file_info, &child, - cancellable, error)) + if (!g_file_enumerator_iterate (dir_enum, &file_info, &child, + cancellable, error)) goto out; if (file_info == NULL) break; @@ -127,8 +127,8 @@ list_all_deployment_directories (OstreeSysroot *self, GFileInfo *file_info = NULL; GFile *child = NULL; - if (!gs_file_enumerator_iterate (dir_enum, &file_info, &child, - NULL, error)) + if (!g_file_enumerator_iterate (dir_enum, &file_info, &child, + NULL, error)) goto out; if (file_info == NULL) break; @@ -215,8 +215,8 @@ list_all_boot_directories (OstreeSysroot *self, GFile *child = NULL; const char *name; - if (!gs_file_enumerator_iterate (dir_enum, &file_info, &child, - NULL, error)) + if (!g_file_enumerator_iterate (dir_enum, &file_info, &child, + NULL, error)) goto out; if (file_info == NULL) break; diff --git a/src/libotutil/ot-gio-utils.c b/src/libotutil/ot-gio-utils.c index eb9b94f5..4ff2feda 100644 --- a/src/libotutil/ot-gio-utils.c +++ b/src/libotutil/ot-gio-utils.c @@ -493,3 +493,55 @@ ot_util_ensure_directory_and_fsync (GFile *dir, out: return ret; } + +#if !GLIB_CHECK_VERSION(2, 44, 0) + +gboolean +ot_file_enumerator_iterate (GFileEnumerator *direnum, + GFileInfo **out_info, + GFile **out_child, + GCancellable *cancellable, + GError **error) +{ + gboolean ret = FALSE; + GError *temp_error = NULL; + + static GQuark cached_info_quark; + static GQuark cached_child_quark; + static gsize quarks_initialized; + + g_return_val_if_fail (direnum != NULL, FALSE); + g_return_val_if_fail (out_info != NULL, FALSE); + + if (g_once_init_enter (&quarks_initialized)) + { + cached_info_quark = g_quark_from_static_string ("ot-cached-info"); + cached_child_quark = g_quark_from_static_string ("ot-cached-child"); + g_once_init_leave (&quarks_initialized, 1); + } + + *out_info = g_file_enumerator_next_file (direnum, cancellable, &temp_error); + if (out_child) + *out_child = NULL; + if (temp_error != NULL) + { + g_propagate_error (error, temp_error); + goto out; + } + else if (*out_info != NULL) + { + g_object_set_qdata_full ((GObject*)direnum, cached_info_quark, *out_info, (GDestroyNotify)g_object_unref); + if (out_child != NULL) + { + const char *name = g_file_info_get_name (*out_info); + *out_child = g_file_get_child (g_file_enumerator_get_container (direnum), name); + g_object_set_qdata_full ((GObject*)direnum, cached_child_quark, *out_child, (GDestroyNotify)g_object_unref); + } + } + + ret = TRUE; + out: + return ret; +} + +#endif diff --git a/src/libotutil/ot-gio-utils.h b/src/libotutil/ot-gio-utils.h index 355073af..030acac6 100644 --- a/src/libotutil/ot-gio-utils.h +++ b/src/libotutil/ot-gio-utils.h @@ -93,4 +93,26 @@ gboolean ot_util_fsync_directory (GFile *dir, GCancellable *cancellable, GError **error); +#if !GLIB_CHECK_VERSION(2, 44, 0) +gboolean +ot_file_enumerator_iterate (GFileEnumerator *direnum, + GFileInfo **out_info, + GFile **out_child, + GCancellable *cancellable, + GError **error); +#else +static inline gboolean +ot_file_enumerator_iterate (GFileEnumerator *direnum, + GFileInfo **out_info, + GFile **out_child, + GCancellable *cancellable, + GError **error) +{ + G_GNUC_BEGIN_IGNORE_DEPRECATIONS; + return (g_file_enumerator_iterate) (direnum, out_info, out_child, cancellable, error); + G_GNUC_END_IGNORE_DEPRECATIONS; +} +#endif +#define g_file_enumerator_iterate ot_file_enumerator_iterate + G_END_DECLS diff --git a/src/ostree/ot-admin-instutil-builtin-selinux-ensure-labeled.c b/src/ostree/ot-admin-instutil-builtin-selinux-ensure-labeled.c index 776ab54f..692a2623 100644 --- a/src/ostree/ot-admin-instutil-builtin-selinux-ensure-labeled.c +++ b/src/ostree/ot-admin-instutil-builtin-selinux-ensure-labeled.c @@ -114,8 +114,8 @@ relabel_recursively (OstreeSePolicy *sepolicy, GFile *child; GFileType ftype; - if (!gs_file_enumerator_iterate (direnum, &file_info, &child, - cancellable, error)) + if (!g_file_enumerator_iterate (direnum, &file_info, &child, + cancellable, error)) goto out; if (file_info == NULL) break; From f6ce04e4807c2934a5aba5522201998c815c7233 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 14 Jun 2016 21:55:40 -0400 Subject: [PATCH 05/39] libglnx porting: Drop uses of gs_file_openat_noatime We're not really doing the "noatime" thing anymore. Closes: #341 Approved by: jlebon --- src/libostree/ostree-repo.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 81fc719f..843a6489 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -2837,9 +2837,12 @@ ostree_repo_load_file (OstreeRepo *self, */ if (S_ISLNK (mode) || out_input) { - if (!gs_file_openat_noatime (self->objects_dir_fd, loose_path_buf, &fd, - cancellable, error)) - goto out; + fd = openat (self->objects_dir_fd, loose_path_buf, O_RDONLY | O_CLOEXEC); + if (fd < 0) + { + glnx_set_error_from_errno (error); + goto out; + } } if (S_ISREG (mode) && out_input) @@ -2876,9 +2879,12 @@ ostree_repo_load_file (OstreeRepo *self, { glnx_fd_close int fd = -1; - if (!gs_file_openat_noatime (self->objects_dir_fd, loose_path_buf, &fd, - cancellable, error)) - goto out; + fd = openat (self->objects_dir_fd, loose_path_buf, O_RDONLY | O_CLOEXEC); + if (fd < 0) + { + glnx_set_error_from_errno (error); + goto out; + } if (out_xattrs) { From 744543110e3d12464a752d59956dc24a0f99b1d2 Mon Sep 17 00:00:00 2001 From: Yu Qi Zhang Date: Mon, 20 Jun 2016 20:20:58 +0000 Subject: [PATCH 06/39] refs: allow overwrite of empty folders We noticed that once a ref folder is created, there is no existing command that can remove it. For example, once "foo/bar" is created, even if the user deletes foo or all the refs under foo, the folder will persist. Now when the user attempts to create a ref "foo" either through commit or refs --create, if a folder "foo" exists but is empty of refs, the folder is removed and the new ref "foo" is created. New unit tests in tests-ref.sh verify this functionality. Closes: #354 Approved by: cgwalters --- src/libostree/ostree-repo-refs.c | 41 ++++++++++++++++++++++++++++++-- src/ostree/ot-builtin-commit.c | 10 +++++++- src/ostree/ot-builtin-refs.c | 10 +++++++- tests/test-refs.sh | 10 +++++++- 4 files changed, 66 insertions(+), 5 deletions(-) diff --git a/src/libostree/ostree-repo-refs.c b/src/libostree/ostree-repo-refs.c index 29f03337..d9af2256 100644 --- a/src/libostree/ostree-repo-refs.c +++ b/src/libostree/ostree-repo-refs.c @@ -100,14 +100,51 @@ write_checksum_file_at (OstreeRepo *self, { size_t l = strlen (sha256); char *bufnl = alloca (l + 2); + g_autoptr(GError) temp_error = NULL; memcpy (bufnl, sha256, l); bufnl[l] = '\n'; bufnl[l+1] = '\0'; if (!_ostree_repo_file_replace_contents (self, dfd, name, (guint8*)bufnl, l + 1, - cancellable, error)) - goto out; + cancellable, &temp_error)) + { + if (g_error_matches (temp_error, G_IO_ERROR, G_IO_ERROR_IS_DIRECTORY)) + { + g_autoptr(GHashTable) refs = NULL; + GHashTableIter hashiter; + gpointer hashkey, hashvalue; + + g_clear_error (&temp_error); + + if (!ostree_repo_list_refs (self, name, &refs, cancellable, error)) + goto out; + + g_hash_table_iter_init (&hashiter, refs); + + while ((g_hash_table_iter_next (&hashiter, &hashkey, &hashvalue))) + { + if (strcmp (name, (char *)hashkey) != 0) + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, + "Conflict: %s exists under %s when attempting write", (char*)hashkey, name); + goto out; + } + } + + if (!glnx_shutil_rm_rf_at (dfd, name, cancellable, error)) + goto out; + + if (!_ostree_repo_file_replace_contents (self, dfd, name, (guint8*)bufnl, l + 1, + cancellable, error)) + goto out; + } + else + { + g_propagate_error (error, g_steal_pointer (&temp_error)); + goto out; + } + } } ret = TRUE; diff --git a/src/ostree/ot-builtin-commit.c b/src/ostree/ot-builtin-commit.c index 85c4c65d..ec3ca960 100644 --- a/src/ostree/ot-builtin-commit.c +++ b/src/ostree/ot-builtin-commit.c @@ -428,7 +428,15 @@ ostree_builtin_commit (int argc, char **argv, GCancellable *cancellable, GError else if (!opt_orphan) { if (!ostree_repo_resolve_rev (repo, opt_branch, TRUE, &parent, error)) - goto out; + { + if (g_error_matches (*error, G_IO_ERROR, G_IO_ERROR_IS_DIRECTORY)) + { + /* A folder exists with the specified ref name, + * which is handled by _ostree_repo_write_ref */ + g_clear_error (error); + } + else goto out; + } } if (opt_editor) diff --git a/src/ostree/ot-builtin-refs.c b/src/ostree/ot-builtin-refs.c index 10647ec6..fbdaf1bc 100644 --- a/src/ostree/ot-builtin-refs.c +++ b/src/ostree/ot-builtin-refs.c @@ -74,7 +74,15 @@ static gboolean do_ref (OstreeRepo *repo, const char *refspec_prefix, GCancellab g_autofree char *checksum_existing = NULL; if (!ostree_repo_resolve_rev (repo, opt_create, TRUE, &checksum_existing, error)) - goto out; + { + if (g_error_matches (*error, G_IO_ERROR, G_IO_ERROR_IS_DIRECTORY)) + { + /* A folder exists with the specified ref name, + * which is handled by _ostree_repo_write_ref */ + g_clear_error (error); + } + else goto out; + } if (checksum_existing != NULL) { diff --git a/tests/test-refs.sh b/tests/test-refs.sh index 3d229031..310b586f 100755 --- a/tests/test-refs.sh +++ b/tests/test-refs.sh @@ -92,8 +92,16 @@ fi ${CMD_PREFIX} ostree --repo=repo refs | wc -l > refscount.create1 assert_file_has_content refscount.create1 "^5$" -${CMD_PREFIX} ostree --repo=repo refs ctest --create ctest-new +${CMD_PREFIX} ostree --repo=repo refs ctest --create=ctest-new ${CMD_PREFIX} ostree --repo=repo refs | wc -l > refscount.create2 assert_file_has_content refscount.create2 "^6$" +#Check to see if a deleted folder can be re-used as the name of a ref +${CMD_PREFIX} ostree --repo=repo refs foo/ctest --delete +${CMD_PREFIX} ostree --repo=repo refs | wc -l > refscount.create3 +assert_file_has_content refscount.create3 "^5$" +${CMD_PREFIX} ostree --repo=repo refs ctest --create=foo +${CMD_PREFIX} ostree --repo=repo refs | wc -l > refscount.create4 +assert_file_has_content refscount.create4 "^6$" + echo "ok refs" From 91ccaff197ddefebe4965108ed19735bcbb6bfd3 Mon Sep 17 00:00:00 2001 From: Krzesimir Nowak Date: Wed, 22 Jun 2016 11:16:21 +0200 Subject: [PATCH 07/39] core: Fix wrong return value docs ostree_commit_get_parent() returns a string form of a checksum, not a binary form. Closes: #360 Approved by: cgwalters --- src/libostree/ostree-core.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libostree/ostree-core.c b/src/libostree/ostree-core.c index 8e8424fc..1a3740f8 100644 --- a/src/libostree/ostree-core.c +++ b/src/libostree/ostree-core.c @@ -1993,8 +1993,9 @@ ostree_validate_structureof_dirmeta (GVariant *dirmeta, /** * ostree_commit_get_parent: * @commit_variant: Variant of type %OSTREE_OBJECT_TYPE_COMMIT - * - * Returns: Binary checksum with parent of @commit_variant, or %NULL if none + * + * Returns: Checksum of the parent commit of @commit_variant, or %NULL + * if none */ gchar * ostree_commit_get_parent (GVariant *commit_variant) From 4cb77c51db5096eea19b82c30f7bb09434a620e0 Mon Sep 17 00:00:00 2001 From: Mathnerd314 Date: Sat, 18 Jun 2016 10:31:19 -0600 Subject: [PATCH 08/39] core: Use OSTREE_SHA256_STRING_LEN instead of 64 Closes: #359 Approved by: cgwalters --- src/libostree/ostree-core.c | 8 ++++---- src/libostree/ostree-core.h | 12 ++++++++++++ src/libostree/ostree-repo-checkout.c | 2 +- src/libostree/ostree-repo-commit.c | 2 +- src/libostree/ostree-repo-file.c | 12 ++++++------ src/libostree/ostree-repo-private.h | 2 +- src/libostree/ostree-repo-pull.c | 2 +- src/libostree/ostree-repo-refs.c | 6 +++--- src/libostree/ostree-repo-static-delta-core.c | 6 +++--- src/libostree/ostree-repo-static-delta-processing.c | 4 ++-- src/libostree/ostree-repo-traverse.c | 4 ++-- src/libostree/ostree-repo.c | 2 +- src/libostree/ostree-repo.h | 2 +- src/ostree/ot-dump.c | 2 +- 14 files changed, 39 insertions(+), 27 deletions(-) diff --git a/src/libostree/ostree-core.c b/src/libostree/ostree-core.c index 1a3740f8..4e3816f5 100644 --- a/src/libostree/ostree-core.c +++ b/src/libostree/ostree-core.c @@ -1269,7 +1269,7 @@ ostree_checksum_to_bytes_v (const char *checksum) /** * ostree_checksum_inplace_from_bytes: (skip) * @csum: (array fixed-size=32): An binary checksum of length 32 - * @buf: Output location, must be at least 65 bytes in length + * @buf: Output location, must be at least OSTREE_SHA256_STRING_LEN+1 bytes in length * * Overwrite the contents of @buf with stringified version of @csum. */ @@ -1340,7 +1340,7 @@ ostree_checksum_b64_inplace_from_bytes (const guchar *csum, char * ostree_checksum_from_bytes (const guchar *csum) { - char *ret = g_malloc (65); + char *ret = g_malloc (OSTREE_SHA256_STRING_LEN+1); ostree_checksum_inplace_from_bytes (csum, ret); return ret; } @@ -1484,7 +1484,7 @@ _ostree_get_relative_object_path (const char *checksum, { GString *path; - g_assert (strlen (checksum) == 64); + g_assert (strlen (checksum) == OSTREE_SHA256_STRING_LEN); path = g_string_new ("objects/"); @@ -1759,7 +1759,7 @@ ostree_validate_structureof_checksum_string (const char *checksum, int i = 0; size_t len = strlen (checksum); - if (len != 64) + if (len != OSTREE_SHA256_STRING_LEN) { g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, "Invalid rev '%s'", checksum); diff --git a/src/libostree/ostree-core.h b/src/libostree/ostree-core.h index 4a0e60e9..b8675672 100644 --- a/src/libostree/ostree-core.h +++ b/src/libostree/ostree-core.h @@ -51,8 +51,20 @@ G_BEGIN_DECLS */ #define OSTREE_MAX_RECURSION (256) +/** + * OSTREE_SHA256_DIGEST_LEN: + * + * Length of a sha256 digest when expressed as raw bytes + */ #define OSTREE_SHA256_DIGEST_LEN (32) +/** + * OSTREE_SHA256_STRING_LEN: + * + * Length of a sha256 digest when expressed as a hexadecimal string + */ +#define OSTREE_SHA256_STRING_LEN (64) + /** * OstreeObjectType: * @OSTREE_OBJECT_TYPE_FILE: Content; regular file, symbolic link diff --git a/src/libostree/ostree-repo-checkout.c b/src/libostree/ostree-repo-checkout.c index 59c8517a..a6d69c42 100644 --- a/src/libostree/ostree-repo-checkout.c +++ b/src/libostree/ostree-repo-checkout.c @@ -480,7 +480,7 @@ checkout_one_file_at (OstreeRepo *repo, key = g_new (OstreeDevIno, 1); key->dev = stbuf.st_dev; key->ino = stbuf.st_ino; - memcpy (key->checksum, checksum, 65); + memcpy (key->checksum, checksum, OSTREE_SHA256_STRING_LEN+1); g_hash_table_add ((GHashTable*)options->devino_to_csum_cache, key); } diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index 0d45dd47..3670dea7 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -2937,7 +2937,7 @@ ostree_repo_write_mtree (OstreeRepo *self, g_autoptr(GHashTable) dir_contents_checksums = NULL; g_autoptr(GVariant) serialized_tree = NULL; g_autofree guchar *contents_csum = NULL; - char contents_checksum_buf[65]; + char contents_checksum_buf[OSTREE_SHA256_STRING_LEN+1]; dir_contents_checksums = g_hash_table_new_full (g_str_hash, g_str_equal, (GDestroyNotify)g_free, (GDestroyNotify)g_free); diff --git a/src/libostree/ostree-repo-file.c b/src/libostree/ostree-repo-file.c index 396820da..a88e5363 100644 --- a/src/libostree/ostree-repo-file.c +++ b/src/libostree/ostree-repo-file.c @@ -113,9 +113,9 @@ _ostree_repo_file_new_root (OstreeRepo *repo, g_return_val_if_fail (repo != NULL, NULL); g_return_val_if_fail (contents_checksum != NULL, NULL); - g_return_val_if_fail (strlen (contents_checksum) == 64, NULL); + g_return_val_if_fail (strlen (contents_checksum) == OSTREE_SHA256_STRING_LEN, NULL); g_return_val_if_fail (metadata_checksum != NULL, NULL); - g_return_val_if_fail (strlen (metadata_checksum) == 64, NULL); + g_return_val_if_fail (strlen (metadata_checksum) == OSTREE_SHA256_STRING_LEN, NULL); self = g_object_new (OSTREE_TYPE_REPO_FILE, NULL); self->repo = g_object_ref (repo); @@ -152,12 +152,12 @@ _ostree_repo_file_new_for_commit (OstreeRepo *repo, g_autoptr(GVariant) commit_v = NULL; g_autoptr(GVariant) tree_contents_csum_v = NULL; g_autoptr(GVariant) tree_metadata_csum_v = NULL; - char tree_contents_csum[65]; - char tree_metadata_csum[65]; + char tree_contents_csum[OSTREE_SHA256_STRING_LEN + 1]; + char tree_metadata_csum[OSTREE_SHA256_STRING_LEN + 1]; g_return_val_if_fail (repo != NULL, NULL); g_return_val_if_fail (commit != NULL, NULL); - g_return_val_if_fail (strlen (commit) == 64, NULL); + g_return_val_if_fail (strlen (commit) == OSTREE_SHA256_STRING_LEN, NULL); if (!ostree_repo_load_variant (repo, OSTREE_OBJECT_TYPE_COMMIT, commit, &commit_v, error)) @@ -807,7 +807,7 @@ ostree_repo_file_tree_query_child (OstreeRepoFile *self, g_autoptr(GVariant) dirs_variant = NULL; g_autoptr(GVariant) content_csum_v = NULL; g_autoptr(GVariant) meta_csum_v = NULL; - char tmp_checksum[65]; + char tmp_checksum[OSTREE_SHA256_STRING_LEN+1]; GFileAttributeMatcher *matcher = NULL; if (!ostree_repo_file_ensure_resolved (self, error)) diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h index f330d169..52dbfdd5 100644 --- a/src/libostree/ostree-repo-private.h +++ b/src/libostree/ostree-repo-private.h @@ -120,7 +120,7 @@ struct OstreeRepo { typedef struct { dev_t dev; ino_t ino; - char checksum[65]; + char checksum[OSTREE_SHA256_STRING_LEN+1]; } OstreeDevIno; #define OSTREE_REPO_TMPDIR_STAGING "staging-" diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 53847340..672da831 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -1096,7 +1096,7 @@ scan_commit_object (OtPullData *pull_data, } else if (parent_csum_bytes != NULL && depth > 0) { - char parent_checksum[65]; + char parent_checksum[OSTREE_SHA256_STRING_LEN+1]; gpointer parent_depthp; int parent_depth; diff --git a/src/libostree/ostree-repo-refs.c b/src/libostree/ostree-repo-refs.c index d9af2256..1eb50e0c 100644 --- a/src/libostree/ostree-repo-refs.c +++ b/src/libostree/ostree-repo-refs.c @@ -341,10 +341,10 @@ ostree_repo_resolve_partial_checksum (OstreeRepo *self, g_return_val_if_fail (error == NULL || *error == NULL, FALSE); - /* If the input is longer than 64 chars or contains non-hex chars, + /* If the input is longer than OSTREE_SHA256_STRING_LEN chars or contains non-hex chars, don't bother looking for it as an object */ off = strspn (refspec, hexchars); - if (off > 64 || refspec[off] != '\0') + if (off > OSTREE_SHA256_STRING_LEN || refspec[off] != '\0') return TRUE; /* this looks through all objects and adds them to the ref_list if: @@ -740,7 +740,7 @@ ostree_repo_remote_list_refs (OstreeRepo *self, { const char *ref_name = NULL; g_autoptr(GVariant) csum_v = NULL; - char tmp_checksum[65]; + char tmp_checksum[OSTREE_SHA256_STRING_LEN+1]; g_variant_get_child (child, 0, "&s", &ref_name); diff --git a/src/libostree/ostree-repo-static-delta-core.c b/src/libostree/ostree-repo-static-delta-core.c index d4f8a573..6253a45f 100644 --- a/src/libostree/ostree-repo-static-delta-core.c +++ b/src/libostree/ostree-repo-static-delta-core.c @@ -136,7 +136,7 @@ ostree_repo_list_static_delta_names (OstreeRepo *self, { g_autofree char *buf = g_strconcat (name1, name2, NULL); GString *out = g_string_new (""); - char checksum[65]; + char checksum[OSTREE_SHA256_STRING_LEN+1]; guchar csum[OSTREE_SHA256_DIGEST_LEN]; const char *dash = strchr (buf, '-'); @@ -187,7 +187,7 @@ _ostree_repo_static_delta_part_have_all_objects (OstreeRepo *repo, { guint8 objtype = *checksums_data; const guint8 *csum = checksums_data + 1; - char tmp_checksum[65]; + char tmp_checksum[OSTREE_SHA256_STRING_LEN+1]; if (G_UNLIKELY(!ostree_validate_structureof_objtype (objtype, error))) goto out; @@ -354,7 +354,7 @@ ostree_repo_static_delta_execute_offline (OstreeRepo *self, guint64 size; guint64 usize; const guchar *csum; - char checksum[65]; + char checksum[OSTREE_SHA256_STRING_LEN+1]; gboolean have_all; g_autoptr(GInputStream) part_in = NULL; g_autoptr(GBytes) delta_data = NULL; diff --git a/src/libostree/ostree-repo-static-delta-processing.c b/src/libostree/ostree-repo-static-delta-processing.c index 681d426f..871ab7bd 100644 --- a/src/libostree/ostree-repo-static-delta-processing.c +++ b/src/libostree/ostree-repo-static-delta-processing.c @@ -60,7 +60,7 @@ typedef struct { OstreeRepoContentBareCommit barecommitstate; guint64 content_size; GOutputStream *content_out; - char checksum[65]; + char checksum[OSTREE_SHA256_STRING_LEN+1]; char *read_source_object; int read_source_fd; gboolean have_obj; @@ -78,7 +78,7 @@ typedef struct { typedef struct { StaticDeltaExecutionState *state; - char checksum[65]; + char checksum[OSTREE_SHA256_STRING_LEN+1]; } StaticDeltaContentWrite; typedef gboolean (*DispatchOpFunc) (OstreeRepo *repo, diff --git a/src/libostree/ostree-repo-traverse.c b/src/libostree/ostree-repo-traverse.c index 503ab329..4cc05e2e 100644 --- a/src/libostree/ostree-repo-traverse.c +++ b/src/libostree/ostree-repo-traverse.c @@ -34,8 +34,8 @@ struct _OstreeRepoRealCommitTraverseIter { const char *name; OstreeRepoCommitIterResult state; guint idx; - char checksum_content[65]; - char checksum_meta[65]; + char checksum_content[OSTREE_SHA256_STRING_LEN+1]; + char checksum_meta[OSTREE_SHA256_STRING_LEN+1]; }; /** diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 843a6489..c6520a80 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -2445,7 +2445,7 @@ list_loose_objects_at (OstreeRepo *self, const char *name = dent->d_name; const char *dot; OstreeObjectType objtype; - char buf[65]; + char buf[OSTREE_SHA256_STRING_LEN+1]; if (strcmp (name, ".") == 0 || strcmp (name, "..") == 0) diff --git a/src/libostree/ostree-repo.h b/src/libostree/ostree-repo.h index ce280ee6..c023b4d2 100644 --- a/src/libostree/ostree-repo.h +++ b/src/libostree/ostree-repo.h @@ -866,7 +866,7 @@ gboolean ostree_repo_traverse_commit_union (OstreeRepo *repo, struct _OstreeRepoCommitTraverseIter { gboolean initialized; gpointer dummy[10]; - char dummy_checksum_data[65*2]; + char dummy_checksum_data[(OSTREE_SHA256_STRING_LEN+1)*2]; }; typedef struct _OstreeRepoCommitTraverseIter OstreeRepoCommitTraverseIter; diff --git a/src/ostree/ot-dump.c b/src/ostree/ot-dump.c index 48b087af..4fc84cbd 100644 --- a/src/ostree/ot-dump.c +++ b/src/ostree/ot-dump.c @@ -181,7 +181,7 @@ dump_summary_ref (const char *ref_name, csum_bytes = ostree_checksum_bytes_peek_validate (csum_v, &csum_error); if (csum_error == NULL) { - char csum[65]; + char csum[OSTREE_SHA256_STRING_LEN+1]; ostree_checksum_inplace_from_bytes (csum_bytes, csum); g_print (" %s\n", csum); From 55f5f73d80e4b9b9ca0c0c2cece210afc2052bce Mon Sep 17 00:00:00 2001 From: Mathnerd314 Date: Thu, 16 Jun 2016 13:59:06 -0600 Subject: [PATCH 09/39] configure: Turn on -Wempty-body I spent half an hour debugging an extra semicolon, and this C "feature" is not used at all in ostree Closes: #359 Approved by: cgwalters --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index d28c410a..d88f24e5 100644 --- a/configure.ac +++ b/configure.ac @@ -17,7 +17,7 @@ AC_PROG_YACC changequote(,)dnl if test "x$GCC" = "xyes"; then - WARN_CFLAGS="-Wall -Wstrict-prototypes -Werror=missing-prototypes \ + WARN_CFLAGS="-Wall -Wempty-body -Wstrict-prototypes -Werror=missing-prototypes \ -Werror=implicit-function-declaration \ -Werror=pointer-arith -Werror=init-self -Werror=format=2 \ -Werror=format-security \ From 23049bbd011924b1e34514b14e58d82519346d36 Mon Sep 17 00:00:00 2001 From: Mathnerd314 Date: Wed, 15 Jun 2016 23:37:29 -0600 Subject: [PATCH 10/39] core: Add OSTREE_OBJECT_TYPE_COMMIT_META This is cleaner than the loose_path_with_suffix approach Closes: #359 Approved by: cgwalters --- src/libostree/ostree-core-private.h | 7 ----- src/libostree/ostree-core.c | 41 +++++++++-------------------- src/libostree/ostree-core.h | 6 +++-- src/libostree/ostree-repo-commit.c | 3 +-- src/libostree/ostree-repo-pull.c | 3 +-- src/libostree/ostree-repo.c | 3 +-- 6 files changed, 20 insertions(+), 43 deletions(-) diff --git a/src/libostree/ostree-core-private.h b/src/libostree/ostree-core-private.h index 91d52f1b..b49e5e47 100644 --- a/src/libostree/ostree-core-private.h +++ b/src/libostree/ostree-core-private.h @@ -128,13 +128,6 @@ _ostree_loose_path (char *buf, OstreeObjectType objtype, OstreeRepoMode repo_mode); -void -_ostree_loose_path_with_suffix (char *buf, - const char *checksum, - OstreeObjectType objtype, - OstreeRepoMode repo_mode, - const char *suffix); - #define _OSTREE_METADATA_GPGSIGS_NAME "ostree.gpgsigs" #define _OSTREE_METADATA_GPGSIGS_TYPE G_VARIANT_TYPE ("aay") diff --git a/src/libostree/ostree-core.c b/src/libostree/ostree-core.c index 4e3816f5..32f0fd44 100644 --- a/src/libostree/ostree-core.c +++ b/src/libostree/ostree-core.c @@ -1047,6 +1047,8 @@ ostree_object_type_to_string (OstreeObjectType objtype) return "commit"; case OSTREE_OBJECT_TYPE_TOMBSTONE_COMMIT: return "tombstone-commit"; + case OSTREE_OBJECT_TYPE_COMMIT_META: + return "commitmeta"; default: g_assert_not_reached (); return NULL; @@ -1070,6 +1072,10 @@ ostree_object_type_from_string (const char *str) return OSTREE_OBJECT_TYPE_DIR_META; else if (!strcmp (str, "commit")) return OSTREE_OBJECT_TYPE_COMMIT; + else if (!strcmp (str, "tombstone-commit")) + return OSTREE_OBJECT_TYPE_TOMBSTONE_COMMIT; + else if (!strcmp (str, "commitmeta")) + return OSTREE_OBJECT_TYPE_COMMIT_META; g_assert_not_reached (); return 0; } @@ -1414,7 +1420,13 @@ _ostree_loose_path (char *buf, OstreeObjectType objtype, OstreeRepoMode mode) { - _ostree_loose_path_with_suffix (buf, checksum, objtype, mode, ""); + *buf = checksum[0]; + buf++; + *buf = checksum[1]; + buf++; + snprintf (buf, _OSTREE_LOOSE_PATH_MAX - 2, "/%s.%s%s", + checksum + 2, ostree_object_type_to_string (objtype), + (!OSTREE_OBJECT_TYPE_IS_META (objtype) && mode == OSTREE_REPO_MODE_ARCHIVE_Z2) ? "z" : ""); } /** @@ -1442,33 +1454,6 @@ _ostree_header_gfile_info_new (mode_t mode, uid_t uid, gid_t gid) return ret; } -/* - * _ostree_loose_path_with_suffix: - * @buf: Output buffer, must be _OSTREE_LOOSE_PATH_MAX in size - * @checksum: ASCII checksum - * @objtype: Object type - * @mode: Repository mode - * - * Like _ostree_loose_path, but also append a further arbitrary - * suffix; useful for finding non-core objects. - */ -void -_ostree_loose_path_with_suffix (char *buf, - const char *checksum, - OstreeObjectType objtype, - OstreeRepoMode mode, - const char *suffix) -{ - *buf = checksum[0]; - buf++; - *buf = checksum[1]; - buf++; - snprintf (buf, _OSTREE_LOOSE_PATH_MAX - 2, "/%s.%s%s%s", - checksum + 2, ostree_object_type_to_string (objtype), - (!OSTREE_OBJECT_TYPE_IS_META (objtype) && mode == OSTREE_REPO_MODE_ARCHIVE_Z2) ? "z" : "", - suffix); -} - /* * _ostree_get_relative_object_path: * @checksum: ASCII checksum string diff --git a/src/libostree/ostree-core.h b/src/libostree/ostree-core.h index b8675672..415369d5 100644 --- a/src/libostree/ostree-core.h +++ b/src/libostree/ostree-core.h @@ -72,6 +72,7 @@ G_BEGIN_DECLS * @OSTREE_OBJECT_TYPE_DIR_META: Directory metadata * @OSTREE_OBJECT_TYPE_COMMIT: Toplevel object, refers to tree and dirmeta for root * @OSTREE_OBJECT_TYPE_TOMBSTONE_COMMIT: Toplevel object, refers to a deleted commit + * @OSTREE_OBJECT_TYPE_COMMIT_META: Detached metadata for a commit * * Enumeration for core object types; %OSTREE_OBJECT_TYPE_FILE is for * content, the other types are metadata. @@ -82,6 +83,7 @@ typedef enum { OSTREE_OBJECT_TYPE_DIR_META = 3, /* .dirmeta */ OSTREE_OBJECT_TYPE_COMMIT = 4, /* .commit */ OSTREE_OBJECT_TYPE_TOMBSTONE_COMMIT = 5, /* .commit-tombstone */ + OSTREE_OBJECT_TYPE_COMMIT_META = 6, /* .commitmeta */ } OstreeObjectType; /** @@ -90,14 +92,14 @@ typedef enum { * * Returns: %TRUE if object type is metadata */ -#define OSTREE_OBJECT_TYPE_IS_META(t) (t >= 2 && t <= 5) +#define OSTREE_OBJECT_TYPE_IS_META(t) (t >= 2 && t <= 6) /** * OSTREE_OBJECT_TYPE_LAST: * * Last valid object type; use this to validate ranges. */ -#define OSTREE_OBJECT_TYPE_LAST OSTREE_OBJECT_TYPE_TOMBSTONE_COMMIT +#define OSTREE_OBJECT_TYPE_LAST OSTREE_OBJECT_TYPE_COMMIT_META /** * OSTREE_DIRMETA_GVARIANT_FORMAT: diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index 3670dea7..662ee21e 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -2058,8 +2058,7 @@ _ostree_repo_get_commit_metadata_loose_path (OstreeRepo *self, const char *checksum) { char buf[_OSTREE_LOOSE_PATH_MAX]; - _ostree_loose_path_with_suffix (buf, checksum, OSTREE_OBJECT_TYPE_COMMIT, self->mode, - "meta"); + _ostree_loose_path (buf, checksum, OSTREE_OBJECT_TYPE_COMMIT_META, self->mode); return g_file_resolve_relative_path (self->objects_dir, buf); } diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 672da831..b4a565e3 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -1316,8 +1316,7 @@ enqueue_one_object_request (OtPullData *pull_data, if (is_detached_meta) { char buf[_OSTREE_LOOSE_PATH_MAX]; - _ostree_loose_path_with_suffix (buf, checksum, OSTREE_OBJECT_TYPE_COMMIT, - pull_data->remote_mode, "meta"); + _ostree_loose_path (buf, checksum, OSTREE_OBJECT_TYPE_COMMIT_META, pull_data->remote_mode); obj_uri = suburi_new (pull_data->base_uri, "objects", buf, NULL); } else diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index c6520a80..e30b48cf 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -3120,8 +3120,7 @@ ostree_repo_delete_object (OstreeRepo *self, { char meta_loose[_OSTREE_LOOSE_PATH_MAX]; - _ostree_loose_path_with_suffix (meta_loose, sha256, - OSTREE_OBJECT_TYPE_COMMIT, self->mode, "meta"); + _ostree_loose_path (meta_loose, sha256, OSTREE_OBJECT_TYPE_COMMIT_META, self->mode); do res = unlinkat (self->objects_dir_fd, meta_loose, 0); From 9a779563bbfcc35fc1a891b74003619fd4556104 Mon Sep 17 00:00:00 2001 From: Mathnerd314 Date: Sat, 18 Jun 2016 11:06:31 -0600 Subject: [PATCH 11/39] refs: Fix a logic error I encountered the Opening remotes/ dir error with some broken pull code, and this fixes it. Closes: #358 Approved by: cgwalters --- src/libostree/ostree-repo-refs.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/libostree/ostree-repo-refs.c b/src/libostree/ostree-repo-refs.c index 1eb50e0c..b18d838e 100644 --- a/src/libostree/ostree-repo-refs.c +++ b/src/libostree/ostree-repo-refs.c @@ -809,8 +809,10 @@ _ostree_repo_write_ref (OstreeRepo *self, goto out; } - if (!glnx_opendirat (refs_remotes_dfd, remote, TRUE, &dfd, error)) + dfd = glnx_opendirat_with_errno (refs_remotes_dfd, remote, TRUE); + if (dfd < 0 && (errno != ENOENT || rev != NULL)) { + glnx_set_error_from_errno (error); g_prefix_error (error, "Opening remotes/ dir %s: ", remote); goto out; } @@ -818,13 +820,16 @@ _ostree_repo_write_ref (OstreeRepo *self, if (rev == NULL) { - if (unlinkat (dfd, ref, 0) != 0) + if (dfd >= 0) { - if (errno != ENOENT) - { - glnx_set_error_from_errno (error); - goto out; - } + if (unlinkat (dfd, ref, 0) != 0) + { + if (errno != ENOENT) + { + glnx_set_error_from_errno (error); + goto out; + } + } } } else From fc4a7ec35e6e7a3a2db80257508faa592329786c Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Thu, 23 Jun 2016 11:51:15 +0200 Subject: [PATCH 12/39] pull: Correctly handle repo->parent_repo when applying static deltas In flatpak i was using a parent repo, and it failed to update with ENOENT when dispatching an set-read-source opcode, because the object it referenced was in the parent repo. This fixes that by making _ostree_repo_read_bare_fd look at parent_repo. Closes: #362 Approved by: cgwalters --- src/libostree/ostree-repo.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index e30b48cf..99dbb581 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -2715,18 +2715,29 @@ _ostree_repo_read_bare_fd (OstreeRepo *self, GError **error) { char loose_path_buf[_OSTREE_LOOSE_PATH_MAX]; - + g_assert (self->mode == OSTREE_REPO_MODE_BARE || self->mode == OSTREE_REPO_MODE_BARE_USER); _ostree_loose_path (loose_path_buf, checksum, OSTREE_OBJECT_TYPE_FILE, self->mode); - - *out_fd = openat (self->objects_dir_fd, loose_path_buf, O_RDONLY | O_CLOEXEC); - if (*out_fd < 0) + + if (!ot_openat_ignore_enoent (self->objects_dir_fd, loose_path_buf, out_fd, error)) + return FALSE; + + if (*out_fd == -1) { - glnx_set_error_from_errno (error); + if (self->parent_repo) + return _ostree_repo_read_bare_fd (self->parent_repo, + checksum, + out_fd, + cancellable, + error); + + g_set_error (error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND, + "No such file object %s", checksum); return FALSE; } + return TRUE; } From 02a2b689dda4fe69491d526a8e1f4f12f5a32bdf Mon Sep 17 00:00:00 2001 From: Yu Qi Zhang Date: Thu, 23 Jun 2016 16:11:50 +0000 Subject: [PATCH 13/39] refs: resolve conflict between local/remote repos Add the functionality to use the same name for refs in local and remote repos. This helps users keep track of local refs of remote origin, much like local and remote git branches. Previously, when a local ref is specified, resolve_refspec would fall back to searching through remote repos if the ref is not found locally. This function now takes an extra flag to specify whether it should search through remote repos. Additionally, ostree_repo_resove_rev_ext was added to call resolve_refspec with fallback_remote being false, so refs --create would no longer complain when trying to create a local ref of the same name as a remote one. Fix remote repo parsing not being handled correctly on refs --create. Closes: #363 Approved by: jlebon --- apidoc/ostree-sections.txt | 1 + src/libostree/libostree.sym | 5 +- src/libostree/ostree-repo-refs.c | 84 +++++++++++++++++++++++--------- src/libostree/ostree-repo.h | 16 ++++++ src/ostree/ot-builtin-refs.c | 9 +++- tests/test-refs.sh | 12 +++++ 6 files changed, 99 insertions(+), 28 deletions(-) diff --git a/apidoc/ostree-sections.txt b/apidoc/ostree-sections.txt index 6dca9ecc..cc77f15a 100644 --- a/apidoc/ostree-sections.txt +++ b/apidoc/ostree-sections.txt @@ -300,6 +300,7 @@ ostree_repo_write_content_trusted ostree_repo_write_content_async ostree_repo_write_content_finish ostree_repo_resolve_rev +ostree_repo_resolve_rev_ext ostree_repo_list_refs ostree_repo_list_refs_ext ostree_repo_remote_list_refs diff --git a/src/libostree/libostree.sym b/src/libostree/libostree.sym index 087f9879..bdfa7c3f 100644 --- a/src/libostree/libostree.sym +++ b/src/libostree/libostree.sym @@ -346,10 +346,7 @@ global: * NOTE NOTE NOTE */ -/* Uncomment this when adding a new symbol */ -/* LIBOSTREE_2016.7 { global: - someostree_some_new_symbol; + ostree_repo_resolve_rev_ext; } LIBOSTREE_2016.6; -*/ diff --git a/src/libostree/ostree-repo-refs.c b/src/libostree/ostree-repo-refs.c index b18d838e..18308d0c 100644 --- a/src/libostree/ostree-repo-refs.c +++ b/src/libostree/ostree-repo-refs.c @@ -199,6 +199,7 @@ resolve_refspec (OstreeRepo *self, const char *remote, const char *ref, gboolean allow_noent, + gboolean fallback_remote, char **out_rev, GError **error); @@ -207,6 +208,7 @@ resolve_refspec_fallback (OstreeRepo *self, const char *remote, const char *ref, gboolean allow_noent, + gboolean fallback_remote, char **out_rev, GCancellable *cancellable, GError **error) @@ -216,8 +218,8 @@ resolve_refspec_fallback (OstreeRepo *self, if (self->parent_repo) { - if (!resolve_refspec (self->parent_repo, remote, ref, - allow_noent, &ret_rev, error)) + if (!resolve_refspec (self->parent_repo, remote, ref, allow_noent, + fallback_remote, &ret_rev, error)) goto out; } else if (!allow_noent) @@ -241,6 +243,7 @@ resolve_refspec (OstreeRepo *self, const char *remote, const char *ref, gboolean allow_noent, + gboolean fallback_remote, char **out_rev, GError **error) { @@ -270,7 +273,7 @@ resolve_refspec (OstreeRepo *self, if (!ot_openat_ignore_enoent (self->repo_dir_fd, local_ref, &target_fd, error)) goto out; - if (target_fd == -1) + if (target_fd == -1 && fallback_remote) { local_ref = glnx_strjoina ("refs/remotes/", ref); @@ -300,7 +303,7 @@ resolve_refspec (OstreeRepo *self, } else { - if (!resolve_refspec_fallback (self, remote, ref, allow_noent, + if (!resolve_refspec_fallback (self, remote, ref, allow_noent, fallback_remote, &ret_rev, cancellable, error)) goto out; } @@ -388,23 +391,13 @@ ostree_repo_resolve_partial_checksum (OstreeRepo *self, return ret; } -/** - * ostree_repo_resolve_rev: - * @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 - * @error: Error - * - * Look up the given refspec, returning the checksum it references in - * the parameter @out_rev. - */ -gboolean -ostree_repo_resolve_rev (OstreeRepo *self, - const char *refspec, - gboolean allow_noent, - char **out_rev, - GError **error) +static gboolean +_ostree_repo_resolve_rev_internal (OstreeRepo *self, + const char *refspec, + gboolean allow_noent, + gboolean fallback_remote, + char **out_rev, + GError **error) { gboolean ret = FALSE; g_autofree char *ret_rev = NULL; @@ -456,7 +449,7 @@ ostree_repo_resolve_rev (OstreeRepo *self, goto out; if (!resolve_refspec (self, remote, ref, allow_noent, - &ret_rev, error)) + fallback_remote, &ret_rev, error)) goto out; } } @@ -467,6 +460,53 @@ ostree_repo_resolve_rev (OstreeRepo *self, return ret; } +/** + * ostree_repo_resolve_rev: + * @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 + * @error: Error + * + * Look up the given refspec, returning the checksum it references in + * the parameter @out_rev. Will fall back on remote directory if cannot + * find the given refspec in local. + */ +gboolean +ostree_repo_resolve_rev (OstreeRepo *self, + const char *refspec, + gboolean allow_noent, + char **out_rev, + GError **error) +{ + return _ostree_repo_resolve_rev_internal (self, refspec, allow_noent, TRUE, out_rev, error); +} + +/** + * ostree_repo_resolve_rev_ext: + * @self: Repo + * @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 + * @error: Error + * + * Look up the given refspec, returning the checksum it references in + * the parameter @out_rev. Differently from ostree_repo_resolve_rev(), + * this will not fall back to searching through remote repos if a + * local ref is specified but not found. + */ +gboolean +ostree_repo_resolve_rev_ext (OstreeRepo *self, + const char *refspec, + gboolean allow_noent, + OstreeRepoResolveRevExtFlags flags, + char **out_rev, + GError **error) +{ + return _ostree_repo_resolve_rev_internal (self, refspec, allow_noent, FALSE, out_rev, error); +} + static gboolean enumerate_refs_recurse (OstreeRepo *repo, const char *remote, diff --git a/src/libostree/ostree-repo.h b/src/libostree/ostree-repo.h index c023b4d2..2f60e633 100644 --- a/src/libostree/ostree-repo.h +++ b/src/libostree/ostree-repo.h @@ -382,6 +382,22 @@ gboolean ostree_repo_resolve_rev (OstreeRepo *self, char **out_rev, GError **error); +/** + * OstreeRepoResolveRevExtFlags: + * @OSTREE_REPO_RESOLVE_REV_EXT_NONE: No flags. + */ +typedef enum { + OSTREE_REPO_RESOLVE_REV_EXT_NONE = 0, +} OstreeRepoResolveRevExtFlags; + +_OSTREE_PUBLIC +gboolean ostree_repo_resolve_rev_ext (OstreeRepo *self, + const char *refspec, + gboolean allow_noent, + OstreeRepoResolveRevExtFlags flags, + char **out_rev, + GError **error); + _OSTREE_PUBLIC gboolean ostree_repo_list_refs (OstreeRepo *self, const char *refspec_prefix, diff --git a/src/ostree/ot-builtin-refs.c b/src/ostree/ot-builtin-refs.c index fbdaf1bc..017b0c74 100644 --- a/src/ostree/ot-builtin-refs.c +++ b/src/ostree/ot-builtin-refs.c @@ -72,8 +72,10 @@ static gboolean do_ref (OstreeRepo *repo, const char *refspec_prefix, GCancellab { g_autofree char *checksum = NULL; g_autofree char *checksum_existing = NULL; + g_autofree char *remote = NULL; + g_autofree char *ref = NULL; - if (!ostree_repo_resolve_rev (repo, opt_create, TRUE, &checksum_existing, error)) + if (!ostree_repo_resolve_rev_ext (repo, opt_create, TRUE, OSTREE_REPO_RESOLVE_REV_EXT_NONE, &checksum_existing, error)) { if (g_error_matches (*error, G_IO_ERROR, G_IO_ERROR_IS_DIRECTORY)) { @@ -94,7 +96,10 @@ static gboolean do_ref (OstreeRepo *repo, const char *refspec_prefix, GCancellab if (!ostree_repo_resolve_rev (repo, refspec_prefix, FALSE, &checksum, error)) goto out; - if (!ostree_repo_set_ref_immediate (repo, NULL, opt_create, checksum, + if (!ostree_parse_refspec (opt_create, &remote, &ref, error)) + goto out; + + if (!ostree_repo_set_ref_immediate (repo, remote, ref, checksum, cancellable, error)) goto out; } diff --git a/tests/test-refs.sh b/tests/test-refs.sh index 310b586f..ee7841bd 100755 --- a/tests/test-refs.sh +++ b/tests/test-refs.sh @@ -104,4 +104,16 @@ ${CMD_PREFIX} ostree --repo=repo refs ctest --create=foo ${CMD_PREFIX} ostree --repo=repo refs | wc -l > refscount.create4 assert_file_has_content refscount.create4 "^6$" +#Check if a name for a ref in remote repo can be used locally, and vice versa. +${CMD_PREFIX} ostree --repo=repo commit --branch=origin:remote1 +${CMD_PREFIX} ostree --repo=repo commit --branch=local1 +${CMD_PREFIX} ostree --repo=repo refs | wc -l > refscount.create5 +assert_file_has_content refscount.create5 "^8$" + +${CMD_PREFIX} ostree --repo=repo refs origin:remote1 --create=remote1 +${CMD_PREFIX} ostree --repo=repo refs origin:remote1 --create=origin/remote1 +${CMD_PREFIX} ostree --repo=repo refs local1 --create=origin:local1 +${CMD_PREFIX} ostree --repo=repo refs | wc -l > refscount.create6 +assert_file_has_content refscount.create6 "^11$" + echo "ok refs" From 073c34ca0898dc417d5e101236684ce9533c2ed5 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 7 Jun 2016 09:50:58 -0400 Subject: [PATCH 14/39] pull: Write commitpartial files for local imports too Just like HTTP fetches, these can be interrupted, so we need to write the commitpartial files. Closes: #324 Approved by: yuqi-zhang --- src/libostree/ostree-repo-pull.c | 51 +++++++++++++++++++++----------- 1 file changed, 34 insertions(+), 17 deletions(-) diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index b4a565e3..2d590bb5 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -415,6 +415,26 @@ fetch_uri_contents_utf8_sync (OtPullData *pull_data, return ret; } +static gboolean +write_commitpartial_for (OtPullData *pull_data, + const char *checksum, + GError **error) +{ + g_autofree char *commitpartial_path = _ostree_get_commitpartial_path (checksum); + glnx_fd_close int fd = -1; + + fd = openat (pull_data->repo->repo_dir_fd, commitpartial_path, O_EXCL | O_CREAT | O_WRONLY | O_CLOEXEC | O_NOCTTY, 0600); + if (fd == -1) + { + if (errno != EEXIST) + { + glnx_set_error_from_errno (error); + return FALSE; + } + } + return TRUE; +} + static void enqueue_one_object_request (OtPullData *pull_data, const char *checksum, @@ -889,18 +909,8 @@ meta_fetch_on_complete (GObject *object, /* Write the commitpartial file now while we're still fetching data */ if (objtype == OSTREE_OBJECT_TYPE_COMMIT) { - g_autofree char *commitpartial_path = _ostree_get_commitpartial_path (checksum); - glnx_fd_close int fd = -1; - - fd = openat (pull_data->repo->repo_dir_fd, commitpartial_path, O_EXCL | O_CREAT | O_WRONLY | O_CLOEXEC | O_NOCTTY, 0600); - if (fd == -1) - { - if (errno != EEXIST) - { - glnx_set_error_from_errno (error); - goto out; - } - } + if (!write_commitpartial_for (pull_data, checksum, error)) + goto out; } ostree_repo_write_metadata_async (pull_data->repo, objtype, checksum, metadata, @@ -1205,11 +1215,18 @@ scan_one_metadata_object_c (OtPullData *pull_data, if (pull_data->remote_repo_local) { - if (!is_stored && - !ostree_repo_import_object_from_with_trust (pull_data->repo, pull_data->remote_repo_local, - objtype, tmp_checksum, !pull_data->is_untrusted, - cancellable, error)) - goto out; + if (!is_stored) + { + if (!ostree_repo_import_object_from_with_trust (pull_data->repo, pull_data->remote_repo_local, + objtype, tmp_checksum, !pull_data->is_untrusted, + cancellable, error)) + goto out; + if (objtype == OSTREE_OBJECT_TYPE_COMMIT) + { + if (!write_commitpartial_for (pull_data, tmp_checksum, error)) + goto out; + } + } is_stored = TRUE; is_requested = TRUE; } From 3640725439a4a83fad569ee3560ae766036d49c9 Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Tue, 7 Jun 2016 14:38:21 +0200 Subject: [PATCH 15/39] tests: Test partial commits for local remotes This was broken before, fixed in the previous commit. Closes: #324 Approved by: yuqi-zhang --- tests/test-pull-subpath.sh | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/test-pull-subpath.sh b/tests/test-pull-subpath.sh index b0bf4839..05f685b9 100755 --- a/tests/test-pull-subpath.sh +++ b/tests/test-pull-subpath.sh @@ -23,16 +23,18 @@ set -euo pipefail setup_fake_remote_repo1 "archive-z2" -echo '1..1' +echo '1..2' repopath=${test_tmpdir}/ostree-srv/gnomerepo cp -a ${repopath} ${repopath}.orig +for remoteurl in $(cat httpd-address)/ostree/gnomerepo \ + file://$(pwd)/ostree-srv/gnomerepo; do cd ${test_tmpdir} rm repo -rf mkdir repo ${CMD_PREFIX} ostree --repo=repo init -${CMD_PREFIX} ostree --repo=repo remote add --set=gpg-verify=false origin $(cat httpd-address)/ostree/gnomerepo +${CMD_PREFIX} ostree --repo=repo remote add --set=gpg-verify=false origin ${remoteurl} ${CMD_PREFIX} ostree --repo=repo pull --subpath=/baz origin main @@ -51,5 +53,5 @@ ${CMD_PREFIX} ostree --repo=repo ls origin:main /firstfile ${CMD_PREFIX} ostree --repo=repo pull origin main assert_not_has_file repo/state/${rev}.commitpartial ${CMD_PREFIX} ostree --repo=repo fsck - echo "ok" +done From 56e652035b67396c3b33d6560c1e9f1c4289c8c9 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 24 Jun 2016 09:35:21 -0400 Subject: [PATCH 16/39] build-sys: Make libostree-1.so depend on the symbol file Otherwise one changing it doesn't cause a symbol to be exported. Closes: #365 Approved by: jlebon --- Makefile-libostree.am | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile-libostree.am b/Makefile-libostree.am index 8c2fd297..efddd37d 100644 --- a/Makefile-libostree.am +++ b/Makefile-libostree.am @@ -148,6 +148,7 @@ libostree_1_la_CFLAGS = $(AM_CFLAGS) -I$(srcdir)/bsdiff -I$(srcdir)/libglnx -I$( -fvisibility=hidden '-D_OSTREE_PUBLIC=__attribute__((visibility("default"))) extern' libostree_1_la_LDFLAGS = -version-number 1:0:0 -Bsymbolic-functions -Wl,--version-script=$(top_srcdir)/src/libostree/libostree.sym libostree_1_la_LIBADD = libotutil.la libbupsplit.la libglnx.la libbsdiff.la libostree-kernel-args.la $(OT_INTERNAL_GIO_UNIX_LIBS) $(OT_INTERNAL_GPGME_LIBS) $(OT_DEP_LZMA_LIBS) $(OT_DEP_ZLIB_LIBS) +EXTRA_libostree_1_la_DEPENDENCIES = $(top_srcdir)/src/libostree/libostree.sym EXTRA_DIST += src/libostree/libostree.sym From f38c33fec8f98812a4e8471b1e887930bd6d16c1 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Sun, 26 Jun 2016 14:26:37 +0100 Subject: [PATCH 17/39] entry_pathname_test_helper: these tests need extended attributes Signed-off-by: Simon McVittie Closes: #366 Approved by: cgwalters --- tests/test-libarchive-import.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/test-libarchive-import.c b/tests/test-libarchive-import.c index aaa3c37b..3ef379e3 100644 --- a/tests/test-libarchive-import.c +++ b/tests/test-libarchive-import.c @@ -433,6 +433,9 @@ entry_pathname_test_helper (gconstpointer data, gboolean on) OstreeRepoCommitModifier *modifier = NULL; gboolean met_etc_file = FALSE; + if (skip_if_no_xattr (td)) + goto out; + modifier = ostree_repo_commit_modifier_new (0, NULL, NULL, NULL); ostree_repo_commit_modifier_set_xattr_callback (modifier, path_cb, NULL, &met_etc_file); From ca899ccfd3458f651c383febc7f28e91d65fbb37 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Sun, 26 Jun 2016 13:56:05 +0100 Subject: [PATCH 18/39] tests: use our own generated libtool, not the one in $PATH libtoolize creates a version of libtool for the right architecture in $(top_builddir), which is guaranteed to be present, and is guaranteed to match what we are compiling (even during cross-compilation). Packaging systems sometimes separate /usr/bin/libtool, which is specific to one architecture, from the libtool development files such as libtoolize and ltmain.sh, which are architecture-independent. For example, in Debian, libtool_*_all.deb contains the files necessary to libtoolize a package and is depended on by the dh-autoreconf package, but libtool-bin_*_amd64.deb (or whatever architecture) contains /usr/bin/libtool and is not normally necessary to depend on. Signed-off-by: Simon McVittie Closes: #367 Approved by: cgwalters --- Makefile-tests.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile-tests.am b/Makefile-tests.am index 80903071..25b8202f 100644 --- a/Makefile-tests.am +++ b/Makefile-tests.am @@ -245,7 +245,7 @@ ALL_LOCAL_RULES += tests/libreaddir-rand.so CLEANFILES += tests/libreaddir-rand.so tests/ostree-symlink-stamp tests/ostree tests/ostree-symlink-stamp: Makefile - @real_bin=`cd $(top_builddir) && libtool --mode=execute echo ostree`; \ + @real_bin=`cd $(top_builddir) && ./libtool --mode=execute echo ostree`; \ ln -sf "$${real_bin}" tests/ostree; \ touch $@ From 0ed9f520daca0dbb58bd278e709aff4b298e238f Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Sun, 26 Jun 2016 13:57:13 +0100 Subject: [PATCH 19/39] tests: fail the build if symlinking tests/ostree fails Signed-off-by: Simon McVittie Closes: #367 Approved by: cgwalters --- Makefile-tests.am | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Makefile-tests.am b/Makefile-tests.am index 25b8202f..9be9061b 100644 --- a/Makefile-tests.am +++ b/Makefile-tests.am @@ -245,7 +245,8 @@ ALL_LOCAL_RULES += tests/libreaddir-rand.so CLEANFILES += tests/libreaddir-rand.so tests/ostree-symlink-stamp tests/ostree tests/ostree-symlink-stamp: Makefile - @real_bin=`cd $(top_builddir) && ./libtool --mode=execute echo ostree`; \ + @set -e; \ + real_bin=`cd $(top_builddir) && ./libtool --mode=execute echo ostree`; \ ln -sf "$${real_bin}" tests/ostree; \ touch $@ From da989b473dc6b812646b65437f96ff5b9e3af3a4 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sun, 26 Jun 2016 10:23:56 -0400 Subject: [PATCH 20/39] rofiles-fuse: Do allow fchmod/fchown on directories The program is called ro*files* and ostree creates physical copies of directories, so changing them is fine. I hit this when trying to do a copy checkout onto an rofiles-fuse mount. Closes: #368 Approved by: jlebon --- src/rofiles-fuse/main.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/rofiles-fuse/main.c b/src/rofiles-fuse/main.c index bdf7ffb8..8468276d 100644 --- a/src/rofiles-fuse/main.c +++ b/src/rofiles-fuse/main.c @@ -261,8 +261,11 @@ can_write (const char *path) else return -errno; } - if (!devino_set_contains (stbuf.st_dev, stbuf.st_ino)) - return -EROFS; + if (!S_ISDIR (stbuf.st_mode)) + { + if (!devino_set_contains (stbuf.st_dev, stbuf.st_ino)) + return -EROFS; + } return 0; } From 439069b2bbe268fae7010e2fcb67c57d914cc22f Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sun, 26 Jun 2016 10:25:03 -0400 Subject: [PATCH 21/39] checkout: Add an option to require hardlinks I've seen a few people hit this and wonder why checkouts are slow/take space. Really, ensuring this happens is the *point* of OSTree. Physical copies should be a last resort fallback for very unusual situations (one of those is rpm-ostree checking out the db since librpm doesn't know how to read from libostree). Even I hit the fact that `/var` is a mountpoint disallowing hardlinks with `/ostree` once and was confused. =) Add this to the rofiles-fuse test case because it creates a mount point. Closes: #368 Approved by: jlebon --- src/libostree/ostree-repo-checkout.c | 2 +- src/libostree/ostree-repo.h | 3 ++- src/ostree/ot-builtin-checkout.c | 5 ++++- tests/test-rofiles-fuse.sh | 12 +++++++++++- 4 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/libostree/ostree-repo-checkout.c b/src/libostree/ostree-repo-checkout.c index a6d69c42..0094aefd 100644 --- a/src/libostree/ostree-repo-checkout.c +++ b/src/libostree/ostree-repo-checkout.c @@ -346,7 +346,7 @@ checkout_file_hardlink (OstreeRepo *self, again: if (linkat (srcfd, loose_path, destination_dfd, destination_name, 0) != -1) ret_was_supported = TRUE; - else if (errno == EMLINK || errno == EXDEV || errno == EPERM) + else if (!options->no_copy_fallback && (errno == EMLINK || errno == EXDEV || errno == EPERM)) { /* EMLINK, EXDEV and EPERM shouldn't be fatal; we just can't do the * optimization of hardlinking instead of copying. diff --git a/src/libostree/ostree-repo.h b/src/libostree/ostree-repo.h index 2f60e633..872f9b81 100644 --- a/src/libostree/ostree-repo.h +++ b/src/libostree/ostree-repo.h @@ -750,7 +750,8 @@ typedef struct { guint enable_uncompressed_cache : 1; guint disable_fsync : 1; guint process_whiteouts : 1; - guint reserved : 29; + guint no_copy_fallback : 1; + guint reserved : 28; const char *subpath; diff --git a/src/ostree/ot-builtin-checkout.c b/src/ostree/ot-builtin-checkout.c index 810a8f72..c6fdf1fc 100644 --- a/src/ostree/ot-builtin-checkout.c +++ b/src/ostree/ot-builtin-checkout.c @@ -40,6 +40,7 @@ static gboolean opt_whiteouts; static gboolean opt_from_stdin; static char *opt_from_file; static gboolean opt_disable_fsync; +static gboolean opt_require_hardlinks; static gboolean parse_fsync_cb (const char *option_name, @@ -67,6 +68,7 @@ static GOptionEntry options[] = { { "from-stdin", 0, 0, G_OPTION_ARG_NONE, &opt_from_stdin, "Process many checkouts from standard input", NULL }, { "from-file", 0, 0, G_OPTION_ARG_STRING, &opt_from_file, "Process many checkouts from input file", "FILE" }, { "fsync", 0, 0, G_OPTION_ARG_CALLBACK, parse_fsync_cb, "Specify how to invoke fsync()", "POLICY" }, + { "require-hardlinks", 'H', 0, G_OPTION_ARG_NONE, &opt_require_hardlinks, "Do not fall back to full copies if hardlinking fails", NULL }, { NULL } }; @@ -85,7 +87,7 @@ process_one_checkout (OstreeRepo *repo, * `ostree_repo_checkout_tree_at` until such time as we have a more * convenient infrastructure for testing C APIs with data. */ - if (opt_disable_cache || opt_whiteouts) + if (opt_disable_cache || opt_whiteouts || opt_require_hardlinks) { OstreeRepoCheckoutOptions options = { 0, }; @@ -97,6 +99,7 @@ process_one_checkout (OstreeRepo *repo, options.process_whiteouts = TRUE; if (subpath) options.subpath = subpath; + options.no_copy_fallback = opt_require_hardlinks; if (!ostree_repo_checkout_tree_at (repo, &options, AT_FDCWD, destination, diff --git a/tests/test-rofiles-fuse.sh b/tests/test-rofiles-fuse.sh index d021df09..736747f7 100755 --- a/tests/test-rofiles-fuse.sh +++ b/tests/test-rofiles-fuse.sh @@ -26,7 +26,7 @@ skip_without_user_xattrs setup_test_repository "bare-user" -echo "1..5" +echo "1..6" mkdir mnt @@ -71,3 +71,13 @@ echo "ok deletion" ${CMD_PREFIX} ostree --repo=repo commit -b test2 -s fromfuse --link-checkout-speedup --tree=dir=checkout-test2 echo "ok commit" + +${CMD_PREFIX} ostree --repo=repo checkout -U test2 mnt/test2-checkout-copy-fallback +assert_file_has_content mnt/test2-checkout-copy-fallback/anewfile-for-fuse anewfile-for-fuse + +if ${CMD_PREFIX} ostree --repo=repo checkout -UH test2 mnt/test2-checkout-copy-hardlinked 2>err.txt; then + assert_not_reached "Checking out via hardlinks across mountpoint unexpectedly succeeded!" +fi +assert_file_has_content err.txt "Invalid cross-device link" + +echo "ok checkout copy fallback" From f4e92a1e06f80e1ad1d66eba6d060adbdbe4bd5b Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Mon, 27 Jun 2016 13:59:22 -0400 Subject: [PATCH 22/39] ostree admin switch: fix short summary Closes: #371 Approved by: cgwalters --- src/ostree/ot-admin-builtin-switch.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ostree/ot-admin-builtin-switch.c b/src/ostree/ot-admin-builtin-switch.c index b313e830..895538aa 100644 --- a/src/ostree/ot-admin-builtin-switch.c +++ b/src/ostree/ot-admin-builtin-switch.c @@ -64,7 +64,7 @@ ot_admin_builtin_switch (int argc, char **argv, GCancellable *cancellable, GErro GKeyFile *old_origin; GKeyFile *new_origin = NULL; - context = g_option_context_new ("REF - Construct new tree from current origin and deploy it, if it changed"); + context = g_option_context_new ("REF - Construct new tree from REF and deploy it"); if (!ostree_admin_option_context_parse (context, options, &argc, &argv, OSTREE_ADMIN_BUILTIN_FLAG_SUPERUSER, From b53fb92a9d200b51d3663ff8798a483dc670402a Mon Sep 17 00:00:00 2001 From: Dan Nicholson Date: Fri, 13 May 2016 06:47:09 -0700 Subject: [PATCH 23/39] tests: Remove gpg verification files from EXTRA_DIST Follow on from 70a11189. These files are already disted. Closes: #372 Approved by: cgwalters --- Makefile-tests.am | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/Makefile-tests.am b/Makefile-tests.am index 9be9061b..f283d592 100644 --- a/Makefile-tests.am +++ b/Makefile-tests.am @@ -232,12 +232,7 @@ EXTRA_DIST += \ tests/libostreetest.h \ tests/libtest.sh \ tests/gpg-verify-data/README.md \ - tests/gpg-verify-data/lgpl2 \ - tests/gpg-verify-data/lgpl2.sig \ - tests/gpg-verify-data/pubring.gpg \ - tests/gpg-verify-data/secring.gpg \ - tests/gpg-verify-data/trustdb.gpg \ - tests/gpg-verify-data/gpg.conf + $(NULL) tests/libreaddir-rand.so: Makefile $(AM_V_GEN) ln -fns ../.libs/libreaddir-rand.so tests From 8b397301c4ed87caafac86c3aaee47ccb644849f Mon Sep 17 00:00:00 2001 From: Dan Nicholson Date: Fri, 13 May 2016 10:22:02 -0700 Subject: [PATCH 24/39] tests: Ensure mutable deployments from libostreetest When creating sysroots with libostreetest, we don't get the benefit of the OSTREE_SYSROOT_DEBUG setting in libtest.sh. That means we'll get immutable deployments that can't be easily cleaned up. Ensure the environment variable is set before creating new sysroots. It would be nice to set the debug flags directly, but that's private API that isn't currently pulled into libostreetest. Closes: #372 Approved by: cgwalters --- tests/libostreetest.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/libostreetest.c b/tests/libostreetest.c index 81f743e9..d0eb3e81 100644 --- a/tests/libostreetest.c +++ b/tests/libostreetest.c @@ -94,6 +94,9 @@ ot_test_setup_sysroot (GCancellable *cancellable, if (!ot_test_run_libtest ("setup_os_repository \"archive-z2\" \"syslinux\"", error)) goto out; + /* Make sure deployments are mutable */ + g_setenv ("OSTREE_SYSROOT_DEBUG", "mutable-deployments", TRUE); + ret_sysroot = ostree_sysroot_new (sysroot_path); ret = TRUE; From 6162fde4f1770c08fcdc41391c2b3bc915e78b19 Mon Sep 17 00:00:00 2001 From: Dan Nicholson Date: Fri, 13 May 2016 11:01:29 -0700 Subject: [PATCH 25/39] build: Distribute libglnx and bsdiff Makefile templates In order to re-run autogen.sh from the tarball, the libglnx and bsdiff Makefile templates need to be provided. Closes: #372 Approved by: cgwalters --- Makefile.am | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Makefile.am b/Makefile.am index 1de31544..0d2fd6b3 100644 --- a/Makefile.am +++ b/Makefile.am @@ -60,12 +60,14 @@ libglnx_srcpath := $(srcdir)/libglnx libglnx_cflags := $(OT_DEP_GIO_UNIX_CFLAGS) "-I$(libglnx_srcpath)" libglnx_libs := $(OT_DEP_GIO_UNIX_LIBS) include libglnx/Makefile-libglnx.am.inc +EXTRA_DIST += libglnx/Makefile-libglnx.am noinst_LTLIBRARIES += libglnx.la libbsdiff_srcpath := $(srcdir)/bsdiff libbsdiff_cflags := $(OT_DEP_GIO_UNIX_CFLAGS) "-I$(bsdiff_srcpath)" libbsdiff_libs := $(OT_DEP_GIO_UNIX_LIBS) include bsdiff/Makefile-bsdiff.am.inc +EXTRA_DIST += bsdiff/Makefile-bsdiff.am noinst_LTLIBRARIES += libbsdiff.la include Makefile-otutil.am From 99a76c9b34a03cf6fc3727b0ecddad99633dc5b3 Mon Sep 17 00:00:00 2001 From: Dan Nicholson Date: Fri, 13 May 2016 11:38:50 -0700 Subject: [PATCH 26/39] tests: Remove extra $CMD_PREFIX from test-auto-summary.sh $OSTREE already has $CMD_PREFIX in it, so adding it again causes you to call env twice with LD_PRELOAD. Closes: #372 Approved by: cgwalters --- tests/test-auto-summary.sh | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/test-auto-summary.sh b/tests/test-auto-summary.sh index 6039f526..96fc068b 100755 --- a/tests/test-auto-summary.sh +++ b/tests/test-auto-summary.sh @@ -31,30 +31,30 @@ mkdir test echo hello > test/a -${CMD_PREFIX} $OSTREE commit -b test -s "A commit" test +$OSTREE commit -b test -s "A commit" test echo "ok commit 1" -${CMD_PREFIX} $OSTREE summary --update +$OSTREE summary --update OLD_MD5=$(md5sum repo/summary) echo hello2 > test/a -${CMD_PREFIX} $OSTREE commit -b test -s "Another commit" test +$OSTREE commit -b test -s "Another commit" test echo "ok commit 2" assert_streq "$OLD_MD5" "$(md5sum repo/summary)" -${CMD_PREFIX} $OSTREE --repo=repo config set core.commit-update-summary true +$OSTREE --repo=repo config set core.commit-update-summary true echo hello3 > test/a -${CMD_PREFIX} $OSTREE commit -b test -s "Another commit..." test +$OSTREE commit -b test -s "Another commit..." test echo "ok commit 3" assert_not_streq "$OLD_MD5" "$(md5sum repo/summary)" # Check that summary --update deletes the .sig file touch repo/summary.sig -${CMD_PREFIX} $OSTREE summary --update +$OSTREE summary --update assert_not_has_file repo/summary.sig From a94530111a31ff3784af293a6cd4da4c3c7bc34c Mon Sep 17 00:00:00 2001 From: Dan Nicholson Date: Fri, 13 May 2016 12:53:01 -0700 Subject: [PATCH 27/39] tests: Improve check for /proc/cmdline kargs On some systems there may be no root= argument, so the tests for appending /proc/cmdline arguments will fail. Instead, loop over each of the arguments in the host's /proc/cmdline and test that they're in the constructed config file. That will actually test if ostree added all of the system's /proc/cmdline args correctly. The regex isn't perfect here, but it's probably good enough for this test. Closes: #372 Approved by: cgwalters --- tests/test-admin-deploy-karg.sh | 7 +++---- tests/test-admin-instutil-set-kargs.sh | 7 +++---- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/tests/test-admin-deploy-karg.sh b/tests/test-admin-deploy-karg.sh index 2ce88627..1165b428 100755 --- a/tests/test-admin-deploy-karg.sh +++ b/tests/test-admin-deploy-karg.sh @@ -43,10 +43,9 @@ assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'option echo "ok deploy with --karg, but same config" ${CMD_PREFIX} ostree admin deploy --karg-proc-cmdline --os=testos testos:testos/buildmaster/x86_64-runtime -# Here we're asserting that the *host* system has a root= kernel -# argument. I think it's unlikely that anyone doesn't have one, but -# if this is not true for you, please file a bug! -assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'options.*root=.' +for arg in $(cat /proc/cmdline); do + assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf "options.*$arg" +done echo "ok deploy --karg-proc-cmdline" diff --git a/tests/test-admin-instutil-set-kargs.sh b/tests/test-admin-instutil-set-kargs.sh index 0af940ff..d2abec2e 100755 --- a/tests/test-admin-instutil-set-kargs.sh +++ b/tests/test-admin-instutil-set-kargs.sh @@ -54,8 +54,7 @@ assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'option echo "ok instutil set-kargs --append" ${CMD_PREFIX} ostree admin instutil set-kargs --import-proc-cmdline -# Here we're asserting that the *host* system has a root= kernel -# argument. I think it's unlikely that anyone doesn't have one, but -# if this is not true for you, please file a bug! -assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'options.*root=.' +for arg in $(cat /proc/cmdline); do + assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf "options.*$arg" +done echo "ok instutil set-kargs --import-proc-cmdline" From 8933c93a554031f27cc68a2629e96dd718d9b629 Mon Sep 17 00:00:00 2001 From: Dan Nicholson Date: Mon, 27 Jun 2016 13:06:23 -0700 Subject: [PATCH 28/39] build: Override systemd unit directory for distcheck distcheck tests that all the files are installed under $prefix. That doesn't work with the systemd unit directory since the path comes from pkg-config. Override the setting to be under $prefix in that case. Closes: #372 Approved by: cgwalters --- Makefile-boot.am | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Makefile-boot.am b/Makefile-boot.am index 8b62d1ba..508f59bc 100644 --- a/Makefile-boot.am +++ b/Makefile-boot.am @@ -38,6 +38,9 @@ endif if BUILDOPT_SYSTEMD systemdsystemunit_DATA = src/boot/ostree-prepare-root.service \ src/boot/ostree-remount.service + +# Allow the distcheck install under $prefix test to pass +AM_DISTCHECK_CONFIGURE_FLAGS += --with-systemdsystemunitdir='$${libdir}/systemd/system' endif if !BUILDOPT_BUILTIN_GRUB2_MKCONFIG From 0d07c7ecdee251bb821ddebd4f8555bdcfd1d089 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 1 Jul 2016 14:39:49 -0400 Subject: [PATCH 29/39] delta: Add --if-not-exists option I often want to have "idempotent" systems that iterate to a known state. If after generating a commit, the system is interrupted, I'd like the next run to still generate a delta. But we don't want to regenerate if one exists, hence this option. Closes: #375 Approved by: jlebon --- src/libostree/ostree-cmdprivate.c | 1 + src/libostree/ostree-cmdprivate.h | 1 + src/libostree/ostree-repo-static-delta-core.c | 33 +++++++++++++++++++ .../ostree-repo-static-delta-private.h | 7 ++++ src/ostree/ot-builtin-static-delta.c | 16 +++++++++ tests/test-delta.sh | 9 ++++- 6 files changed, 66 insertions(+), 1 deletion(-) diff --git a/src/libostree/ostree-cmdprivate.c b/src/libostree/ostree-cmdprivate.c index 2c85bb4a..4367b497 100644 --- a/src/libostree/ostree-cmdprivate.c +++ b/src/libostree/ostree-cmdprivate.c @@ -47,6 +47,7 @@ ostree_cmd__private__ (void) static OstreeCmdPrivateVTable table = { impl_ostree_generate_grub2_config, _ostree_repo_static_delta_dump, + _ostree_repo_static_delta_query_exists, _ostree_repo_static_delta_delete }; diff --git a/src/libostree/ostree-cmdprivate.h b/src/libostree/ostree-cmdprivate.h index 9a74ead1..8d1c653e 100644 --- a/src/libostree/ostree-cmdprivate.h +++ b/src/libostree/ostree-cmdprivate.h @@ -27,6 +27,7 @@ G_BEGIN_DECLS typedef struct { gboolean (* ostree_generate_grub2_config) (OstreeSysroot *sysroot, int bootversion, int target_fd, GCancellable *cancellable, GError **error); gboolean (* ostree_static_delta_dump) (OstreeRepo *repo, const char *delta_id, GCancellable *cancellable, GError **error); + gboolean (* ostree_static_delta_query_exists) (OstreeRepo *repo, const char *delta_id, gboolean *out_exists, GCancellable *cancellable, GError **error); gboolean (* ostree_static_delta_delete) (OstreeRepo *repo, const char *delta_id, GCancellable *cancellable, GError **error); } OstreeCmdPrivateVTable; diff --git a/src/libostree/ostree-repo-static-delta-core.c b/src/libostree/ostree-repo-static-delta-core.c index 6253a45f..b730c40f 100644 --- a/src/libostree/ostree-repo-static-delta-core.c +++ b/src/libostree/ostree-repo-static-delta-core.c @@ -819,6 +819,39 @@ _ostree_repo_static_delta_delete (OstreeRepo *self, return ret; } +gboolean +_ostree_repo_static_delta_query_exists (OstreeRepo *self, + const char *delta_id, + gboolean *out_exists, + GCancellable *cancellable, + GError **error) +{ + gboolean ret = FALSE; + g_autofree char *from = NULL; + g_autofree char *to = NULL; + g_autofree char *superblock_path = NULL; + struct stat stbuf; + + _ostree_parse_delta_name (delta_id, &from, &to); + superblock_path = _ostree_get_relative_static_delta_superblock_path (from, to); + + if (fstatat (self->repo_dir_fd, superblock_path, &stbuf, 0) < 0) + { + if (errno == ENOENT) + { + *out_exists = FALSE; + return TRUE; + } + else + { + glnx_set_error_from_errno (error); + return FALSE; + } + } + *out_exists = TRUE; + return TRUE; +} + gboolean _ostree_repo_static_delta_dump (OstreeRepo *self, const char *delta_id, diff --git a/src/libostree/ostree-repo-static-delta-private.h b/src/libostree/ostree-repo-static-delta-private.h index eeb99c3f..31e8971e 100644 --- a/src/libostree/ostree-repo-static-delta-private.h +++ b/src/libostree/ostree-repo-static-delta-private.h @@ -190,6 +190,13 @@ _ostree_delta_compute_similar_objects (OstreeRepo *repo, GCancellable *cancellable, GError **error); +gboolean +_ostree_repo_static_delta_query_exists (OstreeRepo *repo, + const char *delta_id, + gboolean *out_exists, + GCancellable *cancellable, + GError **error); + gboolean _ostree_repo_static_delta_dump (OstreeRepo *repo, const char *delta_id, diff --git a/src/ostree/ot-builtin-static-delta.c b/src/ostree/ot-builtin-static-delta.c index 09eb90ab..90702c5a 100644 --- a/src/ostree/ot-builtin-static-delta.c +++ b/src/ostree/ot-builtin-static-delta.c @@ -37,6 +37,7 @@ static gboolean opt_empty; static gboolean opt_swap_endianness; static gboolean opt_inline; static gboolean opt_disable_bsdiff; +static gboolean opt_if_not_exists; #define BUILTINPROTO(name) static gboolean ot_static_delta_builtin_ ## name (int argc, char **argv, GCancellable *cancellable, GError **error) @@ -64,6 +65,7 @@ static GOptionEntry generate_options[] = { { "inline", 0, 0, G_OPTION_ARG_NONE, &opt_inline, "Inline delta parts into main delta", NULL }, { "to", 0, 0, G_OPTION_ARG_STRING, &opt_to_rev, "Create delta to revision REV", "REV" }, { "disable-bsdiff", 0, 0, G_OPTION_ARG_NONE, &opt_disable_bsdiff, "Disable use of bsdiff", NULL }, + { "if-not-exists", 'n', 0, G_OPTION_ARG_NONE, &opt_if_not_exists, "Only generate if a delta does not already exist", NULL }, { "set-endianness", 0, 0, G_OPTION_ARG_STRING, &opt_endianness, "Choose metadata endianness ('l' or 'B')", "ENDIAN" }, { "swap-endianness", 0, 0, G_OPTION_ARG_NONE, &opt_swap_endianness, "Swap metadata endianness from host order", NULL }, { "min-fallback-size", 0, 0, G_OPTION_ARG_STRING, &opt_min_fallback_size, "Minimum uncompressed size in megabytes for individual HTTP request", NULL}, @@ -264,6 +266,20 @@ ot_static_delta_builtin_generate (int argc, char **argv, GCancellable *cancellab } if (!ostree_repo_resolve_rev (repo, opt_to_rev, FALSE, &to_resolved, error)) goto out; + + if (opt_if_not_exists) + { + gboolean does_exist; + g_autofree char *delta_id = from_resolved ? g_strconcat (from_resolved, "-", to_resolved, NULL) : g_strdup (to_resolved); + if (!ostree_cmd__private__ ()->ostree_static_delta_query_exists (repo, delta_id, &does_exist, cancellable, error)) + goto out; + if (does_exist) + { + g_print ("Delta %s already exists.\n", delta_id); + ret = TRUE; + goto out; + } + } if (opt_endianness) { diff --git a/tests/test-delta.sh b/tests/test-delta.sh index 4b2b879a..bd735c4a 100755 --- a/tests/test-delta.sh +++ b/tests/test-delta.sh @@ -82,6 +82,8 @@ get_assert_one_direntry_matching() { origrev=$(${CMD_PREFIX} ostree --repo=repo rev-parse test) ${CMD_PREFIX} ostree --repo=repo static-delta generate --empty --to=${origrev} +${CMD_PREFIX} ostree --repo=repo static-delta generate --if-not-exists --empty --to=${origrev} > out.txt +assert_file_has_content out.txt "${origrev} already exists" ${CMD_PREFIX} ostree --repo=repo static-delta list | grep ${origrev} || exit 1 ${CMD_PREFIX} ostree --repo=repo prune ${CMD_PREFIX} ostree --repo=repo static-delta list | grep ${origrev} || exit 1 @@ -91,7 +93,12 @@ ${CMD_PREFIX} ostree --repo=repo commit -b test -s test --tree=dir=files newrev=$(${CMD_PREFIX} ostree --repo=repo rev-parse test) -${CMD_PREFIX} ostree --repo=repo static-delta generate --from=${origrev} --to=${newrev} --inline +${CMD_PREFIX} ostree --repo=repo static-delta generate --if-not-exists --from=${origrev} --to=${newrev} --inline +${CMD_PREFIX} ostree --repo=repo static-delta generate --if-not-exists --from=${origrev} --to=${newrev} --inline > out.txt +assert_file_has_content out.txt "${origrev}-${newrev} already exists" +# Should regenerate +${CMD_PREFIX} ostree --repo=repo static-delta generate --from=${origrev} --to=${newrev} --inline > out.txt +assert_not_file_has_content out.txt "${origrev}-${newrev} already exists" deltaprefix=$(get_assert_one_direntry_matching repo/deltas '.') deltadir=$(get_assert_one_direntry_matching repo/deltas/${deltaprefix} '-') From cbca341a7710dd2db887090f61dbd03cc08a36e7 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 30 Jun 2016 15:29:28 -0400 Subject: [PATCH 30/39] docs: Add a section on Docker This could have a lot more obviously, but just laying down my thoughts as a starting point. Closes: #374 Approved by: jlebon --- docs/manual/related-projects.md | 45 +++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/docs/manual/related-projects.md b/docs/manual/related-projects.md index d37e2cfe..38608b94 100644 --- a/docs/manual/related-projects.md +++ b/docs/manual/related-projects.md @@ -268,3 +268,48 @@ developed by Fedora, Red Hat, and CentOS as part of Project Atomic. This is a service that incrementally rebuilds and tests GNOME on every commit. The need to make and distribute snapshots for this system was the original inspiration for ostree. + +## Docker + +It makes sense to compare OSTree and Docker as far as *wire formats* +go. OSTree is not itself a container tool, but can be used as a +transport/storage format for container tools. + +Docker has (at the time of this writing) two format versions (v1 and +v2). v1 is deprecated, so we'll look at [format version 2](https://github.com/docker/docker/blob/master/image/spec/v1.1.md). + +A Docker image is a series of layers, and a layer is essentially JSON +metadata plus a tarball. The tarballs capture changes between layers, +including handling deleting files in higher layers. + +Because the payload format is just tar, Docker hence captures +(numeric) uid/gid and xattrs. + +This "layering" model is an interesting and powerful part of Docker, +allowing different images to reference a shared base. OSTree doesn't +implement this natively, but it's not difficult to implement in higher +level tools. For example in +[flatpak](https://github.com/flatpak/flatpak), there's a concept of a +SDK and runtime, and it would make a lot of sense for the SDK to +depend on the runtime, to avoid clients downloading data twice (even +if it's deduplicated on disk). + +That gets to an advantage of OSTree over Docker; OSTree checksums +individual files (not tarballs), and uses this for deduplication. +Docker (natively) only shares storage via layering. + +The biggest feature OSTree has over Docker though is support for +(static) deltas, and even without pre-configured static deltas, the +archive-z2 format has "natural" deltas. Particularly for a "base +operating system", one really wants on-wire deltas. It'd likely be +possible to extend Docker with this concept. + +A core challenge both share is around metadata (particularly signing) +and search/discovery (the ostree `summary` file doesn't scale very +well). + +One major issue Docker has is that it [checksums compressed data](https://github.com/projectatomic/skopeo/issues/11), +and furthermore the tar format is flexible, with multiple ways to represent data, +making it hard to impossible to reassemble and verify from on-disk state. +The [tarsum](https://github.com/docker/docker/blob/master/pkg/tarsum/tarsum_spec.md) effort +was intended to address this, but it was not adopted in the end for v2. From 9df846559a8e7364ac710e6f5250f7f2001a37ea Mon Sep 17 00:00:00 2001 From: Bastien Nocera Date: Mon, 4 Jul 2016 10:31:27 +0000 Subject: [PATCH 31/39] libostree: Fix build failure with glib 2.42 G_DEFINE_AUTOPTR_CLEANUP_FUNC is a new function in GLib 2.44, but libglnx contains a backported version of it. A few source files were however using G_DEFINE_AUTOPTR_CLEANUP_FUNC either without including libglnx.h, or without including it early enough. This fix is similar to the one in commit d368624. Closes #376 Closes: #377 Approved by: smcv --- src/libostree/ostree-gpg-verifier.c | 1 + src/libostree/ostree-repo-pull.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libostree/ostree-gpg-verifier.c b/src/libostree/ostree-gpg-verifier.c index 0ec0b515..d487507f 100644 --- a/src/libostree/ostree-gpg-verifier.c +++ b/src/libostree/ostree-gpg-verifier.c @@ -23,6 +23,7 @@ #include "config.h" +#include "libglnx.h" #include "ostree-gpg-verifier.h" #include "ostree-gpg-verify-result-private.h" #include "otutil.h" diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 2d590bb5..438f5958 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -22,12 +22,12 @@ #include "config.h" +#include "libglnx.h" #include "ostree.h" #include "otutil.h" #ifdef HAVE_LIBSOUP -#include "libglnx.h" #include "ostree-core-private.h" #include "ostree-repo-private.h" #include "ostree-repo-static-delta-private.h" From d371aec217295e6d5a6efd11550a925e52342687 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Mon, 4 Jul 2016 12:49:49 -0400 Subject: [PATCH 32/39] static-delta-core.c: squash unused var warning Closes: #379 Approved by: cgwalters --- src/libostree/ostree-repo-static-delta-core.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libostree/ostree-repo-static-delta-core.c b/src/libostree/ostree-repo-static-delta-core.c index b730c40f..68c45055 100644 --- a/src/libostree/ostree-repo-static-delta-core.c +++ b/src/libostree/ostree-repo-static-delta-core.c @@ -826,7 +826,6 @@ _ostree_repo_static_delta_query_exists (OstreeRepo *self, GCancellable *cancellable, GError **error) { - gboolean ret = FALSE; g_autofree char *from = NULL; g_autofree char *to = NULL; g_autofree char *superblock_path = NULL; From b0bfb92831400edfde06425499fb50fd3258b3f8 Mon Sep 17 00:00:00 2001 From: Mathnerd314 Date: Sat, 18 Jun 2016 10:23:12 -0600 Subject: [PATCH 33/39] pull: Free fetch_data by default This should fix the memory leaks in #352 This is a subset of the changes, the other part is in my pull code rewrite Closes: #382 Approved by: cgwalters --- src/libostree/ostree-repo-pull.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 438f5958..82bf66f6 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -703,6 +703,7 @@ content_fetch_on_complete (GObject *object, const char *checksum; g_autofree char *checksum_obj = NULL; OstreeObjectType objtype; + gboolean free_fetch_data = TRUE; temp_path = _ostree_fetcher_request_uri_with_partial_finish (fetcher, result, error); if (!temp_path) @@ -761,11 +762,17 @@ content_fetch_on_complete (GObject *object, object_input, length, cancellable, content_fetch_on_write_complete, fetch_data); + free_fetch_data = FALSE; } out: pull_data->n_outstanding_content_fetches--; check_outstanding_requests_handle_error (pull_data, local_error); + if (free_fetch_data) + { + g_variant_unref (fetch_data->object); + g_free (fetch_data); + } } static void @@ -829,7 +836,7 @@ meta_fetch_on_complete (GObject *object, GError *local_error = NULL; GError **error = &local_error; glnx_fd_close int fd = -1; - gboolean free_fetch_data = FALSE; + gboolean free_fetch_data = TRUE; ostree_object_name_deserialize (fetch_data->object, &checksum, &objtype); checksum_obj = ostree_object_to_string (checksum, objtype); @@ -895,8 +902,6 @@ meta_fetch_on_complete (GObject *object, if (!fetch_data->object_is_stored) enqueue_one_object_request (pull_data, checksum, objtype, FALSE, FALSE); - - free_fetch_data = TRUE; } else { @@ -917,6 +922,7 @@ meta_fetch_on_complete (GObject *object, pull_data->cancellable, on_metadata_written, fetch_data); pull_data->n_outstanding_metadata_write_requests++; + free_fetch_data = FALSE; } out: @@ -924,7 +930,7 @@ meta_fetch_on_complete (GObject *object, pull_data->n_outstanding_metadata_fetches--; pull_data->n_fetched_metadata++; check_outstanding_requests_handle_error (pull_data, local_error); - if (local_error || free_fetch_data) + if (free_fetch_data) { g_variant_unref (fetch_data->object); g_free (fetch_data); @@ -978,6 +984,7 @@ static_deltapart_fetch_on_complete (GObject *object, GError *local_error = NULL; GError **error = &local_error; glnx_fd_close int fd = -1; + gboolean free_fetch_data = TRUE; g_debug ("fetch static delta part %s complete", fetch_data->expected_checksum); @@ -1015,13 +1022,14 @@ static_deltapart_fetch_on_complete (GObject *object, on_static_delta_written, fetch_data); pull_data->n_outstanding_deltapart_write_requests++; + free_fetch_data = FALSE; out: g_assert (pull_data->n_outstanding_deltapart_fetches > 0); pull_data->n_outstanding_deltapart_fetches--; pull_data->n_fetched_deltaparts++; check_outstanding_requests_handle_error (pull_data, local_error); - if (local_error) + if (free_fetch_data) fetch_static_delta_data_free (fetch_data); } From 71301d18244a3a26a6ba6fd3934633ef94811a15 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 7 Jul 2016 17:36:08 -0400 Subject: [PATCH 34/39] tests/libtest.sh: Print non-matching file on failure We clean up the temporary directory on failure, which means it's hard to know *why* a regex didn't match. Print it when we hit an error. Closes: #383 Approved by: mbarnes --- tests/libtest.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/libtest.sh b/tests/libtest.sh index 2d064299..e93b9510 100755 --- a/tests/libtest.sh +++ b/tests/libtest.sh @@ -138,6 +138,7 @@ assert_file_has_content () { if ! grep -q -e "$2" "$1"; then sed -e 's/^/# /' < "$1" >&2 echo 1>&2 "File '$1' doesn't match regexp '$2'" + cat $1 exit 1 fi } From d7629d33a4bdd17816a25f13a09865cfb6248d1f Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 7 Jul 2016 17:39:07 -0400 Subject: [PATCH 35/39] tests: Add some test coverage of repeated pulls w/HTTP 500s Systems like pulp may want to keep retrying in a loop if the server throws a (hopefully transient) 500, and we need test coverage of handling these errors versus our existing 404 and 206 coverage. Closes: #383 Approved by: mbarnes --- Makefile-tests.am | 1 + src/ostree/ot-builtin-trivial-httpd.c | 24 +++++++++++++++ tests/test-pull-repeated.sh | 43 +++++++++++++++++++++++++++ 3 files changed, 68 insertions(+) create mode 100755 tests/test-pull-repeated.sh diff --git a/Makefile-tests.am b/Makefile-tests.am index f283d592..e274c5e6 100644 --- a/Makefile-tests.am +++ b/Makefile-tests.am @@ -57,6 +57,7 @@ dist_test_scripts = \ tests/test-pull-metalink.sh \ tests/test-pull-summary-sigs.sh \ tests/test-pull-resume.sh \ + tests/test-pull-repeated.sh \ tests/test-pull-untrusted.sh \ tests/test-pull-override-url.sh \ tests/test-local-pull.sh \ diff --git a/src/ostree/ot-builtin-trivial-httpd.c b/src/ostree/ot-builtin-trivial-httpd.c index 811e8924..5b942cd5 100644 --- a/src/ostree/ot-builtin-trivial-httpd.c +++ b/src/ostree/ot-builtin-trivial-httpd.c @@ -38,8 +38,14 @@ static char *opt_log = NULL; static gboolean opt_daemonize; static gboolean opt_autoexit; static gboolean opt_force_ranges; +static int opt_random_500s_percentage; +/* We have a strong upper bound for any unlikely + * cases involving repeated random 500s. */ +static int opt_random_500s_max = 100; static gint opt_port = 0; +static guint emitted_random_500s_count = 0; + typedef struct { GFile *root; gboolean running; @@ -52,6 +58,8 @@ static GOptionEntry options[] = { { "port", 'P', 0, G_OPTION_ARG_INT, &opt_port, "Use the specified TCP port", NULL }, { "port-file", 'p', 0, G_OPTION_ARG_FILENAME, &opt_port_file, "Write port number to PATH (- for standard output)", "PATH" }, { "force-range-requests", 0, 0, G_OPTION_ARG_NONE, &opt_force_ranges, "Force range requests by only serving half of files", NULL }, + { "random-500s", 0, 0, G_OPTION_ARG_INT, &opt_random_500s_percentage, "Generate random HTTP 500 errors approximately for PERCENTAGE requests", "PERCENTAGE" }, + { "random-500s-max", 0, 0, G_OPTION_ARG_INT, &opt_random_500s_max, "Limit HTTP 500 errors to MAX (default 100)", "MAX" }, { "log-file", 0, 0, G_OPTION_ARG_FILENAME, &opt_log, "Put logs here", "PATH" }, { NULL } }; @@ -187,6 +195,15 @@ do_get (OtTrivialHttpd *self, goto out; } + if (opt_random_500s_percentage > 0 && + emitted_random_500s_count < opt_random_500s_max && + g_random_int_range (0, 100) < opt_random_500s_percentage) + { + emitted_random_500s_count++; + soup_message_set_status (msg, SOUP_STATUS_INTERNAL_SERVER_ERROR); + goto out; + } + if (path[0] == '/') path++; @@ -388,6 +405,13 @@ ostree_builtin_trivial_httpd (int argc, char **argv, GCancellable *cancellable, app->root = g_file_new_for_path (dirpath); + if (!(opt_random_500s_percentage >= 0 && opt_random_500s_percentage <= 99)) + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, + "Invalid --random-500s=%u", opt_random_500s_percentage); + goto out; + } + if (opt_log) { GOutputStream *stream = NULL; diff --git a/tests/test-pull-repeated.sh b/tests/test-pull-repeated.sh new file mode 100755 index 00000000..5a3af81c --- /dev/null +++ b/tests/test-pull-repeated.sh @@ -0,0 +1,43 @@ +#!/bin/bash +# +# Copyright (C) 2016 Red Hat +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this library; if not, write to the +# Free Software Foundation, Inc., 59 Temple Place - Suite 330, +# Boston, MA 02111-1307, USA. + +set -euo pipefail + +. $(dirname $0)/libtest.sh + +echo "1..1" + +COMMIT_SIGN="--gpg-homedir=${TEST_GPG_KEYHOME} --gpg-sign=${TEST_GPG_KEYID_1}" +setup_fake_remote_repo1 "archive-z2" "${COMMIT_SIGN}" --random-500s=50 + +cd ${test_tmpdir} +${CMD_PREFIX} ostree --repo=repo init --mode=archive-z2 +${CMD_PREFIX} ostree --repo=repo remote add --set=gpg-verify=false origin $(cat httpd-address)/ostree/gnomerepo +for x in $(seq 200); do + if ${CMD_PREFIX} ostree --repo=repo pull --mirror origin main 2>err.txt; then + echo "Success on iteration ${x}" + break; + fi + assert_file_has_content err.txt "500.*Internal Server Error" +done + +${CMD_PREFIX} ostree --repo=repo fsck +${CMD_PREFIX} ostree --repo=repo rev-parse main + +echo "ok repeated pull after 500s" From b4c15209e846aaf3bfdbf12eddea3dd1d9a46226 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 8 Jul 2016 10:01:47 -0400 Subject: [PATCH 36/39] fetcher: Hold a ref to main context for lifetime of thread I don't think this fixes the bug I was seeing, but it makes me more comfortable to know we have a strong ref to the main context across the thread lifetime, and we only unset the default right before we go away. If something in `thread_closure_unref()` used `g_main_context_get_thread_default()` for example it'd be wrong before. Closes: #383 Approved by: mbarnes --- src/libostree/ostree-fetcher.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/libostree/ostree-fetcher.c b/src/libostree/ostree-fetcher.c index 313df6a8..2fb2168a 100644 --- a/src/libostree/ostree-fetcher.c +++ b/src/libostree/ostree-fetcher.c @@ -452,12 +452,13 @@ static gpointer ostree_fetcher_session_thread (gpointer data) { ThreadClosure *closure = data; + g_autoptr(GMainContext) mainctx = g_main_context_ref (closure->main_context); gint max_conns; /* This becomes the GMainContext that SoupSession schedules async * callbacks and emits signals from. Make it the thread-default * context for this thread before creating the session. */ - g_main_context_push_thread_default (closure->main_context); + g_main_context_push_thread_default (mainctx); closure->session = soup_session_async_new_with_options (SOUP_SESSION_USER_AGENT, "ostree ", SOUP_SESSION_SSL_USE_SYSTEM_CA_FILE, TRUE, @@ -481,10 +482,13 @@ ostree_fetcher_session_thread (gpointer data) g_main_loop_run (closure->main_loop); - g_main_context_pop_thread_default (closure->main_context); - thread_closure_unref (closure); + /* Do this last, since libsoup uses g_main_current_source() which + * relies on it. + */ + g_main_context_pop_thread_default (mainctx); + return NULL; } From 5d21650ea5fcf42a828c79e0373be73284fcf69c Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 8 Jul 2016 10:15:47 -0400 Subject: [PATCH 37/39] fetcher: Clear all data for session in session thread Conceptually the session thread owns the session, so let's clear out everything predictably there, rather than sometimes having it happen on the main thread. Also, this moves up clearing the pending/outstanding queues *before* we unreference the session, since conceptually they need to reference it as well. Based on a patch from: Matthew Barnes Closes: #383 Approved by: mbarnes --- src/libostree/ostree-fetcher.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/libostree/ostree-fetcher.c b/src/libostree/ostree-fetcher.c index 2fb2168a..2baf6c86 100644 --- a/src/libostree/ostree-fetcher.c +++ b/src/libostree/ostree-fetcher.c @@ -44,7 +44,7 @@ typedef enum { typedef struct { volatile int ref_count; - SoupSession *session; + SoupSession *session; /* not referenced */ GMainContext *main_context; GMainLoop *main_loop; @@ -140,7 +140,8 @@ thread_closure_unref (ThreadClosure *thread_closure) if (g_atomic_int_dec_and_test (&thread_closure->ref_count)) { - g_clear_object (&thread_closure->session); + /* The session thread should have cleared this by now. */ + g_assert (thread_closure->session == NULL); g_clear_pointer (&thread_closure->main_context, g_main_context_unref); g_clear_pointer (&thread_closure->main_loop, g_main_loop_unref); @@ -156,11 +157,6 @@ thread_closure_unref (ThreadClosure *thread_closure) g_free (thread_closure->tmpdir_name); glnx_release_lock_file (&thread_closure->tmpdir_lock); - while (!g_queue_is_empty (&thread_closure->pending_queue)) - g_object_unref (g_queue_pop_head (&thread_closure->pending_queue)); - - g_clear_pointer (&thread_closure->outstanding, g_hash_table_unref); - g_clear_pointer (&thread_closure->output_stream_set, g_hash_table_unref); g_mutex_clear (&thread_closure->output_stream_set_lock); @@ -460,6 +456,7 @@ ostree_fetcher_session_thread (gpointer data) * context for this thread before creating the session. */ g_main_context_push_thread_default (mainctx); + /* We retain ownership of the SoupSession reference. */ closure->session = soup_session_async_new_with_options (SOUP_SESSION_USER_AGENT, "ostree ", SOUP_SESSION_SSL_USE_SYSTEM_CA_FILE, TRUE, SOUP_SESSION_USE_THREAD_CONTEXT, TRUE, @@ -482,6 +479,14 @@ ostree_fetcher_session_thread (gpointer data) g_main_loop_run (closure->main_loop); + /* Since the ThreadClosure may be finalized from any thread we + * unreference all data related to the SoupSession ourself to ensure + * it's freed in the same thread where it was created. */ + g_clear_pointer (&closure->outstanding, g_hash_table_unref); + while (!g_queue_is_empty (&closure->pending_queue)) + g_object_unref (g_queue_pop_head (&closure->pending_queue)); + g_clear_pointer (&closure->session, g_object_unref); + thread_closure_unref (closure); /* Do this last, since libsoup uses g_main_current_source() which From 972ed3e54eb2840a96e99263c614bf6461e1da0a Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 8 Jul 2016 13:17:33 -0400 Subject: [PATCH 38/39] fetcher: Remove unused GTask structure member Spotted by mbarnes. Closes: #383 Approved by: mbarnes --- src/libostree/ostree-fetcher.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/libostree/ostree-fetcher.c b/src/libostree/ostree-fetcher.c index 2baf6c86..865c7b76 100644 --- a/src/libostree/ostree-fetcher.c +++ b/src/libostree/ostree-fetcher.c @@ -88,8 +88,6 @@ typedef struct { guint64 max_size; guint64 current_size; guint64 content_length; - - GTask *task; } OstreeFetcherPendingURI; /* Used by session_thread_idle_add() */ From c31cf75552af9be404fbc6a24643c569855c1c3b Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 8 Jul 2016 15:29:11 -0400 Subject: [PATCH 39/39] Release 2016.7 Closes: #386 Approved by: jlebon --- configure.ac | 2 +- src/libostree/libostree.sym | 11 +++++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/configure.ac b/configure.ac index d88f24e5..70b3b019 100644 --- a/configure.ac +++ b/configure.ac @@ -1,5 +1,5 @@ AC_PREREQ([2.63]) -AC_INIT([ostree], [2016.6], [walters@verbum.org]) +AC_INIT([ostree], [2016.7], [walters@verbum.org]) AC_CONFIG_HEADER([config.h]) AC_CONFIG_MACRO_DIR([buildutil]) AC_CONFIG_AUX_DIR([build-aux]) diff --git a/src/libostree/libostree.sym b/src/libostree/libostree.sym index bdfa7c3f..702968a9 100644 --- a/src/libostree/libostree.sym +++ b/src/libostree/libostree.sym @@ -341,12 +341,19 @@ global: ostree_repo_remote_fetch_summary_with_options; } LIBOSTREE_2016.5; +LIBOSTREE_2016.7 { +global: + ostree_repo_resolve_rev_ext; +} LIBOSTREE_2016.6; + /* NOTE NOTE NOTE * Versions above here are released. Only add symbols below this line. * NOTE NOTE NOTE */ +/* UNCOMMENT WHEN ADDING THE FIRST NEW SYMBOL FOR 2016.8 LIBOSTREE_2016.7 { global: - ostree_repo_resolve_rev_ext; -} LIBOSTREE_2016.6; + insert_symbol_here; +} LIBOSTREE_2016.8; +*/