Uploaded image for project: 'Ivy'
  1. Ivy
  2. IVY-1142

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

Details

    • Bug
    • Status: Resolved
    • Major
    • Resolution: Fixed
    • 2.1.0
    • 2.2.0-RC1
    • Core
    • None
    • 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 {
      

      Attachments

        1. IVY-1142.patch
          0.8 kB
          Andreas Axelsson

        Activity

          improved summary and description

          axl Andreas Axelsson added a comment - improved summary and description
          maartenc Maarten Coene added a comment -

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

          Thanks,
          Maarten

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

          The patch from the description

          axl Andreas Axelsson added a comment - The patch from the description
          maartenc Maarten Coene added a comment -

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

          thanks!
          Maarten

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

          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 -> /

          axl 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

            maartenc Maarten Coene
            axl Andreas Axelsson
            Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: