Bug 196933 - iconBase="found/in/classpath.gif" does not work in JDK 6
iconBase="found/in/classpath.gif" does not work in JDK 6
Status: RESOLVED FIXED
Product: platform
Classification: Unclassified
Component: Actions
7.0
All All
: P2 (vote)
: 7.0
Assigned To: Jesse Glick
issues@platform
jdk_bug_6929404
: 70_HR_FIX
Depends on: 197104
Blocks: 196452
  Show dependency treegraph
 
Reported: 2011-03-21 18:25 UTC by Jesse Glick
Modified: 2011-04-04 21:54 UTC (History)
4 users (show)

See Also:
Issue Type: DEFECT
:


Attachments
Warn on JDK6, fail on JDK7 (7.22 KB, patch)
2011-03-23 10:14 UTC, Jaroslav Tulach
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jesse Glick 2011-03-21 18:25:11 UTC
See bug #196452 comment #1. If you have a module which has jlfgr-1_0.jar in its classpath and then you try to use

@ActionRegistration(displayName="New", iconBase="toolbarButtonGraphics/general/New16.gif")

you will get an error when building with JDK 6:

"Cannot find iconBase file at toolbarButtonGraphics/general/New16.gif"

even though the icon really exists.

The error is not produced using JDK 7's javac.
Comment 1 Jesse Glick 2011-03-21 18:32:10 UTC
Issue was introduced with

changeset:   189257:b466219620fe
parent:      189245:d8a6a684084f
user:        Jaroslav Tulach <jtulach@netbeans.org>
date:        Sun Feb 20 15:16:24 2011 +0100
files:       openide.awt/src/org/netbeans/modules/openide/awt/ActionProcessor.java openide.awt/test/unit/src/org/netbeans/modules/openide/awt/ActionProcessorTest.java
description:
Detect missing iconBase to easily diagnose typos in the name of the resource

The only workaround I can find is to relax the check a bit so that it still fails in JDK 7 when given an invalid path, but always passes in JDK 6: core-main #5adfdac10656
Comment 2 Quality Engineering 2011-03-22 09:55:55 UTC
Integrated into 'main-golden', will be available in build *201103220400* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/5adfdac10656
User: Jesse Glick <jglick@netbeans.org>
Log: #196933: existence check for icons in -classpath does not work in JDK 6 javac.
Comment 3 Jesse Glick 2011-03-22 20:31:19 UTC
Yarda can you review this fix?
Comment 4 Jaroslav Tulach 2011-03-23 10:14:11 UTC
Created attachment 107206 [details]
Warn on JDK6, fail on JDK7

First of all I think we should write a test.
Second I'd prefer to leave the behavior on for JDK7 and turn it into a warning for JDK6. 

I patch that does the above is attached.
Comment 5 Jaroslav Tulach 2011-03-23 10:15:36 UTC
Jesse also suggested to replace the loop with much simpler:

