You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Joe McDonnell (Code Review)" <ge...@cloudera.org> on 2021/02/25 02:10:03 UTC
[native-toolchain-CR] IMPALA-10488: Add jwt-cpp 0.5.0 to the toolchain
Joe McDonnell has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17118
Change subject: IMPALA-10488: Add jwt-cpp 0.5.0 to the toolchain
......................................................................
IMPALA-10488: Add jwt-cpp 0.5.0 to the toolchain
This adds a JSON Web Token (JWT) library for decoding and
verifying JWTs. This requires OpenSSL 1.0.2.
Tests:
- Ran a build on all platforms
Change-Id: I77aa3b36b45e8ef3c2d7873327948197c2c65d11
---
M buildall.sh
A source/jwt-cpp/build.sh
2 files changed, 43 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/native-toolchain refs/changes/18/17118/1
--
To view, visit http://gerrit.cloudera.org:8080/17118
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I77aa3b36b45e8ef3c2d7873327948197c2c65d11
Gerrit-Change-Number: 17118
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
[native-toolchain-CR] IMPALA-10488: Add jwt-cpp 0.5.0 to the toolchain
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Hello Csaba Ringhofer,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17118
to look at the new patch set (#4).
Change subject: IMPALA-10488: Add jwt-cpp 0.5.0 to the toolchain
......................................................................
IMPALA-10488: Add jwt-cpp 0.5.0 to the toolchain
This adds a JSON Web Token (JWT) library for decoding and
verifying JWTs. This requires OpenSSL 1.0.2.
Tests:
- Ran a build on all platforms
Change-Id: I77aa3b36b45e8ef3c2d7873327948197c2c65d11
---
M buildall.sh
A source/jwt-cpp/build.sh
2 files changed, 43 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/native-toolchain refs/changes/18/17118/4
--
To view, visit http://gerrit.cloudera.org:8080/17118
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I77aa3b36b45e8ef3c2d7873327948197c2c65d11
Gerrit-Change-Number: 17118
Gerrit-PatchSet: 4
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
[native-toolchain-CR] IMPALA-10488: Add jwt-cpp 0.5.0 to the toolchain
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17118 )
Change subject: IMPALA-10488: Add jwt-cpp 0.5.0 to the toolchain
......................................................................
IMPALA-10488: Add jwt-cpp 0.5.0 to the toolchain
This adds a JSON Web Token (JWT) library for decoding and
verifying JWTs. This requires OpenSSL 1.0.2.
Tests:
- Ran a build on all platforms
Change-Id: I77aa3b36b45e8ef3c2d7873327948197c2c65d11
Reviewed-on: http://gerrit.cloudera.org:8080/17118
Reviewed-by: Wenzhe Zhou <wz...@cloudera.com>
Reviewed-by: Joe McDonnell <jo...@cloudera.com>
Tested-by: Joe McDonnell <jo...@cloudera.com>
---
M buildall.sh
A source/jwt-cpp/build.sh
2 files changed, 43 insertions(+), 0 deletions(-)
Approvals:
Wenzhe Zhou: Looks good to me, but someone else must approve
Joe McDonnell: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/17118
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I77aa3b36b45e8ef3c2d7873327948197c2c65d11
Gerrit-Change-Number: 17118
Gerrit-PatchSet: 5
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[native-toolchain-CR] IMPALA-10488: Add jwt-cpp 0.5.0 to the toolchain
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/17118 )
Change subject: IMPALA-10488: Add jwt-cpp 0.5.0 to the toolchain
......................................................................
Patch Set 4: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/17118
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77aa3b36b45e8ef3c2d7873327948197c2c65d11
Gerrit-Change-Number: 17118
Gerrit-PatchSet: 4
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 29 Apr 2021 20:41:04 +0000
Gerrit-HasComments: No
[native-toolchain-CR] IMPALA-10488: Add jwt-cpp 0.5.0 to the toolchain
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/17118 )
Change subject: IMPALA-10488: Add jwt-cpp 0.5.0 to the toolchain
......................................................................
Patch Set 3:
This adds a C++ JWT library, but Impala doesn't use it yet. It doesn't impact anything else.
--
To view, visit http://gerrit.cloudera.org:8080/17118
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77aa3b36b45e8ef3c2d7873327948197c2c65d11
Gerrit-Change-Number: 17118
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Apr 2021 04:43:24 +0000
Gerrit-HasComments: No
[native-toolchain-CR] IMPALA-10488: Add jwt-cpp 0.5.0 to the toolchain
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/17118 )
Change subject: IMPALA-10488: Add jwt-cpp 0.5.0 to the toolchain
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17118/3/source/jwt-cpp/build.sh
File source/jwt-cpp/build.sh:
http://gerrit.cloudera.org:8080/#/c/17118/3/source/jwt-cpp/build.sh@33
PS3, Line 33: # jwt-cpp is currently header-only, so it really is only copying files around
> Should we add header only libraries to native toolchain?
I think it is fairly harmless for native toolchain to have header only libraries. There are some nice properties about doing this through the native toolchain:
- native toolchain builds on all the Linux versions. If the header-only projects have validations through CMake, then those validations confirm that there isn't anything incompatible with all the Linux versions we support.
- The version of the library is clear. Any patches we do on top of it are kept separately.
I think I prefer this to the be/src/thirdparty approach.
I think there are options that may get the best of both. There are some CMake extensions that can go fetch a project as of a particular revision and build it (see https://cmake.org/cmake/help/latest/module/ExternalProject.html ). If we had something like that working in the Impala build system, I would use it.
I'm going to split this off from the stack with the compression changes since I don't need it yet.
--
To view, visit http://gerrit.cloudera.org:8080/17118
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77aa3b36b45e8ef3c2d7873327948197c2c65d11
Gerrit-Change-Number: 17118
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Thu, 08 Apr 2021 03:10:22 +0000
Gerrit-HasComments: Yes
[native-toolchain-CR] IMPALA-10488: Add jwt-cpp 0.5.0 to the toolchain
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/17118 )
Change subject: IMPALA-10488: Add jwt-cpp 0.5.0 to the toolchain
......................................................................
Patch Set 4: Verified+1 Code-Review+2
Carrying +2. This passed a toolchain build, so merging. It doesn't interfere with any other changes to the toolchain.
--
To view, visit http://gerrit.cloudera.org:8080/17118
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77aa3b36b45e8ef3c2d7873327948197c2c65d11
Gerrit-Change-Number: 17118
Gerrit-PatchSet: 4
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 13 May 2021 00:43:26 +0000
Gerrit-HasComments: No
[native-toolchain-CR] IMPALA-10488: Add jwt-cpp 0.5.0 to the toolchain
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/17118 )
Change subject: IMPALA-10488: Add jwt-cpp 0.5.0 to the toolchain
......................................................................
IMPALA-10488: Add jwt-cpp 0.5.0 to the toolchain
This adds a JSON Web Token (JWT) library for decoding and
verifying JWTs. This requires OpenSSL 1.0.2.
Tests:
- Ran a toolchain build on all platforms
- Pulled into an Impala development environment
Change-Id: I77aa3b36b45e8ef3c2d7873327948197c2c65d11
---
M buildall.sh
A source/jwt-cpp/build.sh
2 files changed, 43 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/native-toolchain refs/changes/18/17118/3
--
To view, visit http://gerrit.cloudera.org:8080/17118
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I77aa3b36b45e8ef3c2d7873327948197c2c65d11
Gerrit-Change-Number: 17118
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
[native-toolchain-CR] IMPALA-10488: Add jwt-cpp 0.5.0 to the toolchain
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/17118 )
Change subject: IMPALA-10488: Add jwt-cpp 0.5.0 to the toolchain
......................................................................
Patch Set 3: Code-Review+2
>here are some CMake extensions that can go fetch a project as of a particular revision and build it
I agree, that would be the best. It adds one more step that can fail, but if github doesn't work, then we wouldn't even get to that step.
--
To view, visit http://gerrit.cloudera.org:8080/17118
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77aa3b36b45e8ef3c2d7873327948197c2c65d11
Gerrit-Change-Number: 17118
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Mon, 12 Apr 2021 10:17:14 +0000
Gerrit-HasComments: No
[native-toolchain-CR] IMPALA-10488: Add jwt-cpp 0.5.0 to the toolchain
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/17118 )
Change subject: IMPALA-10488: Add jwt-cpp 0.5.0 to the toolchain
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17118/3/source/jwt-cpp/build.sh
File source/jwt-cpp/build.sh:
http://gerrit.cloudera.org:8080/#/c/17118/3/source/jwt-cpp/build.sh@33
PS3, Line 33: # jwt-cpp is currently header-only, so it really is only copying files around
Should we add header only libraries to native toolchain?
We had this dilemma when adding data sketches, and in the end we simply copied the header files to https://github.com/apache/impala/tree/master/be/src/thirdparty
I don't have a clear preference here, just curious about your opinion.
--
To view, visit http://gerrit.cloudera.org:8080/17118
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77aa3b36b45e8ef3c2d7873327948197c2c65d11
Gerrit-Change-Number: 17118
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Apr 2021 07:10:59 +0000
Gerrit-HasComments: Yes