Bug 48804

Summary:	import and extension-point / extensionOf not working as documented
Product:	Ant	Reporter:	Brian Repko <brianrepko>
Component:	Core	Assignee:	Ant Notifications List <notifications>
Status:	RESOLVED FIXED
Severity:	normal
Priority:	P2
Version:	1.8.0
Target Milestone:	1.8.1
Hardware:	PC
OS:	Windows XP
Attachments:	build file from documentation imported file from documentations changed ProjectHelper changed ProjectHelper2 working ProjectHelper2 working ProjectHelper Patch for AntUnit testfile for showing this bug Consolidated Brian's changes and Jan's testcase

Description Brian Repko 2010-02-23 20:20:02 UTC

As per the documentation on extension-points, there is a note that this is designed to be used with imported build files. I tried to create the example given (see attached files) where build.xml imports targets and an extension-point and then adds a dependency via the extensionOf.
 
This results in an error that the extension-point does not exist, which it doesn't because the import is done after the build is parsed.  I believe that I have the intent correct but that there is a bug in TargetHelper in ProjectHelper2.  I can see that the extensionOf needs to find the other target so that it can add a dependency but that that target doesn't exist since the import stack hasn't been fully popped.
 
I believe that the fix is to change ProjectHelper2 (I will try to create my own in the short term) to add the extension to a stack which then is processed after the import stack is done.  That is, all extensions should be done after the imports are done.

Comment 1 Brian Repko 2010-02-23 20:23:26 UTC

Created attachment 25043 [details]
build file from documentation

Comment 2 Brian Repko 2010-02-23 20:23:54 UTC

Created attachment 25044 [details]
imported file from documentations

Comment 3 Brian Repko 2010-02-23 20:24:41 UTC

Created attachment 25045 [details]
changed ProjectHelper

Comment 4 Brian Repko 2010-02-23 20:25:05 UTC

Created attachment 25046 [details]
changed ProjectHelper2

Comment 5 Matt Benson 2010-02-23 20:36:37 UTC

Hi, Brian, and thanks for the submission.  Typically the Ant developers are much better able to digest the intent of a change if it is provided as a patch against the svn trunk code.  When you attach your changes in this format, you should also edit the title of the issue so that it begins with [PATCH].

Regards,
Matt

Comment 6 Brian Repko 2010-02-23 20:43:09 UTC

Just tried these against a different scenario and it fails - please don't use these (I can't delete them).

I didn't find any quick setup guide so that I could provide you a patch (I can't get the source bundle to not report errors in Eclipse) so you probably won't get that from me.  I'm assuming you can do a diff.

Thanks!

Comment 7 Brian Repko 2010-02-23 21:20:04 UTC

Created attachment 25047 [details]
working ProjectHelper2

Comment 8 Brian Repko 2010-02-23 21:20:32 UTC

Created attachment 25048 [details]
working ProjectHelper

Comment 9 Robert E. Menteer 2010-02-26 03:16:41 UTC

I'm having this same problem and have found a work around. If your build files look like this:
<project name="imported">
  <extension-point name="extend-this"/>
</project>

<project name="importing">
  <import file="imported"/>
  <target name="extending" extentionOf="extend-this"/>
</project>

Ant will complain that extend-this does not exist. By wrapping these build files in another Ant will not complain. The initial build files do not need to be modified so the wrapper files can be deleted when this bug is fixed.

<project name="wrapper">
  <import file="imported"/>
  <import file="importing"/>
</project>

Comment 10 Jan Mat 2010-03-20 13:08:00 UTC

Created attachment 25155 [details]
Patch for AntUnit testfile for showing this bug

Attached a path for the AntUnit test.
- create imported buildfile (defining extension-point)
- create importing buildfile (enhancing the EP)
- invoking the importing file
- checks for the expected output

Comment 11 Stefan Bodewig 2010-04-23 05:11:31 UTC

Jan, your test would even fail if there was no bug since you never call any target 8-)

I've a modified version of the patch just undergoing ants unit test suite, it breaks at least one test: testExtensionPointMustBeKnown.  This on asserts you can't extend an extension point that is defined later inside the same build file, which is no longer true.

I'll poll the dev list to see whether this change is more of a benefit than a problem.

Comment 12 Stefan Bodewig 2010-04-23 05:18:58 UTC

Created attachment 25340 [details]
Consolidated Brian's changes and Jan's testcase

I have modified Brian's original change to use a linked list rather than a
vector and a two-element string array rather than a map, but these changes are
really minor.

Comment 13 Stefan Bodewig 2010-04-27 00:07:22 UTC

fixed with svn revision 938315

Thanks