Rework Android support
Some thinks I noticed which we should reconsider before finalizing the API:
- Various tasks like
sdkUrlorbuildToolsVersionshould be prefixed withandroid - Hardcoded
buildToolsVersiondefaults to35.0.0. I suggest to just remove the default. - Hardcoded
platformsVersiondefaults toandroid-35. We could derive it frombuildToolsVersion. - In general lots of various paths which are just sub-paths of
buildToolsPathhave their own tasks. Instead we could have some "config" class, which returns common paths from abuildToolsPath, e.g. by applying a (version specific) layout/sub-paths. AndroidAppModule.androidSdkModuleshould have the typeModuleRef[AndroidSdkModule].androidResourceshardcodes a path tosrc/main/AndroidManifest.xml. It should either lookup the file fromsourcestask, or be it's ownandroidManifest: Sourcetask.androidKeystoreis probably meant to be a persistent task
@lefou Sir Do i need to resolve these tasks also for Bounty Completion ?
@himanshumahajan138 no need, I'll transfer the bounty based on the initial merge, we can continue to iterate on the code in main
@lefou yeah we can take a few more passes at it. I marked the whole thing as @experimental for now which it definitely is
I'll submit a PR with the suggested updates
BTW i am Interested in contributing to this also and after this i want to work with kotlin examples
Few things i want to mention: Sir Discord link is not working i want to join but unable to do so...
Various tasks like sdkUrl or buildToolsVersion should be prefixed with android
I think in this case we can leave them un-prefixed. The prefixing is mostly for mixin/stackable traits where there is risk of name collisions, e.g. AndroidAppModule stacks on top of JavaModule. AndroidSdkModule seems pretty standalone so not sure if we need to prefix all the tasks
Various tasks like sdkUrl or buildToolsVersion should be prefixed with android
I think in this case we can leave them un-prefixed. The prefixing is mostly for mixin/stackable traits where there is risk of name collisions, e.g.
AndroidAppModulestacks on top ofJavaModule.AndroidSdkModuleseems pretty standalone so not sure if we need to prefix all the tasks
I was thinking a sdkUrl is perfectly reasonable for a Java SDK too, which we will get sooner or later when we are able to select the Java version. Although I overlooked that AndroidSdkModule is standalone. we don't know what users are going to do, e.g. combine a Java and Android SDK module. If we follow our de-facto rule and prefix tasks with the common module name, it won't hurt.
There's two separate discussions here:
-
For this specific case, I don't mind renaming
sdkUrltoandroidSdkUrl. It's not too verbose and does clarify things a bit. Even addingandroid*to all the other members isn't too verbose, although it does get repetitive. -
As a more general policy, I don't think every
traitshould have all of its members prefixed.- e.g. the "base traits" like
JavaModuleorScalaModuleorZincWorkerModuledon't prefix everything withjava*orscala*orzincWorker*. - OTOH traits like
PalantirFormatModuleorTestModuledo tend to prefix everything withpalantirFormat*ortest*, since their primary use case is to be mixed into other traits that collide.
- e.g. the "base traits" like
We haven't had a very strict rule in the past, but I think if we want to formalize one then having some distinction between "base traits" and "mixin traits" is useful, since they do tend to be used pretty differently in practice, and so different constraints apply. I don't think we should have an expectation that arbitrary totally unrelated module traits can be mixed together and continue working without issue.
Maybe in 0.13.0 we can consider turning some of the "base traits" into abstract classes so they can't be used as mixins, but that would be a bit further off.
It's a bit unclear to me, why we don't come up with some default instances of the AndroidSdkModule. Are there any user-specific limitations like licence agreements or personal tokens? If, they should be explicitly mentioned in the docs. If not, it would be nice to have some ready-to-used objects for current major Android versions. Otherwise, users will do repeated work over and over.
And if we can provide it pre-configured, we probably should overthink the use of a ModuleRef, since it can't be switched based on a target result. Instead, we should make it some config class and a worker task, so we can infer the best "AndroidSDKSupport" from the configured androidVersion.
actually, @lefou Sir you are saying right to have readymade setup but what i think could me more better is to use CI (github actions) for installing android of specific version just like we do for java during setup of artifacts...
i think this will eliminate this android sdk module totally and we have to rely only on the AndroidAppModule
Plus point: this will also eliminate reinstalling of android sdk and tools again and again for every build what are your thoughts ....
@lefou I went with a separate module because i wasnt sure how much would need to be typically configured; if it's always the same few instances we can provide them, if they're always specially configured then providing default instances doesnt really help. TBH I don't know enough about typical Android development to have an intuition here so the choice of manually instantiated SDK modules is pretty arbitrary
I guess that users doing a lot of Android stuff will prefer to have some system (or user) installed SDK and want to just point the SDK path to that. But for occasional users who only checkout some project that also has some Android app, Mill should handle it smoothly without any local installations. We probably want a shared external worker module that can fetch and unpack standard SDKs, but accepts environment variable or some other means of shared SDKs.
But for occasional users who only checkout some project that also has some Android app, Mill should handle it smoothly without any local installations
The common expectation is still to have Android SDK installed before using the build tool. The main concern is probably accepting the license: while when running CI tasks for the Mill project it can be accepted there, because end-users are Mill contributors, accepting it on behalf of the user in case of the user builds is no-go.
This is what Android Gradle Plugin is doing, if I'm not wrong - it expects Android SDK to be already installed and be registered via env variable or have a path declaration in Gradle properties.
Exactly, i am too considering installation during Ci as best option as this will make it general and available for everyone during testing just like we do with java setup
And as u said that gradle suppose to have sdk already installed then according to this we should also make sure that android is present during ci testing and rest is user work they have to install Android sdk on their system for local usage.
Ok, then the answer is: The user needs to pre-install the Android SDK because of license requirements.
It's quite like the situation we had for older Java SDKs. If we want to configure it in Mill, but need to refer to it via some local configuration, we probably need some mapping of configured version to local path. Either we look for some env var pattern or some config file under .config, e.g. .config/mill-android-sdks or .config/mill-android-sdk.<version> where <version> is the same as in AndroidSdkModule.buildToolsVersion.
- This is related to https://github.com/com-lihaoyi/mill/issues/2250, so we should try to make the solution as dual/analog/equal as possible, so users can learn the schema once and apply it to various SDK handling.
Either we look for some env var pattern or some config file under .config, e.g. .config/mill-android-sdks or .config/mill-android-sdk.
where is the same as in AndroidSdkModule.buildToolsVersion.
Not exactly like that. To be precise, there is only one Android SDK and to get it one can go to https://developer.android.com/studio, scroll to Command line tools only and download the archive. It then should be unpacked and the final path should be registered under ANDROID_SDK_ROOT (or ANDROID_HOME) env variable (to be precise, the location of cmdline tools shouldn't be the same as Android SDK root folder, but for the simplicity it can be like that).
And then by using sdkmanager coming with cmdline tools build system (Gradle, Mill) can pull the API sources/build tools/etc. version it needs. Just on the first usage user needs to accepts the license, and after it is done, build system should be able to get everything it needs.
To simplify: first Mill should do env variable lookup to locate Android SDK root and fallback to the some configuration path entry if not found. Once path is known, Mill can pull the components needed by using sdkmanager.
@0xnm So, each new version of the SDK comes with all older libraries of older releases included and grows ever larger? Or how are we supposed to target older Android versions?
Let me make this conversation a lil bit easy
Actually we need to follow exactly same steps present in the AndroidSdkModule.scala means same cmd commands needs to be implemented using actions or what we use currently and then talking about version we have update the version manually when new updates comes coz there is no official github actions available for Android and finally we will export the to env and use them from there
I think this will clear the whole process
@lefou No, the set of the components (build tools, sources for the specific API, etc.) can be controlled with sdkmanager which is a part of the cmdline tools. cmdline tools just should just exist under the registered Android SDK root location and then can be leveraged to get everything the particular build system needs.
Thanks for the clarifications. Since me and probably also other Mill devs are not that familiar with Android tooling, let me summarize it:
There is only one Android SDK location, which is required to be at least as new as the target version, we want to use. It needs to pre-exists and can't automatically installed due to licensing requirements.
In this SDK is a tool sdkmanager which a user or Mill can use to configure (read "fetch") support (libs, tools) for a specific target version. These files are stored inside the SDK, so we later just assemble the path from the SDK location, the target version and the tools name.
I think want to support the (most likely) already defined env var ANDROID_SDK_ROOT. We could fall back to read a .config/mill-android-sdk file.
As a consequence, we should redesign AndroidSdkModule to only have a sdkUrl and a worker that manages the target versions. That worker can be ask for the version specific tools support, which can on first access use the sdkmanager to prepare the target version (if not already present).
As a implementation hint, we should enable the revalidate flag on all PathRefs which point to Android SDK locations (outside of Mill's cache), so Mill can detect if these paths are no longer present or changed.