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 <magnus.ihse.bur...@oracle.com> 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 >> > langer are both Reviewers). >> > >> > >> > Thanks! Thomas >> > >> > >> > On Wed, Mar 21, 2018 at 6:20 PM, Thomas Stüfe <thomas.stu...@gmail.com> >> > 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 > >