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

Jump to bottom

androidenv: update packages #58131

Closed
wants to merge 1 commit into from
Closed

androidenv: update packages #58131

wants to merge 1 commit into from

Conversation

tadfisher
Copy link
Contributor

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

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
Copy link
Contributor Author

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.

@svanderburg
Copy link
Member

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

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

@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.

@bkchr
Copy link
Contributor

bkchr commented Dec 29, 2019

Any update here?

This was referenced Mar 8, 2020
Merged
Merged
@lucafavatella
Copy link
Contributor

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

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
Reviewers

@svanderburg svanderburg Awaiting requested review from svanderburg

Assignees
No one assigned
Labels
8.has: package (update) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@tadfisher @svanderburg @numinit @bkchr @lucafavatella @doronbehar @infinisil @GrahamcOfBorg