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

Overcome 'String too large to record' issue with Truffle #40549

Merged
merged 1 commit into from
May 10, 2024
Merged

Overcome 'String too large to record' issue with Truffle #40549

merged 1 commit into from
May 10, 2024

Conversation

geoand
Copy link
Contributor

@geoand geoand commented May 10, 2024 •
edited
Loading

This is better than the current state, but it is not yet the absolutely correct

@geoand
Overcome 'String too large to record' issue with Truffle
56bbb39 
This is better than the current state, but it is not
yet the absolutely correct

Relates: quarkusio#39387
@geoand geoand mentioned this pull request May 10, 2024
Open
gsmet
gsmet approved these changes May 10, 2024
View reviewed changes
core/deployment/src/main/java/io/quarkus/deployment/steps/ClassPathSystemPropBuildStep.java
Comment on lines +48 to +50
List<String> allJarPaths = Stream.concat(parentFirst.stream(), regular.stream()).map(p -> p.toAbsolutePath().toString())
.toList();
recorder.set(allJarPaths);
Copy link
Member

Choose a reason for hiding this comment

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

I agree we could merge this as it's better to not fail but I'm thinking that we are definitely not registering the appropriate paths.
From what I can see, it points to the jars in ~/.m2, which is definitely not expected.

@aloubyansky I suppose we have a way to point to the jars relative to the application? I'm not entirely sure though if the path would be the same in the case of a legacy-jar vs fast-jar vs uber-jar?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree we could merge this as it's better to not fail but I'm thinking that we are definitely not registering the appropriate paths

Yup, I mentioned that in the issue as well

Copy link
Member

Choose a reason for hiding this comment

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

@aloubyansky I suppose we have a way to point to the jars relative to the application? I'm not entirely sure though if the path would be the same in the case of a legacy-jar vs fast-jar vs uber-jar?

It would depend on the packaging. In case of uber-jar it would be paths to the artifacts in the local Maven repo relative to the application root.

@quarkus-bot quarkus-bot
Copy link

quarkus-bot bot commented May 10, 2024 •
edited by github-actions bot
Loading

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 56bbb39.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

Flaky tests - Develocity

⚙️ JVM Tests - JDK 21

📦 extensions/opentelemetry/deployment

✖ io.quarkus.opentelemetry.deployment.OpenTelemetrySpanSecurityEventsTest.testSecurityEventTypes - History

  • event executor terminated - java.util.concurrent.RejectedExecutionException
java.util.concurrent.RejectedExecutionException: event executor terminated
	at io.netty.util.concurrent.SingleThreadEventExecutor.reject(SingleThreadEventExecutor.java:934)
	at io.netty.util.concurrent.SingleThreadEventExecutor.offerTask(SingleThreadEventExecutor.java:351)
	at io.netty.util.concurrent.SingleThreadEventExecutor.addTask(SingleThreadEventExecutor.java:344)
	at io.netty.util.concurrent.SingleThreadEventExecutor.execute(SingleThreadEventExecutor.java:836)
	at io.netty.util.concurrent.SingleThreadEventExecutor.execute0(SingleThreadEventExecutor.java:827)
	at io.netty.util.concurrent.SingleThreadEventExecutor.execute(SingleThreadEventExecutor.java:817)
	at io.vertx.core.impl.EventLoopExecutor.execute(EventLoopExecutor.java:35)

@geoand geoand merged commit e528e79 into quarkusio:main May 10, 2024
51 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.11 - main milestone May 10, 2024
@geoand geoand deleted the #39387-first-take branch May 10, 2024 14:57
@gsmet gsmet modified the milestones: 3.11 - main, 3.10.1 May 10, 2024
aloubyansky
aloubyansky reviewed May 13, 2024
View reviewed changes
core/runtime/src/main/java/io/quarkus/runtime/ClassPathSystemPropertyRecorder.java
public void set(String value) {
System.setProperty("java.class.path", value);
public void set(List<String> allJarPaths) {
System.setProperty("java.class.path", String.join(":", allJarPaths));
Copy link
Member

Choose a reason for hiding this comment

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

Is this meant to be OS-dependent?

@gsmet gsmet modified the milestones: 3.10.1, 3.8.5 May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aloubyansky aloubyansky aloubyansky left review comments

@gsmet gsmet gsmet approved these changes

Assignees
No one assigned
Labels
area/core triage/flaky-test
Projects
None yet
Milestone
3.8.5
Development

Successfully merging this pull request may close these issues.
3 participants
@geoand @aloubyansky @gsmet