> On May 20, 2015, at 9:21 PM, Valerie Peng <[email protected]> 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