[8u] RFR: 8218780: Update MUSCLE PCSC-Lite header files
Severin Gehwolf
sgehwolf at redhat.com
Mon Sep 2 12:14:12 UTC 2019
Hi Andrew,
Thanks for the review!
On Wed, 2019-08-28 at 18:15 +0100, Andrew John Hughes wrote:
> On 26/08/2019 14:23, Severin Gehwolf wrote:
> > Hi,
> >
> > Could I please get a review of this MUSCLE header files update in
> > OpenJDK 8u? I'd like to backport this bug as it's also going to be in
> > Oracle JDK 8u231 (equiv to OpenJDK 8u232) as well. The OpenJDK 11 patch
> > applies almost cleanly post path-unshuffelling. Changes which didn't
> > apply were a copyright header update in pcsc.c. I've omitted these.
> >
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8218780
> > webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8218780/jdk8/01/webrev/
> > JDK 11u changeset: https://hg.openjdk.java.net/jdk-updates/jdk11u-dev/rev/0bcc59a50c88
> >
> > There is going to be a follow-up fix adding back COPYING via
> > JDK-8226607 which I propose for backport to OpenJDK 8u as well.
> >
> > Testing: smartcardio sanity and build on Linux x86_64 (Fedora 30 and RHEL 6)
> >
> > Thanks to Aleksey Shipilev who helped test this patch.
> >
> > Thoughts?
> >
> > Thanks,
> > Severin
> >
>
> Most of this looks good. I was a little confused at first because the
> patch in your webrev looks quite different to the 11u changeset.
> However, once applied locally to the 8u repo, the diff between the two
> was as suggested and the PCSC library files (those in MUSCLE) were
> identical. I don't know what webrev did in creating that patch.
>
> The bit I don't understand is why you've decided to drop the copyright
> header change on the floor. Just because the original in 8u has 2014,
> while the original in 11u had 2015, does not make the change inapplicable.
OK. I see. I wasn't sure how to deal with copyright year updates. I've
added the copyright update back. The patch is now identical to JDK 11u
(modulo differing copyright year base: 2014 - jdk 8 - vs. 2018 - jdk
11):
http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8218780/jdk8/02/webrev
> A better alternative may actually be to backport JDK-8207233 [0] first
> which is a nice little cleanup patch. I suspect this patch would then
> apply cleanly as these PCSC files are rarely touched.
>
> [0] https://bugs.openjdk.java.net/browse/JDK-8207233
Hmm, this seems overkill just to get the copyright hunk to apply. I'd
prefer to keep this dependency out of scope for this patch. While a
nice clean-up it's not without risk backporting that too. Thoughts?
Thanks,
Severin
More information about the jdk8u-dev
mailing list