Bug 124460 - [plan] Support aop.xml for controlling weaving in all compilation scenarios
Summary: [plan] Support aop.xml for controlling weaving in all compilation scenarios
Status: NEW
Alias: None
Product: AspectJ
Classification: Tools
Component: Compiler (show other bugs)
Version: DEVELOPMENT   Edit
Hardware: PC Windows XP
: P3 enhancement (vote)
Target Milestone: ---   Edit
Assignee: aspectj inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-01-19 04:53 EST by Adrian Colyer CLA
Modified: 2013-06-24 11:04 EDT (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Adrian Colyer CLA 2006-01-19 04:53:37 EST
Consider the case where I am building an aspect library with several *concrete* aspects in it. (These aspects are annotation-driven, so only triggered by use of the annotations). 

During development, for rapid iteration etc. I'm using LTW. But I only want to enable *some* of the aspects in the jar, so I write an aop.xml file that includes just the aspects I want. 

When I want to build the app to go into production I'm going to use binary weaving up front. So I put the aspect library on the aspectpath.... and bang - now all of a sudden I have no choice, *all* of the aspects in the library jar are going to be used for weaving and there is nothing I can do about it.

We need to add a weaver (compiler) option that says "use aop.xml files". This would be off by default, and switched on implicitly when doing LTW. This probably means refactoring some of the aop.xml interpretation code from the LTW packages and making sure we call it at the appropriate time.
Comment 1 Matthew Webster CLA 2006-01-20 05:33:25 EST
This has been mentioned before, possibly when aop.xml first came into existence. It would help bring more consistency to the configuration of weaving stages and should probably apply for compile-time as well as binary weaving. As discussed on the call it probably belongs on aspectpath because that is where the compiler will find a library and its accompanying aop.xml (possibly generated with -outxml), so perhaps "-inxml/inxmlfile" would be a good option name.

The aop.xml files are parsed in the order they are found on aspectpath (in the same way they are handled by getResources at load-time) although the only element that is order sensitive is compiler options. There are currently 2 sections in aop.xml: aspects and weaver. It probably makes sense, at least initially, to ignore the latter for non-LTW because compiler options and weaving scope are best determined using other existing mechanisms.
Comment 2 Ramnivas Laddad CLA 2007-01-03 16:48:40 EST
Just wanted to add a specific use case (you most likely already know about it, 
but just in case...).

This is actually becoming quite important in my recent experience with client 
projects. All the cases involved Spring whose library contains two aspects: 
bean configuration and transaction management. Most want to use bean configuration 
(enabled for classes carrying @Configurable), but not transaction management 
(enabled for classes/methods carrying @Transactional). With LTW, it is a simple 
matter of excluding the transaction management aspect. However, when they move to 
binary-weaving, there is no equivalent option (forcing kludgy workarounds).
Comment 3 Ramnivas Laddad CLA 2007-01-05 12:13:30 EST
Will the -inxml/-inxmlfile work identical to aop.xml file(s). If so, that may
be a problem. 

Consider for example, multiple aspects libraries:
spring-aspects.jar (tx mgmt and bean configuration driven by annotation)
aspects-r-us.jar (profiling, tracing, and other aspects driven by annotations)
...

Let's say, initially I want to include all aspects from all libraries.
All I do is put those jars in aspectpath. Later, if I want to exclude 
just one aspect from a library (say tx mgmt aspect), I will have to 
examine all the aspects in all the jars and enlist every single 
aspect under the <aspects> section. It seems to require too 
much effort for a simple change (and the effort is not incremental). Further,
if I ever add another aspects library, I will have to modify the XML file
again.

It almost seem that we need to specify per-jar configuration. 
By default, all aspects are woven in. Specifying a configuration
for a jar file changes the default behavior. My preference would be
to specify exclusion instead of inclusion, since that is an incremental
modification from the current include-by-default behavior.

Combine this with Mathew's comment about ignoring the <weaver> section
and it seems that aop.xml may not be the right format for compile-time 
weave configuration.

Perhaps, something like the following will work:

<aspectpath jar="spring-aspects.jar">
    <exclude>
        <aspect name="*..*Transaction*"/>
    </exclude>
</aspectpath>
<aspectpath jar="aspects-r-us.jar">
    <exclude name="*..*Tracing"/>

    <!-- same as aop.xml format -->
    <concrete-aspect name="com.foo.SQLMonitoring"
                     extends="org.aspectsrus.monitoring.AbstractMonitoring">
        <pointcut name="monitored" expression="call(* java.sql.*.*(..))"/>
    </concrete-aspect>
</aspectpath>

I suppose <include> may be supported as well with the condition that you may
either use <include> or <exclude> but not both. <concrete-aspect> may be used 
in any case.

Of course, -outxml or -outxmlfile should consider this aspect configuration file
when creating aop.xml.
Comment 4 Matthew Webster CLA 2007-01-08 08:26:01 EST
I think the only way it makes sense to use aop.xml for offline weaving is for the implementation to be a complete analog of LTW. This was the motivation for opening the enhancement i.e. to be able to successfully use configuration from LTW with binary weaving, unchanged. Think of -inxml as a switch that says "find all aop.xml files on the classpath". Therefore by default you will use all declared aspects. If you want to exlude an aspect, and it makes sense to do so, then simply use <exclude within="="*..*Tracing"/> in the <aspects> section of the aop.xml defined for your application. There is no need to specify a JAR name.

My comment about ignoring the <weaver> section when operation offline relates to its primary purpose which is to provide configuration. There are already several ways of doing this so adding another may be confusing. It will also help if the aop.xml is to be used both offline and online where different options may be needed.
Comment 5 Ramnivas Laddad CLA 2007-01-08 09:52:20 EST
> If you want to exlude an aspect, and it makes sense to do so,
> then simply use <exclude within="="*..*Tracing"/> in the <aspects> section of
> the aop.xml defined for your application.

But then this will not be a complete analog of LTW.

LTW excludes all aspects except those specified in <aspects><aspect> 
section (but not matching a pattern in <aspects><exclude>). 
The -inxml will include all aspects except those specified in <exclude>. 
Right?

For example, consider the following on classpath and aspectpath:
business.jar, spring-aspects.jar, aspects-r-us.jar
and <aop.xml> with <exclude> for tx mgmt aspect
and nothing in <aspect> or <concrete-aspect>.

With LTW, you will get no aspects woven (<exclude> section will
be redundant, as there is nothing specified in the <aspect> or 
<concrete-aspect> section).

With -inxml, all aspects will be woven in except the tx mgmt. 

The only way for LTW to match the -inxml behavior 
is to include all aspects--one by one--in <aspect> section (since we 
can't use type patterns in <aspect name="..."/>). This is partly because 
there is no way to specify an -aspectpath like path to LTW.
Is this interpretation of your comment correct?

The only way I see to harmonize the multiple kinds of weaving mechanism 
is to have LTW support either wildcards in <aspect name="..."/>
(but has performance consequences) or support -aspectpath like concept,
say, a parameter to -javaagent:aspectjweaver;aspectpath= (still has peformance
consequences, but lowered for typical aspects library use cases). 
Or something along the line I suggested in my last comment.
Comment 6 Matthew Webster CLA 2007-01-08 12:02:16 EST
I have no intention of changing the way LTW works! What I am saying is the user chooses which aspects are used based on name and that the use of filenames in aop.xml is redundant. In the long term I would like to drop aspectpath altogether but for the moment we may need to use both approaches together.

Using aspectpath is simply a way of telling the compiler what aspects to use. It could also be achieve by including a (possibly generated) aop.xml file in each library JAR (which is what you need for LTW). In this case my suggestion still works. When -inxml is specified we search classpath for any aop.xml files. We also search an optional aspectpath for aspects by enumerating its contents as we do today. That gives us a complete list of aspects which we can then filter using an aggregate of the include/exclude statements found according to the rules in the LTW documentation (http://www.eclipse.org/aspectj/doc/released/devguide/ltw-configuration.html). 

The aop.xml file included with an application that selects aspects in this way will work both offline and online regardless of whether aspectpath is used. Would this solve your problem?
Comment 7 Ramnivas Laddad CLA 2007-01-08 20:16:10 EST
Thanks Mathew for explaining your thoughts. If each aspect library includes
an automatically generated aop.xml that lists all aspects, then I think it 
solves use cases I have in mind.
Such a solution is certainly attractive because:
1. It is simple as long as aspect libraries always include aop.xml listing *all*
   contained aspects (definitely must be generated).
2. It is explainable. Essentially, we will have aspect library jars 
   (not very unlike bundles in OSGi, which do include a lot of metadata).
3. It is parallel to what LTW does (the main point in the original bug report)

Of course, this works only under assumption that aspect libraries do include aop.xml. I wonder if 'ajc' should include -outlib option to produce 
jar that also include an automatically generated aop.xml. Or should -outjar
do it anyway (backwards compatibility issues may surface)? Or should
a combination of -outjar -outxml do the trick?
Comment 8 Matthew Webster CLA 2007-01-09 06:36:57 EST
The -outjar option only determines destination and must be used in combination with -outxml which only lists aspects that have been _compiled_ so does not include those on ASPECTPATH or INPATH. We could have an option that in conjunction with ASPECTPATH created a new JAR with an aop.xml. I think a better approach is to make -outxml the default (Bug 138398 "Make -outxml the default") and use ASPECTPATH in the meantime to complement aop.xml in development.
Comment 9 Andrew Clement CLA 2009-01-19 13:28:19 EST
the requirement has come up for this again and I'd like to do something.  However, as the discussion in this bug describes, it isn't straightforward to define the desired behaviour.

The problem came up recently because of someone attempting to compile Spring - they extracted the src zip and defined it as a bunch of projects.  That should be OK but some of the projects contained test aspects that were now leaking across all folders (these aspects were never designed for usage outside of their original module) causing very slow compile times and hundreds of thousands of unnecessary matches.  Basically it would have been nice to have a little aop.xml file alongside the original aspect declaration that said "the scope of weaving for this aspect X is just these files".
Comment 10 Andrew Clement CLA 2009-01-30 17:08:10 EST
I am starting work on this - gradual changes.  First enabling xml files to be passed on the command line so I can do some basic testing before trawling through jar files on the inpath/aspectpath.

Also upgrading the World to know whether it is under the control of XML configs and moving the XML parsing stuff from the loadtime module to the weaver module (although it will still all end up in the aspectjweaver.jar)
Comment 11 Andrew Clement CLA 2009-02-09 15:19:05 EST
I've just committed a bit more of this.  Some stuff is starting to work now.  I have these 4 files:

A.java
aspect A {
  before():staticinitialization(*) {}
}

A2.java
aspect A2 {
  before():staticinitialization(*) {}
}

B.java
public class B { }

B2.java
public class B2 {}

And this xml file:
foo2.xml
<aspectj>
<aspects>
  <aspect name="A" scope="B"/>
  <!-- <aspect name="A2"/> -->
</aspects>
</aspectj>

Notice A2 is not defined and A is scoped to only affect B (the scope is a type pattern).

Compile and run (the command line now accepts XML files):

ajc *.java foo2.xml -verbose -showWeaveInfo
...
Aspect 'A' is scoped to apply against types matching pattern 'B'
...
Join point 'staticinitialization(void B.<clinit>())' in Type 'B' (B.java:1) advised by before advice from 'A' (A.java:2)
...

Notice that the staticinit advice from A only applied to B.  Also the aspect A2 did nothing.

It is a zillion miles from production ready but a small step in the right direction.
Comment 12 Andrew Clement CLA 2009-02-11 14:28:16 EST
I've upgraded the ICompilerConfiguration interface so that, in time, AJDT can pass in the .xml files that should be used to configure weaving.  Because I think this feature is an opt-in, I've added the option "-xmlConfigured" that currently switches the compiler into a mode where it is open to accepting .xml files to configure the process.  This has already revealed a few things that need more thought.  Primarily, how do we recognize the aop.xml files that should be used to configure the behaviour?  As opposed to those that the programmer is just developing alongside their code in the same project or those created by -outxml.  Or even just the xml files that are there for other reasons.  We have to be careful.  

I upgraded the incremental test harness to add .xml files from the src folders to the input files for compilation - and on an incremental build it is accidentally picking up something produced by an earlier build.  I'm thinking we might have to only use the xml files as explicitly selected by the user, rather than searching for them ourselves and passing in everything discovered in the source folder.
Comment 13 Andrew Clement CLA 2009-02-12 11:48:12 EST
everything mentioned in comment 12 is now committed. So the example in comment 11 needs the -xmlConfigured option to be passed to ajc.
Comment 14 Andrew Clement CLA 2009-07-05 14:09:02 EDT
get the basics properly tested in 1.6.6 and released.  Will raise a further enhancement to cover future activities because not everything will be addressed under this one.
Comment 15 Andrew Clement CLA 2009-09-15 15:53:47 EDT
The weaver include/exclude section is now respected when an xml file is passed in on the command line or through the new Ant iajc attribute:

<inxml>
  <pathelement path="foo.xml"/>
</inxml>

The compiler still does not respect concrete aspect definitions in the XML though.

The AJDT support is almost there, probably be working by the end of the week.
Comment 16 Andrew Clement CLA 2010-05-03 14:30:04 EDT
committed the changes to enable this for ltw !
Comment 17 Andrew Clement CLA 2013-06-24 11:04:56 EDT
unsetting the target field which is currently set for something already released