Rework Android support #3890
Replies: 22 comments
-
@lefou Sir Do i need to resolve these tasks also for Bounty Completion ? |
Beta Was this translation helpful? Give feedback.
-
@himanshumahajan138 no need, I'll transfer the bounty based on the initial merge, we can continue to iterate on the code in main |
Beta Was this translation helpful? Give feedback.
-
@lefou yeah we can take a few more passes at it. I marked the whole thing as |
Beta Was this translation helpful? Give feedback.
-
I'll submit a PR with the suggested updates |
Beta Was this translation helpful? Give feedback.
-
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: |
Beta Was this translation helpful? Give feedback.
-
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. |
Beta Was this translation helpful? Give feedback.
-
I was thinking a |
Beta Was this translation helpful? Give feedback.
-
There's two separate discussions here:
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 |
Beta Was this translation helpful? Give feedback.
-
It's a bit unclear to me, why we don't come up with some default instances of the |
Beta Was this translation helpful? Give feedback.
-
And if we can provide it pre-configured, we probably should overthink the use of a |
Beta Was this translation helpful? Give feedback.
-
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 Plus point: this will also eliminate reinstalling of android sdk and tools again and again for every build |
Beta Was this translation helpful? Give feedback.
-
@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 |
Beta Was this translation helpful? Give feedback.
-
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. |
Beta Was this translation helpful? Give feedback.
-
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. |
Beta Was this translation helpful? Give feedback.
-
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. |
Beta Was this translation helpful? Give feedback.
-
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
|
Beta Was this translation helpful? Give feedback.
-
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 And then by using 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 |
Beta Was this translation helpful? Give feedback.
-
@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? |
Beta Was this translation helpful? Give feedback.
-
Let me make this conversation a lil bit easy Actually we need to follow exactly same steps present in the I think this will clear the whole process |
Beta Was this translation helpful? Give feedback.
-
@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 |
Beta Was this translation helpful? Give feedback.
-
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 I think want to support the (most likely) already defined env var As a consequence, we should redesign |
Beta Was this translation helpful? Give feedback.
-
As a implementation hint, we should enable the |
Beta Was this translation helpful? Give feedback.
-
Some thinks I noticed which we should reconsider before finalizing the API:
sdkUrl
orbuildToolsVersion
should be prefixed withandroid
buildToolsVersion
defaults to35.0.0
. I suggest to just remove the default.platformsVersion
defaults toandroid-35
. We could derive it frombuildToolsVersion
.buildToolsPath
have 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.androidSdkModule
should have the typeModuleRef[AndroidSdkModule]
.androidResources
hardcodes a path tosrc/main/AndroidManifest.xml
. It should either lookup the file fromsources
task, or be it's ownandroidManifest: Source
task.androidKeystore
is probably meant to be a persistent taskBeta Was this translation helpful? Give feedback.
All reactions