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
