> On May 20, 2015, at 9:21 PM, Valerie Peng <valerie.p...@oracle.com> wrote: > > Sean, > > Could you please review this change? The changes are mostly the same as the > prototype in Jake, but I have to make some modification due to the difference > in ServiceLoader lookup in OpenJDK (corresponding > META-INF/services/java.security.Provider files in each module) and the > related makefile change (merge their content into one for the final image > build). Then, I adjusted the Provider.configure() method to take a single > String argument to be consistent with the "providerarg" option that keytool > defined. > > In addition, I also made some misc changes, such as configuring the providers > inside ProviderConfig instead of ProviderLoader, add back the doPrivileged > block to all the provider constructors. I also have second thought on making > the switch to privider name (instead of provider class name) in java.security > file, so I reverted the changes on that - that SunPKCS11 provider has its > name specified in its configuration file, so when ServiceLoader loads the > PKCS11 provider, the configuration file has not been passed to it, so the > name is not known at that time. Thus, using the class name for the provider > list entry seems to fit the flow better. I also updated the default policy > for SunPKCS11 provider given its recent change of using sun.misc. > > Webrev: http://cr.openjdk.java.net/~valeriep/7191662/webrev.00/ > CCC: http://ccc.us.oracle.com/7191662 <http://ccc.us.oracle.com/7191662>
A quick comment on the META-INF/services config files and the makefile. Merging the service config files is temporary until the module system is moving further along. src/jdk.crypto.ucrypto/solaris/classes/META-INF/services/java.security.Provider.html and other os-specific service configuration: 1 #[solaris]com.oracle.security.ucrypto.UcryptoProvider - why is this commented out? Does the makefile uncomment it? It should be simple concatenation with The makefile doesn’t seem right though. make/gensrc/Gensrc-java.naming.gmk.html 96 jdk.crypto.ec: $(GENSRC_PROVIDER) 98 all: jdk.crypto.ec java.naming doesn’t seem an appropriate module to be the main module for containing all service provider config files. I initially propose to use jdk.crypto.ec as the gensrc module as indicated in line 96,98. You can rename the file to Gensrc-jdk.crypto.ec and update the content.. GENSRC_PROVIDER := $(SUPPORT_OUTPUTDIR)/gensrc/java.naming/META-INF/services/java.security.Provider GENSRC_PROVIDER is the output file. line 79-89 is building the target list. I think you need another variable to build up the target list but not GENSRC_PROVIDER. You can reference how Gensrc-jdk.jdi.gmk concatenates the service config for jdk.jdi and dk.hotspot.agent module. # Filter com.sun.jdi.connect.Connector $(SUPPORT_OUTPUTDIR)/gensrc/jdk.jdi/META-INF/services/com.sun.jdi.connect.Connector: \ $(JDK_TOPDIR)/src/jdk.jdi/share/classes/META-INF/services/com.sun.jdi.connect.Connector \ $(SUPPORT_OUTPUTDIR)/gensrc/jdk.hotspot.agent/_the.sa.services $(process-provider) # Copy the same service file into jdk.hotspot.agent so that they are kept the same. $(JDK_OUTPUTDIR)/modules/jdk.hotspot.agent/META-INF/services/com.sun.jdi.connect.Connector: \ $(SUPPORT_OUTPUTDIR)/gensrc/jdk.jdi/META-INF/services/com.sun.jdi.connect.Connector $(install-file) Mandy