Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

repo: Add a "force copy" flag to checkout #804

Closed
wants to merge 3 commits into from

Conversation

cgwalters
Copy link
Member

This is intended to be used for copying /usr/etc/etc for
deployments.

A TODO here is to use glnx_file_copy_at() if the repo mode allows
it - then we'd use reflinks if available.

This is intended to be used for copying `/usr/etc` → `/etc` for
deployments.

A TODO here is to use `glnx_file_copy_at()` if the repo mode allows
it - then we'd use reflinks if available.
@@ -102,6 +104,11 @@ process_one_checkout (OstreeRepo *repo,
"Cannot specify both --union and --union-add");
goto out;
}
if (opt_require_hardlinks && opt_force_copy)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be checked at the API level too?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done ⬇️

@@ -41,6 +41,7 @@ fi
validate_checkout_basic() {
(cd $1;
assert_has_file firstfile
assert_not_streq $(stat -c '%h' firstfile) 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, this is failing in test-basic-user.sh. Surprisingly, not in the -U -H case, but just in the vanilla $OSTREE checkout test2 checkout-test2 test. Is there a bug there? Seems like it's hardlinking from a bare-user repo even without -U.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, I read this backwards. The fact that it's failing means that it is copying rather than hardlinking, which is what we want in user-mode without -U. So I think this just needs some conditionals around it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this only works in bare right now. This whole suite really needs rewriting.

@jlebon
Copy link
Member

jlebon commented Apr 24, 2017

@rh-atomic-bot r+ aba5582

@rh-atomic-bot
Copy link

⌛ Testing commit aba5582 with merge 6060abb...

@rh-atomic-bot
Copy link

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

LorbusChris pushed a commit to LorbusChris/rpm-ostree-spec that referenced this pull request Oct 22, 2018
This isn't a new API function so the default rpm detection doesn't work.
But we do need a new ostree for
ostreedev/ostree#804.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants