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.

Sign up for GitHub

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

Jump to bottom

Add jmods #135

Closed
wants to merge 1 commit into from
Closed

Add jmods #135

wants to merge 1 commit into from

Conversation

Sineaggi
Copy link

The jmods is important for anybody attempting to use the jlink command with a custom --add-modules flag.

@Sineaggi Sineaggi requested review from comius and a team as code owners September 27, 2023 06:48
@Sineaggi Sineaggi mentioned this pull request Sep 27, 2023
Closed
@hvadehra
Copy link
Member

@cushon What are your thoughts about this?

@cushon
Copy link
Collaborator

cushon commented Oct 23, 2023

Internally we defined a separate java_runtime target that includes the jmods for use-cases where they are needed. They aren't needed for most actions, and omitting them avoids shipping some extra inputs to those actions.

@Sineaggi
Copy link
Author

Internally we defined a separate java_runtime target that includes the jmods for use-cases where they are needed. They aren't needed for most actions, and omitting them avoids shipping some extra inputs to those actions.

Would it make sense to have something similar in rules_java (a separate target), or to add this?

@hvadehra
Copy link
Member

hvadehra commented Nov 7, 2023

Would it make sense to have something similar in rules_java (a separate target), or to add this?

That sounds reasonable.

@Sineaggi Could you please declare a separate java_runtime with the name jdk_with_jmods and add this new filegroup to the sources of that?

@hvadehra hvadehra added question Further information is requested P3 We're not considering to work on this, but happy to review a PR. (No assignee) labels Mar 15, 2024
@srdo-humio srdo-humio mentioned this pull request Apr 18, 2024
Closed
@hvadehra
Copy link
Member

hvadehra commented May 7, 2024

Closing in favor of #184

@hvadehra hvadehra closed this May 7, 2024
copybara-service bot pushed a commit that referenced this pull request May 7, 2024
@srdo-humio
Make jmods available from the jdk BUILD file
69dd554 
Copybara Import from #184

BEGIN_PUBLIC
Make jmods available from the jdk BUILD file (#184)

The PR at #135 looks dormant, this makes the change requested at #135 (comment)

I don't need the entire change here, having the jmods as a filegroup is sufficient for using jlink, but @cushon suggested adding the files as sources to the java_runtime as well, if I understood him correctly, so I did that too. If that's not necessary, let me know and I'll remove that part.

Closes #184
END_PUBLIC

COPYBARA_INTEGRATE_REVIEW=#184 from srdo-humio:stig/jdk-with-jmods e1513db
PiperOrigin-RevId: 631308886
Change-Id: I455d1ff6abd8d5b92e0e25f92caed796fe019aec
@Sineaggi
Copy link
Author

Sineaggi commented May 7, 2024

@hvadehra sorry this fell off my plate, I checked out #184 and it seems like that solves the issue for us. When can we expect a new release of rules_java containing that fix?

@Sineaggi Sineaggi deleted the add-jmods branch May 7, 2024 23:57
@hvadehra
Copy link
Member

hvadehra commented May 8, 2024

When can we expect a new release of rules_java containing that fix

Hopefully soon, I opened #186 for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@comius comius Awaiting requested review from comius comius is a code owner

Assignees
No one assigned
Labels
P3 We're not considering to work on this, but happy to review a PR. (No assignee) question Further information is requested
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Sineaggi @hvadehra @cushon