RFR: JDK-8027963: Create unlimited policy jars.

[Erik Joelsson]

New webrev: http://cr.openjdk.java.net/~erikj/8027963/webrev.02/

On 2013-11-08 13:08, Magnus Ihse Bursie wrote:
> 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:
>
It's a bit of a mess yes.
> 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.
>
I tried to mimic the old build here where the limited and unlimited 
version are treated as different files all the way, except at initial 
creation.
> 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.
>
Yes, I added the same conditional on UNLIMITED_CRYPTO as for 
local_policy. Again, this isn't really necessary since they are the same 
file.
> 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.
I added the message to make the output more consistent with "Creating 
...jar", but you are probably right. I added the LOG_INFO macro.
>
> 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.
It actually already is, so this isn't an issue.
>
> 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.
>
This is still the case yes. The SetupArchive macro assumes you are 
working on a classes directory in your build output. It stores 
information about which files are added from that particular source 
root. I took a quick look at fixing it, but decided against doing it in 
this bug.
> 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.
>
It's not. Look at the dependencies declared for the SetupArchive macro 
calls. Those define which files will be in the temp directories.
> 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.
>
This is already failing and while it might look better to fail nicely, 
configure will never put us in this situation.

/Erik
> /Magnus