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

nvs.sh: Adjust xz platform checks to be stricter #155

Merged
merged 1 commit into from
Jul 17, 2020

Conversation

DeeDeeG
Copy link
Contributor

@DeeDeeG DeeDeeG commented Jan 29, 2020

macOS only supports extracting xz tarballs with tar in 10.9 and up.

GNU tar needs an xz executable on the PATH to extract xz tarballs.

Fixes #153.

@DeeDeeG DeeDeeG force-pushed the adjust-xz-platform-checks branch from 2f1e9c6 to aee174c Compare January 29, 2020 20:10
macOS only supports extracting xz tarballs with `tar` in 10.9 and up.

GNU tar needs an `xz` executable on the `PATH` to extract xz tarballs.
@DeeDeeG DeeDeeG force-pushed the adjust-xz-platform-checks branch from aee174c to 539f23e Compare January 29, 2020 20:12
@DeeDeeG
Copy link
Contributor Author

DeeDeeG commented Jan 29, 2020

Force pushed to:

  • Fix indentation whitespace (spaces --> tabs)
  • Re-add export NVS_USE_XZ=0 when GNU tar version is too low to support xz (had inadvertently removed it in the first attempt).

@DeeDeeG
Copy link
Contributor Author

DeeDeeG commented Jan 30, 2020

nvs/nvs.sh

Lines 177 to 180 in b6a607d

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

For this part of the script, the variable LIBARCHIVE_VER, which was used above to hold/operate on the version number of libarchive/bsdtar, is re-used to hold/operate on the version of GNU tar. I was tempted to change this variable to GNU_TAR_VER or something like that, for accuracy's sake, but I admit it wouldn't have any impact on the functionality of the script. Not doing that for now, to keep this PR tidy.

(Open to any feedback in general to improve this PR. Or if you'd like to make changes yourself, that is also fine.)

@brody4hire brody4hire mentioned this pull request Jun 26, 2020
@jasongin jasongin merged commit e132972 into jasongin:master Jul 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants