[DUBBO-78] Add options configuration mechanism for JdkCompiler.

[XenoAmess]

What is the purpose of the change

I personally need this mechanism in one of my repos, (I have to copy/change this whole class now because not having such mechanism, I need a JdkCompiler to deal with java 1.8 codes)
and I think it helpful to other people too.

Brief changelog

JdkCompiler

Follow this checklist to help us incorporate your contribution quickly and easily:

• Make sure there is a GITHUB_issue field for the change (usually before you start working on it). Trivial changes like typos do not require a GITHUB issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
• Format the pull request title like [Dubbo-XXX] Fix UnknownException when host config not exist #XXX. Each commit in the pull request should have a meaningful subject line and body.
• Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
• Write necessary unit-test to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add sample in dubbo samples project.
• Run mvn clean install -DskipTests=false & mvn clean test-compile failsafe:integration-test to make sure unit-test and integration-test pass.
• If this contribution is large, please follow the Software Donation Guide.

[AlbumenJ]

Hi, @XenoAmess , you can customize a AbstractCompiler in your project using SPI.

[AlbumenJ]

LGTM.

[XenoAmess]

Hi, @XenoAmess , you can customize a AbstractCompiler in your project using SPI.

Yep, now I copied the JdkCompiler and changed it to a Jdk8Compiler.
It can do.
see https://github.com/XenoAmess/metasploit-java-external-module/blob/develop/src/loader/src/main/java/com/xenoamess/metasploit/java_external_module/loader/Jdk8Compiler.java

I just think things can be easier... because that be just duplicated codes lol.

[codecov-io]

Codecov Report

Merging #7232 (6496fe3) into master (33a547c) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master    #7232      +/-   ##
============================================
+ Coverage     60.00%   60.02%   +0.02%     
+ Complexity      289      288       -1     
============================================
  Files          1004     1004              
  Lines         40015    40016       +1     
  Branches       5935     5935              
============================================
+ Hits          24010    24021      +11     
+ Misses        13304    13294      -10     
  Partials       2701     2701              

Impacted Files	Coverage Δ	Complexity Δ
...che/dubbo/common/compiler/support/JdkCompiler.java	63.55% <100.00%> (+0.34%)	0.00 <0.00> (ø)
...in/java/org/apache/dubbo/common/utils/JVMUtil.java	81.13% <0.00%> (-11.33%)	0.00% <0.00%> (ø%)
...che/dubbo/remoting/transport/mina/MinaChannel.java	39.47% <0.00%> (-10.53%)	15.00% <0.00%> (-1.00%)
...o/remoting/transport/ChannelHandlerDispatcher.java	60.00% <0.00%> (-10.00%)	0.00% <0.00%> (ø%)
.../apache/dubbo/remoting/transport/AbstractPeer.java	58.69% <0.00%> (-4.35%)	0.00% <0.00%> (ø%)
...e/dubbo/remoting/transport/netty/NettyChannel.java	52.27% <0.00%> (-3.41%)	19.00% <0.00%> (-1.00%)
...ava/org/apache/dubbo/config/ServiceConfigBase.java	60.35% <0.00%> (-0.60%)	0.00% <0.00%> (ø%)
...figcenter/file/FileSystemDynamicConfiguration.java	62.38% <0.00%> (-0.48%)	0.00% <0.00%> (ø%)
...rg/apache/dubbo/common/timer/HashedWheelTimer.java	79.38% <0.00%> (-0.35%)	0.00% <0.00%> (ø%)
... and 9 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 33a547c...6496fe3. Read the comment docs.

[XenoAmess]

@AlbumenJ
still wondering...
why JdkCompiler.java be CRLF but JdkCompilerTest.java be LF?
please consider do a unify...

[chickenlj]

Hi @XenoAmess Thank you for the contribution. We have reviewed and merged some of your pull requests recently. However, all the changes are code reformats which do not match the overall evolve plan of the community and will bring us a lot of conflicts and verifications.

I think it will be better for us to focus on such refactoring when we arrive at a certain stage. We have labeled all PR as reopen later so that we can review these PRs later.

Please feel free to let us know if you have any questions.

[XenoAmess]

all the changes are code reformats

Not all of the prs are code reformats. for example, this one is not.

I wonder, have you at least read them all?

I will pin you to reopen the prs which be not reformat pr, and I hope you can at least find some time for them.

I think it will be better for us to focus on such refactoring when we arrive at a certain stage.

According to my former experience there will never be such a perfect stage.

But if you wanna delay some of them, I have no problem, as this is not my lib to maintain.

which do not match the overall evolve plan of the community

Well, you have different ideas about this with people in alibaba/p3c. As you are both alibaba team, I'm actually confused.

I thought you alibaba people will follow one same idea about code maintaining, and I heard every java repo in ali must pass p3c. Maybe I be wrong.

[XenoAmess]

@chickenlj this pr is a functional pr, should not be closed.

[AlbumenJ]

@XenoAmess

In my opinion, this pr should be reopened.

Why we temporarily close those reformatting PRs is because we are currently resolve most of previous PRs which created for servals months and if we merge those reformatting PRs may cause a lot of confilcts with the previous PRs .

Code maintaining is important, and we will make code clean enough once after we resolve those previous PRs.

We are sure that will reopen those reformatting PRs in the future, Also please feel free to notify us if we forget it.

[AlbumenJ]

@XenoAmess

still wondering...
why JdkCompiler.java be CRLF but JdkCompilerTest.java be LF?
please consider do a unify...

There are some file is in CRLF mode due to history problem.

In our opinion, if we do a unify for those files in CRLF mode, the git blame will not work properly.

Also, we cannot find if the file is in CRLF mode only from Github Pull Request website and it is sure that the files in the future PR in CRLF mode is unavoidable.

So, we choose we tolerate multi file formats and we have add .gitattributes file for this repo to make sure new contributors will not modify file format.

Thanks for your understanding.

[AlbumenJ]

LGTM.

[XenoAmess]

Also, we cannot find if the file is in CRLF mode only from Github Pull Request website and it is sure that the files in the future PR in CRLF mode is unavoidable.

This can be done using checkstyle plugin.

We can fail the build when there be CRLF file.

See example at commons-lang.

https://github.com/apache/commons-lang/blob/master/pom.xml#L754

https://github.com/apache/commons-lang/blob/master/src/site/resources/checkstyle/checkstyle.xml#L28

There are some file is in CRLF mode due to history problem.

Yeah, history problems are really awful things...

[codecov-commenter]

Codecov Report

❗ No coverage uploaded for pull request base (master@6c779a9). Click here to learn what that means.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##             master    #7232   +/-   ##
=========================================
  Coverage          ?   59.05%           
  Complexity        ?      529           
=========================================
  Files             ?     1076           
  Lines             ?    43418           
  Branches          ?     6339           
=========================================
  Hits              ?    25639           
  Misses            ?    14933           
  Partials          ?     2846           

Impacted Files	Coverage Δ	Complexity Δ
...che/dubbo/common/compiler/support/JdkCompiler.java	63.55% <100.00%> (ø)	0.00 <0.00> (?)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6c779a9...07230f5. Read the comment docs.

[AlbumenJ]

LGTM.