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

Fix: Make install.sh Alpine linux compatible #623

Merged
merged 2 commits into from
Sep 5, 2017
Merged

Conversation

BYK
Copy link
Member

@BYK BYK commented Sep 4, 2017

Summary

Alpine Linux comes with BusyBox tar, which lacks the --strip option. This patch updates the install.sh script to not rely on this option.

Resolves yarnpkg/yarn#4280.

Test plan

Manually verified on local Ubuntu for Windows.

**Summary**

Alpine Linux comes with BusyBox tar, which lacks the `--strip` option. This patch updates the `install.sh` script to not rely on this option.

**Test plan**

Manually verified on local Ubuntu for Windows.
@BYK BYK requested review from Daniel15 and arcanis September 4, 2017 14:36
@Haroenv
Copy link
Member

Haroenv commented Sep 4, 2017

Deploy preview ready!

Built with commit 525924e

https://deploy-preview-623--yarnpkg.netlify.com

install.sh Outdated
@@ -33,8 +33,7 @@ yarn_get_tarball() {
yarn_verify_integrity $tarball_tmp

printf "$cyan> Extracting to ~/.yarn...$reset\n"
mkdir .yarn
tar zxf $tarball_tmp -C .yarn --strip 1 # extract tarball
temp=$(mktemp -d) && tar zxf $tarball_tmp -C "$temp" && mkdir .yarn && mv "$temp"/*/* .yarn && rm -rf "$temp"
Copy link
Member

Choose a reason for hiding this comment

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

Any particular reason for sticking it all on one line?

temp=$(mktemp -d)
tar zxf $tarball_tmp -C "$temp"
mkdir .yarn
mv "$temp"/*/* .yarn
rm -rf "$temp"

Copy link
Member Author

Choose a reason for hiding this comment

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

Does sheer laziness count? 😛

Update coming.

@Haroenv
Copy link
Member

Haroenv commented Sep 5, 2017

The deploy preview failing is unrelated, I'll take a look later 👍

@Haroenv
Copy link
Member

Haroenv commented Sep 5, 2017

They work now 👌

@BYK
Copy link
Member Author

BYK commented Sep 5, 2017

@Daniel15 if you're good with the latest changes, merge at will!

install.sh Outdated
@@ -33,7 +33,12 @@ yarn_get_tarball() {
yarn_verify_integrity $tarball_tmp

printf "$cyan> Extracting to ~/.yarn...$reset\n"
temp=$(mktemp -d) && tar zxf $tarball_tmp -C "$temp" && mkdir .yarn && mv "$temp"/*/* .yarn && rm -rf "$temp"
# All this dance is because `tar --strip=1` does not work everywhere
Copy link
Member

Choose a reason for hiding this comment

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

I'd specifically mention Alpine - when I encounter this kind of comment I often wonder what environment is causing issues, so that I can check if that's still the case

Copy link
Member Author

Choose a reason for hiding this comment

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

Good thinking but this may not be limited to Alpine. It is mostly all old versions of BusyBox that comes with their own tar.

Copy link
Member

@Daniel15 Daniel15 Sep 5, 2017

Choose a reason for hiding this comment

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

It is mostly all old versions of BusyBox

Hmm, I wonder if I could install Yarn on a router. A large number of routers run BusyBox. It's good for environments with very little RAM 😛

@Daniel15 Daniel15 merged commit fe7611b into master Sep 5, 2017
@Daniel15 Daniel15 deleted the tar-no-strip branch September 5, 2017 21:02
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.

4 participants