Maven Shared Components

Add stricter pattern filters to maven-common-artifact-filters

Details

  • Type: Improvement Improvement
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: maven-common-artifact-filters
  • Labels:
    None
  • Number of attachments :
    2

Description

There is a todo in http://svn.apache.org/viewvc/maven/components/trunk/maven-artifact/src/main/java/org/apache/maven/artifact/resolver/filter/IncludesArtifactFilter.java?view=markup to add regex. I checked all the sources and could only find usages of this code by maven-assembly-plugin, webstart-maven-plugin and exec-maven-plugin. The latter two are in mojo.

If you look at http://svn.palle.net/projects/hauskeeper/hauskeeper-server/src/assemblies/debian.xml, Trygvis is assuming that wildcards work, when in fact they do not. Arguably, this is a documentation bug that it does not work.

The attached patch fixes this problem.

  1. Text File
    IncludesArtifactFilter.patch
    19/Oct/06 6:10 PM
    1.0 kB
    Brian Topping
  2. Text File
    MNG-2621.patch
    15/Jun/07 6:51 AM
    13 kB
    Mark Hobson

Issue Links

relates to

Improvement - An improvement or enhancement to an existing feature or task. MWEBSTART-45 Reuse artifcat filters that understand patterns

  • Minor - Minor loss of function, or other problem where easy workaround is present.
  • Open - The issue is open and ready for the assignee to start work on it.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Jerome Lacoste added a comment -

It would be nice if you could add a unit test. Then +1 from me

Show
Jerome Lacoste added a comment - It would be nice if you could add a unit test. Then +1 from me
Hide
Permalink
Jason van Zyl added a comment -

Not allowing wildcard filters. Powerful but could potentially wreak havoc.

Show
Jason van Zyl added a comment - Not allowing wildcard filters. Powerful but could potentially wreak havoc.
Hide
Permalink
Brett Porter added a comment -

sounds like you meant won't fix

Show
Brett Porter added a comment - sounds like you meant won't fix
Hide
Permalink
Mark Hobson added a comment -

Attaching alternative patch as discussed in:
http://www.mail-archive.com/dev@maven.apache.org/msg67727.html

Show
Mark Hobson added a comment - Attaching alternative patch as discussed in: http://www.mail-archive.com/dev@maven.apache.org/msg67727.html
Hide
Permalink
Mark Hobson added a comment -

For future-reference, wildcard filters are available in the maven-common-artifact-filters shared component:
http://svn.apache.org/repos/asf/maven/shared/trunk/maven-common-artifact-filters/

Show
Mark Hobson added a comment - For future-reference, wildcard filters are available in the maven-common-artifact-filters shared component: http://svn.apache.org/repos/asf/maven/shared/trunk/maven-common-artifact-filters/
Hide
Permalink
John Casey added a comment -

I've checked out the attached patch, and it looks good for the most part. There are a couple of small logical and other errors:

1. short-circuit on tokens vs. patterns length doesn't actually short circuit
2. String.contains() is @since 1.5, so we need to switch to indexOf(..) > -1

We could probably add this as a new class to the common-artifact-filters project, called something like StrictPatternIncludesFilter

Better yet, we could add a new boolean parameter to the constructor, and allow this filter to be used as inclusion or exclusion by simply reversing the output when the constructor flag == false.

Mark, do you have access to add this new filter to the maven-common-artifact-filters project directly?

Also, note MASSEMBLY-180 when dealing with common-artifact-filters, as it looks like something is slightly broken in there (I haven't had time to go over it yet).

Show
John Casey added a comment - I've checked out the attached patch, and it looks good for the most part. There are a couple of small logical and other errors: 1. short-circuit on tokens vs. patterns length doesn't actually short circuit 2. String.contains() is @since 1.5, so we need to switch to indexOf(..) > -1 We could probably add this as a new class to the common-artifact-filters project, called something like StrictPatternIncludesFilter Better yet, we could add a new boolean parameter to the constructor, and allow this filter to be used as inclusion or exclusion by simply reversing the output when the constructor flag == false. Mark, do you have access to add this new filter to the maven-common-artifact-filters project directly? Also, note MASSEMBLY-180 when dealing with common-artifact-filters, as it looks like something is slightly broken in there (I haven't had time to go over it yet).
Hide
Permalink
John Casey added a comment -

I'll follow this up, to make sure it doesn't linger. Assigning to myself.

Show
John Casey added a comment - I'll follow this up, to make sure it doesn't linger. Assigning to myself.
Hide
Permalink
Mark Hobson added a comment -

Hi John, sure I can add this to maven-common-artifact-filters directly. I'll assign this to myself, refactor the code as mentioned above and commit, if that's okay? I'm not sure I understand you regarding point (1), the for-loop will be short-circuited since matched will be false.

Show
Mark Hobson added a comment - Hi John, sure I can add this to maven-common-artifact-filters directly. I'll assign this to myself, refactor the code as mentioned above and commit, if that's okay? I'm not sure I understand you regarding point (1), the for-loop will be short-circuited since matched will be false.
Hide
Permalink
Mark Hobson added a comment -

Updated issue title to reflect current direction.

Show
Mark Hobson added a comment - Updated issue title to reflect current direction.
Hide
Permalink
Mark Hobson added a comment -

Applied patch to maven-common-artifact-filters with John's suggestions: new filters are called StrictPatternIncludesArtifactFilter and StrictPatternExcludesArtifactFilter.

Show
Mark Hobson added a comment - Applied patch to maven-common-artifact-filters with John's suggestions: new filters are called StrictPatternIncludesArtifactFilter and StrictPatternExcludesArtifactFilter.
Hide
Permalink
John Casey added a comment -

my apologies, didn't read the patch closely enough.

Show
John Casey added a comment - my apologies, didn't read the patch closely enough.

People

  • Assignee:
    Mark Hobson
    Reporter:
    Brian Topping
Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: