Wow, Justin, thanks a lot for all of your work! JavaHL is much
improved thanks to your genius build-fu!
> > 2102 [PATCH]
>
> r12016. Although the patch in IZ was wrong - it needed to depend on the
> $(_DEPS) variable not the target - the targets are empty and are always
> re-executed. So, when the empty target is a dependency, it forces a rebuild.
> Yikes.
Good idea. Although the patch for 2103 should fix this issue, it's
generally a better idea to do it the way you suggested. Thanks.
> > 2103 [PATCH]
>
> r12018
>
> FWIW, this one took a long time for me to feel confident in.
Thanks for your work and your little tweaks that make all the difference :)
> > 2040 [PATCH]
>
> I really don't like this patch. The fact that you can even run a test before
> installing is part of GNU libtool's downfall. [ ... ]
>
> That said, apr-*-config has the right logic for doing these shlib-var checks
> instead of assuming that libtool's m4 script runs and has some shell variables
> set as a side effect. So, the patch should be modified to call out to
> apr-config to get this.
Aha. Good point. I'll switch to using "$apr_config --shlib-path-var" instead.
Looking at the patch again myself, I've found two areas where it can improve:
1. I shouldn't have the svnjavahl library filename depend on a fake
target (fix-javahl-lib). This trick causes us to recompile
unnecessarily. I've fixed this by using the filename as the name of
the target instead.
2. Instead of just hardcoding the exact path to the libsvnjavahl
library inside the Makefile.in file, I should generate it inside
build/generator/gen_make.py. I've found a really slick way of doing
this and the code looks better.
If I make the above two changes plus the apr-config change, would the
patch acquire a higher priority? Or is there more work that needs to
be done?
I'll also note here that Ben Reser has endorsed the general idea of
this LIBTOOL trick and uses it for the SWIG Perl bindings.
http://www.contactor.se/~dast/svn/archive-2004-09/0107.shtml
http://www.contactor.se/~dast/svn/archive-2004-09/0401.shtml
http://www.contactor.se/~dast/svn/archive-2004-09/0402.shtml
> If I get some time, I'll try to take a look at this, but this is ultra-low
> priority. (P2 is incorrect, IMHO. The workaround is to just install first.)
P2 is "Workaround required to get stuff done". In this case, in order
to test, we need to workaround this bug by installing first. If you
still think this issue won't be solved before 1.2, then let me know
and I'll post a patch to the documentation instead.
> > 1632
> This is fixed. I wouldn't even recommend testing it. Just close it. =)
Great! I've closed the issue.
--
http://www.cs.toronto.edu/~james
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Nov 24 19:49:36 2004