Maven 2.x Release Plugin

Allow File/Directory Patterns for the checkModificationExcludes Option

Details

  • Type: Improvement Improvement
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: 2.1
  • Fix Version/s: 2.3
  • Component/s: branch, prepare, scm
  • Labels:
    None
  • Environment:
    all
  • Testcase included:
    yes
  • Patch Submitted:
    Yes
  • Number of attachments :
    5

Description

The checkModificationExcludes option does currently only allow the definition single files to be excluded from the SCM modification check. If this option is defined, all files anywhere in the maven project structure with the specified name will be excluded from the check. It is currently not possible to exclude files only within a specific directory or to exclude classes of files, i.e. all files matching a specific file name pattern.
If the checkModificationExcludes option allowed the definition of file and directory patterns, these things would be possible.

Example 1: I'd like to exclude a test resource src/test/resources/foo.properties from the modification check but the real foo.properties in src/main/resources should still be checked.

 
<checkModificationExcludes>
  <checkModificationExclude>src/test/resources/foo.properties</checkModificationExclude>
</checkModificationExcludes>

Example 2: I'd like to exclude all properties files with the prefix bar from the modification check:

 
<checkModificationExcludes>
  <checkModificationExclude>**/bar*.properties</checkModificationExclude>
</checkModificationExcludes>

The attached patch modifies the ScmCheckModificationsPhase to use the DirectoryScanner from plexus-utils instead of doing a strict file name comparison. The patch does not provide more unit tests for this feature but it adjusts the existing tests to run without any failures.

  1. Text File
    maven-release-manager-r1305146.patch
    07/Apr/12 11:40 AM
    8 kB
    Stefan Ferstl
  2. Text File
    maven-release-plugin-it.patch
    07/Apr/12 11:40 AM
    13 kB
    Stefan Ferstl
  3. Text File
    maven-release-plugin-it-with-scm-provider.patch
    10/Apr/12 2:29 PM
    17 kB
    Stefan Ferstl
  4. Text File
    modification-excludes.patch
    06/Feb/11 1:47 PM
    9 kB
    Stefan Ferstl
  5. Text File
    MRELEASE-645.patch
    10/Apr/12 12:28 PM
    9 kB
    Robert Scholte

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Mike Whittemore added a comment -

I would very much like to see the suggested improvements. Plus they would make this plugin's configuration consistent with most other plugins.

Show
Mike Whittemore added a comment - I would very much like to see the suggested improvements. Plus they would make this plugin's configuration consistent with most other plugins.
Hide
Permalink
James Levinson added a comment -

What are the steps to get this patch into the 2.3 pipeline ?

Show
James Levinson added a comment - What are the steps to get this patch into the 2.3 pipeline ?
Hide
Permalink
Robert Scholte added a comment -

James,

I've got a couple of problems with the current patch:

  • it has merge conflicts since there has been some refactoring since it was created. But this shouldn't be too hard to fix.
  • I'd prefer a different test. Now it is based on a stub. What I would like to see is either a testcase based on Mocks (we're using Mockito) or an integration-test (just check out the code, there are enough examples).
    So the base looks good, but it we need better tests. So you could help by fixing the patch you could help us a lot and I can apply it for the 2.3 release.
Show
Robert Scholte added a comment - James, I've got a couple of problems with the current patch: it has merge conflicts since there has been some refactoring since it was created. But this shouldn't be too hard to fix. I'd prefer a different test. Now it is based on a stub. What I would like to see is either a testcase based on Mocks (we're using Mockito) or an integration-test (just check out the code, there are enough examples). So the base looks good, but it we need better tests. So you could help by fixing the patch you could help us a lot and I can apply it for the 2.3 release.
Hide
Permalink
Stefan Ferstl added a comment - - edited

I added two new patches to this issue. The first one does the same code modifications based on the latest trunk revision of the maven-release-manager (r1305146). The second adds a new integration test to the maven-release-plugin. The integration test checks the build.log file for the correct set of excluded files.
Unfortunately, I didn't find a better solution for the ScmCheckModificationsPhaseTest. The ScmCheckModificationsPhase is retrieved via Plexus but the getExcludedFiles() method does not work in the unit test because the files to be excluded do not exist.

