Skip to content

Commit

Permalink
lib/checkout: Fix regression in subpath for regular files
Browse files Browse the repository at this point in the history
This is what caused the merge of
coreos/rpm-ostree#652
to blow up, since #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
  • Loading branch information
cgwalters authored and rh-atomic-bot committed May 12, 2017
1 parent b83d509 commit a195888
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 41 deletions.
1 change: 0 additions & 1 deletion Makefile-tests.am
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
30 changes: 25 additions & 5 deletions src/libostree/ostree-repo-checkout.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions tests/basic-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
35 changes: 0 additions & 35 deletions tests/test-repo-checkout-subpath.sh

This file was deleted.

0 comments on commit a195888

Please sign in to comment.