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
signature.asc
Description: PGP signature
