Ivy
  1. Ivy
  2. IVY-1363

ivy:buildlist task confused by extends feature using two parents

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.3.0-RC1
    • Fix Version/s: 2.3.0-RC2
    • Component/s: Ant, Core
    • Labels:
    • Environment:

      Ant 1.7.1 (but should be the same with Ant 1.8.3)

      Description

      I'm finding that the ivy:buildlist Ant task is erroring when it encounters more than one parent Ivy module that's pulled in through the /ivy-module/info/extends element. This problem is new to Ivy 2.3.0-rc1; I did not encounter it with Ivy 2.2.0. There is no relationship or interaction between the two different parent Ivy modules, i.e. no nesting of parents.

      In my test case, which I explain shortly, when I point the ivy:buildlist Ant task at a project stack that includes a mix of both parents and their children, I see this error:
      ...\multimodule-build\build.xml:28: impossible to parse ivy file for ...\testTwoParents\germany\build.xml: ivyfile=...\testTwoParents\germany\ivy.xml exception=java.text.ParseException: Problem occurred while parsing ivy file: inconsistent module descriptor file found in 'file:/.../testTwoParents/master-parent/ivy.xml': bad module name: expected='bootstrap-parent' found='master-parent'; in file:/.../testTwoParents/germany/ivy.xml

      What's happening is, the germany module extends bootstrap-parent, but somehow the relative path to master-parent/ivy.xml is supplanting the relative path to bootstrap-parent/ivy.xml. It appears buildlist doesn't know how to deal with more than one parent.

      This is what occurs when the haltOnError attribute is set to "true" on ivy:buildlist. If I set haltOnError="false" in my test case, the exception goes away, but I see the following build order:
      1. germany
      2. ireland
      3. bootstrap-parent
      4. master-parent
      5. croatia

      What's wrong about this build order is that germany depends on ireland and ireland depends on bootstrap-parent, so the order of the first three entries is reversed. If I removed the dependency of ireland on bootstrap-parent, the order would be the same. This misordering is clearly related to the presence of more than one parent because comparable tests using (A) the extends feature with a single parent and (B) no extends feature at all get the order right. Plus, I see this unexpected output when haltOnError="false":
      [ivy:buildlist] => adding it at the beginning of the path
      [ivy:buildlist] => adding it at the beginning of the path

      TEST CASE INSTRUCTIONS:
      I've attached a ZIP containing three standalone test cases, each consisting of a suite of Ant projects that together comprise a multimodule build whose build order is to be determined by the ivy:buildlist task:

      • testNoParents: The extends feature is not used.
      • testOneParent: The extends feature is used to pull in content from a single parent Ivy module.
      • testTwoParents: The extends feature is used where one Ivy module pulls in content from one parent and two other Ivy modules pull in content from a different parent.

      The testNoParents and testOneParent tests are the control groups. The testTwoParents test is where things fail.

      When running Ant from one of these test cases, you need to specify the location of the Ivy 2.3.0-rc1 installation using one of the following:

      • an IVY_DIR environment variable
      • an env.IVY_DIR user property, i.e. -Denv.IVY_DIR=...
      • an ivy.dir user property, i.e. -Divy.dir=...

      To run the build in any of these suites, go to the multimodule-build directory, and execute "ant" or "ant init"—while specifying the Ivy installation location. You can also run "ant cleancache" to clear out the Ivy cache. However, you shouldn't need to do this regularly because each of these test cases uses its own dedicated Ivy cache.

      NOTE: This issue is broached in the email thread "extends & buildlist on 2.3.0-rc1 … it gets worse" on the ivy-user and ant-dev mailing lists.

      1. ivy-trunk.patch
        10 kB
        Mitch Gitman
      2. ivy-2.3.x.patch
        10 kB
        Mitch Gitman
      3. IVY-1359-1363-1364-2.3.x.patch
        8 kB
        Mitch Gitman
      4. IVY-1347-1363-trunk.patch
        10 kB
        Mitch Gitman
      5. IVY-1347-1363-2.3.x.patch
        10 kB
        Mitch Gitman
      6. IVY-1347-1363.patch
        50 kB
        Mitch Gitman
      7. BuildlistAndExtendsIntegrationTest.zip
        24 kB
        Mitch Gitman

        Activity

        Hide
        Mitch Gitman added a comment -

        Per request, taking the previous patch I'd created and re-creating it against trunk. Plus, per request, correcting my mistake of letting Java formatting creep into the diff. As a result, the patch its about one-fifth the size of its previous incarnation. For reference's sake, also re-creating the patch against the 2.3.x branch, sans the formatting.

        The two attached files are:
        IVY-1347-1363-2.3.x.patch
        IVY-1347-1363-trunk.patch

        See previous comment from me for details.

        Show
        Mitch Gitman added a comment - Per request, taking the previous patch I'd created and re-creating it against trunk. Plus, per request, correcting my mistake of letting Java formatting creep into the diff. As a result, the patch its about one-fifth the size of its previous incarnation. For reference's sake, also re-creating the patch against the 2.3.x branch, sans the formatting. The two attached files are: IVY-1347 -1363-2.3.x.patch IVY-1347 -1363-trunk.patch See previous comment from me for details.
        Hide
        Nicolas Lalevée added a comment -

        Mitch, you patch is not easily readable, there is a lot of reformatting in there.
        Also, as you know, we are maintaining two branches, trunk and 2.3.x. The way we work is that we fix on the trunk, and merge back in the 2.3.x.
        Hence, it would be nice if you could provide a smaller patch against trunk.

        Show
        Nicolas Lalevée added a comment - Mitch, you patch is not easily readable, there is a lot of reformatting in there. Also, as you know, we are maintaining two branches, trunk and 2.3.x. The way we work is that we fix on the trunk, and merge back in the 2.3.x. Hence, it would be nice if you could provide a smaller patch against trunk.
        Hide
        Mitch Gitman added a comment -

        The attached patch addresses two separate sets of bugs:

        • One set that was introduced with some changes that were made to my series of patches for issues IVY-1359, IVY-1363, IVY-1364.
        • Another bug that went unaddressed by a solution to IVY-1347 that was a rationale for the preceding changes.

        KEEPING TRACK OF PARENT-SPECIFIC MODULE-INHERITANCE-REPOSITORY RESOLVER
        When Maarten Coene merged from trunk into the 2.3.x branch some otherwise worthwhile refinements to the work I'd done to have XmlModuleDescriptorParser correctly locate source parent modules on the filesystem, he removed the following line: IvyContext.getContext().getSettings().addResolver(parentModuleResolver);

        This line was necessary for the resolve-parent-from-source-location mechanism to work in a normal and predictable way. Without it, I would see error output like the following on a regular basis:
        [ivy:resolve] :: problems summary ::
        [ivy:resolve] :::: ERRORS
        [ivy:resolve] unknown resolver module-inheritance-repository-com.foo#bootstrap-parent;1.0
        [ivy:resolve] unknown resolver module-inheritance-repository-com.foo#bootstrap-parent;1.0
        [ivy:resolve] unknown resolver module-inheritance-repository-com.foo#bootstrap-parent;1.0

        At the risk of sounding like a broken record, as I'd noted before with a previous modification of this feature, it violates any reasonable contract to be reporting errors for non-error behaviors that happen in the normal course of things. Simple principle: crying wolf about nonexistent errors is itself an error.

        Worse, right after the error gets reported, something insidious happens. Because the caller, DefaultRepositoryCacheManager, can't find the correct resolver, it goes and either finds or creates another one:
        DependencyResolver resolver = settings.getResolver(resolverName);
        if (resolver == null) {
        Message.debug("/tresolver not found: " + resolverName
        + " => trying to use the one configured for " + mrid);
        resolver = settings.getResolver(depMD.getResolvedModuleRevisionId());

        This second resolver, a ChainResolver, is different from the original FileSystemResolver that was created. So the FileSystemResolver we go to the trouble to create in the method checkParentModuleOnFilesystem is never actually used to located the parent module.

        Therefore, I have restored the line that adds the parent-specific FileSystemResolver to IvySettings.

        Now, Maarten told me the reason he removed that line was that, with it still there, he would still have the StackOverflowError associated with IVY-1347:
        https://issues.apache.org/jira/browse/IVY-1347

        I was not able to reproduce this failure, even though I'm using Windows as Maarten is.

        CHCEKING FOR MISLEADING PARENT SOURCE LOCATION
        However, I noticed something suspicious about the particular test project that was attached to the bug. See:
        ivy-2.3.x/test/repositories/IVY-1347

        This test case contains two source modules, childtwo and childone. The ivy.xml for childtwo indicates that its parent is foo#parent. Since no location is specified, the default location, the ivy.xml one directory above, is used:
        <info organisation="foo" module="childtwo" revision="1.0" status="integration">
        <extends organisation="foo" module="parent" revision="1.0"/>
        </info>

        The problem is, the ivy.xml at that location is the one for the childone module, not parent. As a result, Ivy is getting tricked into treating childOne as the parent of childTwo when it's not.

        When you get into XmlModuleDescriptorParser.resolveParentFromModuleInheritanceRepository, the resolver named module-inheritance-repository-foo#parent;1.0 has for its only ivyPattern ivy-2.3.x/test/repositories/IVY-1347/childone/ivy.xml.

        Then, within the resolveParentFromModuleInheritanceRepository call, you get to the following line in Repository.findResourceUsingPattern:
        String resourceName = IvyPatternHelper.substitute(pattern, mrid, artifact);
        At this point, the values are:
        pattern = ivy-2.3.x/test/repositories/IVY-1347/childone/ivy.xml
        mrid = foo#parent;1.0
        artifact = foo#parent;1.0!ivy.xml(ivy)
        resourceName = ivy-2.3.x/test/repositories/IVY-1347/childone/ivy.xml

        You see the unkosher mixing of parent and childone. And findResourceUsingPattern indeed returns a ResolvedResource of:
        ivy-2.3.x/test/repositories/IVY-1347/childone/ivy.xml (1.0)

        Then in BasicResolver.getDependency, the ResolvedModuleResource rmr becomes foo#parent;1.0.

        My sense is the only reason this test isn't blowing up is that there's nothing in the parent to merge. I also believe that Maarten's experiencing the StackOverflowError had something to do with Ivy trying to resolve this invalid combination.

        Now, with the attached patch, Ivy ignores the false positive, and no purported source-located parent resolver gets added to the Ivy settings. Instead, the parent has to be found in the regular repository.

        To accomplish this, I've added a method isExpectedParentInLocation to XmlModuleDescriptorParser. If that method returns false, resolveParentFromModuleInheritanceRepository is not called.

        The isExpectedParentInLocation implementation relies on a new class, MinimalParentParser, that I've added as a static class in XmlModuleDescriptorParser. This parser parses just enough of the parent ivy.xml to find the organisation/module/revision combination. From here, the Parser in XmlModuleDescriptorParser checks if the ModuleRevisionId in the parent ivy.xml matches the ModuleRevisionId in the extends element of the child ivy.xml. This includes obtaining a VersionMatcher and calling accept on it, so that a desired revision like "latest.integration" in the extends will match something like "1.0" in the parent:
        VersionMatcher versionMatcher = settings.getVersionMatcher();
        if (versionMatcher.accept(expectedParentMrid, actualParentMrid))

        { return true; }

        This solution required adding a getVersionMatcher() method to the ParserSettings interface.

        NOTE: I've created this patch only for the 2.3.x branch, but not for trunk.

        Show
        Mitch Gitman added a comment - The attached patch addresses two separate sets of bugs: One set that was introduced with some changes that were made to my series of patches for issues IVY-1359 , IVY-1363 , IVY-1364 . Another bug that went unaddressed by a solution to IVY-1347 that was a rationale for the preceding changes. KEEPING TRACK OF PARENT-SPECIFIC MODULE-INHERITANCE-REPOSITORY RESOLVER When Maarten Coene merged from trunk into the 2.3.x branch some otherwise worthwhile refinements to the work I'd done to have XmlModuleDescriptorParser correctly locate source parent modules on the filesystem, he removed the following line: IvyContext.getContext().getSettings().addResolver(parentModuleResolver); This line was necessary for the resolve-parent-from-source-location mechanism to work in a normal and predictable way. Without it, I would see error output like the following on a regular basis: [ivy:resolve] :: problems summary :: [ivy:resolve] :::: ERRORS [ivy:resolve] unknown resolver module-inheritance-repository-com.foo#bootstrap-parent;1.0 [ivy:resolve] unknown resolver module-inheritance-repository-com.foo#bootstrap-parent;1.0 [ivy:resolve] unknown resolver module-inheritance-repository-com.foo#bootstrap-parent;1.0 At the risk of sounding like a broken record, as I'd noted before with a previous modification of this feature, it violates any reasonable contract to be reporting errors for non-error behaviors that happen in the normal course of things. Simple principle: crying wolf about nonexistent errors is itself an error. Worse, right after the error gets reported, something insidious happens. Because the caller, DefaultRepositoryCacheManager, can't find the correct resolver, it goes and either finds or creates another one: DependencyResolver resolver = settings.getResolver(resolverName); if (resolver == null) { Message.debug("/tresolver not found: " + resolverName + " => trying to use the one configured for " + mrid); resolver = settings.getResolver(depMD.getResolvedModuleRevisionId()); This second resolver, a ChainResolver, is different from the original FileSystemResolver that was created. So the FileSystemResolver we go to the trouble to create in the method checkParentModuleOnFilesystem is never actually used to located the parent module. Therefore, I have restored the line that adds the parent-specific FileSystemResolver to IvySettings. Now, Maarten told me the reason he removed that line was that, with it still there, he would still have the StackOverflowError associated with IVY-1347 : https://issues.apache.org/jira/browse/IVY-1347 I was not able to reproduce this failure, even though I'm using Windows as Maarten is. CHCEKING FOR MISLEADING PARENT SOURCE LOCATION However, I noticed something suspicious about the particular test project that was attached to the bug. See: ivy-2.3.x/test/repositories/ IVY-1347 This test case contains two source modules, childtwo and childone. The ivy.xml for childtwo indicates that its parent is foo#parent. Since no location is specified, the default location, the ivy.xml one directory above, is used: <info organisation="foo" module="childtwo" revision="1.0" status="integration"> <extends organisation="foo" module="parent" revision="1.0"/> </info> The problem is, the ivy.xml at that location is the one for the childone module, not parent. As a result, Ivy is getting tricked into treating childOne as the parent of childTwo when it's not. When you get into XmlModuleDescriptorParser.resolveParentFromModuleInheritanceRepository, the resolver named module-inheritance-repository-foo#parent;1.0 has for its only ivyPattern ivy-2.3.x/test/repositories/ IVY-1347 /childone/ivy.xml. Then, within the resolveParentFromModuleInheritanceRepository call, you get to the following line in Repository.findResourceUsingPattern: String resourceName = IvyPatternHelper.substitute(pattern, mrid, artifact); At this point, the values are: pattern = ivy-2.3.x/test/repositories/ IVY-1347 /childone/ivy.xml mrid = foo#parent;1.0 artifact = foo#parent;1.0!ivy.xml(ivy) resourceName = ivy-2.3.x/test/repositories/ IVY-1347 /childone/ivy.xml You see the unkosher mixing of parent and childone. And findResourceUsingPattern indeed returns a ResolvedResource of: ivy-2.3.x/test/repositories/ IVY-1347 /childone/ivy.xml (1.0) Then in BasicResolver.getDependency, the ResolvedModuleResource rmr becomes foo#parent;1.0. My sense is the only reason this test isn't blowing up is that there's nothing in the parent to merge. I also believe that Maarten's experiencing the StackOverflowError had something to do with Ivy trying to resolve this invalid combination. Now, with the attached patch, Ivy ignores the false positive, and no purported source-located parent resolver gets added to the Ivy settings. Instead, the parent has to be found in the regular repository. To accomplish this, I've added a method isExpectedParentInLocation to XmlModuleDescriptorParser. If that method returns false, resolveParentFromModuleInheritanceRepository is not called. The isExpectedParentInLocation implementation relies on a new class, MinimalParentParser, that I've added as a static class in XmlModuleDescriptorParser. This parser parses just enough of the parent ivy.xml to find the organisation/module/revision combination. From here, the Parser in XmlModuleDescriptorParser checks if the ModuleRevisionId in the parent ivy.xml matches the ModuleRevisionId in the extends element of the child ivy.xml. This includes obtaining a VersionMatcher and calling accept on it, so that a desired revision like "latest.integration" in the extends will match something like "1.0" in the parent: VersionMatcher versionMatcher = settings.getVersionMatcher(); if (versionMatcher.accept(expectedParentMrid, actualParentMrid)) { return true; } This solution required adding a getVersionMatcher() method to the ParserSettings interface. NOTE: I've created this patch only for the 2.3.x branch, but not for trunk.
        Hide
        Mitch Gitman added a comment -

        I'm communicating with Maarten Coene on this bug, but I can say that his recent related changes in trunk that were merged into the 2.3.x branch are producing errors on regular resolves. For example, I'm seeing something like the following whenever I do a build:

        [ivy:resolve] :: problems summary ::
        [ivy:resolve] :::: ERRORS
        [ivy:resolve] unknown resolver module-inheritance-repository-com.foo#master-parent;1.0
        [ivy:resolve] unknown resolver module-inheritance-repository-com.foo#master-parent;1.0
        [ivy:resolve]
        [ivy:resolve] :: USE VERBOSE OR DEBUG MESSAGE LEVEL FOR MORE DETAILS

        It is an error in itself to be reporting as errors non-error conditions that happen in the normal course of things. From my standpoint as the original bug reporter, I can say that this is not yet fixed or resolved.

        Show
        Mitch Gitman added a comment - I'm communicating with Maarten Coene on this bug, but I can say that his recent related changes in trunk that were merged into the 2.3.x branch are producing errors on regular resolves. For example, I'm seeing something like the following whenever I do a build: [ivy:resolve] :: problems summary :: [ivy:resolve] :::: ERRORS [ivy:resolve] unknown resolver module-inheritance-repository-com.foo#master-parent;1.0 [ivy:resolve] unknown resolver module-inheritance-repository-com.foo#master-parent;1.0 [ivy:resolve] [ivy:resolve] :: USE VERBOSE OR DEBUG MESSAGE LEVEL FOR MORE DETAILS It is an error in itself to be reporting as errors non-error conditions that happen in the normal course of things. From my standpoint as the original bug reporter, I can say that this is not yet fixed or resolved.
        Hide
        Nicolas Lalevée added a comment -

        patch applied, thank you for your feedback.
        And merged into 2.3.x

        Show
        Nicolas Lalevée added a comment - patch applied, thank you for your feedback. And merged into 2.3.x
        Hide
        Mitch Gitman added a comment -

        The attached patch contains changes to two Java classes: XmlModuleDescriptorParser, IvySettings.

        Fixing some breakage that was introduced with Jean-Louis's recent modification to the patch I had submitted to fix three bugs with the parent feature (via the ivy.xml extends element). The modification, as committed to Ivy trunk, was to no longer keep a special map in the IvyContext to find the special filesystem resolver for each unpublished parent ivy.xml; instead we can find each parent resolver by name from the map of resolvers belonging to the IvySettings. If XmlModuleDescriptorParser can't find the resolver there, it can add it.

        The problem was that XmlModuleDescriptorParser was calling IvySettings.getResolver(String resolverName), and this method logs an error if the resolver can't be found. However, there's nothing erroneous about the situation of looking up a parent resolver and then creating one if it's not there. Reporting errors where there are no errors is itself a bug.

        Below is the error output I was seeing (and in Eclipse seeing in red):
        [ivy:resolve] :::: ERRORS
        [ivy:resolve] unknown resolver module-inheritance-repository-com.uefa#bootstrap-parent;1.0

        Now, to check for the existence of the particular parent resolver (in the above case module-inheritance-repository-com.uefa#bootstrap-parent;1.0) XmlModuleDescriptorParser is using a new method in IvySettings, hasResolver(String resolverName).

        In addition, getting rid of an entire method in XmlModuleDescriptorParser that is superfluous now that every different parent must have its own resolver. That method is configureModuleInheritanceRepository(). This was creating a resolver called simply "module-inheritance-repository", but this single resolver for all parents is no longer used.

        NOTE: This change needs to be applied to the ivy-2.3.x branch as well as trunk, considering that it fixes breaking bugs in existing features.

        Show
        Mitch Gitman added a comment - The attached patch contains changes to two Java classes: XmlModuleDescriptorParser, IvySettings. Fixing some breakage that was introduced with Jean-Louis's recent modification to the patch I had submitted to fix three bugs with the parent feature (via the ivy.xml extends element). The modification, as committed to Ivy trunk, was to no longer keep a special map in the IvyContext to find the special filesystem resolver for each unpublished parent ivy.xml; instead we can find each parent resolver by name from the map of resolvers belonging to the IvySettings. If XmlModuleDescriptorParser can't find the resolver there, it can add it. The problem was that XmlModuleDescriptorParser was calling IvySettings.getResolver(String resolverName), and this method logs an error if the resolver can't be found. However, there's nothing erroneous about the situation of looking up a parent resolver and then creating one if it's not there. Reporting errors where there are no errors is itself a bug. Below is the error output I was seeing (and in Eclipse seeing in red): [ivy:resolve] :::: ERRORS [ivy:resolve] unknown resolver module-inheritance-repository-com.uefa#bootstrap-parent;1.0 Now, to check for the existence of the particular parent resolver (in the above case module-inheritance-repository-com.uefa#bootstrap-parent;1.0) XmlModuleDescriptorParser is using a new method in IvySettings, hasResolver(String resolverName). In addition, getting rid of an entire method in XmlModuleDescriptorParser that is superfluous now that every different parent must have its own resolver. That method is configureModuleInheritanceRepository(). This was creating a resolver called simply "module-inheritance-repository", but this single resolver for all parents is no longer used. NOTE: This change needs to be applied to the ivy-2.3.x branch as well as trunk, considering that it fixes breaking bugs in existing features.
        Hide
        Nicolas Lalevée added a comment -

        I have integrated your patch, modified regarding Jean-Louis comments.
        Before letting this into the 2.3.x branch, it would be nice if you can confirm it solves your issues. I didn't took the time to test it under Windows.

        Show
        Nicolas Lalevée added a comment - I have integrated your patch, modified regarding Jean-Louis comments. Before letting this into the 2.3.x branch, it would be nice if you can confirm it solves your issues. I didn't took the time to test it under Windows.
        Hide
        Mitch Gitman added a comment -

        Yes, this should work. And it's certainly preferable to keeping an extra map in the IvyContext. Thanks.

        What next? How will this change make its way into the 2.3.x branch and trunk?

        Show
        Mitch Gitman added a comment - Yes, this should work. And it's certainly preferable to keeping an extra map in the IvyContext. Thanks. What next? How will this change make its way into the 2.3.x branch and trunk?
        Hide
        Jean-Louis Boudart added a comment - - edited

        Then we don't need Map at all, and we could remove also the configureModuleInheritanceRepositoryMap method.

        How ?

        We could simply replace this call:

          if (!moduleInheritanceRepositoryMap.containsKey(urlString)) {
        

        by

         if (ivyContext.getSettings().getResolver(getModuleInheritanceRepositoryParentResolverName(parentMrid)) == null) {
        

        What do you think ?

        Show
        Jean-Louis Boudart added a comment - - edited Then we don't need Map at all, and we could remove also the configureModuleInheritanceRepositoryMap method. How ? We could simply replace this call: if (!moduleInheritanceRepositoryMap.containsKey(urlString)) { by if (ivyContext.getSettings().getResolver(getModuleInheritanceRepositoryParentResolverName(parentMrid)) == null ) { What do you think ?
        Hide
        Mitch Gitman added a comment - - edited

        Answer to Jean-Louis.

        You'll notice from my solution that I am indeed obtaining the particular parent resolver using IvySettings.getResolver. So it's true that there is no need to store the parent resolver in the map. You could just as well store the URL string being stored as the key as the value as well. Likewise, you could just as well store null as the value. The value is never used. The get method on the map is never being called--only the containsKey method.

        With that change, one can remove the comment.

        In summary, yes, you do need the map. You do not need to store the resolver in the map.

        Show
        Mitch Gitman added a comment - - edited Answer to Jean-Louis. You'll notice from my solution that I am indeed obtaining the particular parent resolver using IvySettings.getResolver. So it's true that there is no need to store the parent resolver in the map. You could just as well store the URL string being stored as the key as the value as well. Likewise, you could just as well store null as the value. The value is never used. The get method on the map is never being called--only the containsKey method. With that change, one can remove the comment. In summary, yes, you do need the map. You do not need to store the resolver in the map.
        Hide
        Jean-Louis Boudart added a comment -

        Mitch,
        I just checked your patches and think it definitively takes the good direction.
        You were right, there was lots of issue with how extends mechanism locates unpublished parents in the file system and as original author of this feature, i really apologize!

        I have question through according to following comment :

        // Do we even need to be adding this resolver to the Ivy settings considering that it's being placed in the map and not being used elsewhere?
        ivyContext.getSettings().addResolver(parentModuleResolver);
        

        I would ask the opposite question Do we really need to use a Map for this ?
        I'm not sure other pieces of code needs to use parent resolver (or at least i don't remember if ivy needs it), but it looks like more extendable for me to store it as a resolver.

        Even if it's only used for now inside XmlModuleDescriptorParser, we could imagine it could be used by reports to display resolver's name used while fetching artefacts.
        In Ivy, resolution is usually done by relying on resolvers, and locating a parent looks likes asking a resolver. That's why it was originally made like this.

        Now to get back to your patch i don't see any benefit to store it on a Map as it is not the way ivy consider resolver.

        Did i missed something?

        Show
        Jean-Louis Boudart added a comment - Mitch, I just checked your patches and think it definitively takes the good direction. You were right, there was lots of issue with how extends mechanism locates unpublished parents in the file system and as original author of this feature, i really apologize! I have question through according to following comment : // Do we even need to be adding this resolver to the Ivy settings considering that it's being placed in the map and not being used elsewhere? ivyContext.getSettings().addResolver(parentModuleResolver); I would ask the opposite question Do we really need to use a Map for this ? I'm not sure other pieces of code needs to use parent resolver (or at least i don't remember if ivy needs it), but it looks like more extendable for me to store it as a resolver. Even if it's only used for now inside XmlModuleDescriptorParser, we could imagine it could be used by reports to display resolver's name used while fetching artefacts. In Ivy, resolution is usually done by relying on resolvers, and locating a parent looks likes asking a resolver. That's why it was originally made like this. Now to get back to your patch i don't see any benefit to store it on a Map as it is not the way ivy consider resolver. Did i missed something?
        Hide
        Mitch Gitman added a comment - - edited

        Attaching the two patch files. See preceding comment.

        Show
        Mitch Gitman added a comment - - edited Attaching the two patch files. See preceding comment.
        Hide
        Mitch Gitman added a comment -

        I've come up with a solution to this problem. I'm attaching two patch files: one for the ivy-2.3.x branch, the other for trunk. These patch files contain the fixes for two other issues besides IVY-1363. Those are IVY-1359 and IVY-1364.

        There was indeed a fundamental problem with the way dev-only filesystem paths to parent Ivy modules were being resolved. There was just one Ivy resolver for all possible parents, where you would add an Ivy pattern to the resolver for each parent. Considering Ivy is doing pattern matching, the first parent would always be used, and the subsequently added ones would be ignored.

        Instead of having a single Ivy resolver with multiple Ivy patterns, there should be multiple Ivy resolvers, each with a single Ivy pattern--one resolver per parent path. I was able to implement this solution surgically in the XmlModuleDescriptorParser class.

        Now instead of a "module-inheritance-repository," there's a moduleInheritanceRepositoryMap where the key is the parent URL and the value (albeit not used) is the associated resolver. Since I couldn't store this map in IvySettings, I identified three choices as to where I could store it:

        • As a static field in XmlModuleDescriptorParser. This looked dangerous and appeared to subvert the purpose of IvyContext.
        • As "sticky" objects in the current IvyContext, placed there via IvyContext.push and retrieved via IvyContext.peek. This solution caused some obscure failures in IvyPublishTest, and I abandoned it.
        • As WeakReference objects in the current IvyContext, placed there via IvyContext.set and retrieved via IvyContext.get.

        I chose the third approach, with get and set. The only downside to this choice was that I found intermittent, nondeterministic failures to retrieve the Map object, apparently because the garbage collector had already gotten to the WeakReference. I was able to adjust for this by making an extra call to the new configureModuleInheritanceRepositoryMap from checkParentModuleOnFilesystem.

        I invite anyone to come up with a better way to store this map.

        Show
        Mitch Gitman added a comment - I've come up with a solution to this problem. I'm attaching two patch files: one for the ivy-2.3.x branch, the other for trunk. These patch files contain the fixes for two other issues besides IVY-1363 . Those are IVY-1359 and IVY-1364 . There was indeed a fundamental problem with the way dev-only filesystem paths to parent Ivy modules were being resolved. There was just one Ivy resolver for all possible parents, where you would add an Ivy pattern to the resolver for each parent. Considering Ivy is doing pattern matching, the first parent would always be used, and the subsequently added ones would be ignored. Instead of having a single Ivy resolver with multiple Ivy patterns, there should be multiple Ivy resolvers, each with a single Ivy pattern--one resolver per parent path. I was able to implement this solution surgically in the XmlModuleDescriptorParser class. Now instead of a "module-inheritance-repository," there's a moduleInheritanceRepositoryMap where the key is the parent URL and the value (albeit not used) is the associated resolver. Since I couldn't store this map in IvySettings, I identified three choices as to where I could store it: As a static field in XmlModuleDescriptorParser. This looked dangerous and appeared to subvert the purpose of IvyContext. As "sticky" objects in the current IvyContext, placed there via IvyContext.push and retrieved via IvyContext.peek. This solution caused some obscure failures in IvyPublishTest, and I abandoned it. As WeakReference objects in the current IvyContext, placed there via IvyContext.set and retrieved via IvyContext.get. I chose the third approach, with get and set. The only downside to this choice was that I found intermittent, nondeterministic failures to retrieve the Map object, apparently because the garbage collector had already gotten to the WeakReference. I was able to adjust for this by making an extra call to the new configureModuleInheritanceRepositoryMap from checkParentModuleOnFilesystem. I invite anyone to come up with a better way to store this map.
        Hide
        Mitch Gitman added a comment - - edited

        A follow-up to my previous comment.

        Just to illustrate how the pattern matching works in a normal scenario, consider a call to this method in RepositoryResolver:
        protected ResolvedResource findResourceUsingPattern(ModuleRevisionId mrid, String pattern,
        Artifact artifact, ResourceMDParser rmdparser, Date date)

        Imagine these more typcial arguments:
        mrid=org.apache.ant#ant;1.8.4
        pattern=C:\Users\me\.ivy2\local\integration[organisation][module][revision][artifact]-[revision].[ext]
        artifact=org.apache.ant#ant;1.8.4!ant.jar

        Then you get to this line:
        String resourceName = IvyPatternHelper.substitute(pattern, mrid, artifact);

        The value returned for resourceName is:
        C:\Users\me\.ivy2\local\integration\org.apache.ant\ant\1.8.4\ant-1.8.4.jar

        The indicator that this resolve failed is that there's no file at that path; the above file does not exist.

        But in the case of how RepositoryResolver is being used for module inheritance, the undesired path, master-parent/ivy.xml, DOES exist, when the path you really want is bootstrap-parent/ivy.xml. So the whole idea of pattern-matching is not an appropriate strategy, at least for unpublished parent Ivy modules, especially considering that the patterns being passed in are not even true patterns with substitute-able elements like [organisation] and [module]; they're just fully hard-coded paths (whether absolute or relative) like …/master-parent/ivy.xml and …/bootstrap-parent/ivy.xml that contain nothing to be substituted and thus completely ignore the mrid and artifact arguments to RepositoryResolver.findResourceUsingPattern.

        This is an indication to me that, for at least for locating unpublished parents, URLResolver (being based on RepositoryResolver and AbstractPatternsBasedResolver) is the entirely wrong Ivy resolver to be using. I'm still not sure about published parents.

        Show
        Mitch Gitman added a comment - - edited A follow-up to my previous comment. Just to illustrate how the pattern matching works in a normal scenario, consider a call to this method in RepositoryResolver: protected ResolvedResource findResourceUsingPattern(ModuleRevisionId mrid, String pattern, Artifact artifact, ResourceMDParser rmdparser, Date date) Imagine these more typcial arguments: mrid=org.apache.ant#ant;1.8.4 pattern=C:\Users\me\.ivy2\local\integration[organisation][module][revision][artifact]- [revision] . [ext] artifact=org.apache.ant#ant;1.8.4!ant.jar Then you get to this line: String resourceName = IvyPatternHelper.substitute(pattern, mrid, artifact); The value returned for resourceName is: C:\Users\me\.ivy2\local\integration\org.apache.ant\ant\1.8.4\ant-1.8.4.jar The indicator that this resolve failed is that there's no file at that path; the above file does not exist. But in the case of how RepositoryResolver is being used for module inheritance, the undesired path, master-parent/ivy.xml, DOES exist, when the path you really want is bootstrap-parent/ivy.xml. So the whole idea of pattern-matching is not an appropriate strategy, at least for unpublished parent Ivy modules, especially considering that the patterns being passed in are not even true patterns with substitute-able elements like [organisation] and [module] ; they're just fully hard-coded paths (whether absolute or relative) like …/master-parent/ivy.xml and …/bootstrap-parent/ivy.xml that contain nothing to be substituted and thus completely ignore the mrid and artifact arguments to RepositoryResolver.findResourceUsingPattern. This is an indication to me that, for at least for locating unpublished parents, URLResolver (being based on RepositoryResolver and AbstractPatternsBasedResolver) is the entirely wrong Ivy resolver to be using. I'm still not sure about published parents.
        Hide
        Mitch Gitman added a comment -

        I'm starting to believe that there's a serious flaw in how the whole extends/parent mechanism was written, or at least in how it locates unpublished parents in the filesystem. I don't have a sense yet whether the solution involves just introducing an override somewhere to provide the proper behavior or whether it requires a substantial refactoring.

        XmlModuleDescriptorParser, the class that parses ivy.xml files, creates its own URLResolver. At the point where the wrong parent ivy.xml file gets returned, the URLResolver's ivyPatterns list contains, in order:
        1. The path to master-parent/ivy.xml
        2. The path to bootstrap-parent/ivy.xml

        The desired ModuleRevisionId and Artifact is for bootstrap-parent. URLResolver.findResourceUsingPatterns (patterns plural) cycles through the different patterns, calling findResourceUsingPattern (pattern singular). Because master-parent/ivy.xml is the first pattern in the list, look what happens.

        Here's the passage from RepositoryResolver.findResourceUsingPattern:
        protected ResolvedResource findResourceUsingPattern(ModuleRevisionId mrid, String pattern,
        Artifact artifact, ResourceMDParser rmdparser, Date date) {
        String name = getName();
        VersionMatcher versionMatcher = getSettings().getVersionMatcher();
        try {
        if (!versionMatcher.isDynamic(mrid) || isAlwaysCheckExactRevision()) {
        String resourceName = IvyPatternHelper.substitute(pattern, mrid, artifact);
        ...

        Here:
        mrid=com.uefa#bootstrap-parent;1.0.0-SNAPSHOT
        artifact=com.uefa#bootstrap-parent;1.0.0-SNAPSHOT!ivy.xml(ivy)
        pattern=file:/.../BuildlistAndExtendsIntegrationTest/twoParents/master-parent/ivy.xml

        So the value that comes back for resourceName is:
        file:/.../BuildlistAndExtendsIntegrationTest/twoParents/master-parent/ivy.xml

        Clearly, this method should never be called with this combination. The question is how to preempt that.

        Show
        Mitch Gitman added a comment - I'm starting to believe that there's a serious flaw in how the whole extends/parent mechanism was written, or at least in how it locates unpublished parents in the filesystem. I don't have a sense yet whether the solution involves just introducing an override somewhere to provide the proper behavior or whether it requires a substantial refactoring. XmlModuleDescriptorParser, the class that parses ivy.xml files, creates its own URLResolver. At the point where the wrong parent ivy.xml file gets returned, the URLResolver's ivyPatterns list contains, in order: 1. The path to master-parent/ivy.xml 2. The path to bootstrap-parent/ivy.xml The desired ModuleRevisionId and Artifact is for bootstrap-parent. URLResolver.findResourceUsingPatterns (patterns plural) cycles through the different patterns, calling findResourceUsingPattern (pattern singular). Because master-parent/ivy.xml is the first pattern in the list, look what happens. Here's the passage from RepositoryResolver.findResourceUsingPattern: protected ResolvedResource findResourceUsingPattern(ModuleRevisionId mrid, String pattern, Artifact artifact, ResourceMDParser rmdparser, Date date) { String name = getName(); VersionMatcher versionMatcher = getSettings().getVersionMatcher(); try { if (!versionMatcher.isDynamic(mrid) || isAlwaysCheckExactRevision()) { String resourceName = IvyPatternHelper.substitute(pattern, mrid, artifact); ... Here: mrid=com.uefa#bootstrap-parent;1.0.0-SNAPSHOT artifact=com.uefa#bootstrap-parent;1.0.0-SNAPSHOT!ivy.xml(ivy) pattern= file:/.../BuildlistAndExtendsIntegrationTest/twoParents/master-parent/ivy.xml So the value that comes back for resourceName is: file:/.../BuildlistAndExtendsIntegrationTest/twoParents/master-parent/ivy.xml Clearly, this method should never be called with this combination. The question is how to preempt that.
        Hide
        Mitch Gitman added a comment -

        I saw the preceding comment by Eyad Ebrahim and decided to build the latest Ivy and try for myself. Having done so, I can say with confidence that Mr. Ebrahim is witnessing an "It works on my box" situation and this bug is not fixed.

        A few days ago I synced up with the Ivy 2.3.x branch and built that. Here's the output I got with haltOnError="false" on ivy:buildlist:

        -init.buildlist:
        [ivy:buildlist] :: Apache Ivy 2.3.0-rc1-dev-20120704194824 - 20120704194824 :: http://ant.apache.org/ivy/ ::
        [ivy:buildlist] :: loading settings :: file = ...\ivysettings-multimodule.xml
        [ivy:buildlist] => adding it at the beginning of the path
        [ivy:buildlist] => adding it at the beginning of the path
        [echo] Multimodule build order:
        germany\build.xml
        ireland\build.xml
        bootstrap-parent\build.xml
        master-parent\build.xml
        croatia\build.xml

        Same botched order. Same strange message: => adding it at the beginning of the path

        So then I synced up with the Ivy trunk and built that. Here's the output I got with that with haltOnError="false":

        -init.buildlist:
        [ivy:buildlist] :: Apache Ivy 2.4.0-dev-20120706211638 - 20120706211638 :: http://ant.apache.org/ivy/ ::
        [ivy:buildlist] :: loading settings :: file = ...\ivysettings-multimodule.xml
        [ivy:buildlist] => adding it at the beginning of the path
        [ivy:buildlist] => adding it at the beginning of the path
        [echo] Multimodule build order:
        germany\build.xml
        ireland\build.xml
        bootstrap-parent\build.xml
        master-parent\build.xml
        croatia\build.xml

        Precisely the same symptoms.

        Finally, I tried ivy:buildlist haltOnError="true" with my Ivy 2.3.x build. As expected, it failed:
        -init.buildlist:
        [ivy:buildlist] :: Apache Ivy 2.3.0-rc1-dev-20120704194824 - 20120704194824 :: http://ant.apache.org/ivy/ ::
        [ivy:buildlist] :: loading settings :: file = multimodule-build\ivysettings-multimodule.xml
        multimodule-build\build.xml:28: impossible to parse ivy file for germany\build.xml: ivyfile=germany\ivy.xml exception=java.text.ParseException: Problem occurred while parsing ivy file: inconsistent module descriptor file found in 'file:/master-parent/ivy.xml': bad module name: expected='bootstrap-parent' found='master-parent'; in file:/germany/ivy.xml
        at org.apache.ivy.ant.IvyBuildList.doExecute(IvyBuildList.java:215)
        at org.apache.ivy.ant.IvyTask.execute(IvyTask.java:277)
        ...
        Caused by: java.text.ParseException: Problem occurred while parsing ivy file: inconsistent module descriptor file found in 'file:/master-parent/ivy.xml': bad module name: expected='bootstrap-parent' found='master-parent'; in file:/germany/ivy.xml
        at org.apache.ivy.plugins.parser.xml.XmlModuleDescriptorParser$Parser.parse(XmlModuleDescriptorParser.java:301)
        ...
        Caused by: org.xml.sax.SAXException: Problem occurred while parsing ivy file: inconsistent module descriptor file found in 'file:/master-parent/ivy.xml': bad module name: expected='bootstrap-parent' found='master-parent';
        java.text.ParseException: inconsistent module descriptor file found in 'file:/master-parent/ivy.xml': bad module name: expected='bootstrap-parent' found='master-parent';
        at org.apache.ivy.plugins.parser.xml.XmlModuleDescriptorParser$Parser.startElement(XmlModuleDescriptorParser.java:386)
        ...
        Caused by: java.text.ParseException: inconsistent module descriptor file found in 'file:/master-parent/ivy.xml': bad module name: expected='bootstrap-parent' found='master-parent';
        ...

        So the problem evinces itself precisely the same way in the 2.3.0-rc1 release as well as the 2.3.x branch and trunk.

        I wouldn't be surprised if this is somehow related to the following two other bugs I've recently reported:
        https://issues.apache.org/jira/browse/IVY-1359
        https://issues.apache.org/jira/browse/IVY-1364

        Show
        Mitch Gitman added a comment - I saw the preceding comment by Eyad Ebrahim and decided to build the latest Ivy and try for myself. Having done so, I can say with confidence that Mr. Ebrahim is witnessing an "It works on my box" situation and this bug is not fixed. A few days ago I synced up with the Ivy 2.3.x branch and built that. Here's the output I got with haltOnError="false" on ivy:buildlist: -init.buildlist: [ivy:buildlist] :: Apache Ivy 2.3.0-rc1-dev-20120704194824 - 20120704194824 :: http://ant.apache.org/ivy/ :: [ivy:buildlist] :: loading settings :: file = ...\ivysettings-multimodule.xml [ivy:buildlist] => adding it at the beginning of the path [ivy:buildlist] => adding it at the beginning of the path [echo] Multimodule build order: germany\build.xml ireland\build.xml bootstrap-parent\build.xml master-parent\build.xml croatia\build.xml Same botched order. Same strange message: => adding it at the beginning of the path So then I synced up with the Ivy trunk and built that. Here's the output I got with that with haltOnError="false": -init.buildlist: [ivy:buildlist] :: Apache Ivy 2.4.0-dev-20120706211638 - 20120706211638 :: http://ant.apache.org/ivy/ :: [ivy:buildlist] :: loading settings :: file = ...\ivysettings-multimodule.xml [ivy:buildlist] => adding it at the beginning of the path [ivy:buildlist] => adding it at the beginning of the path [echo] Multimodule build order: germany\build.xml ireland\build.xml bootstrap-parent\build.xml master-parent\build.xml croatia\build.xml Precisely the same symptoms. Finally, I tried ivy:buildlist haltOnError="true" with my Ivy 2.3.x build. As expected, it failed: -init.buildlist: [ivy:buildlist] :: Apache Ivy 2.3.0-rc1-dev-20120704194824 - 20120704194824 :: http://ant.apache.org/ivy/ :: [ivy:buildlist] :: loading settings :: file = multimodule-build\ivysettings-multimodule.xml multimodule-build\build.xml:28: impossible to parse ivy file for germany\build.xml: ivyfile=germany\ivy.xml exception=java.text.ParseException: Problem occurred while parsing ivy file: inconsistent module descriptor file found in 'file:/master-parent/ivy.xml': bad module name: expected='bootstrap-parent' found='master-parent'; in file:/germany/ivy.xml at org.apache.ivy.ant.IvyBuildList.doExecute(IvyBuildList.java:215) at org.apache.ivy.ant.IvyTask.execute(IvyTask.java:277) ... Caused by: java.text.ParseException: Problem occurred while parsing ivy file: inconsistent module descriptor file found in 'file:/master-parent/ivy.xml': bad module name: expected='bootstrap-parent' found='master-parent'; in file:/germany/ivy.xml at org.apache.ivy.plugins.parser.xml.XmlModuleDescriptorParser$Parser.parse(XmlModuleDescriptorParser.java:301) ... Caused by: org.xml.sax.SAXException: Problem occurred while parsing ivy file: inconsistent module descriptor file found in 'file:/master-parent/ivy.xml': bad module name: expected='bootstrap-parent' found='master-parent'; java.text.ParseException: inconsistent module descriptor file found in 'file:/master-parent/ivy.xml': bad module name: expected='bootstrap-parent' found='master-parent'; at org.apache.ivy.plugins.parser.xml.XmlModuleDescriptorParser$Parser.startElement(XmlModuleDescriptorParser.java:386) ... Caused by: java.text.ParseException: inconsistent module descriptor file found in 'file:/master-parent/ivy.xml': bad module name: expected='bootstrap-parent' found='master-parent'; ... So the problem evinces itself precisely the same way in the 2.3.0-rc1 release as well as the 2.3.x branch and trunk. I wouldn't be surprised if this is somehow related to the following two other bugs I've recently reported: https://issues.apache.org/jira/browse/IVY-1359 https://issues.apache.org/jira/browse/IVY-1364
        Hide
        Eyad Ebrahim added a comment - - edited

        I think this one is solved now, after the last pull of code I have this output

        [echo] path\BuildlistAndExtendsIntegrationTest2\testTwoParents\bootstrap-parent\build.xml
        [echo] path\BuildlistAndExtendsIntegrationTest2\testTwoParents\master-parent\build.xml
        [echo] path\fixing.ivy.bug\BuildlistAndExtendsIntegrationTest2\testTwoParents\croatia\build.xml
        [echo] path\BuildlistAndExtendsIntegrationTest2\testTwoParents\ireland\build.xml
        [echo] path\BuildlistAndExtendsIntegrationTest2\testTwoParents\germany\build.xml

        Show
        Eyad Ebrahim added a comment - - edited I think this one is solved now, after the last pull of code I have this output [echo] path\BuildlistAndExtendsIntegrationTest2\testTwoParents\bootstrap-parent\build.xml [echo] path\BuildlistAndExtendsIntegrationTest2\testTwoParents\master-parent\build.xml [echo] path\fixing.ivy.bug\BuildlistAndExtendsIntegrationTest2\testTwoParents\croatia\build.xml [echo] path\BuildlistAndExtendsIntegrationTest2\testTwoParents\ireland\build.xml [echo] path\BuildlistAndExtendsIntegrationTest2\testTwoParents\germany\build.xml

          People

          • Assignee:
            Nicolas Lalevée
            Reporter:
            Mitch Gitman
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development