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

Gradle plugin: Replace findProperty with Isolated Project compatible … #811

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

hfhbd
Copy link

@hfhbd hfhbd commented Sep 29, 2024

…api gradleProperty

Summary

findProperty is not compatible with Gradle Isolated Projects, but providers.gradleProperty is. To test this change, you need an Isolated Projects compatible build, eg plain java library.

Release Note

Add support for Gradle Isolated Projects.

Documentation

@loosebazooka
Copy link
Member

I think this is fine, but I'll defer to @vlsi

@loosebazooka
Copy link
Member

I guess we haven't had many forked PR created. Tests are failing on the availability of OIDC... which shouldn't be affected by this. I can override for the merge once we get approval.

@vlsi
Copy link
Collaborator

vlsi commented Oct 2, 2024

Unfortunately, providers.gradleProperty is not the same as findProperty since providers.gradleProperty queries the values from the root project only which is conter-intuitive.
See gradle/gradle#23572

Do you know if there's an alternative?
Do you know if hasProperty supports isolated projects?

Could you add a test case so we know "isolated project" is supported or not?
Otherwise it could be we replace findProperty and gain nothing with it

@hfhbd
Copy link
Author

hfhbd commented Oct 5, 2024

Yes, this is a change of the behavior but it is the only API compatible with isolated projects. hasProperty is also incompatible.

Will add a test.

@vlsi
Copy link
Collaborator

vlsi commented Oct 8, 2024

The mentioned property is an escape hatch, so it is probably fine to use providers.gradleProperty. Let's just merge it and be done with the PR.

A test with isolated projects would be very welcome.

Signed-off-by: hfhbd <[email protected]>
…in/dev/sigstore/sign/SigstoreSignExtension.kt

Co-authored-by: Vladimir Sitnikov <[email protected]>
Signed-off-by: Philip Wedemann <[email protected]>
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.

3 participants