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

feat: Using dependency walking to verify JDK9 Plugin works #1065

Merged
merged 9 commits into from
Jan 30, 2024

Conversation

Roiocam
Copy link
Member

@Roiocam Roiocam commented Jan 28, 2024

Motivation

solve #1040

Use dependency walk(inspired by sbt inspect) to detect JDK9 plugin works or not.

Benefits:

  • No need to compile and package, which means no need unzip either.
  • Fast detect(only few seconds)
  • Could verify in one or all modules.

Usage

It is easier to use, with only one command.

sbt dependWalkerCheck

Verify It

You can verify this plugin works by using two ways:

  • Rollback the change of Fix JDK9 plugin when using OSGi #1047, and then run command sbt jdk9Check
  • enable this plugin on other jdk8-only module(i.e. actor), and then run command sbt actor/jdk9Check

@Roiocam Roiocam marked this pull request as draft January 28, 2024 11:17
@He-Pin He-Pin marked this pull request as ready for review January 28, 2024 11:18
@Roiocam Roiocam marked this pull request as draft January 28, 2024 11:18
project/Jdk9Check.scala Outdated Show resolved Hide resolved
@Roiocam
Copy link
Member Author

Roiocam commented Jan 28, 2024

@mdedetrich Do you have time to review this? It solves #1040 and is simpler.

@Roiocam Roiocam marked this pull request as ready for review January 28, 2024 12:20
Copy link
Member Author

@Roiocam Roiocam left a comment

Choose a reason for hiding this comment

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

Explain.

project/Jdk9Check.scala Outdated Show resolved Hide resolved
project/Jdk9Check.scala Outdated Show resolved Hide resolved
project/Jdk9Check.scala Outdated Show resolved Hide resolved
@Roiocam
Copy link
Member Author

Roiocam commented Jan 28, 2024

One more update, when i had apply this plugin check for all JDK9 modules, some of these complained CompileJdk9 wasn't in the tree.

It is a good signal because these modules do not have JDK9 classes to be packaged, or not even producing a jar.

截屏2024-01-28 21 36 00

Copy link
Contributor

@mdedetrich mdedetrich left a comment

Choose a reason for hiding this comment

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

lgtm. My only complaint is that we now seem to have multiple JDK9 checks (@He-Pin also wrote one). While there technically isn't a problem with having multiple checks in this case it can be confusing for the readers of the code and I have a slight preference for this variant since its actually checking the root issue and importantly not checking for things that historically hasn't been issues

@He-Pin
Copy link
Member

He-Pin commented Jan 29, 2024

Should be fine, this is pre-checking and my PR is post-checking, we chatted on WeChat about how to check the jdk9 classes and come up with different solutions.

@Roiocam Roiocam force-pushed the detect-jdk9-by-walking branch from b1b4316 to 5aaf2a2 Compare January 29, 2024 09:27
Copy link
Member

@He-Pin He-Pin left a comment

Choose a reason for hiding this comment

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

looks good to me, just naming thing.

@He-Pin He-Pin changed the title Using dependency walking to verify JDK9 works feat: Using dependency walking to verify JDK9 Plugin works Jan 29, 2024
Roiocam added a commit to Roiocam/pekko that referenced this pull request Jan 29, 2024
Signed-off-by: Andy.Chen <[email protected]>
@He-Pin He-Pin added this to the 1.1.0-M1 milestone Jan 29, 2024
@He-Pin He-Pin merged commit 8a759ba into apache:main Jan 30, 2024
18 checks passed
@Roiocam Roiocam deleted the detect-jdk9-by-walking branch January 30, 2024 06:45
Roiocam added a commit to Roiocam/pekko that referenced this pull request Jan 30, 2024
Signed-off-by: Andy.Chen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants