Hi again!

I wrote a verification script to test my changes. You can find the script and full console outputs in this gist.

My problem was with sharp library, so I ran my tests with it. Sharp contains a separate installation script which contains a similar issue. I'm going send them a similar PR once we get the issue fixed in prebuild-install.

Here's a table describing what different commands installed. I ran the test scripts on Darwin, Debian and Alpine Linux machines. All machines were running on arm64, but Darwin is using x64 node to verify node on that architecture works as well.

As we can see from the table, on current version of prebuild-install, LIBC or --libc only accepts value "musl". Giving it the value "glibc" will try to download inexsistent "linuxglibc"-package. My changes fix this to download expected "linux" package on all platforms.

It is still depatable what prebuild-install --platform=linux should install. On Debian and Darwin, it installs "linux" packages, but on Alpine Linux it installs "linuxmusl" package. I consider this to be acceptable behavior, since you can now specify --libc=glibc target.

The table also reveals using platform value "linuxmusl" is problematic. Using that platform on Alpine Linux will always fail, since prebuild-install will append the libc musl value after it. It might be better to drop platform "linuxmusl" and only use --platform=linux --libc=musl to get expected results, but that would be a breaking change. We could implement something like

if (rc.platform === 'linuxmusl' && Boolean(rc.libc)) {
  rc.plaform = 'linux'
}

But that feels hacky and error prone. I think a better solution might be to soft-depricate the "linuxmusl" platform and instead instruct the users to define the target libc value in readme. If you want, I can update the readme like this as part of the PR. Important thing is that this PR should not change old expected behavior, at least as far as I can see.

I have a case where our build machines are on Alpine Linux, but we're targeting AWS Lambda (running glibc). I can't currently make the CI machine install correct version of sharp for our production environment with supported methods. If you have any concerns, I'm happy to look into them.

The sensible suggestion to split libc into its own option, rather than merging it into the OS as linuxmusl does, is something I've been expecting for a while, so thank you for making the time to submit this PR.

This will be a semver major change, and, should such a change land here, the logic in sharp and other native modules that rely on prebuild will hopefully change to reflect this also.

Good moring!

How should we proceed with this issue? Would you like me to verify some scenarios, futher update the readme or make some changes to the code?

I think the big change to drop platform linuxmusl completely isn't something a first-time contributor as myself should be doing. These changes should fix the immediate problem that Alpine Linux cannot build non-musl packages by introducing the predictable cross-platform support for --libc=glibc and LIBC=glibc.

Once we get things settled here, I'd like to start opening a draft PR to Sharp's additional install scripts so @lovell can start considering those as well. I would like make the changes to Sharp as similar to these as possible, so users can control the installation there with just one option/environment value.

Reading the issue opened in sharp, I realized this change might only fix Linux platform. I haven't had time to test, but even with these changes, Alpine Linux machine might still generate incorrect platform name for windows (windowsmusl) or darwin (darwinmusl). If that's the case, should I make changes in this PR or create another one?

@joonamo Thanks, yes, it makes sense to consider libc only when the platform is "linux" for now. Happy to have that included as part of this PR as it's very much related.

Hi! I just pushed a change that makes prebuild-install discard libc-value completely when platform is not exactly "linux". As a side effect, --platform=linuxmusl --libc=musl works as well, as does installing win32 and darwin on Alpine Linux.

I only tested this on Darwin and Alpine Linux, but you can find my extended test script and outputs in this gist. I don't have access to Windows machine to test.

Is this semver-minor?

Yes, originally I was concerned this might be a breaking change, but the final implementation appears to be backwards compatible, so I agree with semver-minor.

Hi! Would you still require something from me or are we good to go for merge?

I haven't had time to review but I trust @lovell. Good to go.

@lovell would you like to help out with merging and releasing? If so, what's your npm username? I'll add you

@vweevers https://www.npmjs.com/~lovell

Added!

Note we have a CHANGELOG.md here, which can be updated by hand or with npx hallmark cc add minor. Other than that, it's the usual npm version .. && git push --follow-tags (the tag will trigger a workflow that creates a GitHub release) and npm publish.

Published to npm as v7.1.0, thanks both.