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

Conversation

Projects
None yet
4 participants
@aramisf
Copy link

commented Dec 31, 2018

Hello all,

In order to download and unpack node source files under OpenBSD it is necessary to execute the extraction in two steps. Since OpenBSD's tar is not integrated with xz (as GNU tar is), the -J option is not available.

This commit adds a function that figures this out and handles the proper file extraction under OpenBSD, removing GNU tar's dependency. The previous behaviour for other operating systems should stay untouched (more testing and feedback are welcome) (and maybe aix can also benefit from it, I don't know).

This PR does not handles the next issue, which is to compile certain versions of Node.js under OpenBSD. This issue, a bit more specific, is related to SSL libraries and requires more attention.

I'm creating the pull request to ask you, if it makes sense to add this simple feature, even though the process is not entirely automated.

Does it?

Notes:

  • I did not add any tests because I wasn't sure what exactly should be tested in this case;
  • OpenBSD versions previous to 6.4 were not tested.
Adds support for OpenBSD.
In order to download and unpack node source files under OpenBSD it is
necessary to run the extraction into two steps. Since `tar` and `xz` are
not integrated, the `-J` option is not available on OpenBSD default
`tar` tool.

This commit adds a test in order to figure this out, and handles the
proper file extraction under OpenBSD. The previous behaviour for other
Operating Systems should be untouched (more testing is welcome).

This commit does not yet handles the next issue, which is to compile
certain versions of node under OpenBSD. This issue is a bit more
delicated, since OpenBSD uses LibreSSL, and node support for it is still
not complete.
@ljharb
Copy link
Collaborator

left a comment

Thanks, I think this is a great direction.

Show resolved Hide resolved nvm.sh Outdated
Show resolved Hide resolved nvm.sh Outdated
nvm.sh Outdated

tar_compression_flag='z'

if nvm_supports_xz "${VERSION}" && [ "_$NVM_OS" != "_openbsd" ]; then

This comment has been minimized.

Copy link
@ljharb

ljharb Dec 31, 2018

Collaborator

Is there any way to determine -J support that doesn't involve checking the OS? (ie, feature test instead of user agent sniff)

This comment has been minimized.

Copy link
@aramisf

aramisf Jan 2, 2019

Author

We could run tar -J and check if the stderr's first line matches:

tar: unknown option -- J

But I have mixed feelings about this, haha. What do you think?

This comment has been minimized.

Copy link
@ljharb

ljharb Jan 2, 2019

Collaborator

lol, i mean, if every single possible version of tar will either error with the same error message, or support -J, then that's pretty robust

This comment has been minimized.

Copy link
@aramisf

aramisf Jan 4, 2019

Author

I couldn't test all possible versions, but GNU tar 1.28 on Linux and bsdtar 2.8.3 on macosx all provide sufficiently clear error messages to understand whether the support for J option exists.

I'm adding a test to the specific error string given by tar on OpenBSD, that should solve the issue for OpenBSD. Since I didn't test on other operating systems (other than the ones mentioned above), I'm not sure it is enough to go through it without checking the OS. On the other hand this situation only happens on OpenBSD.

I'm uploading a new version.

Show resolved Hide resolved nvm.sh Outdated
@KFPJ4XL9JMNP

This comment was marked as spam.

Copy link

commented Jan 1, 2019

Thanks

ljharb and others added some commits Jan 2, 2019

Update nvm.sh
Co-Authored-By: aramisf <aramisf@users.noreply.github.com>
Update nvm.sh
Co-Authored-By: aramisf <aramisf@users.noreply.github.com>
Show resolved Hide resolved nvm.sh Outdated

ljharb and others added some commits Jan 4, 2019

Update nvm.sh
Co-Authored-By: aramisf <aramisf@users.noreply.github.com>
@@ -1907,6 +1908,64 @@ nvm_download_artifact() {
nvm_echo "${TARBALL}"
}

nvm_tar_knows_J() {
ERR_MSG=$(tar -J 2>&1 | head -1)
[ "_$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.

Show resolved Hide resolved nvm.sh Outdated
Show resolved Hide resolved nvm.sh
Show resolved Hide resolved nvm.sh Outdated
Show resolved Hide resolved nvm.sh Outdated

ljharb and others added some commits Jan 5, 2019

Update nvm.sh
Co-Authored-By: aramisf <aramisf@users.noreply.github.com>
Update nvm.sh
Co-Authored-By: aramisf <aramisf@users.noreply.github.com>
@tylerdmace

This comment has been minimized.

Copy link

commented Jun 16, 2019

What's the latest here? Anyone know?

@aramisf

This comment has been minimized.

Copy link
Author

commented Jun 17, 2019

Hey @tylerdmace, I've stopped on the point where a fresh node installation breaks. By default it expects to find OpenSSL, but it finds LibreSSL instead.

I did not have time to dig through it since my last commit. Would you like to join this effort?
Are you an expert on those matters, and would like to help?

Cheers.

@ljharb

This comment has been minimized.

Copy link
Collaborator

commented Jun 17, 2019

If anyone wants to help, please do not open a new PR - either @aramisf can add you to their fork, or you can post a link to your commits here and I can pull them in. Thanks!

@tylerdmace

This comment has been minimized.

Copy link

commented Jun 21, 2019

@aramisf Unfortunately, I do not have the time nor energy to spend on this right now. Was just curious if there was active work being done that I just couldn't see. Thanks for the update!

@aramisf

This comment has been minimized.

Copy link
Author

commented Jun 21, 2019

@tylerdmace same here bro, not enough time to dedicate :(
I shall update my branch, at least.. 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.