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.
This is cover issue for three small API change issues which are so related that I implemented them at once. These issues are: * AntArtifact should provide optional properties for target execution <http://www.netbeans.org/issues/show_bug.cgi?id=47788> * AntArtifact should permit multiple build artifacts <http://www.netbeans.org/issues/show_bug.cgi?id=50484> * Foreign project cannot set dependency on freeform project artifact if build.xml not inside projdir <http://www.netbeans.org/issues/show_bug.cgi?id=50092> Implementation affected both existing APIs and references schema. Some API methods were deprecated and replacements were added. The new schema for references was added and upgrade to new schema is done carefully only when really needed, that is if reference to project A is added to project B and project A build script lies under project's directory and its artifact has just one output and no properties then old reference schema is used whenever possible. I will attach diff of API changes and also full diff which includes also changes in other modules. These are for example in J2SE project type the artifact chooser lists all outputs of all artifacts and allows user to chose only some of them. Unit tests were updated to test apart from old deprecated methods also the new ones. Summary of API changes is: Ant Artifact: * deprecated FO getArtifactFile; use newly added FO[] getArtifactFiles() * deprecated URI getArtifactLocation; use newly added URI[] getArtifactLocations() * added Properties getProperties() ReferenceHelper. Some of these changes (like cleanup in reference removal methods) are not strictly necessary for the implementation of the issue, but they are simplifying the APIs: * deprecated boolean addReference(AA); use newly added String addReference(AA, URI) and/or newly added boolean isReferenced(AA, URI) * deprecated AA getForeignFileReferenceAsArtifact(String); use newly added Object[] findArtifactAndLocation(String) instread which returns array with both AA and URI * deprecated String createForeignFileReference(AA); was not doing anything different from addReference(AA); use directly new String addReference(AA, URI) * deprecated void destroyForeignFileReference(String) and boolean removeReference(String) and boolean removeReferenec(String, String); all these method were very similar; use newly added boolean destroyReference(String) ReferenceHelper.RawReference: * deprecated URI getScriptLocation(); use String getScriptLocationValue() * added constructor which acccept script location in new format and also accept properties Questions/suggestions are welcome.
Created attachment 19452 [details] API diff
Created attachment 19453 [details] generated new Javadoc for ReferenceHelper
Created attachment 19454 [details] full diff
What level of compatibility you want to keep? Not much I guess, but how much? It just seems to me that future extension like this, that would try to be compatible might be much more complicated.
Yarda, I'm not sure I understand you. What are the levels I can choose from? And I guess you speak mainly about change in schema? Anyway, the solution is 100% backward compatible.
Just a short note: 100% means everything that worked before will work again. If that was the goal, I do not think it was reached: class MyAA extends AntArtifact { protected FileObject[] getArtifactFiles() { } } would compile and run on 4.0 but not now, now it will not compile and will not even link. So 100% was not reached. Maybe 99%. class MyAA2 extends AntArtifact { protected Properties getProperties { } } is similar, but it will not just compile, it will link and run well on 4.1 if compiled with 4.0. So say 98.5% Originally I thought the problem will be: class MyAA2 extends AntArtifact { } MyAA2 aa2 = new MyAA2(); aa2.getArtifactLocation(); if this compiled on 4.0, then it would run on 4.0, but on 4.1 it would throw StackOverFlow. This is not possible, as the method was abstract in 4.0 and this could not compile. So still 98.5% But (and that is the reason I said that in future this will not be that easy), if the method was not abstract (and in 4.1 version it is not and getArtifactLocations is not as well), the MyAA2 would fail exactly in the way I described. If you want to eliminate all of the above mentioned problems, you should separate API and SPI (http://openide.netbeans.org/tutorial/api-design.html#design.apiandspi) for example by having AntArtifact final and taking AntArtifactImpl and AntArtifactImpl2 interfaces: pu. fi. cl. AntArtifact { public AntArtifact (AntArtifactImpl i) { } public final URI getArtifactLocation () { return i.getArtifactLocation(); } } would be the impl in 4.0 and in 4.1 you would do: pu. fi. cl. AntArtifact { public AntArtifact (AntArtifactImpl i) { } public AntArtifact (AntArtifactImpl2 i) { } public final URI getArtifactLocation () { if (i instanceof AntArtifactImpl2) { return i.getArtifactLocations()[0]; } else { return i.getAtrifactLocation(); } } public final URI[] getArtifactLocations() { if (i instanceof AntArtifactImpl2) { return i.getArtifactLocations(); } else { return new URI[] { i.getAtrifactLocation() }; } } } this approach would be 100% compatible. I do not mind much that in this release you reached only 98.5% compatiblity, but the more users, the more you will want to be 100% compatible in future. That is why I suggest to adopt API and SPI separation now, it will be harder with every release. PS: ReferenceHelper seems to be final. That is good, it will save you a lot of troubles.
No subclasses of AntArtifact ought to be adding random public/protected methods. The only reason to subclass is to override those methods which are declared as concrete but nonfinal in the superclass. There is no reason to pass an AntArtifact around arbitrarily since the class has no real functionality; it is simply an enhanced struct designed to be returned from AntArtifactQuery and read by code in another module. Therefore there is no reason to add additional signature methods to subclasses; there is no one to call them. As usual, anyone who adds their own accessible methods in a subclass does so at their own peril - they will not generally be able to compile the same source against a newer API (don't know the exact circumstances under which linking will work). No legitimate users of the API should be affected.
In ant-project-references2.xsd, why do you declare property-definition to be an extension of substitutable-text? (1) It isn't substitutable text, AFAIK, it is a fixed string. (2) substitutable-text is not even defined in this schema, so an XML parser will just reject the schema as erroneous before it begins validation. Re. subclasses of AntArtifact - I think Yarda's scenario, while of course possible, is extremely unlikely in practice and going out of our way to prevent it is not a good use of time. Making an AntArtifactImpl in particular would just make the API less intuitive for typical users. We could deprecate all of AntArtifact and create an AntArtifact2 with the new (non-deprecated) signature and update ReferencesHelper to use only the new class in nondeprecated methods if this is straightforward, but if not I would rather spend time on higher-priority issues. ReferencesHelper.canUseVersion10: is aa.gSL().gAP().sW(pD.gAP()) safe on Windows and so on? Are we sure all relevant file paths have already been normalized? In tests: a tip: new TreeSet(Arrays.asList(new String[] {"hello"})) can be simplified to new TreeSet(Collections.singleton("hello")) or in this context even Collections.singleton("hello") In form.ClassPathUtils: call artifact.getArtifactLocations() only once. ProjectClassPathExtender changed signatures; make sure impl dep form -> java/project is updated (java/project/spec-ver.properties). Note that tzezula already has an issue to make a proper public API from this. In j2seproject build-impl.xsl, the body of the properties loop could be simplified to <property name="{@name}" value="{.}"/> (I think; try it.) Otherwise the proposed diff looks good to me. Please make sure developers of web and j2ee project types have seen the proposed API change (and just as importantly, the patch to j2seproject) and agree they can make an analogous change, and that developers of the j2me project type have seen the API and agree they can use its new features for properties in an artifact.
Re. "Schema" - oops, I copy&pasted it form freeform schema and overlooked it. I changed base from "substitutable-text" to "xsd:string". Re. "is aa.gSL().gAP().sW(pD.gAP()) safe or must be normalized" - I checked SimpleAA and it is safe, it calls APH.resolveFile. The rule is that io.File must be normalize when created so that any other usage of it is safe.
J2ME and Web team: it would be great if you could update your project types to use these new APIs. In full diff you can see impact on J2SE project type (start from AntArtifactChooser.java). More or less it is manual change in couple of places where AntArtifact was used and where also URI must be used now. Do you think it is doable for you for 4.1 to update your project type? Do you have any objections against the API? For now I plan to integrate the change later this week. Thanks.
When forwarding issues related to web project, please include also j2ee module (ejbjarproject)
wow - 2400 lines of differencies that's pretty big change going across all the code Do you have it in some branch ?
No branch, just diff. As I said lot of changes outside of ant/project are about searching for "AA" and replacing it with "AA+URI". Quite trivial.
Integrated: ant/freeform/src/org/netbeans/modules/ant/freeform/ArtifactProvider.java; new revision: 1.4; previous revision: 1.3 ant/freeform/test/unit/src/org/netbeans/modules/ant/freeform/ArtifactProviderTest.java; new revision: 1.4; previous revision: 1.3 ant/project/apichanges.xml; new revision: 1.4; previous revision: 1.3 ant/project/manifest.mf; new revision: 1.8; previous revision: 1.7 ant/project/src/org/netbeans/api/project/ant/AntArtifact.java; new revision: 1.10; previous revision: 1.9 ant/project/src/org/netbeans/modules/project/ant/StandardAntArtifactQueryImpl.java; new revision: 1.3; previous revision: 1.2 ant/project/src/org/netbeans/modules/project/ant/ant-project-references2.xsd; initial revision: 1.1 ant/project/src/org/netbeans/spi/project/support/ant/ReferenceHelper.java; new revision: 1.22; previous revision: 1.21 ant/project/src/org/netbeans/spi/project/support/ant/SimpleAntArtifact.java; new revision: 1.10; previous revision: 1.9 ant/project/test/unit/src/org/netbeans/api/project/ant/AntArtifactQueryTest.java; new revision: 1.5; previous revision: 1.4 ant/project/test/unit/src/org/netbeans/spi/project/support/ant/ReferenceHelperTest.java; new revision: 1.15; previous revision: 1.14 ant/project/test/unit/src/org/netbeans/spi/project/support/ant/SimpleAntArtifactTest.java; new revision: 1.8; previous revision: 1.7 java/freeform/src/org/netbeans/modules/java/freeform/JavaProjectGenerator.java; new revision: 1.3; previous revision: 1.2 java/j2seproject/src/org/netbeans/modules/java/j2seproject/classpath/J2SEProjectClassPathExtender.java; new revision: 1.9; previous revision: 1.8 java/j2seproject/src/org/netbeans/modules/java/j2seproject/resources/build-impl.xsl; new revision: 1.50; previous revision: 1.49 java/j2seproject/src/org/netbeans/modules/java/j2seproject/ui/LibrariesNode.java; new revision: 1.6; previous revision: 1.5 java/j2seproject/src/org/netbeans/modules/java/j2seproject/ui/ProjectNode.java; new revision: 1.5; previous revision: 1.4 java/j2seproject/src/org/netbeans/modules/java/j2seproject/ui/customizer/AntArtifactChooser.java; new revision: 1.10; previous revision: 1.9 java/j2seproject/src/org/netbeans/modules/java/j2seproject/ui/customizer/J2SEProjectProperties.java; new revision: 1.37; previous revision: 1.36 java/j2seproject/src/org/netbeans/modules/java/j2seproject/ui/customizer/VisualClassPathItem.java; new revision: 1.11; previous revision: 1.10 java/j2seproject/src/org/netbeans/modules/java/j2seproject/ui/customizer/VisualClasspathSupport.java; new revision: 1.18; previous revision: 1.17 form/src/org/netbeans/modules/form/project/ClassPathUtils.java; new revision: 1.9; previous revision: 1.8
deprecating a type safe method for a method which requires casting: Brilliant! I love to do Smalltalk programming in Java. deprecated AA getForeignFileReferenceAsArtifact(String); use newly added Object[] findArtifactAndLocation(String) instread which returns array with both AA and URI And I just love having to check for null, length and embedded null-ness, instead of just null returning URI[] instead of URI, etc.
You do not need to check the return value of RH.fAAL for null or check its length; it is documented to always return an array of length 2. Sorry about the casts but Java has no tuple syntax and producing a new struct class for this method return value seems like overkill. BTW next time if you want to comment on an API it would be better to do so while it is still under review and there is a chance to make stylistic adjustments.