Maven 2 & 3

MavenEmbedder.execute() doesn't run reactor modules

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Blocker Blocker
  • Resolution: Fixed
  • Affects Version/s: None
  • Fix Version/s: 2.0.5
  • Component/s: Embedding
  • Labels:
    None
  • Complexity:
    Intermediate
  • Number of attachments :
    3

Description

MavenEmbedder.execute() doesn't run reactor modules.

I've been trying to run it with "generate-sources" goal, but it only build the root project.

  1. MavenEmbedder2.java
    21/Feb/06 5:37 AM
    23 kB
    Jochen Wiedmann
  2. MavenEmbedder2.java
    15/Feb/06 7:29 PM
    16 kB
    Jochen Wiedmann
  3. MNG-1181.tar.gz
    15/Feb/06 10:41 AM
    2 kB
    Jochen Wiedmann

Activity

Hide
Jochen Wiedmann added a comment -

Sample project demonstrating the problem. Run "mvn package" in the top level directory.

Show
Jochen Wiedmann added a comment - Sample project demonstrating the problem. Run "mvn package" in the top level directory.
Hide
Jochen Wiedmann added a comment -

Attached is a file, which I do suggest as a replacement for the MavenEmbedder. When using this embedder, submodules are called fine.

I have decided against fixing the MavenEmbedder, because it is, IMO, a real mess. Not so much, because the code is bad or something similar, but because it is basically a replacement of the DefaultMaven class. However, both classes are really difficult and are doing basically the same things. IMO, they cannot, and should not, be maintained separately. A more sensible approach seems to me to use the DefaultMaven for both the usual Maven, and for embedded Mavens.

My suggestion does not resolve the problem of duplicate code. Rather it introduces new duplicates, this time between the MavenCli class and the suggested MavenEmbedder2. To resolve this, I propose the following:

  • Introduce a new class, or interface, MavenCliOptions, or whatever it may be called.
  • Move the methods isDebug(), getAlternatePomFile(), ... from MavenEmbedder2
    to the new class.
  • Change the method MavenEmbedder2.execute(String[], File) to
    execute(String[], File, MavenCliOptions)
  • Remove most code from MavenCli. The class should basically create an instance
    of MavenCliOptions and call MavenEmbedder2.
Show
Jochen Wiedmann added a comment - Attached is a file, which I do suggest as a replacement for the MavenEmbedder. When using this embedder, submodules are called fine. I have decided against fixing the MavenEmbedder, because it is, IMO, a real mess. Not so much, because the code is bad or something similar, but because it is basically a replacement of the DefaultMaven class. However, both classes are really difficult and are doing basically the same things. IMO, they cannot, and should not, be maintained separately. A more sensible approach seems to me to use the DefaultMaven for both the usual Maven, and for embedded Mavens. My suggestion does not resolve the problem of duplicate code. Rather it introduces new duplicates, this time between the MavenCli class and the suggested MavenEmbedder2. To resolve this, I propose the following:
  • Introduce a new class, or interface, MavenCliOptions, or whatever it may be called.
  • Move the methods isDebug(), getAlternatePomFile(), ... from MavenEmbedder2 to the new class.
  • Change the method MavenEmbedder2.execute(String[], File) to execute(String[], File, MavenCliOptions)
  • Remove most code from MavenCli. The class should basically create an instance of MavenCliOptions and call MavenEmbedder2.
Hide
Milos Kleint added a comment -

the embedder2 class seems to be missing some functionality of the original, namely the reading of models and MavenProjects.

can you elaborate on if/how MNG-1665 will work with your embedder2? that's a crucial functionality for my netbeans ide integration.

Show
Milos Kleint added a comment - the embedder2 class seems to be missing some functionality of the original, namely the reading of models and MavenProjects. can you elaborate on if/how MNG-1665 will work with your embedder2? that's a crucial functionality for my netbeans ide integration.
Hide
Jochen Wiedmann added a comment -

> the embedder2 class seems to be missing some functionality of the original,
> namely the reading of models and MavenProjects.

