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

Simplify declaration of module path in sbt build #10836

Merged
merged 8 commits into from
Aug 19, 2024

Conversation

Akirathan
Copy link
Member

@Akirathan Akirathan commented Aug 16, 2024

Closes #10726

Pull Request Description

Simplify declarations of module path in the whole sbt build script by introducing a new task key moduleDependencies that has the same format as libraryDependencies. Everything from moduleDependencies will be added on --module-path by the JPMSPlugin.

Note that it is not possible to derive module path directly from libraryDependencies as explained in the issue comment.

Important Notes

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • The documentation has been updated, if necessary.
  • Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
  • All code follows the
    Scala,
    Java,
    TypeScript,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.
  • Unit tests have been written where possible.

@Akirathan Akirathan added CI: No changelog needed Do not require a changelog entry for this PR. CI: Clean build required CI runners will be cleaned before and after this PR is built. labels Aug 16, 2024
@Akirathan Akirathan self-assigned this Aug 16, 2024
),
streams.value.log,
shouldContainAll = true
moduleDependencies := {
Copy link
Member Author

Choose a reason for hiding this comment

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

Same format as libraryDependencies. No longer need to call this cryptic filterModulesFromUpdate methods ...

GraalVM.modules ++ GraalVM.langsPkgs ++ logbackPkg ++ helidon ++ Seq(
"org.slf4j" % "slf4j-api" % slf4jVersion,
"org.netbeans.api" % "org-netbeans-modules-sampler" % netbeansApiVersion,
(`runtime-fat-jar` / projectID).value,
Copy link
Member Author

Choose a reason for hiding this comment

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

This is how you specify that you want to put runtime-fat-jar/target/classes directory on module-path. I have found no way to infer this directly.

Copy link
Member

@JaroslavTulach JaroslavTulach left a comment

Choose a reason for hiding this comment

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

I like the approach. There is a buildNativeImage failure to be fixed. Hopefully straightforward.

syntaxMod,
profilingMod
Test / moduleDependencies := {
GraalVM.modules ++ GraalVM.langsPkgs ++ GraalVM.insightPkgs ++ logbackPkg ++ helidon ++ Seq(
Copy link
Member

Choose a reason for hiding this comment

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

This looks pretty understandable! It is a format I hope to be able to work with.

Copy link
Member Author

Choose a reason for hiding this comment

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

I am glad. More info in the scaladocs of JPMSPlugin

modulePath := Seq.empty,
moduleDependencies := Seq.empty,
// modulePath is set based on moduleDependencies
modulePath := {
Copy link
Member

Choose a reason for hiding this comment

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

Do we still need to expose modulePath? Shouldn't that be hidden as an internal detail of JPMSPlugin?

Copy link
Member Author

@Akirathan Akirathan Aug 19, 2024

Choose a reason for hiding this comment

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

Unfortunately yes. There are still some places that directly access modulePath, like

enso/build.sbt

Line 1307 in 906442f

val mp = modulePath.value ++ (`profiling-utils` / modulePath).value

Generally, accessing modulePath setting is needed whenever you would like to assembly your own cmd line options. modulePath is then very useful, as it is of type Seq[File], whereas moduleDependencies is of type Seq[ModuleID] and getting files from that is difficult.

@Akirathan Akirathan merged commit 00abfc4 into develop Aug 19, 2024
41 checks passed
@Akirathan Akirathan deleted the wip/akirathan/improve-jpms-plugin branch August 19, 2024 17:39
Akirathan added a commit that referenced this pull request Aug 27, 2024
runtime-fat-jar has to be as Jar on module-path, not exploded.
@Akirathan Akirathan mentioned this pull request Aug 27, 2024
5 tasks
Akirathan added a commit that referenced this pull request Aug 28, 2024
* Ensure the annotation processor error message is not lost

* Dont limit ContextBuilder in BenchProcessor to enso language

* Add libraryDependencies on GraalVM.toolsPkgs to std-benchmarks

* Add some dependencies to std-benchmarks - fixes compilation

* Revert std-benchmarks project settings from #10836.

runtime-fat-jar has to be as Jar on module-path, not exploded.

* fmt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: Clean build required CI runners will be cleaned before and after this PR is built. CI: No changelog needed Do not require a changelog entry for this PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use transitive dependency resolution in JPMSPlugin
2 participants