Re: RFR: JDK-8195689 Remove generated-configure.sh and instead use autoconf

2018-01-18 Thread Magnus Ihse Bursie

On 2018-01-19 08:08, Erik Helin wrote:

On 01/19/2018 07:18 AM, Martin Buchholz wrote:

Differing projects have come to different conclusions about whether to
include a generated configure.

But the standard seems to be to include one. The mantra is: 
"./configure &&

make" without an autoconf step.


And this is still the mantra (except we don't have an executable 
configure file in the repo so you have to run `bash configure && 
make`). The only thing we are discussing is whether the script 
"configure" should depend on the program "autoconf" or not.


If I'm downloading a .tar.gz source code bundle of a project (like the 
ones usually generated via `make dist`), then it seems more common 
that "configure" will not depend on autoconf. However, if I'm cloning 
a project from source, then I'm used to having autoconf being run for 
me (or sometimes having to run it myself).
Good point. If we were to start shipping source code bundles, it would 
certainly make sense to include a generated configure script for it. I'm 
all for it. The problem here is that the current model presupposes a 
working, generated configure script for every separate commit.


I'll admit that I was part of introducing this model some five (or 
more?) years ago. I didn't really like it then either, but at that time 
I perceived it to be a quite massive resistance amongst the JDK 
developers (perhaps mostly inside Oracle) for any kind of change in the 
environment, so adding a new build requirement seemed like a huge war 
that was needed to be fought. (Just removing the old build system was 
enough effort...) Nowadays, we're using jib inside Oracle, so a new 
requirement is virtually undetectable by most developers. And for all 
others, it's just an "sudo apt install autoconf" (or brew, or whatever) 
away, in the unlikely case that you're a developer without autoconf 
installed already.


/Magnus



The number of people building openjdk is
much larger than the number of people patching configure.  So I agree 
with

David that we should stick with the status quo.


I disagree, I don't think depending on autoconf will make it harder to 
build OpenJDK. Remember that the build steps are still:


$ bash configure
$ make

the only difference now is that configure will use autoconf to first 
generate .build/generated-configure.sh. As noted in the documentation, 
installing autoconf is trivial on essentially any system today (the 
configure script will also provide a useful help message for your 
platform if you are missing autoconf).


So for me, this patch gets +1. I'll leave the actual Makefile changes 
and details for Erik J to review though ;)


Thanks,
Erik


On Thu, Jan 18, 2018 at 6:14 PM, David Holmes 
wrote:


On 18/01/2018 11:28 PM, 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.



I'm not convinced just by you saying it is so - sorry. This seems to 
make
an already complex build process even more complex for every single 
person
who wants to build OpenJDK, for the benefit of a handful of people 
who may

want to modify configure options and whom already work closely with the
build team and so there's really little hardship in getting a 
sponsor, or

just someone with access to autoconf.

It introduces a new point of failure in the build for everyone.

Has this been beta-tested with external contributors? I'd be happier
knowing we've put this through its paces with people developing on a 
wide


Re: RFR: JDK-8195689 Remove generated-configure.sh and instead use autoconf

2018-01-18 Thread Magnus Ihse Bursie

On 2018-01-19 07:18, Martin Buchholz wrote:
Differing projects have come to different conclusions about whether to 
include a generated configure.


But the standard seems to be to include one. The mantra is: 
"./configure && make" without an autoconf step. The number of people 
building openjdk is much larger than the number of people patching 
configure.  So I agree with David that we should stick with the status 
quo.


This patch will not introduce a separate autoconf step. Please see the 
updated build readme in my patch. The only difference is an added build 
requirement.


If someone wants to contribute to OpenJDK, installing external 
dependencies for the build can hardly be the biggest hurdle. If we 
*really* thought that was an issue, then we should start bundling much 
more external dependencies. Like freetype, which is a *real* problem for 
all Windows users, for instance.


I don't think that's the right way to go, though. And in general, the 
direction of OpenJDK for the last few years has been in the opposite 
direction: less bundling, more external dependencies. The community has 
requested configure arguments for using the system provided versions of 
libraries that we also bundle. We've stopped linking statically with 
glibc. We've removed bundled source code and introduced a new dependency 
for fontconfig.


/Magnus


On Thu, Jan 18, 2018 at 6:14 PM, David Holmes > wrote:


On 18/01/2018 11:28 PM, 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.


