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

Proof of concept for installation properties #18555

Closed
wants to merge 1 commit into from

Conversation

jbartok
Copy link
Member

@jbartok jbartok commented Oct 7, 2021

First attempts at a new API for Toolchain specs.

@jbartok jbartok self-assigned this Oct 7, 2021
@DPUkyle
Copy link
Contributor

DPUkyle commented Oct 9, 2021

Can you also share the dummy project we created to test this?

@jbartok
Copy link
Member Author

jbartok commented Oct 10, 2021

Can you also share the dummy project we created to test this?

java {
  toolchain {
    languageVersion = JavaLanguageVersion.of(11)
    installationProperties = withProperties {
      property JvmInstallationPropertiesSpec.JvmInstallationProperty.VM_VERSION, contains("11.0.12")
    }
  }
}

@bric3
Copy link
Contributor

bric3 commented Oct 29, 2021

Hi,

Thank you very much for taking this into consideration !

Looking at the code I ask myself a few observations

  1. The current angle is using the vocabulary of properties, I wonder why not reusing the notion of dimensions that is already used by the dependencies. What about this classification (it is a very naive one)

    • Incubating JDK: CRaC (currently variant of JDK17), Loom (currently variant of JDK18), Panama Foreign (Current variant of JDK17), etc. mutually exclusive
    • Patches: DCEVM, JCEF, etc. patches can be additive
    • Debug build, eg fastdebug
    • Java version (eg Java 8, Java 11, Java 17, etc)
    • Vendors (Temurin, Azul, Amazon)
    • Runtime version (eg GraalVM 21.3.0, also Mandrel 21.3.0, JDK 11.0.13, JDK 17.0.1, etc.)
  2. How those properties are acquired ? Some information can only be obtained by running the java --version, but maybe this is not enough in certain case. Which leads me to my second point.

  3. It's not advisable for Gradle to cover every cases out there, and to improve future-proof-ness, the property retrieval should be extendable.

    JvmInstallationMetadata is currently internal. Maybe there's a way to introduce some contributor to these metadata.

  4. This leads me to how to validate the property, do all these properties will be String ?


For reference here's the builds that I am playing with

And I am not even concerned (yet) by different architectures.

@ljacomet
Copy link
Member

ljacomet commented Nov 8, 2021

@bric3 I finally made an issue to describe our objective here. Could you take a look at #18896 and copy your comment / questions over there so they could be answered in that (better) context?

@bric3
Copy link
Contributor

bric3 commented Nov 14, 2021

@ljacomet Thank you I will do it.

@stale
Copy link

stale bot commented Jan 14, 2022

This pull request has been automatically marked as stale because it has not had recent activity. Given the limited bandwidth of the team, it will be closed if no further activity occurs. If you intend to work on this pull request, please ask the team to reopen the PR or push a new PR. Thank you for your contributions.

@stale stale bot added the stale label Jan 14, 2022
@jbartok jbartok removed their assignment Jan 20, 2022
@stale stale bot removed the stale label Jan 20, 2022
@stale
Copy link

stale bot commented Mar 22, 2022

This pull request has been automatically marked as stale because it has not had recent activity. Given the limited bandwidth of the team, it will be closed if no further activity occurs. If you intend to work on this pull request, please ask the team to reopen the PR or push a new PR. Thank you for your contributions.

@stale
Copy link

stale bot commented Apr 13, 2022

This pull request has been automatically closed due to inactivity. If you are still interested in contributing this, please ensure that it is rebased against the latest branch (usually master), all review comments have been addressed and the build is passing.

@stale stale bot closed this Apr 13, 2022
@bric3
Copy link
Contributor

bric3 commented Apr 13, 2022

@ljacomet @jbartok Is this one stale (or closed) ?

@ljacomet
Copy link
Member

This is a topic we hope to resume work on soon. I am not going to eagerly re-open this one though as the implementation may end up being quite different. If we end up reusing this, it can be re-opened then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in:toolchains Java Toolchains stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants