This version looks good to me.
/Erik
On 2018-01-25 13:33, Magnus Ihse Bursie wrote:
On 2018-01-18 19:37, Erik Joelsson wrote:
Hello Magnus,
Nice to see this finally happening!
Thanks!
In building.md, when getting autoconf for Cygwin, I believe you also
need the autoconf wrapper scripts package.
Hm, well, no, it's the other way around. "autoconf" is the name of the
cygwin "autoconf wrapper scripts" package. It depends on "m4",
"autoconf2.69" (or whatever), etc. I have verified that just
installing "autoconf" will be enough, and will pull in all needed
dependencies.
Perhaps it's also worth mentioning that you can download the autoconf
src and build/install from there?
Well, I'm already giving a link to
http://www.gnu.org/software/autoconf in the build readme. I'm not sure
if it's helpful for anyone to download the autoconf source by
themselves. Most users will have it available prebuilt. And if not,
you'll also most likely need to pull down and build m4, and that's
trickier. The reason why we used to download autoconf and build it
from source was to make sure that we got the exactly right version,
but that only made sense when we checked in the
generated-configure.sh. (And this worked fine for us, since we had m4...)
In jib-profiles.js, you need to use build_platform instead of
target_platform when creating the module name for the dependency.
Good point. Fixed.
I would also like if you added the dependency on the other main
profiles as well. At least linux-x86, linux-arm64 and windows-x86 are
still possible to build and should work fine with this. For
solaris-x64 you would need to generate the package for it to work.
Sure.
The TMPDIR logic looks weird. Are you unconditionally setting it to
/cygdrive/t/...? That's a valid dir in our build farm, but not on my
local windows box:
mkdir: cannot create directory ‘/cygdrive/t’: No such file or directory
autom4te: cannot create
/cygdrive/t/workspace/.build/tmpdir/am4t7009.7040: No such file or
directory
at
/cygdrive/c/cygwin64/var/tmp/jib-erik/install/jpg/infra/builddeps/autoconf-windows_x64/2.69+1.0.1/autoconf-windows_x64-2.69+1.0.1.tar.gz/usr/bin/autom4te
line 954.
Error: Failed to generate runnable configure script
Yeah, that was broken by design. :-(
Some offline discussion later, it turned out that the reason for the
trouble was mismatch between cygwin-32 and cygwin-64. I've solved this
by generating two different packages, for cygwin-32 and cygwin-64.
I've also made some other minor fixes to the build script, e.g. using
perl instead of sed due to the braindead sed being available on
solaris. :(
Updated webrev:
http://cr.openjdk.java.net/~ihse/JDK-8195689-remove-generated-configure/webrev.02/
/Magnus
/Erik
On 2018-01-18 05:28, Magnus Ihse Bursie wrote:
Currently, we require all developers who modify the configure script
to run autoconf locally, to update the generated-configure.sh
script, which is then checked in. This is the only instance of
checked in "compiled" code in OpenJDK, and this has brought along
several problems:
* Only a specific version of autoconf, 2.69, can be used, to avoid
large code changes in the generated file. Unfortunately, Ubuntu
ships a version of autoconf that claims to be 2.69 but is actually
heavily patched. This requires all Ubuntu users to compiler their
own autoconf from source.
* The Oracle JDK closed sources has a closed version that needs to
be updated. In practice, this has meant that all non-Oracle
developers, need an Oracle sponsor for patches modifying the
configure script.
* If the configure script is not properly updated, the build will
fail. The same happens on the Oracle side if the closed version is
not in sync with the open version. It is easy to miss re-generating
the script after the last fix of a typo in the comments in an .m4
file...
* Merging between two changes containing configure modifications is
almost impossible. In practice, the entire generated-configure.sh
needs to be thrown away and regenerated.
The entire benefit of having the file in the repo is to save
first-time developers the hassle of installing autoconf. On most
platforms, this is a no-brainer (like "apt install autoconf"), and
the requirement is similar to other open source projects using
autoconf and "./configure". It's just not worth it.
Bug: https://bugs.openjdk.java.net/browse/JDK-8195689
WebRev:
http://cr.openjdk.java.net/~ihse/JDK-8195689-remove-generated-configure/webrev.01
/Magnus
