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

Fix for "Kotlin sourceDir isn't take into account" #703

Closed
wants to merge 1 commit into from

Conversation

StefMa
Copy link

@StefMa StefMa commented Aug 19, 2023

Closes #702

Description:

An detailed explanation why can be read in #702

I also had the upgrade the AGP to version 7.0.0 because the 4.1.0 doesn't know the kotlin property 😁
7.0.0 is the first version that supports hit.
Yes, this would be an potential breaking change.
But please take a look at the release dates for those plugins:
https://maven.google.com/web/index.html?q=tools.bui#com.android.tools.build:gradle:4.1.0
-> Release at 10th December 2020

https://maven.google.com/web/index.html?q=tools.bui#com.android.tools.build:gradle:7.0.0
-> Release at 28th July 2021

As the Android community is normaly quite good in upgrading the core plugin to develop, I guess we are fine 😁

Changes:

  • Upgrade Android Gradle Plugin from 4.1.0 to 7.0.0
  • Use kotlin sourceSet instead of java sourceSet

How to test:

  1. Change the version in /plugin/VERSION_CURRENT.txt to something else e.g. "testKotlinSourceSet"
cd plugin 
./gradlew publishToMavenLocal
  1. Include the mavenLocal() repository (to find that version)
  2. Replace the current ktlint-plugin version (in your project) with the one you change din the /plugin/VERSION_CURRENT.txt

@StefMa StefMa changed the title Fix for "Kotlin sourceDir isn't take into account" Fix for "Kotlin sourceDir isn't take into account" Aug 19, 2023
@wakingrufus wakingrufus self-assigned this Aug 19, 2023
@wakingrufus
Copy link
Collaborator

To be clear about what this breaks, it would mean that we now require AGP >=7.0 and Gradle >= 7.0?

I am ok with this, but I would like to queue up some related maintenance work around dropping Gradle 6 support so we can merge them all around the same time and release it as 16.0

@StefMa
Copy link
Author

StefMa commented Aug 21, 2023

