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

Implement lua #749

Open
cgwalters opened this issue Apr 24, 2017 · 18 comments
Open

Implement lua #749

cgwalters opened this issue Apr 24, 2017 · 18 comments

Comments

@cgwalters
Copy link
Member

cgwalters commented Apr 24, 2017

Today rpm-ostree doesn't implement -p <lua>. We currently today carry overrides for a few packages in Fedora. The reason we don't implement Lua is it directly conflicts with our per-script sandboxing (using bwrap). We theoretically could run Lua in a new process, but doing so basically obviates all of the point of using Lua in the first place. It might as well be shell script.

A major challenge in doing even the new process model is that the posix API that a lot of scripts use is actually implemented inside the guts of librpm. We'd have to implement something like rpm --lua-eval where we pass the script on stdin or so.

For more background, see this thread on rpm-ecosystem around lua and %pretrans.

And this thread: http://lists.rpm.org/pipermail/rpm-ecosystem/2017-May/000477.html

cgwalters added a commit to cgwalters/rpm-ostree that referenced this issue Apr 24, 2017
I honestly didn't really dive into this, but it looks like
this is yet another "hack upgrades in the yum case", which we
should be able to ignore since we always do reassembly.

I at least tested `java -version` works with this.

This is a band-aid for the bigger issue of:
coreos#749

(Doing this one since an AtomicWS user reported it)
rh-atomic-bot pushed a commit that referenced this issue Apr 24, 2017
I honestly didn't really dive into this, but it looks like
this is yet another "hack upgrades in the yum case", which we
should be able to ignore since we always do reassembly.

I at least tested `java -version` works with this.

This is a band-aid for the bigger issue of:
#749

(Doing this one since an AtomicWS user reported it)

Closes: #750
Approved by: jlebon
@cgwalters cgwalters changed the title Implement lua/%pretrans Implement lua 🐍 May 1, 2017
@cgwalters cgwalters changed the title Implement lua 🐍 Implement lua May 1, 2017
@cgwalters
Copy link
Member Author

Changing this to just Lua, since #763

cgwalters added a commit to cgwalters/systemd that referenced this issue Mar 22, 2018
This is compatible with rpm-ostree, which implements offline and
transactional updates by creating a new root, sandboxing each RPM script
in a bwrap container: coreos/rpm-ostree#749

Further, the "test for a directory and run a binary" pattern is significantly
shorter and more obvious in shell script.

Discussion in: systemd#8090 (comment)

One counter was that this causes systemd to depend on `/bin/sh`, but that's
true today in Fedora at least, and I'd be extremely surprised if there were a
distribution where that wasn't the case.  Down the line RPM could probably
learn to omit `Requires` for `%transfiletrigger` on `/bin/sh`.

Downstream: https://bugzilla.redhat.com/show_bug.cgi?id=1559141
@Conan-Kudo
Copy link

We theoretically could run Lua in a new process, but doing so basically obviates all of the point of using Lua in the first place. It might as well be shell script.

It does not obviate all the point of using Lua. The only reason this is a problem for you is because you refuse to implement the rpmlua interpreter and you use nearly zero of librpm's facilities to actually handle scriptlets.

While most of what rpm-ostree is doing is good, I vehemently disagree that you should be forcing people to rely on shell when they want to use Lua for scriptlets.

@cgwalters
Copy link
Member Author

The only reason this is a problem for you is because you refuse to implement the rpmlua interpreter

I wouldn't say "refuse"...I'm not opposed to doing it. One thing that holds this type of thing back a bit is rpm-ostree needs to work on RHEL7 today, so it's hard to require new changes to librpm.

and you use nearly zero of librpm's facilities to actually handle scriptlets.

True. The way I see this is rpm-ostree makes some fundamental changes to the rpm stack; there are a lot of improvements, but also risky/invasive.

Perhaps down the line we could try to add the "run scripts via bwrap" logic to librpm at some point, but note it's fairly intertwined with the higher level rpm-ostree model. For example, how we make /var read-only. That's a key aspect of our "offline/transactional" updates; the rpm-ostree stack never touches your user data.

cgwalters added a commit to cgwalters/systemd that referenced this issue Mar 22, 2018
This is compatible with rpm-ostree, which implements offline and
transactional updates by creating a new root, sandboxing each RPM script
in a bwrap container: coreos/rpm-ostree#749

Further, the "test for a directory and run a binary" pattern is significantly
shorter and more obvious in shell script.

Discussion in: systemd#8090 (comment)

One counter was that this causes systemd to depend on `/bin/sh`, but that's
true today in Fedora at least, and I'd be extremely surprised if there were a
distribution where that wasn't the case.  Down the line RPM could probably
learn to omit `Requires` for `%transfiletrigger` on `/bin/sh`.

