Hi Tim, I was confused with your comments that seemed to hint the code changes I put in removed the support for java5. Hence my previous response. I did notice the issue of not supporting java5 and wondered why no one ever asked for this support. I spoke to Rich who put the proxy subclassing originally. Not supporting java5 is probably a bug.
I have fixed it under the 'Java7 support' jira. Thanks for bringing it up. Regards, Emily On Mon, Feb 20, 2012 at 2:10 PM, Timothy Ward <[email protected]>wrote: > Emily, > > The proxy impl component uses the java-5-parent, so it is definitely > intended to work with Java 5. The current code will not pass tests using > Java 5, and so is broken. > > This may be a pre-existing issue, and if you'd prefer for me to raise a > critical JIRA bug for this then I will. > > In any event the code needs to be changed, your commit simply flagged it > up and so I pointed it out. None of the code generated by any of the > proxying should need Java 6, and I thought you'd be happy to include the > one-liner in the code you were changing. > > Tim > > > Date: Sun, 19 Feb 2012 11:53:58 +0000 > > Subject: Re: svn commit: r1245223 - in > /aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl: > ProxyUtils.java gen/ProxySubclassAdapter.java > interfaces/InterfaceCombiningClassAdapter.java > > From: [email protected] > > To: [email protected] > > > > Hi Tim, > > In the previous version of the class of ProxySubClassAdapter > > andInterfaceCombiningClassAdapter.java, both of the weaving is targeting > to > > java6. I cannot see how the previous code support java5 or earlier. Any > > pointer will be apprecated. > > > > previous code: adapter.visit(V1_6, ACC_PUBLIC | ACC_SYNTHETIC, className, > > null,... > > > > As for the utility class, there are some debates whether to create a > > utility class or do some workaround to create some irrelavant class > > dependencies, my view is to create a utility class and gradually the > class > > will grow. > > > > Regards, > > Emily > > > > > > On Sun, Feb 19, 2012 at 9:48 AM, Timothy Ward <[email protected] > >wrote: > > > > > > > > Hi Emily, > > > > > > Previous releases of the proxy code have absolutely supported Java 5. > The > > > latest release of AbstracWovenProxyAdapter contains the following > constants: > > > > > > public static final int JAVA_CLASS_VERSION = new > > > BigDecimal(System.getProperty("java.class.version")).intValue(); > > > public static final boolean IS_AT_LEAST_JAVA_6 = JAVA_CLASS_VERSION >= > > > Opcodes.V1_6; > > > > > > > > > These should probably have been used rather than creating a Utility > class > > > from them that contains exactly that one method. The weaving code can > read > > > and weave classes using any bytecode version, so there's no need to > > > artificially restrict the VMs that this code can be run on. A lot of > people > > > still use Java 5, and there's no good reason that this code needs to > > > require Java 6. > > > > > > Regards, > > > > > > Tim Ward > > > ------------------- > > > Apache Aries PMC member & Enterprise OSGi advocate > > > Enterprise OSGi in Action (http://www.manning.com/cummins) > > > ------------------- > > > > > > > > > > Date: Fri, 17 Feb 2012 16:22:28 +0000 > > > > Subject: Re: svn commit: r1245223 - in > > > > /aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl: > > > ProxyUtils.java gen/ProxySubclassAdapter.java > > > interfaces/InterfaceCombiningClassAdapter.java > > > > From: [email protected] > > > > To: [email protected] > > > > > > > > Previous, it only supports java 6 (as you can tell from the previous > code > > > > visit(Opcodes.V1_6, ...), so I don't think anyone use it (the weving > code > > > > has been around for a couple of years. No one asks for weaving at > java5). > > > > > > > > For caching, it is a good idea. I have committed the change. > > > > Thanks > > > > Emily > > > > > > > > On Fri, Feb 17, 2012 at 11:50 AM, Timothy Ward < > [email protected] > > > >wrote: > > > > > > > > > > > > > > Hi, > > > > > > > > > > Have I missed something here? The weaving code should support Java > 5 as > > > > > well as Java 6 and 7, but there's no version code for it in the > util > > > that's > > > > > just been committed. > > > > > > > > > > Also this value can be calculated and cached in a static init > block so > > > > > that the method can just return a constant value. This may sound > like > > > > > overkill, but we've had a number of people ask for every clock > cycle > > > we can > > > > > save! > > > > > > > > > > Regards, > > > > > > > > > > Tim Ward > > > > > ------------------- > > > > > Apache Aries PMC member & Enterprise OSGi advocate > > > > > Enterprise OSGi in Action (http://www.manning.com/cummins) > > > > > ------------------- > > > > > > > > > > > > > > > > Subject: svn commit: r1245223 - in > > > > > > > > > /aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl: > > > > > ProxyUtils.java gen/ProxySubclassAdapter.java > > > > > interfaces/InterfaceCombiningClassAdapter.java > > > > > > Date: Thu, 16 Feb 2012 22:38:28 +0000 > > > > > > To: [email protected] > > > > > > From: [email protected] > > > > > > > > > > > > Author: ejiang > > > > > > Date: Thu Feb 16 22:38:27 2012 > > > > > > New Revision: 1245223 > > > > > > > > > > > > URL: http://svn.apache.org/viewvc?rev=1245223&view=rev > > > > > > Log: > > > > > > ARIES-817: ASM4 for Java7 support > > > > > > > > > > > > Added: > > > > > > > > > > > > > > > aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java > > > > > > Modified: > > > > > > > > > > > > > > > aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java > > > > > > > > > > > > > > > aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java > > > > > > > > > > > > Added: > > > > > > > > > aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java > > > > > > URL: > > > > > > > > > http://svn.apache.org/viewvc/aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java?rev=1245223&view=auto > > > > > > > > > > > > > > > ============================================================================== > > > > > > --- > > > > > > > > > aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java > > > > > (added) > > > > > > +++ > > > > > > > > > aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java > > > > > Thu Feb 16 22:38:27 2012 > > > > > > @@ -0,0 +1,45 @@ > > > > > > +/* > > > > > > + * Licensed to the Apache Software Foundation (ASF) under one > > > > > > + * or more contributor license agreements. See the NOTICE file > > > > > > + * distributed with this work for additional information > > > > > > + * regarding copyright ownership. The ASF licenses this file > > > > > > + * to you under the Apache License, Version 2.0 (the > > > > > > + * "License"); you may not use this file except in compliance > > > > > > + * with the License. You may obtain a copy of the License at > > > > > > + * > > > > > > + * http://www.apache.org/licenses/LICENSE-2.0 > > > > > > + * > > > > > > + * Unless required by applicable law or agreed to in writing, > > > > > > + * software distributed under the License is distributed on an > > > > > > + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY > > > > > > + * KIND, either express or implied. See the License for the > > > > > > + * specific language governing permissions and limitations > > > > > > + * under the License. > > > > > > + */ > > > > > > +package org.apache.aries.proxy.impl; > > > > > > + > > > > > > +import java.math.BigDecimal; > > > > > > + > > > > > > +import org.objectweb.asm.Opcodes; > > > > > > +import org.slf4j.Logger; > > > > > > +import org.slf4j.LoggerFactory; > > > > > > + > > > > > > +public class ProxyUtils > > > > > > +{ > > > > > > + private static Logger LOGGER = > > > > > LoggerFactory.getLogger(ProxyUtils.class); > > > > > > + /** > > > > > > + * Get the java version to be woven at. > > > > > > + * @return > > > > > > + */ > > > > > > + public static int getWeavingJavaVersion() { > > > > > > + int javaClassVersion = new > > > > > BigDecimal(System.getProperty("java.class.version")).intValue(); > > > > > > + if (javaClassVersion >= Opcodes.V1_7) { > > > > > > + LOGGER.debug("Weaving to Java 7"); > > > > > > + return Opcodes.V1_7; > > > > > > + } else { > > > > > > + LOGGER.debug("Weaving to Java 6"); > > > > > > + return Opcodes.V1_6; > > > > > > + } > > > > > > + > > > > > > + } > > > > > > +} > > > > > > > > > > > > Modified: > > > > > > > > > aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java > > > > > > URL: > > > > > > > > > http://svn.apache.org/viewvc/aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java?rev=1245223&r1=1245222&r2=1245223&view=diff > > > > > > > > > > > > > > > ============================================================================== > > > > > > --- > > > > > > > > > aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java > > > > > (original) > > > > > > +++ > > > > > > > > > aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java > > > > > Thu Feb 16 22:38:27 2012 > > > > > > @@ -21,6 +21,7 @@ package org.apache.aries.proxy.impl.gen; > > > > > > import java.io.IOException; > > > > > > import java.lang.reflect.InvocationHandler; > > > > > > > > > > > > +import org.apache.aries.proxy.impl.ProxyUtils; > > > > > > import org.objectweb.asm.AnnotationVisitor; > > > > > > import org.objectweb.asm.Attribute; > > > > > > import org.objectweb.asm.ClassReader; > > > > > > @@ -107,9 +108,7 @@ public class ProxySubclassAdapter extend > > > > > > throw new TypeNotPresentException(superclassBinaryName, > cnfe); > > > > > > } > > > > > > > > > > > > - // move the existing class name to become the superclass > > > > > > - // modify the version of the dynamic subclass to be Java 1.6 > > > > > > - int newVersion = Opcodes.V1_6; > > > > > > + > > > > > > // keep the same access and signature as the superclass > (unless > > > > > it's abstract) > > > > > > // remove all the superclass interfaces because they will be > > > > > inherited > > > > > > // from the superclass anyway > > > > > > @@ -117,7 +116,7 @@ public class ProxySubclassAdapter extend > > > > > > //If the super was abstract the subclass should not be! > > > > > > access &= ~ACC_ABSTRACT; > > > > > > } > > > > > > - cv.visit(newVersion, access, newClassName, signature, name, > > > null); > > > > > > + cv.visit(ProxyUtils.getWeavingJavaVersion(), access, > > > newClassName, > > > > > signature, name, null); > > > > > > > > > > > > // add a private field for the invocation handler > > > > > > // this isn't static in case we have multiple instances of > the > > > same > > > > > > > > > > > > Modified: > > > > > > > > > aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java > > > > > > URL: > > > > > > > > > http://svn.apache.org/viewvc/aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java?rev=1245223&r1=1245222&r2=1245223&view=diff > > > > > > > > > > > > > > > ============================================================================== > > > > > > --- > > > > > > > > > aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java > > > > > (original) > > > > > > +++ > > > > > > > > > aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java > > > > > Thu Feb 16 22:38:27 2012 > > > > > > @@ -25,6 +25,7 @@ import java.util.Collection; > > > > > > import java.util.List; > > > > > > > > > > > > import org.apache.aries.proxy.UnableToProxyException; > > > > > > +import org.apache.aries.proxy.impl.ProxyUtils; > > > > > > import > org.apache.aries.proxy.impl.common.AbstractWovenProxyAdapter; > > > > > > import > org.apache.aries.proxy.impl.common.OSGiFriendlyClassVisitor; > > > > > > import > org.apache.aries.proxy.impl.common.OSGiFriendlyClassWriter; > > > > > > @@ -75,7 +76,7 @@ final class InterfaceCombiningClassAdapt > > > > > > i++; > > > > > > } > > > > > > > > > > > > - adapter.visit(V1_6, ACC_PUBLIC | ACC_SYNTHETIC, className, > null, > > > > > > + adapter.visit(ProxyUtils.getWeavingJavaVersion(), > ACC_PUBLIC | > > > > > ACC_SYNTHETIC, className, null, > > > > > > (superclass == null) ? > > > > > AbstractWovenProxyAdapter.OBJECT_TYPE.getInternalName() : > > > > > > Type.getInternalName(superclass), interfaceNames); > > > > > > } > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > Thanks > > > > Emily > > > > ================= > > > > Emily Jiang > > > > [email protected] > > > > > > > > > > > > > > -- > > Thanks > > Emily > > ================= > > Emily Jiang > > [email protected] > > -- Thanks Emily ================= Emily Jiang [email protected]
