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

add CI check for the Java 9+ classes #1040

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

add CI check for the Java 9+ classes #1040

pjfanning opened this issue Jan 25, 2024 · 12 comments · Fixed by #1064
Assignees
Labels
build sbt or build of the project help wanted Extra attention is needed
Milestone

Comments

@pjfanning
Copy link
Contributor

see #1039

I think we could have a check that uses a unzip -l call and that searches the output for expected class files.

There are other ways to check too.

@pjfanning pjfanning added this to the 1.1.0 milestone Jan 25, 2024
@He-Pin
Copy link
Member

He-Pin commented Jan 25, 2024

How about using scala-cli
https://scala-cli.virtuslab.org/docs/reference/directives to verfify it.
https://scala-cli.virtuslab.org/docs/cookbooks/introduction/gh-action/
seems like we can use it in github action and use the local repo

@He-Pin He-Pin added build sbt or build of the project help wanted Extra attention is needed labels Jan 25, 2024
@pjfanning
Copy link
Contributor Author

How about using scala-cli https://scala-cli.virtuslab.org/docs/reference/directives to verfify it. https://scala-cli.virtuslab.org/docs/cookbooks/introduction/gh-action/ seems like we can use it in github action and use the local repo

If you want to try using this, that's fine. The links provided do not appear to mention how to check a jar for the presence of a file. If implement this, I will use the easier solution of using the unzip command.

@Roiocam
Copy link
Member

Roiocam commented Jan 25, 2024

Too complex for me, if let me do this, I will be trying there easy way:

  • create a wrong test on java-jdk-9 path, and then run it on verify CI. If the unit test failed, that means jdk9 path was compiled!
  • verify unmanagedSourceDirectories before build

@He-Pin
Copy link
Member

He-Pin commented Jan 25, 2024

seems using jar xf and then test if find will works too, and simpler. I was thinking using sbt to generate an scala-cli.sc and test the local published jars.

@He-Pin
Copy link
Member

He-Pin commented Jan 25, 2024

I think checking the unmanagedSourceDirectores will not work:

//sbt with on Java 21
sbt:pekko-stream> show sourceDirectories
[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+
[info] * C:\Users\hepin\IdeaProjects\incubator-pekko\stream\target\scala-2.13\src_managed\main

@pjfanning
Copy link
Contributor Author

I think checking the unmanagedSourceDirectores will not work:

//sbt with on Java 21
sbt:pekko-stream> show sourceDirectories
[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+
[info] * C:\Users\hepin\IdeaProjects\incubator-pekko\stream\target\scala-2.13\src_managed\main

I spotted that too. Something about the way the Jdk9 stuff works seems not to expose the extra entries in this show. I still think checking the jar is better than checking sbt properties.

@He-Pin
Copy link
Member

He-Pin commented Jan 25, 2024

The only benefits of scala-cli jdk9features.sc is flexible but complex enough, need two tools in sync.

Sbt generates the scripts and scala cli run it

@mdedetrich
Copy link
Contributor

an sbt-scripted test is probably the easiest way to implemented this. Such a test can run the publishLocal command, unzip the jar and check the existence of a file within the jar.

You can read about it at https://www.scala-sbt.org/1.x/docs/Testing-sbt-plugins.html

@Roiocam
Copy link
Member

Roiocam commented Jan 25, 2024

Try to check the result via the reason for the problem.

After some digging, found the reason why CompileJDK9 is broken when using it with OSGi: https://github.com/sbt/sbt-osgi/pull/64/files#diff-1cb4aa05c813d70c1f075d72300bb104f2d4f5f192264b426e982bd335b693e0L47-R47

I use sbt inspect to dig out why:

  • use inspect tree actor-typed / package on sbt-osgi-0.9.4, CompileJDK9 contains in the dependency
  • use inspect tree actor-typed on sbt-osgi-0.9.11, CompileJDK9 wan't contains in the dependency

I have tried using fullClasspathAsJars and unmanagedSourceDirectories to replace fullClassPath, but neither one can fix this.

Not sure why, but sbt inspect tree actor-typed/package won't work, if it has a way to make it work, we can use something like this for the CI check.

sbt inspect tree actor-typed/package | grep -q CompileJdk9 || exit 1

@mdedetrich
Copy link
Contributor

mdedetrich commented Jan 25, 2024

@Roiocam Yes this I already know, if you want to go down the rabbit hole you can read #1039 (comment) and sbt/sbt-osgi#102

I am about to do a release of sbt-osgi that contains this fix sbt/sbt-osgi#121 which may solve an issue in trying to solve this the easy way (i.e. using inConfig(Jdk9Compile)(SbtOsgiPlugin.projectSettings)) otherwise I will have to do some other changes.

@Roiocam
Copy link
Member

Roiocam commented Jan 26, 2024

@Roiocam Yes this I already know, if you want to go down the rabbit hole you can read #1039 (comment) and sbt/sbt-osgi#102

I am about to do a release of sbt-osgi that contains this fix sbt/sbt-osgi#121 which may solve an issue in trying to solve this the easy way (i.e. using inConfig(Jdk9Compile)(SbtOsgiPlugin.projectSettings)) otherwise I will have to do some other changes.

I have created a fixed PR after this digging, it solves the JDk9 compile for me, can you take a look? Thanks.

@mdedetrich
Copy link
Contributor

mdedetrich commented Jan 26, 2024

@Roiocam Since your fix is more correct maybe you can also apply a similar fix instead of what was done in #920, i.e. instead of adding the sbt-assembly jar to OsgiKeys.explodedJars as was done at https://github.com/apache/incubator-pekko/pull/920/files#diff-49fc9a69964c909b343968bdcb2de99b610b6122ec40900ccab4f2e532f2915aR84-R90, instead you can modify the dependencyClasspathAsJars here as you did in your PR at https://github.com/apache/incubator-pekko/pull/1047/files#diff-378ca99a990595050ef83c6d976edc6e26e23b49622c4bd3ca5acda8bfb411ffR58 .

Might be easiest to modify the PR at #1047 to include both fixes if you can get it to work (if you do so just ping me and I will re-review).

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 help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants