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.

Sign up for GitHub

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

Jump to bottom

Add linux-arm64, android-arm and android-arm64 prebuilds #587

Merged
merged 25 commits into from
Feb 1, 2019
Merged

Add linux-arm64, android-arm and android-arm64 prebuilds #587

Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
579b4c6
Add linux-arm64, android-arm and android-arm64 prebuilds
vweevers Jan 27, 2019
99aaf92
5.0.0-2-arm
vweevers Jan 27, 2019
92a2b3b
Disable package-lock.json
ralphtheninja Jan 27, 2019
b384ab5
Disable android for now
ralphtheninja Jan 27, 2019
b886ee8
Use plain docker for linux and armv7/arm64
ralphtheninja Jan 27, 2019
821a9fa
Print out file type after build
ralphtheninja Jan 27, 2019
aba704b
5.0.0-2-docker
ralphtheninja Jan 27, 2019
c38859c
Print out users and groups
ralphtheninja Jan 27, 2019
53e8575
Print out environment
ralphtheninja Jan 27, 2019
92bb36e
Only do linux-armv7 for now
ralphtheninja Jan 27, 2019
12c43df
Run docker with user travis on Travis
ralphtheninja Jan 27, 2019
db6e652
5.0.0-2-docker2
ralphtheninja Jan 27, 2019
fac4027
Revert "Only do linux-armv7 for now"
ralphtheninja Jan 27, 2019
14c3c44
Clean up
ralphtheninja Jan 27, 2019
43c07af
Update arm64 script
ralphtheninja Jan 27, 2019
9c76803
5.0.0-2-docker3
ralphtheninja Jan 27, 2019
e057ee6
Add back android armv7
ralphtheninja Jan 27, 2019
6ea1cba
5.0.0-2-docker4
ralphtheninja Jan 27, 2019
08e0d59
Add back android arm64
ralphtheninja Jan 27, 2019
1435e49
5.0.0-2-docker5
ralphtheninja Jan 27, 2019
b513742
Actually run prebuild-android-arm64
ralphtheninja Jan 28, 2019
7112fe1
5.0.0-2-docker6
ralphtheninja Jan 28, 2019
51c4cfb
Refactor into cross-compile script + remove prebuildify-cross dependency
ralphtheninja Jan 28, 2019
d48e9dd
Revert temporary version
ralphtheninja Jan 28, 2019
dfd15d6
Remove -it
ralphtheninja Feb 1, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .npmrc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
package-lock=false
3 changes: 2 additions & 1 deletion .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ jobs:
- name: arm
os: linux
node_js: node
env: [BUILD_CMD=prebuildify-cross-armv7, BUILD_GROUP=arm]
env: [BUILD_CMD=prebuild-arm, BUILD_GROUP=arm]
if: tag is present

