{"thread":{"id":"31768","subject":"[PATCH] configure.ac: Add missing comma to CC_LD_DYNPATH","startedAt":"2012-10-09T16:36:12Z","lastAt":"2012-10-09T21:10:46Z","messageCount":4,"participants":["Øyvind A. Holm","Stefano Lattarini"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"200839","messageId":"1349800572-2963-1-git-send-email-sunny@sunbase.org","threadId":"31768","inReplyTo":"1349800026-10717-1-git-send-email-sunny@sunbase.org","subject":"[PATCH] configure.ac: Add missing comma to CC_LD_DYNPATH","fromName":"Øyvind A. Holm","fromEmail":"sunny@sunbase.org","sentAt":"2012-10-09T16:36:12Z","receivedAt":"2012-10-09T16:36:12Z","isPatch":true,"sender":{"key":"sunny@sunbase.org","avatar":"https://avatars.githubusercontent.com/u/113445?v=4"},"body":"From: \"Øyvind A. Holm\" \n\n40bfbde (\"build: don't duplicate substitution of make variables\",\n2012-09-11) breaks make by removing a necessary comma at the end of\n\"CC_LD_DYNPATH=-rpath\" in line 414 and 423.\n\nWhen executing \"./configure --with-zlib=PATH\", this resulted in\n\n [...]\n CC xdiff/xhistogram.o\n AR xdiff/lib.a\n LINK git-credential-store\n /usr/bin/ld: bad -rpath option\n collect2: ld returned 1 exit status\n make: *** [git-credential-store] Error 1\n $\n\nduring make.\n\nSigned-off-by: Øyvind A. Holm \n---\n configure.ac | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/configure.ac b/configure.ac\nindex da1f41f..ea79ea2 100644\n--- a/configure.ac\n+++ b/configure.ac\n@@ -411,7 +411,7 @@ else\n LDFLAGS=\"${SAVE_LDFLAGS}\"\n ])\n if test \"$git_cv_ld_wl_rpath\" = \"yes\"; then\n- CC_LD_DYNPATH=-Wl,-rpath\n+ CC_LD_DYNPATH=-Wl,-rpath,\n else\n AC_CACHE_CHECK([if linker supports -rpath], git_cv_ld_rpath, [\n SAVE_LDFLAGS=\"${LDFLAGS}\"\n@@ -420,7 +420,7 @@ else\n LDFLAGS=\"${SAVE_LDFLAGS}\"\n ])\n if test \"$git_cv_ld_rpath\" = \"yes\"; then\n- CC_LD_DYNPATH=-rpath\n+ CC_LD_DYNPATH=-rpath,\n else\n CC_LD_DYNPATH=\n AC_MSG_WARN([linker does not support runtime path to dynamic libraries])\n-- \n1.8.0.rc1\n"},{"id":"200841","messageId":"CAA787rmpTe1L4TY41X9Szt8FS9T7bYRA2sgb=K_st4Ck2N9M1Q@mail.gmail.com","threadId":"31768","inReplyTo":"1349800572-2963-1-git-send-email-sunny@sunbase.org","subject":"Re: [PATCH] configure.ac: Add missing comma to CC_LD_DYNPATH","fromName":"Øyvind A. Holm","fromEmail":"sunny@sunbase.org","sentAt":"2012-10-09T16:40:29Z","receivedAt":"2012-10-09T16:40:29Z","isPatch":true,"sender":{"key":"sunny@sunbase.org","avatar":"https://avatars.githubusercontent.com/u/113445?v=4"},"body":"Please discard the first patch, I reckon line 423 also should be changed.\n\nSorry about the noise,\nØyvind\n"},{"id":"200845","messageId":"CAA787rnXw2fR5DG5bhz9uF4ub8=jZgUvpJLVGJ5sGXGSF8VMEQ@mail.gmail.com","threadId":"31768","inReplyTo":"7v4nm3h8yr.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] configure.ac: Add missing comma to CC_LD_DYNPATH","fromName":"Øyvind A. Holm","fromEmail":"sunny@sunbase.org","sentAt":"2012-10-09T17:23:54Z","receivedAt":"2012-10-09T17:23:54Z","isPatch":true,"sender":{"key":"sunny@sunbase.org","avatar":"https://avatars.githubusercontent.com/u/113445?v=4"},"body":"On 9 October 2012 19:05, Junio C Hamano wrote:\n> Øyvind A. Holm writes:\n> > 40bfbde (\"build: don't duplicate substitution of make variables\",\n> > 2012-09-11) breaks make by removing a necessary comma at the end of\n> > \"CC_LD_DYNPATH=-rpath\" in line 414 and 423.\n>\n> The earlier one is a cut-and-paste-error regression.\n>\n> Isn't the one at line 423 from before 40bfbde, though? If that is the\n> case, I'm a bit hesitant to take that part of this patch without a\n> second opinion.\n\nIt looks like it is, yes. More accurately, from 798a945 way back in\n2008. If it hasn't caused any trouble since then, it probably won't. :)\nThe line was changed in 40bfbde, though, but AC_SUBST doesn't contain a\ncomma, so to be on the safe side, the first patch should be used.\n\nBut I made a minor copy+paste error in the commit message of that patch:\n\n \"CC_LD_DYNPATH=-rpath\" in line 414.\n\nshould be\n\n \"CC_LD_DYNPATH=-Wl,-rpath\" in line 414.\n\nJust a minor, but slightly annoying detail.\n\nRegards,\nØyvind\n"},{"id":"200861","messageId":"507492D6.3090207@gmail.com","threadId":"31768","inReplyTo":"1349800572-2963-1-git-send-email-sunny@sunbase.org","subject":"Re: [PATCH] configure.ac: Add missing comma to CC_LD_DYNPATH","fromName":"Stefano Lattarini","fromEmail":"stefano.lattarini@gmail.com","sentAt":"2012-10-09T21:10:46Z","receivedAt":"2012-10-09T21:10:46Z","isPatch":true,"sender":{"key":"stefano.lattarini@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1429199?v=4"},"body":"[Re-sending because I forgot to CC: the list, sorry]\n\nOn 10/09/2012 06:36 PM, Øyvind A. Holm wrote:\n> From: \"Øyvind A. Holm\" \n>\n> 40bfbde (\"build: don't duplicate substitution of make variables\",\n> 2012-09-11)\n>\nOops, stupid copy and paste error on my part. Sorry.\n\n> breaks make by removing a necessary comma at the end of\n> \"CC_LD_DYNPATH=-rpath\" in line 414 and 423.\n>\nHere, s/-rpath/-Wl,-rpath/, as you've noted yourself in a follow-up\nmessage. And the reference to \"line 423\" should be removed.\n\nAlso, as a very minor nit, I'd write \"might break make\" rather then\n\"breaks make\", because the breakage depends on which code path is\ntaken at configure time (and that's why I hadn't noticed the error\nuntil now -- I never ran configure with the '--with-zlib' option).\n\n> When executing \"./configure --with-zlib=PATH\", this resulted in\n>\n> [...]\n> CC xdiff/xhistogram.o\n> AR xdiff/lib.a\n> LINK git-credential-store\n> /usr/bin/ld: bad -rpath option\n> collect2: ld returned 1 exit status\n> make: *** [git-credential-store] Error 1\n> $\n>\n> during make.\n>\nIndeed, I can reproduce and confirm this error :-(\n\n> Signed-off-by: Øyvind A. Holm \n> ---\n> configure.ac | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/configure.ac b/configure.ac\n> index da1f41f..ea79ea2 100644\n> --- a/configure.ac\n> +++ b/configure.ac\n> @@ -411,7 +411,7 @@ else\n> LDFLAGS=\"${SAVE_LDFLAGS}\"\n> ])\n> if test \"$git_cv_ld_wl_rpath\" = \"yes\"; then\n> - CC_LD_DYNPATH=-Wl,-rpath\n> + CC_LD_DYNPATH=-Wl,-rpath,\n> else\n> AC_CACHE_CHECK([if linker supports -rpath], git_cv_ld_rpath, [\n> SAVE_LDFLAGS=\"${LDFLAGS}\"\n> @@ -420,7 +420,7 @@ else\n> LDFLAGS=\"${SAVE_LDFLAGS}\"\n> ])\n> if test \"$git_cv_ld_rpath\" = \"yes\"; then\n> - CC_LD_DYNPATH=-rpath\n> + CC_LD_DYNPATH=-rpath,\n>\nAnd as Junio noted, this second hunk is unneeded, and in fact wrong.\nJust remove it please.\n\nWith that done,\nAcked-by: Stefano Lattarini \n\n> else\n> CC_LD_DYNPATH=\n> AC_MSG_WARN([linker does not support runtime path to dynamic libraries])\nThanks,\n Stefano\n"}]}