Mojo's AspectJ Maven Plugin
  1. Mojo's AspectJ Maven Plugin
  2. MASPECTJ-94

<excludes> could also filter aspects included in <aspectLibrary>

    Details

    • Type: Improvement Improvement
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Won't Fix
    • Affects Version/s: 1.3.1
    • Fix Version/s: 1.5
    • Labels:
      None
    • Environment:
      all
    • Testcase included:
      yes
    • Number of attachments :
      1

      Description

      I was trying to exclude an aspect contained in spring-aspects library, that I was specifying as <aspectLibrary> in the config, when I realized it couldn't be filtered.

      I've created a test that fails when aspectLibrary aspect are woven despite their exclusion. It is to be added to the src/it tests.
      It's not very clean as it's based on an external aspect library (spring-aspects), but it shows what is expected clearly I believe.

      Let me know if I'm being unclear.

        Activity

        Hide
        nodje added a comment - - edited

        I've been investigating a bit the problem, and it seems the only things the plugin does with the specified <aspectLibraries> is to add them to the compile command line following the -aspectpath argument.
        That leave no simple direct option to be able to combine that with the specified <excludes> which work only on the included source path level.

        First of all, is it a good idea to have <excludes> working on both sourcePath and aspectLibraries?
        I'm not able to answer that question. ASAIK, -classpath seems to mix up classes to weave and aspect to apply.
        So why not apply exclusions to -aspectpath as well?

        If it makes sense, I can't find a good way to exclude aspect from -aspectpath in the native ajc doc anyway.
        It would then be necessary to treat the jar as classes and include them as classpath.

        Writing this made me realize that since sources to weave and aspect sources are mixed, I could include the aspect jars as <weaveDependency>, and it indeed works! But it's not filtered by the maven exclusion either... too bad.

        Show
        nodje added a comment - - edited I've been investigating a bit the problem, and it seems the only things the plugin does with the specified <aspectLibraries> is to add them to the compile command line following the -aspectpath argument. That leave no simple direct option to be able to combine that with the specified <excludes> which work only on the included source path level. First of all, is it a good idea to have <excludes> working on both sourcePath and aspectLibraries? I'm not able to answer that question. ASAIK, -classpath seems to mix up classes to weave and aspect to apply. So why not apply exclusions to -aspectpath as well? If it makes sense, I can't find a good way to exclude aspect from -aspectpath in the native ajc doc anyway. It would then be necessary to treat the jar as classes and include them as classpath. Writing this made me realize that since sources to weave and aspect sources are mixed, I could include the aspect jars as <weaveDependency>, and it indeed works! But it's not filtered by the maven exclusion either... too bad.
        Hide
        Robert Scholte added a comment -

        If it makes sense, I can't find a good way to exclude aspect from -aspectpath in the native ajc doc anyway.

        Here you hit the problem. If there's no way to explicit include or exclude files from a jar, you're kind of stuck.
        Maybe it's possible to use the maven-dependency-plugin to unpack the dependencies and select this folder instead of the dependencies.

        Show
        Robert Scholte added a comment - If it makes sense, I can't find a good way to exclude aspect from -aspectpath in the native ajc doc anyway. Here you hit the problem. If there's no way to explicit include or exclude files from a jar, you're kind of stuck. Maybe it's possible to use the maven-dependency-plugin to unpack the dependencies and select this folder instead of the dependencies.
        Hide
        nodje added a comment -

        maven-dependency-plugin seems good to me.
        Since all included JARS, be it weaveDependency or aspectLibrary, can contain aspect, I can't see any reason to make a difference between these two.

        There should be a level of abstraction between the plugin and AJC, providing something in the line of ~ <includeJarDependency> that should be unpacked and included as source, so that we can a single consistent behavior across all aspect to apply.

        I can't help but finding it very odd that ajc provides a -inpath and a -aspectpath and that both can contain aspect to be applied on source. There must be a good reason. I've open the question on Stackoverflow.

        Anyway, even if it makes sense to keep weaveDependency or aspectLibrary, they both should be unpacked and included as source folder.

        Show
        nodje added a comment - maven-dependency-plugin seems good to me. Since all included JARS, be it weaveDependency or aspectLibrary, can contain aspect, I can't see any reason to make a difference between these two. There should be a level of abstraction between the plugin and AJC, providing something in the line of ~ <includeJarDependency> that should be unpacked and included as source, so that we can a single consistent behavior across all aspect to apply. I can't help but finding it very odd that ajc provides a -inpath and a -aspectpath and that both can contain aspect to be applied on source. There must be a good reason. I've open the question on Stackoverflow. Anyway, even if it makes sense to keep weaveDependency or aspectLibrary, they both should be unpacked and included as source folder.
        Hide
        Robert Scholte added a comment -

        Anyway, even if it makes sense to keep weaveDependency or aspectLibrary, they both should be unpacked and included as source folder.

        Not by default, maybe as an option because it's an expensive and unusual operation (using the jars often works as required). And you'll need to keep the classloading-order in mind.
        If the m-dependency-p solutions works, it's maybe better to change this to a minor wish and adjust the title of the issue.

        Show
        Robert Scholte added a comment - Anyway, even if it makes sense to keep weaveDependency or aspectLibrary, they both should be unpacked and included as source folder. Not by default, maybe as an option because it's an expensive and unusual operation (using the jars often works as required). And you'll need to keep the classloading-order in mind. If the m-dependency-p solutions works, it's maybe better to change this to a minor wish and adjust the title of the issue.
        Hide
        nodje added a comment -

        I'll give a try to m-dependency-p asaic
        and will try to think about some good way to present the end user the option to include JARS in the filtering system.

        Show
        nodje added a comment - I'll give a try to m-dependency-p asaic and will try to think about some good way to present the end user the option to include JARS in the filtering system.
        Hide
        Lennart Jörelid added a comment -

        The AspectJ Maven Plugin is currently a wrapper around the AJC tool as
        delivered by the AspectJ project. While we could certainly include more
        functionality into the A-M-P, it makes more sense to include such functionality
        into the AJC tool itself.

        I'd recommend pitch this idea to the AspectJ project.

        Show
        Lennart Jörelid added a comment - The AspectJ Maven Plugin is currently a wrapper around the AJC tool as delivered by the AspectJ project. While we could certainly include more functionality into the A-M-P, it makes more sense to include such functionality into the AJC tool itself. I'd recommend pitch this idea to the AspectJ project.

          People

          • Assignee:
            Lennart Jörelid
            Reporter:
            nodje
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: