Felix
  1. Felix
  2. FELIX-3358

Enhance the maven-scr-plugin to be compatible with recent Maven/Eclipse integrations

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: maven-scr-plugin-1.8.0
    • Fix Version/s: maven-scr-plugin-1.9.0
    • Component/s: SCR Tooling
    • Labels:
      None
    • Environment:
      Eclipse Indigo or newer

      Description

      With the recent changes brought to the Maven Eclipse integration, any unknown [0] plugins are flagged as problematic and an error is reported in the pom.xml . Typically this is solved by writing a thin integration layer between the Eclipse integration and the Maven plugin [1] or by instructing Eclipse to ignore some plugin executions.

      The new 1.1 version of the m2eclipse plugin will allow a Maven plugin ( with no links to Eclipse plugin development ) to use an enhanced API to become compatible out of the box with the Eclipse integration [2] .

      The maven-scr-plugin should take advantage of these new APIs to allow seamless integration with Eclipse.

      [0]: http://wiki.eclipse.org/M2E_plugin_execution_not_covered
      [1]: http://wiki.eclipse.org/M2E/Extension_Development
      [2]: http://wiki.eclipse.org/M2E_compatible_maven_plugins

        Issue Links

          Activity

          Hide
          Timo Naroska added a comment -

          attached lifecycle mapping as needed for m2eclipse 1.1.

          Show
          Timo Naroska added a comment - attached lifecycle mapping as needed for m2eclipse 1.1.
          Hide
          Carsten Ziegeler added a comment -

          I've applied a slightly modified version which ignores the plugin instead of executing it as a first step. WDYT?

          Show
          Carsten Ziegeler added a comment - I've applied a slightly modified version which ignores the plugin instead of executing it as a first step. WDYT?
          Hide
          Timo Naroska added a comment -

          Marking it as ignored means I don't get any feedback in the IDE during development of SCR annotated classes. It also means that projects using scr annotations are not usable in eclipse PDE (using m2e-tycho connectors) or pax-exam tests run in the IDE. Both workflows require that the ds files are created within the IDE.
          I'd even prefer to have no lifecycle mapping at all rather than having it ignored. Suppose someone with no knowledge about scr imports an scr project into eclipse. Without the lifecycle mapping he gets an error in the pom, which is bad, but at least you get a hint that something is amiss. Now, with the ignore-lifecycle mapping scr is silently ignored. No warning, but also no ds files being generated. This is something that can cost a poor developer hours of investigation. JM2C

          Show
          Timo Naroska added a comment - Marking it as ignored means I don't get any feedback in the IDE during development of SCR annotated classes. It also means that projects using scr annotations are not usable in eclipse PDE (using m2e-tycho connectors) or pax-exam tests run in the IDE. Both workflows require that the ds files are created within the IDE. I'd even prefer to have no lifecycle mapping at all rather than having it ignored. Suppose someone with no knowledge about scr imports an scr project into eclipse. Without the lifecycle mapping he gets an error in the pom, which is bad, but at least you get a hint that something is amiss. Now, with the ignore-lifecycle mapping scr is silently ignored. No warning, but also no ds files being generated. This is something that can cost a poor developer hours of investigation. JM2C
          Hide
          Robert Munteanu added a comment -

          In reply to comment #2:
          > I've applied a slightly modified version which ignores the plugin instead of
          > executing it as a first step. WDYT?

          I think this is a good first step. I've considered implementing a version of the plugin based on the 'm2e compatible maven plugins' approach. However, I recently became aware that the implementation is suboptimal when performing CLI builds, as it considers all source files changed. [1]

          I'm currently not sure what the best approach is, out of:

          • using the m2e-compatible approach of a BuildContext
          • using an annotation processor, which in theory has better change detection
          • providing an minimal m2e connector which delegates work to the scr plugin when needed

          [1]: http://dev.eclipse.org/mhonarc/lists/m2e-dev/msg01052.html

          Show
          Robert Munteanu added a comment - In reply to comment #2: > I've applied a slightly modified version which ignores the plugin instead of > executing it as a first step. WDYT? I think this is a good first step. I've considered implementing a version of the plugin based on the 'm2e compatible maven plugins' approach. However, I recently became aware that the implementation is suboptimal when performing CLI builds, as it considers all source files changed. [1] I'm currently not sure what the best approach is, out of: using the m2e-compatible approach of a BuildContext using an annotation processor, which in theory has better change detection providing an minimal m2e connector which delegates work to the scr plugin when needed [1] : http://dev.eclipse.org/mhonarc/lists/m2e-dev/msg01052.html
          Hide
          Carsten Ziegeler added a comment -

          Ok, I see, yes I agree we should try to make this a friendly plugin

          Now, I think we could simply create a separate descriptor xml for each component and then use the BuildContext to just process changed files and regenerate the xml for those.
          I'm still not against going the annotation processor way for the future, but right now I think its the harder way to go
          WDYT?

          Show
          Carsten Ziegeler added a comment - Ok, I see, yes I agree we should try to make this a friendly plugin Now, I think we could simply create a separate descriptor xml for each component and then use the BuildContext to just process changed files and regenerate the xml for those. I'm still not against going the annotation processor way for the future, but right now I think its the harder way to go WDYT?
          Hide
          rmuntean added a comment -

          In reply to comment #5:
          > Now, I think we could simply create a separate descriptor xml for each component
          > and then use the BuildContext to just process changed files and regenerate the
          > xml for those.

          Sounds good to me.

          Show
          rmuntean added a comment - In reply to comment #5: > Now, I think we could simply create a separate descriptor xml for each component > and then use the BuildContext to just process changed files and regenerate the > xml for those. Sounds good to me.
          Hide
          Carsten Ziegeler added a comment -

          For the BuildContext stuff, do you know of any sample plugin? Especially which dependencies to import, there seems to be an old version of the context and the new tesla one

          Show
          Carsten Ziegeler added a comment - For the BuildContext stuff, do you know of any sample plugin? Especially which dependencies to import, there seems to be an old version of the context and the new tesla one
          Hide
          rmuntean added a comment -

          The BuildContext seems used in regular m2e configurators, see for instance the xml beans connector

          As for the tesla version, it seems to be unfinished according to m2e-dev: Scanning for changed files in a m2e compatible Maven plugin

          Show
          rmuntean added a comment - The BuildContext seems used in regular m2e configurators, see for instance the xml beans connector As for the tesla version, it seems to be unfinished according to m2e-dev: Scanning for changed files in a m2e compatible Maven plugin
          Hide
          Carsten Ziegeler added a comment -

          I've committed a first version which uses the BuildContext - the basics seem to work somehow

          Show
          Carsten Ziegeler added a comment - I've committed a first version which uses the BuildContext - the basics seem to work somehow
          Hide
          rmuntean added a comment -

          I've tried the latest trunk version of both the plugin and the scr annotations for a simple bundle and the CLI build works as expected, including the interaction with the maven-felix-plugin for adding the Service-Component header. Additionally, the Eclipse plugin creates and updates the service descriptor files for components, which is great.

          I've noticed that if I rename a component or delete it, the descriptor files still remain in the workspace. Additionally there is one nice-to-have which could be added - the BuildContext seems to be able to add messages to source fiels, so that warnings which are reported on the CLI can be reported inside Eclipse as well.

          Show
          rmuntean added a comment - I've tried the latest trunk version of both the plugin and the scr annotations for a simple bundle and the CLI build works as expected, including the interaction with the maven-felix-plugin for adding the Service-Component header. Additionally, the Eclipse plugin creates and updates the service descriptor files for components, which is great. I've noticed that if I rename a component or delete it, the descriptor files still remain in the workspace. Additionally there is one nice-to-have which could be added - the BuildContext seems to be able to add messages to source fiels, so that warnings which are reported on the CLI can be reported inside Eclipse as well.
          Hide
          Carsten Ziegeler added a comment -

          Patches are welcome But yes, I don't know how to handle remove/rename and it totally makes sense to add warnings/errors to Eclipse

          Show
          Carsten Ziegeler added a comment - Patches are welcome But yes, I don't know how to handle remove/rename and it totally makes sense to add warnings/errors to Eclipse
          Hide
          rmuntean added a comment -

          Good point I'll try to dig in and see how these two issues can be fixed. Do you have a plan for when the 1.8.2 version should be released?

          Show
          rmuntean added a comment - Good point I'll try to dig in and see how these two issues can be fixed. Do you have a plan for when the 1.8.2 version should be released?
          Hide
          Carsten Ziegeler added a comment -

          Great We don't have a fixed date for the release yet, but I think once this is solved we should cut a new release

          Show
          Carsten Ziegeler added a comment - Great We don't have a fixed date for the release yet, but I think once this is solved we should cut a new release
          Hide
          rmuntean added a comment -

          The attached patch is a proof-of-concept implementation, not yet fully polished.

          I've adapted the current MavenLog to use the BuildContext whenever an error is reported with a partial or complete source location. This works in the following manner:

          CLI

          The error/warning is logged with a specific format, e.g.

          [WARNING] /home/rmuntean/w/workspace/bundle-sample/src/main/java/rmuntean/bundle_sample/ComplexDSComponent.java [0:0]: @Component : Lifecycle method deactivate has wrong number of arguments
          

          Eclipse

          A marker is attached to the file. However, since I don't have any source information ( line number and column number would be ideal ) the errors are not flagged in the editor, but only visible in the Erorrs/Markers view.

          I've started adding this lineNumber/columnNumber information to the scrplugin, but I've stopped for two reasons:

          1. I think it's best to add a SourceLocation class to the project to encapsulate the (file, line, column) information.
          2. I realised I need to modify the ASM processing in order to get the information, which I didn't have enough time to.

          All in all, this works right now, with the single caveat that markers are not deleted on incremental builds in Eclipse. But that will probably come when we will make full use of the BuildContext.

          Show
          rmuntean added a comment - The attached patch is a proof-of-concept implementation, not yet fully polished. I've adapted the current MavenLog to use the BuildContext whenever an error is reported with a partial or complete source location. This works in the following manner: CLI The error/warning is logged with a specific format, e.g. [WARNING] /home/rmuntean/w/workspace/bundle-sample/src/main/java/rmuntean/bundle_sample/ComplexDSComponent.java [0:0]: @Component : Lifecycle method deactivate has wrong number of arguments Eclipse A marker is attached to the file. However, since I don't have any source information ( line number and column number would be ideal ) the errors are not flagged in the editor, but only visible in the Erorrs/Markers view. I've started adding this lineNumber/columnNumber information to the scrplugin, but I've stopped for two reasons: 1. I think it's best to add a SourceLocation class to the project to encapsulate the (file, line, column) information. 2. I realised I need to modify the ASM processing in order to get the information, which I didn't have enough time to. All in all, this works right now, with the single caveat that markers are not deleted on incremental builds in Eclipse. But that will probably come when we will make full use of the BuildContext.
          Hide
          Carsten Ziegeler added a comment -

          Thanks for the patch and yes I agree something like a SourceLocation makes sense. I couldn't figure out how to get the line number information with ASM but I guess there is some way.
          If applied your patch so we can build up on your work. Thanks!

          Show
          Carsten Ziegeler added a comment - Thanks for the patch and yes I agree something like a SourceLocation makes sense. I couldn't figure out how to get the line number information with ASM but I guess there is some way. If applied your patch so we can build up on your work. Thanks!

            People

            • Assignee:
              Carsten Ziegeler
              Reporter:
              Robert Munteanu
            • Votes:
              2 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development