chromebrew
chromebrew copied to clipboard
Remove need for essential_deps field in device.json
Description
The definition of what is an 'essential' package has been confused. Essential packages are those that are absolutely required for chromebrew to run, and only chromebrew. If you remove lz4, zstd can no longer decompress .tar.lz binaries-- this is not required for ruby to run, and thus is not an essential package.
As such, the list of essential packages is actually quite short, and the only complexity required is in ensuring we have the right glibc_lib_* or glibc_build_* package.
Tested & Working properly:
- [x]
x86_64 - [x]
i686 - [x]
armv7l
Run the following to get this pull request's changes locally for testing.
CREW_REPO=https://github.com/Zopolis4/chromebrew.git CREW_BRANCH=ughhhh crew update \
&& yes | crew upgrade
Also, we want to make sure zstd isn't removed, because otherwise we are going to get support issues with people who didn't realize they were breaking their systems.
Being able to install packages requires zstd and its dependencies, so I don't understand why zstd isn't essential
zstd is essential, which is why I've kept it as an essential package.
as you can see, removing lz4 does not break the basic functionality of crew, hence why it is not an essential package.
chronos@cheekon-x86_64:2.27 M90 /usr/local/lib/crew/packages (master >|SPARSE)$ upx -d `which zstd`
Ultimate Packer for eXecutables
Copyright (C) 1996 - 2024
UPX 4.2.4 Markus Oberhumer, Laszlo Molnar & John Reiser May 9th 2024
File size Ratio Format Name
-------------------- ------ ----------- -----------
1102781 <- 382512 34.69% linux/amd64 zstd
Unpacked 1 file.
chronos@cheekon-x86_64:2.27 M90 /usr/local/lib/crew/packages (master >|SPARSE)$ ../tools/getrealdeps.rb zstd
Checking for the runtime dependencies of zstd...
Package zstd has runtime library dependencies on these packages:
depends_on 'gcc_lib' # R
depends_on 'glibc' # R
depends_on 'lz4' # R
depends_on 'xzutils' # R
depends_on 'zlib' # R
chronos@cheekon-x86_64:2.27 M90 /usr/local/lib/crew/packages (master >|SPARSE)$ lddtree `which zstd`
/usr/local/bin/zstd (interpreter => /lib64/ld-linux-x86-64.so.2)
libz.so.1 => /usr/local/lib64/libz.so.1
liblzma.so.5 => /usr/local/lib64/liblzma.so.5
liblz4.so.1 => /usr/local/lib64/liblz4.so.1
libpthread.so.0 => /usr/local/lib64/libpthread.so.0
libc.so.6 => /usr/local/lib64/libc.so.6
ld-linux-x86-64.so.2 => /lib64/ld-linux-x86-64.so.2
chronos@cheekon-x86_64:2.27 M90 /usr/local/lib/crew/packages (master >|SPARSE)$
All dependencies of zstd need to be installed...
Otherwise installs break.
All dependencies of zstd need to be installed...
This is not the case. As I have shown, lz4 can be removed and crew will still function.
All dependencies of zstd need to be installed...
This is not the case. As I have shown,
lz4can be removed and crew will still function.
My experience is that installs break.
Please confirm that installs do not break with your changes by passing the REPO and BRANCH flags to the installer after deleting Chromebrew in a container.
We should really just have a unit test for this triggered with any changes in install.sh or to stuff like packages in buildessential and core when packages in those have their dependencies change, just to make sure installs don't break. Would that be overkill?
(Maybe a locally triggered unit test that doesn't burden GitHub's Action servers would make the most sense? Doing a test install takes about 15 min on my systems at most.)
All dependencies of zstd need to be installed...
This is not the case. As I have shown,
lz4can be removed and crew will still function.
Yes, but do fresh installs still work with it removed? We cannot break fresh installs under any circumstances or we will antagonize new users and create a stampede of incoming issues.
(I'm not trying to be dogmatic for the sake of being dogmatic — just being overly protective because it is annoying to track down breakage after the fact. It would be nice to delegate that protectiveness to some automatic mechanism like we have done with our other unit tests, because refactoring can be really nice to streamline and cut out excess.)
I have just tested a fresh install using this branch, everything works.
I have just tested a fresh install using this branch, everything works.
Just keep in mind that dependencies are lowest common denominator for all of our systems for the most part.
Do installs work on i686 + pre-M126/post-M126 systems on both x86_64 + Arm...
There is a reason that essential packages is currently set to expand to include all dependencies.
If zstd doesn't have dependencies installed, it will try to use system libraries. If those aren't available... I think crew then tries to use the system zstd... That isn't available on the oldest systems.
What i'm trying to convey here is that zstd doesn't need some dependencies in order to function for chromebrew's usage of it. zstd is able to decompress .tar.zst files and run ruby without lz4 installed, so we don't need lz4 for basic functionality.
i.e. we aren't breaking zstd by removing lz4. we might be removing its ability to decompress .tar.lz files, but it still works for ruby's purposes.
Also, I expanded essential deps because installs were failing...
Also, I expanded essential deps because installs were failing...
In what situations were they failing? If I can test this PR and it works in that case, then we should be pretty good.
any update on this? I'd like to get this PR in before it joins the pile of "out of sight out of mind" pull requests that have been waiting on reviews for months
and i've got some cleanup changes after this, including dealing with the warning constant message on crew startup, so i'd like to get those in as well
Will look at this again after I finish this massive python cleanup/update I'm on final approach for...
- What was the reason to remove
generate_compatible? I was using that to refresh crew's state of updatable packages locally without doing a git pull. - Do you want to solve https://github.com/chromebrew/chromebrew/issues/10981 with this work? It will require you to test in the M124 container:
#!/bin/bash
# crewbase-octopus-x86_64.m124.sh
docker pull satmandu/crewbase:octopus-x86_64.m124
docker run --rm -v $(pwd):/output -h $(hostname)-x86_64 -it satmandu/crewbase:octopus-x86_64.m124 /bin/bash
You can do the usual dance to test the install in the container:
passwd -d chronos
echo "chronos ALL=(ALL) NOPASSWD: ALL" >> /etc/sudoers
chown chronos /usr/local
/usr/bin/sudo -E -n -u chronos /bin/bash
cd /tmp
export HOME=/home/chronos/user/
curl -OLf https://github.com/Zopolis4/chromebrew/raw/refs/heads/ughhhh/install.sh
chmod +x install.sh
cd /usr/local
OWNER=Zopolis4 REPO=chromebrew BRANCH=ughhhh /tmp/install.sh
Re: generate_compatible-- I don't see anything in that code that does anything other than check essential deps and package compatibility-- what exactly was your usecase?
I'll check out that other bug soon, swapped to a new laptop and distro and so am still setting up my development environment.
Unit tests appear to be hanging-- I'll look into this when I get back.
It looks like removing packages previously deemed essential, such as gcc_lib, is breaking the container unit tests that check to ensure that trying to remove essential packages doesn't break things.
FIxed-- the issue was just about using yes | ... instead of crew -f in the unit tests.
It looks like removing packages previously deemed essential, such as
gcc_lib,is breaking the container unit tests that check to ensure that trying to remove essential packages doesn't break things.
gcc_lib is still listed as an essential package and cannot be removed.