This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.
There is missing a way to add or register 3rd party library for Ant custom tasks. Such library is then used to define custom Ant tasks via taskdef. I propose extending AntBuilderExtender API to provide a method for adding a library AntBuildExtender.addLibrary(String globalLibID) AntBuildExtender.removeLibrary(String globalLibID) Library ID will be stored in project.properties in property 'ant.customtasks.libs' (comma separated list of values), the property will be used when project was created as non-sharable and then switched to sharable to copy also libraries that are required for build process and not only for project classpaths.
Created attachment 62461 [details] diff file with API change
Please review the API change, thanks. The diff file contains so far only the implementation, other parts will follow.
MK1: public AntBuildExtender createExtender(AntBuildExtenderImplementation impl) shall be deprecated (or how do you workaround the fact that you are missing the ReferenceHelper?) MK2: I suggest using Library as parameter instead of a String ID. That's consistent with stuff like ClassPathModifier/Extender Otherwise looks good.
MK1: There is @deprecated tag in javadoc for createExtender(AntBuildExtenderImplementation impl) MK2: I'd rather use String ID, because a) LibraryManager.getLibrary() expects String ID anyway and b) removeLibrary() is in fact only about removing name from property in project.properties and in case when the library would not be presented in the IDE installed Libraries it would not be possible to remove such library (e.g. project is created as non sharable; library is added by AntExtender.addLibrary() and then the library is removed from the IDE libraries -> library name cannot be removed from project.properties).
Forgot API_REVIEW_FAST kw.
DK01 - Javadoc for AntBuildExtender.addLibrary should clarify that library must exists and impl should throw an exception if library cannot be found; or as Milos suggested use Library instead of libraryID DK02 - perhaps it make sense to keep removeLibrary(stringID) for reason you described. Alternative solution could be to have AntBuildExtender.removeLibrary(Library) and document in Javadoc that library instance must be either global or sharable library DK03 - instead of lookup(J2SEPropertyEvaluator) you can use uiProperties.getProject().evaluator(); DK04 - you are already adding ReferenceHelper to AntBuildExtender constructor so perhaps you can add AntProjectHelper as well and then use existing APH API in setValueOfProperty() to read/write properties. DK05 - could you fix web/j2ee project types as well please? web.project, j2ee.clientproject, j2ee.earproject and j2ee.ejbjarproject
DK06 - instead of "," separator I would use standard classpath separator ";" or ":" and PropertyUtils.tokenizePath to split items
DK01 & DK02: I've used finally add/removeLibrary(Library) DK03: Done. DK04: I will rather stick with ReferenceHelper only, there were some weird issues when saving properties using AntProjectHelper. DK05: Will do, but probably in next changeset. DK06: Done. Attaching new diff, now with apichanges.xml. I'd like to commit this change on Wednesday 11th.
Created attachment 62556 [details] diff file with API change
apichanges.xml contains wrong author, already fixed.
Re. "there were some weird issues when saving properties using AntProjectHelper" - sounds very suspicious. :-)
Yes, indeed, the very first change of project to javawebstart enabled project was not saved to project.properties. Any next change was OK.
If there are no other opinions I will push this API change tomorrow.
Part of the following push: http://hg.netbeans.org/core-main/rev/78f94196ce39
In the future please try to commit changes in more digestible units. MQ can be helpful for this.
I know that it should be separated, but mq needs to be used before making any change and in that time I had no idea that I'd have needed to.
Sorry I had no time to review this earlier. Some belated comments on the whole diff: [JG01] javawebstart/nbproject/project.properties probably needs to define extra.modules.files=ant/extra/org-netbeans-modules-javawebstart-anttasks.jar Otherwise DeleteExtraClusterFiles is going to fail because your tasks will not be in the NBM fileset. [JG02] AntBuildExtender.add/removeLibrary are missing @since. [JG03] You can use Parameters.notNull. [JG04] Use FileObject.getOutputStream() so you do not need to deal with FileLock explicitly. [JG05] It would be more conventional to use ',' as a separator for library names rather than ';'. [JG06] @deprecated tags should have text in them. The member should also be marked with the @Deprecated annotation. [JG07] javawebstart/AntTasks/manifest.mf should not exist; there is no main class. And delete manifest.file from project.properties. [JG08] Do not use file.reference.ant.jar=${ant.home}/lib/ant.jar javac.classpath=${file.reference.ant.jar} rather use javac.classpath=${ant.core.lib} [JG09] Ant tasks should use Project.log for warnings; any real failures should result in a BuildException being thrown. [JG10] createJNLPComponentFile should use DOM serialization to write out XML if possible; println's are generally dangerous (as you can accidentally emit sequences such as '<', or ']]', or control characters...). Ideally it should emit a document with a DTD and actually validate it before returning. [JG11] You can omit empty volumes from a library declaration. But we should probably ship Ant task sources with the IDE, in the 'sources' dir of the cluster. [JG12] ExtensionResourcesPanel.java is missing license text.
[JG13] javawebstart module does not seem to express a dependency on the new spec version of project.ant.
Regarding JG01, you are about to break tests on trunk unless you fix. To check, run 'ant clean-untracked-files'. Failure: Some extra files were present java2: untracked file ant/extra/org-netbeans-modules-javawebstart-anttasks.jar
Regarding MQ - generally I would recommend using it for all API_REVIEW changes or anything similar. Makes things easier. Up to you of course.
I am fixing JG01 in #2325a1b8dfd8.
Jesse, thanks for catching and fixing JG01, I will fix JG02 - JG13.
[JG05] - I actually suggested opposite - seems to me more consistent to use the same path separator within Ant properties - ";" or ":" - and use PropertyUtils.tokenizePath.
For JG05 vs. DK06 - semicolon ';' (or preferably colon ':') would be right if it were a file path, but (at least as I understand it) this is not a path, it is a list, and Ant normally separates lists with commas ',' as you might expect.
JG05 vs. DK06 - it is list; though list of jars which (in theory) could be used as classpath for your taskdef; never mind - I do not care that much
From looking at the patch I thought it was a list of library IDs, not JAR names.
In that case sorry - make sense to keep it as list. I was always thinking about it as a "classpath" with references to jars and libraries.
I'm not sure it is a problem but API revision 1.21 was used for this issue and also for AntProjectHelper-createAuxiliaryProperties and apichanges.xml now contains two <change>s for this revision. I'm about to merge and commit my change which I will number 1.22 so if you decide to fix this use 1.23. Thanks, -D
It's difficult to express how I like our bulletproof commit process across n repositories. This was the first time (and hopefully also the last time) I used core- main repository. In time of commit to core-main only my 1.21 version was in apichanges.xml and after merge to main there are two. Now I have main repository core-main repository and some work repo. Which repo should I use to continue on this issue to actually work on current state of sources?
Davide, are you going to push your changeset to main or core-main repo?
I will push my change to *main* sometimes tomorrow (Friday).
Always make sure your changes make sense on their own in whatever repo you normally work in (which should be a clone of core-main) and if there is a merge conflict deal with it later as a separate step.
JG02: Fixed. JG03: Fixed. JG04: If I don't use explicit lock I got FileAlreadyLockedException, I will stick with the explicit lock. JG05: Fixed. JG06: Fixed. JG07: Fixed. JG08: I could use ${ant.core.lib} only when inheritAll="true", but if I do that jar task creates jar with content from javawebstart module, not AntTasks. JG09: FIxed. JG10: I will fix in some future changeset. JG11: I probably don't understand exactly this point. I can omit libraries with empty volumes during adding them in AntBuildExtender.addLibrary(), so what has bundling of AntTasks sources to do with this? JG12: Already fixed in some previous push. JG13: Fixed. http://hg.netbeans.org/main/rev/2342715ee7d9
To JG08: make a note to do it right when we bundle Ant 1.7.1. See: https://issues.apache.org/bugzilla/show_bug.cgi?id=42263 JG11 was referring to javawebstart/src/org/netbeans/modules/javawebstart/JWSAntTasks.xml. The javadoc volume can probably be omitted. For the src volume, you could use bundle sources for the Ant tasks. However from looking around it seems that other modules bundling special Ant tasks required by user projects are not distributing sources for them, so I guess for consistency you could just omit the src volume too.