add support for alibaba dragonwell #444
Conversation
|
hi @Accelerator1996, thank you for the PR, we will take a look at it |
Hello, @panticmilos ! Could you please help me to review the PR? I tested it locally and it was fine. Thank you very much! |
|
Hi, @panticmilos ! There has been no reply for a long time. I want to know your opinions about this PR. If there is something inappropriate in my PR, I will revise it in time according to your opinions. Thank you so much! |
|
Hi, @Accelerator1996 👋 Currently, we are investigating licenses of the Alibaba Dragonwell JDK. As soon as we finish we will get back to your PR. |
Thank you very much! |
|
I resolve the merge conflict and the local workflow works fine |
|
Hi, @IvanZosimov! How is the progress of the investigation? Some of our customers hope to simplify the process of deploying JDK through setup-java as soon as possible, and we are very eager to join your project. Thanks! |
|
Hi, @Accelerator1996, unfortunately, we are still in the process. Don't worry, as soon as it's done, we will let you know 😉 |
Accelerator1996
force-pushed
the
main
branch
8 times, most recently
from
e87f68d
to
cd9a559
Compare
|
I solved the merge conflict and action workflow passed. |
|
Hi, @IvanZosimov . How is the progress of the investigation license? if possible, we are willing to provide relevant information to reduce your investigation work. |
Accelerator1996
force-pushed
the
main
branch
3 times, most recently
from
383c264
to
40c66d5
Compare
|
@IvanZosimov can you elaborate on what the licensing concerns are here? Dragonwell is shipped under the same GPL V2 (+ classpath exception) that the other OpenJDK distributions are. |
|
Hi, @Accelerator1996 and @gdams. The question of licenses is solved, and we are starting to review and test the PR. |
| majorVersion = splits[0]; | ||
| version = splits.length >= 3 ? splits.slice(0, 3).join('.') : version; | ||
| } | ||
| const edition = majorVersion == '17' ? 'Standard' : 'Extended'; |
There was a problem hiding this comment.
Could you please describe what is the difference between Standard and Extended version of the JDK. Also, I see that all versions have Standard and Extended editions but you have limited 17 to Standard only.
There was a problem hiding this comment.
Could you please describe what is the difference between Standard and Extended version of the JDK. Also, I see that all versions have Standard and Extended editions but you have limited 17 to Standard only.
- Alibaba Dragonwell Standard Edition: OpenJDK upstream based and have more enhancements, including bug-fixing, security patches, tooling support, etc.
- Alibaba Dragonwell Extended Edition: everything in the standard edition, plus: extra customized/significant features optimized for the cloud and widely used in Alibaba's production environment.
And dragonwell17 only has standard edition.
| const resolvedVersion = matchVersions.length > 0 ? matchVersions[0] : null; | ||
| if (!resolvedVersion) { | ||
| const versionsMsg = core.isDebug() | ||
| ? ' Available versions: ${availableVersions}' | ||
| : ''; | ||
| throw new Error( | ||
| `Cannot find satisfied version for ${version}.${versionsMsg}` | ||
| ); | ||
| } |
There was a problem hiding this comment.
I'd suggest to compile all these lines into one piece of code with the logic such as: If length of mathedVersions array is 0 -> throw; If not -> take the first satisfied version.
There was a problem hiding this comment.
I'd suggest to compile all these lines into one piece of code with the logic such as: If length of mathedVersions array is 0 -> throw; If not -> take the first satisfied version.
But I should return the value of resolvedVersion, so I need to define it as above.
| if (['8', '11', '17'].includes(majorVersion) != true) { | ||
| throw new Error('Support dragonwell versions: 8, 11, 17'); | ||
| } |
There was a problem hiding this comment.
Are we going to change these lines of code when a new version of the Dragonwell revealed? I believe throwing the error after the version matching operation is enough.
There was a problem hiding this comment.
I think this is the pre-operation of the full check. If there is an invalid majorVersion, it can reduce the frequency of request to the website.
| arch = arch.includes('x86') ? 'x64' : arch; | ||
|
|
||
| const availableVersionsUrl = | ||
| 'https://dragonwell-jdk.io/map_with_checksum.json'; |
There was a problem hiding this comment.
Currently this link doesn't work and it didn't yesterday. The whole https://dragonwell-jdk.io is down. I have concerns about reliability of this API. What do you think?
There was a problem hiding this comment.
Currently this link doesn't work and it didn't yesterday. The whole https://dragonwell-jdk.io is down. I have concerns about reliability of this API. What do you think?
Sorry, our website is closed for a few days for some reason. I redeployed this morning.
Accelerator1996
force-pushed
the
main
branch
2 times, most recently
from
318a89e
to
631bb99
Compare
|
|
|
@IvanZosimov Sorry, I've update code to fix the bug about 'arch' is never reassigned. |
Description:
Add support for Alibaba Dragonwell JDK.
Related issue:
Issue #443
Check list: