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]
                                          

Reply via email to