Skip to content

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

Merged
vlsi merged 3 commits intosigstore:mainfrom
hfhbd:isolatedProject
Mar 6, 2025
Merged

Gradle plugin: Replace findProperty with Isolated Project compatible …#811
vlsi merged 3 commits intosigstore:mainfrom
hfhbd:isolatedProject

Conversation

@hfhbd
Copy link
Contributor

@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
Contributor 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.

@hfhbd
Copy link
Contributor Author

hfhbd commented Feb 10, 2025

Any update?

@loosebazooka
Copy link
Member

Oh I didn't realize @vlsi approved.

@loosebazooka
Copy link
Member

can you rebase and update the pr?

hfhbd and others added 3 commits February 11, 2025 22:20
…api gradleProperty

Signed-off-by: hfhbd <22521688+hfhbd@users.noreply.github.com>
Signed-off-by: hfhbd <22521688+hfhbd@users.noreply.github.com>
…in/dev/sigstore/sign/SigstoreSignExtension.kt

Co-authored-by: Vladimir Sitnikov <sitnikov.vladimir@gmail.com>
Signed-off-by: Philip Wedemann <22521688+hfhbd@users.noreply.github.com>
@hfhbd
Copy link
Contributor Author

hfhbd commented Feb 11, 2025

@loosebazooka rebased

@vlsi vlsi merged commit 69c55a6 into sigstore:main Mar 6, 2025
12 of 13 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.

3 participants

Comments