RFR: JDK-8027963: Create unlimited policy jars. (original) (raw)

Bradford Wetmore bradford.wetmore at oracle.com
Tue Dec 3 02:52:05 UTC 2013

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 USexportpolicyjar.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 ($(UNLIMITEDCRYPTO), 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) (LOGINFO)Copying(LOGINFO) Copying (LOGINFO)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 EXTRAMANIFESTATTR. 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 USexportpolicy 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 UNLIMITEDCRYPTO 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 " $(USEXPORTPOLICYJARDST): $(USEXPORTPOLICYJARUNSIGNED)" will not work since USEXPORTPOLICYJARUNSIGNED is not defined anymore. And the new equivalent, USEXPORTPOLICYJARUNLIMITEDUNSIGNED is only defined inside an infeq. Yes, I added the same conditional on UNLIMITEDCRYPTO as for localpolicy. Again, this isn't really necessary since they are the same file. The new log message " (ECHO)Copying(ECHO) Copying (ECHO)Copying(patsubst $(OUTPUTROOT)/%,%,$@)" will be output on the default log level, but the previous log message "$(ECHO) (LOGINFO)Copying(LOGINFO) Copying (LOGINFO)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 LOGINFO 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 USexport 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 localpolicy, when building unlimited crypto, the defaultUSexport.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 BUILDCRYPTO=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

More information about the build-dev mailing list