On 09/03/11 18:13, Alex Rousskov wrote:
On 03/08/2011 06:47 PM, Amos Jeffries wrote:
src/adaptation/Makefile.am:
Changed based on your comments below, but I am not sure I got all of
them correctly. Since most "natural" macro combination (IMHO) does not
work with libtool, I cannot improve it further on my own. If something
else needs to be done, please let me know.
* Why is an automake macro called *_LIBS being added to LDFLAGS?
Because the macro (EXTLIBECAP_LIBS) contains ld flags (-L and -l). The
macro is created by pkg-config, and Squid has no control over its name.
Furthermore, I suspect the _LIBS suffix is standard in this pkg-config
context.
Ah, -lecap a flag?
That is a MUST for adding to LDADD instead of FLAGS according to the
autotools docs. I found that while investigating kerberos helper build
failures on 3.1. Don't want to add those same failures again for ecap.
-L is apparently optionally usable in both FLAGS and LDADD. We in Squid
have a tendency to place it in LDADD immediately before the library
which uses it. But we take what pkg-config gives us on that.
Meh. Ignore me. LIBADD is not LDADD :(
I started with _LIBADD, but libtool fails when I add EXTLIBECAP_LIBS to
libxecap_la_LIBADD:
libtool: link: rm -fr .libs/libxecap.a .libs/libxecap.la
libtool: link: (cd .libs/libxecap.lax/libecap.a&& /usr/bin/ar x
"/home/rousskov/Edit/squid3/3p2-ecap/src/adaptation/ecap/.libs/libecap.a")
/usr/bin/ar:
/home/rousskov/Edit/squid3/3p2-ecap/src/adaptation/ecap/.libs/libecap.a:
No such file or directory
make: *** [libxecap.la] Error 9
The above is for adding EXTLIBECAP_LIBS to libxecap_la_LIBADD in
adaptation/ecap/Makefile.am, not in adaptation/Makefile.am. However,
adding EXTLIBECAP_LIBS to adaptation_la_LIBADD (in
adaptation/Makefile.am) seems to work. So that is what I did to address
your comment. Please let me know if that was wrong.
If you can make it work using a more natural arrangement, please let me
know how.
Looks like a dirty Makefile or build area to start with.
ar is trying to play with adaptation/ecap/libecap.la instead of the
renamed new adaptation/ecap/libxecap.la
That was my thought as well, after I renamed our library to understand
which libecap libtool is talking about. I did "make clean all" and
watched it succeed. After doing the celebratory dance, I did "touch
ServerRep.cc; make" which failed with the above error, despite all that
cleaning. I believe I tried "make distclean" as well. This would not be
the first time I am doing this libtool magic wrong, of course.
So, I have now built trunk, patched, then rebuilt in the same sandbox
builddir. NO problems.
The tests: make clean all + rebuild, touch a file and rebuild.
NO problems for either.
Altering the ecap/Makefile.am to uncomment the LIBADD set. And removing
EXT* from adaptation/Makefile.am. Rebuild, NO problems.
Repeat the clean and touch tests. NO problems.
This is on Ubuntu 10.9 (Maverick) with latest toolchains.
I think with this brief check the problem was an anomaly, possibly of
the shared "libecap" naming (resolved by your use of x) or of the
incorrect use of LDFLAGS.
I agree the linking into libxecap is probably best. I also feel the
alternative you used was suitable workaround. Whichever you want to
commit is fine.
Would you be happy using libecap-squid.la instead of libxecap.la on the
final commit? to make it clear which was the squid internal one.
src/cf.data.pre:
* while you are updating the documentation for
icap_client_username_header please correct the typo which mentions a
non-existent "send_username" to be "adaptation_send_username"
Out of scope. Will fix later.
Very much in scope IMO.
Alter the option name means altering all its public references to match,
even if they started off as typo versions of it.
We disagree because I try to limit ICAP changes in this project to
renaming/moving things that must be renamed or moved, even if they are
buggy (primarily because it makes it harder to introduce bugs and makes
it easier to review the changes) while you want to fix related ICAP bugs
(they are bugs after all!).
Since I promise to fix those bugs and polish that code anyway, I think
it is a minor disagreement.
Maybe. On the others yes.
On this config typo one I am considering what you would have done had
the typo not been there:
You would have seen it as icap_send_client_username and updated it to
adaptation_send_username.
Therefore; it is entirely relevant to rename the typo directly to
adaptation_send_username in this patch.
I said on IRC, but again in case you missed that.
When testing I found the HttpMsg.h hunk from line 220 altering Msg()
const-ness. Exists in the second complete bundle but is already part of
trunk.
Amos
--
Please be using
Current Stable Squid 2.7.STABLE9 or 3.1.11
Beta testers wanted for 3.2.0.5
