You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2021/02/10 06:55:38 UTC

[kudu-CR] [thirdparty] use system curl for cmake

Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17053


Change subject: [thirdparty] use system curl for cmake
......................................................................

[thirdparty] use system curl for cmake

Since building Kudu from source requires curl to be installed,
let's rely on that system package when building cmake.  In addition
to reducing the build time of the third-party components, it also
helps avoiding build failures in environments where multiple
OpenSSL versions are installed.

I guess a proper fix would be updating cmake configure/build scripts
and makefiles, but I'm not sure it's worth it: removing an extra
component from the build gives the benefit of shorter build times.

Change-Id: I10a22e1e864b19491177c25a352e2378a6f6dea3
---
M thirdparty/build-definitions.sh
1 file changed, 1 insertion(+), 0 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/17053/1
-- 
To view, visit http://gerrit.cloudera.org:8080/17053
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I10a22e1e864b19491177c25a352e2378a6f6dea3
Gerrit-Change-Number: 17053
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>

[kudu-CR] [thirdparty] use system curl for cmake

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17053 )

Change subject: [thirdparty] use system curl for cmake
......................................................................

[thirdparty] use system curl for cmake

Since building Kudu from source requires curl to be installed,
let's rely on that system package when building cmake.  In addition
to reducing the build time of the third-party components, it also
helps avoiding build failures in environments where multiple
OpenSSL versions are installed.

I guess a proper fix would be updating cmake configure/build scripts
and makefiles, but I'm not sure it's worth it: removing an extra
component from the build gives the benefit of shorter build times.

Change-Id: I10a22e1e864b19491177c25a352e2378a6f6dea3
Reviewed-on: http://gerrit.cloudera.org:8080/17053
Tested-by: Kudu Jenkins
Reviewed-by: Grant Henke <gr...@apache.org>
---
M thirdparty/build-definitions.sh
1 file changed, 6 insertions(+), 0 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Grant Henke: Looks good to me, approved

-- 
To view, visit http://gerrit.cloudera.org:8080/17053
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I10a22e1e864b19491177c25a352e2378a6f6dea3
Gerrit-Change-Number: 17053
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] [thirdparty] use system curl for cmake

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17053 )

Change subject: [thirdparty] use system curl for cmake
......................................................................


Patch Set 1:

> Build Failed
 > 
 > http://jenkins.kudu.apache.org/job/kudu-gerrit/23328/ : FAILURE

Heh, it seems at build servers there isn't any curl installed.  Surprising, eh?


-- 
To view, visit http://gerrit.cloudera.org:8080/17053
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I10a22e1e864b19491177c25a352e2378a6f6dea3
Gerrit-Change-Number: 17053
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 10 Feb 2021 16:32:05 +0000
Gerrit-HasComments: No

[kudu-CR] [thirdparty] use system curl for cmake

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/17053 )

Change subject: [thirdparty] use system curl for cmake
......................................................................


Patch Set 2: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/17053
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I10a22e1e864b19491177c25a352e2378a6f6dea3
Gerrit-Change-Number: 17053
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 10 Feb 2021 19:51:19 +0000
Gerrit-HasComments: No

[kudu-CR] [thirdparty] use system curl for cmake

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17053 )

Change subject: [thirdparty] use system curl for cmake
......................................................................


Patch Set 2:

> I looks like this change "poisoned" the Jenkins build. I will clear
 > the working directories.

Thank you!

I posted a fix already: that's about sensing if libcurl is present, and if so, build cmake with it (i.e. add --system-curl flag to bootstrap), otherwise build cmcurl with built-in curl.


-- 
To view, visit http://gerrit.cloudera.org:8080/17053
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I10a22e1e864b19491177c25a352e2378a6f6dea3
Gerrit-Change-Number: 17053
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 10 Feb 2021 17:48:48 +0000
Gerrit-HasComments: No

[kudu-CR] [thirdparty] use system curl for cmake

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong, Grant Henke, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/17053

to look at the new patch set (#2).

Change subject: [thirdparty] use system curl for cmake
......................................................................

[thirdparty] use system curl for cmake

Since building Kudu from source requires curl to be installed,
let's rely on that system package when building cmake.  In addition
to reducing the build time of the third-party components, it also
helps avoiding build failures in environments where multiple
OpenSSL versions are installed.

I guess a proper fix would be updating cmake configure/build scripts
and makefiles, but I'm not sure it's worth it: removing an extra
component from the build gives the benefit of shorter build times.

Change-Id: I10a22e1e864b19491177c25a352e2378a6f6dea3
---
M thirdparty/build-definitions.sh
1 file changed, 6 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/17053/2
-- 
To view, visit http://gerrit.cloudera.org:8080/17053
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I10a22e1e864b19491177c25a352e2378a6f6dea3
Gerrit-Change-Number: 17053
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] [thirdparty] use system curl for cmake

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/17053 )

Change subject: [thirdparty] use system curl for cmake
......................................................................


Patch Set 1:

I could update the jenkins docker images to add it. As an alternative can we check if one exists and use it if so and otherwise fall back to thirdparty? I guess this is similar to how we handle python.


-- 
To view, visit http://gerrit.cloudera.org:8080/17053
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I10a22e1e864b19491177c25a352e2378a6f6dea3
Gerrit-Change-Number: 17053
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 10 Feb 2021 16:33:58 +0000
Gerrit-HasComments: No

[kudu-CR] [thirdparty] use system curl for cmake

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/17053 )

Change subject: [thirdparty] use system curl for cmake
......................................................................


Patch Set 1:

I looks like this change "poisoned" the Jenkins build. I will clear the working directories.


-- 
To view, visit http://gerrit.cloudera.org:8080/17053
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I10a22e1e864b19491177c25a352e2378a6f6dea3
Gerrit-Change-Number: 17053
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 10 Feb 2021 17:15:08 +0000
Gerrit-HasComments: No