Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20519 )
Change subject: IMPALA-12354: Add aarch64 native-toolchain build ...................................................................... Patch Set 7: (4 comments) http://gerrit.cloudera.org:8080/#/c/20519/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/20519/5//COMMIT_MSG@10 PS5, Line 10: adds an aarch64 (64-bit ARM) native-toolchain build, separate from the > Could you add some high-level details on the structure you're building here Done http://gerrit.cloudera.org:8080/#/c/20519/5/bin/bootstrap_toolchain.py File bin/bootstrap_toolchain.py: http://gerrit.cloudera.org:8080/#/c/20519/5/bin/bootstrap_toolchain.py@257 PS5, Line 257: toolchain_host = os.environ["IMPALA_TOOLCHAIN_HOST"] > Would it make sense to add a check for the known and supported architecture Decided to move this logic to impala-config.sh to make it easier to inspect or override if needed. http://gerrit.cloudera.org:8080/#/c/20519/5/bin/impala-config.sh File bin/impala-config.sh: http://gerrit.cloudera.org:8080/#/c/20519/5/bin/impala-config.sh@88 PS5, Line 88: IMPALA_TOOLCHAIN_BUILD_ID_aarch64=4-ba9ab5f36c : IMPALA_TOOLCHAIN_BUILD_ID_x86_64=25-ba9ab5f36c > What happens if the build numbers happened to clash? If I'm doing something like that, I'm tempted to change the package name since that opens up using the same TOOLCHAIN_BUILD_ID for both architectures at some point in the future. Something like https://gerrit.cloudera.org/c/20539/. It would require a corresponding change in bootstrap_toolchain.py to download the right package. http://gerrit.cloudera.org:8080/#/c/20519/5/bin/impala-config.sh@95 PS5, Line 95: rem > Could you add a brief comment here pointing out the Bash magic? It's subtle Done -- To view, visit http://gerrit.cloudera.org:8080/20519 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9bfa7125dbc647b33041c5572d97b7f7ccad6258 Gerrit-Change-Number: 20519 Gerrit-PatchSet: 7 Gerrit-Owner: Michael Smith <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Laszlo Gaal <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Comment-Date: Thu, 05 Oct 2023 22:03:44 +0000 Gerrit-HasComments: Yes
