On Mon, Oct 24, 2011 at 11:33 PM, Will Dietz <span dir="ltr"><<a href="mailto:w@wdtz.org">w@wdtz.org</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div class="im">On 10/24/2011 04:10 PM, Nicolas Geoffray wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Very nice! These kinds of checks should have been there from the<br>
beginning, it would have save me and probably other lots of debugging<br>
time finding out what's going wrong :) There's one thing you should fix<br>
though to make it portable, especially on macosx. Feel free to drop the<br>
code that is not portable if it takes too long to find out how to fix it.<br>
<br>
</blockquote>
<br></div>
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.<br></blockquote>
<div><br></div><div>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. </div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">
Small request: next time you change the <a href="http://configure.ac" target="_blank">configure.ac</a><br></div>
<<a href="http://configure.ac" target="_blank">http://configure.ac</a>> file, you can drop the generated configure file<div class="im"><br>
from the review. Also, please do two separate commits: one for the<br>
"real" files, and one for the auto-generated, so that one does not<br>
pollute the other. Thanks!<br>
<br>
</div></blockquote>
<br>
Of course, and understood. Sorry :).<br></blockquote><div><br></div><div>No problem.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">
+case "${classpathimpl}" in<br>
+ gnuclasspath)<br>
+ AC_MSG_NOTICE(Validing GNU Classpath installation...)<br>
+ AC_CHECK_FILES([${<u></u>classpathlibs}]<br>
+ [${classpathlibs}/libjavaio.<u></u>so]<br>
+ [${classpathlibs}/libjavalang.<u></u>so]<br>
+ [${classpathlibs}/<u></u>libjavalangreflect.so]<br>
+ [${classpathlibs}/libjavanet.<u></u>so]<br>
+ [${classpathlibs}/libjavanio.<u></u>so]<br>
+ [${classpathlibs}/libjavautil.<u></u>so],,<br>
<br>
<br></div><div class="im">
Extra comma or is it necessary?<br>
Also, this won't work on macosx (nor windows, but that's not supported<br>
anyways), and unfortunately I don't know enough of autoconf to make it<br>
portable. But that's a really neat thing to have, so if you can find it<br>
out, it'll be great! :)<br>
</div></blockquote>
<br>
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.<br>
</blockquote><div><br></div><div>I see. Thanks!</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">
-AC_CONFIG_FILES([lib/J3/<u></u>Classpath/Classpath.h])<br>
+AC_CONFIG_FILES([lib/J3/<u></u>ClassLib/Classpath.h])<br>
<br>
<br></div><div class="im">
I don't think that's supposed to go in (just yet :))<br>
</div></blockquote>
<br>
Heh. No, no indeed, just got lost in the patch reordering. Will fix.<br><font color="#888888">
<br>
~Will<br>
</font></blockquote></div><br><div>Nicolas</div>