Ivy
  1. Ivy
  2. IVY-531

ivyconf: <classpath/> to accept complex path

    Details

    • Type: Improvement Improvement
    • Status: Open
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: Ant
    • Labels:
    • Environment:

      Any

      Description

      Eric:

      The ivyconf.xml classpath tag accepts file or url elements which seem capable of pointing to only a single classpath element, either a path or a jar. Can a full classpath be expressed?

      For instance, I would like to set an ant property called my.classpath and have it contain something like "a.jar:b.jar:c.jar". I'd like to use that property in my ivyconf.xml in a classpath element to set the classpath for resolver with complex dependencies

      Xavier:

      Sounds like a good idea, but this is not implemented yet. Open a jira issue.

      1. ivy-531-r644587.patch
        67 kB
        Jason Trump
      2. ivy-ant-classpath.patch
        53 kB
        Jason Trump

        Activity

        Hide
        Jason Trump added a comment - - edited

        very true, consistency is best. attached is a revised version of the patch that adds path support to Ivy standalone, with consistent behavior with or without Ant. the patch is against today's trunk, revision 644587.

        IvyClassLoaderManagerImpl has absorbed much code from IvyAntClassLoaderManager. The code is just a little trickier than what I had originally imagined, so I tried to add some more unit test coverage to make up for it.

        also: JIRA notifications seem to be back!

        Show
        Jason Trump added a comment - - edited very true, consistency is best. attached is a revised version of the patch that adds path support to Ivy standalone, with consistent behavior with or without Ant. the patch is against today's trunk, revision 644587. IvyClassLoaderManagerImpl has absorbed much code from IvyAntClassLoaderManager. The code is just a little trickier than what I had originally imagined, so I tried to add some more unit test coverage to make up for it. also: JIRA notifications seem to be back!
        Hide
        Xavier Hanin added a comment -

        I think I'd stick with pure compatibility between the two modes for the path / pathsep attributes. Leveraging Ant path is smart, but I think we'll quickly run into users not understanding why something work in a place and not in the other. If we really want to have support for any ant path syntax, I'd rather go with an 'antpath' attribute or something like that.
        Besides this everything sounds great!

        Show
        Xavier Hanin added a comment - I think I'd stick with pure compatibility between the two modes for the path / pathsep attributes. Leveraging Ant path is smart, but I think we'll quickly run into users not understanding why something work in a place and not in the other. If we really want to have support for any ant path syntax, I'd rather go with an 'antpath' attribute or something like that. Besides this everything sounds great!
        Hide
        Jason Trump added a comment - - edited

        So I was thinking about providing an updated patch, and would like to make sure it does what it should based on this discussion. How does this sound:

        Ivy standalone supports:

           <!-- complex path, system default separator is used when no 
                  pathsep is provided -->
           <classpath path="path/p1.jar:path/p2.jar"/>
        
           <!-- complex path, with an alternate path separator -->
           <classpath path="p1.jar~p2.jar~http://url/also/ok.jar" pathsep="~"/>
        

        Ivy + Ant supports:

           <!-- refid loads a named path when Ivy is embedded -->
           <classpath refid="some.path.id"/>
        
           <!-- complex path supports any path that Ant does.  
              I can't decide if this is a good idea or not.  Comments? 
           -->
           <classpath path="p1.jar;p2.jar:p3.jar"/>
        
           <!-- complex path with pathsep behaves exactly the same as standalone -->
           <classpath path="p1.jar~p2.jar" pathsep="~"/>
        
        Show
        Jason Trump added a comment - - edited So I was thinking about providing an updated patch, and would like to make sure it does what it should based on this discussion. How does this sound: Ivy standalone supports: <!-- complex path, system default separator is used when no pathsep is provided --> <classpath path="path/p1.jar:path/p2.jar"/> <!-- complex path, with an alternate path separator --> <classpath path="p1.jar~p2.jar~http://url/also/ok.jar" pathsep="~"/> Ivy + Ant supports: <!-- refid loads a named path when Ivy is embedded --> <classpath refid="some.path.id"/> <!-- complex path supports any path that Ant does. I can't decide if this is a good idea or not. Comments? --> <classpath path="p1.jar;p2.jar:p3.jar"/> <!-- complex path with pathsep behaves exactly the same as standalone --> <classpath path="p1.jar~p2.jar" pathsep="~"/>
        Hide
        Xavier Hanin added a comment -

        It seems we have a problem with our notification scheme, I'm currently investigating this...

        About path parsing, I think we can make something simple enough, with no automatic switching: just a split using a path separator which can be configured with an attribute sounds like a simple and good enough option.

        About the refid, I like your idea. So I'd be in favour of keeping your implementation for refid supported from Ant, and keep the door open for more implementations in other contexts.

        Show
        Xavier Hanin added a comment - It seems we have a problem with our notification scheme, I'm currently investigating this... About path parsing, I think we can make something simple enough, with no automatic switching: just a split using a path separator which can be configured with an attribute sounds like a simple and good enough option. About the refid, I like your idea. So I'd be in favour of keeping your implementation for refid supported from Ant, and keep the door open for more implementations in other contexts.
        Hide
        Jason Trump added a comment -

        sorry it has taken me so long to respond to your comments – i was waiting for a JIRA notification. I'll try to check back here more frequently.

        thanks for reviewing my patch. Only supporting complex paths from within Ant is definitely a major limitation. But when I looked at the amount of code that existed in Ant for parsing paths and building classloaders, I got nervous about the idea of adding so much without a good review. I'd also be worried about adding <classpath> to <ivy:settings> in a way that is different from <classpath> elements in other standard Ant tasks.

        I thought that we could add support for 'path' in IvyClassLoaderManagerImpl like in Xavier's first comment. Maybe it wouldn't have to support the full Ant syntax (no automatic switching between ';' and ':', for example)?

        Regarding refid: I thought in the future the "refid" syntax might be useful in more environments than just Ant. For example, when Ivy is used in Eclipse, refid could be used to refer to the name of an Eclipse User Libraries or project build paths.

        Interesting to note that Maarten's second bullet point is exactly what we are doing in our build system. We try to use Ivy to load its own plugins. Thus we don't have to put plugin jars in source control, or require developers to update them manually. It is also useful to have the plugin jars in the Ivy cache instead of using a URL classloader, so that remote developers don't have to be on VPN to run the build.

        Show
        Jason Trump added a comment - sorry it has taken me so long to respond to your comments – i was waiting for a JIRA notification. I'll try to check back here more frequently. thanks for reviewing my patch. Only supporting complex paths from within Ant is definitely a major limitation. But when I looked at the amount of code that existed in Ant for parsing paths and building classloaders, I got nervous about the idea of adding so much without a good review. I'd also be worried about adding <classpath> to <ivy:settings> in a way that is different from <classpath> elements in other standard Ant tasks. I thought that we could add support for 'path' in IvyClassLoaderManagerImpl like in Xavier's first comment. Maybe it wouldn't have to support the full Ant syntax (no automatic switching between ';' and ':', for example)? Regarding refid: I thought in the future the "refid" syntax might be useful in more environments than just Ant. For example, when Ivy is used in Eclipse, refid could be used to refer to the name of an Eclipse User Libraries or project build paths. Interesting to note that Maarten's second bullet point is exactly what we are doing in our build system. We try to use Ivy to load its own plugins. Thus we don't have to put plugin jars in source control, or require developers to update them manually. It is also useful to have the plugin jars in the Ivy cache instead of using a URL classloader, so that remote developers don't have to be on VPN to run the build.
        Hide
        Xavier Hanin added a comment -

        Allowing to specify the path separator sounds like the best option.

        Show
        Xavier Hanin added a comment - Allowing to specify the path separator sounds like the best option.
        Hide
        Maarten Coene added a comment -

        I agree with you Xavier, I was showing an alternative if you really wanted to use a path refid to load the plugins...

        I think the <classpath> element in <ivy:settings> could be usefull in the following situations:

        • you have different settings with different plugins (for instance, a resolve-only-settings with pluginA and a publish-settings with pluginB). It could be usefull sometimes to load the settings with different classpaths, especially if both plugins depend on different revisions of the same module (like pluginA depends on http-client-2.x en pluginB depends on http-client-3.x)
        • you use Ivy to resolve the plugins (and their dependencies). For instance, you could a kind of "bootstrap"-settings to resolve your plugins with ivy:cachepath, and with that classpath initialize another settings containing your plugins.
          But that's indeed out of the scope for this issue, and there is an easy workaround: just convert the Ant path into a ':' separated list (using the pathconvert task for instance) and pass that to Ivy when loading the settings.

        One thing we should take care of is the choice of the path-separator. Will we always take ':' as separator, or ';' or will we use the system default, or will we allow the user to specify the separator:

        <ivy:settings>
            <classpath path="path/jar1.jar:path/jar2.jar" pathsep=":" />
        </ivy:settings>
        
        Show
        Maarten Coene added a comment - I agree with you Xavier, I was showing an alternative if you really wanted to use a path refid to load the plugins... I think the <classpath> element in <ivy:settings> could be usefull in the following situations: you have different settings with different plugins (for instance, a resolve-only-settings with pluginA and a publish-settings with pluginB). It could be usefull sometimes to load the settings with different classpaths, especially if both plugins depend on different revisions of the same module (like pluginA depends on http-client-2.x en pluginB depends on http-client-3.x) you use Ivy to resolve the plugins (and their dependencies). For instance, you could a kind of "bootstrap"-settings to resolve your plugins with ivy:cachepath, and with that classpath initialize another settings containing your plugins. But that's indeed out of the scope for this issue, and there is an easy workaround: just convert the Ant path into a ':' separated list (using the pathconvert task for instance) and pass that to Ivy when loading the settings. One thing we should take care of is the choice of the path-separator. Will we always take ':' as separator, or ';' or will we use the system default, or will we allow the user to specify the separator: <ivy:settings> <classpath path= "path/jar1.jar:path/jar2.jar" pathsep= ":" /> </ivy:settings>
        Hide
        Xavier Hanin added a comment -

        What I like with jason implementation is that it doesn't disolve classpath information in multiple locations. Adding a classpath element to the settings task would be one more location to set the classpath, so I'm not sure to like it (as I said you can already set your classpath in ant by using the taskdef task).

        For supporting path with no dependency on Ant, I agree, and I think it's the original intent of this issue.

        Show
        Xavier Hanin added a comment - What I like with jason implementation is that it doesn't disolve classpath information in multiple locations. Adding a classpath element to the settings task would be one more location to set the classpath, so I'm not sure to like it (as I said you can already set your classpath in ant by using the taskdef task). For supporting path with no dependency on Ant, I agree, and I think it's the original intent of this issue.
        Hide
        Maarten Coene added a comment -

        I think we can support the path attribute, even when we run outside ant, it's just a separated list of single files as I see it. No Ant-related code needed for this attribute.

        The refid attribute is more Ant-related, so I'd rather keep it outside the settings. But perhaps we can add support for a classpath-element to the settings task like this:

        <ivy:settings ...>
            <classpath refid="my.ant.path" />
        </ivy:settings>
        

        or

        <ivy:settings ...>
            <classpath>
               <pathelement ... />
               <pathelement ... />
               <path refid="..." />
            </classpath>
        </ivy:settings>
        

        Maarten

        Show
        Maarten Coene added a comment - I think we can support the path attribute, even when we run outside ant, it's just a separated list of single files as I see it. No Ant-related code needed for this attribute. The refid attribute is more Ant-related, so I'd rather keep it outside the settings. But perhaps we can add support for a classpath-element to the settings task like this: <ivy:settings ...> <classpath refid= "my.ant.path" /> </ivy:settings> or <ivy:settings ...> <classpath> <pathelement ... /> <pathelement ... /> <path refid= "..." /> </classpath> </ivy:settings> Maarten
        Hide
        Xavier Hanin added a comment -

        I've quickly reviewed the patch, very good quality, good job. I've only one concern about this implementation: supporting 'path' attribute only when used from Ant is a strong limitation IMO. The classpath mechanism in Ivy settings is mainly used to be able to use plugins from any kind of Ivy use consistently (command line, Ant, IvyDE, ...). When used from Ant, you can use Ant path mechanism when loading Ivy itself, so that plugins are available to the classloader loading Ivy itself. So supporting refid is a nice addition for a better Ant integration, but I really believe this issue would better be supported if the path attribute was implemented in Ivy core, without relying on Ant. One could say that we could later do that, and provide a limited feature, but the problem is that we'll later need to support all Ant path syntax if we do it that way.

        So I see two options:

        • rename 'path' to 'antpath', to keep the 'path' attribute for a later implementation of what was really asked in this issue
        • implement 'path' in IvyClassLoaderManagerImpl

        WDYT?

        Show
        Xavier Hanin added a comment - I've quickly reviewed the patch, very good quality, good job. I've only one concern about this implementation: supporting 'path' attribute only when used from Ant is a strong limitation IMO. The classpath mechanism in Ivy settings is mainly used to be able to use plugins from any kind of Ivy use consistently (command line, Ant, IvyDE, ...). When used from Ant, you can use Ant path mechanism when loading Ivy itself, so that plugins are available to the classloader loading Ivy itself. So supporting refid is a nice addition for a better Ant integration, but I really believe this issue would better be supported if the path attribute was implemented in Ivy core, without relying on Ant. One could say that we could later do that, and provide a limited feature, but the problem is that we'll later need to support all Ant path syntax if we do it that way. So I see two options: rename 'path' to 'antpath', to keep the 'path' attribute for a later implementation of what was really asked in this issue implement 'path' in IvyClassLoaderManagerImpl WDYT?
        Hide
        Jason Trump added a comment -

        We needed this feature too, so I tried to implement it. Attached is a patch against today's trunk including test and documentation. What do you think?

        The patch adds two optional attributes to <classpath>, refid and path. The new attributes only work when Ivy is loaded from an Ant build script. For example,

        <ivysettings>

        <!-- references an ant path by id -->
        <classpath refid="my.ant.path"/>

        <!-- use a complex path expression -->
        <classpath path="path/jar1.jar:path/jar2.jar"/>

        ...

        </ivysettings>

        The 'url' and 'file' attributes still work exactly as they did before.

        Before you run the unit tests in the patch, you have to generate some new test jar files:

        ant build-custom-resolver-jar

        Show
        Jason Trump added a comment - We needed this feature too, so I tried to implement it. Attached is a patch against today's trunk including test and documentation. What do you think? The patch adds two optional attributes to <classpath>, refid and path. The new attributes only work when Ivy is loaded from an Ant build script. For example, <ivysettings> <!-- references an ant path by id --> <classpath refid="my.ant.path"/> <!-- use a complex path expression --> <classpath path="path/jar1.jar:path/jar2.jar"/> ... </ivysettings> The 'url' and 'file' attributes still work exactly as they did before. Before you run the unit tests in the patch, you have to generate some new test jar files: ant build-custom-resolver-jar

          People

          • Assignee:
            Unassigned
            Reporter:
            Eric Crahen
          • Votes:
            2 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:

              Development