On 2013-06-11 10:59, David Holmes wrote:
On 11/06/2013 5:47 PM, Erik Joelsson wrote:
On 2013-06-11 09:22, David Holmes wrote:
Erik,
On 7/06/2013 7:50 PM, Erik Joelsson wrote:
On 2013-06-07 04:08, David Holmes wrote:
On 4/06/2013 10:38 PM, David Holmes wrote:
On 4/06/2013 9:21 PM, Erik Joelsson wrote:
On 2013-06-04 06:56, David Holmes wrote:
I think we should add a (configure time?) check to watch for the
illegal: BUILD_CRYPTO=no, OPENJDK=true.
Not sure where that would fit. Configure is only setting
BUILD_CRYPTO
false in the closed logic, so this would be to catch someone either
setting it on command line to configure or make, or a licensee
adding
--enable-openjdk-only to configure. To catch both we would need the
check in the open. (And if we start looking at BUILD_CRYPTO in open
configure, we might as well set it to yes there for a default and
skip
ever having it empty.)
Can we do something in the makefile then? Straight after the comment
You never addressed this. Was there some reason not to put a check in
the Makefile?
Right, I started to look at it but gave it some more thought.
BUILD_CRYPTO=no will only be set when building with closed present and
not setting --enable-openjdk-only. The logic in the closed configure for
this variable will be skipped if --enable-openjdk-only is given. So even
if a licensee with a security purged source tree is trying to do this,
the failure will be missing source code when compiling java classes and
the variable BUILD_CRYPTO will still be unset, making a makefile check
pointless.
For the pure open case, why would anyone get the idea to set
BUILD_CRYPTO=no on the command line? I'm sure we have a number of
variables that would screw up the build if set to random values on the
command line and we can't really be expected to check all of them. For
this I would think the comment explaining about the variable in
CreateJars.gmk should be enough to discourage anyone to go about and set
it.
So to sum it up, I couldn't come up with a situation where both of these
variables would be set to conflicting values, that was likely enough to
warrant an explicit check.
Okay. Seemed like a simple sanity check to me without a need for too
much thought :)
You were indeed correct to bring it up and I was close to adding a
check. But when weighing the pros and cons of where to put it, I started
thinking about when it would be triggered, and discovered that I
couldn't really trigger it. At that point it felt silly to implement it.
Thanks for the review!
/Erik
David
/Erik
Otherwise looks okay.
Thanks,
David
475 # The source for the crypto jars is not available for all
licensees.
The BUILD_CRYPTO
476 # variable is set to false if these jars can't be built to skip
that
step of the build.
477 # Note that for OPENJDK, the build will fail if
BUILD_CRYPTO=false
since then there is no
478 # other way to get the jars then to build them.
BTW in the above comment need to change "false" to "no".
Gahh, again, fixed in hopefully the last webrev on this issue. Think
this will be a new record for me.
http://cr.openjdk.java.net/~erikj/8010785/webrev.jdk.05/
Thanks for your patience.
/Erik
David
ifeq ($(BUILD_CRYPTO), no)
ifdef OPENJDK
$(error Crypto libraries must be built in an OpenJDK build)
endif
endif
David
/Erik
David
-----
On 2013-06-03 06:22, David Holmes wrote:
Hi Erik,
In CreateJars.gmk I don't understand why you move the update to
the
JARS variable inside the BUILD_CRYPTO conditional when the jar
file is
a pre-req for a target defined outside of that conditional.
What are
the allowed combinations:
BUILD_CRYPTO=yes, OPENJDK=true == OK (normal OpenJDK build)
BUILD_CRYPTO=yes, OPENJDK=false == OK? (builds but doesn't use
it?)
BUILD_CRYPTO=no, OPENJDK=true == ILLEGAL? (missing re-req in
rule?)
BUILD_CRYPTO=no, OPENJDK=false == OK (normal Oracle JDK build)
This also seems to indicate that the earlier comment block:
469
##########################################################################################
470 # For all security jars, always build the jar, but for
closed,
install the prebuilt signed
471 # version instead of the newly built jar. Unsigned jars are
treated as intermediate targets
472 # and explicitly added to the JARS list. For open, signing
is not
needed. See SignJars.gmk
473 # for more information.
needs updating to account for this new condition. ("security"
covers
these crypto jars).
This is true and I've updated the comment to point it out.
BUILD_CRYPTO=false and OPENJDK=true is not a legal combination.
---
In Setup.gmk, wouldn't this:
38 ifndef OPENJDK
39 # Some licensees do not get the Security Source
bundles. We
will
40 # fall back on the prebuilt jce.jar so that we can do a
best
41 # attempt at building.
42 ifeq ($(wildcard
$(JDK_TOPDIR)/src/share/classes/javax/crypto/Cipher.java),)
43 JCE_PATH :=
$(PATH_SEP)$(JDK_TOPDIR)/make/closed/tools/crypto/jce/jce.jar
44 endif
45 endif
be better handled by a configure check that the sources exist -
as is
done for other potentially not-present components? Further I
think
this kind of check belongs in a closed build file as it doesn't
relate
to building the openjdk sources.
Also true. The condition is actually similar enough to that of
BUILD_CRYPTO to be treated as the same. I moved this to a closed
file.
/Erik
Thanks,
David
On 31/05/2013 8:14 PM, Erik Joelsson wrote:
Finally getting back to this. Updated webrevs:
http://cr.openjdk.java.net/~erikj/8010785/webrev.jdk.02/
http://cr.openjdk.java.net/~erikj/8010785/webrev.root.02/
The javascript part is no longer needed since it has been
removed.
/Erik
On 2013-04-11 12:53, Erik Joelsson wrote:
Open part of this review.
The licensee bundles aren't buildable with the new build for
several
reasons. I've tried to fix all the issues that I've found and
have
now
successfully built them on linux, windows and solaris. Here
is a
list
of the changes that I had to do to OpenJDK:
* Filter out javascript src when the rhino source isn't
available.
Also do not copy rhino resource files when not available.
This is
controlled by a new variable, INCLUDE_JAVASCRIPT, which we
control
from closed configure and shouldn't affect the OpenJDK
build. I
also
moved the copying of the resources to the correct makefile,
CopyIntoClasses.gmk.
* If javax/crypto isn't available, jce.jar needs to be
added to
the
bootclasspath of the main java compilation. Also, a number of
security
jar files shouldn't be built at all. (Normally these are built
just to
exercise the logic, but not used.) The kerberos library is
also
excluded by this. Introduced the variable BUILD_CRYPTO, also
set by
closed configure to control this. I used the logic ifneq
($(BUILD_CRYPTO),no) to not change the behavior if the
variable
isn't
set, which it won't be in the open case.
* I removed the logic for setting the closed cacerts file
in the
open
configure script.
* Also fixing JDK-8005655 by adding logic for unzipping
sec-bin
(and
friends) if available.
http://cr.openjdk.java.net/~erikj/8010785/webrev.jdk.01/
http://cr.openjdk.java.net/~erikj/8010785/webrev.root.01/
/Erik
