Allow --installroot on read-only bootc system

[ppisar]

Some people use --installroot on a read-only bootc system to install a system into a chroot subtree. However, current bootc check did not take into account --installroot and rejected the operation.

This patch augments the check for a writable installroot being different from /.

The comparison for / is there to deal with bootc systems with a writeable /. Currently it's uncerain whether systems like that exist.

Resolves: #2108

[pep8speaks]

Hello @ppisar! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2024-08-22 07:46:47 UTC

[travier]

Checking for os.path.realpath(self.conf.installroot) == "/" should be sufficient. The _is_bootc_host function already checks for the writable case (in #2110).

[jan-kolarik]

Checking for os.path.realpath(self.conf.installroot) == "/" should be sufficient. The _is_bootc_host function already checks for the writable case (in #2110).

But in this case, I believe the check is happening with a custom installroot on an otherwise read-only system. The _is_bootc_host function only checks the permissions of /usr.

[ppisar]

The reason why I added a check for self.conf.installroot writability is that dnf --instalroot /usr/myroot on read-only bootc system would not fail with the new "Error: system is configured to be read-only" message.

However, even my currently proposed code does not cover this case because the check happens too late, before confirming a transaction but after DNF attempting to update repositories and computing the transaction; DNF still reports:

# dnf --installroot /usr/foo install bash
error: cannot open Packages database in /usr/foo/usr/share/rpm
Error: Error: rpmdb open failed

@travier, you are right that the new or not os.access(self.conf.installroot, os.W_OK) clause is useless here.

Even if we moved the check before updating the repositories, it wouldn't catch all cases: E.g. "dnf --installroot /opt/foo" wouldn't work because /opt is not made writable by "bootc usr-overlay".

I guess we need to admit that we cannot cover all cases and if the user is smart enough to use --installroot option, he's also smart enough to diagnose why DNF cannot write. (Why librpm does report "read-only filesystem" properly is another issue.)

I will remove the clause and keep it as dnf.util._is_bootc_host() and os.path.realpath(self.conf.installroot) == "/" to unblock the use case of --installroot.

[jan-kolarik]

The reason why I added a check for self.conf.installroot writability is that dnf --instalroot /usr/myroot on read-only bootc system would not fail with the new "Error: system is configured to be read-only" message.

However, even my currently proposed code does not cover this case because the check happens too late, before confirming a transaction but after DNF attempting to update repositories and computing the transaction; DNF still reports:

# dnf --installroot /usr/foo install bash
error: cannot open Packages database in /usr/foo/usr/share/rpm
Error: Error: rpmdb open failed

@travier, you are right that the new or not os.access(self.conf.installroot, os.W_OK) clause is useless here.

Even if we moved the check before updating the repositories, it wouldn't catch all cases: E.g. "dnf --installroot /opt/foo" wouldn't work because /opt is not made writable by "bootc usr-overlay".

I guess we need to admit that we cannot cover all cases and if the user is smart enough to use --installroot option, he's also smart enough to diagnose why DNF cannot write. (Why librpm does report "read-only filesystem" properly is another issue.)

I will remove the clause and keep it as dnf.util._is_bootc_host() and os.path.realpath(self.conf.installroot) == "/" to unblock the use case of --installroot.

OK, thanks for the explanation.