[OpenJDK 2D-Dev] RFR(xxxs): 8200052: libjavajpeg: Fix compile warning in jchuff.c

Mon May 21 08:51:48 UTC 2018

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 <adam.farley at uk.ibm.com> 
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 <philip.race at oracle.com>
> >> To: Adam Farley8 <adam.farley at uk.ibm.com>
> >> Cc: 2d-dev <2d-dev at openjdk.java.net>, Andrew Leonard
> >> <andrew_m_leonard at uk.ibm.com>, build-dev 
<build-dev at openjdk.java.net>,
> >> Magnus Ihse Bursie <magnus.ihse.bursie at oracle.com>, "Thomas Stüfe"
> >> <thomas.stuefe at gmail.com>
> >>
> >>
> >> 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
> >> > <magnus.ihse.bursie at 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.stuefe at 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
> >> > >
> >> > >
> >>
> >> 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20180521/3c64ec29/attachment-0001.html>