[7u4] Review request for CR 7134730
Kumar Srinivasan
kumar.x.srinivasan at oracle.COM
Mon Feb 13 10:46:28 PST 2012
>> 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
>
>
More information about the macosx-port-dev
mailing list