Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/24687 )
Change subject: KUDU-3788 download source archives by demand only ...................................................................... Patch Set 1: (6 comments) http://gerrit.cloudera.org:8080/#/c/24687/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/24687/1//COMMIT_MSG@1 PS1, Line 1: Parent: 565fa2a0 (remove std::move on optional<MonoTime> deadline) > Not related to this patch, low priority: If a package version has changed a It's a good point -- feel free to file a JIRA on that improvement. As for KUDU-3788, the scope is updating the 3rd-party scripts w.r.t. using pre-built artifacts. This patch unifies the approach to the downloading source archives for 3rd-party components. It's a minor re-factoring once fetch/expand/config is now run only when necessary in pre-built mode, so it was natural and no extra cost to do the same in legacy (non-pre-built) mode as well. http://gerrit.cloudera.org:8080/#/c/24687/1//COMMIT_MSG@11 PS1, Line 11: would be still > would still be Done http://gerrit.cloudera.org:8080/#/c/24687/1//COMMIT_MSG@15 PS1, Line 15: download-thirdparty.sh > So do you think we can get rid of this script eventually? Yes, we can get rid of the script. I wasn't sure whether it makes sense to do so in this changelist. I also thought it might be convenient to have a way to download all the dist files and have custom Kudu patches applied, so it's easier to develop/troubleshoot adding an updating source archives. Do you think it makes sense to get rid of the script? Maybe, instead we should rename it into download-all-src.sh or something and keep it around? http://gerrit.cloudera.org:8080/#/c/24687/1//COMMIT_MSG@16 PS1, Line 16: by > only Done http://gerrit.cloudera.org:8080/#/c/24687/1//COMMIT_MSG@17 PS1, Line 17: However, it's no longer used when running : build-if-necessary.sh. > With this change, make sure to verify the documentation inside: docs/installation.adoc: no references to download-thirdparty.sh docs/contributing.adoc: still need to update the list of components in the download-thirdparty.sh, so this stays as-is (maybe, we'll need eventually move the list into vars.sh or so if still keeping the functionality of downloading all source archives as a separate script) http://gerrit.cloudera.org:8080/#/c/24687/1/thirdparty/download-thirdparty.sh File thirdparty/download-thirdparty.sh: http://gerrit.cloudera.org:8080/#/c/24687/1/thirdparty/download-thirdparty.sh@83 PS1, Line 83: $comp > "$comp" It's not necessary since we don't allow spaces in the names of a component. If there were spaces, a few more places would be broken. However, I added double quotes here anyway. -- To view, visit http://gerrit.cloudera.org:8080/24687 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I51a1822070f6e37726e4efd53ba5e536c9eb8fb9 Gerrit-Change-Number: 24687 Gerrit-PatchSet: 1 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: Kudu Jenkins (120) Gerrit-Reviewer: Zoltan Martonka <[email protected]> Gerrit-Comment-Date: Thu, 13 Aug 2026 19:22:33 +0000 Gerrit-HasComments: Yes
