On 15/03/2013 12:55 PM, Andrew Hughes wrote:
----- Original Message -----
On 15/03/2013 5:37 AM, Omair Majid wrote:
Hi,
Updated webrev at:
http://cr.openjdk.java.net/~omajid/webrevs/intree-ec/01/
I switched from DISABLE_INTREE_EC to ENABLE_INTREE_EC to avoid the
confusion with double negatives.
Looking just at the mechanics of this it looks fine to me.
I concur.
This needs to
be coordinated with someone from the build team (which isn't me) so
that we can keep the closed generated-configure.sh in sync.
I don't think this is a reasonable requirement generally, though it
may be fine in this case. We should now be getting to the point
where an OpenJDK committer can post a patch, an OpenJDK reviewer
can review it and the committer can push it to an appropriate
repository without either of these people being Oracle employees.
Certainly, the bylaws allow this.
What you're asking here seems no different than me asking for
someone to co-ordinate with the IcedTea team to update their
build machinery. I wouldn't expect this to happen and I don't think
Oracle should either.
We know this is a problem. Hopefully we will have a solution soon that
will alleviate the need for this. In the meantime I would request your
forbearance. I should have said "Can you please ..." rather than "This
needs ...".
Personally I wonder whether the existence check should be in the
Makefile rather than done at configure time? I worry that we end up
with too many make variables becoming configure variables as well.
One of the main functions of a configure script is to make these
variables more easily accessible to the point that you should
only need to set make variables when you need to do a one-time override
of something set by configure. We should no longer be passing a huge
list of make variables.
Totally agreed with the last point.
But to me the division of labor between configure and make is not always
clear.
David
-----
I'd argue that Omair's work could be extended, in a further patch, so
that the option was set by --disable-intree-ec and the behaviour
seen here was just the default if the option wasn't specified.
Having configure options also acts as a means of documenting the
various build options (see configure --help), something which
doesn't happen when they are imposed via make.
David
-----
Note that because of the ifeq comparison, if you use the new build
system and just update the jdk tree, the ifeq ($ENABLE_INTREE_EC),
yes)
comparison will fail (since ENABLE_INTREE_EC was not previously
defined)
and EC will not be part of the build.
This problem wont happen if you update the root repo and re-run
configure.
On 03/14/2013 06:03 AM, David Holmes wrote:
On 14/03/2013 3:12 PM, David Holmes wrote:
Note that this isn't changing any functionality simply exposing
an
existing make variable at configure time.
Correction. I misunderstood what was being done here. This
forcibly
set/clears the make variable based solely on the existence of a
directory:
test -d "${SRC_ROOT}/jdk/src/share/native/sun/security/ec/impl"
Yes, it is forcibly set. But since this directory always exists in
OpenJDK 8, this should never evaluate to false.
This change is very useful for distributions, though, since they
can
delete the directory when creating a source tarball for OpenJDK8,
and
the ECC implementation will be automagically disabled from the
build.
It doesn't expose a configure option for this. This may be
perfectly
fine but the person who wrote the original TODO comment needs to
verify
that.
Andrew Hughes wrote the original changeset in the old build system
[1]
and I believe this is exactly what he wanted out of the changeset.
I am
CC'ing Erik, who wrote the TODO [2] so he can clarify what he
meant.
Thanks,
Omair
[1] http://hg.openjdk.java.net/jdk7/tl/jdk/rev/39c15c0a71f7
[2]
http://hg.openjdk.java.net/build-infra/jdk8/jdk/diff/1953cf522107/makefiles/CompileNativeLibraries.gmk
