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
> <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/
<http://cr.openjdk.java.net/%7Estuefe/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
> >
> >
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