script:
Expand All @@ -35,6 +35,7 @@ after_success:
before_deploy:
- export ARCHIVE_NAME="${TRAVIS_TAG:-latest}-$BUILD_GROUP.tar.gz"
- npm run $BUILD_CMD
- file prebuilds/*/*
- tar -zcvf "$ARCHIVE_NAME" -C prebuilds .

deploy:
Expand Down
7 changes: 5 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@
"hallmark": "hallmark --fix",
"dependency-check": "dependency-check . test/*.js bench/*.js",
"prepublishOnly": "npm run dependency-check",
"prebuildify-cross-armv7": "prebuildify-cross --platform=linux --arch=arm --arm-version=7 -- -t 8.14.0 --napi --strip"
"prebuild-arm": "npm run prebuild-linux-armv7 && npm run prebuild-linux-arm64 && npm run prebuild-android-armv7 && npm run prebuild-android-arm64",
"prebuild-linux-armv7": "IMAGE=linux-armv7 ./scripts/cross-compile",
"prebuild-linux-arm64": "IMAGE=linux-arm64 ./scripts/cross-compile",
"prebuild-android-armv7": "IMAGE=android-armv7 ./scripts/cross-compile",
"prebuild-android-arm64": "IMAGE=android-arm64 ./scripts/cross-compile"
},
"dependencies": {
"abstract-leveldown": "~6.0.0",
Expand All @@ -38,7 +42,6 @@
"optimist": "~0.6.1",
"prebuildify": "prebuild/prebuildify#override-platform",
"prebuildify-ci": "^1.0.4",
"prebuildify-cross": "ralphtheninja/prebuildify-cross#TARGET_PLATFORM",
"readfiletree": "~0.0.1",
"rimraf": "^2.6.1",
"slump": "^3.0.0",
Expand Down
7 changes: 7 additions & 0 deletions scripts/cross-compile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#!/bin/bash

DOCKER_USER="node"
if [[ "$TRAVIS" == "true" ]]; then DOCKER_USER="travis"; fi
Copy link
Member Author

Choose a reason for hiding this comment

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

@ralphtheninja Can you explain the users? I'm guessing the working directory on the host is owned by travis, and docker must use the same user? Can't we change permissions instead?

Copy link
Member

Choose a reason for hiding this comment

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

I'm open to suggestions. If we don't do anything about the user (e.g. set USER in the Dockerfile) the container will run as root, thus producing binaries owned by root. Which leaves us with two options (as I see it):

  1. either we do something related to user, i.e. run the container with the correct user
  2. keep running the container as root, but copy out the binaries from the container, which is done in prebuildify-cross

Copy link
Member

Choose a reason for hiding this comment

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

Thinking about it some more, I think I prefer 2. since the docker image doesn't need any specific users. This needs some tweaking to the cross-compile script as well. Also need to look into how prebuildify-cross does this with output folders etc.

Copy link
Member Author

Choose a reason for hiding this comment

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

I liked the initial approach of running as the node user. Why did that not work?

Copy link
Member

Choose a reason for hiding this comment

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

Because it runs as user with uid 1000 and the jenkins user is 2000.

Copy link
Member Author

@vweevers vweevers Feb 1, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Because it runs as user with uid 1000 and the jenkins user is 2000.

*travis user? 😉

Alright, I'm fine with both the current solution and the alternative, copying out the binaries. The latter would also remove the npm i problem (#587 (comment)), because if we copy out the binaries then we don't have to mount any volume. ignore me, it's late

Copy link
Member

@ralphtheninja ralphtheninja Feb 1, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, travis user. I don't know where jenkins came from. Jenkins! Get out of my head!

Copy link
Member

Choose a reason for hiding this comment

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

It's not a perfect solution, but it works. Lets tweak it as we go?


exec docker run -u ${DOCKER_USER} --rm -it -v $(pwd):/app prebuild/${IMAGE} \
ralphtheninja marked this conversation as resolved.
Show resolved Hide resolved
bash -c "npm i --ignore-scripts && npx prebuildify -t 8.14.0 --napi --strip"
Copy link
Member Author

Choose a reason for hiding this comment

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

  1. Seeing as each invocation of this script runs in the same working directory, they all work against the same node_modules, and reinstall each time. Can we A) skip that or B) mount node_modules as a named volume, so that changes in the docker guest are hidden from the host?
  2. Does npx use a locally installed bin if available, or does it always install the latest version of prebuildify?

Copy link
Member

@ralphtheninja ralphtheninja Feb 1, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Can we A) skip that

Not sure what you mean by that. Unsure what you mean here, mind writing snippet that explains the changes you want to make?

Afaik, npx uses a locally installed .bin if available. If it didn't, we wouldn't be able to use the specific branch of prebuildify we need to override e.g. platform.

Copy link
Member Author

@vweevers vweevers Feb 1, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Not sure what you mean by that.

npm i.

Unsure what you mean here, mind writing snippet that explains the changes you want to make?

I'll elaborate. We can either A) skip npm i in docker. We simply reuse what's already installed on the host. That only works (safely) in Travis though, where both host and guest are Linux. Not when testing things locally (on Mac, Windows or a different Linux flavor). That's also why the current approach is less than ideal, because after npm run prebuild-arm, you're stuck with a node_modules meant for a different platform.

So option B is to use a named volume. An anonymous volume may also work. Something like docker run -v $(pwd):/app -v :/app/node_modules. Initially, Docker will copy the existing /app/node_modules into the second volume. From then on, any changes to node_modules (e.g. by npm i) are applied to the second volume, meaning they don't affect node_modules on the host.

Afaik, npx uses a locally installed .bin if available. If it didn't, we wouldn't be able to use the specific branch of prebuildify we need to override e.g. platform.

👍