Show
Stefan Ferstl added a comment - - edited I added two new patches to this issue. The first one does the same code modifications based on the latest trunk revision of the maven-release-manager (r1305146). The second adds a new integration test to the maven-release-plugin. The integration test checks the build.log file for the correct set of excluded files. Unfortunately, I didn't find a better solution for the ScmCheckModificationsPhaseTest . The ScmCheckModificationsPhase is retrieved via Plexus but the getExcludedFiles() method does not work in the unit test because the files to be excluded do not exist.
Hide
Permalink
Robert Scholte added a comment -

I've rewritten the patch and replaced the DirectoryScanner with the SelectorUtils which seems to be a better fit. It should require less IO, because it doesn't have to walk through all the folders on the system, it will only do some ant-pattern-matches.
With this the Mocks can be removed, but the IT will fail, because the match should be done on the status-result of the SCM. I haven't found a way to resolve this yet.
So here's a new patch to start and test with.

Show
Robert Scholte added a comment - I've rewritten the patch and replaced the DirectoryScanner with the SelectorUtils which seems to be a better fit. It should require less IO, because it doesn't have to walk through all the folders on the system, it will only do some ant-pattern-matches. With this the Mocks can be removed, but the IT will fail, because the match should be done on the status-result of the SCM. I haven't found a way to resolve this yet. So here's a new patch to start and test with.
Hide
Permalink
Stefan Ferstl added a comment -

I modified the integration test so that it uses a dummy scm provider (src/it/setup/maven-scm-provider-mrelease-645). The status() method of this scm provider indicates modified files that are excluded in the POM of the integration test.
What is still missing, is a negative integration test which fails when files are modified but not excluded. However, I haven't figured out yet how to write such a test (i.e. how to configure the invoker-plugin to consider a build failure as success).
Here is the new patch (maven-release-plugin-it-with-scm-provider.patch).

Show
Stefan Ferstl added a comment - I modified the integration test so that it uses a dummy scm provider (src/it/setup/maven-scm-provider-mrelease-645). The status() method of this scm provider indicates modified files that are excluded in the POM of the integration test. What is still missing, is a negative integration test which fails when files are modified but not excluded. However, I haven't figured out yet how to write such a test (i.e. how to configure the invoker-plugin to consider a build failure as success). Here is the new patch (maven-release-plugin-it-with-scm-provider.patch).
Hide
Permalink
Stefan Ferstl added a comment -

new patch as described before

Show
Stefan Ferstl added a comment - new patch as described before
Hide
Permalink
Robert Scholte added a comment -

To let the invoker-plugin check for a build failure you should add a invoker.properties with invoker.buildResult = failure (see http://maven.apache.org/plugins/maven-invoker-plugin/run-mojo.html#invokerPropertiesFile)
After going through the SCM-code, it looks like a consumer is used to read the output and to filter out the files. I'd hoped that an ScmFile.getPath() always returns me a path relative to the working directory, but it looks like it cannot guarantee that. In that case it doesn't matter if there's an IT.
I'll try to figure out what different SCM's can return and see if I should add some adjustments.

Show
Robert Scholte added a comment - To let the invoker-plugin check for a build failure you should add a invoker.properties with invoker.buildResult = failure (see http://maven.apache.org/plugins/maven-invoker-plugin/run-mojo.html#invokerPropertiesFile ) After going through the SCM-code, it looks like a consumer is used to read the output and to filter out the files. I'd hoped that an ScmFile.getPath() always returns me a path relative to the working directory, but it looks like it cannot guarantee that. In that case it doesn't matter if there's an IT. I'll try to figure out what different SCM's can return and see if I should add some adjustments.
Hide
Permalink
Robert Scholte added a comment -

Fixed in r1325475 and r1325481

Show
Robert Scholte added a comment - Fixed in r1325475 and r1325481

People

  • Assignee:
    Robert Scholte
    Reporter:
    Stefan Ferstl
Vote (12)
Watch (3)

Dates

  • Created:
    Updated:
    Resolved: