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: > > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8228434/02/webrev/ > > > > > > 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 TEST.ROOT): 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). 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. Could I perhaps convince you to accept this version? http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8228434/02/webrev/ Thanks, Severin