Yes, that's true, and I already noted that. I see three possible solutions:

  • My personal favourite: Open the DefaultMaven class to provide this functionality.
    Possibly open it as protected methods only and make MavenEmbedder2 a
    subclass of DefaultMaven, which opens this.
  • Copy code from DefaultMaven, or MavenEmbedder. Personally, I do not like
    this, because copying code is bad.
  • Change the MavenEmbedders execute methods to work with or like the
    MavenEmbedder. I also do not like this, because it still leaves the problem
    of doing things different than the "raw Maven".

> can you elaborate on if/how MNG-1665 will work with your embedder2?

The test case attached to MNG-1181 doesn't check that. However, considering
that the MavenEmbedder2 really does nothing different than the command line
Maven, I see no reason to believe the it won't work.

Show
Jochen Wiedmann added a comment - > the embedder2 class seems to be missing some functionality of the original, > namely the reading of models and MavenProjects. Yes, that's true, and I already noted that. I see three possible solutions:
  • My personal favourite: Open the DefaultMaven class to provide this functionality. Possibly open it as protected methods only and make MavenEmbedder2 a subclass of DefaultMaven, which opens this.
  • Copy code from DefaultMaven, or MavenEmbedder. Personally, I do not like this, because copying code is bad.
  • Change the MavenEmbedders execute methods to work with or like the MavenEmbedder. I also do not like this, because it still leaves the problem of doing things different than the "raw Maven".
> can you elaborate on if/how MNG-1665 will work with your embedder2? The test case attached to MNG-1181 doesn't check that. However, considering that the MavenEmbedder2 really does nothing different than the command line Maven, I see no reason to believe the it won't work.
Hide
Jochen Wiedmann added a comment -

Updated version of the MavenEmbedder2, adding the possibility to read a POM file and convert it into an instance of MavenProject.

This is still a proof of concept, and would need a lot of reengineering. However, using it, I have got a version of the Eclipse plugin with MNGECLIPSE-75 working.

Show
Jochen Wiedmann added a comment - Updated version of the MavenEmbedder2, adding the possibility to read a POM file and convert it into an instance of MavenProject. This is still a proof of concept, and would need a lot of reengineering. However, using it, I have got a version of the Eclipse plugin with MNGECLIPSE-75 working.
Hide
Jochen Wiedmann added a comment -

Sorry, not MNGECLIPSE-75, but MNGECLIPSE-71.

Show
Jochen Wiedmann added a comment - Sorry, not MNGECLIPSE-75, but MNGECLIPSE-71.
Hide
Jason van Zyl added a comment -

I apologize for not looking at these issues sooner and I do appreciate the effort and I would like to integrate some of your work but the embedder is not simply a repackaging of DefaultMaven. The intention is to also provide access/manipulation of models, artifacts and repositories. I'm not intentionally trying to duplicate the code, I was attemping to entirely decouple the embedder from the CLI notions. Eventually the embedder will be used in the MavenCli, the plugin integration testing code and the Ant tasks. I'm not trying to dupe things here intentionally.

I will start looking at integrating bits and pieces of your code, but patches and some discussion will get you further then just rewriting the embedder. It's not likely I'm going to replace it with something I'm unfamiliar with, formatted differently and working differently then I intended. I'm not trying to discourage you and gladly welcome the help but some smaller patches and dialog will get you much further.

Show
Jason van Zyl added a comment - I apologize for not looking at these issues sooner and I do appreciate the effort and I would like to integrate some of your work but the embedder is not simply a repackaging of DefaultMaven. The intention is to also provide access/manipulation of models, artifacts and repositories. I'm not intentionally trying to duplicate the code, I was attemping to entirely decouple the embedder from the CLI notions. Eventually the embedder will be used in the MavenCli, the plugin integration testing code and the Ant tasks. I'm not trying to dupe things here intentionally. I will start looking at integrating bits and pieces of your code, but patches and some discussion will get you further then just rewriting the embedder. It's not likely I'm going to replace it with something I'm unfamiliar with, formatted differently and working differently then I intended. I'm not trying to discourage you and gladly welcome the help but some smaller patches and dialog will get you much further.
Hide
Jochen Wiedmann added a comment -