boolean found = false;
for (Location l : new Location[] {StandardLocation.SOURCE_PATH, StandardLocation.CLASS_OUTPUT}) {
   try {
     processingEnv.getFiler().getResource(l, "", ar.iconBase()).openInputStream().close();
     found = true;
     break;
   } catch (IOException x) {}
}
if (!found) {...}
Comment 6 Jesse Glick 2011-03-23 13:02:42 UTC
(In reply to comment #4)
> turn it into a warning for JDK6

I do not think this is a good idea, because it will produce an incorrect warning which you cannot suppress when the icon really exists in your classpath (as in the original motivating use case). It is better to miss some opportunities to warn about incorrect code than to falsely warn about correct code.

Anyway checking for AutoCloseable is wrong because what you want to check has nothing to do with whether the JRE is 6 or 7; you only care about the version of langtools (javac), which can run on JRE 5+. For example, the processor when run inside the NB editor on JDK 6 should be using the fixed version of javac (assuming nb-javac is synching regularly).

(In reply to comment #5)
> for (Location l : new Location[] {StandardLocation.SOURCE_PATH,
> StandardLocation.CLASS_OUTPUT}) {

...and CLASS_PATH.
Comment 7 Jaroslav Tulach 2011-03-25 06:28:37 UTC

> > turn it into a warning for JDK6
> 
> I do not think this is a good idea, because it will produce an incorrect
> warning which you cannot suppress when the icon really exists in your classpath

I've seen a comment from you, Jesse, somewhere saying that having icons outside of module JAR is a poor practice anyway. 

OK, so we should disable the check on JDK6 completely, right? We want to keep it on JDK7, right? If so, we can add the tests and run them only on JDK7.

> Anyway checking for AutoCloseable is wrong; you only care about the version
> of langtools (javac), 

I believe that it is safe to assume that when AutoCloseable is present, the compiler is comming from JDK7 as well. I don't think there is any explicit versioning in the compiler itself. We could use 
    static boolean isJDK7() {
        try {
            SourceVersion v = SourceVersion.valueOf("RELEASE_7"); // NOI18N
            return true;
        } catch (IllegalArgumentException ex) {
            return false;
        }
    }
but I am not sure if that is reliable (does not work in the test).

Passing to you, as I will not be able to integrate to release70 branch anytime soon.
Comment 8 Jesse Glick 2011-03-25 18:40:25 UTC
(In reply to comment #7)
> I've seen a comment from you, Jesse, somewhere saying that having icons outside
> of module JAR is a poor practice anyway. 

I was referring to icons in _another_ module. Did not even consider the use case of keeping icons in a separate Class-Path extension, which is perhaps unusual but I think valid - such a JAR is bundled in the same NBM and put in the same ClassLoader, so there is no possibility that the icon will not be there at runtime.

> OK, so we should disable the check on JDK6 completely, right? We want to keep
> it on JDK7, right?

That is what my current patch does.

> If so, we can add the tests and run them only on JDK7.

OK. Don't know of any builder which would do so, but useful for the future.

> I believe that it is safe to assume that when AutoCloseable is present, the
> compiler is coming from JDK7 as well.

No, because you may be running the JDK 6 compiler with an explicit boot CP.

I will see if there is some other check that can be used.
Comment 9 Jesse Glick 2011-03-25 19:16:36 UTC
core-main #7e55a4244466
Comment 10 Jesse Glick 2011-03-25 19:36:50 UTC
Several actions in com.visitrend.ndvis.actions are affected.
Comment 11 Quality Engineering 2011-03-28 08:46:19 UTC
Integrated into 'main-golden', will be available in build *201103280400* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/7e55a4244466
User: Jesse Glick <jglick@netbeans.org>
Log: #196933: iconBase="found/in/classpath.gif" does not work in JDK 6
Enabling test when we are really using JDK 7 javac.
Comment 12 Jesse Glick 2011-03-28 14:45:15 UTC
releases #dde9ff981058 (related prep commit), releases #ba288d024da9 (fix), releases #3c446f2e181c (test fix).
Comment 13 Tomas Danek 2011-04-01 11:19:04 UTC
could you please help me find a way to verify this?

i created simple jar with icon, created nb platform app, created module with action inside that app, in action changed iconbase to point at icon from jar, put that jar with icon inside nbplatform app (added it using library wrapper), ran...but cannot reproduce in old build (beta2)?

am i doing something wrong, or mac java is what matters here?

Product Version: NetBeans IDE 7.0 Beta 2 (Build 201102140001)
Java: 1.6.0_24; Java HotSpot(TM) 64-Bit Server VM 19.1-b02-334
System: Mac OS X version 10.6.7 running on x86_64; MacRoman; en_US (nb)
Userdir: /Users/tomas/.netbeans/7.0beta2
Comment 14 Jesse Glick 2011-04-04 21:54:34 UTC
(In reply to comment #13)
> i created simple jar with icon, created nb platform app, created module with
> action inside that app, in action changed iconbase to point at icon from jar,
> put that jar with icon inside nbplatform app (added it using library wrapper),
> ran

Good.

> cannot reproduce in old build (beta2)?

See comment #1; beta 2 performed no iconBase validation.


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2012, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo