-
Notifications
You must be signed in to change notification settings - Fork 216
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
Use compileOnly for kotlin/agp plugins #648
base: master
Are you sure you want to change the base?
Conversation
build.gradle
Outdated
@@ -10,7 +10,7 @@ buildscript { | |||
|
|||
dependencies { | |||
classpath libs.plugin.kotlin | |||
classpath libs.plugin.android | |||
classpath 'com.android.tools.build:gradle:7.3.1' |
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.
Not sure if this change was intented? 7.4-rc1
is in the toml.
/paparazzi/build.gradle
and your new compileOnly still uses the 7.4 so you might get overridden.
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 have a feeling this was him testing the solution.
See the last classpath
below and https://github.com/cashapp/paparazzi/blob/master/settings.gradle#L9 where that plugin is being replaced.
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 I understand this more now. As I ran into an issue with this.
The the paparazzi-gradle-plugin can use the latest version, but for testing and building the paparazzi AAR an older version can be used.
@jrodbx I suggest splitting it into 3:
libs.plugin.android.build
(for AAR build)libs.plugin.android.test
(for integration tests)libs.plugin.android.api
(for dependency)
The api
should be behind a bit (stable), the build
can be the latest alpha, and the test
is the best compatible version.
524be48
to
581a1ed
Compare
implementation libs.plugin.kotlin | ||
implementation libs.plugin.android |
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.
please merge this PR, I can't use Android Studio Dolphin with 1.2.0-SNAPSHOT, because AS-D doesn't support 7.4:
The project is using an incompatible version (AGP 7.4.0-rc01) of the Android Gradle plugin. Latest supported version is AGP 7.3.1
See Android Studio & AGP compatibility options.
I'm not even sure of a workaround yet, because I'm using plugins {}
so there's no classpath(...) { exclude }
.
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.
Tried this, but it doesn't apply transitively and I don't want to list all modules:
buildscript {
configurations.classpath.configure {
resolutionStrategy {
force(libs.android.gradle)
force(libs.android.lint.common)
}
This crazy hack works 🤦♂️:
buildscript {
configurations.classpath.configure {
resolutionStrategy {
eachDependency {
if (requested.version == "30.4.0-rc01") {
useVersion("30.3.1")
}
if (requested.version == "7.4.0-rc01") {
useVersion("7.3.1")
}
}
}
}
}
Looks like this PR is superseded by -> |
581a1ed
to
1500596
Compare
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've got little context here but this seems fine? Cutting out the dependencies at runtime seems like an easy win
@BrianGardnerAtl the context is that the strict dependency that paparazzi places on AGP and Kotlin makes upgrading any of the three rather difficult because you have to do them all at once, or add some ugly workarounds. |
I think that makes sense. Is that the requirement on the consuming project since paparazzi was declaring the dependencies outside of just compile time? |
yeah, any build that uses papparazzi gets forced to use the same AGP/Kotlin versions as paparazzi unless you apply multiple layers of version constraints in gradle. |
No description provided.