Downstream: https://bugzilla.redhat.com/show_bug.cgi?id=1559141
cgwalters added a commit to cgwalters/systemd that referenced this issue Mar 22, 2018
This is compatible with rpm-ostree, which implements offline and
transactional updates by creating a new root, sandboxing each RPM script
in a bwrap container: coreos/rpm-ostree#749

Further, the "test for a directory and run a binary" pattern is significantly
shorter and more obvious in shell script.

Discussion in: systemd#8090 (comment)

One counter was that this causes systemd to depend on `/bin/sh`, but that's
true today in Fedora at least, and I'd be extremely surprised if there were a
distribution where that wasn't the case.  Down the line RPM could probably
learn to omit `Requires` for `%transfiletrigger` on `/bin/sh`.

Downstream: https://bugzilla.redhat.com/show_bug.cgi?id=1559141
@cgwalters
Copy link
Member Author

One thing we could do fairly easily is support embedding a shell implementation in Lua comments, for projects like glibc that may want to do both, something like this:

%transfiletriggerpostun -p <lua> common -P 2000000 -- /lib /usr/lib /lib64 /usr/lib64
-- nolua begin
-- /sbin/ldconfig
-- nolua end
pid = posix.fork()
if pid == 0 then
  posix.exec("/sbin/ldconfig")
else
  posix.wait(pid)
end
%end

@pmatilai
Copy link

pmatilai commented Apr 15, 2020

We theoretically could run Lua in a new process, but doing so basically obviates all of the point of using Lua in the first place. It might as well be shell script.

This misses the point of -p <lua> by a mile.

I'd actually like to have rpm run -p in a newly forked process to protect the main rpm from side-effects, but there are compatibility etc concerns with that. The original main point of -p <lua> was being able to do stuff when there's nothing at all to exec() in the initial install phase. Plus, people are starting to like and adapt it outside that domain because it's simply more nimble and elegant than throwing shell + dozen helpers at the same problem.

@jlebon
Copy link
Member

jlebon commented Apr 20, 2020

I'd actually like to have rpm run -p in a newly forked process to protect the main rpm from side-effects, but there are compatibility etc concerns with that.

Would be great if rpm had that. Are there any plans to work through the compat issues there and support this?

@cgwalters
Copy link
Member Author

cgwalters commented Apr 20, 2020

The original main point of -p was being able to do stuff when there's nothing at all to exec() in the initial install phase.

See: http://lists.rpm.org/pipermail/rpm-ecosystem/2016-August/000396.html

If there's nothing to exec, there's also no reason to have a %pretrans at all - those are always "upgrade hack/workarounds".

Now, it's true today that the Fedora setup RPM ends up re-implementing filesystem in Lua because...well it's just a pile of crazy.

Anyways per that thread I think what we really want is something like %pretrans -p lua --upgrade-workaround which would only run when upgrading from a previous root. Then rpm-ostree could happily ignore it and so could the initial mock root etc.

I'd actually like to have rpm run -p in a newly forked process to protect the main rpm from side-effects, but there are compatibility etc concerns with that.

And agree this would be great! We could probably spare a bit of bandwidth to help with design/testing if a librpm developer was able to start the ball rolling.

@Conan-Kudo
Copy link

Conan-Kudo commented Apr 20, 2020

If there's nothing to exec, there's also no reason to have a %pretrans at all - those are always "upgrade hack/workarounds".

Lua isn't used just for %pretrans. It's used in places where we want to avoid a circular dependency on bash, too. In fact, that's why I wrote the systemd scriptlets in Lua upstream, only for you guys to force the systemd maintainer to rewrite them downstream in Fedora in shell (and thus, reintroduce the circular dependency).

@cgwalters
Copy link
Member Author

It's used in places where we want to avoid a circular dependency on bash, too.

Fair; though...why the heck does bash require systemd?

Anyways, that also came up in the above-linked thread; today rpm-ostree unpacks all packages before running any scripts which solves that problem. And as I said in the thread, there's really no reason for "installing" bash to involve anything more than unpacking the files to get basic execution.
IOW we don't need necessarily to unpack all packages before scripts, but it should work to unpack bash and coreutils etc. at least without processing any dependencies.

Is there a specific motivation for the increased activity in this ticket? "We want to use the lua systemd macros"?

@cgwalters cgwalters reopened this Apr 20, 2020
@jlebon
Copy link
Member

jlebon commented Apr 20, 2020

Is there a specific motivation for the increased activity in this ticket? "We want to use the lua systemd macros"?

Likely because I linked to it from https://bugzilla.redhat.com/show_bug.cgi?id=1780827#c30 (see also coreos/fedora-coreos-tracker#459).

@Conan-Kudo
Copy link

Please just fix RPM-OSTree to handle Lua scriptlets properly. Being artificially restricted to shell is a terrible situation to be in...

@cgwalters
Copy link
Member Author

Please just fix RPM-OSTree to handle Lua scriptlets properly. Being artificially restricted to shell is a terrible situation to be in...

