Security fixes are back; other fixes can go in. Time for build 18?
Andrew John Hughes
gnu_andrew at member.fsf.org
Mon Jan 11 17:03:49 PST 2010
2010/1/12 Joe Darcy <Joe.Darcy at sun.com>:
> On 01/11/10 03:53 PM, Kelly O'Hair wrote:
>>
>> Andrew John Hughes wrote:
>>>
>>> 2010/1/11 Joseph D. Darcy <Joe.Darcy at sun.com>:
>>>>
>>>> Joseph D. Darcy wrote:
>>>>>
>>>>> Andrew John Hughes wrote:
>>>>>>
>>>>>> 2010/1/8 Joe Darcy <Joe.Darcy at sun.com>:
>>>>>>
>>>>>>> Andrew John Hughes wrote:
>>>>>>>
>>>>>>>> 2010/1/7 Joseph D. Darcy <Joe.Darcy at sun.com>:
>>>>>>>>
>>>>>>>>
>>>>>>>>> Andrew John Hughes wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> 2009/12/25 Andrew John Hughes <gnu_andrew at member.fsf.org>:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>> 2009/12/24 Joseph D. Darcy <Joe.Darcy at sun.com>:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>> Andrew John Hughes wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>> [big snip]
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>>>> The com.sun.java.swing package in OpenJDK should have the same
>>>>>>>>>>>> effective
>>>>>>>>>>>> compile-time visibility as in Sun JDK.
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>> I don't know what that is; does this mean
>>>>>>>>>>> com.sun.java.swing.plaf.nimbus is hidden in the proprietary JDK6?
>>>>>>>>>>> I
>>>>>>>>>>> don't mind either way, it just seems that the other plaf packages
>>>>>>>>>>> are
>>>>>>>>>>> visible.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>> I'm going to start taking my vacation in earnest now so we can
>>>>>>>>>>>> finish
>>>>>>>>>>>> working through this issue early in 2010.
>>>>>>>>>>>>
>>>>>>>>>>>> Happy holidays,
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>> Happy new year! Any more thoughts on the above?
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>> Yes, easing back from vacation and donning my fedora and bullwhip,
>>>>>>>>> I've
>>>>>>>>> dug
>>>>>>>>> into what is going on here.
>>>>>>>>>
>>>>>>>>> In brief, make/common/Release.gmk has a list of packages to exclude
>>>>>>>>> from
>>>>>>>>> the
>>>>>>>>> ct.sym warning (6476749: "Exclude Swing plaf classes from Sun
>>>>>>>>> Proprietary
>>>>>>>>> warning"); from
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> http://hg.openjdk.java.net/jdk6/jdk6/jdk/raw-file/c00f461c45bc/make/common/Release.gmk
>>>>>>>>>
>>>>>>>>> # The compiler should not issue a "Sun Propietary" warning when
>>>>>>>>> compiling
>>>>>>>>> # classes in the com.sun.java.swing.plaf packages, since we've
>>>>>>>>> always
>>>>>>>>> # allowed, and even advocated, extending them (see bug 6476749).
>>>>>>>>> #
>>>>>>>>> # This approach is NOT to be used as a general purpose way to avoid
>>>>>>>>> such
>>>>>>>>> # compiler warnings for non-core packages. The correct way is to
>>>>>>>>> document
>>>>>>>>> # the packages in NON_CORE_PKGS.gmk, and include them in the
>>>>>>>>> NON_CORE_PKGS
>>>>>>>>> # definition.
>>>>>>>>> #
>>>>>>>>> # Swing has taken this approach only as a temporary measure to
>>>>>>>>> avoid
>>>>>>>>> # the compiler warnings until we can properly document these
>>>>>>>>> packages.
>>>>>>>>> # This is covered under 6491853.
>>>>>>>>> EXCLUDE_PROPWARN_PKGS = com.sun.java.swing.plaf \
>>>>>>>>> com.sun.java.swing.plaf.windows \
>>>>>>>>> com.sun.java.swing.plaf.motif \
>>>>>>>>> com.sun.java.swing.plaf.gtk
>>>>>>>>>
>>>>>>>>> In Sun's 6 update train, com.sun.java.swing.plaf.nimbus is included
>>>>>>>>> on
>>>>>>>>> that
>>>>>>>>> package list. Therefore, the test file in question compiles
>>>>>>>>> without
>>>>>>>>> warning
>>>>>>>>> using Sun's 6 update release. The corresponding addition to this
>>>>>>>>> list
>>>>>>>>> has
>>>>>>>>> *not* been made in JDK 7, which is probably just an oversight.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>> In 7, it's now a standard API javax.swing.plaf.nimbus. So you'll
>>>>>>>> find
>>>>>>>> it listed in make/docs/CORE_PKGS.gmk instead:
>>>>>>>>
>>>>>>>>
>>>>>>> Yes, for JDK 7 javax.swing.plaf.nimbus is a core package.
>>>>>>>
>>>>>>>
>>>>>>>> javax.swing.plaf.basic \
>>>>>>>> javax.swing.plaf.metal \
>>>>>>>> javax.swing.plaf.multi \
>>>>>>>> javax.swing.plaf.nimbus \
>>>>>>>> javax.swing.plaf.synth \
>>>>>>>>
>>>>>>>>
>>>>>>> However, javax.swing.plaf.nimbus being a core package in JDK 7 has no
>>>>>>> direct
>>>>>>> bearing how com.sun.java.swing.plaf.nimbus should be regarding in
>>>>>>> either
>>>>>>> OpenJDK 6 or the 6 update release.
>>>>>>>
>>>>>>> Generally, the "exported external" APIs in the JDK are in the java.*
>>>>>>> and
>>>>>>> javax.* namespaces. These are the APIs that everyone is supposed to
>>>>>>> call,
>>>>>>> etc. Nearly all of the sun.* and com.sun.* API are not in this
>>>>>>> category;
>>>>>>> rather they are only intended to be used by a restricted set of
>>>>>>> parties,
>>>>>>> such as the JDK implementation. The Swing packages listed in
>>>>>>> EXCLUDE_PROPWARN_PKGS are an exception to this rule.
>>>>>>>
>>>>>>> The ct.sym feature is a compile-time proto module system to add an
>>>>>>> enforcement mechanism to this long-standing API policy.
>>>>>>>
>>>>>>>
>>>>>> Indeed. I wasn't saying that it should be treated the same. I was
>>>>>> responding to your point that com.sun.java.swing.plaf.nimbus was not
>>>>>> added to the exclusion list in OpenJDK7 i.e. it obviously hasn't
>>>>>> because the package no longer exists in 7 and had to be recreated from
>>>>>> 6.
>>>>>>
>>>>>>
>>>>>>>>> I'd support com.sun.java.swing.plaf.nimbus being included in this
>>>>>>>>> list
>>>>>>>>> in
>>>>>>>>> OpenJDK 6 as long as
>>>>>>>>>
>>>>>>>>> * The API of the package is the same as in Sun's 6 update release
>>>>>>>>>
>>>>>>>>>
>>>>>>>> We have no way of verifying what's in the 6 update release as it is
>>>>>>>> proprietary software.
>>>>>>>>
>>>>>>> After hacking together a small annotation processor, I will send the
>>>>>>> source
>>>>>>> for the processor and a list of the signatures of the methods and
>>>>>>> constructors of the types in the Nimbus package in the 6 update
>>>>>>> train.
>>>>>>>
>>>>>>>
>>>>>> But what do we do if they don't match? Are we supposed to start
>>>>>> recreating missing methods from the proprietary 6 code when we have no
>>>>>> idea how they function?
>>>>>> The code from 7 is all we have.
>>>>>>
>>>>> We can evaluate the difference once we see them. Options include not
>>>>> exposing the com.sun package in OpenJDK 6 and implementing any missing
>>>>> functionality or everything might match and they'll be nothing to do
>>>>> :-)
>>>>>
>>>>>>>> The Nimbus code in OpenJDK6 is a backport of
>>>>>>>> what's in 7, moved back to the com.sun.java.swing.plaf.nimbus
>>>>>>>> namespace wihich I believe was used in the proprietary 6 update
>>>>>>>> train.
>>>>>>>>
>>>>>>>> Remember that this is an open-source project; regardless of whether
>>>>>>>> arbitrary bits of code are made invisible or not in a unpatched
>>>>>>>> build
>>>>>>>> of OpenJDK6, anyone can easily read the code and even rip the ct.sym
>>>>>>>> hack right back out again.
>>>>>>>>
>>>>>>> Just because something is technically feasible does not mean it is
>>>>>>> advisable. Many technically possible changes are not done so that
>>>>>>> various
>>>>>>> other goals can be met, such as following the Java SE 6
>>>>>>> specification,
>>>>>>> etc.
>>>>>>>
>>>>>>> If one takes steps to circumvent ct.sym or other enforcement
>>>>>>> mechanism,
>>>>>>> it
>>>>>>> is clear who should be responsible for any adverse consequences. For
>>>>>>> example, if someone circumvents ct.sym and then a type being accessed
>>>>>>> is
>>>>>>> changed in a subsequent release, that is a "close not a bug"
>>>>>>> situation.
>>>>>>>
>>>>>>>
>>>>>> I didn't say it was advisable. I'm just pointing out that it was a
>>>>>> weak solution to begin with, and even weaker now that people can study
>>>>>> and alter the source code rather than just having a binary blob to
>>>>>> work with.
>>>>>>
>>>>> Stronger mechanisms are planned for JDK internal implementation classes
>>>>> new in JDK 7 where backwards compatibility is not a concern.
>>>>>
>>>>>> I'm a fan of warning about the use of this code so it's visibly a bug
>>>>>> and something that should be fixed. I also think it's perfectly
>>>>>> legitimately to close bugs that refer to code that's not part of the
>>>>>> public API (though Sun don't produce OpenJDK6 binaries anyway).
>>>>>> Plenty of other open source projects do that without actively trying
>>>>>> to stop people using it in the first place.
>>>>>>
>>>>>> I don't like the idea of code appearing to disappear as it just causes
>>>>>> confusion, usually for end users who don't work on the code itself.
>>>>>> It also impacts debugging, see Andrew Haley's blog on trying to track
>>>>>> down an issue in javac: http://andrew-haley.livejournal.com/ The very
>>>>>> fact that you term it 'an enforcement mechanism' suggests treating
>>>>>> developers as potential criminals rather than as equals.
>>>>>>
>>>>> There are many enforcement mechanism in the Java platform. The class
>>>>> file
>>>>> verifier. The type checker in javac. The applet sandbox. The
>>>>> security
>>>>> model. No criminality need be inferred.
>>>>>
>>>>>>>> Additionally, not doing this would make
>>>>>>>> OpenJDK6 different from the proprietary release -- as you mention
>>>>>>>> above, it doesn't mask this package.
>>>>>>>>
>>>>>>>>
>>>>>>> Because the proprietary release does not unmask this package, if
>>>>>>> OpenJDK
>>>>>>> 6
>>>>>>> also unmasks this package, IMO it is important that the OpenJDK 6 API
>>>>>>> match
>>>>>>> the API exposed in the proprietary release.
>>>>>>>
>>>>>>>
>>>>>> I'm confused here -- does the proprietary JDK6 make this package
>>>>>> visible or not?
>>>>>
>>>>> The com.sun.* nimbus package is visible in the proprietary JDK6
>>>>> starting
>>>>> (I assume) in 6u10 when Nimbus was added; it is visible in 6u17 where I
>>>>> was
>>>>> able to successfully compile your test program.
>>>>>>
>>>>>> You seemed to be saying earlier that it did and your
>>>>>> idea of checking what the API is using an annotation processor also
>>>>>> suggests this.
>>>>>>
>>>>> Correct.
>>>>>
>>>>>> I agree it's important they match. I just don't see what we can do if
>>>>>> they don't.
>>>>>>
>>>>> Let's see how far the two packages are from an alpha rename and
>>>>> evaluate
>>>>> our options.
>>>>>
>>>> Greetings.
>>>>
>>>> Before writing an annotation processor, I tried a simple diff. Good
>>>> news!
>>>> To make the API signatures of the com.sun.java.swing.plaf.nimbus
>>>> packages
>>>> match in OpenJDK 6 and the proprietary JDK the following two classes in
>>>> OpenJDK 6 need to be made public:
>>>>
>>>> LoweredBorder.java
>>>> TableScrollPaneCorner.java
>>>>
>>>> So I will approve the following change
>>>>
>>>> * The two classes above are changed to be public
>>>> * The package in question is added to the EXCLUDE_PROPWARN_PKGS list in
>>>> make/common/Release.gmk
>>>>
>>>> as long as Kelly also reviews any build impact.
>>
>> Makefile change seems harmless.
>>
>> -kto
>>
>
> With that, consider the change approved; please push using bug id 6915980
> "Do not issue build warnings for use of com.sun.java.swing.plaf.nimbus."
>
> Thanks,
>
> -Joe
>
Pushed:http://hg.openjdk.java.net/jdk6/jdk6/jdk/rev/f6d8d0c0e6ad
There's another Nimbus patch that should be backported before b18:
# HG changeset patch
# User peterz
# Date 1258555006 -10800
# Node ID fc3997fd1bcebf74031fdc43d44b371e4be3d29b
# Parent 4f0275ea56fdb3f8199d16951954cfee80f931c2
6882917: Nimbus and DefaultTableCellRenderer: must start with normal background
Reviewed-by: rupashka
http://hg.openjdk.java.net/jdk7/jdk7/jdk/rev/fc3997fd1bcebf7403
--
Andrew :-)
Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)
Support Free Java!
Contribute to GNU Classpath and the OpenJDK
http://www.gnu.org/software/classpath
http://openjdk.java.net
PGP Key: 94EFD9D8 (http://subkeys.pgp.net)
Fingerprint: F8EF F1EA 401E 2E60 15FA 7927 142C 2591 94EF D9D8
More information about the jdk6-dev
mailing list