Comment 1 Michael Schnell 2020-03-03 08:04:52 EST

Note that running ANT using the external tool launchers still works without issues.

Comment 2 Andrey Loskutov 2020-03-03 10:30:25 EST

I can't reproduce (build I20200302-0600, Linux, Java 8).
 
What I see is that the ant editor shows an error on the first line "Default target deploy does not exist in this project".

The NPE in question is coming from the code added for bug 412809, but this is a valid code that assumes that there is a build file related to current XML:

String buildFilePath = context.getBuildFile().getAbsolutePath();

Comment 3 Andrey Loskutov 2020-03-03 10:32:23 EST

(In reply to Andrey Loskutov from comment #2)
> I can't reproduce (build I20200302-0600, Linux, Java 8).

Most likely because I don't have a BaseBuild.xml in a jar :-)

I think it will be easier if you could create a self-containing project that includes both build.xml and the jar with BaseBuild.xml.

Comment 4 Michael Schnell 2020-03-03 10:38:26 EST

The error "Default target deploy does not exist in this project" is expected in this example as the default target is defined in the imported BaseBuild.xml, which is not included in the example provided by me.

We use Windows 10 and Java 11... 

And I tested it with 4.15.0.I20200226-1800 where the issue is still present.

I can try with your build verion tomorrow.

Comment 5 Michael Schnell 2020-03-03 10:39:29 EST

OK I will create such a self-containing example tomorrow. :-)

Comment 6 Michael Schnell 2020-03-04 02:02:08 EST

Created attachment 281994 [details]
Minimal self containing example

So I've created the minimal example. 

It turns out that using a JAR to import from is not required. The same error occurs even if I import a plain ANT xml file.

build.xml:

   <project name="TestProject" default="deploy">
     <import>
       <javaresource name="BaseBuild.xml" classpath="." />
     </import>
   </project>

BaseBuild.xml:

   <project name="ProjectBase">
     <target name="deploy">
       <echo message="Hello ANT" />
     </target>
   </project>

Note that I can launch the build using "Run As" -> "Ant Build"

   Buildfile: [...]\AntError\build.xml
   deploy:
        [echo] Hello ANT
   BUILD SUCCESSFUL
   Total time: 275 milliseconds

Only the ANT editor and the "External Tools Configuration" fail to load the ANT file.

I tested again with 2020-03-RC1. 
Download of eclipse-SDK-I20200303-0600-win32-x86_64.zip is still in progress...

Comment 7 Michael Schnell 2020-03-04 02:08:02 EST

Created attachment 281995 [details]
External Tools Configuration (screenshots)

Added screenshots of the External Tools Configuration and the error log entry.

Comment 8 Michael Schnell 2020-03-04 02:12:44 EST

Created attachment 281996 [details]
Screenshot of the ANT editor tab and the error log entry

Added a screenshot that shows the editor tab and the log entry.

Comment 9 Michael Schnell 2020-03-04 03:47:10 EST

I can still reproduce the issue with build I20200302-0600, Window 10, Java 8 (1.8.0_192)

Comment 10 Sarika Sinha 2020-03-04 04:12:37 EST

I am able to reproduce on Mac as well with 4.14 and 4.15 builds. Works fine with 4.13 build.
We updated to Ant 1.10.7 in 4.14 M1 which might have caused this.
Investigating further.

Comment 11 Michael Schnell 2020-03-04 04:22:08 EST

Created attachment 281998 [details]
Screenshot of debuging session showing the issue

I've setup Ecplipse SDK to debug the problem.

As you can see in the screenshot the buildFile of the AntXMLContext is null.
This is the case as the BaseBuild.xml is imported using the classpath attribute of the <import ..> element. (Remember: We import that script from a JAR)

As the result the BaseBuild.xml is not loaded from a File, but form an URL instance. 

The bug was introduced by the fix for bug 412809.

Comment 12 Sarika Sinha 2020-03-04 05:33:26 EST

@Alexander,
Can you look at this?

We are in the last phase for 4.15 release, so if we you have a fix we need to work ASAP.

Comment 13 Michael Schnell 2020-03-04 06:59:44 EST

Created attachment 282004 [details]
Possible/partial fix

The fix is easy as soon as we know if and how we need to create the parsedProjectNames Map key in this case.

The map contains the absolute path to the build file (as String) mapped to the ANT project name.

The propblem is that when we import or include an ANT script using the classpath, that the imported file does not have an absolute file path, but an URL!

The map entries are required only in clase of an include that does not define an "as" attribute. It is not used when an "as" attribute is specified in an include and it is not used at all for imports (as import does not prefix the imported elements)

The location that fails is where the map is filled with entries. So working around the NPE there fixes the issues for the import and the include with an "as" attriubute.

The patch attached also fixes the issue for the include, where the included file is actually a plain file with an file:/ URL. 

But I do not have the time or recources to look into the case where the include is used to include an ANT file from a JAR file using a javaresource and the classpath attribute (without an "as" attribute on the include).

I AGREE with the ECA for this patch contribution, so feel free to use it as a starting point.

Comment 14 Andrey Loskutov 2020-03-04 07:05:50 EST

(In reply to Michael Schnell from comment #13)
> I AGREE with the ECA for this patch contribution, so feel free to use it as
> a starting point.

No, we need you to sign ECA to be able to use the patch, which also should be provided via Gerrit.

See 
https://www.eclipse.org/legal/ECA.php
https://wiki.eclipse.org/Platform/How_to_Contribute

Comment 15 Sarika Sinha 2020-03-04 07:57:09 EST

Please provide the fix in gerrit and if you can add a test case that will help.
We will like to have this fix included for RC2.

Comment 16 Sarika Sinha 2020-03-04 08:05:31 EST

I tested the patch and it looks good for RC2.

Comment 17 Michael Schnell 2020-03-04 08:21:42 EST

Created attachment 282005 [details]
Updated patch containing all changes (two files)

(In reply to Andrey Loskutov from comment #14)
> (In reply to Michael Schnell from comment #13)
> > I AGREE with the ECA for this patch contribution, so feel free to use it as
> > a starting point.
> 
> No, we need you to sign ECA to be able to use the patch, which also should
> be provided via Gerrit.
> 
> See 
> https://www.eclipse.org/legal/ECA.php
> https://wiki.eclipse.org/Platform/How_to_Contribute

I will probably do this in the future, if I find the time.

But for now: If you cannot or must not look into my small patch I can give you some additional notes:

- ProjectHelper Line 345 fails with NPE because context.getBuildFile() returns null.
- context.getBuildFileURL() is not null in this case and could be used to extract the absolute path for file:// URLs

- AntIncludeNode.extractProjectNameSingleNode fails to retrieve an IFile (this is where it could get tricky with URLs)
- In this case (IFile returned by handleCorrectBuildFile(importedFromNode) is null) I used this.getFilePath()
- I've added an additional getProjectNameOfBuildFile overload to ProjectHelper that takes the absolute path string to directly access the map with it.


My last patch was missing the changes in AntIncludeNode. So I added the full patch now. And I still AGREE with the ECA for this path :-)
I've created the patch on top of Git commit 8befab272c364af6d94c5da2c892eaf65e57beca in org.eclipse.ant.ui

IANAL, but as you signed the ECA and according to the "Developers's Certivicate of Origin" "Question C" my patch was "provided directly to [you] by some other person who certified (a), (b) or (c)" 

In this cas the other person is me, and I certified for (a) "The contribution was created in whole or in part by me and I have the right to submit it under the open source license indicated in the file"

I think you can use my patch as starting point to fix the issue.


I would also like to see this fixed in 2020-03-RC2 :)

Comment 18 Eclipse Genie 2020-03-04 15:27:41 EST

New Gerrit change created: https://git.eclipse.org/r/158826

Comment 19 Andrey Loskutov 2020-03-04 15:34:13 EST

(In reply to Eclipse Genie from comment #18)
> New Gerrit change created: https://git.eclipse.org/r/158826

This is a minimal patch without tests, that seem to work for me (editor works file, launch config works). 

In org.eclipse.ant.internal.ui.editor.utils.ProjectHelper.ProjectHandler.onStartElement(String, String, String, Attributes, AntXMLContext) I can't see the need for the key given to org.eclipse.ant.internal.ui.editor.utils.ProjectHelper.storeParsedProjectName(String, String) to be a valid path, and I also can't see any errors in AntIncludeNode, so I only patched the NPE in the stack from comment 0 by passing the URL path like "file:/C:/workspaces/runtime-New_configuration/AntError/hello.zip!/hello/ant/BaseBuild.xml" without extracting anything from it.

That seem to work for both plain files and files included in zip files referenced like 

<import>
  	<javaresource name="hello/ant/BaseBuild.xml" classpath="." />
</import>

or

<import>
  	<javaresource name="hello/ant/BaseBuild.xml" classpath="./hello.jar"
/>
</import>.

Comment 20 Andrey Loskutov 2020-03-04 15:55:56 EST

(In reply to Andrey Loskutov from comment #19)
> This is a minimal patch without tests

I've added a simple test to the patch.

Comment 21 Eclipse Genie 2020-03-04 15:59:35 EST

New Gerrit change created: https://git.eclipse.org/r/158828

Comment 22 Michael Schnell 2020-03-04 16:10:13 EST

Cool, thank you for looking into this!

As you can see I signed the ECA :-)
And I just managed to push my first commit to gerrit :)

Comment 23 Andrey Loskutov 2020-03-04 16:16:18 EST

(In reply to Michael Schnell from comment #22) 
> As you can see I signed the ECA :-)
> And I just managed to push my first commit to gerrit :)