I'm not convinced just by you saying it is so - sorry. This seems
to make an already complex build process even more complex for
every single person who wants to build OpenJDK, for the benefit of
a handful of people who may want to modify configure options and
whom already work closely with the build team and so there's
really little hardship in getting a sponsor, or just someone with
access to autoconf.

It introduces a new point of failure in the build for everyone.

Has this been beta-tested with external contributors? I'd be
happier knowing we've put this through its paces with people
developing on a wide range of platforms, before making it the default.

Have the devkits been updated so I can try this out myself?

Thanks,
David


Bug: https://bugs.openjdk.java.net/browse/JDK-8195689

WebRev:

http://cr.openjdk.java.net/~ihse/JDK-8195689-remove-generated-configure/webrev.01






/Magnus






Re: RFR: JDK-8195689 Remove generated-configure.sh and instead use autoconf

2018-01-18 Thread Erik Helin

On 01/19/2018 07:18 AM, Martin Buchholz wrote:

Differing projects have come to different conclusions about whether to
include a generated configure.

But the standard seems to be to include one. The mantra is: "./configure &&
make" without an autoconf step.


And this is still the mantra (except we don't have an executable 
configure file in the repo so you have to run `bash configure && make`). 
The only thing we are discussing is whether the script "configure" 
should depend on the program "autoconf" or not.


If I'm downloading a .tar.gz source code bundle of a project (like the 
ones usually generated via `make dist`), then it seems more common that 
"configure" will not depend on autoconf. However, if I'm cloning a 
project from source, then I'm used to having autoconf being run for me 
(or sometimes having to run it myself).



The number of people building openjdk is
much larger than the number of people patching configure.  So I agree with
David that we should stick with the status quo.


I disagree, I don't think depending on autoconf will make it harder to 
build OpenJDK. Remember that the build steps are still:


$ bash configure
$ make

the only difference now is that configure will use autoconf to first 
generate .build/generated-configure.sh. As noted in the documentation, 
installing autoconf is trivial on essentially any system today (the 
configure script will also provide a useful help message for your 
platform if you are missing autoconf).


So for me, this patch gets +1. I'll leave the actual Makefile changes 
and details for Erik J to review though ;)


Thanks,
Erik


On Thu, Jan 18, 2018 at 6:14 PM, David Holmes 
wrote:


On 18/01/2018 11:28 PM, 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.



I'm not convinced just by you saying it is so - sorry. This seems to make
an already complex build process even more complex for every single person
who wants to build OpenJDK, for the benefit of a handful of people who may
want to modify configure options and whom already work closely with the
build team and so there's really little hardship in getting a sponsor, or
just someone with access to autoconf.

It introduces a new point of failure in the build for everyone.

Has this been beta-tested with external contributors? I'd be happier
knowing we've put this through its paces with people developing on a wide
range of platforms, before making it the default.

Have the devkits been updated so I can try this out myself?

Thanks,
David


Bug: https://bugs.openjdk.java.net/browse/JDK-8195689

WebRev: http://cr.openjdk.java.net/~ihse/JDK-8195689-remove-generate
d-configure/webrev.01





/Magnus





Re: RFR: JDK-8195689 Remove generated-configure.sh and instead use autoconf

2018-01-18 Thread Martin Buchholz
Another possibility is implementing the invariant that configure is
generated via autoconf 2.69 by a mercurial commit hook.

On Thu, Jan 18, 2018 at 10:18 PM, Martin Buchholz 
wrote:

> Differing projects have come to different conclusions about whether to
> include a generated configure.
>
> But the standard seems to be to include one. The mantra is: "./configure
> && make" without an autoconf step.  The number of people building openjdk
> is much larger than the number of people patching configure.  So I agree
> with David that we should stick with the status quo.
>
> On Thu, Jan 18, 2018 at 6:14 PM, David Holmes 
> wrote:
>
>> On 18/01/2018 11:28 PM, 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.
>>>
>>
>> I'm not convinced just by you saying it is so - sorry. This seems to make
>> an already complex build process even more complex for every single person
>> who wants to build OpenJDK, for the benefit of a handful of people who may
>> want to modify configure options and whom already work closely with the
>> build team and so there's really little hardship in getting a sponsor, or
>> just someone with access to autoconf.
>>
>> It introduces a new point of failure in the build for everyone.
>>
>> Has this been beta-tested with external contributors? I'd be happier
>> knowing we've put this through its paces with people developing on a wide
>> range of platforms, before making it the default.
>>
>> Have the devkits been updated so I can try this out myself?
>>
>> Thanks,
>> David
>>
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8195689
>>> WebRev: http://cr.openjdk.java.net/~ihse/JDK-8195689-remove-generate
>>> d-configure/webrev.01
>>>
>>
>>
>>> /Magnus
>>>
>>
>


Re: RFR: JDK-8195689 Remove generated-configure.sh and instead use autoconf

2018-01-18 Thread Martin Buchholz
Differing projects have come to different conclusions about whether to
include a generated configure.

But the standard seems to be to include one. The mantra is: "./configure &&
make" without an autoconf step.  The number of people building openjdk is
much larger than the number of people patching configure.  So I agree with
David that we should stick with the status quo.

On Thu, Jan 18, 2018 at 6:14 PM, David Holmes 
wrote:

> On 18/01/2018 11:28 PM, 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.
>>
>
> I'm not convinced just by you saying it is so - sorry. This seems to make
> an already complex build process even more complex for every single person
> who wants to build OpenJDK, for the benefit of a handful of people who may
> want to modify configure options and whom already work closely with the
> build team and so there's really little hardship in getting a sponsor, or
> just someone with access to autoconf.
>
> It introduces a new point of failure in the build for everyone.
>
> Has this been beta-tested with external contributors? I'd be happier
> knowing we've put this through its paces with people developing on a wide
> range of platforms, before making it the default.
>
> Have the devkits been updated so I can try this out myself?
>
> Thanks,
> David
>
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8195689
>> WebRev: http://cr.openjdk.java.net/~ihse/JDK-8195689-remove-generate
>> d-configure/webrev.01
>>
>
>
>> /Magnus
>>
>


Re: jdk10 on macOS

2018-01-18 Thread Jonathan Gibbons



On 01/18/2018 05:51 PM, David Holmes wrote:

On 19/01/2018 3:20 AM, Alan Snyder wrote:
I tried building from the jdk repo (vs jdk10). The 
StringPlatformChars test passed. The other four tests appear to have 
been removed.


However, I did get failures on these tests:

• MethodHandleConstants (internal error: appendix)

> • MethodReferenceTestMethodHandle (internal error: appendix)

https://bugs.openjdk.java.net/browse/JDK-8194963


• JavadocHelperTest (out of memory — Java heap space)


Common problem with javadoc.


Regrettably true, but that's not the problem in this case.The test 
is a jshell test that is analysing doc-comments, but without using the 
javadoc tool.


The heap needs to be increased. I've encountered the problem myself and 
already raised the issue with the JShell folk.


-- Jon



David



On Jan 17, 2018, at 2:24 PM, David Holmes  
wrote:


Alan,

On 18/01/2018 6:52 AM, Alan Snyder wrote:

To summarize, these are the test failures/errors:
• StringPlatformChars (error — native code not found)


This seems potentially a makefile issue.


• NewUnsafeString (did not use provided string)
• APIExtraction (class file for TestClass not found)
• ClassDependenciesTest (assertion error — null pointer in 
loadClass)

• IgnoreSymbolFile (package sun.reflect.annotation is not visible)


Any other test failures should be taken up with the component area 
related to the test - and you can also search the bug systems for 
references to such failures. Not all tests are guaranteed to always 
pass in every environment - though the intent for tier1/2 tests that 
(unless excluded via the ProblemList.txt file, or marked 
"intermittent") they do pass.


David






Re: RFR: JDK-8195689 Remove generated-configure.sh and instead use autoconf

2018-01-18 Thread Erik Joelsson
Just apply the patch and you can try it. The current version of the patch is 
somewhat broken on windows though, needs another iteration.
/Erik

⁣Sent from BlueMail ​

On Jan 18, 2018, 18:14, at 18:14, David Holmes  wrote:
>On 18/01/2018 11:28 PM, 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.
>
>I'm not convinced just by you saying it is so - sorry. This seems to
>make an already complex build process even more complex for every
>single
>person who wants to build OpenJDK, for the benefit of a handful of
>people who may want to modify configure options and whom already work
>closely with the build team and so there's really little hardship in
>getting a sponsor, or just someone with access to autoconf.
>
>It introduces a new point of failure in the build for everyone.
>
>Has this been beta-tested with external contributors? I'd be happier
>knowing we've put this through its paces with people developing on a
>wide range of platforms, before making it the default.
>
>Have the devkits been updated so I can try this out myself?
>
>Thanks,
>David
>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8195689
>> WebRev:
>>
>http://cr.openjdk.java.net/~ihse/JDK-8195689-remove-generated-configure/webrev.01
>
>
>>
>> /Magnus


