bineanzhou / google-guice

Automatically exported from code.google.com/p/google-guice
Apache License 2.0
0 stars 0 forks source link

Guice references invalid class "com.google.inject.asm.util.TraceClassVisitor()" #208

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
In net.sf.cglib.core.DebuggingClassWriter there is a call to
Class.forName("org.objectweb.asm.util.TraceClassVisitor").  After the
jarjar transformation for Guice this becomes
Class.forName("com.google.inject.cglib.asm.util.TraceClassVisitor").  This
class does not exist, and this spooks some OSGi implementations (which is
how I found it).  This bug is sorta inherited from cglib-nodep, which has
an analogous problem in its attempt to inline org.objectweb.asm -- in that
case the unresolveable class is net.sf.cglib.asm.util.TraceClassVisitor().
 This isn't an issue in the non-nodep cglib, since the class name reverts
to its original org.objectweb.asm.util.TraceClassVisitor(), which can be
resolved correctly so long as the asm-utils jar is available.  However,
after the jarjar transformation into the com.google.inject namespace, the
renamed class is again no longer resolvable, effectively reimplementing the
original cglib-nodep bug.  The simplest fix is probably to add
asm-util-3.1.jar to the call to jarjar, thus bringing the asm.util classes
into the com.google.inject class hierarchy.  n.b. this will only work in
the current tree, fixing this for 1.0 would require repackaging cglib-nodep
itself with asm-util.

Original issue reported on code.google.com by A.E.MacS...@gmail.com on 12 Jun 2008 at 2:47

GoogleCodeExporter commented 9 years ago
Chris, can you investigate? I need somebody who knows jarjar and cglib, and 
you're
the best we got.

Original comment by limpbizkit on 13 Jun 2008 at 1:25

GoogleCodeExporter commented 9 years ago
What do you mean "this spooks some OSGi implementations"?

It uses forName for the explicit purpose of making the util jar optional--the
corresponding feature will only be enabled if the jar is present. i.e. it's not 
a bug
it's a feature :-)

Original comment by chris.no...@gmail.com on 13 Jun 2008 at 4:05

GoogleCodeExporter commented 9 years ago
Do we need to update our OSGi metadata to say that if org.objectweb.asm.util.* 
is available, we want it?

Original comment by limpbizkit on 13 Jun 2008 at 9:46

GoogleCodeExporter commented 9 years ago
SpringSource has a tool specific to its new Application Platform that we use to
generate our own OSGi manifests.  It's this that spots the errant forName() 
lookup. 
Decompiling DebuggingClassWriter.class built from a recent head revision, we 
see:

...
    static  {
        debugLocation = System.getProperty("cglib.debugLocation");
        if(debugLocation != null) {
            System.err.println("CGLIB debugging enabled, writing to '" +
debugLocation + "'");
            try {
                Class.forName("com.google.inject.asm.util.TraceClassVisitor");
                traceEnabled = true;
            }
            catch(Throwable ignore) { }
        }
    }
...

Jarjar has renamed the class being looked up; it can no longer be satisfied by 
the
objectweb asm-util.jar, as that exports 
org.objectweb.asm.util.TraceClassVisitor(),
which will never match, regardless of your OSGi metadata.  The feature will not 
be
enabled if the jar is present: i.e. it is a bug :-/ .  Again, the simple fix is 
to
have jarjar inline the asm-util jar as well, then the class lookup will succeed
internally.

Original comment by A.E.MacS...@gmail.com on 17 Jun 2008 at 10:54

GoogleCodeExporter commented 9 years ago
> SpringSource has a tool specific to its new Application Platform that we use 
to
> generate our own OSGi manifests. It's this that spots the errant forName() 
lookup.

You should be able to exclude errant packages from the manifest, at least 
that's what
most bundling tools support (such as the maven-bundle-plugin, the Bnd tool and 
the
Knopflerfish ant task).