Very nice. Can you answer to comment 19 - why do we need to deal with the AntIncludeNode at all? In the case where the build file is inside an archive, there will be no unique "IFile" anyway, and the only code that uses ProjectHelper.getProjectNameOfBuildFile(IFile) in AntIncludeNode.getIncludePrefix(AntTaskNode) has null check, so that changes on AntIncludeNode seem to be unneeded.

Comment 24 Michael Schnell 2020-03-04 16:20:06 EST

Yes the <import> does not use the stored project names at all!

So I also started by using the URL.toExternalForm().

But then I was looking who is retrieving the ANT project names and when.

The issue only arises when you replace <import> with <include> over three bbuild files and do _not_ use the "as" attribute. In this case the included ANT targets are prefixed by the ANT project name, so they need to be resolved.

build.xml:
  <project name="TOP" default="Middle.Base.DoBase">
    <include>
      <javaresource name="BaseBuild.xml" classpath="." />
    </include>
  </project>

BaseBuild.xml
  <project name="Middle">
    <include>
      <javaresource name="BaseBaseBuild.xml" classpath="." />
    </include>

    <target name="DoMore">
      <echo message="Hello ANT" />
    </target>
  </project>

BaseBase.xml
  <project name="Base">
    <target name="DoBase">
      <echo message="Hello ANT" />
    </target>
  </project>

As you can see the default target in build.xml is "Middle.Base.DoBase"
My latest patch pushed to gerrit supports this scenario.

Comment 25 Andrey Loskutov 2020-03-04 16:57:08 EST

