[7u4] Review request for CR 7134730

[Kumar Srinivasan]

>> 4. I also find the version string hard-coded in build.xml, we need to have a mechanism
>>    such that version information is derived from the make/build logic.
> Do you mean the version that we get from build.properties?
It looks like you have a todo for this as well, you can always tunnel in
the version information from make into ant, the langtools make and
ant have examples of these, if the ant is going to be run with built
jdk, then you can infer these from the ant system itself.
http://hg.openjdk.java.net/jdk7u/jdk7u-dev-gate/langtools/file/4672e092f096/make/Makefile


   50<!-- TODO Move this to a makefile so we can reference the defines required by JLI_Launch() -->
   51<exec executable="gcc">
   52<arg value="-I"/>
   53<arg value="../../../../build/macosx-universal/j2sdk-image/1.7.0.jdk/Contents/Home/include"/>
   54<arg value="-I"/>
   55<arg value="../../../../build/macosx-universal/j2sdk-image/1.7.0.jdk/Contents/Home/include/darwin"/>
   56<arg value="-o"/>
   57<arg value="${ant.project.name}/${folder.classes}/com/oracle/appbundler/JavaAppLauncher"/>
   58<arg value="-framework"/>
   59<arg value="Cocoa"/>
   60<arg value="-F"/>
   61<arg value="../../../../build/macosx-universal/j2sdk-image/1.7.0.jdk"/>
   62<arg value="-m64"/>
   63<arg value="-std=c99"/>
   64<arg value="appbundler/native/main.m"/>
   65</exec>
   66</target>


Kumar
>
>> 5. The test  is great, but how will the test be executed ? typically
>>    in the jdk, the tests are located under jdk/test/......, and these are all run by
>>    jtreg automatically on a periodic basis (nightly, weekly, promotion etc), we
>>   need this hooked up into that framework.
> You are correct - enabling automated unit tests for this code is still a TODO.
>
>> 6. AppBundlerTask.java.html
>>
>> Does the ant code get built and run with jdk6 ?
> No, Java 7 is required.
>
>> it that is the case then the following
>> may be moot, but of course you can use for-each and simplify some code below.
>>
>>   71     private ArrayList<File>   classPath = new ArrayList<File>();
>>   72     private ArrayList<File>   nativeLibraries = new ArrayList<File>();
>>   73     private ArrayList<String>   options = new ArrayList<String>();
>>   74     private ArrayList<String>   arguments = new ArrayList<String>();
>>
>> you can set the assiged type to List.
>> private List<File>  classPath = new ArrayList<>();
> That's true. I haven't fully acclimated myself to Java 7 syntax yet.  :-)
>
>> lines 135....
>>
>> 135     public void addConfiguredClassPath(FileSet classPath) {
>> 136         File parent = classPath.getDir();
>> 137
>> 138         DirectoryScanner directoryScanner = classPath.getDirectoryScanner(getProject());
>> -139         String[] includedFiles = directoryScanner.getIncludedFiles();
>> 140
>> -141         for (int i = 0; i<   includedFiles.length; i++) {
>> -142             this.classPath.add(new File(parent, includedFiles[i]));
>> -143         }
>> 	      for (String name : directoryScanner.getIncludedFiles() {
>> 		  this.classPath.add(new File(parent, name);
>>               }
>>   144     }
> Thanks for this suggestion. Apparently I'm not yet fully acclimated to Java 7 APIs, either.  ;-)
>
>> similarly lines 453 and 477.
>>
>> Test.java, I would make the test program dump out the args as well,
>> and pass some dummy args to ensure that args are passed correctly
>> to the app's main.
> Good idea.
>
>> I noticed that you set the starting heapsize and maximum heap size ?
> That's just to test setting VM options using the launcher. I can change them to something else if there are better test options to use.
>
> G
>
>