RFR: JDK-8247589: Implementation of Alpine Linux/x64 Port [v5]
Aleksei Voitylov
avoitylov at openjdk.java.net
Fri Oct 2 15:45:56 UTC 2020
> continuing the review thread from here https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-September/068546.html
>
>> The download side of using JNI in these tests is that it complicates the
>> setup a bit for those that run jtreg directly and/or just build the JDK
>> and not the test libraries. You could reduce this burden a bit by
>> limiting the load library/isMusl check to Linux only, meaning isMusl
>> would not be called on other platforms.
>>
>> The alternative you suggest above might indeed be better. I assume you
>> don't mean splitting the tests but rather just adding a second @test
>> description so that the vm.musl case runs the test with a system
>> property that allows the test know the expected load library path behavior.
>
> I have updated the PR to split the two tests in multiple @test s.
>
>> The updated comment in java_md.c in this looks good. A minor comment on
>> Platform.isBusybox is Files.isSymbolicLink returning true implies that
>> the link exists so no need to check for exists too. Also the
>> if-then-else style for the new class in ProcessBuilder/Basic.java is
>> inconsistent with the rest of the test so it stands out.
>
> Thank you, these changes are done in the updated PR.
>
>> Given the repo transition this weekend then I assume you'll create a PR
>> for the final review at least. Also I see JEP 386 hasn't been targeted
>> yet but I assume Boris, as owner, will propose-to-target and wait for it
>> to be targeted before it is integrated.
>
> Yes. How can this be best accomplished with the new git workflow?
> - we can continue the review process till the end and I will request the integration to happen only after the JEP is
> targeted. I guess this step is now done by typing "slash integrate" in a comment.
> - we can pause the review process now until the JEP is targeted.
>
> In the first case I'm kindly asking the Reviewers who already chimed in on that to re-confirm the review here.
Aleksei Voitylov has refreshed the contents of this pull request, and previous commits have been removed. The
incremental views will show differences compared to the previous content of the PR.
-------------
Changes:
- all: https://git.openjdk.java.net/jdk/pull/49/files
- new: https://git.openjdk.java.net/jdk/pull/49/files/5feda5ff..b7ffed87
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk&pr=49&range=04
- incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=49&range=03-04
Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
Patch: https://git.openjdk.java.net/jdk/pull/49.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/49/head:pull/49
PR: https://git.openjdk.java.net/jdk/pull/49
More information about the core-libs-dev
mailing list