From: William Hubbs <williamh@gentoo.org>
To: gentoo-dev@lists.gentoo.org
Cc: mgorny@gentoo.org
Subject: Re: [gentoo-dev] rfc: usrmerge script
Date: Wed, 24 Mar 2021 10:09:35 -0500 [thread overview]
Message-ID: <YFtWL8Mbrq1XaBDG@linux1.home> (raw)
In-Reply-To: <ff811abd4e7bdfae50a9cd3ec770447906dd0a7b.camel@gentoo.org>
[-- Attachment #1: Type: text/plain, Size: 7686 bytes --]
On Wed, Mar 24, 2021 at 08:48:41AM +0100, Michał Górny wrote:
> 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.
Another reason I went with bb is there are no deps outside of bb and the
script. Also bb is on every gentoo system.
In short, I went for something that I knew would be everywhere.
If I'm going to do the work, it isn't going to be in python because I'm
not a fan. I can write some in python, but I don't consider my skills
there to be the best, and the indentation requirements make it tedious
for me to debug.
> > 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.
What really can help is more info instead of being terse like this.
Which filesystems support it?
> > 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'.
This is early in development. Ideally I would handle most of the error
conditions before people start using this on production systems; that is
why I put it out here.
> > > 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.
You are probably right, that's why I put this out here, and
listed my thoughts about it below to generate discussion.
> > 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.
With all respect, knock it off. I'm not going to sit and read through
all of the ebuilds in the tree looking for every little thing.
> 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).
Your example doesn't exist here, so I wouldn't have known about it if
you hadn't mentioned it.
It looks like that specific situation is controlled by the
symlink use flag in lbzip, so it should be ok.
> 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.
Hmm, I think a better response here would have been to point out how I
could look at the other attributes of a file you are talking about.
> > 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.
With all respect, I think most of this email was sarcasm, trolling etc
instead of the type of constructive feedback we look for when sharing
code on the list. I will do what I can to use parts of it, but I really
have issues with your tone.
William
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
next prev parent reply other threads:[~2021-03-24 15:09 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-21 17:39 [gentoo-dev] rfc: usrmerge script William Hubbs
2021-03-21 18:00 ` Matthias Maier
2021-03-21 19:08 ` Luigi Mantellini
2021-03-21 19:27 ` William Hubbs
2021-03-21 19:22 ` William Hubbs
2021-03-23 9:23 ` Michał Górny
2021-03-24 5:20 ` William Hubbs
2021-03-24 7:48 ` Michał Górny
2021-03-24 15:09 ` William Hubbs [this message]
2021-03-24 17:09 ` Rich Freeman
2021-03-24 18:37 ` William Hubbs
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YFtWL8Mbrq1XaBDG@linux1.home \
--to=williamh@gentoo.org \
--cc=gentoo-dev@lists.gentoo.org \
--cc=mgorny@gentoo.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox