RFR: 8253000: Remove redundant MAKE_SUBDIR argument
Hi All, Reviews and sponsor requested for the removal of the now-redundant MAKE_SUBDIR argument in DeclareRecipesForPhase's comment, along with its removal from anything that calls that macro. Bug: https://bugs.openjdk.java.net/browse/JDK-8253000 Webrev: http://cr.openjdk.java.net/~afarley/8253000/webrev/ Best Regards Adam Farley IBM Runtimes 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
RFR: 8252998: ModuleWrapper.gmk doesn't consult include path
Hi All, Requesting reviews and sponsor for the following change. A previous change (JDK-8244044) appears to prevent make from checking the include dirs for an included gmk file. This means that you can't override the included file using the include dirs, as was previously the case. This sounds like a bug, and should be fixed. Thoughts welcome. Bug: https://bugs.openjdk.java.net/browse/JDK-8252998 Webrev: http://cr.openjdk.java.net/~afarley/8252998/webrev/ Best Regards Adam Farley IBM Runtimes 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(XS): 8214063: [AIX] Disable symbol visibility flags
Thanks Volker. :) Best Regards Adam Farley IBM Runtimes Volker Simonis wrote on 04/12/2018 08:30:58: > From: Volker Simonis > To: adam.far...@uk.ibm.com > Cc: build-dev , Magnus Ihse Bursie > , ppc-aix-port-...@openjdk.java.net > Date: 04/12/2018 08:31 > Subject: Re: RFR(XS): 8214063: [AIX] Disable symbol visibility flags > > Hi Adam, > > I've just pushed the change: > > INVALID URI REMOVED > u=http-3A__hg.openjdk.java.net_jdk_jdk_rev_fc54d27e58d8&d=DwIBaQ&c=jf_iaSHvJObTbx- > siA1ZOg&r=P5m8KWUXJf- > CeVJc0hDGD9AQ2LkcXDC0PMV9ntVw5Ho&m=NhALBBoEo6HsbPIjB8bJJj30UR8DRP- > PuJckMbmJvA0&s=gLabfGk2XJdLwimruwQdLAmjBXtCueO7qR01_xw5wuw&e= > > Best regards, > Volker > On Thu, Nov 29, 2018 at 5:54 PM Adam Farley8 wrote: > > > > Hi All, > > > > The build passed on xlC 13.1 with the makefile patch I proposed > (good catch on the comments Volkar!). > > > > With Volkar, Erik, Matthias, and Magnus all approving the change, > it sounds like we're good to merge! > > > > Volkar, can you do the honours? > > > > Best Regards > > > > Adam Farley > > IBM Runtimes > > > > P.S. I approve the change too. ;) > > > > > > Volker Simonis wrote on 29/11/2018 11:54:33: > > > > > From: Volker Simonis > > > To: Magnus Ihse Bursie > > > Cc: build-dev , ppc-aix-port- > > > d...@openjdk.java.net, adam.far...@uk.ibm.com > > > Date: 29/11/2018 11:54 > > > Subject: Re: RFR(XS): 8214063: [AIX] Disable symbol visibility flags > > > > > > On Thu, Nov 29, 2018 at 12:20 PM Magnus Ihse Bursie > > > wrote: > > > > > > > > On 2018-11-27 16:33, Volker Simonis wrote: > > > > > > > > > Hi, > > > > > > > > > > can I please have a review for the following trivial change which > > > > > simply disables the symbol visibility flags on AIX: > > > > > > > > > > INVALID URI REMOVED > > > > u=http-3A__cr.openjdk.java.net_-7Esimonis_webrevs_2018_8214063_&d=DwIBaQ&c=jf_iaSHvJObTbx- > > > siA1ZOg&r=P5m8KWUXJf- > > > CeVJc0hDGD9AQ2LkcXDC0PMV9ntVw5Ho&m=6y4Npxy6aG4q8E9Xca-- > > > YxF4UGVrVEIqu_wVvivFVUA&s=DptrWUUtJCcpUCbCWkkBOeFJCVk5im3hm9T_DcD0Jd8&e= > > > > > INVALID URI REMOVED > > > > u=https-3A__bugs.openjdk.java.net_browse_JDK-2D8214063&d=DwIBaQ&c=jf_iaSHvJObTbx- > > > siA1ZOg&r=P5m8KWUXJf- > > > CeVJc0hDGD9AQ2LkcXDC0PMV9ntVw5Ho&m=6y4Npxy6aG4q8E9Xca-- > > > YxF4UGVrVEIqu_wVvivFVUA&s=jBFABkJb5E5W9K8pMX794-3gnpLfPyi3oASA1kizQ7A&e= > > > > Looks good to me. I am sorry for the mess I caused by optimisically > > > > trying to fix things on a platform I could not compile on... :( > > > > > > > > > > Thanks for the review and don't worry! We really appreciate your > > > continued help. It's really us who should have tested and spotted the > > > problems earlier :) > > > > > > Regards, > > > Volker > > > > > > > This also reminds me that the visibility flags *really* shouldmove into > > > > configure/spec, not be sprinkled like this in the make files. > > > > > > > > /Magnus > > > > > > > > > > Change "8202322: AIX: symbol visibility flags not support onxlc 12.1" > > > > > [1] blindly introduced these flags for all xlC compiler versions > > > > > > 12.1 without ever testing it (which should not have happened). Now > > > > > that people are starting to really use xlC 13 it turns out that there > > > > > is more to do than just enabling the flags. This future work is > > > > > covered by "8204541: Correctly support AIX xlC 13.1 symbol visibility > > > > > flags". > > > > > > > > > > Thank you and best regards, > > > > > Volker > > > > > > > > > > [1] INVALID URI REMOVED > > > > u=https-3A__bugs.openjdk.java.net_browse_JDK-2D8202322&d=DwIBaQ&c=jf_iaSHvJObTbx- > > > siA1ZOg&r=P5m8KWUXJf- > > > CeVJc0hDGD9AQ2LkcXDC0PMV9ntVw5Ho&m=6y4Npxy6aG4q8E9Xca-- > > > YxF4UGVrVEIqu_wVvivFVUA&s=pd7-rH7OPxeaq2g6S0dQPmb_3-8PLi8JZFKcP_Abp6Q&e= > > > > > [2] INVALID URI REMOVED > > > > u=https-3A__bugs.openjdk.java.net_browse_JDK-2D8204541&d=DwIBaQ&c=jf_iaSHvJObTbx- > > > siA1ZOg&r=P5m8KWUXJf- > > > CeVJc0hDGD9AQ2LkcXDC0PMV9ntVw5Ho&m=6y4Npxy6aG4q8E9Xca-- > > > YxF4UGVrVEIqu_wVvivFVUA&s=q7KHUASpF-opdcLXbTTUT1bPoKrkTeaHTtd7c2jN4rc&e= > > > > > > > > > > > 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 > 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(XS): 8214063: [AIX] Disable symbol visibility flags
Hi All, The build passed on xlC 13.1 with the makefile patch I proposed (good catch on the comments Volkar!). With Volkar, Erik, Matthias, and Magnus all approving the change, it sounds like we're good to merge! Volkar, can you do the honours? Best Regards Adam Farley IBM Runtimes P.S. I approve the change too. ;) Volker Simonis wrote on 29/11/2018 11:54:33: > From: Volker Simonis > To: Magnus Ihse Bursie > Cc: build-dev , ppc-aix-port- > d...@openjdk.java.net, adam.far...@uk.ibm.com > Date: 29/11/2018 11:54 > Subject: Re: RFR(XS): 8214063: [AIX] Disable symbol visibility flags > > On Thu, Nov 29, 2018 at 12:20 PM Magnus Ihse Bursie > wrote: > > > > On 2018-11-27 16:33, Volker Simonis wrote: > > > > > Hi, > > > > > > can I please have a review for the following trivial change which > > > simply disables the symbol visibility flags on AIX: > > > > > > INVALID URI REMOVED > u=http-3A__cr.openjdk.java.net_-7Esimonis_webrevs_2018_8214063_&d=DwIBaQ&c=jf_iaSHvJObTbx- > siA1ZOg&r=P5m8KWUXJf- > CeVJc0hDGD9AQ2LkcXDC0PMV9ntVw5Ho&m=6y4Npxy6aG4q8E9Xca-- > YxF4UGVrVEIqu_wVvivFVUA&s=DptrWUUtJCcpUCbCWkkBOeFJCVk5im3hm9T_DcD0Jd8&e= > > > INVALID URI REMOVED > u=https-3A__bugs.openjdk.java.net_browse_JDK-2D8214063&d=DwIBaQ&c=jf_iaSHvJObTbx- > siA1ZOg&r=P5m8KWUXJf- > CeVJc0hDGD9AQ2LkcXDC0PMV9ntVw5Ho&m=6y4Npxy6aG4q8E9Xca-- > YxF4UGVrVEIqu_wVvivFVUA&s=jBFABkJb5E5W9K8pMX794-3gnpLfPyi3oASA1kizQ7A&e= > > Looks good to me. I am sorry for the mess I caused by optimisically > > trying to fix things on a platform I could not compile on... :( > > > > Thanks for the review and don't worry! We really appreciate your > continued help. It's really us who should have tested and spotted the > problems earlier :) > > Regards, > Volker > > > This also reminds me that the visibility flags *really* should move into > > configure/spec, not be sprinkled like this in the make files. > > > > /Magnus > > > > > > Change "8202322: AIX: symbol visibility flags not support on xlc 12.1" > > > [1] blindly introduced these flags for all xlC compiler versions > > > > 12.1 without ever testing it (which should not have happened). Now > > > that people are starting to really use xlC 13 it turns out that there > > > is more to do than just enabling the flags. This future work is > > > covered by "8204541: Correctly support AIX xlC 13.1 symbol visibility > > > flags". > > > > > > Thank you and best regards, > > > Volker > > > > > > [1] INVALID URI REMOVED > u=https-3A__bugs.openjdk.java.net_browse_JDK-2D8202322&d=DwIBaQ&c=jf_iaSHvJObTbx- > siA1ZOg&r=P5m8KWUXJf- > CeVJc0hDGD9AQ2LkcXDC0PMV9ntVw5Ho&m=6y4Npxy6aG4q8E9Xca-- > YxF4UGVrVEIqu_wVvivFVUA&s=pd7-rH7OPxeaq2g6S0dQPmb_3-8PLi8JZFKcP_Abp6Q&e= > > > [2] INVALID URI REMOVED > u=https-3A__bugs.openjdk.java.net_browse_JDK-2D8204541&d=DwIBaQ&c=jf_iaSHvJObTbx- > siA1ZOg&r=P5m8KWUXJf- > CeVJc0hDGD9AQ2LkcXDC0PMV9ntVw5Ho&m=6y4Npxy6aG4q8E9Xca-- > YxF4UGVrVEIqu_wVvivFVUA&s=q7KHUASpF-opdcLXbTTUT1bPoKrkTeaHTtd7c2jN4rc&e= > > > 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] RFR(xxxs): 8200052: libjavajpeg: Fix compile warning in jchuff.c
Sounds OK to me. :)
Best Regards
Adam Farley
Philip Race wrote on 01/06/2018 03:04:12:
> From: Philip Race
> To: Adam Farley8
> Cc: 2d-dev <2d-...@openjdk.java.net>, build-dev d...@openjdk.java.net>, Andrew Leonard ,
> "Stuefe, Thomas"
> Date: 01/06/2018 03:04
> Subject: Re: [OpenJDK 2D-Dev] RFR(xxxs): 8200052: libjavajpeg: Fix
> compile warning in jchuff.c
>
> > It looks fine to me but I am a bit hazy about who to give
> attribution for the fix ..
>
> I pondered this for a little while and decided it should be
> joint between Adam who identified the issue and championed
> it and Thomas who I think first suggested the code change, modified only
> slightly at Guido's suggestion.
>
> I'll push it tomorrow if every one is OK with that.
>
> -phil.
>
> On 5/31/18, 10:33 AM, Phil Race wrote:
>
> I've grabbed the bug from Thomas and re-opened it
>
> https://bugs.openjdk.java.net/browse/
>
> Your webrev was stripped so I've uploaded here :
>
> http://cr.openjdk.java.net/~prr/8200052/
>
> It looks fine to me but I am a bit hazy about who to give
> attribution for the fix ..
> It is small enough to not require an OCA so there's no issue there
> if we attribute
> it to the IJG guy.
>
> -phil.
> On 05/31/2018 06:31 AM, Adam Farley8 wrote:
> An attachment in the email has been found to contain executable code
> and has been removed.
>
> File removed : webrev.zip, zip,html,js
> Hi Phil,
>
> As requested:
>
>
>
> FYI: I've also contacted Guido, confirmed that the fix worked, and asked
> him to go ahead with merging the fix into his code base.
>
> Best Regards
>
> Adam Farley
>
>
> Phil Race wrote on 30/05/2018 18:06:19:
>
> > From: Phil Race
> > To: Adam Farley8
> > Cc: 2d-dev <2d-...@openjdk.java.net>, Andrew Leonard
> > , build-dev > d...@openjdk.java.net>, Magnus Ihse Bursie
> > , "Thomas Stüfe"
> > Date: 30/05/2018 18:07
> > Subject: Re: [OpenJDK 2D-Dev] RFR(xxxs): 8200052: libjavajpeg: Fix
> > compile warning in jchuff.c
> >
> > Thank you for persevering with this. Please submit a webrev with this
> > change .. I think you can leave out the change to jerror.h in the
> jpeg6b case.
> >
> > -phil.
>
> > On 05/30/2018 09:08 AM, Adam Farley8 wrote:
> > Hi Phil,
> >
> > I spoke with the jpegclub rep "Guido", and he was very helpful.
> >
> > He agreed to accept a code change, but recommended an error instead
> > of a while check.
> >
> > -- Line 808:
> > < while (bits[j] == 0)
> > < j--;
> > ---
> > > while (bits[j] == 0) {
> > > if (j == 0)
> > > ERREXIT(cinfo, JERR_HUFF_CLEN_OVERFLOW);
> > > j--;
> > > }
> > --
> >
> > This makes sense to me, and I verified that it prevents the error.
> >
> > He says:
> >
> > For the release version I would replace the specific
> > JERR_HUFF_CLEN_OVERFLOW by the more general
> > JERR_HUFF_CLEN_OUTOFBOUNDS so that it suits both cases, this
> > requires a change in jerror.h:
> >
> > -JMESSAGE(JERR_HUFF_CLEN_OVERFLOW, "Huffman code size table overflow")
> > +JMESSAGE(JERR_HUFF_CLEN_OUTOFBOUNDS, "Huffman code size table out
> of bounds")
> >
> > The next version (9d) is planned for release in January 2020.
> > A pre-release package will be provided in 2019 on http://
> > jpegclub.org/reference/reference-sources/, I will send you a
notification.
> >
> >
> > While we *could* make the jerror.h change, I suggest we don't for
> > now. If we merge from upstream in January 2020, we'll get that
> > change anyway when the time comes.
> >
> > So what do you say to including the dashed change referenced above
> > to fix our problem now, safe in the knowledge that upstream will be
> > similarly modified (except with the new error type).
> >
> > Best Regards
> >
> > Adam Farley
> >
> > P.S. I'm holding off on giving Guido the green light until after
> > people say if they're happy with the error-enabled version of the fix.
> >
> > Adam Farley8/UK/IBM wrote on 14/05/2018 11:06:28:
> >
> > > From: Adam Farley8/UK/IBM
> > > To: Phil Race
> > > Cc: 2d-dev <2d-...@openjdk.java.net>, Andrew Leonard
> > &g
Re: [OpenJDK 2D-Dev] RFR(xxxs): 8200052: libjavajpeg: Fix compile warning in jchuff.c
Hi Phil,
As requested:
FYI: I've also contacted Guido, confirmed that the fix worked, and asked
him to go ahead with merging the fix into his code base.
Best Regards
Adam Farley
Phil Race wrote on 30/05/2018 18:06:19:
> From: Phil Race
> To: Adam Farley8
> Cc: 2d-dev <2d-...@openjdk.java.net>, Andrew Leonard
> , build-dev d...@openjdk.java.net>, Magnus Ihse Bursie
> , "Thomas Stüfe"
> Date: 30/05/2018 18:07
> Subject: Re: [OpenJDK 2D-Dev] RFR(xxxs): 8200052: libjavajpeg: Fix
> compile warning in jchuff.c
>
> Thank you for persevering with this. Please submit a webrev with this
> change .. I think you can leave out the change to jerror.h in the jpeg6b
case.
>
> -phil.
> On 05/30/2018 09:08 AM, Adam Farley8 wrote:
> Hi Phil,
>
> I spoke with the jpegclub rep "Guido", and he was very helpful.
>
> He agreed to accept a code change, but recommended an error instead
> of a while check.
>
> -- Line 808:
> < while (bits[j] == 0)
> < j--;
> ---
> > while (bits[j] == 0) {
> > if (j == 0)
> > ERREXIT(cinfo, JERR_HUFF_CLEN_OVERFLOW);
> > j--;
> > }
> --
>
> This makes sense to me, and I verified that it prevents the error.
>
> He says:
>
> For the release version I would replace the specific
> JERR_HUFF_CLEN_OVERFLOW by the more general
> JERR_HUFF_CLEN_OUTOFBOUNDS so that it suits both cases, this
> requires a change in jerror.h:
>
> -JMESSAGE(JERR_HUFF_CLEN_OVERFLOW, "Huffman code size table overflow")
> +JMESSAGE(JERR_HUFF_CLEN_OUTOFBOUNDS, "Huffman code size table out of
bounds")
>
> The next version (9d) is planned for release in January 2020.
> A pre-release package will be provided in 2019 on http://
> jpegclub.org/reference/reference-sources/, I will send you a
notification.
>
>
> While we *could* make the jerror.h change, I suggest we don't for
> now. If we merge from upstream in January 2020, we'll get that
> change anyway when the time comes.
>
> So what do you say to including the dashed change referenced above
> to fix our problem now, safe in the knowledge that upstream will be
> similarly modified (except with the new error type).
>
> Best Regards
>
> Adam Farley
>
> P.S. I'm holding off on giving Guido the green light until after
> people say if they're happy with the error-enabled version of the fix.
>
> Adam Farley8/UK/IBM wrote on 14/05/2018 11:06:28:
>
> > From: Adam Farley8/UK/IBM
> > To: Phil Race
> > Cc: 2d-dev <2d-...@openjdk.java.net>, Andrew Leonard
> > , build-dev > d...@openjdk.java.net>, Magnus Ihse Bursie
> > , "Thomas Stüfe"
> > Date: 14/05/2018 11:06
> > Subject: Re: [OpenJDK 2D-Dev] RFR(xxxs): 8200052: libjavajpeg: Fix
> > compile warning in jchuff.c
> >
> > Hi Phil,
> >
> > Would an acceptable compromise be to deliver the source code change
> > and send the code to the upstream community, allowing them to include
> > the fix if/when they are able?
> >
> > I believe Magnus was advocating this idea as well. See below.
> >
> > Best Regards
> >
> > Adam Farley
> >
> > > Same here. I would like to have this fix in, but do not want to go
> > > over Phils head.
> > >
> > > I think Phil was the main objector, maybe he could reconsider?`
> > >
> > > Thanks, Thomas
> > >
> > > On Thu, Apr 26, 2018 at 10:39 AM, Magnus Ihse Bursie
> > > wrote:
> > > > I don't object, but it's not build code so I don't have the final
say.
> > > >
> > > > /Magnus
> > > >
> > > >
> > > > On 2018-04-25 17:43, Adam Farley8 wrote:
> > > >
> > > > Hi All,
> > > >
> > > > Does anyone have an objection to pushing this tiny change in?
> > > >
> > > > It doesn't break anything, it fixes a build break on two supported
> > > > platforms, and it seems
> > > > like we never refresh the code from upstream.
> > > >
> > > > - Adam
> > > >
> > > >> I also advocate the source code fix, as Make isn't meant to
> use the sort
> > > >> of logic required
> > > >> to properly analyse the toolchain version string.
> > > >>
> > > >> e.g. An "eq" match
Re: [OpenJDK 2D-Dev] RFR(xxxs): 8200052: libjavajpeg: Fix compile warning in jchuff.c
Hi Phil,
I spoke with the jpegclub rep "Guido", and he was very helpful.
He agreed to accept a code change, but recommended an error instead of a
while check.
-- Line 808:
< while (bits[j] == 0)
< j--;
---
> while (bits[j] == 0) {
> if (j == 0)
> ERREXIT(cinfo, JERR_HUFF_CLEN_OVERFLOW);
> j--;
> }
--
This makes sense to me, and I verified that it prevents the error.
He says:
For the release version I would replace the specific
JERR_HUFF_CLEN_OVERFLOW by the more general JERR_HUFF_CLEN_OUTOFBOUNDS so
that it suits both cases, this requires a change in jerror.h:
-JMESSAGE(JERR_HUFF_CLEN_OVERFLOW, "Huffman code size table overflow")
+JMESSAGE(JERR_HUFF_CLEN_OUTOFBOUNDS, "Huffman code size table out of
bounds")
The next version (9d) is planned for release in January 2020.
A pre-release package will be provided in 2019 on
http://jpegclub.org/reference/reference-sources/, I will send you a
notification.
While we *could* make the jerror.h change, I suggest we don't for now. If
we merge from upstream in January 2020, we'll get that change anyway when
the time comes.
So what do you say to including the dashed change referenced above to fix
our problem now, safe in the knowledge that upstream will be similarly
modified (except with the new error type).
Best Regards
Adam Farley
P.S. I'm holding off on giving Guido the green light until after people
say if they're happy with the error-enabled version of the fix.
Adam Farley8/UK/IBM wrote on 14/05/2018 11:06:28:
> From: Adam Farley8/UK/IBM
> To: Phil Race
> Cc: 2d-dev <2d-...@openjdk.java.net>, Andrew Leonard
> , build-dev d...@openjdk.java.net>, Magnus Ihse Bursie
> , "Thomas Stüfe"
> Date: 14/05/2018 11:06
> Subject: Re: [OpenJDK 2D-Dev] RFR(xxxs): 8200052: libjavajpeg: Fix
> compile warning in jchuff.c
>
> Hi Phil,
>
> Would an acceptable compromise be to deliver the source code change
> and send the code to the upstream community, allowing them to include
> the fix if/when they are able?
>
> I believe Magnus was advocating this idea as well. See below.
>
> Best Regards
>
> Adam Farley
>
> > Same here. I would like to have this fix in, but do not want to go
> > over Phils head.
> >
> > I think Phil was the main objector, maybe he could reconsider?`
> >
> > Thanks, Thomas
> >
> > On Thu, Apr 26, 2018 at 10:39 AM, Magnus Ihse Bursie
> > wrote:
> > > I don't object, but it's not build code so I don't have the final
say.
> > >
> > > /Magnus
> > >
> > >
> > > On 2018-04-25 17:43, Adam Farley8 wrote:
> > >
> > > Hi All,
> > >
> > > Does anyone have an objection to pushing this tiny change in?
> > >
> > > It doesn't break anything, it fixes a build break on two supported
> > > platforms, and it seems
> > > like we never refresh the code from upstream.
> > >
> > > - Adam
> > >
> > >> I also advocate the source code fix, as Make isn't meant to use the
sort
> > >> of logic required
> > >> to properly analyse the toolchain version string.
> > >>
> > >> e.g. An "eq" match on 4.8.5 doesn't protect the user who is using
4.8.6,
> > >> and Make doesn't
> > >> seem to do substring stuff unless you mess around with shells.
> > >>
> > >> Plus, as people have said, it's better to solve problem x (or work
around
> > >> a specific
> > >> instance of x) than to ignore the exception, even if the ignoring
code is
> > >> toolchain-
> > >> specific.
> > >>
> > >> - Adam Farley
> > >>
> > >> > On 2018-03-27 18:44, Phil Race wrote:
> > >> >
> > >> >> As I said I prefer the make file change, since this is a change
to an
> > >> >> upstream library.
> > >> >
> > >> > Newtons fourth law: For every reviewer, there's an equal and
opposite
> > >> > reviewer. :)
> > >> >
> > >> > Here I am, advocating a source code fix. As Thomas says, the
compiler
> > >> > complaint seems reasonable, and disabling it might cause us
> to miss other
> > >> > future errors.
> > >> >
> > >> > Why can't we push the source code fix, and also submit it
upstream?
> > >> >
> > >> > /Magnus
> > >> &g
Re: [OpenJDK 2D-Dev] RFR(xxxs): 8200052: libjavajpeg: Fix compile warning in jchuff.c
Thanks for the compliment, but I plan to leave this open for a little while longer. I have already heard back from the 6x community, and they say they are no longer associated with the IJG group (jpegclub.org) that creates 9x. Unless we see the potential for an upstream merge from the sourceforge 6x group, I'll be concentrating exclusively on pushing the change to the IJG 9x group. I found some email addresses on the IJG "Site Notice" page, and have already emailed the most likely one to try and push the change. In the meantime, I recommend getting the change into our local copy of the code to fix the bug. Phil, your thoughts? Best Regards Adam > I admire your perseverance :) but I think this is a fools errand. > > The amount of work you have to put into this far outbalances the > amount of work the OpenJDK maintainers would have to spend when (if > ever) they were to merge down upstream libjpeg. > > Note that we have a lot of experience with downstream work, since we > are downstream from OpenJDK - and Oracle JDK before that - since like > 14 years. Merging upstream with downstream-local patches is part of > our daily business. That is why I also think that it is quite normal > to keep local changes downstream and to merge, not just to replace, > upstream changes. But to do that differently is Oracle's prerogative. > > I for now am resigned to live with local patches (the irony :-) to the > s390 build. It is annoying but no big deal breaker. > > Thanks & best Regards, Thomas > > > On Fri, May 18, 2018 at 5:23 PM, Adam Farley8 wrote: > > Hi, > > > > I tried to use the IJG's contact page, but no joy. Seems broken; there a > > spinning icon when you hit "send", but nothing happens. > > > > http://jpegclub.org/reference/contact/ > > > > So I used a slightly older mailing list on sourceforge. The request to > > update their code has been sent, and I hope it will appear on the mailing > > list soon. > > > > https://sourceforge.net/p/libjpeg/mailman/libjpeg-devel-6x/ > > > > However, that list seems fairly idle too. > > > > Does anyone know of another method we can use to communicate this fix > > upstream? > > > > Best Regards > > > > Adam > > > >> Re: [OpenJDK 2D-Dev] RFR(xxxs): 8200052: libjavajpeg: Fix compile warning > >> in jchuff.cPhilip Race to: Adam Farley8 17/05/2018 03:32 > >> Cc: 2d-dev, Andrew Leonard, build-dev, Magnus Ihse Bursie, "Thomas Stüfe" > >> From: Philip Race > >> To: Adam Farley8 > >> Cc: 2d-dev <2d-...@openjdk.java.net>, Andrew Leonard > >> , build-dev , > >> Magnus Ihse Bursie , "Thomas Stüfe" > >> > >> > >> > >> Hi, > >> > >> OK .. if you can convince upstream this is worth doing, then we can accept > >> it > >> as we would not regress when updating. As I noted previously : > >> http://mail.openjdk.java.net/pipermail/2d-dev/2018-March/009086.html > >> this is still an issue in the currently being developed 9c train. > >> > >> -phil. > >> > >> On 5/14/18, 3:06 AM, Adam Farley8 wrote: > >> Hi Phil, > >> > >> Would an acceptable compromise be to deliver the source code change > >> and send the code to the upstream community, allowing them to include > >> the fix if/when they are able? > >> > >> I believe Magnus was advocating this idea as well. See below. > >> > >> Best Regards > >> > >> Adam Farley > >> > >> > Same here. I would like to have this fix in, but do not want to go > >> > over Phils head. > >> > > >> > I think Phil was the main objector, maybe he could reconsider?` > >> > > >> > Thanks, Thomas > >> > > >> > On Thu, Apr 26, 2018 at 10:39 AM, Magnus Ihse Bursie > >> > wrote: > >> > > I don't object, but it's not build code so I don't have the final say. > >> > > > >> > > /Magnus > >> > > > >> > > > >> > > On 2018-04-25 17:43, Adam Farley8 wrote: > >> > > > >> > > Hi All, > >> > > > >> > > Does anyone have an objection to pushing this tiny change in? > >> > > > >> > > It doesn't break anything, it fixes a build break on two supported > >> > > platforms, and it seems > >> > > like we never refresh the code from upstream. > >>
Re: [OpenJDK 2D-Dev] RFR(xxxs): 8200052: libjavajpeg: Fix compile warning in jchuff.c
Hi, I tried to use the IJG's contact page, but no joy. Seems broken; there a spinning icon when you hit "send", but nothing happens. http://jpegclub.org/reference/contact/ So I used a slightly older mailing list on sourceforge. The request to update their code has been sent, and I hope it will appear on the mailing list soon. https://sourceforge.net/p/libjpeg/mailman/libjpeg-devel-6x/ However, that list seems fairly idle too. Does anyone know of another method we can use to communicate this fix upstream? Best Regards Adam > Re: [OpenJDK 2D-Dev] RFR(xxxs): 8200052: libjavajpeg: Fix compile warning in jchuff.cPhilip Race to: Adam Farley8 17/05/2018 03:32 > Cc: 2d-dev, Andrew Leonard, build-dev, Magnus Ihse Bursie, "Thomas Stüfe" > From: Philip Race > To: Adam Farley8 > Cc: 2d-dev <2d-...@openjdk.java.net>, Andrew Leonard , build-dev , Magnus Ihse Bursie , "Thomas Stüfe" > > > Hi, > > OK .. if you can convince upstream this is worth doing, then we can accept it > as we would not regress when updating. As I noted previously : > http://mail.openjdk.java.net/pipermail/2d-dev/2018-March/009086.html > this is still an issue in the currently being developed 9c train. > > -phil. > > On 5/14/18, 3:06 AM, Adam Farley8 wrote: > Hi Phil, > > Would an acceptable compromise be to deliver the source code change > and send the code to the upstream community, allowing them to include > the fix if/when they are able? > > I believe Magnus was advocating this idea as well. See below. > > Best Regards > > Adam Farley > > > Same here. I would like to have this fix in, but do not want to go > > over Phils head. > > > > I think Phil was the main objector, maybe he could reconsider?` > > > > Thanks, Thomas > > > > On Thu, Apr 26, 2018 at 10:39 AM, Magnus Ihse Bursie > > wrote: > > > I don't object, but it's not build code so I don't have the final say. > > > > > > /Magnus > > > > > > > > > On 2018-04-25 17:43, Adam Farley8 wrote: > > > > > > Hi All, > > > > > > Does anyone have an objection to pushing this tiny change in? > > > > > > It doesn't break anything, it fixes a build break on two supported > > > platforms, and it seems > > > like we never refresh the code from upstream. > > > > > > - Adam > > > > > >> I also advocate the source code fix, as Make isn't meant to use the sort > > >> of logic required > > >> to properly analyse the toolchain version string. > > >> > > >> e.g. An "eq" match on 4.8.5 doesn't protect the user who is using 4.8.6, > > >> and Make doesn't > > >> seem to do substring stuff unless you mess around with shells. > > >> > > >> Plus, as people have said, it's better to solve problem x (or work around > > >> a specific > > >> instance of x) than to ignore the exception, even if the ignoring code is > > >> toolchain- > > >> specific. > > >> > > >> - Adam Farley > > >> > > >> > On 2018-03-27 18:44, Phil Race wrote: > > >> > > > >> >> As I said I prefer the make file change, since this is a change to an > > >> >> upstream library. > > >> > > > >> > Newtons fourth law: For every reviewer, there's an equal and opposite > > >> > reviewer. :) > > >> > > > >> > Here I am, advocating a source code fix. As Thomas says, the compiler > > >> > complaint seems reasonable, and disabling it might cause us to miss other > > >> > future errors. > > >> > > > >> > Why can't we push the source code fix, and also submit it upstream? > > >> > > > >> > /Magnus > > >> > > > >> > > > >> > I've looked at jpeg-9c and it still looks identical to what we have in > > >> > 6b, so this code > > >> > seems to have stood the test of time. I'm also unclear why the compiler > > >> > would > > >> > complain about that and not the one a few lines later > > >> > > > >> > > > >> > 819 while (bits[i] == 0) /* find largest codelength still in > > >> > use */ > > >> > 820 i--; > > >> > > > >> >
Re: [OpenJDK 2D-Dev] RFR(xxxs): 8200052: libjavajpeg: Fix compile warning in jchuff.c
Hi Phil, Would an acceptable compromise be to deliver the source code change and send the code to the upstream community, allowing them to include the fix if/when they are able? I believe Magnus was advocating this idea as well. See below. Best Regards Adam Farley > Same here. I would like to have this fix in, but do not want to go > over Phils head. > > I think Phil was the main objector, maybe he could reconsider?` > > Thanks, Thomas > > On Thu, Apr 26, 2018 at 10:39 AM, Magnus Ihse Bursie > wrote: > > I don't object, but it's not build code so I don't have the final say. > > > > /Magnus > > > > > > On 2018-04-25 17:43, Adam Farley8 wrote: > > > > Hi All, > > > > Does anyone have an objection to pushing this tiny change in? > > > > It doesn't break anything, it fixes a build break on two supported > > platforms, and it seems > > like we never refresh the code from upstream. > > > > - Adam > > > >> I also advocate the source code fix, as Make isn't meant to use the sort > >> of logic required > >> to properly analyse the toolchain version string. > >> > >> e.g. An "eq" match on 4.8.5 doesn't protect the user who is using 4.8.6, > >> and Make doesn't > >> seem to do substring stuff unless you mess around with shells. > >> > >> Plus, as people have said, it's better to solve problem x (or work around > >> a specific > >> instance of x) than to ignore the exception, even if the ignoring code is > >> toolchain- > >> specific. > >> > >> - Adam Farley > >> > >> > On 2018-03-27 18:44, Phil Race wrote: > >> > > >> >> As I said I prefer the make file change, since this is a change to an > >> >> upstream library. > >> > > >> > Newtons fourth law: For every reviewer, there's an equal and opposite > >> > reviewer. :) > >> > > >> > Here I am, advocating a source code fix. As Thomas says, the compiler > >> > complaint seems reasonable, and disabling it might cause us to miss other > >> > future errors. > >> > > >> > Why can't we push the source code fix, and also submit it upstream? > >> > > >> > /Magnus > >> > > >> > > >> > I've looked at jpeg-9c and it still looks identical to what we have in > >> > 6b, so this code > >> > seems to have stood the test of time. I'm also unclear why the compiler > >> > would > >> > complain about that and not the one a few lines later > >> > > >> > > >> > 819 while (bits[i] == 0) /* find largest codelength still in > >> > use */ > >> > 820 i--; > >> > > >> > A push to jchuff.c will get blown away if/when we upgrade. > >> > A tool-chain specific fix in the makefile with an appropriate comment is > >> > more targeted. > >> > >> Phil, > >> > >> Returning to this. > >> > >> While I understand your reluctance to patch upstream code, let me point > >> out that we have not updated libjpeg a single time since the JDK was open > >> sourced. We're using 6b from 27-Mar-1998. I had a look at the Wikipedia page > >> on libjpeg, and this is the latest "uncontroversial" version of the source. > >> Versions 7 and up have proprietary extensions, which in turn has resulted in > >> multiple forks of libjpeg. As it stands, it seems unlikely that we will ever > >> replace libjpeg 6b with a simple upgrade from upstream. > >> > >> I therefore maintain my position that a source code fix would be the best > >> way forward here. > >> > >> /Magnus > >> > >> > > >> > > >> > -phil. > >> > > >> > > >> > On 03/27/2018 05:44 AM, Thomas Stüfe wrote: > >> > > >> > Hi all, > >> > > >> > > >> > just a friendly reminder. I would like to push this tiny fix because > >> > tripping over this on our linux s390x builds is annoying (yes, we can > >> > maintain patch queues, but this is a constant error source). > >> > > >> > > >> > I will wait for 24 more hours until a reaction. If no serious objections > >> > are forcoming, I want to push it (tier1 tests ran thru, and me and Christoph
Re: [OpenJDK 2D-Dev] RFR(xxxs): 8200052: libjavajpeg: Fix compile warning in jchuff.c
Hi All, Does anyone have an objection to pushing this tiny change in? It doesn't break anything, it fixes a build break on two supported platforms, and it seems like we never refresh the code from upstream. - Adam > I also advocate the source code fix, as Make isn't meant to use the sort of logic required > to properly analyse the toolchain version string. > > e.g. An "eq" match on 4.8.5 doesn't protect the user who is using 4.8.6, and Make doesn't > seem to do substring stuff unless you mess around with shells. > > Plus, as people have said, it's better to solve problem x (or work around a specific > instance of x) than to ignore the exception, even if the ignoring code is toolchain- > specific. > > - Adam Farley > > > On 2018-03-27 18:44, Phil Race wrote: > > > >> As I said I prefer the make file change, since this is a change to an upstream library. > > > > Newtons fourth law: For every reviewer, there's an equal and opposite reviewer. :) > > > > Here I am, advocating a source code fix. As Thomas says, the compiler complaint seems reasonable, and disabling it might cause us to miss other future errors. > > > > Why can't we push the source code fix, and also submit it upstream? > > > > /Magnus > > > > > > I've looked at jpeg-9c and it still looks identical to what we have in 6b, so this code > > seems to have stood the test of time. I'm also unclear why the compiler would > > complain about that and not the one a few lines later > > > > > > 819 while (bits[i] == 0) /* find largest codelength still in use */ > > 820 i--; > > > > A push to jchuff.c will get blown away if/when we upgrade. > > A tool-chain specific fix in the makefile with an appropriate comment is more targeted. > > Phil, > > Returning to this. > > While I understand your reluctance to patch upstream code, let me point out that we have not updated libjpeg a single time since the JDK was open sourced. We're using 6b from 27-Mar-1998. I had a look at the Wikipedia page on libjpeg, and this is the latest "uncontroversial" version of the source. Versions 7 and up have proprietary extensions, which in turn has resulted in multiple forks of libjpeg. As it stands, it seems unlikely that we will ever replace libjpeg 6b with a simple upgrade from upstream. > > I therefore maintain my position that a source code fix would be the best way forward here. > > /Magnus > > > > > > > -phil. > > > > > > On 03/27/2018 05:44 AM, Thomas Stüfe wrote: > > > > Hi all, > > > > > > just a friendly reminder. I would like to push this tiny fix because tripping over this on our linux s390x builds is annoying (yes, we can maintain patch queues, but this is a constant error source). > > > > > > I will wait for 24 more hours until a reaction. If no serious objections are forcoming, I want to push it (tier1 tests ran thru, and me and Christoph langer are both Reviewers). > > > > > > Thanks! Thomas > > > > > > On Wed, Mar 21, 2018 at 6:20 PM, Thomas Stüfe wrote: > > > > Hi all, > > > > > > may I please have another review for this really trivial change. It fixes a gcc warning on s390 and ppc. Also, it is probably a good idea to fix this. > > > > > > bug: https://bugs.openjdk.java.net/browse/JDK-8200052 > > webrev: http://cr.openjdk.java.net/~stuefe/webrevs/8200052-fix-warning-in-jchuff.c/webrev.00/webrev/ > > > > > > This was contributed by Adam Farley at IBM. > > > > > > I already reviewed this. I also test-built on zlinux and it works. > > > > > > Thanks, Thomas > > > > 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 > > 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] RFR(xxxs): 8200052: libjavajpeg: Fix compile warning in jchuff.c
I also advocate the source code fix, as Make isn't meant to use the sort of logic required to properly analyse the toolchain version string. e.g. An "eq" match on 4.8.5 doesn't protect the user who is using 4.8.6, and Make doesn't seem to do substring stuff unless you mess around with shells. Plus, as people have said, it's better to solve problem x (or work around a specific instance of x) than to ignore the exception, even if the ignoring code is toolchain- specific. - Adam Farley > On 2018-03-27 18:44, Phil Race wrote: > >> As I said I prefer the make file change, since this is a change to an upstream library. > > Newtons fourth law: For every reviewer, there's an equal and opposite reviewer. :) > > Here I am, advocating a source code fix. As Thomas says, the compiler complaint seems reasonable, and disabling it might cause us to miss other future errors. > > Why can't we push the source code fix, and also submit it upstream? > > /Magnus > > > I've looked at jpeg-9c and it still looks identical to what we have in 6b, so this code > seems to have stood the test of time. I'm also unclear why the compiler would > complain about that and not the one a few lines later > > > 819 while (bits[i] == 0) /* find largest codelength still in use */ > 820 i--; > > A push to jchuff.c will get blown away if/when we upgrade. > A tool-chain specific fix in the makefile with an appropriate comment is more targeted. > > > -phil. > > > On 03/27/2018 05:44 AM, Thomas Stüfe wrote: > > Hi all, > > > just a friendly reminder. I would like to push this tiny fix because tripping over this on our linux s390x builds is annoying (yes, we can maintain patch queues, but this is a constant error source). > > > I will wait for 24 more hours until a reaction. If no serious objections are forcoming, I want to push it (tier1 tests ran thru, and me and Christoph langer are both Reviewers). > > > Thanks! Thomas > > > On Wed, Mar 21, 2018 at 6:20 PM, Thomas Stüfe wrote: > > Hi all, > > > may I please have another review for this really trivial change. It fixes a gcc warning on s390 and ppc. Also, it is probably a good idea to fix this. > > > bug: https://bugs.openjdk.java.net/browse/JDK-8200052 > webrev: http://cr.openjdk.java.net/~stuefe/webrevs/8200052-fix-warning-in-jchuff.c/webrev.00/webrev/ > > > This was contributed by Adam Farley at IBM. > > > I already reviewed this. I also test-built on zlinux and it works. > > > Thanks, Thomas > 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] RFR(xxxs): 8200052: libjavajpeg: Fix compile warning in jchuff.c
I also advocate the source code fix, as Make isn't meant to use the sort of logic required to properly analyse the toolchain version string. e.g. An "eq" match on 4.8.5 doesn't protect the user who is using 4.8.6, and Make doesn't seem to do substring stuff unless you mess around with shells. Plus, as people have said, it's better to solve problem x (or work around a specific instance of x) than to ignore the exception, even if the ignoring code is toolchain- specific. - Adam Farley > On 2018-03-27 18:44, Phil Race wrote: > >> As I said I prefer the make file change, since this is a change to an upstream library. > > Newtons fourth law: For every reviewer, there's an equal and opposite reviewer. :) > > Here I am, advocating a source code fix. As Thomas says, the compiler complaint seems reasonable, and disabling it might cause us to miss other future errors. > > Why can't we push the source code fix, and also submit it upstream? > > /Magnus > > > I've looked at jpeg-9c and it still looks identical to what we have in 6b, so this code > seems to have stood the test of time. I'm also unclear why the compiler would > complain about that and not the one a few lines later > > > 819 while (bits[i] == 0) /* find largest codelength still in use */ > 820 i--; > > A push to jchuff.c will get blown away if/when we upgrade. > A tool-chain specific fix in the makefile with an appropriate comment is more targeted. > > > -phil. > > > On 03/27/2018 05:44 AM, Thomas Stüfe wrote: > > Hi all, > > > just a friendly reminder. I would like to push this tiny fix because tripping over this on our linux s390x builds is annoying (yes, we can maintain patch queues, but this is a constant error source). > > > I will wait for 24 more hours until a reaction. If no serious objections are forcoming, I want to push it (tier1 tests ran thru, and me and Christoph langer are both Reviewers). > > > Thanks! Thomas > > > On Wed, Mar 21, 2018 at 6:20 PM, Thomas Stüfe wrote: > > Hi all, > > > may I please have another review for this really trivial change. It fixes a gcc warning on s390 and ppc. Also, it is probably a good idea to fix this. > > > bug: https://bugs.openjdk.java.net/browse/JDK-8200052 > webrev: http://cr.openjdk.java.net/~stuefe/webrevs/8200052-fix-warning-in-jchuff.c/webrev.00/webrev/ > > > This was contributed by Adam Farley at IBM. > > > I already reviewed this. I also test-built on zlinux and it works. > > > Thanks, Thomas > 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] RFR: Bug Pending: Build fails to compile jchuff.c
-- Summary -- I ask for a committer to go into jdk/jdk and add one word to make/lib/Awt2dLibraries.gmk We need to go to line 495 and add array-bounds into the list of disabled warnings. So this: DISABLED_WARNINGS_gcc := clobbered implicit-fallthrough shift-negative-value, \ becomes this: DISABLED_WARNINGS_gcc := clobbered implicit-fallthrough shift-negative-value array-bounds, \ This fixes a build-breaking problem which occurs if you don't disable errors-as-warnings on zLinux or Linux for ppcle. P.S. Backporting this to jdk8 would be awesome, but the priority is jdk/jdk. -- Conversation -- > Am I understanding this correctly that it's really not tied to a gcc version > but a cpu architecture, so it's only really affecting s390x? I'm saying it is tied to a combination of CPU architecture and gcc version. Any combination of the affected gcc versions (4.8.5, 5.4.0) and affected platforms (zLinux, ppcle Linux) see this error. x86 Linux is not affected, not are gcc versions equal to (or, I assume, later than) 7.2.1. > Are you also saying that gcc 7.2.1 is also affected but with a different > message? I'm fine with disabling this warning conditional on s390x, no need > for specific gcc versions. I was unclear. 7.2.1 fails my unit test with a different warning, but a build I ran proves that this warning doesn't fail the build. That being said, the addition of this option to a 7.2.1 test didn't seem to break anything, so we should be fine to just stick "DISABLED_WARNINGS_gcc := clobbered array-bounds" into Awt2dLibraries.gmk. > This discussion has already taken more time than it really warrants. :) Agreed. :) > Regarding warning chasing. I agree that we it's not feasible to chase down every > warning in every version of GCC, or any other toolchain, but I also think that > for platforms/configurations where people are actively developing changes for > OpenJDK, it makes sense to try to keep it clean. This helps prevent new code from > introducing warnings. For the configurations Oracle use, we keep a strict -Werror > policy because we want the code to be clean. I'm fine with other users trying to > keep the same standards on their configurations, but knowing that it will be their > responsibility to keep up to date. I also think we need to be reasonably fine grained > in when we disable warnings. Specifying every affected version of a toolchain is too > much, but if there are specific well defined limits to where the disabling relevant, > then I think we should use them, within reason. This also helps with keeping track > of why a particular warning is disabled in a future attempt to fix them. I agree with all of this. Well put. :) > On the other hand, if you are just building OpenJDK to produce binaries, without > producing and up streaming new code changes, there really isn't much need for > making the effort of trying to keep things clean, and trying to do so will likely > just end up being more work than it's worth. > /Erik I'm building OpenJDK to test fixes and new features, which I will eventually contribute to OpenJDK. I consider this to be one of those fixes. One fix at a time. :) Given all of this, I ask for a volunteer to raise a bug so we can integrate this change into JDK8 (as it's still very popular), and JDK. 10 would be great too, though I understand it's locked against all but the worst bugs. 9 is optional, as it's soon to be replaced by 10. 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] RFR: Bug Pending: Build fails to compile jchuff.c
Re: [OpenJDK 2D-Dev] RFR: Bug Pending: Build fails to compile jchuff.cErik Joelsson to: Adam Farley8 01/02/2018 17:06 Cc: build-dev, David Holmes, John Paul Adrian Glaubitz, Magnus Ihse Bursie From: Erik Joelsson To: Adam Farley8 Cc: build-dev , David Holmes , John Paul Adrian Glaubitz , Magnus Ihse Bursie > Am I understanding this correctly that it's really not tied to a gcc version > but a cpu architecture, so it's only really affecting s390x? I'm saying it is tied to a combination of CPU architecture and gcc version. Any combination of the affected gcc versions (4.8.5, 5.4.0) and affected platforms (zLinux, ppcle Linux) see this error. x86 Linux is not affected, not are gcc versions equal to (or, I assume, later than) 7.2.1. > Are you also saying that gcc 7.2.1 is also affected but with a different > message? I'm fine with disabling this warning conditional on s390x, no need > for specific gcc versions. I was unclear. 7.2.1 fails my unit test with a different warning, but a build I ran proves that this warning doesn't fail the build. That being said, the addition of this option to a 7.2.1 test didn't seem to break anything, so we should be fine to just stick "DISABLED_WARNINGS_gcc := clobbered array-bounds" into Awt2dLibraries.gmk. > This discussion has already taken more time than it really warrants. :) Agreed. :) > Regarding warning chasing. I agree that we it's not feasible to chase down every > warning in every version of GCC, or any other toolchain, but I also think that > for platforms/configurations where people are actively developing changes for > OpenJDK, it makes sense to try to keep it clean. This helps prevent new code from > introducing warnings. For the configurations Oracle use, we keep a strict -Werror > policy because we want the code to be clean. I'm fine with other users trying to > keep the same standards on their configurations, but knowing that it will be their > responsibility to keep up to date. I also think we need to be reasonably fine grained > in when we disable warnings. Specifying every affected version of a toolchain is too > much, but if there are specific well defined limits to where the disabling relevant, > then I think we should use them, within reason. This also helps with keeping track > of why a particular warning is disabled in a future attempt to fix them. I agree with all of this. Well put. :) > On the other hand, if you are just building OpenJDK to produce binaries, without > producing and up streaming new code changes, there really isn't much need for > making the effort of trying to keep things clean, and trying to do so will likely > just end up being more work than it's worth. > /Erik I'm building OpenJDK to test fixes and new features, which I will eventually contribute to OpenJDK. I consider this to be one of those fixes. One fix at a time. :) Given all of this, I ask for a volunteer to raise a bug so we can integrate this change into JDK8 (as it's still very popular), and JDK. 10 would be great too, though I understand it's locked against all but the worst bugs. 9 is optional, as it's soon to be replaced by 10. 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] RFR: Bug Pending: Build fails to compile jchuff.c
> On 02/01/2018 12:36 PM, Adam Farley8 wrote: >> After doing some experimenting, I've discovered the problem isn't limited to >> SLES, or gcc 4.8.5, or zLinux. >> >> (...) >> Thoughts? > I think the problem is more that if you are trying to silence warnings, > you will end up being busy for a very long time since there are simply > too many possible configurations out there [1]. Unfortunately it's not just a warning, it's a build-killer. Across those platforms, in those configurations, this is the only warning which causes the build to fail. Adding array-bounds to DISABLED_WARNINGS_gcc suppresses this warning-turned-error, and the builds then run and pass. > > For this reason, both Debian and openSUSE/SLE [2] build OpenJDK with the > configure parameter "--disable-warnings-as-errors". --disable-warnings-as-errors (in the configure step) can work around this problem, but then we need to add it to the relevant documentation, and it complicates the build process a little bit more from the user's POV. In my world, "bash ./configure" should be enough. I like to keep things simple. :) - Adam > > Adrian > >> [1] https://urldefense.proofpoint.com/v2/url?u=http-3A__mail.openjdk.java.net_pipermail_hotspot-2Ddev_2018-2DJanuary_029754.html&d=DwICaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=P5m8KWUXJf-CeVJc0hDGD9AQ2LkcXDC0PMV9ntVw5Ho&m=DL2no3G3wg34U0Tl9cbhAcWOQVVU4mv-0EiiF2QNh5U&s=0uzfk9CUqR2GtWgwtbQFBs7zFHnAs6hwM-LGYDMdeHs&e= >> [2] https://urldefense.proofpoint.com/v2/url?u=https-3A__build.opensuse.org_package_view-5Ffile_openSUSE-3AFactory_java-2D10-2Dopenjdk_java-2D10-2Dopenjdk.spec-3Fexpand-3D1&d=DwICaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=P5m8KWUXJf-CeVJc0hDGD9AQ2LkcXDC0PMV9ntVw5Ho&m=DL2no3G3wg34U0Tl9cbhAcWOQVVU4mv-0EiiF2QNh5U&s=9NoPxdwrV4dfjgB4SpDpLgLApTLCTURy6xwVUU5LS6k&e= > 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] RFR: Bug Pending: Build fails to compile jchuff.c
Hi All, After doing some experimenting, I've discovered the problem isn't limited to SLES, or gcc 4.8.5, or zLinux. Platforms affected: zLinux, ppcle Linux Platforms not affected: x86 Linux gcc versions affected: 4.8.5, 5.4.0 gcc versions unaffected: 7.2.1 linux variations affected: Ubuntu, SLES E.g. when I told SLES 12 (zLinux) to install gcc, it installed 5.4.0. All this tells me that, if we intent to encourage SLES 12 users to build Java 11, we need this warning suppression hard coded. There are ways to do "greater than"s and "first character in"s in make, but they require either shell commands, or more dollar signs than you can shake a stick at. I propose we hard code "array-bounds" into javajpeg's DISABLED_WARNINGS_gcc for all java versions, and move on. On the basis that gcc 7.2.1 produces a different error message when an array-bounds problem occurs (e.g. frog[-1] produces the warning "error: ‘frog[-7]’ is used uninitialized in this function"), I don't see leaving array-bounds in DISABLED_WARNINGS_gcc as a problem. Thoughts? Best Regards Adam Farley > The claim on the wiki was made by Volker on July 13 2017. It was done on > SuSE linux while the reported problem is on zLinux. Maybe they have > different vendor specific patches in their respective gcc builds? Is SAP > still able to build without --disable-warnings-as-errors? This wiki is > supposed to be a community effort. > > /Erik > > >On 2018-01-24 16:37, David Holmes wrote: >> Hi Magnus, >> >> On 25/01/2018 9:55 AM, Magnus Ihse Bursie wrote: >>> I'm not sure I'm able to follow where this discussion is heading, but >>> at the very least I'd like to chime in on the patch below: >>> >>> I don't like how the entire DISABLED_WARNINGS_gcc line is lifted out. >>> A better solution is something like this: >>> >>> ifeq ($(CC_VERSION_NUMBER), 4.8.5) >>> #Turn off array-bounds warnings for libjava compilation because >>> jchuff.c >>> #fails to compile on gcc 4.8.5 with a warning that an array is >>> potentially >>> #out of bounds. This kills a warnings=errors build. >>> LIBJAVAJPEG_DISABLED_WARNINGS_gcc485 := array-bounds >>> endif >>> ... >>> DISABLED_WARNINGS_gcc := clobbered >>> $(LIBJAVAJPEG_DISABLED_WARNINGS_gcc485), \ >>> >>> I also can't see this going into anything else than the current >>> mainline, jdk/jdk i.e. JDK 11. Since the issue is trivially avoided >>> by using --disable-warnings-as-errors, I hardly see that it warrants >>> a backport. >> >> It's pointless in JDK 11 as we're not going to claim support for gcc >> 4.8.5. >> >> The issue is that out-of-the-box if you are building 8u or 9 and use >> gcc 4.8.5, which is supposed to be a valid compiler to use based on >> the wiki, then it fails because of this gcc bug, unless you disable >> warnings-as-errors. So either we put in a workaround for the bug or we >> update to the build docs (and/or the wiki) to make it very clear that >> you may need to disable warnings-as-errors (something there is no >> mention of in the build doc in 9). >> >> David >> - >> >>> /Magnus >>> >>> On 2018-01-23 14:44, Adam Farley8 wrote: >>>> Hi All, >>>> >>>> All: I think I responded to everyone below. Please could a committer or >>>> author raise a bug and, if people are happy with this change, line >>>> it up >>>> for contribution to JDKs 8-11 (assuming 4.8.5 is still the >>>> recommended gcc >>>> for JDK10 and 11 on SUSE sles)? >>>> >>>> Erik: One toolchain-specific change, as requested. This was tested >>>> on JDK9, >>>> so please apply an offset of -1 to the line numbers before applying >>>> to JDK10. >>>> >>>> --- Start of Diff --- >>>> >>>> diff --git a/make/lib/Awt2dLibraries.gmk b/make/lib/Awt2dLibraries.gmk >>>> --- a/make/lib/Awt2dLibraries.gmk >>>> +++ b/make/lib/Awt2dLibraries.gmk >>>> @@ -482,6 +482,14 @@ >>>>BUILD_LIBJAVAJPEG_HEADERS := $(addprefix -I, $(LIBJAVAJPEG_SRC)) >>>> endif >>>> >>>> +LIBJAVAJPEG_DISABLED_WARNINGS_gcc := clobbered >>>> +ifeq ($(CC_VERSION_NUMBER), 4.8.5) >>>> + #Turn off array-bounds warnings for libjava compilation because >>>> jchuff.c >>>> + #fails to compile on gcc 4.
Re: RFR: Bug Pending: Build fails to compile jchuff.c
>On 01/23/2018 05:25 PM, Adam Farley8 wrote: >>> SLE-11:* doesn't even have OpenJDK-8 and is also going to be out of support >>> next year anyway. >> >> Does this mean the gcc version will change? If you have hard information on >> this, I'd appreciate the URL. > >I'm not sure what you mean. SLE12-SP3 ships gcc-4.8.x while SLE-15 will >ship gcc-7, see: > >> https://urldefense.proofpoint.com/v2/url?u=https-3A__build.opensuse.org_package_view-5Ffile_SUSE-3ASLE-2D15-3AGA_gcc_gcc.spec-3Fexpand-3D1&d=DwIC-g&c=jf_iaSHvJObTbx-siA1ZOg&r=P5m8KWUXJf-CeVJc0hDGD9AQ2LkcXDC0PMV9ntVw5Ho&m=dIGHRmVpTLUCdNXpk5OeZoRTr4KMZfiyFp7leAxQ1x4&s=kvSfKGn4zfKUDx14bZlDZsWrY3uorXE_6lBuTmOMchw&e= > >Is that what you mean when you say the gcc version is changing? Apologies, I was unclear. I was asking if the minimum gcc version on David's website was likely to change when SLE11 went out of service. From what you're telling me, the sles 11 bit on the site will likely be updated to sles 12, and the gcc version won't change (as you're saying SLE12 ships with 4.8.x). > >> If the minimum gcc version for 10 or 11 is above 4.8.5 across all platforms, >> then I agree, but I don't have that information, so I figured I'd ask to >> cover all of the JDK versions, to be safe. > >I don't know what the minimum version is at the moment, to be honest. I haven't >tried building OpenJDK-10 or OpenJDK-11 on SLE-12:SP3 yet. I could do that >if that's important. > >> Even if the gcc version does change, adding 4.8.5-specific code shouldn't >> break anything. > >It most likely doesn't break anything. But it leaves workaround in the code >base which we could potentially forget to clean out later when it is no >longer needed. Agreed. I was hedging my bets on the gcc version not changing. Be good if we had some reliable intel on the minimum gcc version that we could use to make a decision. > >> What do you think? > >My opinion is that the codebase for OpenJDK-11 should be kept clean because >we are working on getting rid of unnecessary cruft. But this decision isn't >up to me, of course. I'm just arguing that I consider the chances that someone >will try OpenJDK-11 on SLE-12:SP3 or even SLE-11:SP4 very low. > >Adrian A reasonable opinion. I may disagree with your conclusions, but you present your arguments well. Could others on this email chain act as tie breaker on the jdk10+11 matter please? 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: Bug Pending: Build fails to compile jchuff.c
> On 01/23/2018 03:13 PM, Adam Farley8 wrote: >> The URL supplied by David (Holmes) lists 4.8.5 as the gcc version for >> building JDK9 on SLES 11.3/12.1. Whether it's in a repository or not, >> I read that as "this is the gcc version you should be building JDK9 on". >> >> https://urldefense.proofpoint.com/v2/url?u=https-3A__wiki.openjdk.java.net_display_Build_Supported-2BBuild-2BPlatforms&d=DwIC-g&c=jf_iaSHvJObTbx-siA1ZOg&r=P5m8KWUXJf-CeVJc0hDGD9AQ2LkcXDC0PMV9ntVw5Ho&m=63AR5zmRQCx5v-MXloqnarJvGvjLXVVnJsui6XMOMoE&s=bDVDenH2BOpYeVaU504vQiL0lqdBlELIy_wWpX44Juw&e= >> >> I asked for this change to be put in 10 and 11 because I don't know the >> minimum/recommended gcc version for jdk10 and 11. I'm assuming it's >> the same as 9 until told otherwise. > > Building OpenJDK-10 or OpenJDK-11 requires OpenJDK-9, so you won't be able > to build them in a fresh SLE-12:SP3 installation anyway. I'm working on the assumption that people will grab binaries from AdoptOpenJDK. > > SLE-11:* doesn't even have OpenJDK-8 and is also going to be out of support > next year anyway. Does this mean the gcc version will change? If you have hard information on this, I'd appreciate the URL. > > I don't necessarily disagree bringing your changes into JDK-8 or JDK-9, > but I don't think they make sense to introduce them to JDK-10 and JDK-11. > > Adrian If the minimum gcc version for 10 or 11 is above 4.8.5 across all platforms, then I agree, but I don't have that information, so I figured I'd ask to cover all of the JDK versions, to be safe. Even if the gcc version does change, adding 4.8.5-specific code shouldn't break anything. What do you think? 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: Bug Pending: Build fails to compile jchuff.c
> On 01/23/2018 02:44 PM, Adam Farley8 wrote: >> John: I read your email, and I understand your position. I disagree with it, >> but I understand it. 4.8.5 is an old version of gcc, but right now it is the >> listed gcc version for SUSE sles on intel, ppc, ppcle, and zLinux. Even if >> this is not the case for JDK 10 or 11, we should ensure this fix is fully >> propagated to ensure consistent behaviour. >> >> That is my position. > I happen to work at SUSE. And while you're right that SLE-12:GA is on gcc > 4.8.x, SLE-12 doesn't have openjdk-9 or openjdk-10 or even -11 in its > update repositories (looking at the internal SUSE build system at the moment). > > openjdk-9 is part of SLE-15:GA which itself ships gcc-7. > > So, what exactly is the usecase for OpenJDK-9, 10 and 11? > > Adrian Hi Adrian, The URL supplied by David (Holmes) lists 4.8.5 as the gcc version for building JDK9 on SLES 11.3/12.1. Whether it's in a repository or not, I read that as "this is the gcc version you should be building JDK9 on". https://wiki.openjdk.java.net/display/Build/Supported+Build+Platforms I asked for this change to be put in 10 and 11 because I don't know the minimum/recommended gcc version for jdk10 and 11. I'm assuming it's the same as 9 until told otherwise. 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
RFR: Bug Pending: Build fails to compile jchuff.c
Hi All, All: I think I responded to everyone below. Please could a committer or author raise a bug and, if people are happy with this change, line it up for contribution to JDKs 8-11 (assuming 4.8.5 is still the recommended gcc for JDK10 and 11 on SUSE sles)? Erik: One toolchain-specific change, as requested. This was tested on JDK9, so please apply an offset of -1 to the line numbers before applying to JDK10. --- Start of Diff --- diff --git a/make/lib/Awt2dLibraries.gmk b/make/lib/Awt2dLibraries.gmk --- a/make/lib/Awt2dLibraries.gmk +++ b/make/lib/Awt2dLibraries.gmk @@ -482,6 +482,14 @@ BUILD_LIBJAVAJPEG_HEADERS := $(addprefix -I, $(LIBJAVAJPEG_SRC)) endif +LIBJAVAJPEG_DISABLED_WARNINGS_gcc := clobbered +ifeq ($(CC_VERSION_NUMBER), 4.8.5) + #Turn off array-bounds warnings for libjava compilation because jchuff.c + #fails to compile on gcc 4.8.5 with a warning that an array is potentially + #out of bounds. This kills a warnings=errors build. + LIBJAVAJPEG_DISABLED_WARNINGS_gcc := clobbered array-bounds +endif + $(eval $(call SetupNativeCompilation,BUILD_LIBJAVAJPEG, \ LIBRARY := javajpeg, \ OUTPUT_DIR := $(INSTALL_LIBRARIES_HERE), \ @@ -491,7 +499,7 @@ CFLAGS := $(CFLAGS_JDKLIB) $(BUILD_LIBJAVAJPEG_HEADERS) \ $(LIBJAVA_HEADER_FLAGS) \ -I$(SUPPORT_OUTPUTDIR)/headers/java.desktop, \ -DISABLED_WARNINGS_gcc := clobbered, \ +DISABLED_WARNINGS_gcc := $(LIBJAVAJPEG_DISABLED_WARNINGS_gcc), \ MAPFILE := $(JDK_TOPDIR)/make/mapfiles/libjpeg/mapfile-vers, \ LDFLAGS := $(LDFLAGS_JDKLIB) \ $(call SET_SHARED_LIBRARY_ORIGIN), \ --- End of Diff --- Phil: I've changed the title as asked and supplied the diff above. However, I can't upload files to cr.openjdk.java.net, nor can I create bugs myself. I understand that authors and committers can, but I don't have those privileges yet. Be nice if I did. :) John: I read your email, and I understand your position. I disagree with it, but I understand it. 4.8.5 is an old version of gcc, but right now it is the listed gcc version for SUSE sles on intel, ppc, ppcle, and zLinux. Even if this is not the case for JDK 10 or 11, we should ensure this fix is fully propagated to ensure consistent behaviour. That is my position. David: Thanks for the URL. I agree with your position on 4.8.5 gcc needing to compile OpenJDK without errors or special options. :) Best Regards Adam Farley From: Phil Race To: Adam Farley8 , 2d-...@openjdk.java.net Cc: build-dev Date: 18/01/2018 19:16 Subject:Re: [OpenJDK 2D-Dev] [PATCH] Build fails to compile jchuff.c 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 Unless stated otherwise above: IBM United Kingdom Limited - Registered in
Re: [PATCH] (Title Corrected) Build fails to compile jchuff.c using gcc 4.8.5 on zLinux
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
> 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
Re: [PATCH] (Title Corrected) Build fails to compile jchuff.c using gcc 4.8.5 on zLinux
>> Switching the OPTIMIZATION to LOW will solve this at a stroke. > >And regress performance for all platforms I expect in a case where >performance matters .. >in order to work around a gcc bug ? I don't think so. I wasn't considering the performance impact on java jpeg. A fair statement. >Disabling the specific error with the specific tool chain is the only >acceptable option I can think of. I'll look into that. >And bear in mind build-dev is not the keeper of the JPEG libraries. >2d-dev is the right place. > >-phil. Also fair. I will send an email to 2d-dev with the same title. Figured I'd start here because it breaks all builds that don't use disable-warnings-as-errors. Thank you everyone for your time. Feel free to follow these adventures on 2d-dev. :) 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] Freetype Directory Bug On zLinux
Hi All, In JDK9 on zLinux 64bit, it seems we don't look for libfreetype.so in /usr/lib/$OPENJDK_TARGET_CPU-linux-gnu by default. If you DO have pkg-config, "configure" searches for freetype in several places, including a place relative to gcc, (gcc/../../etc) where it uses the correct folder name and finds libfreetype.so. If you DON'T have pkg-config, it searches a few places, gets desperate, and ends up in a 64bit-only if statement (where it assumes it's running on intel linux and fails trying to check x86_64-linux-gnu). As far as I can tell, this is a build bug, and can be fixed in one of three ways: 1) We add the aforementioned IF statement, effectively adding /usr/lib/s390x-linux-gnu to the list of places configure looks for libfreetype.so 2) We add something into the configure files that checks for the presence of pkg-config before we look for freetype, if only on zlinux. 3) We backport the jdk10 solution, or a simplified version of it. Note that we may need to fix the 64bit-only if statement anyway, as there are many 64-bit locations that are not 64bit linux. 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: [PATCH] (Title Corrected) Build fails to compile jchuff.c using gcc 4.8.5 on zLinux
>> If this is the consensus, then perhaps we should consider setting >> --disable-warnings-as-errors by default (in the code), rather than >> depending on the user using an option which is not part of the formal >> build instructions. >I'm not sure why. Because the default build instructions don't work in this scenario, and if all the effort to impliment a clone-config-make model was intended to encourage more users to attempt a local build (in order to try their hand at a fixing a bug themselves or something) it makes sense to me to try to maintain a scenario where OpenJDK can build to completion across a wide variety of toolchains. >Building OpenJDK from source isn't exactly something >that is done by normal users. If someone is willing to hack on the OpenJDK >code base, I would assume they know about -Werror and similar options and >how to control them. I don't agree. Someone should not have to be familiar with gcc options in order to fix a typo, or change some Java code. And besides, we have a clear and simple four-step build process (clone, get source, configure, make). Why would we want people to have to fail their build and experiment with different options, when we can fix the problem right here and now. >I mean, yes, you can change that to have -Werror turned off by default, >but having the compiler complain less is usually a bad idea. In general, yes. In this one compile it's breaking the build. David suggested disabling this warning. The simplest way I see to do this is to change Awt2dLibraries.gmk. The code is here: $(eval $(call SetupNativeCompilation,BUILD_LIBJAVAJPEG, \ LIBRARY := javajpeg, \ OUTPUT_DIR := $(INSTALL_LIBRARIES_HERE), \ SRC := $(LIBJAVAJPEG_SRC), \ INCLUDE_FILES := $(BUILD_LIBJAVAJPEG_INCLUDE_FILES), \ OPTIMIZATION := HIGHEST, \ Switching the OPTIMIZATION to LOW will solve this at a stroke. 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] (Title Corrected) Build fails to compile jchuff.c using gcc 4.8.5 on zLinux
Hi John, David, >> If you compile jchuff.c (part of javajpeg) without >> "--disable-warnings-as-errors", >> then you get an error that kills the build. This is seen in these >> circumstances: >Last time this particular discussion came up, the conclusion was that >hunting for warnings is a lost battle as the generated warnings depend >heavily on the toolchain used [1,2]. >So, I think for now we're not going to address build errors which occur >when omitting "--disable-warnings-as-errors" in the configure line. If this is the consensus, then perhaps we should consider setting --disable-warnings-as-errors by default (in the code), rather than depending on the user using an option which is not part of the formal build instructions. 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
[PATCH] (Title Corrected) Build fails to compile jchuff.c using gcc 4.8.5 on zLinux
Hi All, If you compile jchuff.c (part of javajpeg) without "--disable-warnings-as-errors", then you get an error that kills the build. This is seen in these circumstances: Build: JDK9 gcc and g++ Version: 4.8.5 Platform: zLinux 64bit (s390x) The error message 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) ^ It looks to me that this error happens because the while loop can technically reduce j down to beneath 0, resulting in us attempting to find the array entry with index -1. On the basis that if we get down to -1 here bad things will happen regardless, perhaps we should change that line to: while ((bits[j] == 0) && (j != 0)) This appears to prevent the compiler failing with this error, by providing unambiguous handling for the "index -1" scenario. 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
[PATCH] Build fails to compile jchuff.c using gcc 4.5 on zLinux
Hi All, If you compile jchuff.c (part of javajpeg) without "--disable-warnings-as-errors", then you get an error that kills the build. This is seen in these circumstances: Build: JDK9 gcc and g++ Version: 4.8.5 Platform: zLinux 64bit (s390x) The error message 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) ^ It looks to me that this error happens because the while loop can technically reduce j down to beneath 0, resulting in us attempting to find the array entry with index -1. On the basis that if we get down to -1 here bad things will happen regardless, perhaps we should change that line to: while ((bits[j] == 0) && (j != 0)) This appears to prevent the compiler failing with this error, by providing unambiguous handling for the "index -1" scenario. 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: [PATCH] Freetype Directory Bug On zLinux
>On 2018-01-16 18:54, Erik Joelsson wrote: >> >> >> On 2018-01-16 09:50, Erik Joelsson wrote: >>> >>> >>> On 2018-01-16 09:03, Adam Farley8 wrote: >>>> >Configure already looks in: >>>> >>>> >$SYSROOT/usr/lib/$OPENJDK_TARGET_CPU-linux-gnu >>>> >>>> >Which I would expect to cover your case, unless there is a mismatch >>>> >between s390 and s390x here. Is your OPENJDK_TARGET_CPU set to s390 or >>>> >s390x in this case? If this discrepancy between arch names cannot be >>>> >resolved, then a special case like the one you propose is needed. >>>> >>>> I have tried and tried, and cannot find this code line in the JDK, >>>> anywhere. Nor can I find variations on it. The closest I can find is a >>>> list of possible locations, but these are hard-coded (and don't have >>>> OPENJDK_TARGET_CPU in their code), and none are s390 or s390x. >>>> >>>> Please spell out exactly where this line is, as I have no idea. >>>> >>> I'm referring to this block: >>> >>> if test "x$FOUND_FREETYPE" != xyes; then >>> FREETYPE_BASE_DIR="$SYSROOT/usr" >>> if test "x$OPENJDK_TARGET_CPU_BITS" = x64; then >>> LIB_CHECK_POTENTIAL_FREETYPE([$FREETYPE_BASE_DIR/include], >>> [$FREETYPE_BASE_DIR/lib/$OPENJDK_TARGET_CPU-linux-gnu], [well-known >>> location]) >>> else >>> LIB_CHECK_POTENTIAL_FREETYPE([$FREETYPE_BASE_DIR/include], >>> [$FREETYPE_BASE_DIR/lib/i386-linux-gnu], [well-known location]) >>> if test "x$FOUND_FREETYPE" != xyes; then >>> LIB_CHECK_POTENTIAL_FREETYPE([$FREETYPE_BASE_DIR/include], >>> [$FREETYPE_BASE_DIR/lib32], [well-known location]) >>> fi >>> fi >>> fi I'm embarassed that it took me this long to figure out you were referring to the jdk10 code. I was scouring my copy of the jdk9 source and wondering what on earth you were talking about. Whoops. :) >>> >>> Reading it again I realize that the directory is only searched for >>> 64bit builds. Looking in platforms.m4, I see that s390 is 32bit and >>> s390x is 64bit. This means it will work fine for s390x, but not for >>> s390. Given that your libraries are found in a directory with s390x >>> in the name, I assume that you actually do want to produce a 64bit >>> build, or am I missing something? If you need this to work for 32bit, >>> then it's possible we need to tweak something. I think it wouldn't be >>> such a bad idea to always search >>> $SYSROOT/usr/lib/$OPENJDK_TARGET_CPU-linux-gnu, regardless of >>> architecture. >>> >> Are the libraries in your s390x directory 32bit or 64bit? Am I correct >> in assuming that the x suffix is signifying 64bit instead of 32bit? >Yes, s390x means 64-bit. We do not support 32-bit s390, so I don't think >it's likely that Adam was trying to build that. > >Just like we sometimes just say "sparc" when we really mean "sparcv9", I >presume that "s390" here really mean "s390x". > >I'm guessing that the config.guess script (and our wrapper) perhaps is >not 100% correct in determining s390x. It has certainly not seen much >testing. For some reason my code now works perfectly. I am 100% certain that this defect was occuring before I installed a selection of packages on the machine, so I figure one of these, or something else I did, resulted in the correct s390x being passed. I'll try to figure out what has happened. P.S. I'm trying to build s390x (64bit) on a 64bit machine. I don't know where the code was getting s390. > >/Magnus > >> >> /Erik >>> Also note that your proposed code checked for >>> OPENJDK_TARGET_CPU_ARCH=s390, that's a different variable than >>> OPENJDK_TARGET_CPU. The arch in our model is more of a family of >>> cpus, ignoring things like address width. >>> >>> /Erik >>> >>>> Thanks everyone for your time. We'll get to the bottom of this! :) >>>> >>>> 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] Freetype Directory Bug On zLinux
Hi All, > >On Mon, 2018-01-15 at 20:21 +0100, John Paul Adrian Glaubitz wrote: >> Hi Adam! >> >> On 01/15/2018 06:15 PM, Adam Farley8 wrote: >> > > Which I would expect to cover your case, unless there is a >> > > mismatch >> > > between s390 and s390x here. Is your OPENJDK_TARGET_CPU set to >> > > s390 or >> > > s390x in this case? If this discrepancy between arch names cannot >> > > be >> > > resolved, then a special case like the one you propose is needed. >> > >> > My IF statement checks if OPENJDK_TARGET_CPU = s390. I have to >> > assume that >> > means OPENJDK_TARGET_CPU is set to s390. Since the folder is s390x- >> > linux-gnu, >> > it's likely the extra x causing the problem. >> >> I have just done a fresh clone of OpenJDK-11 from jdk/hs and >> performed test >> builds on Debian/s390x with the following configure lines: >> >> Server: >> >> CONF=linux-s390x-normal-server-release make clean ; CONF=linux-s390x- >> normal-server-release MAKE_VERBOSE=y QUIETLY= LOG=debug sh >> ./configure >> --with-jvm-variants=server --with-boot-jdk=/usr/lib/jvm/java-9- >> openjdk-s390x/ --disable-precompiled-headers --disable-warnings-as- >> errors && make JOBS=32 >> MAKE_VERBOSE=y QUIETLY= LOG=debug CONF=linux-s390x-normal-server- >> release >> >> Zero: >> >> CONF=linux-s390x-normal-zero-release MAKE_VERBOSE=y QUIETLY= >> LOG=debug make clean CONF=linux-s390x-normal-zero-release ; sh >> ./configure --with-jvm-variants=zero >> --with-boot-jdk=/usr/lib/jvm/java-9-openjdk-s390x/ --disable- >> precompiled-headers --disable-warnings-as-errors && make JOBS=32 >> MAKE_VERBOSE=y QUIETLY= LOG=debug >> CONF=linux-s390x-normal-zero-release >> >> Both builds succeed without any issues, so I'm not sure there isn't >> something >> wrong with your build environment. For reference, Debian/s390x has >> the freetype >> runtime and development library components in /usr/lib/s390x-linux- >> gnu: >> >> (sid_s390x-dchroot)glaubitz@zelenka:~/openjdk/hs$ dpkg -L >> libfreetype6:s390x |grep s390 && dpkg -L libfreetype6-dev:s390x |grep >> s390 >> /usr/lib/s390x-linux-gnu >> /usr/lib/s390x-linux-gnu/libfreetype.so.6.15.0 >> /usr/lib/s390x-linux-gnu/libfreetype.so.6 >> /usr/lib/s390x-linux-gnu >> /usr/lib/s390x-linux-gnu/libfreetype.a >> /usr/lib/s390x-linux-gnu/libfreetype.la >> /usr/lib/s390x-linux-gnu/pkgconfig >> /usr/lib/s390x-linux-gnu/pkgconfig/freetype2.pc >> /usr/lib/s390x-linux-gnu/libfreetype.so >> (sid_s390x-dchroot)glaubitz@zelenka:~/openjdk/hs$ >> >> Can you post your configure lines? bash ./configure Though now the problem has suddenly vanished. I don't know why. :( Will continue to poke at this. > >FYI: > >Adam mentioned in another thread[1] that --disable-warnings-as-errors >to configure makes the error go away. > >Thanks, >Severin > And thank you for mentioning that. :) >Hello Adam, >Configure already looks in: >$SYSROOT/usr/lib/$OPENJDK_TARGET_CPU-linux-gnu >Which I would expect to cover your case, unless there is a mismatch >between s390 and s390x here. Is your OPENJDK_TARGET_CPU set to s390 or >s390x in this case? If this discrepancy between arch names cannot be >resolved, then a special case like the one you propose is needed. >/Erik I have tried and tried, and cannot find this code line in the JDK, anywhere. Nor can I find variations on it. The closest I can find is a list of possible locations, but these are hard-coded (and don't have OPENJDK_TARGET_CPU in their code), and none are s390 or s390x. Please spell out exactly where this line is, as I have no idea. Thanks everyone for your time. We'll get to the bottom of this! :) 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
[PATCH] Freetype Directory Bug On zLinux
Hi All, On zLinux, freetype's .so file is typically installed in /usr/lib/s390x-linux-gnu, however the generated configure script doesn't look for it there. This causes configure to fail. I know you can avoid that with options, but I think a fix would be better. If we add this code to lib-freetype.m4 (line 365) and re-run autogen.sh, this fixes the problem. if test "x$FOUND_FREETYPE" != xyes; then FREETYPE_BASE_DIR="$SYSROOT/usr" if test "x$OPENJDK_TARGET_CPU_ARCH" = xs390; then LIB_CHECK_POTENTIAL_FREETYPE([$FREETYPE_BASE_DIR/include], [$FREETYPE_BASE_DIR/lib/s390x-linux-gnu], [well-known location]) fi fi 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: [PATCH] Freetype Directory Bug On zLinux
Hi John, Erik, -- Erik's comments -- >Configure already looks in: >$SYSROOT/usr/lib/$OPENJDK_TARGET_CPU-linux-gnu >Which I would expect to cover your case, unless there is a mismatch >between s390 and s390x here. Is your OPENJDK_TARGET_CPU set to s390 or >s390x in this case? If this discrepancy between arch names cannot be >resolved, then a special case like the one you propose is needed. My IF statement checks if OPENJDK_TARGET_CPU = s390. I have to assume that means OPENJDK_TARGET_CPU is set to s390. Since the folder is s390x-linux-gnu, it's likely the extra x causing the problem. As detailed below, s390x and s390 are both valid OPENJDK_TARGET_CPU values, according to the default generated_configure.sh file, so I think we need to accomodate both if we want to continue supporting them. This appears the most straight-forward way to do that. What do you think? -- John's comments -- >> Which I would expect to cover your case, unless there is a mismatch >>between s390 and s390x here. Is your OPENJDK_TARGET_CPU set to s390 or >>s390x in this case? If this discrepancy between arch names cannot be resolved, >>then a special case like the one you propose is needed. > From Adam's last mail it seems there is something wrong with his build >environment: Not as far as I can tell. In generated_configure.sh, both s390 and s390x are listed as valid OPENJDK_TARGET_CPU values. I figure, if some code is looking for $SYSROOT/usr/lib/$OPENJDK_TARGET_CPU-linux-gnu as Erik suggests, it's the fact that the $OPENJDK_TARGET_CPU value is s390, but the folder name is s390x-linux-gnu (note the extra x) that's causing this. >> From the configure output, it appears to look for it in x86_64-linux-gnu >> so I don't know what to tell you. >So, we need to figure out first the configure is looking in x86_64-linux-gnu >on the s390x environment. There's a bunch of IF statements in lib-freetype.m4 that appear to check platform-specific freetype locations sequentially, regardless of whether we have any evidence that we're on that platform. Hence my addition to that list of IF statements. Best Regards Adam Farley From: John Paul Adrian Glaubitz To: Erik Joelsson , Adam Farley8 , build-dev@openjdk.java.net Date: 12/01/2018 17:29 Subject:Re: [PATCH] Freetype Directory Bug On zLinux On 01/12/2018 06:03 PM, Erik Joelsson wrote: > Which I would expect to cover your case, unless there is a mismatch between s390 and s390x here. Is your OPENJDK_TARGET_CPU set to s390 or s390x in this case? If this discrepancy between arch names cannot be resolved, then a special case like the one you propose is needed. From Adam's last mail it seems there is something wrong with his build environment: > From the configure output, it appears to look for it in x86_64-linux-gnu > so I don't know what to tell you. So, we need to figure out first the configure is looking in x86_64-linux-gnu on the s390x environment. 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 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] Freetype Directory Bug On zLinux
On 01/12/2018 03:29 PM, Adam Farley8 wrote:
>> On zLinux, freetype's .so file is typically installed in
>> /usr/lib/s390x-linux-gnu, however the generated configure script
doesn't
>> look for it there.
>
>Odd. Normally I would expect it to look in the locations that are
>set through /etc/ld.so.conf{,.d}
>From the configure output, it appears to look for it in x86_64-linux-gnu
so I don't know what to tell you.
>
>> This causes configure to fail. I know you can avoid that with options,
but
>> I think a fix would be better.
>>
>> If we add this code to lib-freetype.m4 (line 365) and re-run
autogen.sh,
>> this fixes the problem.
>>
>> if test "x$FOUND_FREETYPE" != xyes; then
>> FREETYPE_BASE_DIR="$SYSROOT/usr"
>> if test "x$OPENJDK_TARGET_CPU_ARCH" = xs390; then
>>LIB_CHECK_POTENTIAL_FREETYPE([$FREETYPE_BASE_DIR/include],
>> [$FREETYPE_BASE_DIR/lib/s390x-linux-gnu], [well-known location])
>> fi
>> fi
>>
>> Thoughts?
>
>Seems like a workaround for an actual bug to me. Also, on Debian s390x,
>the directory for shared libraries is also /usr/lib/s390x-linux-gnu:
>
>glaubitz@zelenka:~$ ls -dl /usr/lib/s390x-linux-gnu
>drwxr-xr-x 29 root root 28672 Dec 18 08:20 /usr/lib/s390x-linux-gnu
>glaubitz@zelenka:~$
>
>And I'm quite sure we don't have a quirk in the Debian openjdk package
>in the form of a patch. So, I'm not sure why the configure doesn't work
>in your case.
When you run configure, do you use --disable-warnings-as-errors?
Like here: https://github.com/linux-on-ibm-z/docs/wiki/Building-OpenJDK
If you use that, the error goes away, and it doesn't seem to adversely
affect the build.
>
>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
>
>
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
[PATCH] Freetype Directory Bug On zLinux
Hi All, On zLinux, freetype's .so file is typically installed in /usr/lib/s390x-linux-gnu, however the generated configure script doesn't look for it there. This causes configure to fail. I know you can avoid that with options, but I think a fix would be better. If we add this code to lib-freetype.m4 (line 365) and re-run autogen.sh, this fixes the problem. if test "x$FOUND_FREETYPE" != xyes; then FREETYPE_BASE_DIR="$SYSROOT/usr" if test "x$OPENJDK_TARGET_CPU_ARCH" = xs390; then LIB_CHECK_POTENTIAL_FREETYPE([$FREETYPE_BASE_DIR/include], [$FREETYPE_BASE_DIR/lib/s390x-linux-gnu], [well-known location]) fi fi 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