(In reply to Michael Schnell from comment #24)
> The issue only arises when you replace <import> with <include> over three
> bbuild files and do _not_ use the "as" attribute. In this case the included
> ANT targets are prefixed by the ANT project name, so they need to be
> resolved.

OK 

> As you can see the default target in build.xml is "Middle.Base.DoBase"
> My latest patch pushed to gerrit supports this scenario.

I've updated your patch and added tests to it for both include/import cases.

Comment 26 Sarika Sinha 2020-03-04 23:16:31 EST

I got this exception while doing something (don't know exactly) after this patch:
java.lang.ClassCastException: class org.apache.tools.ant.types.resources.JavaResource cannot be cast to class org.apache.tools.ant.Task (org.apache.tools.ant.types.resources.JavaResource and org.apache.tools.ant.Task are in unnamed module of loader org.eclipse.osgi.internal.loader.EquinoxClassLoader @42c55978)
	at org.eclipse.ant.internal.ui.editor.utils.ProjectHelper$ElementHandler.onStartElement(ProjectHelper.java:119)
	at org.apache.tools.ant.helper.ProjectHelper2$RootHandler.startElement(ProjectHelper2.java:575)
	at java.xml/com.sun.org.apache.xerces.internal.parsers.AbstractSAXParser.startElement(AbstractSAXParser.java:510)
	at java.xml/com.sun.org.apache.xerces.internal.parsers.AbstractXMLDocumentParser.emptyElement(AbstractXMLDocumentParser.java:183)
	at java.xml/com.sun.org.apache.xerces.internal.impl.XMLNSDocumentScannerImpl.scanStartElement(XMLNSDocumentScannerImpl.java:351)
	at java.xml/com.sun.org.apache.xerces.internal.impl.XMLDocumentFragmentScannerImpl$FragmentContentDriver.next(XMLDocumentFragmentScannerImpl.java:2708)
	at java.xml/com.sun.org.apache.xerces.internal.impl.XMLDocumentScannerImpl.next(XMLDocumentScannerImpl.java:605)
	at java.xml/com.sun.org.apache.xerces.internal.impl.XMLNSDocumentScannerImpl.next(XMLNSDocumentScannerImpl.java:112)
	at java.xml/com.sun.org.apache.xerces.internal.impl.XMLDocumentFragmentScannerImpl.scanDocument(XMLDocumentFragmentScannerImpl.java:534)
	at java.xml/com.sun.org.apache.xerces.internal.parsers.XML11Configuration.parse(XML11Configuration.java:888)
	at java.xml/com.sun.org.apache.xerces.internal.parsers.XML11Configuration.parse(XML11Configuration.java:824)
	at java.xml/com.sun.org.apache.xerces.internal.parsers.XMLParser.parse(XMLParser.java:141)
	at java.xml/com.sun.org.apache.xerces.internal.parsers.AbstractSAXParser.parse(AbstractSAXParser.java:1216)
	at java.xml/com.sun.org.apache.xerces.internal.jaxp.SAXParserImpl$JAXPSAXParser.parse(SAXParserImpl.java:635)
	at org.apache.tools.ant.helper.ProjectHelper2.parse(ProjectHelper2.java:280)
	at org.eclipse.ant.internal.ui.editor.utils.ProjectHelper.parse(ProjectHelper.java:610)
	at org.eclipse.ant.internal.ui.editor.utils.ProjectHelper.parse(ProjectHelper.java:732)
	at org.apache.tools.ant.taskdefs.ImportTask.importResource(ImportTask.java:221)
	at org.apache.tools.ant.taskdefs.ImportTask.execute(ImportTask.java:165)
	at org.apache.tools.ant.UnknownElement.execute(UnknownElement.java:292)
	at org.eclipse.ant.internal.ui.model.AntImportNode.configure(AntImportNode.java:73)
	at org.eclipse.ant.internal.ui.model.AntModel.resolveBuildfile(AntModel.java:557)
	at org.eclipse.ant.internal.ui.model.AntModel.parseDocument(AntModel.java:370)
	at org.eclipse.ant.internal.ui.model.AntModel.reconcile(AntModel.java:304)
	at org.eclipse.ant.internal.ui.editor.text.XMLReconcilingStrategy.internalReconcile(XMLReconcilingStrategy.java:45)
	at org.eclipse.ant.internal.ui.editor.text.XMLReconcilingStrategy.reconcile(XMLReconcilingStrategy.java:55)
	at org.eclipse.jface.text.reconciler.MonoReconciler.process(MonoReconciler.java:76)
	at org.eclipse.ant.internal.ui.editor.text.NotifyingReconciler.process(NotifyingReconciler.java:39)
	at org.eclipse.jface.text.reconciler.AbstractReconciler$BackgroundThread.run(AbstractReconciler.java:210)

Comment 27 Andrey Loskutov 2020-03-05 01:02:34 EST

(In reply to Sarika Sinha from comment #26)
> I got this exception while doing something (don't know exactly) after this
> patch:
> java.lang.ClassCastException: class
> org.apache.tools.ant.types.resources.JavaResource cannot be cast to class
> org.apache.tools.ant.Task (org.apache.tools.ant.types.resources.JavaResource
> and org.apache.tools.ant.Task are in unnamed module of loader
> org.eclipse.osgi.internal.loader.EquinoxClassLoader @42c55978)
> 	at
> org.eclipse.ant.internal.ui.editor.utils.ProjectHelper$ElementHandler.
> onStartElement(ProjectHelper.java:119)

Could be an error during invalid xml parsing, e. g. while typing?

Comment 28 Sarika Sinha 2020-03-05 01:18:27 EST

(In reply to Andrey Loskutov from comment #27)

> 
> Could be an error during invalid xml parsing, e. g. while typing?

Yes, it should be some incorrect typing and not bale to reproduce. Mostly I used JavaResource tag at some incorrect place.

I should not have got NPE but that is a separate issue and I will create a bug for 4.16 to put the check.


I am ok with the change and looks good for RC2.

Comment 29 Michael Schnell 2020-03-05 01:20:54 EST

Cool! Thanks to both of you :)

Comment 30 Eclipse Genie 2020-03-05 01:50:37 EST

Gerrit change https://git.eclipse.org/r/158828 was merged to [master].
Commit: http://git.eclipse.org/c/platform/eclipse.platform.git/commit/?id=d9b6fdbb488f92f424484527e9ef40c8b39c7283

Comment 31 Sarika Sinha 2020-03-05 01:52:16 EST

Thanks Michael for reporting the problem and providing the fix!

Comment 32 Sarika Sinha 2020-03-05 05:41:42 EST

Build id: I20200305-0155