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

Add new plugin slots for pre/post chroot events #2669

Closed
wants to merge 1 commit into from

Conversation

pmatilai
Copy link
Member

These pre/post hooks get called before and after every chroot() call rpm makes. The initial inspiration came from the now removed experimental non-privileged chroot support with unshare() but this is probably an interesting boundary for various other purposes too.

@pmatilai pmatilai added RFE API API related labels Sep 15, 2023
@ffesti
Copy link
Contributor

ffesti commented Sep 19, 2023

Looks good and pretty straight forward. May be should get a test case.

It might also be a good idea to point out in the docs that these are called even when running without --root. Technically we omit the chroot call in this case but the hooks are ofc executed no matter what (as they should be)

@pmatilai
Copy link
Member Author

Looks good and pretty straight forward. May be should get a test case.

Yep. I actually have a test-case written in the form of "plugin development" test, but it needs some cmake infra bits first. But, this PR isn't in any particular hurry.

It might also be a good idea to point out in the docs that these are called even when running without --root. Technically we omit the chroot call in this case but the hooks are ofc executed no matter what (as they should be)

Um, no they don't. rpmChrootIn() and rpmChrootOut() do always get called but they return with a no-op from the first condition.

These pre/post hooks get called before and after every chroot() call
rpm makes. The initial inspiration came from the now removed experimental
non-privileged chroot support with unshare() but this is probably an
interesting boundary for various other purposes too.

Adding a testcase is annoyingly complicated as we need to install into
an empty root for this to trigger. And in the empty chroot have no shell,
and because this is a real empty chroot we can't just fake it either.
We could use a -p <lua> scriptlet but then that wouldn't test
scriptlet_fork_post because Lua scriptlets aren't currently forked.
So we need two test-cases anyhow, just reuse the same case with
--nodeps and --noscripts.
@pmatilai
Copy link
Member Author

pmatilai commented Oct 5, 2023

And now with a testcase.

@pmatilai
Copy link
Member Author

pmatilai commented Oct 9, 2023

The more I sleep on this, the less convinced I become that this is a good idea. Quite the contrary actually. "Uneasy" is the word.

That instinct is rarely wrong, so I'm canceling this PR. If at some point an actual use-case presents itself, we can always re-evaluate.

@pmatilai pmatilai closed this Oct 9, 2023
@pmatilai pmatilai deleted the chplug branch February 13, 2024 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API API related RFE
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants