You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Yingchun Lai (Code Review)" <ge...@cloudera.org> on 2021/11/11 11:29:35 UTC
[kudu-CR] [build] Introduce an env variable to indicate the thirdparty path
Yingchun Lai has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18018
Change subject: [build] Introduce an env variable to indicate the thirdparty path
......................................................................
[build] Introduce an env variable to indicate the thirdparty path
Kudu thirdparty will cost dozens of minutes to build, now introduce
an env variable THIRDPARTY_DIR to indicate the thirdparty path, it
can be a well built thirdparty libraries path.
With this feature, we can
- Share the same thirdparty path by different code pathes, these code
pathes are not needed to build thirdparty libraries repeatly
- Build an docker image with thirdparty libraries well built
Change-Id: I5e0bdf4faa44322622c48cacadf8e1165eccd38a
---
M CMakeLists.txt
M build-support/ccache-clang/clang
M build-support/ccache-clang/clang++
M build-support/clang_tidy_gerrit.py
M build-support/iwyu.py
M build-support/iwyu/iwyu_tool.py
M build-support/jenkins/build-and-test.sh
M build-support/kudu_util.py
M build-support/lint.sh
M build-support/llvm-gcov-wrapper
M build-support/mini-cluster/build_mini_cluster_binaries.sh
M src/kudu/clock/CMakeLists.txt
M src/kudu/hms/CMakeLists.txt
M src/kudu/postgres/CMakeLists.txt
M src/kudu/ranger/CMakeLists.txt
M src/kudu/scripts/dump_breakpad_symbols.py
16 files changed, 69 insertions(+), 33 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/18/18018/1
--
To view, visit http://gerrit.cloudera.org:8080/18018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5e0bdf4faa44322622c48cacadf8e1165eccd38a
Gerrit-Change-Number: 18018
Gerrit-PatchSet: 1
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [build] Introduce an env variable to indicate the thirdparty path
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/18018 )
Change subject: [build] Introduce an env variable to indicate the thirdparty path
......................................................................
Patch Set 4: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/18018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5e0bdf4faa44322622c48cacadf8e1165eccd38a
Gerrit-Change-Number: 18018
Gerrit-PatchSet: 4
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Tue, 16 Nov 2021 09:25:55 +0000
Gerrit-HasComments: No
[kudu-CR] [build] Introduce an env variable to indicate the thirdparty path
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/18018 )
Change subject: [build] Introduce an env variable to indicate the thirdparty path
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/18018/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18018/2//COMMIT_MSG@15
PS2, Line 15: - Build an docker image with thirdparty libraries well built, then we
> The Kudu docker doc [1] says images like 'apache/kudu:thirdparty-[OS]-[VERS
I'd discussed this with Grant when he contributed the Docker deployments. IIRC, he explicitly opted not to package thirdparty and store it in DockerHub because of space concerns. I.e. the thirdparty Docker build should be functional today, we just don't publish to DockerHub because in general, the DockerHub images are very stripped down and are the bare minimum to get Kudu running.
That isn't to say we shouldn't host a thirdparty build, but it does come with some other questions. Right now, DockerHub only hosts released versions. If we upload a thirdparty image, should that change? Or should we upload a new thirdparty image every time thirdparty changes? If the goal is developer velocity, or faster pre-commits, it'd be important that the thirdparty image be as up-to-date as possible. But that could also mean it's more confusing to reason about what versions are hosted on DockerHub.
Specifically regarding faster pre-commits in a CI/CD pipeline, one approach that we've taken is to have a couple of long-lived machines that keep a long-lived workspace when building Kudu. Upon building a patched version of Kudu in this workspace, Kudu's build detects whether anything has changed in thirdparty compared to the cached version, and potentially doesn't build anything.
http://gerrit.cloudera.org:8080/#/c/18018/2/build-support/jenkins/build-and-test.sh
File build-support/jenkins/build-and-test.sh:
http://gerrit.cloudera.org:8080/#/c/18018/2/build-support/jenkins/build-and-test.sh@314
PS2, Line 314:
: if [ ! -n "${THIRDPARTY_DIR}" ]; then
: THIRDPARTY_DIR=$SOURCE_ROOT/thirdparty
: fi
nit: consider using the same syntax used elsewhere? Same in other files, if you want. E.g. similar to
export KUDU_FLAKY_TEST_ATTEMPTS=${KUDU_FLAKY_TEST_ATTEMPTS:-1}
export KUDU_ALLOW_SLOW_TESTS=${KUDU_ALLOW_SLOW_TESTS:-$DEFAULT_ALLOW_SLOW_TESTS}
export KUDU_COMPRESS_TEST_OUTPUT=${KUDU_COMPRESS_TEST_OUTPUT:-1}
export TEST_TMPDIR=${TEST_TMPDIR:-/tmp/kudutest-$UID}
export PARALLEL=${PARALLEL:-$(getconf _NPROCESSORS_ONLN)}
export PARALLEL_TESTS=${PARALLEL_TESTS:-$PARALLEL}
--
To view, visit http://gerrit.cloudera.org:8080/18018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5e0bdf4faa44322622c48cacadf8e1165eccd38a
Gerrit-Change-Number: 18018
Gerrit-PatchSet: 3
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Fri, 12 Nov 2021 19:33:43 +0000
Gerrit-HasComments: Yes
[kudu-CR] [build] Introduce an env variable to indicate the thirdparty path
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/18018 )
Change subject: [build] Introduce an env variable to indicate the thirdparty path
......................................................................
[build] Introduce an env variable to indicate the thirdparty path
Kudu thirdparty will cost dozens of minutes to build, now introduce
an env variable THIRDPARTY_DIR to indicate the thirdparty path, it
can be a well built thirdparty libraries path.
With this feature, we can
- Share the same thirdparty path by different code paths, these code
paths are not needed to build thirdparty libraries repeatly
- Build an docker image with thirdparty libraries well built, then we
can use this image for building Kudu, CI, and etc.
Change-Id: I5e0bdf4faa44322622c48cacadf8e1165eccd38a
Reviewed-on: http://gerrit.cloudera.org:8080/18018
Tested-by: Kudu Jenkins
Reviewed-by: Attila Bukor <ab...@apache.org>
Reviewed-by: Andrew Wong <aw...@cloudera.com>
---
M CMakeLists.txt
M build-support/ccache-clang/clang
M build-support/ccache-clang/clang++
M build-support/clang_tidy_gerrit.py
M build-support/iwyu.py
M build-support/iwyu/iwyu_tool.py
M build-support/jenkins/build-and-test.sh
M build-support/kudu_util.py
M build-support/lint.sh
M build-support/llvm-gcov-wrapper
M build-support/mini-cluster/build_mini_cluster_binaries.sh
M docs/support/scripts/make_site.sh
M src/kudu/clock/CMakeLists.txt
M src/kudu/hms/CMakeLists.txt
M src/kudu/postgres/CMakeLists.txt
M src/kudu/ranger/CMakeLists.txt
M src/kudu/scripts/dump_breakpad_symbols.py
17 files changed, 66 insertions(+), 37 deletions(-)
Approvals:
Kudu Jenkins: Verified
Attila Bukor: Looks good to me, but someone else must approve
Andrew Wong: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/18018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I5e0bdf4faa44322622c48cacadf8e1165eccd38a
Gerrit-Change-Number: 18018
Gerrit-PatchSet: 5
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [build] Introduce an env variable to indicate the thirdparty path
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18018 )
Change subject: [build] Introduce an env variable to indicate the thirdparty path
......................................................................
Patch Set 4:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/18018/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18018/2//COMMIT_MSG@15
PS2, Line 15: - Build an docker image with thirdparty libraries well built, then we
> I'd discussed this with Grant when he contributed the Docker deployments. I
I agree not to provide thirdparty images on DockerHub, but I think provide a DockerFile to build such a image is reasonable, although the images will be outdated when thirdparties changes, but thirdparties are not changed frequently.
Yes, cache is a way to reduce building time in CI/CD pipeline, but it's not the only use case.
http://gerrit.cloudera.org:8080/#/c/18018/2/build-support/jenkins/build-and-test.sh
File build-support/jenkins/build-and-test.sh:
http://gerrit.cloudera.org:8080/#/c/18018/2/build-support/jenkins/build-and-test.sh@314
PS2, Line 314: THIRDPARTY_TYPE=tsan
: fi
:
: if
> +1
Done
--
To view, visit http://gerrit.cloudera.org:8080/18018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5e0bdf4faa44322622c48cacadf8e1165eccd38a
Gerrit-Change-Number: 18018
Gerrit-PatchSet: 4
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Sat, 13 Nov 2021 05:00:46 +0000
Gerrit-HasComments: Yes
[kudu-CR] [build] Introduce an env variable to indicate the thirdparty path
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Attila Bukor, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18018
to look at the new patch set (#3).
Change subject: [build] Introduce an env variable to indicate the thirdparty path
......................................................................
[build] Introduce an env variable to indicate the thirdparty path
Kudu thirdparty will cost dozens of minutes to build, now introduce
an env variable THIRDPARTY_DIR to indicate the thirdparty path, it
can be a well built thirdparty libraries path.
With this feature, we can
- Share the same thirdparty path by different code paths, these code
pathes are not needed to build thirdparty libraries repeatly
- Build an docker image with thirdparty libraries well built, then we
can use this image for building Kudu, CI, and etc.
Change-Id: I5e0bdf4faa44322622c48cacadf8e1165eccd38a
---
M CMakeLists.txt
M build-support/ccache-clang/clang
M build-support/ccache-clang/clang++
M build-support/clang_tidy_gerrit.py
M build-support/iwyu.py
M build-support/iwyu/iwyu_tool.py
M build-support/jenkins/build-and-test.sh
M build-support/kudu_util.py
M build-support/lint.sh
M build-support/llvm-gcov-wrapper
M build-support/mini-cluster/build_mini_cluster_binaries.sh
M docs/support/scripts/make_site.sh
M src/kudu/clock/CMakeLists.txt
M src/kudu/hms/CMakeLists.txt
M src/kudu/postgres/CMakeLists.txt
M src/kudu/ranger/CMakeLists.txt
M src/kudu/scripts/dump_breakpad_symbols.py
17 files changed, 76 insertions(+), 34 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/18/18018/3
--
To view, visit http://gerrit.cloudera.org:8080/18018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5e0bdf4faa44322622c48cacadf8e1165eccd38a
Gerrit-Change-Number: 18018
Gerrit-PatchSet: 3
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [build] Introduce an env variable to indicate the thirdparty path
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18018
to look at the new patch set (#2).
Change subject: [build] Introduce an env variable to indicate the thirdparty path
......................................................................
[build] Introduce an env variable to indicate the thirdparty path
Kudu thirdparty will cost dozens of minutes to build, now introduce
an env variable THIRDPARTY_DIR to indicate the thirdparty path, it
can be a well built thirdparty libraries path.
With this feature, we can
- Share the same thirdparty path by different code pathes, these code
pathes are not needed to build thirdparty libraries repeatly
- Build an docker image with thirdparty libraries well built
Change-Id: I5e0bdf4faa44322622c48cacadf8e1165eccd38a
---
M CMakeLists.txt
M build-support/ccache-clang/clang
M build-support/ccache-clang/clang++
M build-support/clang_tidy_gerrit.py
M build-support/iwyu.py
M build-support/iwyu/iwyu_tool.py
M build-support/jenkins/build-and-test.sh
M build-support/kudu_util.py
M build-support/lint.sh
M build-support/llvm-gcov-wrapper
M build-support/mini-cluster/build_mini_cluster_binaries.sh
M docs/support/scripts/make_site.sh
M src/kudu/clock/CMakeLists.txt
M src/kudu/hms/CMakeLists.txt
M src/kudu/postgres/CMakeLists.txt
M src/kudu/ranger/CMakeLists.txt
M src/kudu/scripts/dump_breakpad_symbols.py
17 files changed, 73 insertions(+), 34 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/18/18018/2
--
To view, visit http://gerrit.cloudera.org:8080/18018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5e0bdf4faa44322622c48cacadf8e1165eccd38a
Gerrit-Change-Number: 18018
Gerrit-PatchSet: 2
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [build] Introduce an env variable to indicate the thirdparty path
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/18018 )
Change subject: [build] Introduce an env variable to indicate the thirdparty path
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18018/2/build-support/jenkins/build-and-test.sh
File build-support/jenkins/build-and-test.sh:
http://gerrit.cloudera.org:8080/#/c/18018/2/build-support/jenkins/build-and-test.sh@314
PS2, Line 314:
: if [ ! -n "${THIRDPARTY_DIR}" ]; then
: THIRDPARTY_DIR=$SOURCE_ROOT/thirdparty
: fi
> nit: consider using the same syntax used elsewhere? Same in other files, if
+1
--
To view, visit http://gerrit.cloudera.org:8080/18018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5e0bdf4faa44322622c48cacadf8e1165eccd38a
Gerrit-Change-Number: 18018
Gerrit-PatchSet: 3
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Fri, 12 Nov 2021 21:00:28 +0000
Gerrit-HasComments: Yes
[kudu-CR] [build] Introduce an env variable to indicate the thirdparty path
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/18018 )
Change subject: [build] Introduce an env variable to indicate the thirdparty path
......................................................................
Patch Set 2:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/18018/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18018/2//COMMIT_MSG@13
PS2, Line 13: pathes
nit: paths
http://gerrit.cloudera.org:8080/#/c/18018/2//COMMIT_MSG@15
PS2, Line 15: - Build an docker image with thirdparty libraries well built
Can you elaborate on this? Is there a problem with thirdparty in our Docker images?
http://gerrit.cloudera.org:8080/#/c/18018/2/build-support/iwyu.py
File build-support/iwyu.py:
http://gerrit.cloudera.org:8080/#/c/18018/2/build-support/iwyu.py@33
PS2, Line 33: from kudu_util import get_upstream_commit, check_output, ROOT, Colors, init_logging, get_thirdparty_dir
nit: line is too long
http://gerrit.cloudera.org:8080/#/c/18018/2/build-support/iwyu/iwyu_tool.py
File build-support/iwyu/iwyu_tool.py:
http://gerrit.cloudera.org:8080/#/c/18018/2/build-support/iwyu/iwyu_tool.py@88
PS2, Line 88: sys.path.append(os.path.abspath(os.path.join(os.path.dirname(__file__), "..")))
isn't there a cleaner way to import a module from a parent directory?
http://gerrit.cloudera.org:8080/#/c/18018/2/src/kudu/scripts/dump_breakpad_symbols.py
File src/kudu/scripts/dump_breakpad_symbols.py:
http://gerrit.cloudera.org:8080/#/c/18018/2/src/kudu/scripts/dump_breakpad_symbols.py@91
PS2, Line 91: return env['THIRDPARTY_DIR'] if env.has_key('THIRDPARTY_DIR') else os.path.join(ROOT, "thirdparty")
nit: line is too long
http://gerrit.cloudera.org:8080/#/c/18018/2/src/kudu/scripts/dump_breakpad_symbols.py@106
PS2, Line 106: dump_syms = os.path.join(get_thirdparty_dir(), 'installed', 'uninstrumented', 'bin', 'dump_syms')
nit: line is too long
--
To view, visit http://gerrit.cloudera.org:8080/18018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5e0bdf4faa44322622c48cacadf8e1165eccd38a
Gerrit-Change-Number: 18018
Gerrit-PatchSet: 2
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 11 Nov 2021 15:01:38 +0000
Gerrit-HasComments: Yes
[kudu-CR] [build] Introduce an env variable to indicate the thirdparty path
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/18018 )
Change subject: [build] Introduce an env variable to indicate the thirdparty path
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/18018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5e0bdf4faa44322622c48cacadf8e1165eccd38a
Gerrit-Change-Number: 18018
Gerrit-PatchSet: 4
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Tue, 16 Nov 2021 19:30:05 +0000
Gerrit-HasComments: No
[kudu-CR] [build] Introduce an env variable to indicate the thirdparty path
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Attila Bukor, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18018
to look at the new patch set (#4).
Change subject: [build] Introduce an env variable to indicate the thirdparty path
......................................................................
[build] Introduce an env variable to indicate the thirdparty path
Kudu thirdparty will cost dozens of minutes to build, now introduce
an env variable THIRDPARTY_DIR to indicate the thirdparty path, it
can be a well built thirdparty libraries path.
With this feature, we can
- Share the same thirdparty path by different code paths, these code
paths are not needed to build thirdparty libraries repeatly
- Build an docker image with thirdparty libraries well built, then we
can use this image for building Kudu, CI, and etc.
Change-Id: I5e0bdf4faa44322622c48cacadf8e1165eccd38a
---
M CMakeLists.txt
M build-support/ccache-clang/clang
M build-support/ccache-clang/clang++
M build-support/clang_tidy_gerrit.py
M build-support/iwyu.py
M build-support/iwyu/iwyu_tool.py
M build-support/jenkins/build-and-test.sh
M build-support/kudu_util.py
M build-support/lint.sh
M build-support/llvm-gcov-wrapper
M build-support/mini-cluster/build_mini_cluster_binaries.sh
M docs/support/scripts/make_site.sh
M src/kudu/clock/CMakeLists.txt
M src/kudu/hms/CMakeLists.txt
M src/kudu/postgres/CMakeLists.txt
M src/kudu/ranger/CMakeLists.txt
M src/kudu/scripts/dump_breakpad_symbols.py
17 files changed, 66 insertions(+), 37 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/18/18018/4
--
To view, visit http://gerrit.cloudera.org:8080/18018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5e0bdf4faa44322622c48cacadf8e1165eccd38a
Gerrit-Change-Number: 18018
Gerrit-PatchSet: 4
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [build] Introduce an env variable to indicate the thirdparty path
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18018 )
Change subject: [build] Introduce an env variable to indicate the thirdparty path
......................................................................
Patch Set 3:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/18018/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18018/2//COMMIT_MSG@13
PS2, Line 13: paths,
> nit: paths
Done
http://gerrit.cloudera.org:8080/#/c/18018/2//COMMIT_MSG@15
PS2, Line 15: - Build an docker image with thirdparty libraries well built, then we
> Can you elaborate on this? Is there a problem with thirdparty in our Docker
The Kudu docker doc [1] says images like 'apache/kudu:thirdparty-[OS]-[VERSION]' are provided, but I didn't see it on DockerHub [2], seems the doc is outdated. On the other hand, currently, the 3rdparty path and Kudu code path must be in the same root path, this patch can resolve this situaction.
1. https://github.com/apache/kudu/blob/master/docker/README.adoc
2. https://hub.docker.com/r/apache/kudu
http://gerrit.cloudera.org:8080/#/c/18018/2/build-support/iwyu.py
File build-support/iwyu.py:
http://gerrit.cloudera.org:8080/#/c/18018/2/build-support/iwyu.py@33
PS2, Line 33: from kudu_util import get_upstream_commit, check_output, ROOT, Colors, \
> nit: line is too long
Done
http://gerrit.cloudera.org:8080/#/c/18018/2/build-support/iwyu/iwyu_tool.py
File build-support/iwyu/iwyu_tool.py:
http://gerrit.cloudera.org:8080/#/c/18018/2/build-support/iwyu/iwyu_tool.py@88
PS2, Line 88: sys.path.append(os.path.join(os.path.dirname(__file__), '..'))
> isn't there a cleaner way to import a module from a parent directory?
improved a little...
http://gerrit.cloudera.org:8080/#/c/18018/2/src/kudu/scripts/dump_breakpad_symbols.py
File src/kudu/scripts/dump_breakpad_symbols.py:
http://gerrit.cloudera.org:8080/#/c/18018/2/src/kudu/scripts/dump_breakpad_symbols.py@91
PS2, Line 91: return env['THIRDPARTY_DIR'] if env.has_key('THIRDPARTY_DIR') else \
> nit: line is too long
Done
http://gerrit.cloudera.org:8080/#/c/18018/2/src/kudu/scripts/dump_breakpad_symbols.py@106
PS2, Line 106: # TODO: Use dump_syms_mac if on macOS.
> nit: line is too long
Done
--
To view, visit http://gerrit.cloudera.org:8080/18018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5e0bdf4faa44322622c48cacadf8e1165eccd38a
Gerrit-Change-Number: 18018
Gerrit-PatchSet: 3
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Fri, 12 Nov 2021 02:12:03 +0000
Gerrit-HasComments: Yes