Skip to content

lib/checkout: Fix regression in subpath for regular files#854

Closed
cgwalters wants to merge 1 commit intoostreedev:masterfrom
cgwalters:checkout-file-regression
Closed

lib/checkout: Fix regression in subpath for regular files#854
cgwalters wants to merge 1 commit intoostreedev:masterfrom
cgwalters:checkout-file-regression

Conversation

@cgwalters
Copy link
Copy Markdown
Member

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.

This is what caused the merge of
coreos/rpm-ostree#652
to blow up, since ostreedev#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.
@jlebon
Copy link
Copy Markdown
Member

jlebon commented May 12, 2017

Ahh, subtle! @rh-atomic-bot r+ 8c24082

@rh-atomic-bot
Copy link
Copy Markdown

⌛ Testing commit 8c24082 with merge a195888...

@rh-atomic-bot
Copy link
Copy Markdown

☀️ Test successful - status-atomicjenkins
Approved by: jlebon
Pushing a195888 to master...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants