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

document env vars #21

Closed
ahdinosaur opened this issue Dec 25, 2018 · 5 comments · Fixed by #26
Closed

document env vars #21

ahdinosaur opened this issue Dec 25, 2018 · 5 comments · Fixed by #26

Comments

@ahdinosaur
Copy link
Contributor

forking from #19, 🍴

here is an issue to document prebuildify's environment variables: 🌟

i'm also curious about the reason for both ARCH and PREBUILD_ARCH, and whether we should commit to one or the other.

cheers! ❤️

@ralphtheninja
Copy link
Member

It seems to me that PREBUILD_ARCH and PREBUILD_PLATFORM aren't used inside prebuildify and not inside node-gyp. I'm guessing they are meant to be passed on to preinstall and postinstall scripts?

@vweevers
Copy link
Member

They were introduced in #12. Could you shed some light on this @jimpick?

@ralphtheninja
Copy link
Member

They were introduced in #12. Could you shed some light on this @jimpick?

Actually, #12 introduced PREBUILD_PLATFORM and PREBUILD_NODE_GYP but PREBUILD_ARCH seems to have been introduced earlier. Just fyi.

@jimpick
Copy link
Contributor

jimpick commented Jan 1, 2019

You can see where I used it here:

https://github.com/jimpick/utp-native-prebuilds-nodejs-mobile/blob/master/build-ios-arm64.sh#L15

I think PREBUILD_NODE_GYP was needed with the particular cross-compiling toolchain I was using (from nodejs-mobile) to get it to use a specific node-gyp path.

It's been a while since I did it, and I didn't keep the prebuilds updated, but you can see where I used PREBUILD_PLATFORM here:

https://github.com/jimpick/sodium-native/blob/ios-prebuild/preinstall.js#L12

I never created a PR for that fork as it was targeting a rapidly evolving toolchain from nodejs-mobile, and I didn't expect the upstream sodium-native developers to have to maintain those prebuilds (or myself, for that matter).

@ralphtheninja
Copy link
Member

@ahdinosaur As @jimpick just linked, it seems we want to use PLATFORM in the same way as PREBUILD_PLATFORM above so it makes sense to rename PLATFORM to PREBUILD_PLATFORM in #20

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 a pull request may close this issue.

4 participants