Alexey Serbin 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
> That does sound too many adjustments to make it work for 'home' directory a
Ah, right -- in my response I rather meant 'USE_PREBUILT_THIRDPARTY=0', yes.


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.
Good catch -- it's no longer used: now PREBUILT_THIRDPARTY_URL is used instead 
(see prebuilt-utils.sh).


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?
It was here in a few very first revisions (not published in gerrit, though), 
but I removed it later.  This is a remnant of that; it's fixed now.


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
As of PS5, it's only the major version, correct.  That's reflected in the 
commit description as well.


http://gerrit.cloudera.org:8080/#/c/24583/11/thirdparty/prebuilt-utils.sh@107
PS11, Line 107: $CC
> Should this be $compiler ?
Done


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.
Good catch -- indeed, this isn't used anymore.


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 ?
Indeed: this should logically mirror the setting of the PREFIX_xxx variables in 
vars.sh.

It seems I missed verifying how this works with USE_PREBUILT_THIRDPARTY=0 after 
recent modifications.  Will do and update the rest of this patch as necessary.


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 t
Done



--
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 22:58:12 +0000
Gerrit-HasComments: Yes

Reply via email to