-
Notifications
You must be signed in to change notification settings - Fork 90
New input format proposition #36
base: sharing-resources-between-android-modules
Are you sure you want to change the base?
New input format proposition #36
Conversation
I like that the number of packages, classes and methods for java and kotlin are separated now from the total. |
"moduleCount": 5 | ||
} | ||
], | ||
"flavours": [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be possible to also support input flavors as done in 917229b, that way helps when adding many flavors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem with that approach is that amount of flavour*dimens grows like n^2. We can think about adding "bulk" flavour/dimen generation like it is done with packages, would that work for you? Then there'll just be multiple types of FlavourConfig
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the number of variants is the product of the number of flavors per dimension, it grows really fast (exponentially assuming each dimension has the same number of flavors).
Adding bulk as in packages can also work (having a prefix, a dimension and a counter). Another way is by adding an optional counter, similar to what is done with the modules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I meant modules, not packages. I'll think about it and push to this branch want I'll come up with.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that having two counters instead of one is more practical, one for the number of flavours the other for dimensions, then it would be more flexible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Every dimension can have a different number of flavors. What the input would look like with what I said is something like this:
"flavours": [
{
"name": "red",
"count": 5,
"dimension": "color"
},
{
"name": "green",
"count": 2,
"dimension": "color"
"buildConfig": {
"minSdkVersion": 21
}
},
{
"name": "paid",
"dimension": "license"
},
{
"name": "free",
"dimension": "license"
}
]
This would generate 7 flavors ("red0" to "red4", "green0" and "green1") for dimension "color" and 2 for "license" (the total number of variants in this case would be 28 ( 7 in color x 2 in license x {debug | release} )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this the format is perfect. We can make it a tiny little bit more flexible. We can add one more param "dimensionCounter", then we can easily create tons of dimensions with tons of flavours. Otherwise we would be forced to write one block for each dimension
I added support for kotlinVersion / androidGradleVersion in #41 |
Not sure if this is the best place, but could there be a flag to specify the Java source version of the generate code. In the AGP, this is controlled using this DSL option - https://google.github.io/android-gradle-dsl/current/com.android.build.gradle.internal.CompileOptions.html |
@gavra0 do you think it should be specified once per project, or each module should be able to override it? |
"kotlinClassCount": 5, | ||
"kotlinMethodCount": 1000, | ||
|
||
"dependencies": [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think on also having explicit dependencies outside ModuleConfigs? Following this structure:
class ExplicitDependency {
val from : List
val to : List
val type : String?
}
The format accepted by "from" and "to" would be a list of strings, each representing a prefix (meaning all the modules of that ModuleConfig) or a specific module.
An example is like this:
"dependencies": [{"from": ["prefixA", "prefixB5"], "to": ["prefixC", "prefixB1"]}, {"from": ["prefixA"], "to": ["prefixB0"], "type": "api"}]
That would create the following:
- ((all modules in prefixA) and prefixB5) depend on ((all modules in prefixC) and prefixB1) with default type (implementation)
- (all modules in prefixA) depend on prefixB0 for api
We can let the type for later since that has other implications (for example, how the code dependencies should be generated). This format (a list of prefixes and modules) can be used in other places to describe a set of modules and can be reused in topologies.
While thinking on how input processing will be implemented, at the end, all of this will be used to generate a list of explicit module names that will be added to the build.gradle files. I think both formats can easily be adapted to use the same structure that will hold those strings. Right now it is a List<Dependency> that contains pairs of integers. This structure needs to change to support dependencies between different ModuleConfig. What do you think of changing this list to Map<String, List<Dependency>>? The key will be the module name while the list holds the modules it depends on. We can also change the structure of Dependency since now the "from" will be redundant and also add the dependency type ("api", "implementation", etc.).
One advantage of using a map instead of a list is that we will not require to filter on every module since the map will give us that in constant time, also improving performance (in the worst case, this list can have N*(N-1)/2 dependencies, causing the process of adding dependencies into build files to be cubic on the number of modules, while using a map is quadratic).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now it is a List that contains pairs of integers.
That is true, but it will change completely with the new format, each ModuleConfig
will have a List where it refers other ModuleConfig by prefix. https://github.com/NikitaKozlov/android-studio-poet/blob/71be1842b8c112d3e979ae083926df09a3dfef1d/src/main/kotlin/com/google/androidstudiopoet/input/DependencyConfig.kt#L3
It is possible to express with new format everything you listed ("api" and "implementation" is easy to add), although it will be scattered through different ModuleConfig
s:
{
"inputVersion": "0.0.1",
"projectConfig": {
"projectName": "genny",
"root": "./modules/",
"moduleConfigs": [
{
"moduleCount": 5,
"moduleNamePrefix": "prefixA",
"dependencies": [
{
"type": "BULK",
"moduleNamePrefix": "prefixC",
"moduleCount": 5
},
{
"type": "BULK",
"moduleNamePrefix": "prefixB1",
"moduleCount": 5
},
{
"type": "BULK",
"moduleNamePrefix": "prefixB0",
"moduleCount": 5,
"dependencyType": "api"
}
]
},
{
"moduleCount": 5,
"moduleNamePrefix": "prefixB5",
"dependencies": [
{
"type": "BULK",
"moduleNamePrefix": "prefixC",
"moduleCount": 5
},
{
"type": "BULK",
"moduleNamePrefix": "prefixB1",
"moduleCount": 5
}
]
},
{
"moduleCount": 5,
"moduleNamePrefix": "prefixC"
},
{
"moduleCount": 5,
"moduleNamePrefix": "prefixB1"
},
{
"moduleCount": 5,
"moduleNamePrefix": "prefixB0"
}
]
}
}
Because each ModuleConfig
explicitly shows what other ModuleConfig
s it depends on then there'll be no more filtering.
I think we can first implement one way of doing things and after we play with it we can reevaluate it. If it would be too verbose, or it wouldn't fit some of the needs (for example topologies) we can add ExplicitDependency
later.
I'm not sure how do you want to use ExplicitDependency
for topologies, can you please explain it?
Unfortunately module name is not enough for generating dependency, we also need method that needs to be called and resources that need to be refered, currently figuring out those things could be computationally expensive.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please explain as well the problem you are trying to solve? Then it would be easier for me to agree with you or offer some alternatives.
I finished a draft for a new input.
Unfortunately, I have to change our input a lot. But now it supports some cool things, like overriding "minSdkVersion" for a single flavour or understands things like "each one of these 5 modules depends on 2 of those 5 modules". I remember your idea of "random" dependencies, I think this input will allow us to make it easier. It is also would possible to generate more than one "application" module or have a different amount of activities in different modules.
Please check it very carefully, I believe this is a big decision. I put special attention to the things that should be optional and those that shouldn't, so, please, check that as well.
There is an example of input in "NewInputExample.json". The rest is classes, sometimes a few related classes in one file.
It would be very easy to extend this input format to support AIDL/renderscript, "api" and "implementation" dependencies and hopefully all the other crazy stuff we or somebody else can come up with. :)