As is being discussed above, doing so requires new librpm API.

@cgwalters
Copy link
Member Author

I guess one thing we could do is synthesize an RPM containing just the target script to run and install it running something like bwrap ... rpm --dbpath=/tmp/throwawaydb -i /proc/self/fd/N --nodeps --notriggers.

jlebon added a commit to jlebon/rpm-ostree that referenced this issue Jun 17, 2020
The latest crypto-policies package changed recently to dynamically set
the policy at install time so that if FIPS is enabled, the selected
backend is `FIPS`:

https://src.fedoraproject.org/rpms/crypto-policies/c/9b9c9f7378c3fd375b9a08d5283c530a51a5de34?branch=master

This doesn't really make sense for us though since the compose server
configuration should be decoupled from the installroot. (More generally,
this also affects e.g. `yum install --installroot`). Override the script
so that we always select the `DEFAULT` policy.

This also works around the fact that rpm-ostree doesn't yet implement
Lua (coreos#749).

Related: https://bugzilla.redhat.com/show_bug.cgi?id=1847454
Resolves: coreos/fedora-coreos-tracker#540
jlebon added a commit to jlebon/rpm-ostree that referenced this issue Jun 17, 2020
The latest crypto-policies package changed recently to dynamically set
the policy at install time so that if FIPS is enabled, the selected
backend is `FIPS`:

https://src.fedoraproject.org/rpms/crypto-policies/c/9b9c9f7378c3fd375b9a08d5283c530a51a5de34?branch=master

This doesn't really make sense for us though since the compose server
configuration should be decoupled from the installroot. (More generally,
this also affects e.g. `yum install --installroot`). Override the script
so that we always select the `DEFAULT` policy.

This also works around the fact that rpm-ostree doesn't yet implement
Lua (coreos#749).

Related: https://bugzilla.redhat.com/show_bug.cgi?id=1847454
Resolves: coreos/fedora-coreos-tracker#540
jlebon added a commit to jlebon/rpm-ostree that referenced this issue Jun 17, 2020
The latest crypto-policies package changed recently to dynamically set
the policy at install time so that if FIPS is enabled, the selected
backend is `FIPS`:

https://src.fedoraproject.org/rpms/crypto-policies/c/9b9c9f7378c3fd375b9a08d5283c530a51a5de34?branch=master

This doesn't really make sense for us though since the compose server
configuration should be decoupled from the installroot. (More generally,
this also affects e.g. `yum install --installroot`).

Override the script for now so that we always select the `DEFAULT`
policy. We'll discuss with upstream to see what the right solution is
there.

This also works around the fact that rpm-ostree doesn't yet implement
Lua (coreos#749).

Related: https://bugzilla.redhat.com/show_bug.cgi?id=1847454
Resolves: coreos/fedora-coreos-tracker#540
openshift-merge-robot pushed a commit that referenced this issue Jun 17, 2020
The latest crypto-policies package changed recently to dynamically set
the policy at install time so that if FIPS is enabled, the selected
backend is `FIPS`:

https://src.fedoraproject.org/rpms/crypto-policies/c/9b9c9f7378c3fd375b9a08d5283c530a51a5de34?branch=master

This doesn't really make sense for us though since the compose server
configuration should be decoupled from the installroot. (More generally,
this also affects e.g. `yum install --installroot`).

Override the script for now so that we always select the `DEFAULT`
policy. We'll discuss with upstream to see what the right solution is
there.

This also works around the fact that rpm-ostree doesn't yet implement
Lua (#749).

Related: https://bugzilla.redhat.com/show_bug.cgi?id=1847454
Resolves: coreos/fedora-coreos-tracker#540
@jlebon
Copy link
Member

jlebon commented Jun 17, 2020

I'd actually like to have rpm run -p in a newly forked process to protect the main rpm from side-effects, but there are compatibility etc concerns with that.

Would be great if rpm had that. Are there any plans to work through the compat issues there and support this?

OK, I split this out into rpm-software-management/rpm#1273. Let's discuss there how feasible this is before we resort to something a bit more drastic on the rpm-ostree side?

@heyakyra
Copy link

Seems related to fedora-silverblue/issue-tracker#210

is there a workaround? or a way to pinpoint the dependency leading to this?

@travier
Copy link
Member

travier commented Dec 17, 2021

rpm-software-management/rpm#1867 > This one might enable us to do what we need here.

Edit: Apparently it's not enough as we will be missing the execution context.

@martinpitt
Copy link

This now breaks Fedora 36 builds due to the filesystem package using lua in posttrans.

Does anyone know some workaround? Like, can we tell rpm-ostree build to simply ignore these scripts? They don't seem to be critical for building the tree.

@lucab
Copy link
Contributor

lucab commented Aug 8, 2022

@martinpitt #3909

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

No branches or pull requests

8 participants