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

[wip] cross-compile prebuilds #572

Closed
wants to merge 5 commits into from
Closed

Conversation

ahdinosaur
Copy link

hi! 😺

here is a pull request using prebuildify-cross to cross-compile prebuilds.

reference: #360

depends on


so far i've only tested linux-armv7, as my internet is slow at the moment and downloading the docker images takes a while.

also, prebuildify doesn't distinguish between versions of arm, so building linux-armv6 and linux-armv7 will both build into prebuilds/linux-arm. i'm not sure if building for another platform (e.g. android) works yet.

package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@ralphtheninja
Copy link
Member

ralphtheninja commented Dec 25, 2018

also, prebuildify doesn't distinguish between versions of arm, so building linux-armv6 and linux-armv7 will both build into prebuilds/linux-arm.

I was thinking maybe this would be a good spot for prebuildify to do a similar arm-dance in the style of what zeromq does.

i'm not sure if building for another platform (e.g. android) works yet.

Maybe we can get some input/feedback from @staltz or someone else building on android. I'd love to help out here as well, if I just knew where to start and how to test it.

package.json Outdated Show resolved Hide resolved
package.json Outdated
@@ -47,6 +48,13 @@
"coverage": "nyc report --reporter=text-lcov | coveralls",
"rebuild": "node-gyp rebuild",
"prebuild": "prebuildify -t 8.14.0 -t electron@3.0.0 --napi --strip",
"prebuild:cross:android-arm": "prebuildify-cross --target android-arm",
"prebuild:cross:android-arm64": "prebuildify-cross --target android-arm64",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ahdinosaur So I guess these scripts is the main entry point for building and needs to be started somewhere, e.g. from .travis.yml. And we should run one Travis job for each one of them we'd like to build for, with different values for --target.

Copy link
Member

@ralphtheninja ralphtheninja Dec 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically, since each of these scripts have a similar structure we could also do something like:

"scripts": {
  "prebuildify-cross": "prebuildify-cross --target $TARGET"
}

And then set $TARGET from within .travis.yml. This makes it more generic and the package.json can settle in shape. Instead it's up to the caller of npm run prebuildify-cross to set the environment variable.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes! if it was easier, we could also make a single prebuild:cross script that expects you to pass in --target ${TARGET}, like npm run prebuild:cross -- --target linux-armv7.

@ralphtheninja
Copy link
Member

I've been trying to dig a little deeper into how arch is handled and especially in conjunction with arm/arm64. Since we want to be able to distinguish between different arm versions we should try to keep this information around as long as possible.

For example, in the case of linux-armv{5,6,7} we lose this information once we give control over to prebuildify-cross since it just sets ARCH=arm before running npm run prebuild (https://github.com/ahdinosaur/prebuildify-cross/blob/67f9fe3af4a45769bb2d01d1176face03b51da8f/build#L141).

What happens if we ignore this and instead set e.g. ARCH=armv7 (for the linux-armv7 case)? To try answering this, lets dig a little bit deeper into prebuildify and how it passes on this value.

It seems to me that it's being used at two places:

  1. while constructing the output path: https://github.com/mafintosh/prebuildify/blob/54d87e9ba3ebd5892fe8ed0a759163954b33a963/index.js#L33
  2. when passing --target_arch into node-gyp: https://github.com/mafintosh/prebuildify/blob/54d87e9ba3ebd5892fe8ed0a759163954b33a963/index.js#L124

The first should be perfectly ok, it's just a path on the file system and this is good since it means we would create prebuilds/linux-armv{5,6,7} folders, which is exactly what we want later on when node-gyp-build tries to find matching prebuilt binaries (given that it can know which type of arm it is, but should be doable now with @vweevers's build-arch module).

The second is slightly more complex since we need to dig further into what node-gyp does with this cli argument. I made a git grep in the node-gyp repository and it seems to me that it's only used for pointing out .lib files in the windows case, which is irrelevant in the linux/android case.

When running npm run prebuildify --verbose in leveldown you see the following output:

