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

Apply legacy plugin last, and declare capabilities for old plugins #991

Merged
merged 6 commits into from
Oct 24, 2024

Conversation

jpenilla
Copy link
Contributor

@jpenilla jpenilla commented Sep 29, 2024

fixes #964 using the solution suggested by @ljacomet at #964 (comment)

Tested to fix the issue (bring behavior back to how it was before 8.3.1/with the old plugin), and to give an error when both the current and old plugin are on the classpath.

Something else that came to mind regarding the legacy plugin is that if someone puts the new plugin on the classpath, but then applies the legacy plugin id, nothing will happen, which could be confusing. But resolving that is likely better addressed in a separate PR (if at all).

> Could not resolve all files for configuration ':build-logic:compileClasspath'.
   > Could not resolve com.gradleup.shadow:shadow-gradle-plugin:8.3.3-SNAPSHOT.
     Required by:
         project :build-logic
      > Module 'com.gradleup.shadow:shadow-gradle-plugin' has been rejected:
           Cannot select module with conflict on capability 'com.github.johnrengelman:shadow:8.3.3-SNAPSHOT' also provided by [com.github.johnrengelman:shadow:8.1.1(apiElements)]
   > Could not resolve com.github.johnrengelman:shadow:8.1.1.
     Required by:
         project :build-logic
      > Module 'com.github.johnrengelman:shadow' has been rejected:
           Cannot select module with conflict on capability 'com.github.johnrengelman:shadow:8.1.1' also provided by [com.gradleup.shadow:shadow-gradle-plugin:8.3.3-SNAPSHOT(apiElements)]

  • CHANGELOG's "Unreleased" section has been updated, if applicable.

src/docs/changes/README.md Outdated Show resolved Hide resolved
@Goooler
Copy link
Member

Goooler commented Sep 29, 2024

CC @ljacomet

Copy link
Contributor

@ljacomet ljacomet left a comment

Choose a reason for hiding this comment

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

Sorry, I said I would attempt this and in the end did not find the time.

This LGTM!

The one thing I wondered was about the capability addition: should it be done in a patch release or in a minor?
My preference would be to do it in a minor as it might end up breaking builds (for good reasons) while the fix to the order of application fits perfectly a patch release.

@ljacomet
Copy link
Contributor

And thank you @jpenilla for taking care of fixing my mistake!

@Goooler
Copy link
Member

Goooler commented Oct 24, 2024

I'm planning to release this in 8.3.4, WDYT?

@Goooler Goooler enabled auto-merge (squash) October 24, 2024 08:31
@Goooler Goooler merged commit 8aad9ad into GradleUp:main Oct 24, 2024
9 checks passed
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.

Legacy plugin application order breaks shadow detection logic updated for new id
3 participants