Personally speaking, I'm not sure of the benefit of adding this jar to the 
bundle
just to satisfy a tool :) besides, trunk is already bundling Guice as a bundle. 
Is
the problem that you're re-bundling Guice?

Original comment by mccu...@gmail.com on 25 Jun 2008 at 2:18

GoogleCodeExporter commented 9 years ago
We can and do instruct our bundle tool to ignore the bogus class name lookup for
Guice.  We're constructing a repository of OSGi bundles for use with our 
Application
Platform.  For each bundle we publish to the repository, we also upload the 
packages
that form its transitive dependencies, so that the bundle will run in the 
platform. 
Where possible, we try to match the versions of the dependencies to those that 
the
original package was compiled with.  Apache MyFaces depends on Guice 1.0, so 
that's
what we are bundling.  

This bug is not affected in any way by our OSGi tooling, the tool just happens 
to
spot the unresolvable class name as something it might need to worry about.  If 
you
are happy that CGLIB debugging is effectively (and silently) disabled in Guice, 
feel
free to ignore the tool warning (and this bug report ;-).

Original comment by A.E.MacS...@gmail.com on 26 Jun 2008 at 2:45

GoogleCodeExporter commented 9 years ago
Not sure how many people will actually need to enable CGLIB debugging, given 
that
Guice doesn't use anything "out of the ordinary". But even then, debugging can 
be
enabled by providing an extra jar containing the relocated "Trace*" classes.

Otherwise adding the "asm-util-3.1.jar" jar to lib/build and adding:

  <zipfileset src="lib/build/asm-util-3.1.jar"/>

to the build.xml will put the required classes in the distributable (around 
15Kb).

Personally speaking, I don't mind either way - but for what it's worth, leaving 
these
trace classes out of the distributable won't affect the normal operation of 
CGLIB/ASM
or cause any problems with OSGi frameworks in general.

...BTW, do you know that the CGLIB bundle in your repository:

http://www.springsource.com/repository/app/bundle/version/detail?name=com.spring
source.net.sf.cglib&version=2.1.3

has a similar problem? It also looks up 
"net.sf.cglib.asm.util.TraceClassVisitor",
which isn't imported or available in the bundle. Note that this too is a 
relocated
class, as the original source is "org.objectweb.asm.util.TraceClassVisitor".

Original comment by mccu...@gmail.com on 26 Jun 2008 at 6:48

GoogleCodeExporter commented 9 years ago
Yes, I did know that our repository bundle had an analogous problem.  That's 
what I
was driving at in the references to cglib-nodep in my original bug report at 
the top
of this thread.  I didn't personally add that bundle to our repository, but I
discovered that problem while researching this one, hence my "sorta inherited 
from
cglib-nodep" characterisation of the Guice variant of the problem.  In our case 
we
can't really fix it, since we're just putting an OSGi wrapper round an upstream 
jar.
 I logged a bug with cglib as well:

http://sourceforge.net/tracker/index.php?func=detail&aid=1991915&group_id=56933&
atid=482368

Original comment by A.E.MacS...@gmail.com on 27 Jun 2008 at 1:41

GoogleCodeExporter commented 9 years ago
You might want to clarify the statement:

  "the unresolvable class spooks some OSGi implementations"

in that bug report, because the Class.forName statement should not "spook" the 
OSGi
runtime. If the named class cannot be found, an exception will be thrown - and 
this
is silently consumed by the CGLIB class (perhaps it should log a warning 
instead).

It's more of a tooling/packaging issue than a framework problem.

Original comment by mccu...@gmail.com on 27 Jun 2008 at 1:54

GoogleCodeExporter commented 9 years ago
Yes, I was handwaving a bit there; I was trying to avoid getting into detail 
about
our internal OSGi tooling, as it hasn't been published (yet).

Tony.

Original comment by A.E.MacS...@gmail.com on 27 Jun 2008 at 4:11

GoogleCodeExporter commented 9 years ago
Closing. If there's something actionable that we should do, please reopen...

Original comment by limpbizkit on 31 Dec 2008 at 12:28