From a195888b0f74ae3739a1a18fc46d39ae4686c5aa Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 11 May 2017 20:29:21 -0400 Subject: [PATCH] lib/checkout: Fix regression in subpath for regular files This is what caused the merge of https://github.com/projectatomic/rpm-ostree/pull/652 to blow up, since https://github.com/ostreedev/ostree/pull/848 landed right before we tried to merge it. When I was writing that PR I remember having an uncertain feeling since we were doing a `mkdirat` above, but at the time I thought we'd have test suite coverage...turns out we didn't. For backwards compatibility, we need to continue to do a `mkdirat` here of the parent. However...I can't think of a reason anyone would *want* that behavior. Hence, let's add a special trick - if the destination name is `.`, we skip `mkdirat()`. That way rpm-ostree for example can open a dfd for `/etc` and avoid the `mkdir`. Fold the subpath tests into `test-basic.sh` since it's not worth a separate file. Add a test case for checking out a file. Closes: #854 Approved by: jlebon --- Makefile-tests.am | 1 - src/libostree/ostree-repo-checkout.c | 30 ++++++++++++++++++++---- tests/basic-test.sh | 12 ++++++++++ tests/test-repo-checkout-subpath.sh | 35 ---------------------------- 4 files changed, 37 insertions(+), 41 deletions(-) delete mode 100755 tests/test-repo-checkout-subpath.sh diff --git a/Makefile-tests.am b/Makefile-tests.am index 335ba9b7bd..8967528867 100644 --- a/Makefile-tests.am +++ b/Makefile-tests.am @@ -96,7 +96,6 @@ _installed_or_uninstalled_test_scripts = \ tests/test-admin-pull-deploy-split.sh \ tests/test-admin-locking.sh \ tests/test-admin-deploy-clean.sh \ - tests/test-repo-checkout-subpath.sh \ tests/test-reset-nonlinear.sh \ tests/test-oldstyle-partial.sh \ tests/test-delta.sh \ diff --git a/src/libostree/ostree-repo-checkout.c b/src/libostree/ostree-repo-checkout.c index f5ef9517b1..236f514b1f 100644 --- a/src/libostree/ostree-repo-checkout.c +++ b/src/libostree/ostree-repo-checkout.c @@ -812,11 +812,31 @@ checkout_tree_at (OstreeRepo *self, /* Special case handling for subpath of a non-directory */ if (g_file_info_get_file_type (source_info) != G_FILE_TYPE_DIRECTORY) - return checkout_one_file_at (self, options, &state, - ostree_repo_file_get_checksum (source), - destination_parent_fd, - g_file_info_get_name (source_info), - cancellable, error); + { + /* For backwards compat reasons, we do a mkdir() here. However, as a + * special case to allow callers to directly check out files without an + * intermediate root directory, we will skip mkdirat() if + * `destination_name` == `.`, since obviously the current directory + * exists. + */ + int destination_dfd = destination_parent_fd; + glnx_fd_close int destination_dfd_owned = -1; + if (strcmp (destination_name, ".") != 0) + { + if (mkdirat (destination_parent_fd, destination_name, 0700) < 0 + && errno != EEXIST) + return glnx_throw_errno_prefix (error, "mkdirat"); + if (!glnx_opendirat (destination_parent_fd, destination_name, TRUE, + &destination_dfd_owned, error)) + return FALSE; + destination_dfd = destination_dfd_owned; + } + return checkout_one_file_at (self, options, &state, + ostree_repo_file_get_checksum (source), + destination_dfd, + g_file_info_get_name (source_info), + cancellable, error); + } /* Cache any directory metadata we read during this operation; * see commit b7afe91e21143d7abb0adde440683a52712aa246 diff --git a/tests/basic-test.sh b/tests/basic-test.sh index 6ddf7b2e54..b209b8390d 100644 --- a/tests/basic-test.sh +++ b/tests/basic-test.sh @@ -341,6 +341,18 @@ cd ${test_tmpdir} $OSTREE checkout --subpath /yet/another test2 checkout-test2-subpath cd checkout-test2-subpath assert_file_has_content tree/green "leaf" +cd ${test_tmpdir} +rm checkout-test2-subpath -rf +$OSTREE ls -R test2 +# Test checking out a file +$OSTREE checkout --subpath /baz/saucer test2 checkout-test2-subpath +assert_file_has_content checkout-test2-subpath/saucer alien +# Test checking out a file without making a subdir +mkdir t +cd t +$OSTREE checkout --subpath /baz/saucer test2 . +assert_file_has_content saucer alien +rm t -rf echo "ok checkout subpath" cd ${test_tmpdir} diff --git a/tests/test-repo-checkout-subpath.sh b/tests/test-repo-checkout-subpath.sh deleted file mode 100755 index 2da5adc216..0000000000 --- a/tests/test-repo-checkout-subpath.sh +++ /dev/null @@ -1,35 +0,0 @@ -#!/bin/bash -# -# Copyright (C) 2011,2013 Colin Walters -# Copyright (C) 2014 Red Hat, Inc. -# -# 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 - -setup_test_repository "bare" - -echo '1..1' - -repopath=${test_tmpdir}/ostree-srv/gnomerepo - -${CMD_PREFIX} ostree --repo=repo checkout -U --subpath=/ test2 checkedout - -${CMD_PREFIX} ostree --repo=repo checkout -U --subpath=/firstfile test2 checkedout2 - -echo "ok"