[vmkit-commits] [PATCH] Allow user to specify which java runtime to use when configuring

[Nicolas Geoffray]

On Mon, Oct 24, 2011 at 11:33 PM, Will Dietz <w at wdtz.org> wrote:

> On 10/24/2011 04:10 PM, Nicolas Geoffray wrote:
>
>> Very nice! These kinds of checks should have been there from the
>> beginning, it would have save me and probably other lots of debugging
>> time finding out what's going wrong :) There's one thing you should fix
>> though to make it portable, especially on macosx. Feel free to drop the
>> code that is not portable if it takes too long to find out how to fix it.
>>
>>
> Would changing the ".so" to $DYLIB_EXTENSION (just verified to work here,
> don't have a Mac handy to verify) satisfy the portability concerns?  If so,
> I can change that and include in the commit.
>

Great, it think it will! If it works on your machine, then please use it and
commit. I'll test on my mac once you have committed.


>  Small request: next time you change the configure.ac
>> <http://configure.ac> file, you can drop the generated configure file
>>
>> from the review. Also, please do two separate commits: one for the
>> "real" files, and one for the auto-generated, so that one does not
>> pollute the other. Thanks!
>>
>>
> Of course, and understood.  Sorry :).
>

No problem.


>
>     +case "${classpathimpl}" in
>>    + gnuclasspath)
>>    + AC_MSG_NOTICE(Validing GNU Classpath installation...)
>>    + AC_CHECK_FILES([${**classpathlibs}]
>>    + [${classpathlibs}/libjavaio.**so]
>>    + [${classpathlibs}/libjavalang.**so]
>>    + [${classpathlibs}/**libjavalangreflect.so]
>>    + [${classpathlibs}/libjavanet.**so]
>>    + [${classpathlibs}/libjavanio.**so]
>>    + [${classpathlibs}/libjavautil.**so],,
>>
>>
>> Extra comma or is it necessary?
>> Also, this won't work on macosx (nor windows, but that's not supported
>> anyways), and unfortunately I don't know enough of autoconf to make it
>> portable. But that's a really neat thing to have, so if you can find it
>> out, it'll be great! :)
>>
>
> The comma is necessary, but good eye! Just friendly FWIW/FYI: The field
> between the commas defines what to do if those files are found, and we're
> only interested in defining what happens if they are not, so we omit it.
>

I see. Thanks!


>
>     -AC_CONFIG_FILES([lib/J3/**Classpath/Classpath.h])
>>    +AC_CONFIG_FILES([lib/J3/**ClassLib/Classpath.h])
>>
>>
>> I don't think that's supposed to go in (just yet :))
>>
>
> Heh.  No, no indeed, just got lost in the patch reordering.  Will fix.
>
> ~Will
>

Nicolas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/vmkit-commits/attachments/20111024/02995fb6/attachment.html>