Hello Max,
I couldn't find the link to the review but I'm guessing this is the one:
http://cr.openjdk.java.net/~weijun/8011402/webrev.00/
On 2013-09-06 15:30, Weijun Wang wrote:
Hi Sean
Please review the code changes at
8011402: Move blacklisting certificate logic from hard code to data
Hard coded blacklisted certificates are moved out of the class file
and now inside a data file. Furthermore, only their fingerprints are
released in the JRE. The makefile covers blacklist files in both open
and closed repo.
No regression test, cleanup.
*build-dev*, I am not an export of Makefile, and I have some questions:
1. I create a new macro (or function?) called cat-files. Its only
difference from install-file is that it needs to deal with two inputs.
Do we already have a similar macro somewhere?
I do not think so.
2. cat-files is defined inside CopyFiles.gmk right beside its usage.
Do you think it's better to define it in a common file?
We could put this in a common file, in that case it would be
common/makefiles/MakeBase.gmk. However, this macro does more than just
cat files. It sorts and removes duplicates too. In this case, since it's
used only once, I would skip the macro def and just inline the recipe.
Also note that prep-target doesn't exist anymore. What you want is probably:
$(MKDIR) -p $(@D)
3. Most important: it only works if both $(BLACKLISTED_CERTS_SRC_OPEN)
and $(BLACKLISTED_CERTS_SRC_CLOSED) already exists. Currently there is
no closed blacklist, but I still have to create an empty file there.
Otherwise, there will be
make[2]: *** No rule to make target
`/space/repos/jdk8/tl/jdk/src/closed/share/lib/security/blacklisted.certs',
needed by
`/space/repos/jdk8/tl/build/macosx-x86_64-normal-server-release/jdk/lib/security/blacklisted.certs'.
Stop.
Is there a way to make it work without adding that empty file?
Yes:
BLACKLISTED_CERTS_SRCS := /path/to/open/certs
ifndef OPENJDK
BLACKLISTED_CERTS_SRCS += /path/to/closed/certs
endif
$(BLACKLISTED_CERTS_DEST): $(BLACKLISTED_CERTS_SRCS)
<do stuff>
/Erik
Thanks
Max
