RFR: JDK-8027963: Create unlimited policy jars.

[Bradford Wetmore]

Erik,

Given the name of your workspace /localhome/hg/jdk8-tl, I'm assuming 
this will be going through the TL gate instead of build?  Please do use 
tl, as there will be some parallel changes to the RE scripts that I need 
to make, and we won't be surprised when the changes hit the gate weeks 
later.

When do you think this will go in?

One minor thing which is probably not a problem, there's a new blank 
line at the end of the new MANIFEST files probably created by SetupArchive.

>> CreateSecurityJars.gmk
>> ======================


>> 197/254/257/281:  This webrev was probably generated before the
>> makefile reorg was done.  jdk/make/javax/crypto ->
>> jdk/make/data/cryptopolicy/*
>>
>> As a result, I didn't apply the bits and run a test build.  If you
>> want to supply me either a built WS or an updated webrev, I can check
>> things out.
>>
> Yes it was, the new one is generated from jdk8/tl which has the reorg.

Ok.

>> 198/251: Since US_export_policy_jar.tmp is a tmp dir for policy,
>> shouldn't this be in the jce/policy directory instead of the top level
>> jce where the jars end up?
>>
> Yes, that will look tidier in the output dir. Moved it.

Great.

>> 221/289: Can you put in a error here for the case if an unlimited
>> signed version is requested?  There is no prebuilt unlimited version,
>> and people may not realize it.
>>
>>     ifndef OPENJDK
>>       ifeq ($(UNLIMITED_CRYPTO), true)
>>         $(warning/error) "no prebuilt available"
>>       endif
>>
> Added it on the first occurrence. Ideally configure should complain even
> earlier though.

Yes, it could.

>> 221/290: Can you update these two copies of the policy file to say
>> that you're copying the prebuilt policy files?
>>
>>         $(ECHO) $(LOG_INFO) Copying $(@F)
> Done. Also changed all the other similar messages to the same format.
> The old copy messages have been annoying me for a while for not matching
> the style of other messages.

Ok.  I personally liked the old style, but I'm ok with your change.

>> 284:  JARS?  Do you want to call it FILES now?  ;)
> I agree the name is not right. Renamed it TARGETS.

Ok.

>> jdk/make/data/cryptopolicy/limited
>> ==================================
>> LIMITED file can be removed since you are now adding this extra
>> attribute via EXTRA_MANIFEST_ATTR.  Also, since you're explicitly
>> listing the files to include, do you need the "SUFFIXES =" line?
>>
>> jdk/make/data/cryptopolicy/unlimited
>> ====================================
>> UNLIMITED can be removed, for same reason.
>>
> After the removal of the old build, these can now be removed yes, done.

Looks good.  Thanks!

Brad


> /Erik
>
>>
>> I'll update 8027266 with what remains to be done.
>>
>> Brad
>>
>>
>>
>>
>> On 11/12/2013 2:24 AM, Erik Joelsson wrote:
>>> 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
>>>
>