gyp verb gyp gyp format was not specified; forcing "make"
gyp info spawn /usr/bin/python2
gyp info spawn args [ '/home/lms/.nvm/versions/node/v10.14.1/lib/node_modules/npm/node_modules/node-gyp/gyp/gyp_main.py',
gyp info spawn args   'binding.gyp',
gyp info spawn args   '-f',
gyp info spawn args   'make',
gyp info spawn args   '-I',
gyp info spawn args   '/home/lms/src/level/leveldown2/build/config.gypi',
gyp info spawn args   '-I',
gyp info spawn args   '/home/lms/.nvm/versions/node/v10.14.1/lib/node_modules/npm/node_modules/node-gyp/addon.gypi',
gyp info spawn args   '-I',
gyp info spawn args   '/home/lms/.node-gyp/8.14.0/include/node/common.gypi',
gyp info spawn args   '-Dlibrary=shared_library',
gyp info spawn args   '-Dvisibility=default',
gyp info spawn args   '-Dnode_root_dir=/home/lms/.node-gyp/8.14.0',
gyp info spawn args   '-Dnode_gyp_dir=/home/lms/.nvm/versions/node/v10.14.1/lib/node_modules/npm/node_modules/node-gyp',
gyp info spawn args   '-Dnode_lib_file=/home/lms/.node-gyp/8.14.0/<(target_arch)/node.lib',
gyp info spawn args   '-Dmodule_root_dir=/home/lms/src/level/leveldown2',
gyp info spawn args   '-Dnode_engine=v8',
gyp info spawn args   '--depth=.',
gyp info spawn args   '--no-parallel',
gyp info spawn args   '--generator-output',
gyp info spawn args   'build',
gyp info spawn args   '-Goutput_dir=.' ]

