Please use the Apache issue tracking system for new NetBeans issues (https://issues.apache.org/jira/projects/NETBEANS0/issues) !!
Bug 136623 - Provide a way to add a library for custom Ant tasks
Provide a way to add a library for custom Ant tasks
Status: RESOLVED FIXED
Product: projects
Classification: Unclassified
Component: Ant Project
6.x
All All
: P3 with 1 vote (vote)
: 6.x
Assigned To: Milan Kubec
issues@projects
: API_REVIEW_FAST
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-06-06 09:04 UTC by Milan Kubec
Modified: 2008-06-13 15:42 UTC (History)
2 users (show)

See Also:
Issue Type: DEFECT
:


Attachments
diff file with API change (13.47 KB, text/plain)
2008-06-06 12:39 UTC, Milan Kubec
Details
diff file with API change (16.21 KB, text/plain)
2008-06-09 13:06 UTC, Milan Kubec
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Milan Kubec 2008-06-06 09:04:09 UTC
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.
Comment 1 Milan Kubec 2008-06-06 12:39:20 UTC
Created attachment 62461 [details]
diff file with API change
Comment 2 Milan Kubec 2008-06-06 12:43:15 UTC
Please review the API change, thanks. 
The diff file contains so far only the implementation, other parts will follow.
Comment 3 Milos Kleint 2008-06-06 14:02:20 UTC
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.
Comment 4 Milan Kubec 2008-06-06 15:43:39 UTC
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).

Comment 5 Milan Kubec 2008-06-08 21:43:27 UTC
Forgot API_REVIEW_FAST kw.
Comment 6 David Konecny 2008-06-08 23:01:43 UTC
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
Comment 7 David Konecny 2008-06-08 23:43:42 UTC
DK06 - instead of "," separator I would use standard classpath separator ";" or ":" and PropertyUtils.tokenizePath to
split items
Comment 8 Milan Kubec 2008-06-09 13:04:29 UTC
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.

Comment 9 Milan Kubec 2008-06-09 13:06:10 UTC
Created attachment 62556 [details]
diff file with API change
Comment 10 Milan Kubec 2008-06-09 13:08:21 UTC
apichanges.xml contains wrong author, already fixed.
Comment 11 David Konecny 2008-06-09 23:00:33 UTC
Re. "there were some weird issues when saving properties using AntProjectHelper" - sounds very suspicious. :-)
Comment 12 Milan Kubec 2008-06-10 13:12:25 UTC
Yes, indeed, the very first change of project to javawebstart enabled project was not saved to project.properties. Any next change was OK.
Comment 13 Milan Kubec 2008-06-10 13:17:32 UTC
If there are no other opinions I will push this API change tomorrow.
Comment 14 Milan Kubec 2008-06-11 11:06:00 UTC
Part of the following push: http://hg.netbeans.org/core-main/rev/78f94196ce39
Comment 15 Jesse Glick 2008-06-11 15:18:58 UTC
In the future please try to commit changes in more digestible units. MQ can be helpful for this.
Comment 16 Milan Kubec 2008-06-11 15:29:59 UTC
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.
Comment 17 Jesse Glick 2008-06-11 15:36:50 UTC
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.
Comment 18 Jesse Glick 2008-06-11 15:42:10 UTC
[JG13] javawebstart module does not seem to express a dependency on the new spec version of project.ant.
Comment 19 Jesse Glick 2008-06-11 15:43:26 UTC
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
Comment 20 Jesse Glick 2008-06-11 15:58:26 UTC
Regarding MQ - generally I would recommend using it for all API_REVIEW changes or anything similar. Makes things easier.
Up to you of course.
Comment 21 Jesse Glick 2008-06-11 18:44:49 UTC
I am fixing JG01 in #2325a1b8dfd8.
Comment 22 Milan Kubec 2008-06-11 21:02:11 UTC
Jesse, thanks for catching and fixing JG01, I will fix JG02 - JG13.
Comment 23 David Konecny 2008-06-11 22:45:21 UTC
[JG05] - I actually suggested opposite - seems to me more consistent to use the same path separator within Ant
properties - ";" or ":" - and use PropertyUtils.tokenizePath.
Comment 24 Jesse Glick 2008-06-11 23:18:25 UTC
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.
Comment 25 David Konecny 2008-06-11 23:27:19 UTC
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
Comment 26 Jesse Glick 2008-06-11 23:35:19 UTC
From looking at the patch I thought it was a list of library IDs, not JAR names.
Comment 27 David Konecny 2008-06-11 23:47:28 UTC
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.
Comment 28 David Konecny 2008-06-12 10:01:07 UTC
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
Comment 29 Milan Kubec 2008-06-12 11:01:38 UTC
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?
Comment 30 Milan Kubec 2008-06-12 11:16:57 UTC
Davide, are you going to push your changeset to main or core-main repo?
Comment 31 David Konecny 2008-06-12 11:35:33 UTC
I will push my change to *main* sometimes tomorrow (Friday).
Comment 32 Jesse Glick 2008-06-12 22:33:01 UTC
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.
Comment 33 Milan Kubec 2008-06-13 09:06:22 UTC
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

Comment 34 Jesse Glick 2008-06-13 15:42:41 UTC
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.


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2014, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo