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

1.1.x pekko snapshots are no longer including the java 9+ classes #1039

Closed
Tracked by #1052
pjfanning opened this issue Jan 24, 2024 · 18 comments · Fixed by #1064
Closed
Tracked by #1052

1.1.x pekko snapshots are no longer including the java 9+ classes #1039

pjfanning opened this issue Jan 24, 2024 · 18 comments · Fixed by #1064
Assignees
Labels
build sbt or build of the project high priority required for next release
Milestone

Comments

@pjfanning
Copy link
Contributor

Have a look at the projects/Jdk9.scala file.

The classes that this is supposed to build appear to be no longer built.

Java 1.0.x jars still have the expected classes.

Compare
https://repository.apache.org/content/groups/snapshots/org/apache/pekko/pekko-cluster-sharding_2.13/1.0.2+26-870129ef-SNAPSHOT/

to
https://repository.apache.org/content/groups/snapshots/org/apache/pekko/pekko-cluster-sharding_2.13/1.1.0-M0+283-e597a702-SNAPSHOT/

Have a look for this Java9+ class.
org.apache.pekko.cluster.sharding.internal.jfr.JFRShardingFlightRecorder

Included in the 1.0 jar but not in the 1.1 jar.

This appears to be happening in other jars where we have Jdk9(+) classes.

@pjfanning pjfanning added the high priority required for next release label Jan 24, 2024
@pjfanning pjfanning added this to the 1.1.0 milestone Jan 24, 2024
@mdedetrich
Copy link
Contributor

We need to bisect the commit where this happened

@He-Pin
Copy link
Member

He-Pin commented Jan 24, 2024

This is a blocker for 1.1.0, we are published with java 11 , this should not happen.

@pjfanning
Copy link
Contributor Author

Last pekko-cluster-sharding_2.13 snapshot to have JFR classes is 1.1.0-M0+221-959c98db-SNAPSHOT - Jan 9

Missing in next snapshot - 1.1.0-M0+224-ccce5c04-SNAPSHOT (Jan 11)

@pjfanning
Copy link
Contributor Author

nothing obvious but b9d2cc6 was in Jan 9 and that sbt-osgi plugin has been causing us serious grief

@He-Pin He-Pin added the build sbt or build of the project label Jan 24, 2024
@pjfanning
Copy link
Contributor Author

@mdedetrich I reverted b9d2cc6 locally and it fixed the issue with missing JDK9 classes

@He-Pin
Copy link
Member

He-Pin commented Jan 24, 2024

Run with java 11

sbt:pekko-stream> show unmanagedSourceDirectories
[info] * C:\Users\hepin\IdeaProjects\incubator-pekko\stream\src\main\scala
[info] * C:\Users\hepin\IdeaProjects\incubator-pekko\stream\src\main\scala-2.13
[info] * C:\Users\hepin\IdeaProjects\incubator-pekko\stream\src\main\scala-2
[info] * C:\Users\hepin\IdeaProjects\incubator-pekko\stream\src\main\java
[info] * C:\Users\hepin\IdeaProjects\incubator-pekko\stream\src\main\scala-2.13+
sbt:pekko-stream>

@mdedetrich
Copy link
Contributor

@mdedetrich I reverted b9d2cc6 locally and it fixed the issue with missing JDK9 classes

Thanks I'll look into it

@mdedetrich
Copy link
Contributor

mdedetrich commented Jan 24, 2024

So I already know what the likely cause is, its another fallout from sbt/sbt-osgi#64

@He-Pin
Copy link
Member

He-Pin commented Jan 24, 2024

We will need a task to verify the jar contains these Java 9+ classes in the nightly build after packaging.

@mdedetrich
Copy link
Contributor

So I can indeed confirm that its sbt/sbt-osgi#64, if I print out what the Compile / fullClasspath is for stream (which is what contains JDK 9 classes) you can see

[info] * Attributed(/Users/mdedetrich/github/incubator-pekko/stream/target/scala-2.13/classes)
[info] * Attributed(/Users/mdedetrich/github/incubator-pekko/actor/target/scala-2.13/classes)
[info] * Attributed(/Users/mdedetrich/github/incubator-pekko/protobuf-v3/target/scala-2.13/stripped/stripped/pekko-protobuf-v3-assembly-1.0.2+27-46618d08-SNAPSHOT.jar)
[info] * Attributed(/Users/mdedetrich/github/incubator-pekko/protobuf-v3/target/scala-2.13/stripped/pekko-protobuf-v3-assembly-1.0.2+27-46618d08-SNAPSHOT.jar)
[info] * Attributed(/Users/mdedetrich/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/scala-library/2.13.11/scala-library-2.13.11.jar)
[info] * Attributed(/Users/mdedetrich/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/reactivestreams/reactive-streams/1.0.4/reactive-streams-1.0.4.jar)
[info] * Attributed(/Users/mdedetrich/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/com/typesafe/ssl-config-core_2.13/0.6.1/ssl-config-core_2.13-0.6.1.jar)
[info] * Attributed(/Users/mdedetrich/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/com/typesafe/config/1.4.3/config-1.4.3.jar)
[info] * Attributed(/Users/mdedetrich/github/incubator-pekko/stream/target/scala-2.13/CompileJdk9-classes)

As you can see the last line is the compiled jdk9 classes which are missing

However if we print out the replacement which is (dependencyClasspathAsJars in Compile).value.map(_.data) ++ (products in Compile) then we get this

[info] * Attributed(/Users/mdedetrich/github/incubator-pekko/actor/target/scala-2.13/stripped/pekko-actor_2.13-1.0.2+27-46618d08-SNAPSHOT.jar)
[info] * Attributed(/Users/mdedetrich/github/incubator-pekko/protobuf-v3/target/scala-2.13/stripped/stripped/pekko-protobuf-v3-assembly-1.0.2+27-46618d08-SNAPSHOT.jar)
[info] * Attributed(/Users/mdedetrich/github/incubator-pekko/protobuf-v3/target/scala-2.13/stripped/pekko-protobuf-v3-assembly-1.0.2+27-46618d08-SNAPSHOT.jar)
[info] * Attributed(/Users/mdedetrich/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/scala-library/2.13.11/scala-library-2.13.11.jar)
[info] * Attributed(/Users/mdedetrich/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/reactivestreams/reactive-streams/1.0.4/reactive-streams-1.0.4.jar)
[info] * Attributed(/Users/mdedetrich/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/com/typesafe/ssl-config-core_2.13/0.6.1/ssl-config-core_2.13-0.6.1.jar)
[info] * Attributed(/Users/mdedetrich/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/com/typesafe/config/1.4.3/config-1.4.3.jar)

plus

[info] * /Users/mdedetrich/github/incubator-pekko/stream/target/scala-2.13/classes

As we can see its missing the JDK 9 classes folder. I will need to do changes in sbt-osgi upstream to fix this

@mdedetrich
Copy link
Contributor

So I already have a PR ready for sbt-osgi that will allow me to fix this issue, just waiting for sbt/sbt-osgi#118 to get merged.

Should be fixed in a couple of days

@mdedetrich mdedetrich self-assigned this Jan 25, 2024
@He-Pin
Copy link
Member

He-Pin commented Jan 25, 2024

after revert the osgi to 0.9.4 and publish local
image
before
image

@mdedetrich yes, it's osgi plugin's problem.

@Roiocam
Copy link
Member

Roiocam commented Jan 25, 2024

#959 It seems to be caused by this problem.

@Roiocam
Copy link
Member

Roiocam commented Jan 25, 2024

@mdedetrich I reverted b9d2cc6 locally and it fixed the issue with missing JDK9 classes

@pjfanning Thanks for investigating , this problem is also led to #959.

@mdedetrich
Copy link
Contributor

I am quite sure that the core problem is what I described at #1039 (comment) which is leading to all of the other described issues. There might be a simple solution for this that doesn't require extending sbt-osgi with additional functionality but I need to first merge + release sbt/sbt-osgi#121 .

@Roiocam
Copy link
Member

Roiocam commented Jan 27, 2024

@pjfanning I think this has been solve by #1047

@pjfanning
Copy link
Contributor Author

@pjfanning I think this has been solve by #1047

I disagree - #1040 is to test #1039 and #1039 is not even fixed yet. There is zero proof that this is a fix for #1040.

@Roiocam
Copy link
Member

Roiocam commented Jan 27, 2024

I disagree - #1040 is to test #1039 and #1039 is not even fixed yet. There is zero proof that this is a fix for #1040.

My last repeated comment was published in the wrong ISSUE. This comment was published under #1039, that is, we have fixed the compile of the JDK9 class on the package task.

You can verify it on latestmain branch.

@He-Pin He-Pin linked a pull request Jan 28, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build sbt or build of the project high priority required for next release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants