Fix missing LDFLAGS when linking squid binary#2396
Fix missing LDFLAGS when linking squid binary#2396yadij wants to merge 1 commit intosquid-cache:masterfrom
Conversation
The auto-tools build system defines AM_LDFLAGS as the place for tools to add linker flags automatically determined to be needed, so as not to clobber user provide flags in LDFLAGS variable. This AM_* variable can be replaced in the linker command line for binary 'foo' by defining a replacement foo_LDFLAGS variable in Makefile.am. When third-party build and test tool use AM_LDFLAGS to pass flags to our build process additional flags the current squid_LDFLAGS ignores them.
rousskov
left a comment
rousskov
left a comment
There was a problem hiding this comment.
Thank you for explaining the problem in the PR description.
I have adjusted the last PR description paragraph for clarity (and to polish grammar). Please check that I got the original intent right.
| squid_LDFLAGS = -export-dynamic -dlopen force | ||
| squid_LDFLAGS = $(AM_LDFLAGS) -export-dynamic -dlopen force | ||
| ## when static module linking is supported and enabled: | ||
| ## squid_LDFLAGS = -all-static -dlopen self |
There was a problem hiding this comment.
If AM_LDFLAGS addition above is proven to be needed, please keep this commented out hint in sync (or remove it).
| ## squid_LDFLAGS = -all-static -dlopen self | |
| ## squid_LDFLAGS = $(AM_LDFLAGS) -all-static -dlopen self |
| squid_SOURCES += $(LOADABLE_MODULES_SOURCES) | ||
| squid_LDADD += -L$(top_builddir) $(LIBLTDL) | ||
| squid_LDFLAGS = -export-dynamic -dlopen force | ||
| squid_LDFLAGS = $(AM_LDFLAGS) -export-dynamic -dlopen force |
There was a problem hiding this comment.
PR description (with emphasis added by me): "AM_LDFLAGS [is] the place for tools to add linker flags automatically determined to be needed".
Autotools documentation (with emphasis added by me): "This is the variable the Makefile.am author can use to pass in additional linker flags"
AFAICT, the two quotes above contradict each other.
If autotools documentation is correct, then this PR is not needed because we (i.e. Makefile.am authors) do not define AM_LDFLAGS; we apparently do not want to "pass additional linker flags" (at this time) and cannot tell whether future "additional linker flags" (if any) would need to be added when linking squid executable.
If PR description is proven to be correct, then more Makefile.am changes are needed in this PR because, in that case, we need to add AM_LDFLAGS to all ~36 executables that use custom linking commands (e.g., testMath and testMem) and, hence, do not get AM_LDFLAGS auto-added by autotools today.
Before making widespread changes, let's determine what documentation is correct and confirm the need for this PR: In your environment, what "tool" adds "linker flags automatically determined to be needed" to AM_LDFLAGS?
rousskov
added
the
S-waiting-for-author
2 participants
The auto-tools build system defines AM_LDFLAGS as the place for
tools to add linker flags automatically determined to be needed,
so as not to clobber user provide flags in LDFLAGS variable.
This AM_* variable can be replaced in the linker command line
for binary 'foo' by defining a replacement foo_LDFLAGS variable
in Makefile.am.
When third-party build and test tools use AM_LDFLAGS to pass additional
flags to our build process, the current squid_LDFLAGS ignores them.