Ivy
  1. Ivy
  2. IVY-1142

ivy:retrieve sync="true" does nothing if first variable is optional

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.1.0
    • Fix Version/s: 2.2.0-RC1
    • Component/s: Core
    • Labels:
      None
    • Environment:

      Not relevant

      Description

      if the ivy.retrieve.pattern contains an optional parameter as the first parameter, the sync feature of ivy:retrieve will get a path with a trailing '(' causing it to find nothing to sync, leaving old files still in the retrieve path.

      Example:
      ivy.retrieve.pattern = lib/([type]/)/[artifact].[ext]
      sync will try to match orphaned files against lib/( which obviously contains nothing.

      The problem is in IvyPatternHelper.java getTokenRoot(), which only checks for the first '[', but in the pattern above it needs to check for the first '('.

      I could see that nothing but the Resolver uses this function so it looks like it'll be pretty safe to just fix the check. I've attached a suggested fix below. I rarely touch java, so there might be simpler ways to do it, but at least it works.

      diff -r 616a4e764dd1 src/java/org/apache/ivy/core/IvyPatternHelper.java
      --- a/src/java/org/apache/ivy/core/IvyPatternHelper.java        Fri Nov 20 11:38:53 2009 +0100
      +++ b/src/java/org/apache/ivy/core/IvyPatternHelper.java        Fri Nov 20 12:54:27 2009 +0100
      @@ -472,7 +472,15 @@
           }
      
           public static String getTokenRoot(String pattern) {
      -        int index = pattern.indexOf('[');
      +        int[] delimiters = {'[', '('};
      +        for (int index = 0; index < delimiters.length; ++index) {
      +            pattern = getTokenRoot(pattern, delimiters[index]);
      +        }
      +        return pattern;
      +    }
      +
      +    private static String getTokenRoot(String pattern, int delimiter) {
      +        int index = pattern.indexOf(delimiter);
               if (index == -1) {
                   return pattern;
               } else {
      
      1. IVY-1142.patch
        0.8 kB
        Andreas Axelsson

        Activity

        Hide
        Andreas Axelsson added a comment -

        improved summary and description

        Show
        Andreas Axelsson added a comment - improved summary and description
        Hide
        Maarten Coene added a comment -

        Could you attach your patch as an attachment and check the "grant license to ASF" option?

        Thanks,
        Maarten

        Show
        Maarten Coene added a comment - Could you attach your patch as an attachment and check the "grant license to ASF" option? Thanks, Maarten
        Hide
        Andreas Axelsson added a comment -

        The patch from the description

        Show
        Andreas Axelsson added a comment - The patch from the description
        Hide
        Maarten Coene added a comment -

        I've updated IvyPatternHelper so your problem should be fixed.
        Could you give it a try?

        thanks!
        Maarten

        Show
        Maarten Coene added a comment - I've updated IvyPatternHelper so your problem should be fixed. Could you give it a try? thanks! Maarten
        Hide
        Andreas Axelsson added a comment -

        Thanks, it's working fine!

        I see that your code avoids stripping at left parenthesis when there are no variables in the path, which was a better solution than mine. Perhaps it should be detected only if the brackets are actually enclosed by the parenthesis, but it'd have to be checked against how the pattern substitution code deals with the same case I guess. Not that using () in foldernames is a best practice IMHO, but someone might want it.

        The following root will be correct now: ([optional] being empty)
        /foo/([optional]/)bar -> /foo/
        /foo/[module]/bar -> /foo/
        /foo/(subdir)/bar -> /foo/(subdir)/bar

        This won't:
        /(foo)/[module]/bar -> /

        Show
        Andreas Axelsson added a comment - Thanks, it's working fine! I see that your code avoids stripping at left parenthesis when there are no variables in the path, which was a better solution than mine. Perhaps it should be detected only if the brackets are actually enclosed by the parenthesis, but it'd have to be checked against how the pattern substitution code deals with the same case I guess. Not that using () in foldernames is a best practice IMHO, but someone might want it. The following root will be correct now: ( [optional] being empty) /foo/( [optional] /)bar -> /foo/ /foo/ [module] /bar -> /foo/ /foo/(subdir)/bar -> /foo/(subdir)/bar This won't: /(foo)/ [module] /bar -> /

          People

          • Assignee:
            Maarten Coene
            Reporter:
            Andreas Axelsson
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development