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

Adds support for OpenBSD's native tar #1972

Open
wants to merge 9 commits into
base: master
from
78 nvm.sh
@@ -1582,6 +1582,7 @@ nvm_get_os() {
Darwin\ *) NVM_OS=darwin ;;
SunOS\ *) NVM_OS=sunos ;;
FreeBSD\ *) NVM_OS=freebsd ;;
OpenBSD\ *) NVM_OS=openbsd ;;
AIX\ *) NVM_OS=aix ;;
esac
nvm_echo "${NVM_OS-}"
@@ -1907,6 +1908,64 @@ nvm_download_artifact() {
nvm_echo "${TARBALL}"
}

nvm_tar_knows_J() {
ERR_MSG=$(tar -J 2>&1 | head -1)
This conversation was marked as resolved by ljharb

This comment has been minimized.

Copy link
@ljharb

ljharb Jan 4, 2019

Collaborator

maybe this would be better, since we know for sure that openbsd won't support it?

Suggested change
ERR_MSG=$(tar -J 2>&1 | head -1)
if [ "$(nvm_get_os)" = 'openbsd' ]; then
return 1;
fi
ERR_MSG=$(tar -J 2>&1 | head -1)

This comment has been minimized.

Copy link
@aramisf

aramisf Jan 5, 2019

Author

Hm, not sure man. We just moved away from OS verification.

This comment has been minimized.

Copy link
@ljharb

ljharb Jan 5, 2019

Collaborator

lol yes, i'm mainly thinking about not limiting the scope of the fix to bsd. but i guess it's valid that if bsd starts shipping -J support, we'd want it to start working

This comment has been minimized.

Copy link
@aramisf

aramisf Jan 9, 2019

Author

@ljharb sorry for late reply. In that case, we still don't need the suggested change, correct?

This comment has been minimized.

Copy link
@ljharb

ljharb Jan 9, 2019

Collaborator

yeah, agreed, let's keep it this way for now

[ "_$ERR_MSG" != "_tar: unknown option -- J" ]

This comment has been minimized.

Copy link
@ljharb

ljharb Jan 4, 2019

Collaborator

interestingly enough, on my Mac, tar -J gives tar: Must specify one of -c, -r, -t, -u, -x, but tar --help | grep '\-J' shows that it's supported.

This comment has been minimized.

Copy link
@aramisf

aramisf Jan 5, 2019

Author

Yes, I agree that this message does not states clearly that -J is not an unknown option.
I had to explore error messages a bit more in order to figure it out, like trying tar -Q. The error message differs.

Also, long options like --help are not available on OpenBSD.

Besides being very specific to OpenBSD, the error message is clear enough when we need to understand that -J is not supported.

}


# args: tarball path, NVM_OS var
nvm_extract_tar_artifact() {
local tar_compression_flag
local TARBALL
local TAR_COMMAND_SUFFIX
local TMPDIR
local NVM_OS
local tar

TARBALL="${1}"
NVM_OS="${2}"

tar='tar'
if [ "${NVM_OS}" = 'aix' ]; then
tar='gtar'
fi

tar_compression_flag='z'

if nvm_supports_xz "${VERSION}" && nvm_tar_knows_J; then
tar_compression_flag='J'
This conversation was marked as resolved by ljharb

This comment has been minimized.

Copy link
@ljharb

ljharb Jan 4, 2019

Collaborator

let's move this if block into the else below, since that's the only path it matters.

This comment has been minimized.

Copy link
@aramisf

aramisf Jan 5, 2019

Author

Ok, I'll try to upload it today.

This comment has been minimized.

Copy link
@aramisf
fi

if [ "_$NVM_OS" = "_openbsd" ] && nvm_has xz && nvm_supports_xz "${VERSION}"; then
This conversation was marked as resolved by aramisf

This comment has been minimized.

Copy link
@ljharb

ljharb Jan 4, 2019

Collaborator
Suggested change
if [ "_$NVM_OS" = "_openbsd" ] && nvm_has xz && nvm_supports_xz "${VERSION}"; then
if ! nvm_tar_knows_J && nvm_has xz && nvm_supports_xz "${VERSION}"; then

This comment has been minimized.

Copy link
@aramisf

aramisf Jan 5, 2019

Author

Good one! :)

if [ -f "${TARBALL}" ]; then
command xz -dk "${TARBALL}"
fi

TARBALL=${TARBALL%.xz}
tar_compression_flag=
TAR_COMMAND_SUFFIX=
TMPDIR="$(dirname "${TARBALL}")"

else
TAR_COMMAND_SUFFIX="--strip-components 1"
TMPDIR="$(dirname "${TARBALL}")/files"
fi

if [ -f "${TARBALL}" ] && [ -d "${TMPDIR}" ]; then
# shellcheck disable=SC2086
command ${tar} -x${tar_compression_flag}f ${TARBALL} -C ${TMPDIR} ${TAR_COMMAND_SUFFIX}
This conversation was marked as resolved by aramisf

This comment has been minimized.

Copy link
@ljharb

ljharb Jan 4, 2019

Collaborator
Suggested change
command ${tar} -x${tar_compression_flag}f ${TARBALL} -C ${TMPDIR} ${TAR_COMMAND_SUFFIX}
command ${tar} "-x${tar_compression_flag}f" "${TARBALL}" -C "${TMPDIR}" ${TAR_COMMAND_SUFFIX}

?

This comment has been minimized.

Copy link
@aramisf
fi


if [ "_$NVM_OS" = "_openbsd" ]; then
BASE=$(basename "${TMPDIR}")
TMPDIR="${TMPDIR}/${BASE}"
fi

nvm_echo "$TMPDIR"
}

nvm_get_make_jobs() {
if nvm_is_natural_num "${1-}"; then
NVM_MAKE_JOBS="$1"
@@ -2004,7 +2063,7 @@ nvm_install_source() {
make='gmake'
MAKE_CXX="CC=${CC:-cc} CXX=${CXX:-c++}"
;;
'darwin')
'openbsd' | 'darwin')
MAKE_CXX="CC=${CC:-cc} CXX=${CXX:-c++}"
;;
'aix')
@@ -2018,18 +2077,6 @@ nvm_install_source() {
fi
fi

local tar_compression_flag
tar_compression_flag='z'
if nvm_supports_xz "${VERSION}"; then
tar_compression_flag='J'
fi

local tar
tar='tar'
if [ "${NVM_OS}" = 'aix' ]; then
tar='gtar'
fi

local TARBALL
local TMPDIR
local VERSION_PATH
@@ -2045,11 +2092,8 @@ nvm_install_source() {

TARBALL="$(PROGRESS_BAR="${PROGRESS_BAR}" nvm_download_artifact "${FLAVOR}" source "${TYPE}" "${VERSION}" | command tail -1)" && \
[ -f "${TARBALL}" ] && \
TMPDIR="$(dirname "${TARBALL}")/files" && \
TMPDIR="$(nvm_extract_tar_artifact "${TARBALL}" "${NVM_OS}" | command tail -1)" && \
if ! (
# shellcheck disable=SC2086
command mkdir -p "${TMPDIR}" && \
command "${tar}" -x${tar_compression_flag}f "${TARBALL}" -C "${TMPDIR}" --strip-components 1 && \
VERSION_PATH="$(nvm_version_path "${PREFIXED_VERSION}")" && \
nvm_cd "${TMPDIR}" && \
nvm_echo '$>'./configure --prefix="${VERSION_PATH}" $ADDITIONAL_PARAMETERS'<' && \
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.