Re: RFR: JDK-8195689 Remove generated-configure.sh and instead use autoconf

2018-01-18 Thread David Holmes

On 18/01/2018 11:28 PM, 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.


I'm not convinced just by you saying it is so - sorry. This seems to 
make an already complex build process even more complex for every single 
person who wants to build OpenJDK, for the benefit of a handful of 
people who may want to modify configure options and whom already work 
closely with the build team and so there's really little hardship in 
getting a sponsor, or just someone with access to autoconf.


It introduces a new point of failure in the build for everyone.

Has this been beta-tested with external contributors? I'd be happier 
knowing we've put this through its paces with people developing on a 
wide range of platforms, before making it the default.


Have the devkits been updated so I can try this out myself?

Thanks,
David


Bug: https://bugs.openjdk.java.net/browse/JDK-8195689
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8195689-remove-generated-configure/webrev.01 




/Magnus


Re: jdk10 on macOS

2018-01-18 Thread David Holmes

On 19/01/2018 3:20 AM, Alan Snyder wrote:

I tried building from the jdk repo (vs jdk10). The StringPlatformChars test 
passed. The other four tests appear to have been removed.

However, I did get failures on these tests:

• MethodHandleConstants (internal error: appendix)

>• MethodReferenceTestMethodHandle (internal error: appendix)

https://bugs.openjdk.java.net/browse/JDK-8194963


• JavadocHelperTest (out of memory — Java heap space)


Common problem with javadoc.

David




On Jan 17, 2018, at 2:24 PM, David Holmes  wrote:

Alan,

On 18/01/2018 6:52 AM, Alan Snyder wrote:

To summarize, these are the test failures/errors:
• StringPlatformChars (error — native code not found)


This seems potentially a makefile issue.


• NewUnsafeString (did not use provided string)
• APIExtraction (class file for TestClass not found)
• ClassDependenciesTest (assertion error — null pointer in loadClass)
• IgnoreSymbolFile (package sun.reflect.annotation is not visible)


Any other test failures should be taken up with the component area related to the test - 
and you can also search the bug systems for references to such failures. Not all tests 
are guaranteed to always pass in every environment - though the intent for tier1/2 tests 
that (unless excluded via the ProblemList.txt file, or marked "intermittent") 
they do pass.

David




Re: [OpenJDK 2D-Dev] [PATCH] Build fails to compile jchuff.c

2018-01-18 Thread Phil Race
I agree with what Erik said on build-dev that being specific about the 
tool chain
and the reason are worthwhile and important. We've done that in similar 
cases.


Also these review threads usually should have a subject like
RFR: : 

which means you first need a bug id .. the patch can't be pushed without 
one anyway.


Then the patch should be an in-line diff or a webrev hosted on 
cr.openjdk.java.net.


I think in-line would be OK for this small change.

-phil.

On 01/17/2018 09:30 AM, Adam Farley8 wrote:

Hi All,

Under these circumstances, jchuff.c will not compile:

Platform: zLinux (s390x)
Release: JDK9 (may affect other JDKs).
GCC Version: 4.8.5
Notes: --disable-warnings-as-errors suppresses this error.

The error is:

/home/adamfarl/hotspot/jdk9/jdk/src/java.desktop/share/native/libjavajpeg/jchuff.c:
In function 'jGenOptTbl':
/home/adamfarl/hotspot/jdk9/jdk/src/java.desktop/share/native/libjavajpeg/jchuff.c:808:18:
error: array subscript is below array bounds [-Werror=array-bounds]
  while (bits[j] == 0)
 ^

This is a continuation of a conversation in the build-dev mailing 
list, if anyone wants to

check the history.

The short version is that, while you *can* suppress the problem by adding
--disable-warnings-as-errors to your configure step, I posit that a 
builder shouldn't

have to.

Various solutions were debated. One involves changing Awt2dLibraries.gmk.

Basically you change line 494 to this:

DISABLED_WARNINGS_gcc := clobbered array-bounds, \

I'm running a build now to check that works, but basically we should 
end up with a
-Wno-array-bounds on the gcc compile command for jchuff.c, thereby 
ignoring the warning.


