Skip to content

Conversation

@Vampire
Copy link
Contributor

@Vampire Vampire commented Jun 4, 2024

Fixes #313

@bdellegrazie
Copy link
Collaborator

@Vampire would you be willing to rebase this please if the solution is still appropriate?

@ursjoss
Copy link

ursjoss commented Nov 12, 2025

@Vampire In case you don't have capacity to do the rebase, I could jump in and create a new PR based on your changes. I would be interested in getting this change into a new version.

@Vampire
Copy link
Contributor Author

Vampire commented Nov 12, 2025

Thanks for the reminder, rebase done

@bdellegrazie
Copy link
Collaborator

Hi @Vampire - thank you for this. I'll let @chadlwilson comment on the code itself.

One thing I noticed when looking at this was the deprecation of the ArtifactResolutionQuery API by Gradle and their
recommendation to use Artifact Views
instead.

Would it be difficult to convert this PR to use this newer API? Can you perceive of a reason why it should not be?

@Vampire
Copy link
Contributor Author

Vampire commented Nov 12, 2025

Because it is not possible :-)

@Vampire
Copy link
Contributor Author

Vampire commented Nov 12, 2025

Also - unless something changed - this is not deprecated.
It is legacy and not variant-aware and so on.
But that is exactly what we want here.
And as far as I heard from Gradle folks, the ARQ will also not become deprecated for now, as there exactly is no proper replacement and noone has time to develop one.

Artifact Views are great for what you can use them and much better, at least if the Gradle Module Metadata is either correct, or absent.
But for retrieving the POM, the ARQ is still the only proper Gradle-y way I'm aware of.

@chadlwilson
Copy link
Contributor

chadlwilson commented Nov 12, 2025

Ahh, classic Gradle. 😅 The way the docs read, the views can only be used for sources and javadoc?

The way the problem is described in #313 made it sound like the issue was that the current approach needed to be variant and attribute aware, so if the ARQ is not variant aware, it's a bit confusing as to how it addresses the problem (or whether it solves some cases but not others) - but I'll have a look myself as I'm not an expert.

But I also don't have an issue with using ARQs if it does what we want and is not marked as deprecated.

FWIW, the dependency-check-gradle plugin uses ARQs in a similar way for this purpose although it's recent and I don't know its limitations: https://github.com/dependency-check/dependency-check-gradle/blob/d4756bff43880d27d18e13451f7b8d027878b0fd/src/main/groovy/org/owasp/dependencycheck/gradle/tasks/AbstractAnalyze.groovy#L442

@bdellegrazie
Copy link
Collaborator

@Vampire thanks for the correction, my mistake.
@chadlwilson no objection from me - are you okay to merge?

@chadlwilson
Copy link
Contributor

@bdellegrazie i haven’t looked at it yet. Would like to take a more detailed look in IDE as I’m not deeply familiar with the APIs here, but it’s midnight here, so will have to wait :)

@Vampire
Copy link
Contributor Author

Vampire commented Nov 12, 2025

thanks for the correction, my mistake.

No problem, I could also have overlooked something, that's what we have code reviews for. :-)

@Vampire
Copy link
Contributor Author

Vampire commented Nov 12, 2025

The way the docs read, the views can only be used for sources and javadoc?

Yes and no.

You can use the artifact view to get variants and even use artifact transforms to transform artifacts into another form.

If there is Gradle Module Metadata, and it contains a proper variant for sources and / or javadoc, you can retrieve those using an artifact view.

If there is no Gradle Module Metadata present, and the sources and / or javadoc jars are published, it will also work, because for POM-only dependencies Gradle makes up some conventional varaints, including the sources and javadoc variants.

But if the sources and / or javadoc jars are published, and there is a Gradle Module Metadata, and it does not contain variants for those jars, then you will not find them using an artifact view, as the GMM is the source of truth if present, so the publisher made the decision (no matter whether intentionally or accidentally) to not provide those jars as Gradle variants for whatever reason.

If one for example uses artifact(sourcesJar) and artifact(javadocJar), it does exactly what it says, it just publishes along those pure artifacts without adding them to any metadata. If you instead used withJavadocJar() and / or withSourcesJar(), those define the tasks for you and properly publish them as feature variants in the GMM.

The way the problem is described in #313 made it sound like the issue was that the current approach needed to be variant and attribute aware, so if the ARQ is not variant aware, it's a bit confusing as to how it addresses the problem (or whether it solves some cases but not others) - but I'll have a look myself as I'm not an expert.

Don't miss the followup comment.
The original description was without much knowledge about what you do.
Then I found that you just use it to get the POM and the POM is not variant- / attribute-aware.
Gradle and the GMM are variant- / attribute-aware.
And if there were a variant of the libraries that has the POM as artifact, you could use that with a variant- / attribute-aware mechanism.
But as all you want to get is the POM, variants and attributes are irrelevant and so you can just use the ARQ to get the POM.

@chadlwilson
Copy link
Contributor

Well, the goal is to get <licenses> metadata which I believe is only in POM and not part of the GMM spec, so yeah, agree that it doesn't seem the GMM or artifact views (in their current state) are relevant.

@Vampire
Copy link
Contributor Author

Vampire commented Nov 12, 2025

Exactly :-)

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.

License of JavaFX modules cannot be determined - probably the same with Kotlin Multiplatform libraries - a.k.a. variant- / attribute-awareness

4 participants