Jason, I can follow your points, including the "formatted differently". This one wasn't intended, btw, I went as far as creating a special Maven formatter in my Eclipse.

However, please understand the following points:

  • I am not proposing a patch. I am posting a "proof of concept". I am mainly asking for guidance.
  • Without help, I am most possibly unable to fix the current MavenEmbedder, because it is too
    complex for me. You're the Maven expert, I am the novice. At least, I won't be able to fix it
    with a small patch.
  • I do not even understand the intended lifecycle of the MavenEmbedder, the DefaultMaven,
    and the objects they are using.
  • I am happy, if you decide to fix the MavenEmbedder without following my extended proposals.
    In that case I'll step aside and terminate my work on this request.

If you want me to carry on, here's my plan. Understand, that I make the steps as small as possible:

  • I propose an object (MavenRequestConfiguration?), that may serve as a replacement of the CLI
    within DefaultMaven. The object will receive additional properties, as required by the
    MavenEmbedder. This first step doesn't include other changes than proposing this object.
  • You split the objects properties into two categories: Those, which should be per
    MavenEmbedder instance and those, which are per request. Additionally, you should provide
    notes on using these properties, if they have lifcycle considerations, or something similar.
  • The former properties are removed from the object.
  • I propose a patch for DefaultMaven, the Maven interface, and the MavenCli, which makes
    the DefaultMaven use the above object (get/setMavenRequestConfiguration(), and getters/
    setters for the per embedder properties).
  • You accept the patch.
  • I propose a patch for the MavenEmbedder, which makes it use the DefaultMaven internally.
  • You accept the patch.
  • I propose a patch for the MavenCli, which makes it use the MavenEmbedder.

Changes to the above proposal are fine for me. My intention is to get this thing going. (Remember,
this is a five months old blocker!)

Show
Jochen Wiedmann added a comment - Jason, I can follow your points, including the "formatted differently". This one wasn't intended, btw, I went as far as creating a special Maven formatter in my Eclipse. However, please understand the following points:
  • I am not proposing a patch. I am posting a "proof of concept". I am mainly asking for guidance.
  • Without help, I am most possibly unable to fix the current MavenEmbedder, because it is too complex for me. You're the Maven expert, I am the novice. At least, I won't be able to fix it with a small patch.
  • I do not even understand the intended lifecycle of the MavenEmbedder, the DefaultMaven, and the objects they are using.
  • I am happy, if you decide to fix the MavenEmbedder without following my extended proposals. In that case I'll step aside and terminate my work on this request.
If you want me to carry on, here's my plan. Understand, that I make the steps as small as possible:
  • I propose an object (MavenRequestConfiguration?), that may serve as a replacement of the CLI within DefaultMaven. The object will receive additional properties, as required by the MavenEmbedder. This first step doesn't include other changes than proposing this object.
  • You split the objects properties into two categories: Those, which should be per MavenEmbedder instance and those, which are per request. Additionally, you should provide notes on using these properties, if they have lifcycle considerations, or something similar.
  • The former properties are removed from the object.
  • I propose a patch for DefaultMaven, the Maven interface, and the MavenCli, which makes the DefaultMaven use the above object (get/setMavenRequestConfiguration(), and getters/ setters for the per embedder properties).
  • You accept the patch.
  • I propose a patch for the MavenEmbedder, which makes it use the DefaultMaven internally.
  • You accept the patch.
  • I propose a patch for the MavenCli, which makes it use the MavenEmbedder.
Changes to the above proposal are fine for me. My intention is to get this thing going. (Remember, this is a five months old blocker!)
Hide
Jason van Zyl added a comment -

You will want to take a look here:

http://svn.apache.org/viewcvs.cgi/maven/components/branches/maven-embedder-refactor/

The MavenCli class now uses the embedder and I've pushed a lot of the code that was in the MavenCli class into DefaultMaven. Many of your points there have been addressed and next week I'll be actively working on this code so we probably need to exchange some email or chat on IRC if you want to do some work that I will incorporate.