Yes, that's correct.
AGP 7.0 has to be used and this would required Gradle 7.0 (as also documented here https://developer.android.com/build/releases/gradle-plugin#expandable-1)

Sure, do the maintaince work and come back as you're finish with that ☺.

You can also let me when I can help with that somewhere.

@StefMa
Copy link
Author

StefMa commented Sep 16, 2023

@wakingrufus did you had the time to look into this yet?

@wakingrufus
Copy link
Collaborator

@wakingrufus did you had the time to look into this yet?

Not yet. I need to finish up #708 first

@wakingrufus wakingrufus self-requested a review November 28, 2023 23:13
@StefMa
Copy link
Author

StefMa commented Nov 29, 2023

It's going to be happen 🤘 🙃

@JLLeitschuh
Copy link
Owner

Yes, this would be an potential breaking change.

Do we want to accept this API breaking change? Does this break our guarantee of android plugin support range?

Do we currently have unit tests that cover the range of android plugins we want to ensure we support? If we don't we should.

@StefMa
Copy link
Author

StefMa commented Dec 13, 2023

I just saw that we set the AGP only as a compileOnly dependency

compileOnly(libs.android.gradle.plugin)

With that we can also workaround the breaking change by adding a try-catch 🤔

val androidSourceSet = try {
  sourceSet.kotlin as DefaultAndroidSourceDirectorySet
} catch(_: java.lang.NoSuchMethodError) {
  java as DefaultAndroidSourceDirectorySet
} 

Update

Unfortunately this is not really working as the signature of the CommonExtension also changed :/

@StefMa StefMa force-pushed the android-kotlin-sourceset branch from 34eeeae to 2141717 Compare December 13, 2023 08:01
For detailed reasons check the following issue:
JLLeitschuh#702
@StefMa StefMa force-pushed the android-kotlin-sourceset branch from 2141717 to eb47391 Compare December 13, 2023 08:02
@StefMa StefMa requested a review from wakingrufus December 13, 2023 08:18
wakingrufus added a commit that referenced this pull request Dec 20, 2023
wakingrufus added a commit that referenced this pull request Dec 20, 2023
increase minimum supported AGP version to 4.2.2 because the plugin coordinates/id changed between 4.1 and 4.2
increase maximum gradle version to 8.5
increase maximum ktlint version to 1.1.0

fixes #702
based on work in #703
wakingrufus added a commit that referenced this pull request Dec 20, 2023
increase maximum gradle version to 8.5
increase maximum ktlint version to 1.1.0

fixes #702
based on work in #703
wakingrufus added a commit that referenced this pull request Dec 20, 2023
increase maximum gradle version to 8.5
increase maximum ktlint version to 1.1.0
increase maximum kotlin version to 1.9.21

fixes #702
based on work in #703
wakingrufus added a commit that referenced this pull request Dec 20, 2023
increase maximum gradle version to 8.5
increase maximum ktlint version to 1.1.0
increase maximum kotlin version to 1.9.21

fixes #702
based on work in #703
wakingrufus added a commit that referenced this pull request Dec 20, 2023
increase maximum gradle version to 8.5
increase maximum ktlint version to 1.1.0
increase maximum kotlin version to 1.9.21

fixes #702
based on work in #703
wakingrufus added a commit that referenced this pull request Dec 20, 2023
increase maximum gradle version to 8.5
increase maximum ktlint version to 1.1.0
increase maximum kotlin version to 1.9.21
fix lint failures

fixes #702
based on work in #703
wakingrufus added a commit that referenced this pull request Dec 20, 2023
increase maximum gradle version to 8.5
increase maximum ktlint version to 1.1.0
increase maximum kotlin version to 1.9.21
fix lint failures

fixes #702
based on work in #703
wakingrufus added a commit that referenced this pull request Jan 3, 2024
increase maximum gradle version to 8.5
increase maximum ktlint version to 1.1.0
increase maximum kotlin version to 1.9.21
fix lint failures

fixes #702
based on work in #703
wakingrufus added a commit that referenced this pull request Jan 3, 2024
increase maximum gradle version to 8.5
increase maximum ktlint version to 1.1.0
increase maximum kotlin version to 1.9.21
fix lint failures

fixes #702
based on work in #703
wakingrufus added a commit that referenced this pull request Jan 3, 2024
increase maximum gradle version to 8.5
increase maximum ktlint version to 1.1.0
increase maximum kotlin version to 1.9.21
fix lint failures

fixes #702
based on work in #703
wakingrufus added a commit that referenced this pull request Jan 3, 2024
increase maximum gradle version to 8.5
increase maximum ktlint version to 1.1.0
increase maximum kotlin version to 1.9.21
fix lint failures

fixes #702
based on work in #703
wakingrufus added a commit that referenced this pull request Jan 4, 2024
increase maximum gradle version to 8.5
increase maximum ktlint version to 1.1.0
increase maximum kotlin version to 1.9.21
fix lint failures

fixes #702
based on work in #703
wakingrufus added a commit that referenced this pull request Jan 4, 2024
increase maximum gradle version to 8.5
increase maximum ktlint version to 1.1.0
increase maximum kotlin version to 1.9.21
fix lint failures

fixes #702
based on work in #703
wakingrufus added a commit that referenced this pull request Jan 4, 2024
increase maximum gradle version to 8.5
increase maximum ktlint version to 1.1.0
increase maximum kotlin version to 1.9.21
fix lint failures

fixes #702
based on work in #703
wakingrufus added a commit that referenced this pull request Jan 4, 2024
increase maximum gradle version to 8.5
increase maximum ktlint version to 1.1.0
increase maximum kotlin version to 1.9.21
fix lint failures

fixes #702
based on work in #703
wakingrufus added a commit that referenced this pull request Jan 8, 2024
increase maximum gradle version to 8.5
increase maximum ktlint version to 1.1.0
increase maximum kotlin version to 1.9.21
fix lint failures

fixes #702
based on work in #703
wakingrufus added a commit that referenced this pull request Jan 8, 2024
increase maximum gradle version to 8.5
increase maximum ktlint version to 1.1.0
increase maximum kotlin version to 1.9.21
fix lint failures

fixes #702
based on work in #703
wakingrufus added a commit that referenced this pull request Jan 8, 2024
increase maximum gradle version to 8.5
increase maximum ktlint version to 1.1.0
increase maximum kotlin version to 1.9.21
fix lint failures

fixes #702
based on work in #703

fix bashisms by manually specifying correct version of libdistro-info-perl
wakingrufus added a commit that referenced this pull request Jan 8, 2024
increase maximum gradle version to 8.5
increase maximum ktlint version to 1.1.0
increase maximum kotlin version to 1.9.21
fix lint failures

fixes #702
based on work in #703

fix bashisms by manually specifying correct version of libdistro-info-perl
wakingrufus added a commit that referenced this pull request Jan 8, 2024
increase maximum gradle version to 8.5
increase maximum ktlint version to 1.1.0
increase maximum kotlin version to 1.9.21
fix lint failures

fixes #702
based on work in #703

fix bashisms by manually specifying correct version of libdistro-info-perl
JLLeitschuh pushed a commit that referenced this pull request Jan 9, 2024
increase maximum gradle version to 8.5
increase maximum ktlint version to 1.1.0
increase maximum kotlin version to 1.9.21
fix lint failures

fixes #702
based on work in #703

fix bashisms by manually specifying correct version of libdistro-info-perl
@Goooler
Copy link
Contributor

Goooler commented Jan 20, 2024

Seems this could be closed for now, handled by #733.

@StefMa StefMa closed this Jan 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Android: ktlint[SourceSetName]SourceSetCheck task doesn't take kotlin srcDir into account
4 participants