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 microsoft distribution of the JDK. #252
Changes from 7 commits
1bc2528
9c1600a
2828873
f073089
434a5bf
3d55049
c3cb155
41cb4d7
50297fe
bf48000
ef8955b
d2ec614
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,8 +20,11 @@ jobs: | |
fail-fast: false | ||
matrix: | ||
os: [macos-latest, windows-latest, ubuntu-latest] | ||
distribution: ['temurin', 'adopt', 'adopt-openj9', 'zulu', 'liberica'] # internally 'adopt-hotspot' is the same as 'adopt' | ||
distribution: ['temurin', 'adopt', 'adopt-openj9', 'zulu', 'liberica', 'microsoft' ] # internally 'adopt-hotspot' is the same as 'adopt' | ||
version: ['8', '11', '16'] | ||
exclude: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should also exclude Microsoft's version 16, as it is not an LTS release and we will no longer provide updates. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think for now, since Microsoft hosts it, we should keep it in the spirit of meeting users where they are. Many users haven't updated their CI/CD for JDK 17. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, that makes sense. We are not exposing support timelines for any of the distros through the GH Action anyways, so, it is up to the end-user to check the documentation of each distro. |
||
- distribution: microsoft | ||
version: 8 | ||
steps: | ||
- name: Checkout | ||
uses: actions/checkout@v2 | ||
|
@@ -175,16 +178,25 @@ jobs: | |
shell: bash | ||
|
||
setup-java-custom-architecture: | ||
name: ${{ matrix.distribution }} ${{ matrix.version }} (jdk-x86) - ${{ matrix.os }} | ||
# Only Zulu provides x86 arch for now and only for windows / ubuntu | ||
# Only Microsoft provides arm64 for windows / ubuntu / os x | ||
if: ${{ (matrix.distribution == 'zulu' && matrix.architecture == 'x86' && matrix.os != 'macos-latest' ) || (matrix.distribution == 'microsoft' && matrix.architecture == 'arm64') }} | ||
brcrista marked this conversation as resolved.
Show resolved
Hide resolved
|
||
name: ${{ matrix.distribution }} ${{ matrix.version }} (jdk-${{ matrix.architecture }}) - ${{ matrix.os }} | ||
needs: setup-java-major-minor-versions | ||
runs-on: ${{ matrix.os }} | ||
strategy: | ||
fail-fast: false | ||
matrix: | ||
<<<<<<< HEAD | ||
# Only Zulu and Liberica provides x86 arch for now and only for windows / ubuntu | ||
os: [windows-latest, ubuntu-latest] | ||
distribution: ['zulu', 'liberica'] | ||
======= | ||
os: [macos-latest, windows-latest, ubuntu-latest] | ||
distribution: ['zulu', 'microsoft'] | ||
>>>>>>> Add microsoft distribution of the JDK. | ||
brcrista marked this conversation as resolved.
Show resolved
Hide resolved
|
||
version: ['11'] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we move up to 17? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As above. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Noted. |
||
architecture: [ 'x86', 'arm64' ] | ||
steps: | ||
- name: Checkout | ||
uses: actions/checkout@v2 | ||
|
@@ -194,7 +206,7 @@ jobs: | |
with: | ||
distribution: ${{ matrix.distribution }} | ||
java-version: ${{ matrix.version }} | ||
architecture: x86 | ||
architecture: ${{ matrix.architecture }} | ||
- name: Verify Java | ||
run: bash __tests__/verify-java.sh "${{ matrix.version }}" "${{ steps.setup-java.outputs.path }}" | ||
shell: bash |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,88 @@ | ||
import { MicrosoftDistributions } from '../../src/distributions/microsoft/installer'; | ||
|
||
describe('findPackageForDownload', () => { | ||
let distribution: MicrosoftDistributions; | ||
|
||
beforeEach(() => { | ||
distribution = new MicrosoftDistributions({ | ||
version: '', | ||
architecture: 'x64', | ||
packageType: 'jdk', | ||
checkLatest: false | ||
}); | ||
}); | ||
|
||
it.each([ | ||
[ | ||
'17.x', | ||
'17.0.1', | ||
'https://aka.ms/download-jdk/microsoft-jdk-17.0.1.12.1-{{OS_TYPE}}-x64.{{ARCHIVE_TYPE}}' | ||
], | ||
[ | ||
'16.0.x', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given 16 is not an LTS release of MS Build of OpenJDK, I think we should not list it here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. as above, but if you feel strongly I will remove it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, it's fine. See previous comment. |
||
'16.0.2', | ||
'https://aka.ms/download-jdk/microsoft-jdk-16.0.2.7.1-{{OS_TYPE}}-x64.{{ARCHIVE_TYPE}}' | ||
], | ||
[ | ||
'11.0.13', | ||
'11.0.13', | ||
'https://aka.ms/download-jdk/microsoft-jdk-11.0.13.8.1-{{OS_TYPE}}-x64.{{ARCHIVE_TYPE}}' | ||
] | ||
])('version is %s -> %s', async (input, expectedVersion, expectedUrl) => { | ||
const result = await distribution['findPackageForDownload'](input); | ||
expect(result.version).toBe(expectedVersion); | ||
let os: string; | ||
let archive: string; | ||
switch (process.platform) { | ||
case 'darwin': | ||
os = 'macos'; | ||
archive = 'tar.gz'; | ||
break; | ||
case 'win32': | ||
os = 'windows'; | ||
archive = 'zip'; | ||
break; | ||
default: | ||
os = process.platform.toString(); | ||
archive = 'tar.gz'; | ||
break; | ||
} | ||
const url = expectedUrl.replace('{{OS_TYPE}}', os).replace('{{ARCHIVE_TYPE}}', archive); | ||
expect(result.url).toBe(url); | ||
}); | ||
|
||
it('should throw an error', async () => { | ||
await expect(distribution['findPackageForDownload']('8')).rejects.toThrow( | ||
/Could not find satisfied version for SemVer */ | ||
); | ||
}); | ||
}); | ||
|
||
describe('getPlatformOption', () => { | ||
const distributions = new MicrosoftDistributions({ | ||
architecture: 'x64', | ||
version: '11', | ||
packageType: 'jdk', | ||
checkLatest: false | ||
}); | ||
|
||
it.each([ | ||
['linux', 'tar.gz', 'linux'], | ||
['darwin', 'tar.gz', 'macos'], | ||
['win32', 'zip', 'windows'] | ||
])('os version %s -> %s', (input, expectedArchive, expectedOs) => { | ||
const actual = distributions['getPlatformOption'](input as NodeJS.Platform); | ||
|
||
expect(actual.archive).toEqual(expectedArchive); | ||
expect(actual.os).toEqual(expectedOs); | ||
}); | ||
|
||
it.each(['aix', 'android', 'freebsd', 'openbsd', 'netbsd', 'solaris', 'cygwin'])( | ||
'not support os version %s', | ||
input => { | ||
expect(() => distributions['getPlatformOption'](input as NodeJS.Platform)).toThrow( | ||
/Platform '\w+' is not supported\. Supported platforms: .+/ | ||
); | ||
} | ||
); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we replace 16 with 17 here, since 17 is the latest LTS now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably worth doing in a different PR since it is unrelated to adding MSFT.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, yes. That'd be better indeed.