Skip to content

Maven tweaks for usability#4039

Draft
headius wants to merge 3 commits intoruby:mainfrom
headius:maven_tweaks
Draft

Maven tweaks for usability#4039
headius wants to merge 3 commits intoruby:mainfrom
headius:maven_tweaks

Conversation

@headius
Copy link
Contributor

@headius headius commented Mar 24, 2026

Based on review comments from #4031 I've made a few additional tweaks.

  • Template sources are now generated into src/main/java-templates so they are not deleted by mvn clean.
  • The WASM build is generated to src/main/wasm since it is an input to the build (and not a test resource as before).
  • The build-java-truffleruby GHA job now uploads a copy of the generated sources for TruffleRuby, to make it easier to copy.

headius added 3 commits March 24, 2026 11:37
This path avoids the sources getting wiped out during `mvn clean`,
since they are not generated during the maven build.

This patch also moves the generated WASM build under src/main/wasm
since it is really a source file and not a test file. It will not
be included in the built artifact.
@headius
Copy link
Contributor Author

headius commented Mar 24, 2026

The archive of the sources generated specifically for TruffleRuby could be moved to a separate job if that would be cleaner.

@eregon
Copy link
Member

eregon commented Mar 24, 2026

Could Loader.java and Nodes.java simply be in java/api/src/main/java/org/ruby_lang/prism, as siblings of e.g ParseResult.java?
They are not generated by the Maven build and it's expected to run rake so it should be fine, no?

Comment on lines +204 to +206
jar --create --file java/prism-truffleruby-sources.jar -C java/api/src/main/java `ls -r java/api/src/main/java`
jar --update --file java/prism-truffleruby-sources.jar -C java/api/src/main/java-templates `ls -r java/api/src/main/java-templates`
jar --update --file java/prism-truffleruby-sources.jar -C java/native/src/main/java `ls -r java/native/src/main/java`
Copy link
Member

Choose a reason for hiding this comment

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

I appreciate the thought and effort but it's better to directly copy files from my Prism checkout, that way it also works if e.g. I have some local changes in Prism I'm testing in TruffleRuby.
So this can be removed.

package org.ruby_lang.prism;
package org.ruby_lang.prism.jni;

public abstract class Parser {
Copy link
Member

Choose a reason for hiding this comment

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

This class, which is the only class under java/native doesn't really do anything currently.
Notably the corresponding C code is not here, it's in https://github.com/truffleruby/truffleruby/blob/master/src/main/c/yarp_bindings/src/yarp_bindings.c.

Maybe we should actually remove this class? Or import the C file here?
If the C file would get compiled in CI that could be nice, as it'd catch cases that it needs to be updated, e.g. with the recent changes in the C API.

OTOH this is an area where usages might want different things (e.g. how the reading is done, maybe from a file/using mmap/etc), and notably truffleruby compiles that .c file together with libprism.a.

I'd tend to say the simplest is to just remove this file, but then maybe there is an issue because java/native is empty?

@kddnewton kddnewton added the java Pull requests that update Java code label Mar 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

java Pull requests that update Java code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants