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

androidenv: update packages #58131

Closed
wants to merge 1 commit into from
Closed

Conversation

@tadfisher
Copy link
Contributor

tadfisher commented Mar 22, 2019

Motivation for this change

Update generate.sh to run using nix-shell. Also make it fail with meaningful output instead of writing empty output files.

Generate the latest Android packages.

Update composeAndroidPackages with versions that exist in the generated package set.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@tadfisher
Copy link
Contributor Author

tadfisher commented Mar 22, 2019

Actually this doesn't build when including system images; system-images-google_apis_playstore.nix is somehow using "28" as the key for "Q" images. This is probably in the XSLT, so I'll take a look in there.

@tadfisher tadfisher force-pushed the tadfisher:androidenv-update branch from c98374c to 861b16f Mar 22, 2019
@tadfisher
Copy link
Contributor Author

tadfisher commented Mar 22, 2019

Updated with fixes for convertsystemimages.xsl:

  • Use type-details/codename if it exists, falling back to type-details/api-level: this results in "Q" rather than "28" for preview images
  • Use <xsl:text> elements to control whitespace in the output.
@tadfisher tadfisher force-pushed the tadfisher:androidenv-update branch from 861b16f to b30fbf0 Mar 22, 2019
@svanderburg
Copy link
Member

svanderburg commented Mar 24, 2019

Hi looks good so far, but I need a bit of time to review this. I need to check whether the testcases still pass and whether the frameworks built on top of it don't break.

Moreover, I may still have to set the default tools version to 25.x. 26.x is recommended nowadays, but it has the disadvantage that it does not include any ant-functionality. As a result, it will break the buildApp {} function.

@tadfisher
Copy link
Contributor Author

tadfisher commented Mar 25, 2019

Ah, understood. I've been trying to work out how to make Gradle builds possible in sandboxed mode, and it's not an easy problem to solve.

@numinit
Copy link
Contributor

numinit commented Apr 28, 2019

How's this coming? (I re-ran update.sh on my local copy, and got a few duplicates, BTW).

For things like the NDK, I think we should include a couple older versions here (maybe back to r17?)

@numinit
Copy link
Contributor

numinit commented Apr 28, 2019

(BTW: if you want me to take over this PR, I'd be more than happy.)

@svanderburg
Copy link
Member

svanderburg commented Apr 29, 2019

@numinit I'm happy to integrate this, as long as the default tools package version remains a 25.x. Can you change the pull request so that this version becomes the default again?

Once we have decent gradle build support then the 26.1.x version can be made the default.

Making 26.1.x the default now will break the androidenv.buildApp {} function in the default installation, which IMO isn't very nice.

@matthewbauer matthewbauer requested a review from svanderburg Jun 11, 2019
@bkchr
Copy link
Contributor

bkchr commented Dec 29, 2019

Any update here?

@lucafavatella
Copy link
Contributor

lucafavatella commented Mar 9, 2020

Most changes in this PR are now in PR #82067 (scripts generate.sh and related) and PR #82118 (actual package updates).

@tadfisher Could you please consider reviewing #82067 , and preparing a branch with any enhancements you envisage (e.g. I noticed you had amended the default versions - probably fruit of some testing) so we get most androidenv-related enhancements merged.

@doronbehar
Copy link
Contributor

doronbehar commented May 16, 2020

Most changes in this PR are now in PR #82067 (scripts generate.sh and related) and PR #82118 (actual package updates).

@tadfisher Could you please consider reviewing #82067 , and preparing a branch with any enhancements you envisage (e.g. I noticed you had amended the default versions - probably fruit of some testing) so we get most androidenv-related enhancements merged.

I'm closing as all mentioned PRs above were merged.

@doronbehar doronbehar closed this May 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

8 participants
You can’t perform that action at this time.