Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/20870 )
Change subject: IMPALA-12686: Build the toolchain with basic debug information (-g1) ...................................................................... Patch Set 1: (7 comments) http://gerrit.cloudera.org:8080/#/c/20870/1/init-compiler.sh File init-compiler.sh: http://gerrit.cloudera.org:8080/#/c/20870/1/init-compiler.sh@52 PS1, Line 52: # ARCH_FLAGS are used to convey architectur dependent flags that should > Want to fix the typo "architectur" while you're making changes? Done http://gerrit.cloudera.org:8080/#/c/20870/1/init-compiler.sh@102 PS1, Line 102: CFLAGS="$ARCH_FLAGS -fPIC -O3 -g1 -gz" > Looks like CFLAGS did not previously have all the ARCH_FLAGS. Yeah, that is odd. It seems cleaner to have CFLAGS to contain ARCH_FLAGS. http://gerrit.cloudera.org:8080/#/c/20870/1/source/arrow/build.sh File source/arrow/build.sh: http://gerrit.cloudera.org:8080/#/c/20870/1/source/arrow/build.sh@32 PS1, Line 32: CFLAGS="$CFLAGS -fPIC -DPIC" wrap make VERBOSE=1 -j${BUILD_THREADS:-4} all install > -fPIC is redundant now. Should we set -DPIC everywhere too? Specifying it at the top level caused the LLVM build to fail. I removed DPIC across the board. I can't find anything indication that it is still needed. Anything that wants to check whether this is a position independent build has other things to check. http://gerrit.cloudera.org:8080/#/c/20870/1/source/bzip2/build.sh File source/bzip2/build.sh: http://gerrit.cloudera.org:8080/#/c/20870/1/source/bzip2/build.sh@33 PS1, Line 33: CFLAGS="$CFLAGS -fPIC -DPIC" wrap make -f Makefile-libbz2_so CFLAGS="$CFLAGS -fPIC -DPIC" > Why declare CFLAGS twice? Something about the second command doesn't respect the CFLAGS environment variable. I removed one set of CFLAGS settings. http://gerrit.cloudera.org:8080/#/c/20870/1/source/flatbuffers/build.sh File source/flatbuffers/build.sh: http://gerrit.cloudera.org:8080/#/c/20870/1/source/flatbuffers/build.sh@33 PS1, Line 33: CXXFLAGS=${CXXFLAGS} > Is this line even needed? Removed http://gerrit.cloudera.org:8080/#/c/20870/1/source/lz4/build.sh File source/lz4/build.sh: http://gerrit.cloudera.org:8080/#/c/20870/1/source/lz4/build.sh@32 PS1, Line 32: CFLAGS="$CFLAGS -fPIC" > This whole line is unnecessary, it already includes -fPIC. Good point, fixed http://gerrit.cloudera.org:8080/#/c/20870/1/source/orc/build.sh File source/orc/build.sh: http://gerrit.cloudera.org:8080/#/c/20870/1/source/orc/build.sh@47 PS1, Line 47: wrap make VERBOSE=1 -j${BUILD_THREADS:-4} install > This removed -DPIC. I asked elsewhere if we should add that globally. I'm removing DPIC everywhere as nothing really uses it. -- To view, visit http://gerrit.cloudera.org:8080/20870 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iee2e264b281f83ebc226d9bf7d4e5a99a52f1fc6 Gerrit-Change-Number: 20870 Gerrit-PatchSet: 1 Gerrit-Owner: Joe McDonnell <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Comment-Date: Fri, 31 May 2024 00:11:48 +0000 Gerrit-HasComments: Yes
