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

Bootstrapping: xz platform support check is incorrect for OS X Lion, Mountain Lion (macOS 10.7 and 10.8), may be incorrect for certain Linux setups #153

Closed
DeeDeeG opened this issue Jan 29, 2020 · 8 comments · Fixed by #155

Comments

@DeeDeeG
Copy link
Contributor

DeeDeeG commented Jan 29, 2020

Hi,

I did a fair amount of research (see: shadowspawn/nvh#8 (comment)) for implementing an xz platform detection function over at shadowspawn/nvh and tj/n.

It turns out there is a situation on OS X Lion and Mountain Lion (macOS 10.7 and 10.8), as follows:

macOS version 10.6 10.7 10.8 10.9+
libarchive version 2.6.2 ❌ 2.8.3 ✔️ 2.8.3 ✔️ 2.8.3 ✔️
liblzma shipped? no ❌ no ❌ no ❌ yes ✔️
xz configured on in libarchive? N/A ❌ no ❌ no ❌ yes ✔️
xz actually supported by tar? no ❌ no ❌ no ❌ yes ✔️

This means that the current check in the bootstrap script is wrong for macOS Lion and Mountain Lion (10.7, 10.8 respectively).

nvs/nvs.sh

Lines 168 to 175 in 97315d1

export LIBARCHIVE_VER="$(tar --version | sed -n "s/.*libarchive \([0-9][0-9]*\(\.[0-9][0-9]*\)*\).*/\1/p")"
if [ -n "${LIBARCHIVE_VER}" ]; then
LIBARCHIVE_VER="$(printf "%.3d%.3d%.3d" $(echo "${LIBARCHIVE_VER}" | sed "s/\\./ /g"))"
if [ $LIBARCHIVE_VER -ge 002008000 ]; then
export NVS_USE_XZ=1
else
export NVS_USE_XZ=0
fi

By only checking the libarchive version, this check misses the fact that two minor versions of macOS shipped with libarchive that was new enough to potentially support xz compression/decompression, but nonetheless had support for xz configured off.

I would suggest one of the following:

  • Adding an additional check on macOS to confirm the OS is 10.9 or above...
  • Switching only to a macOS version check (as libarchive/bsdtar are uncommon outside of macOS and BSD...)
  • Leaving as a known issue, since most people are not using OS X Lion and Mountain Lion anymore?
@DeeDeeG
Copy link
Contributor Author

DeeDeeG commented Jan 29, 2020

I hit enter too early, please wait as I finish writing/updating the original comment for this issue... Updated.

tl;dr the libarchive version is high enough to support xz on OS X/macOS starting with Lion (10.7), but llibarchive was configured pre-compile-time to disable the xz feature, and the liblzma/xz library is not shipped.

Starting in OS X/macOS Mavericks (10.9) the xz feature is enabled in libarchive, and the required liblzma library is shipped on the system.

tl;dr is too long, didn't read: macOS only supports xz in Mavericks (10.9) and up.

You may want to add a macOS version check using e.g. sw_vers to properly detect this.

Only checking the libarchive version will miss the fact that Lion and Mountain Lion (10.7, 10.8) don't support xz.

@DeeDeeG
Copy link
Contributor Author

DeeDeeG commented Jan 29, 2020

Oh, and for that matter: GNU tar (generally found on Linux) usually uses the xz program from the system PATH.

Some older Linux distributions ship with new enough GNU tar that xz is supported by tar, but the distro doesn't ship the xz program by default or have xz available in the repos.

So this check may be incorrect for certain distros released around 2009, or any newer distro that doesn't ship the xz executable, for that matter:

nvs/nvs.sh

Lines 177 to 185 in 97315d1

LIBARCHIVE_VER="$(tar --version | sed -n "s/.*(GNU tar) \([0-9][0-9]*\(\.[0-9][0-9]*\)*\).*/\1/p")"
if [ -n "${LIBARCHIVE_VER}" ]; then
LIBARCHIVE_VER="$(printf "%.3d%.3d%.3d" $(echo "${LIBARCHIVE_VER}" | sed "s/\\./ /g"))"
if [ $LIBARCHIVE_VER -ge 001022000 ]; then
export NVS_USE_XZ=1
else
export NVS_USE_XZ=0
fi
fi

By not checking for xz on the PATH, this check can potentially have a false positive "supports xz" determination, when the system doesn't actually support xz.

tl;dr: Consider checking for xz on the PATH for GNU tar.

@DeeDeeG DeeDeeG changed the title Bootstrapping: xz platform support check is incorrect for OS X Lion, Mountain Lion (macOS 10.7 and 10.8) Bootstrapping: xz platform support check is incorrect for OS X Lion, Mountain Lion (macOS 10.7 and 10.8), may be incorrect for certain Linux setups Jan 29, 2020
@DeeDeeG
Copy link
Contributor Author

DeeDeeG commented Jan 29, 2020

Would be willing to work on a pull request for this if desired.

@DeeDeeG
Copy link
Contributor Author

DeeDeeG commented Jan 29, 2020

To be fair, most modern systems DO support xz decompression with tar, and most very old systems just plain don't, and don't even appear like they would support it; There are probably very few systems that would be detected as false positives that are actually being used with Node today. Just pointing it out, since I did all that research and testing about it, and wanted to spread the word.

@jasongin
Copy link
Owner

I'm open to a PR to fix this, if the fix is not super complicated.

@jasongin
Copy link
Owner

Also, thanks for all the research! I didn't try that many old OS versions when I originally coded that xz check. If nothing else this will informative to anyone else who hits this issue.

@jasongin
Copy link
Owner

BTW, if the xz support detection isn't working for anyone, you can always set NVS_USE_XZ=0 (or 1) to override the detection.

@DeeDeeG
Copy link
Contributor Author

DeeDeeG commented Jan 29, 2020

Will put together a PR when I get the time.

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 a pull request may close this issue.

2 participants