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

Migrate from convention to extension #577

Merged
merged 6 commits into from
Jul 15, 2022

Conversation

rougsig
Copy link
Collaborator

@rougsig rougsig commented Jul 6, 2022

Background
Gradle convention does not support dsl generation. For automatic dsl generation gradle requires extensions. In that reason while we use convention, we need to write dsl accessors by hand (see ProtobufConfiguratorExts.kt).

Changes

  1. Migrated from convention to extension.
  2. Removed Android/JavaGenerateProtoTaskCollection. Now it one class named GenerateProtoTaskCollection.
  3. Removed unnecessary kotlin dsl accessors, now it generated by gradle.

Breaking changes

  1. In reason Changes.1. Now project.protobuf.protobuf is not valid code. User should write project.protobuf.
  2. In reason Changes.3. User should remove those imports from kotlin gradle scripts:
    . com.google.protobuf.gradle.protobuf
    . com.google.protobuf.gradle.protoc
    . com.google.protobuf.gradle.plugins
    . com.google.protobuf.gradle.generateProtoTasks
    . com.google.protobuf.gradle.builtins
    . com.google.protobuf.gradle.ofSourceSet
    . com.google.protobuf.gradle.ofFlavor
    . com.google.protobuf.gradle.ofBuildType
    . com.google.protobuf.gradle.ofVariant
    . com.google.protobuf.gradle.ofNonTest
    . com.google.protobuf.gradle.ofTest

Test plan
Green pipelines. Production code was changed. Changes does not affect any plugin logic.

References issues: #505

More details can be found here: google#505
@rougsig rougsig marked this pull request as draft July 6, 2022 15:38
@ejona86
Copy link
Collaborator

ejona86 commented Jul 6, 2022

Do you think we use separate extensions for Android vs non-Android? That would mean they'd have to have different names, like project.protobuf and project.protobufAndroid?

@rougsig
Copy link
Collaborator Author

rougsig commented Jul 6, 2022

DSL for android is not the same as for java/kotlin. Maybe protobuf configuration for android should be inside android Block. It's open question how to separate configuration for java/kotlin, android, kmm, any_other_lang

@rougsig
Copy link
Collaborator Author

rougsig commented Jul 10, 2022

Do you think we use separate extensions for Android vs non-Android? That would mean they'd have to have different names, like project.protobuf and project.protobufAndroid?

For now, I've merged them into one. As it was with kotlin. i.e. for android selectors (ofFlavour, etc.) inside a java project, will be returned an empty list of tasks

@rougsig rougsig marked this pull request as ready for review July 10, 2022 11:16
@rougsig rougsig requested a review from ejona86 July 10, 2022 11:17
Copy link
Collaborator

@ejona86 ejona86 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks fair. Just a few comments.

testProjectBase/build_base.gradle Outdated Show resolved Hide resolved
@ejona86 ejona86 requested a review from YifeiZhuang July 11, 2022 23:16
@rougsig rougsig force-pushed the convention-extension branch from 8500641 to 20f56ca Compare July 12, 2022 16:15
@rougsig rougsig force-pushed the convention-extension branch from 20f56ca to 81fed13 Compare July 12, 2022 16:19
@rougsig
Copy link
Collaborator Author

rougsig commented Jul 14, 2022

Any updates?

Copy link
Collaborator

@ejona86 ejona86 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops. I had looked over the changes after the update but had forgotten to approve.

@ejona86 ejona86 merged commit 499431d into google:master Jul 15, 2022
@ejona86 ejona86 linked an issue Jul 15, 2022 that may be closed by this pull request
bot-gradle added a commit to gradle/gradle that referenced this pull request Sep 10, 2022
…from the org.gradle.util package

* #21839

For some members of `org.gradle.util.*` deprecation logging could not be added because smoke tested plugins and other third parties don't offer a version that is free of their use:

* **Kotlin Gradle Plugin**
  * `WrapUtil.toDomainObjectSet()`
  * [KT-53882 - Kotlin Gradle Plugin should not use the deprecated WrapUtil.toDomainObjectSet() function](https://youtrack.jetbrains.com/issue/KT-53882/Kotlin-Gradle-Plugin-should-not-use-the-deprecated-WrapUtiltoDomainObjectSet-function)
  * #21882
* **Android Gradle Plugin**
  * `GUtil.toWords(string)`
  * [ANDROID-245405989 - Android Gradle Plugin should not use the deprecated GUtil.toWords(string) function](https://issuetracker.google.com/issues/245405989)
  * #21899
  * `ConfigureUtil.configure(closure, target)`
  * [ANDROID-245405989 - Android Gradle Plugin should not use the deprecated ConfigureUtil.configure(closure, target) function](https://issuetracker.google.com/issues/245405994)
  * #21901
* **Spring Dependency Management Plugin**
  * `GradleVersion.getNextMajor()`
  * spring-gradle-plugins/dependency-management-plugin#338
  * #21898
* **Google Protobuf Gradle Plugin**
  * `GUtil.toCamelCase(string)`
  * google/protobuf-gradle-plugin#604
  * #21900
  * `ConfigureUtil.configure(closure, target)`
  * google/protobuf-gradle-plugin#577
  * #21901
* **OSDetector Gradle Plugin (via the Protobuf plugin)**
  * `VersionNumber.*`
  * google/osdetector-gradle-plugin#24
  * #21902
* **Asciidoctor Gradle Plugin / Grolifant**
  * `CollectionUtils.stringize(collection)`
  * [GROLIFANT-85 - Grolifant should not use the deprecated CollectionUtils](https://gitlab.com/ysb33rOrg/grolifant/-/issues/85)
  * #21903
* **Nebula Lint Plugin**
  * `GUtil.loadProperties(inputStream)`
  * nebula-plugins/gradle-lint-plugin#378
  * #21912
* **Nebula Dependency Lock Plugin**
  * `NameMatcher`
  * nebula-plugins/gradle-dependency-lock-plugin#243
  * #21913
* **IntelliJ IDEA**
  * `GUtil.toCamelCase(string)`
  * `GUtil.toLowerCamelCase(string)`
  * `GUtil.loadProperties(inputStream)`
  * `ConfigureUtil.configureByMap(properties, delegate)`
  * [IDEA-301430 - IntelliJ IDEA should not use deprecated types from the org.gradle.util package](https://youtrack.jetbrains.com/issue/IDEA-301430/IntelliJ-IDEA-should-not-use-deprecated-types-from-the-orggradleutil-package)
  * #21912
  * #21900
  * #21914

Co-authored-by: Bo Zhang <[email protected]>
@rougsig rougsig deleted the convention-extension branch September 29, 2022 20:02
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.

Migrate ProtobufConvention to Gradle extensions
3 participants