-
Notifications
You must be signed in to change notification settings - Fork 10
Modified os detection refer to ID and VARIANT_ID instead of NAME #12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
I'll try to check this out later, thanks! |
echo "Stopped installation! Please remove ~/deps." | ||
exit 1 | ||
# replace 'ubuntu' with 'debian' and 'rhel' with 'fedora' (among others) | ||
# this has some weird issues on some distros where they list multiple possibilities |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What kind of issues does this cause? Does it extract the name weirdly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ubuntu is a derivative of Debian, so the ID_LIKE has Debian in to. Basically saying "Treat me like debian, if you don't care I'm actually ubuntu". RHEL points to Fedora, Manjaro and steamOS point to Arch. This simplifies the script, because we don't have to have a bunch of cases if we don't care about specifics.
Bazzite (and some others) are weird, and have a bunch of ids here. It lists rhel centos fedora bluefin aurora nobara ultramarine
. If the goal is to not have a giant list of distros and their package managers, how do we figure out that this distro uses dnf as its package manager?
We could iterate until we find something we recognize, but that has some questions about multiple matches and match precedence.
In the end, I ended up dropping the ID_LIKE if it has multiple values. I think this is a decent solution.
I will update the comment to reflect that it just drops it if there are multiple values.
sudo dnf install -y gcc-c++ clang libcxx-devel cmake ninja-build libX11-devel libXcursor-devel libXi-devel mesa-libGL-devel fontconfig-devel git | ||
;; | ||
'apt') | ||
sudo apt-get install -y g++ clang libc++-dev libc++abi-dev cmake ninja-build libx11-dev libxcursor-dev libxi-dev libgl1-mesa-dev libfontconfig1-dev git |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was just thinking about this, should we do a sudo apt update so that the package lists aren't outdated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should for two (albeit weak) reasons:
- The instructions given by Aseprite don't include it.
- The user might intentionally have packages cached already, and does not want to package list updated.
Instead, thoughts on checking the exit code of the install and giving run "sudo apt update"
as a suggestion if it failed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good
Looks good though and I can probably merge this after testing (no guarantees on it being soon though) |
Also added exceptions for know atomic distros. Also added PACKAGE_MANAGER env var
daf5818
to
dfc50e9
Compare
Also added exceptions for known atomic distros.
Also added PACKAGE_MANAGER env var
This addresses #10