I'm not sure if you are subscribed to the dev mailing list but I will be sending a message there talking about incorporating what is on the branch into the trunk.

Now to address your points above:

  • Everything that goes into DefaultMaven should be couched in terms of the MavenExecutionRequest. Internally Maven should deal with a MavenExecutionRequest and the MavenSession. Anything that comes in via the CLI should be translated into a request and the Settings object that is used throughout the core should be taken out of the core.
  • I've has started figuring out what is required per request to make sure that the embedder works in a multi threaded environment. So this you could certainly help me with. The embedder is pretty chopped up on the branch but you can see how the embedder is being using in the MavenCli class in the branch.
  • I would be happy to work with you to figure out how to best configure the embedder.
  • This I've done by allowing the configuration of the request itself. You can see exactly what's happening by looking at the MavenCli in the branch.
  • I think what I've started is heading down that path.
  • No patch is ever going to be accepted unless we are actually in discussion on a regular basis. That's how I can guide you, with some dialog.
  • This has been done on the branch.
  • Same as above regarding integrating large patches/changes.
  • This has been done on the branch.

Everyday this week and on the weekend I'll be doing some work on the embedder to prepare for a presentation on the 7th of March.

Show
Jason van Zyl added a comment - You will want to take a look here: http://svn.apache.org/viewcvs.cgi/maven/components/branches/maven-embedder-refactor/ The MavenCli class now uses the embedder and I've pushed a lot of the code that was in the MavenCli class into DefaultMaven. Many of your points there have been addressed and next week I'll be actively working on this code so we probably need to exchange some email or chat on IRC if you want to do some work that I will incorporate. I'm not sure if you are subscribed to the dev mailing list but I will be sending a message there talking about incorporating what is on the branch into the trunk. Now to address your points above:
  • Everything that goes into DefaultMaven should be couched in terms of the MavenExecutionRequest. Internally Maven should deal with a MavenExecutionRequest and the MavenSession. Anything that comes in via the CLI should be translated into a request and the Settings object that is used throughout the core should be taken out of the core.
  • I've has started figuring out what is required per request to make sure that the embedder works in a multi threaded environment. So this you could certainly help me with. The embedder is pretty chopped up on the branch but you can see how the embedder is being using in the MavenCli class in the branch.
  • I would be happy to work with you to figure out how to best configure the embedder.
  • This I've done by allowing the configuration of the request itself. You can see exactly what's happening by looking at the MavenCli in the branch.
  • I think what I've started is heading down that path.
  • No patch is ever going to be accepted unless we are actually in discussion on a regular basis. That's how I can guide you, with some dialog.
  • This has been done on the branch.
  • Same as above regarding integrating large patches/changes.
  • This has been done on the branch.
Everyday this week and on the weekend I'll be doing some work on the embedder to prepare for a presentation on the 7th of March.
Hide
Jason van Zyl added a comment -

The command line and embedder now work identically when executing.

Show
Jason van Zyl added a comment - The command line and embedder now work identically when executing.
Hide
Eugene Kuleshov added a comment -

Jason, what embedder version or snapshot I could use?

Show
Eugene Kuleshov added a comment - Jason, what embedder version or snapshot I could use?
Hide
Stepan Roh added a comment -

I tried to use maven-embedder from SVN branch maven-2.0.x which claims to hold 2.0.5-SNAPSHOT, but that is not working. Where can I download working code which is interface-compatible with 2.0.4? 2.1 works, but it is too much refactored and I don't think there will be any release soon. I need something to try now and use official release in the near future. Thank you.

Show
Stepan Roh added a comment - I tried to use maven-embedder from SVN branch maven-2.0.x which claims to hold 2.0.5-SNAPSHOT, but that is not working. Where can I download working code which is interface-compatible with 2.0.4? 2.1 works, but it is too much refactored and I don't think there will be any release soon. I need something to try now and use official release in the near future. Thank you.

People

Vote (9)
Watch (12)

Dates

  • Created:
    Updated:
    Resolved: