Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/24581 )
Change subject: WIP [thirdparty] update boost up 1.91.0 ...................................................................... Patch Set 3: (5 comments) http://gerrit.cloudera.org:8080/#/c/24581/2/thirdparty/build-definitions.sh File thirdparty/build-definitions.sh: http://gerrit.cloudera.org:8080/#/c/24581/2/thirdparty/build-definitions.sh@968 PS2, Line 968: > nit: 1.91.0 Done http://gerrit.cloudera.org:8080/#/c/24581/2/thirdparty/build-definitions.sh@971 PS2, Line 971: # heap: Kudu : # locale: Thrift : # serialization: Kudu > Can these be cmake -D flags (e.g. -DCMAKE_CXX_FLAGS) instead of environment I didn't verify whether they can or cannot in this particular case (but I guess most likely the answer is 'yes'), but if making such an update here, I'd rather do it uniformly across this file. This is the common pattern we use for every 'build_xxx()' function -- I'd rather separate such an update into its own patch. http://gerrit.cloudera.org:8080/#/c/24581/2/thirdparty/download-thirdparty.sh File thirdparty/download-thirdparty.sh: http://gerrit.cloudera.org:8080/#/c/24581/2/thirdparty/download-thirdparty.sh@389 PS2, Line 389: "patch -p0 < $TP_DIR/patches/boost-bootstrap.patch" > Is this still applicable? Good point -- this is no longer necessary once building with cmake. Removed. http://gerrit.cloudera.org:8080/#/c/24581/3/thirdparty/vars.sh File thirdparty/vars.sh: PS3: > What are your thoughts on using the cmake compliant sources which they just That's a good option, but since I was exploring various approaches of checking out only the needed modules, I went ahead with options similar to 'git archive'. I guess we might consider using those archives from github.com. However, the alternative path of a custom tarball is better from the perspective of faster downloading and archive expansion -- that's done multiple times, but archive preparation is performed once when upgrading to a new version, even if the procedure contains many trivial steps: git submodule update --depth 1 -q --init tools/boostdep git submodule update --depth 1 -q --init libs/date_time git submodule update --depth 1 -q --init libs/heap git submodule update --depth 1 -q --init libs/locale git submodule update --depth 1 -q --init libs/serialization git submodule update --depth 1 -q --init libs/signals2 git submodule update --depth 1 -q --init libs/uuid python tools/boostdep/depinst/depinst.py -X test -g "--depth 1" date_time python tools/boostdep/depinst/depinst.py -X test -g "--depth 1" heap python tools/boostdep/depinst/depinst.py -X test -g "--depth 1" locale python tools/boostdep/depinst/depinst.py -X test -g "--depth 1" serialization python tools/boostdep/depinst/depinst.py -X test -g "--depth 1" signals2 python tools/boostdep/depinst/depinst.py -X test -g "--depth 1" uuid http://gerrit.cloudera.org:8080/#/c/24581/3/thirdparty/vars.sh@188 PS3, Line 188: git clone --recursive https://github.com/boostorg/boost.git > nit: Why not do something like: Right, that's what some of their guides recommend to reduce the amount of data to be fetched. I didn't add it from the perspective of uniformity with other sets of 'git archive' instructions. I can add that, sure. -- To view, visit http://gerrit.cloudera.org:8080/24581 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4c9c2e9260b892c721ec2ba887c61b8aeef35888 Gerrit-Change-Number: 24581 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Ashwani Raina <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 15 Jul 2026 00:03:58 +0000 Gerrit-HasComments: Yes
