Re: [gentoo-dev] rfc: usrmerge script

[Michał Górny]

On Wed, 2021-03-24 at 00:20 -0500, William Hubbs wrote:
> On Tue, Mar 23, 2021 at 10:23:11AM +0100, Michał Górny wrote:
> > On Sun, 2021-03-21 at 12:39 -0500, William Hubbs wrote:
> > > All,
> > >
> > > the following is a script which will migrate a Gentoo system to the usr
> > > merge layout. This is similar to the unsymlink-lib tool used to migrate
> > > a system  from the 17.0 to the 17.1 profiles.
> > >
> > > I'm attaching it here to get some comments before I package it, so
> > > please let me know if I have missed something.
> >
> > To be honest, I don't think critical system modifications should be done
> > in shell script, and especially not via a fringe non-standards complaint
> > shell implementation in busybox.  Even if you can assume you make no
> > mistakes, shell is unreliable by design.  For example, your script may
> > start behaving in unexpected ways if you run out of space.
>
> I went with busybox shell in this case because busybox is a static
> binary and I figured with everything being moved around on the system it
> would be a good choice.
>
> I could pretty easily go with straight sh/bash, I just was focusing on
> bb since the commands are built in.
>
> You are right about shell behaving in weird ways if you run out of
> space, but that's true for anything that happens to be running when a
> system runs out of space.

This is not true.  Most of the programming languages can behave sanely
in out-of-space conditions.  Sure, I/O fails one way or another but if
it's just a matter of proper error handling.  Shells on the other hand,
tend to use temporary files under the hood and can misbehave
in unexpected ways.

> That is why I put the rollback directory in /var by
> default figuring there is more space there than in /, especially on
> systems with separate /var.

What really can help is reflinking on filesystems supporting that.

> The run_command wrapper in the script causes an immediate exit when the
> command it runs fails so things don't go any
> further than the failing command, so I'm not sure what else I can do
> with this situation. One option is to always echo the command we are
> about to run then just not run it if -d is specified. That means you
> would always see the commands the script is running.

Did you see the verbose explanatory messages unsymlink-lib produces
on every error condition?  Compared to this, your script is printing
Windows-level 'error figure-it-out-yourself'.

> > You don't seem to be handling file collisions at all.  Even today we
> > have files like /bin/bzip2 and /usr/bin/bzip2, not to mention shared
> > libraries.  Silently ignoring the problem or requiring the users to
> > manually ensure their system is clean is not going to solve it.

> I have added -i to the cp commands in my latest version of this so I'll
> see when we have duplicate names, and yes that is an issue, even without
> the /usr merge.

I don't think that's the best UI for this possible.  I doubt that
a random user running the script will figure out why it is asking about
file replacements.

> I'm curious why we are duplicating all of these names in the first
> place honestly.
>
> One example of this is in coreutils, and we even say in the ebuild
> comment that we need to figure out why we are doing it.
>
> There are a couple of ways forward for this.
>
> 1) attempt to open bugs on the packages and remove the duplicate names
> by dropping symlinks and putting the files in their cannonical
> locations.  this could lead  to breakages if one of the alternate names is
> expected by something until the /usr merge is done.
>
> 2) try to guess at resolving the duplicate names as part of the
>    usr merge process.
>
> a. symmlinks in /bin or /sbin that point to the same name in /usr/bin or
> /usr/sbin should be removed.
> b. symlinks in /usr/bin or /usr/sbin that point to the same name in /
> should be removed.
> c. Other symlinks that I can think of should be preserved.
>
> Which path for handling this is best? Do you have any thoughts?

This is a thing that should have been researched before writing
a script.

One case I'm aware of is using symlinks to replace tools with
alternatives, e.g. /usr/bin/bzip2 -> lbzip2 (/bin/bzip2 remains
as the original).

I don't know what's the best way forward.

>
> > You don't seem to provide any helpful messages.  When things fail, user
> > will be left in the blue with an error message from some system tool (or
> > rather, cheap-ass busybox rewrite, I guess).
>
> If the rollback directory is populated, you can use -r to recover,
> and the script will not let you perform the /usr merge if the rollback
> directory is not populated.
>
> > Also, have you verified that busybox's cp(1) actually preserves all file
> > properties (including xattrs, ACLs, caps...)?
>
> The documentation for busybox states that -p preserves file attributes
> if possible,  and by doing ls -l in the chroot I can see that
> ownership/permissions are preserved. So, logically I would guess it
> preserves the others.

I don't see how you would come to this conclusion.  Just because some
tool is doing the absolute minimum it doesn't mean it does anything
else, and especially operations that require additional syscalls.

> If switching back to non-busybox is fine for this operation, I have no
> problem doing that either.
>
> > Please don't forget to include tests with it.  Docker's good for testing
> > stuff like this.
>
> Docker can't be used during src_test that I'm aware of, so I'm not sure
> how Docker could be used to test this.

You can test it outside of ebuild.  Some automated testing is better
than no testing at all.

-- 
Best regards,
Michał Górny