Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20267 )

Change subject: IMPALA-12441: Simplify local toolchain development
......................................................................


Patch Set 5:

(2 comments)

> Patch Set 5:
>
> (2 comments)
>
> Please file a JIRA.

Done.

>
> This simplifies the case where someone wants to
> modify the toolchain. It can be checked out to the
> appropriate hash, and then any future modifications
> will take effect through buildall.sh. I think this
> looks good.
>
> What this doesn't do is deal with this local
> toolchain consistently if you switch branches. If
> the IMPALA_TOOLCHAIN_COMMIT_HASH goes from x to y,
> the toolchain checkout isn't touched and stays at x.
> This is a use case we don't currently care about
> much, so we could do that separately. Heading in
> that direction gets us closer to Kudu's thirdparty
> model where it just gets triggered and kept up to
> date as part of the build.

There are two conflict workflows here:
- make local changes in native-toolchain that you want to test. There would be 
no remote branch to fetch and buildall would get in the way if it tried to 
switch to another commit hash.
- switching branches, when you want to use a different toolchain commit. Most 
of the time if I switch branches I probably want to go back to using prebuilt 
toolchains, so you'd want to clear NATIVE_TOOLCHAIN_HOME and re-source 
impala-config.sh.

I don't have a clear idea how to resolve those, so going to leave it alone.

>
> We also have a new toolchain since this upload.

I believe I already resolved that conflict.

http://gerrit.cloudera.org:8080/#/c/20267/5/buildall.sh
File buildall.sh:

http://gerrit.cloudera.org:8080/#/c/20267/5/buildall.sh@433
PS5, Line 433: cd -
> To make it explicit, should we use pushd/popd?
Sure, this is the only place that used the 'cd -' pattern.


http://gerrit.cloudera.org:8080/#/c/20267/5/buildall.sh@452
PS5, Line 452: cp $IMPALA_HOME/../hadoopAarch64NativeLibs/lib*  
$HADOOP_HOME/lib/native/
> A theory I have is that this can create problems for a running cluster. If
Ack



--
To view, visit http://gerrit.cloudera.org:8080/20267
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3a9e51b7f54c738d8cc01b32428ac88a344de376
Gerrit-Change-Number: 20267
Gerrit-PatchSet: 5
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: Mon, 11 Sep 2023 18:28:17 +0000
Gerrit-HasComments: Yes

Reply via email to