Maven 2.x Release Plugin

Add dryRun flag to release:perform

Details

  • Type: Improvement Improvement
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 2.2.2
  • Fix Version/s: 2.3
  • Component/s: perform
  • Labels:
    None
  • Environment:
    mvn 3.0.3
  • Number of attachments :
    0

Description

Please consider to add the "dryRun" flag from the "release:prepare" to the "release:perform" goal too.

I have the following use case:
Jenkins/Hudson has a plugin called m2release which basically is there to help the user to trigger a maven release. This plugin allows to define a commandline which should be executed in case of a release build. The common configuration of this arguments are:

"-Dresume=false release:prepare release:perform"

the next version of the plugin would like to add a "dryRun" option when triggering a build and in this case it would just add "-DdryRun=true" to the configured arguments and thats it.
Unfortunately this will not work, as the "release:perform" still gets executed and will fail.
Of course I could also change the Jenkins/Hudson plugin, but this would mean I have to remove "release:perform" from the passed argument, which is relay ugly.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Olivier Lamy added a comment -

looks to be a simple skip of perform execution.
Others ?

Show
Olivier Lamy added a comment - looks to be a simple skip of perform execution. Others ?
Hide
Permalink
Robert Scholte added a comment -

@olamy, almost but not quite. Every ReleasePhase must implement the execute and simulate methods. Right now there's no option to pass the dryRun option on a perform-method of the ReleaseManager, so it should be added. Actually, the number of arguments is growing and growing, so I think we should create a ReleasePerformRequest as an argument container.

Show
Robert Scholte added a comment - @olamy, almost but not quite. Every ReleasePhase must implement the execute and simulate methods. Right now there's no option to pass the dryRun option on a perform-method of the ReleaseManager , so it should be added. Actually, the number of arguments is growing and growing, so I think we should create a ReleasePerformRequest as an argument container.
Hide
Permalink
Dominik Bartholdi added a comment -

for now I did implement a workaround on the jenkins plugin - but I still think this would be a good complement.

Show
Dominik Bartholdi added a comment - for now I did implement a workaround on the jenkins plugin - but I still think this would be a good complement.
Hide
Permalink
Olivier Lamy added a comment -

@Robert +1

Show
Olivier Lamy added a comment - @Robert +1
Hide
Permalink
Robert Scholte added a comment -

Fixed in rev. 1299068 for the plugin, the manager has already been prepared in previous commits.
There's only one thing which I'm not sure about: the perform-goals will be executed against the original project instead of the checked out project (that's good). But this also means that it will deploy (and often site-deploy) this SNAPSHOT-project. Should we try to catch these goals and replace it with verify and site respectively? Or add another option so you can choose? Prompt for it? Don't execute the goals, just say that it normally would? Or just leave it like this?

Show
Robert Scholte added a comment - Fixed in rev. 1299068 for the plugin, the manager has already been prepared in previous commits. There's only one thing which I'm not sure about: the perform-goals will be executed against the original project instead of the checked out project (that's good). But this also means that it will deploy (and often site-deploy ) this SNAPSHOT-project. Should we try to catch these goals and replace it with verify and site respectively? Or add another option so you can choose? Prompt for it? Don't execute the goals, just say that it normally would? Or just leave it like this?
Hide
Permalink
Dominik Bartholdi added a comment -

I would just say what it normally would do...
But there is one more thing which I think would be a good thing in a dryRun: check whether the credentials work e.g. check if the build would at least be allowed to connect to the location where the site should be deployed to.
This is an annoying thing in most of the dryRun implementations, because a dryRun should test as many of the preconditions as possible but most do not test whether the configured connections would actually work. Specially in this case: if the release works fine, but the deploy of the site would fail - then a release job would be marked as failed - but in fact the artifact was released, but just not the site which leafs the user in an unsure state: "what should I do now? start over and create another version/release?"

Show
Dominik Bartholdi added a comment - I would just say what it normally would do... But there is one more thing which I think would be a good thing in a dryRun: check whether the credentials work e.g. check if the build would at least be allowed to connect to the location where the site should be deployed to. This is an annoying thing in most of the dryRun implementations, because a dryRun should test as many of the preconditions as possible but most do not test whether the configured connections would actually work. Specially in this case: if the release works fine, but the deploy of the site would fail - then a release job would be marked as failed - but in fact the artifact was released, but just not the site which leafs the user in an unsure state: "what should I do now? start over and create another version/release?"
Hide
Permalink
Robert Scholte added a comment -

Fixed in rev. 1299865.
We've decided that the plugin will skip the execution of the perform goals. The tests have already run during the prepare and since we don't check out the code it would run twice.
For the authentication and authorization to deploy the artifacts or the site we might need other release-phases or separate goal.
Most important: it's now possible to do a mvn release:prepare release:perform -Ddryrun at once.

Show
Robert Scholte added a comment - Fixed in rev. 1299865 . We've decided that the plugin will skip the execution of the perform goals. The tests have already run during the prepare and since we don't check out the code it would run twice. For the authentication and authorization to deploy the artifacts or the site we might need other release-phases or separate goal. Most important: it's now possible to do a mvn release:prepare release:perform -Ddryrun at once.
Hide
Permalink
Dominik Bartholdi added a comment -

perfect, thanks Robert!

Show
Dominik Bartholdi added a comment - perfect, thanks Robert!
Hide
Permalink
Jesse Glick added a comment -

Perhaps I am missing something but rev. 1299865 seems incomplete. At least according to http://maven.apache.org/plugins/maven-release-plugin/perform-mojo.html#dryRun “[t]he goals (by default at least deploy) will be executed against the current project”. Is this just a matter of updating the Javadoc for PerformReleaseMojo.dryRun?

Show
Jesse Glick added a comment - Perhaps I am missing something but rev. 1299865 seems incomplete. At least according to http://maven.apache.org/plugins/maven-release-plugin/perform-mojo.html#dryRun “ [t] he goals (by default at least deploy ) will be executed against the current project”. Is this just a matter of updating the Javadoc for PerformReleaseMojo.dryRun ?
Hide
Permalink
Robert Scholte added a comment -

Jesse, what would you expect? Any suggestion to improve the javadocs is appreciated.

Show
Robert Scholte added a comment - Jesse, what would you expect? Any suggestion to improve the javadocs is appreciated.
Hide
Permalink
Jesse Glick added a comment -

@Robert your comment of Mar 12 suggests that this Javadoc is quite wrong, and that it should say something like:

“The deploy goal (or whatever is configured in goals) will not be executed.”

Show
Jesse Glick added a comment - @Robert your comment of Mar 12 suggests that this Javadoc is quite wrong, and that it should say something like: “The deploy goal (or whatever is configured in goals ) will not be executed.”
Hide
Permalink
Robert Scholte added a comment -

You're right, fixed in r1390723

Show
Robert Scholte added a comment - You're right, fixed in r1390723

People

  • Assignee:
    Robert Scholte
    Reporter:
    Dominik Bartholdi
Vote (0)
Watch (3)

Dates

  • Created:
    Updated:
    Resolved: