On 2013-11-07 11:22, Erik Joelsson wrote:
Here is the next part of fixing things for the JCE team. With this
patch, the build will always build both the limited and unlimited set
of policy jars, just like the old build did. In an OPENJDK build, the
configure option --enable-unlimited-crypto will still be used to
choose which one gets installed in jdk/lib and the resulting images.
Webrev: http://cr.openjdk.java.net/~erikj/8027963/webrev.01/
Bug: https://bugs.openjdk.java.net/browse/JDK-8027963
It took me some amount of reading and re-reading to get a grip on this.
Not only your changes but the original code as well. Some comments:
I think the code for the US_export_policy could be clearer in that we
actually have only one file, the unlimited policy, and that we just make
an exact copy of this for the limited directory. For instance, I got
confused by the fact that we did not check the UNLIMITED_CRYPTO
variable. It might be more clear if we remove the limited version out of
the equation, and then as a separate step makes a copy. Or maybe not.
And maybe such a change should not be a part of this fix.
However, there is a bug. For openjdk, the rule "
$(US_EXPORT_POLICY_JAR_DST): $(US_EXPORT_POLICY_JAR_UNSIGNED)" will not
work since US_EXPORT_POLICY_JAR_UNSIGNED is not defined anymore. And the
new equivalent, US_EXPORT_POLICY_JAR_UNLIMITED_UNSIGNED is only defined
inside an infeq.
The new log message " $(ECHO) Copying $(patsubst $(OUTPUT_ROOT)/%,%,$@)"
will be output on the default log level, but the previous log message
"$(ECHO) $(LOG_INFO) Copying $(@F)" was only on the info level. I think
this is more appropriate for information about individual files.
The changes in spec.gmk. in and SignJars.gmk look good, although the
README.txt file is only available if closed sources are present. I
suggest the whole SignJars file to be put in a "if not openjdk" block.
Long term, the file should probably move to closed sources.
For US_export as well as local policy: what is the reason of copying the
policy files to a temp directory before jar:ing them? Can't you just
point to the directory where the policy files reside? Aha, I see now a
comment: " TODO fix so that SetupArchive does not write files into SRCS
then we don't need this extra copying". Is that really still the case?
It sounds unlikely to me that SetupArchive modifies the source path.
For local_policy, when building unlimited crypto, the
default_US_export.policy seems to be included as well. On the other
hand, that seems to have been the case before your change as well.
However, this seems suspicious and I suspect that this is a bug.
For local policy, if building openjdk only and have BUILD_CRYPTO=no,
then install-file is going to fail, probably with a strange error since
the dependency variables are not defined. I know this is not a supported
combination but maybe we should fail gracefully.
/Magnus