A smarter variant involves checking for that specific version of the 
gcc, but that seems

wordy to me for this problem. Keeping it simple. :)

Thoughts?

Best Regards

Adam Farley

Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with 
number 741598.

Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU




Re: [OpenJDK 2D-Dev] [PATCH] Build fails to compile jchuff.c

2018-01-18 Thread Phil Race

Try again with build-dev cc'd ..

-phil.

On 01/18/2018 11:14 AM, Phil Race wrote:
I agree with what Erik said on build-dev that being specific about the 
tool chain
and the reason are worthwhile and important. We've done that in 
similar cases.


Also these review threads usually should have a subject like
RFR: : 

which means you first need a bug id .. the patch can't be pushed 
without one anyway.


Then the patch should be an in-line diff or a webrev hosted on 
cr.openjdk.java.net.


I think in-line would be OK for this small change.

-phil.

On 01/17/2018 09:30 AM, Adam Farley8 wrote:

Hi All,

Under these circumstances, jchuff.c will not compile:

Platform: zLinux (s390x)
Release: JDK9 (may affect other JDKs).
GCC Version: 4.8.5
Notes: --disable-warnings-as-errors suppresses this error.

The error is:

/home/adamfarl/hotspot/jdk9/jdk/src/java.desktop/share/native/libjavajpeg/jchuff.c:
In function 'jGenOptTbl':
/home/adamfarl/hotspot/jdk9/jdk/src/java.desktop/share/native/libjavajpeg/jchuff.c:808:18:
error: array subscript is below array bounds [-Werror=array-bounds]
  while (bits[j] == 0)
 ^

This is a continuation of a conversation in the build-dev mailing 
list, if anyone wants to

check the history.

The short version is that, while you *can* suppress the problem by 
adding
--disable-warnings-as-errors to your configure step, I posit that a 
builder shouldn't

have to.

Various solutions were debated. One involves changing 
Awt2dLibraries.gmk.


Basically you change line 494 to this:

DISABLED_WARNINGS_gcc := clobbered array-bounds, \

I'm running a build now to check that works, but basically we should 
end up with a
-Wno-array-bounds on the gcc compile command for jchuff.c, thereby 
ignoring the warning.


A smarter variant involves checking for that specific version of the 
gcc, but that seems

wordy to me for this problem. Keeping it simple. :)

Thoughts?

Best Regards

Adam Farley

Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with 
number 741598.
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire 
PO6 3AU






Re: RFR: JDK-8195689 Remove generated-configure.sh and instead use autoconf

2018-01-18 Thread Erik Joelsson

Hello Magnus,

Nice to see this finally happening!

In building.md, when getting autoconf for Cygwin, I believe you also 
need the autoconf wrapper scripts package. Perhaps it's also worth 
mentioning that you can download the autoconf src and build/install from 
there?


In jib-profiles.js, you need to use build_platform instead of 
target_platform when creating the module name for the dependency. 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.


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

/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




Re: [PATCH] (Title Corrected) Build fails to compile jchuff.c using gcc 4.8.5 on zLinux

2018-01-18 Thread John Paul Adrian Glaubitz

Hi!

On 01/18/2018 06:27 PM, Erik Joelsson wrote:

When adding a disabled warning like this, we need to also add a clear comment
describing why it's necessary. In this case it's caused by a bug in GCC and only
affects certain versions. Otherwise, we will likely try to remove them later and
without information on why it was added, we will just conclude that the warning
is not triggering with the official GCC version and remove it.


I don't quite understand why we would want to patch OpenJDK-11 in the first 
place
when the reason this bug is triggered is a) a known toolchain bug in an old and
unsupported version of gcc and b) can be easily disabled by building with
"--disable-warnings-as-errors".

The only thing here to patch is obviously gcc and not any other project that is
built with gcc. I am pretty confident that OpenJDK isn't the only project that
is affected by this particular gcc bug.

And if a Linux distribution is using gcc-4.8.5 as their default compiler, they 
will
most likely not be using OpenJDK-11 but rather be stuck with OpenJDK-7 or 8.

Thus, I don't see a point in working around bugs in an ancient toolchain in the
current OpenJDK code.

Thanks,
Adrian

