RFR: 8228434: jdk/net/Sockets/Test.java fails after JDK-8227642
mandy.chung at oracle.com
Fri Jul 19 17:32:06 UTC 2019
On 7/19/19 10:13 AM, Severin Gehwolf wrote:
> Hi Mandy,
> On Fri, 2019-07-19 at 09:58 -0700, Mandy Chung wrote:
>> Hi Severin,
>> On 7/19/19 9:55 AM, Severin Gehwolf wrote:
>>>> There might be other tests with policy files where this is not the case.
>>> My issue is with finding those tests :-/ If we know the set of *all*
>>> tests affected by the breakage we could do approach 2. Approach 1 (or
>>> 3) seems safer.
>>>> Severin - how about a combination of the two approaches, meaning add
>>>> Docker.DOCKER_COMMAND as per the first version but use
>>>> privilegedGetProperty to read the value. That way only container tests
>>>> using a SM and their own policy files will need to grant the permission
>>>> to read this property.
>>> Sure, fine with me. Here you go:
>> It seems better to define Docker as a new top-level class as Platform
>> doesn't seem the right place to define DOCKER_COMMAND.
> Note that Platform.java is already on the boot library path for jtreg
> and moving it to a separate class would need another entry here (in
> requires.extraPropDefns.bootlibs = ../lib/sun ../lib/jdk/test/lib/Platform.java
> The reason I've moved it to Platform for JDK-8227642 was that
> VMProps.java previously hard-coded the docker command (it needs to use
> the property value instead).
It accepts multiple files, doesn't it?
> This seems work for little gain. Note that the intention is to change
> "Docker" and DOCKER_COMMAND to something more neutral, like Container
> or ContainerEngine. In light of that, Platform.ContainerEngine.COMMAND
> makes sense to me.
This is a test library specific to containers. "Container" sounds fine.
Why not doing it now?
> Could I perhaps convince you to accept this version?
I still think separating container-specific APIs in its own class will
prevent running similar kind of issue in the future.
More information about the core-libs-dev