Skip to content

Conversation

@syvb
Copy link
Contributor

@syvb syvb commented Jan 31, 2024

I want to build Servo on NixOS, without installing the non-free Android SDK. This PR makes it so Android support is only built if the buildAndroid argument is true (e.g. by running nix-shell --arg buildAndroid true etc/shell.nix)

I've verified that you can still build/run non-Android Servo with this change, but I haven't tested on Android (it should still work though, since the derivation is unaffected by this change when buildAndroid is true).


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors - it reports a diff in components/config/prefs.rs, which this PR didn't change (I think Document media configs from prefs #31223 caused this by adding a trailing space?)
  • These changes fix #___ (GitHub issue number if applicable)
  • There are tests for these changes OR
  • These changes do not require tests because the Nix configuration doesn't have tests

I want to build Servo without also installing the entire Android SDK.
This makes it so Android support is only built if `buildAndroid` is
true.

Signed-off-by: syvb <me@iter.ca>
@mukilan
Copy link
Member

mukilan commented Jan 31, 2024

Hi @syvb, thank you for the contribution. This PR looks good, but it will break ./mach build --android when run outside the nix-shell. We have logic to enter the nix-shell env for all mach commands on Nix, so ideally we should pass buildAndroid true there. However, this is handled very early on in the mach where we haven't parsed all the arguments. I was thinking about having an environment variable like SERVO_ANDROID_BUILD that we check in that code to decide if buildAndroid needs to be set. What do you think?

@syvb
Copy link
Contributor Author

syvb commented Jan 31, 2024

I made it so that mach sets buildAndroid to true if the SERVO_ANDROID_BUILD environment variable is present, and added a specific error message for the case of running ./mach build --android in a Nix-shell without ANDROID_{S,N}DK_ROOT.

I didn't make the SERVO_ANDROID_BUILD enable --android by default - to build Android with Nix, one has to set both SERVO_ANDROID_BUILD and pass --android to mach build (not sure if that would be useful).

@syvb syvb requested a review from mukilan February 1, 2024 23:06
Copy link
Member

@mukilan mukilan left a comment

Choose a reason for hiding this comment

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

Thank you!

@mukilan mukilan enabled auto-merge February 2, 2024 01:43
@mukilan mukilan added this pull request to the merge queue Feb 2, 2024
Merged via the queue into servo:main with commit f27227b Feb 2, 2024
@syvb syvb deleted the opt-android branch February 3, 2024 00:14
Taym95 pushed a commit to Taym95/servo that referenced this pull request Feb 11, 2024
* Make Android build optional on Nix

I want to build Servo without also installing the entire Android SDK.
This makes it so Android support is only built if `buildAndroid` is
true.

Signed-off-by: syvb <me@iter.ca>

* Add Android support to nix-shell if SERVO_ANDROID_BUILD set

---------

Signed-off-by: syvb <me@iter.ca>
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.

2 participants