--
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaub...@debian.org
`. `'   Freie Universitaet Berlin - glaub...@physik.fu-berlin.de
  `-GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913


Re: RFR: JDK-8195689 Remove generated-configure.sh and instead use autoconf

2018-01-18 Thread Jonathan Gibbons



On 1/18/18 8:39 AM, Erik Joelsson wrote:



On 2018-01-18 07:42, Jonathan Gibbons wrote:

On 1/18/18 5:28 AM, Magnus Ihse Bursie wrote:



* 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. 
Can you provide more details here?  My recent experience is that some 
older versions of Ubuntu are OK, with respect to autoconf 2.69.  Is 
there are list somewhere of which versions of Ubuntu are good and 
which are bad?


I know 16.04 is bad and 14.04 is good. Likely other non LTS versions 
are bad as well. The point is though that with this change, we won't 
need to care anymore. The autoconf in Ubuntu still works fine for 
building the product. We just don't like it now because it introduces 
spurious changes in the history for generated-configure.sh.


/Erik


OK, thanks for the clarification.

-- Jon


Re: [PATCH] (Title Corrected) Build fails to compile jchuff.c using gcc 4.8.5 on zLinux

2018-01-18 Thread Erik Joelsson

Hello Adam,

When adding a disabled warning like this, we need to also add a clear 
comment describing why it's necessary. In this case it's caused by a bug 
in GCC and only affects certain versions. Otherwise, we will likely try 
to remove them later and without information on why it was added, we 
will just conclude that the warning is not triggering with the official 
GCC version and remove it.


If the affected versions is limited, then we may also consider making 
this conditional on the GCC version. The version is available in the 
variables TOOLCHAIN_VERSION, CC_VERSION_NUMBER and CXX_VERSION_NUMBER.


/Erik

On 2018-01-18 03:15, Adam Farley8 wrote:

Hi All

I sent an email to the 2d-dev list yesterday, but I'll respond here as 
well

so you guys know I'm not ignoring you. :)

> This is all correct, thanks David!
>
> For the official toolchains (basically what Oracle builds with), we very
> much like to keep warnings-as-errors active, because it's a very
> valuable tool in keeping the code healthy. For other toolchains, it
> depends, as David says.
>
> We have a mechanism for disabling warnings for specific toolchain types
> (gcc, clang, solstudio, visualstudio) on a per library basis. We also
> have the ability to add flags globally for specific toolchain versions
> in configure, in flags.m4. If we want to solve this by disabling a
> warning due to a bug in a specific gcc version, I would recommend the
> latter.
>
> /Erik

This is correct. In flags.m4, GCC has a potential
DISABLE_WARNING_PREFIX value of "-Wno-".

Yesterday I posted to 2d-dev and recommended changing
Awt2dLibraries.gmk, which supplies suffixes for that prefix

Basically you change line 494 to this:

    DISABLED_WARNINGS_gcc := clobbered array-bounds, \

This puts a -Wno-array-bounds on the gcc compile command for
jchuff.c, thereby ignoring the error-warning I'm seeing.

I ran a build to confirm this works. It did, and the build completed
without further errors.

This fix, if accepted, means --disable-warnings-as-errors will not be 
needed
in future zLinux compiles using this gcc (which, as David points out, 
is the

gcc version on the build list).

Just "bash ./compile" and "make all". Simples!

Please send future responses through my email to the 2d-dev list.

http://mail.openjdk.java.net/pipermail/2d-dev/2018-January/008836.html

Thanks for your time. :)

Best Regards

Adam Farley

Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with 
number 741598.

Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU




Re: jdk10 on macOS

2018-01-18 Thread Alan Snyder
I tried building from the jdk repo (vs jdk10). The StringPlatformChars test 
passed. The other four tests appear to have been removed.

However, I did get failures on these tests:

• MethodHandleConstants (internal error: appendix)
• JavadocHelperTest (out of memory — Java heap space)
• MethodReferenceTestMethodHandle (internal error: appendix)


> On Jan 17, 2018, at 2:24 PM, David Holmes  wrote:
> 
> Alan,
> 
> On 18/01/2018 6:52 AM, Alan Snyder wrote:
>> To summarize, these are the test failures/errors:
>>  • StringPlatformChars (error — native code not found)
> 
> This seems potentially a makefile issue.
> 
>>  • NewUnsafeString (did not use provided string)
>>  • APIExtraction (class file for TestClass not found)
>>  • ClassDependenciesTest (assertion error — null pointer in loadClass)
>>  • IgnoreSymbolFile (package sun.reflect.annotation is not visible)
> 
> Any other test failures should be taken up with the component area related to 
> the test - and you can also search the bug systems for references to such 
> failures. Not all tests are guaranteed to always pass in every environment - 
> though the intent for tier1/2 tests that (unless excluded via the 
> ProblemList.txt file, or marked "intermittent") they do pass.
> 
> David



Re: RFR: JDK-8195689 Remove generated-configure.sh and instead use autoconf

2018-01-18 Thread Erik Joelsson



On 2018-01-18 07:42, Jonathan Gibbons wrote:

On 1/18/18 5:28 AM, Magnus Ihse Bursie wrote:



* 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. 
Can you provide more details here?  My recent experience is that some 
older versions of Ubuntu are OK, with respect to autoconf 2.69.  Is 
there are list somewhere of which versions of Ubuntu are good and 
which are bad?


I know 16.04 is bad and 14.04 is good. Likely other non LTS versions are 
bad as well. The point is though that with this change, we won't need to 
care anymore. The autoconf in Ubuntu still works fine for building the 
product. We just don't like it now because it introduces spurious 
changes in the history for generated-configure.sh.


/Erik


Re: RFR: JDK-8195689 Remove generated-configure.sh and instead use autoconf

2018-01-18 Thread Jonathan Gibbons

On 1/18/18 5:28 AM, Magnus Ihse Bursie wrote:



* 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. 
Can you provide more details here?  My recent experience is that some 
older versions of Ubuntu are OK, with respect to autoconf 2.69.  Is 
there are list somewhere of which versions of Ubuntu are good and which 
are bad?


-- Jon


Re: RFR: JDK-8195689 Remove generated-configure.sh and instead use autoconf

2018-01-18 Thread Severin Gehwolf
On Thu, 2018-01-18 at 14:28 +0100, 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

Very cool! +1

Note: I haven't really reviewed the patch. This change in approach is
very welcome, though.

Thanks,
Severin


RFR: JDK-8195689 Remove generated-configure.sh and instead use autoconf

2018-01-18 Thread Magnus Ihse Bursie
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


Re: [PATCH] (Title Corrected) Build fails to compile jchuff.c using gcc 4.8.5 on zLinux

2018-01-18 Thread Adam Farley8
Hi All

I sent an email to the 2d-dev list yesterday, but I'll respond here as 
well 
so you guys know I'm not ignoring you. :)

> This is all correct, thanks David!
>
> For the official toolchains (basically what Oracle builds with), we very 

> much like to keep warnings-as-errors active, because it's a very 
> valuable tool in keeping the code healthy. For other toolchains, it 
> depends, as David says.
>
> We have a mechanism for disabling warnings for specific toolchain types 
> (gcc, clang, solstudio, visualstudio) on a per library basis. We also 
> have the ability to add flags globally for specific toolchain versions 
> in configure, in flags.m4. If we want to solve this by disabling a 
> warning due to a bug in a specific gcc version, I would recommend the 
> latter.
>
> /Erik

This is correct. In flags.m4, GCC has a potential 
DISABLE_WARNING_PREFIX value of "-Wno-". 

Yesterday I posted to 2d-dev and recommended changing 
Awt2dLibraries.gmk, which supplies suffixes for that prefix

Basically you change line 494 to this:

DISABLED_WARNINGS_gcc := clobbered array-bounds, \

This puts a -Wno-array-bounds on the gcc compile command for 
jchuff.c, thereby ignoring the error-warning I'm seeing.

I ran a build to confirm this works. It did, and the build completed 
without further errors.

This fix, if accepted, means --disable-warnings-as-errors will not be 
needed
in future zLinux compiles using this gcc (which, as David points out, is 
the
gcc version on the build list).

Just "bash ./compile" and "make all". Simples!

Please send future responses through my email to the 2d-dev list.

http://mail.openjdk.java.net/pipermail/2d-dev/2018-January/008836.html

Thanks for your time. :)

Best Regards

Adam Farley

Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598. 
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU


Re: [PATCH] Build fails to compile jchuff.c using gcc 4.5 on zLinux

2018-01-18 Thread Adam Farley8
> This isn't really a question for build-dev. It should be brought to the 
> component team owning that particular source. I believe in this case 
> that would be 2d-dev.

> /Erik

Hi Erik,

This has been mentioned. One of my responses yesterday indicates I raised 
a counterpart in 2d-dev already.

Thanks for your time. :)

Best Regards

Adam Farley

Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598. 
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU