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

Add Android support to preinstall.js #121

Closed
wants to merge 1 commit into from

Conversation

christianbundy
Copy link
Contributor

@christianbundy christianbundy commented Feb 22, 2020

Problem: When compiling from source on Android, the process exits with
code 1 without printing an error.

Solution: Add console output in case of an error, which may be useful
for debugging, and treat the Android platform the same way that we're
currently treating OpenBSD, FreeBSD, and Linux.


See also: https://github.com/sodium-friends/sodium-native/issues/119#issuecomment-590003696

@christianbundy
Copy link
Contributor Author

After this change I'm able to compile sodium-native (3.0.0) from source on my device (ARM64 on Android 10)!

Problem: When compiling from source on Android, the process exits with
code 1 without printing an error.

Solution: Add console output in case of an error, which may be useful
for debugging, and treat the Android platform the same way that we're
currently treating OpenBSD, FreeBSD, and Linux.
@@ -35,7 +36,7 @@ if (process.argv.indexOf('--print-lib') > -1) {
console.log('../libsodium/Build/ReleaseDLL/' + warch + '/libsodium.lib')
break
default:
process.exit(1)
throw new Error('Unsupported platform: ' + os.platform())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have two other options here:

console.log()

console.log('Unsupported platform: ' + os.platform())
process.exit(1)

No output

I was confused when the process exited without any output, but maybe there's a good reason to do it that way?

@kasperisager
Copy link
Contributor

This should be solved by #174 and #175.

@mafintosh mafintosh closed this Jul 4, 2023
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.

3 participants