I noticed some other usage of target_arch inside node-gyp (please correct me if I'm wrong) but seems to be related to some test data in gyp/tools/emacs/testdata/media.gyp.

In closing, I also think we should do the same for the android case, so to summarize we should have the following arm variants:

  • prebuild-cross --target android-armv5 -> PLATFORM=android, ARCH=armv5
  • prebuild-cross --target android-armv6 -> PLATFORM=android, ARCH=armv6
  • prebuild-cross --target android-armv7 -> PLATFORM=android, ARCH=armv7
  • prebuild-cross --target android-armv8 -> PLATFORM=android, ARCH=armv8
  • prebuild-cross --target linux-armv5 -> PLATFORM=linux, ARCH=armv5
  • prebuild-cross --target linux-armv6 -> PLATFORM=linux, ARCH=armv6
  • prebuild-cross --target linux-armv7 -> PLATFORM=linux, ARCH=armv7
  • prebuild-cross --target linux-armv8 -> PLATFORM=linux, ARCH=armv8

To me it feels better to be able to reason about different arm versions. It also makes it more evident which arm versions a certain native addon supports.

@vweevers
Copy link
Member

@ralphtheninja Perhaps we should check (on an actual ARM machine) that node-gyp has no special behavior for (target_)arch=arm or (target_arch)=arm64 (because that's what it'd get if we didn't pass a custom arch) that we'd be missing out on.

@ralphtheninja
Copy link
Member

ralphtheninja commented Dec 25, 2018

@ralphtheninja Perhaps we should check (on an actual ARM machine) that node-gyp has no special behavior for (target_)arch=arm or (target_arch)=arm64 (because that's what it'd get if we didn't pass a custom arch) that we'd be missing out on.

Yeah, also, if this is a must we can still name the output folder using armvX but still pass in arm and arm64 to node-gyp. Since the name of the folder has nothing to do with that.

This makes me confident we can pull this off.

@ralphtheninja
Copy link
Member

ralphtheninja commented Dec 25, 2018

I have to back a little bit on my previous post. We still need to keep TARGET intact since that maps to the corresponding dockcross/$TARGET docker images (https://hub.docker.com/search?q=dockcross&type=image), which is passed in here https://github.com/ahdinosaur/prebuildify-cross/blob/67f9fe3af4a45769bb2d01d1176face03b51da8f/build#L200 when building the image, which then points out the FROM here https://github.com/ahdinosaur/prebuildify-cross/blob/67f9fe3af4a45769bb2d01d1176face03b51da8f/Dockerfile#L3.

Gah this is annoying.

@ralphtheninja
Copy link
Member

Given the following (base) docker images for all arm variants (see list here https://github.com/dockcross/dockcross#cross-compilers):

  • dockcross/android-arm
  • dockcross/android-arm64
  • dockcross/linux-armv5
  • dockcross/linux-armv6
  • dockcross/linux-armv7
  • dockcross/linux-arm64

Would then mean:

  • prebuild-cross --target android-arm -> PLATFORM=android, ARCH=armv? (which version of arm does android-arm corresponds to?)
  • prebuild-cross --target android-arm64 -> PLATFORM=android, ARCH=armv8
  • prebuild-cross --target linux-armv5 -> PLATFORM=linux, ARCH=armv5
  • prebuild-cross --target linux-armv6 -> PLATFORM=linux, ARCH=armv6
  • prebuild-cross --target linux-armv7 -> PLATFORM=linux, ARCH=armv7
  • prebuild-cross --target linux-arm64 -> PLATFORM=linux, ARCH=armv8

@vweevers
Copy link
Member

I was wondering about those dockcross names. Is it simply a case of inconsistent naming, or is it following some preexisting naming scheme from the linux world?

@ralphtheninja
Copy link
Member

I was wondering about those dockcross names. Is it simply a case of inconsistent naming, or is it following some preexisting naming scheme from the linux world?

That's a good question. No idea about the naming "convention" used here. It doesn't seem that consistent to me ;)

@ralphtheninja
Copy link
Member

ralphtheninja commented Dec 25, 2018

Another option we have is to create our own consistency layer on top of this. One way to achieve this is to ignore the dockcross/foobar naming and create our own prebuildify-cross/XYZ docker images, which corresponds to the targets we want to use (which layers on top of dockcross/foobar as right now). We could pre build those docker images and put on hub.docker.com.

@ahdinosaur
Copy link
Author

We still need to keep TARGET intact since that maps to the corresponding dockcross/$TARGET docker images

@ralphtheninja i just made up the behavior for prebuildify-cross, happy to make up different behavior. as in, if instead of --target corresponding to the dockcross image names, i could change it so we pass in --arch and --platform corresponding to our own preference (and within the script derive TARGET).

@ahdinosaur
Copy link
Author

ahdinosaur commented Dec 25, 2018

I was wondering about those dockcross names. Is it simply a case of inconsistent naming, or is it following some preexisting naming scheme from the linux world?

some references to the preexisting naming schemes: Rust Platform Support, Debian Tuples

these are all what are called "target triples"

@ahdinosaur
Copy link
Author

and while i'm sharing links, here's an impressive (and useful) project in Rust land: https://github.com/rust-embedded/cross. it supports pretty much every target triple using a separate Dockerfile for each target.

@vweevers
Copy link
Member

There's no consistency to it. I tried to map it..

image

@ahdinosaur
Copy link
Author

i found another project that solved this problem: lovell/sharp#581, https://github.com/lovell/sharp/blob/db4df6f0b2f5e900d1ff06c36b50b726641178d1/lib/platform.js

i like their approach, which i'll translate to:

(changing --armv to more verbose --arm-version)

prebuildify-cross --arch=arm --arm-version=7 --platform=linux

then prebuildify-cross would pass down:

  • ARCH=arm
  • ARM_VERSION=7
  • PLATFORM=linux
  • TARGET=linux-armv7

i think i prefer this because then we can keep ARCH the same as what Node.js uses, to reduce any potential interference with node-gyp.

@ahdinosaur
Copy link
Author

ahdinosaur commented Dec 25, 2018

for completeness, i'm proposing:

  • prebuildify-cross --platform=linux --arch=arm --arm-version=5 -> TARGET=linux-armv5
  • prebuildify-cross --platform=linux --arch=arm --arm-version=6 -> TARGET=linux-armv6
  • prebuildify-cross --platform=linux --arch=arm --arm-version=7 -> TARGET=linux-armv7
  • prebuildify-cross --platform=linux --arch=arm64 -> TARGET=linux-arm64 ARM_VERSION=8
  • prebuildify-cross --platform=android --arch=arm --arm-version=6 -> TARGET=android-arm
  • prebuildify-cross --platform=android --arch=arm --arm-version=7 -> TARGET=android-arm
  • prebuildify-cross --platform=android --arch=arm64 -> TARGET=android-arm64 ARM_VERSION=8

@ralphtheninja
Copy link
Member

ralphtheninja commented Dec 25, 2018

for completeness, i'm proposing:

Now we're getting somewhere. Love the idea. Also, @lovell is the one that wrote the proposal on prebuild that I linked to earlier in #360 , maybe he wants to chime in here as well.

This PR contains a lot of goodies already. We should shake out the gems and use for documentation!

@ralphtheninja
Copy link
Member

@vweevers We're going to need a matrix in .travis.yml. Feel like cooking up/thinking about what we need to do here?

ahdinosaur added a commit to prebuild/prebuildify-cross that referenced this pull request Dec 25, 2018
@ahdinosaur
Copy link
Author

okay, updated prebuildify-cross and this pull request to match the changes i proposed. ✨

also happy solstice holidays to everyone! ☀️ 🌈 ❄️ 🌕

@ralphtheninja
Copy link
Member

ralphtheninja commented Dec 26, 2018

@ahdinosaur Just need to get ARM_VERSION env var into prebuildify and make a special case for it when naming the currently named ${CWD}/prebuilds-${PLATFORM}-${ARCH} folder.

EDIT: And node-gyp-build needs to take this folder scheme into account as well (using build-arch module).

@vweevers
Copy link
Member

@vweevers We're going to need a matrix in .travis.yml. Feel like cooking up/thinking about what we need to do here?

A single job that builds all cross-builds might be faster, because they need to pull docker images (between 300 and 600MB each) that have a common base. So shared layers will only have to be downloaded once.

@vweevers
Copy link
Member

vweevers commented Dec 26, 2018

@ahdinosaur Just need to get ARM_VERSION env var into prebuildify and make a special case for it when naming the currently named ${CWD}/prebuilds-${PLATFORM}-${ARCH} folder.

EDIT: And node-gyp-build needs to take this folder scheme into account as well (using build-arch module).

Do we stick with armv8 for the folder name, or arm64? The latter seems more common (I guess because armv8 could mean both 32- and 64 bits instruction sets; arm64 is less ambiguous).

@ahdinosaur
Copy link
Author

ahdinosaur commented Dec 26, 2018

@vweevers

@ahdinosaur Just need to get ARM_VERSION env var into prebuildify and make a special case for it when naming the currently named ${CWD}/prebuilds-${PLATFORM}-${ARCH} folder.
EDIT: And node-gyp-build needs to take this folder scheme into account as well (using build-arch module).

Do we stick with armv8 for the folder name, or arm64? The latter seems more common (I guess because armv8 could mean both 32- and 64 bits instruction sets; arm64 is less ambiguous).

this question has been on my mind as well. arm64 is definitely the common way to refer to armv8, but do we need to prepare for a hypothetical armv9 which is also 64-bit but a different compilation target? i wonder if for arm prebuilds, we should do ${CWD}/prebuilds/${PLATFORM}-${ARCH}-v${ARM_VERSION}. so for example: linux-arm-v7 and linux-arm64-v8.

@vweevers
Copy link
Member

i wonder if for arm prebuilds, we should do ${CWD}/prebuilds/${PLATFORM}-${ARCH}-v${ARM_VERSION}. so for example: linux-arm-v7 and linux-arm64-v8.

I kinda like that, because it means we're not changing the existing arch part of the name (which might break things for other users of prebuildify / node-gyp-build). We're merely adding a suffix that further differentiates it.

@ralphtheninja
Copy link
Member

When are we going to start disagreeing with each other? This is too easy! 😁

This was referenced Dec 26, 2018
@vweevers
Copy link
Member

Something to take into consideration: if we later want alpine/musl prebuilds (#388), we'll need another suffix. The open proposal at prebuild/node-gyp-build#9 uses the scheme ${PLATFORM}${LIBC}-${ARCH}.

Doesn't conflict with the arm version suffix, so just FYI.

@ahdinosaur
Copy link
Author

ahdinosaur commented Dec 26, 2018

by the way, when i said "so far i've only tested linux-armv7" in the original post, i actually meant i successfully built for linux-armv7, i never actually ran the prebuild to see if it worked. 😅

well i'm happy to report that using mafintosh/raspberry-pi-container i successfully ran the cross-compiled prebuild. 🎉 😺

@vweevers
Copy link
Member

I'll take a stab at a PR for node-gyp-build that tackles both libc and arm flavors, just to familiarize myself with the problem space.

What tasks are left here? Besides the travis setup, which we can do in a separate PR.

@lovell
Copy link

lovell commented Dec 26, 2018

Hello, the proposal to use Node's existing arm and arm64 to differentiate AArch32 and AArch64 plus the addition of the arm_version as a separate suffix all makes complete sense.

I notice the internals of sharp were mentioned, which currently converts arm64 to armv8, but this will change to arm64v8 in the very near future.

I suspect there will be separate versions of Node targetting ARMv8 vs ARMv8.3 so the approach being discussed here is future-proof too.

If you've not already seen them, and should anyone find them useful, the Docker containers used by sharp/libvips for ARM cross-compilation are at https://github.com/lovell/sharp-libvips

@vweevers
Copy link
Member

@ralphtheninja Can we close this in favor of #584?

@ralphtheninja
Copy link
Member

@ralphtheninja Can we close this in favor of #584?

Yep.

@vweevers vweevers closed this Jan 26, 2019
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 this pull request may close these issues.

4 participants