Skip to content
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

Additional arch install #1562

Merged
merged 6 commits into from
Feb 1, 2024
Merged

Additional arch install #1562

merged 6 commits into from
Feb 1, 2024

Conversation

tlockney
Copy link
Contributor

Took a stab at supporting the additional architectures in the generated install script. I admit I haven't thoroughly tested this by any means and the shell scripting probably leads a lot to be desired, but perhaps it can be a jumping off point, at least.

@jdx
Copy link
Owner

jdx commented Jan 31, 2024

there is a script you can use to test this

install_path="${MISE_INSTALL_PATH:-$HOME/.local/bin/mise}"
install_dir="$(dirname "$install_path")"
tarball_url="https://github.com/jdx/mise/releases/download/${version}/mise-${version}-${os}-${arch}.tar.gz"
if [ -z "$musl" ]; then
tarball_url="https://github.com/jdx/mise/releases/download/${version}/mise-${version}-${os}-${arch}-${musl}.tar.gz"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if you just made musl part of $arch it'll clean up a lot of this logic

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Also made the code to check for musl slightly cleaner.

if [ -z "$libc" ]; then
echo "musl"
else
echo ""
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can just get rid of these else conditions and do nothing

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got rid of this entirely. Moving the check to get_arch() cleaned things up a bit.

@@ -46,6 +59,8 @@ get_arch() {
echo "x64"
elif [ "$arch" = aarch64 ] || [ "$arch" = arm64 ]; then
echo "arm64"
elif [ "$arch" = armv7l ]; then
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

go ahead and add "armv6l" idk if it will work, but it's not like not trying to handle it would do anything useful anyways

of course if you want to find a way to test it that's great but I don't really mind

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ha, I actually thought I had added that earlier, but it's there now.

@jdx jdx merged commit bbaf112 into jdx:main Feb 1, 2024
5 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants