search

prebuild / prebuildify-cross

Compile prebuilds in Docker.
17 stars 6 forks source link

Rewrite as node cli using prebuild/docker-images #7

Closed vweevers closed 4 years ago

vweevers commented 4 years ago

fyi @ahdinosaur

Closes #6, closes #5.

vweevers commented 4 years ago

Almost there. Before de6fa6e I spawned docker run which automatically pulls the image, but doesn't support piping both stdin and stdout, so I switched to the docker-run module, but that doesn't pull the image. I might try dockerode (but it's API is so much more complicated) or docker-pull.

vweevers commented 4 years ago

Dumping thoughts for finishing this later.

I dislike the coupling with npm run prebuild, a somewhat hidden and surprising behavior? I think I want to remove that. Small downside is that you'll have to repeat prebuildify arguments, e.g.

{
  "scripts": {
    "prebuild": "prebuildify -t 8.14.0 --napi --strip",
    "prebuild-cross": "prebuildify-cross -i linux -i alpine -t 8.14.0 --napi --strip"
  }
}

Rather than:

{
  "scripts": {
    "prebuild": "prebuildify -t 8.14.0 --napi --strip",
    "prebuild-cross": "prebuildify-cross -i linux -i alpine"
  }
}

I also don't like that prebuildify is an implicit peer dependency, but it is easier to invoke this way (as a dependency of the project being built in the docker container, rather than a dependency of prebuildify-cross which lives in the host machine) so I'll keep that as-is for now.

Lastly, perhaps rename the "linux" alias to "linux-x64" to match dockcross naming and add an "linux-x64-musl" alias for "alpine". Or ditch the aliases altogether, maybe they'd only cause more confusion. Best would be renaming the docker images themselves.

vweevers commented 4 years ago

@ahdinosaur Could you npm owner add vweevers prebuildify-cross? ❤️