Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/24583 )
Change subject: KUDU-3788 an option to use pre-built thirdparty artifacts ...................................................................... Patch Set 11: (8 comments) http://gerrit.cloudera.org:8080/#/c/24583/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/24583/5//COMMIT_MSG@45 PS5, Line 45: -built arc > The 'home' directory approach wouldn't help: with pre-built artifacts, it's That does sound too many adjustments to make it work for 'home' directory approach. I think you meant 'USE_PREBUILT_THIRDPARTY=0' http://gerrit.cloudera.org:8080/#/c/24583/11/thirdparty/build-thirdparty.sh File thirdparty/build-thirdparty.sh: http://gerrit.cloudera.org:8080/#/c/24583/11/thirdparty/build-thirdparty.sh@50 PS11, Line 50: PREBUILT_URL Is this a typo? Don't see this anywhere else. http://gerrit.cloudera.org:8080/#/c/24583/11/thirdparty/build-thirdparty.sh@51 PS11, Line 51: UPLOAD_PREBUILT_THIRDPARTY What is the purpose of this if upload is not happening automatically? http://gerrit.cloudera.org:8080/#/c/24583/11/thirdparty/prebuilt-utils.sh File thirdparty/prebuilt-utils.sh: http://gerrit.cloudera.org:8080/#/c/24583/11/thirdparty/prebuilt-utils.sh@80 PS11, Line 80: cut -d. -f1 Wouldn't this just be major version? (comment says major and minor version are used) Same goes for below at line:91 Could be leftover typo in the comment from previous PS where both were considered? http://gerrit.cloudera.org:8080/#/c/24583/11/thirdparty/prebuilt-utils.sh@107 PS11, Line 107: $CC Should this be $compiler ? http://gerrit.cloudera.org:8080/#/c/24583/11/thirdparty/prebuilt-utils.sh@266 PS11, Line 266: prepare_staging_prefix This seems unused. fetch_prebuilt_or_build is doing this already. http://gerrit.cloudera.org:8080/#/c/24583/11/thirdparty/prebuilt-utils.sh@337 PS11, Line 337: INSTALL_DESTDIR="$TP_STAGING_DIR/$component" Should this be null if USE_PREBUILT_THIRDPARTY=0 ? http://gerrit.cloudera.org:8080/#/c/24583/11/thirdparty/prebuilt-utils.sh@389 PS11, Line 389: rm -rf "$staging_root" Is a similar check (from prepare_staging_prefix) required before removing the directory? ++ if [ -z "$staging_root" ]; then echo "ERROR: staging directory must not be empty" >&2 exit 1 fi ++ -- To view, visit http://gerrit.cloudera.org:8080/24583 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I55b5b99fcbe60eec3f6ae19fa4f2e335f361d143 Gerrit-Change-Number: 24583 Gerrit-PatchSet: 11 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-Reviewer: Marton Greber <[email protected]> Gerrit-Comment-Date: Wed, 29 Jul 2026 09:22:34 +0000 Gerrit-HasComments: Yes
