[kudu-CR] KUDU-2427: adjust gold linker detection
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10428 ) Change subject: KUDU-2427: adjust gold linker detection .. KUDU-2427: adjust gold linker detection This patch makes two adjustments to the existing gold linker detection: 1. Ubuntu 18.04's gcc 7 no longer respects a /usr/bin/ld symlink; it is hard-wired to use ld.bfd. Instead, we need to pass -fuse-ld=gold to choose the gold linker. 2. The old bug that led to tcmalloc being dropped from Kudu's dependency list has been fixed, so let's condition the workaround on the appropriate version of the gold linker. Change-Id: Ib1fae9893aaaf4916205d4c5ae6bb5c93e0505d0 Reviewed-on: http://gerrit.cloudera.org:8080/10428 Tested-by: Kudu Jenkins Reviewed-by: Mike Percy --- M CMakeLists.txt M src/kudu/codegen/CMakeLists.txt 2 files changed, 87 insertions(+), 21 deletions(-) Approvals: Kudu Jenkins: Verified Mike Percy: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/10428 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ib1fae9893aaaf4916205d4c5ae6bb5c93e0505d0 Gerrit-Change-Number: 10428 Gerrit-PatchSet: 5 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2427: adjust gold linker detection
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/10428 ) Change subject: KUDU-2427: adjust gold linker detection .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10428 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib1fae9893aaaf4916205d4c5ae6bb5c93e0505d0 Gerrit-Change-Number: 10428 Gerrit-PatchSet: 4 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 17 May 2018 23:44:55 + Gerrit-HasComments: No
[kudu-CR] KUDU-2427: adjust gold linker detection
Hello Mike Percy, Alexey Serbin, Kudu Jenkins, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10428 to look at the new patch set (#4). Change subject: KUDU-2427: adjust gold linker detection .. KUDU-2427: adjust gold linker detection This patch makes two adjustments to the existing gold linker detection: 1. Ubuntu 18.04's gcc 7 no longer respects a /usr/bin/ld symlink; it is hard-wired to use ld.bfd. Instead, we need to pass -fuse-ld=gold to choose the gold linker. 2. The old bug that led to tcmalloc being dropped from Kudu's dependency list has been fixed, so let's condition the workaround on the appropriate version of the gold linker. Change-Id: Ib1fae9893aaaf4916205d4c5ae6bb5c93e0505d0 --- M CMakeLists.txt M src/kudu/codegen/CMakeLists.txt 2 files changed, 87 insertions(+), 21 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/10428/4 -- To view, visit http://gerrit.cloudera.org:8080/10428 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib1fae9893aaaf4916205d4c5ae6bb5c93e0505d0 Gerrit-Change-Number: 10428 Gerrit-PatchSet: 4 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2427: adjust gold linker detection
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10428 ) Change subject: KUDU-2427: adjust gold linker detection .. Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/10428/3/CMakeLists.txt File CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/10428/3/CMakeLists.txt@479 PS3, Line 479: typically > really? I always use enable_devtoolset.sh on RHEL6 via some custom aliases, Nope. Technically you only need to run cmake within devtoolset-3; make and ninja needn't use it. See https://kudu.apache.org/docs/installation.html#rhel_from_source for more details. If make/ninja were also invoked inside devtoolset, this wouldn't be necessary. But I didn't feel comfortable requiring that workflow change, hence the workaround. http://gerrit.cloudera.org:8080/#/c/10428/3/CMakeLists.txt@484 PS3, Line 484: devtoolset-3 > if we must do this, maybe use string MATCHES regex Yeah I'll follow Todd's suggestion and use a prefix match via if(MATCHES). -- To view, visit http://gerrit.cloudera.org:8080/10428 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib1fae9893aaaf4916205d4c5ae6bb5c93e0505d0 Gerrit-Change-Number: 10428 Gerrit-PatchSet: 3 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 17 May 2018 21:59:06 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2427: adjust gold linker detection
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/10428 ) Change subject: KUDU-2427: adjust gold linker detection .. Patch Set 3: Code-Review+1 (2 comments) looks good although I'm not sure skipping gold on el6 is necessary http://gerrit.cloudera.org:8080/#/c/10428/3/CMakeLists.txt File CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/10428/3/CMakeLists.txt@479 PS3, Line 479: typically really? I always use enable_devtoolset.sh on RHEL6 via some custom aliases, but I thought that was required, i.e. enable_devtoolset.sh ninja http://gerrit.cloudera.org:8080/#/c/10428/3/CMakeLists.txt@484 PS3, Line 484: devtoolset-3 > It would be true for all of them. Should I make this a prefix match? if we must do this, maybe use string MATCHES regex -- To view, visit http://gerrit.cloudera.org:8080/10428 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib1fae9893aaaf4916205d4c5ae6bb5c93e0505d0 Gerrit-Change-Number: 10428 Gerrit-PatchSet: 3 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 17 May 2018 20:58:39 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2427: adjust gold linker detection
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10428 ) Change subject: KUDU-2427: adjust gold linker detection .. Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/10428/3/CMakeLists.txt File CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/10428/3/CMakeLists.txt@442 PS3, Line 442: to > drop Done http://gerrit.cloudera.org:8080/#/c/10428/3/CMakeLists.txt@442 PS3, Line 442: extract > extracts Done http://gerrit.cloudera.org:8080/#/c/10428/3/CMakeLists.txt@510 PS3, Line 510: Error: > Nit: does it add more clarity for the message? I saw another error message Sure, will drop it here. -- To view, visit http://gerrit.cloudera.org:8080/10428 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib1fae9893aaaf4916205d4c5ae6bb5c93e0505d0 Gerrit-Change-Number: 10428 Gerrit-PatchSet: 3 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 17 May 2018 17:41:06 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2427: adjust gold linker detection
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10428 ) Change subject: KUDU-2427: adjust gold linker detection .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/10428/3/CMakeLists.txt File CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/10428/3/CMakeLists.txt@484 PS3, Line 484: devtoolset-3 > is this dts-3 specific? or would also be true of devtoolset-7 for example? It would be true for all of them. Should I make this a prefix match? -- To view, visit http://gerrit.cloudera.org:8080/10428 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib1fae9893aaaf4916205d4c5ae6bb5c93e0505d0 Gerrit-Change-Number: 10428 Gerrit-PatchSet: 3 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 17 May 2018 17:38:43 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2427: adjust gold linker detection
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10428 ) Change subject: KUDU-2427: adjust gold linker detection .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/10428/3/CMakeLists.txt File CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/10428/3/CMakeLists.txt@484 PS3, Line 484: devtoolset-3 is this dts-3 specific? or would also be true of devtoolset-7 for example? -- To view, visit http://gerrit.cloudera.org:8080/10428 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib1fae9893aaaf4916205d4c5ae6bb5c93e0505d0 Gerrit-Change-Number: 10428 Gerrit-PatchSet: 3 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 17 May 2018 17:37:09 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2427: adjust gold linker detection
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/10428 ) Change subject: KUDU-2427: adjust gold linker detection .. Patch Set 3: Code-Review+1 (3 comments) http://gerrit.cloudera.org:8080/#/c/10428/3/CMakeLists.txt File CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/10428/3/CMakeLists.txt@442 PS3, Line 442: extract extracts http://gerrit.cloudera.org:8080/#/c/10428/3/CMakeLists.txt@442 PS3, Line 442: to drop http://gerrit.cloudera.org:8080/#/c/10428/3/CMakeLists.txt@510 PS3, Line 510: Error: Nit: does it add more clarity for the message? I saw another error message above, and it does not contain extra suffix like this. Maybe, for more consistency, add this suffix for the message at like 463 or drop it here. -- To view, visit http://gerrit.cloudera.org:8080/10428 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib1fae9893aaaf4916205d4c5ae6bb5c93e0505d0 Gerrit-Change-Number: 10428 Gerrit-PatchSet: 3 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 17 May 2018 17:32:11 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2427: adjust gold linker detection
Hello Mike Percy, Kudu Jenkins, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10428 to look at the new patch set (#2). Change subject: KUDU-2427: adjust gold linker detection .. KUDU-2427: adjust gold linker detection This patch makes two adjustments to the existing gold linker detection: 1. Ubuntu 18.04's gcc 7 no longer respects a /usr/bin/ld symlink; it is hard-wired to use ld.bfd. Instead, we need to pass -fuse-ld=gold to choose the gold linker. 2. The old bug that led to tcmalloc being dropped from Kudu's dependency list has been fixed, so let's condition the workaround on the appropriate version of the gold linker. Change-Id: Ib1fae9893aaaf4916205d4c5ae6bb5c93e0505d0 --- M CMakeLists.txt M src/kudu/codegen/CMakeLists.txt 2 files changed, 86 insertions(+), 21 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/10428/2 -- To view, visit http://gerrit.cloudera.org:8080/10428 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib1fae9893aaaf4916205d4c5ae6bb5c93e0505d0 Gerrit-Change-Number: 10428 Gerrit-PatchSet: 2 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2427: adjust gold linker detection
Hello Todd Lipcon, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/10428 to review the following change. Change subject: KUDU-2427: adjust gold linker detection .. KUDU-2427: adjust gold linker detection This patch makes two adjustments to the existing gold linker detection: 1. Ubuntu 18.04's gcc 7 no longer respects a /usr/bin/ld symlink; it is hard-wired to use ld.bfd. Instead, we need to pass -fuse-ld=gold to choose the gold linker. 2. The old bug that led to tcmalloc being dropped from Kudu's dependency list has been fixed, so let's condition the workaround on the appropriate version of the gold linker. Change-Id: Ib1fae9893aaaf4916205d4c5ae6bb5c93e0505d0 --- M CMakeLists.txt M src/kudu/codegen/CMakeLists.txt 2 files changed, 78 insertions(+), 21 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/10428/1 -- To view, visit http://gerrit.cloudera.org:8080/10428 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ib1fae9893aaaf4916205d4c5ae6bb5c93e0505d0 Gerrit-Change-Number: 10428 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Todd Lipcon
