You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Quanlong Huang (Code Review)" <ge...@cloudera.org> on 2019/02/15 02:10:27 UTC
[Impala-ASF-CR](2.x) IMPALA-8204: checkout to the right commit of Impala-lzo in buildall.sh
Quanlong Huang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12495
Change subject: IMPALA-8204: checkout to the right commit of Impala-lzo in buildall.sh
......................................................................
IMPALA-8204: checkout to the right commit of Impala-lzo in buildall.sh
Impala depends on the Impala-lzo project in builds. However, Impala-lzo
is evolving. Different branches of Impala depends on different versions
of Impala-lzo. This patch adds a variable in bin/impala-config-branch.sh
to specify the depended version for Impala-lzo.
Using this variable(IMPALA_LZO_VERSION), buildall.sh will checkout to
the right version at each run.
Change-Id: I5e4b53d695de04d31d39b6909572907944713ba4
---
M bin/impala-config-branch.sh
M buildall.sh
2 files changed, 9 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/12495/1
--
To view, visit http://gerrit.cloudera.org:8080/12495
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5e4b53d695de04d31d39b6909572907944713ba4
Gerrit-Change-Number: 12495
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR](2.x) IMPALA-8204: checkout to the right commit of Impala-lzo in buildall.sh
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12495 )
Change subject: IMPALA-8204: checkout to the right commit of Impala-lzo in buildall.sh
......................................................................
Patch Set 2: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/12495
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I5e4b53d695de04d31d39b6909572907944713ba4
Gerrit-Change-Number: 12495
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Feb 2019 10:46:27 +0000
Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-8204: Add IMPALA LZO VERSION to pin Impala-lzo commit
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Philip Zeyliger, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12495
to look at the new patch set (#3).
Change subject: IMPALA-8204: Add IMPALA_LZO_VERSION to pin Impala-lzo commit
......................................................................
IMPALA-8204: Add IMPALA_LZO_VERSION to pin Impala-lzo commit
Impala depends on the Impala-lzo project in builds. However, Impala-lzo
is evolving. Different branches/releases of Impala depends on different
versions of Impala-lzo, which always causes build failures when
switching between branches or building different branches consecutively
in a Jenkins worker.
This patch introduces IMPALA_LZO_VERSION) in impala-config-branch.sh to
specify the depended version of Impala-lzo. We also add a variable
(SKIP_IMPALA_LZO_VERIFICATION) to specify whether buildall.sh should
check out to the right Impala-lzo commit. The default is true, meaning
skip checking.
This patch also make it possible to run GVO on Impala-lzo relative
patches. Previously when we need to modify both Impala and Impala-lzo
repos (e.g. IMPALA-7980), we can't run GVO for verification since it
depends on the new Impala-lzo commit which has not been submitted too.
After this patch, we can first submit changes to Impala-lzo, then set
the right IMPALA_LZO_VERSION with SKIP_IMPALA_LZO_VERIFICATION=false
in the GVO job to verify both patches from Impala and Impala-lzo.
Change-Id: I5e4b53d695de04d31d39b6909572907944713ba4
---
M README.md
M bin/impala-config-branch.sh
M bin/impala-config.sh
M buildall.sh
4 files changed, 13 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/12495/3
--
To view, visit http://gerrit.cloudera.org:8080/12495
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5e4b53d695de04d31d39b6909572907944713ba4
Gerrit-Change-Number: 12495
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR](2.x) IMPALA-8204: checkout to the right commit of Impala-lzo in buildall.sh
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12495 )
Change subject: IMPALA-8204: checkout to the right commit of Impala-lzo in buildall.sh
......................................................................
Patch Set 1:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3779/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/12495
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I5e4b53d695de04d31d39b6909572907944713ba4
Gerrit-Change-Number: 12495
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Feb 2019 02:11:35 +0000
Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-8204: checkout to the right commit of Impala-lzo in buildall.sh
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12495 )
Change subject: IMPALA-8204: checkout to the right commit of Impala-lzo in buildall.sh
......................................................................
Patch Set 2:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3782/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/12495
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I5e4b53d695de04d31d39b6909572907944713ba4
Gerrit-Change-Number: 12495
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Feb 2019 07:03:09 +0000
Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-8204: checkout to the right commit of Impala-lzo in buildall.sh
Posted by "Philip Zeyliger (Code Review)" <ge...@cloudera.org>.
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/12495 )
Change subject: IMPALA-8204: checkout to the right commit of Impala-lzo in buildall.sh
......................................................................
Patch Set 2:
(2 comments)
I don't think this approach is wise. We have two places where we check out Impala-lzo:
a) bin/bootstrap_system.sh: git clone --branch master https://github.com/cloudera/impala-lzo.git "$IMPALA_LZO_HOME"
b) Jenkins jobs.
I think it's ok to move a git branch when doing the checkout, but it's not so nice to move the branch when you're doing buildall. (For example, if I were to need to make a change to impala-lzo, this would confuse me incredibly, since buildall would be undoing my changes!)
http://gerrit.cloudera.org:8080/#/c/12495/2/bin/impala-config-branch.sh
File bin/impala-config-branch.sh:
http://gerrit.cloudera.org:8080/#/c/12495/2/bin/impala-config-branch.sh@26
PS2, Line 26: IMPALA_LZO_VERSION=8a984e0
Do you want to use a long git hash or a tag or a branch name instead? The short git hash is sort of the worst of all worlds, since it's not guaranteed to work if there's a collision.
http://gerrit.cloudera.org:8080/#/c/12495/2/buildall.sh
File buildall.sh:
http://gerrit.cloudera.org:8080/#/c/12495/2/buildall.sh@337
PS2, Line 337: # Checkout to the right version of Impala-lzo. IMPALA_LZO_VERSION is set in
: # bin/impala-config-branch.sh
: if [[ -e "$IMPALA_LZO" ]]; then
: echo "Checkout to commit $IMPALA_LZO_VERSION for Impala-lzo repo in $IMPALA_LZO"
: (pushd "$IMPALA_LZO" && git checkout "$IMPALA_LZO_VERSION")
: fi
I don't think it's appropriate for buildall.sh to move a repo around like this.
--
To view, visit http://gerrit.cloudera.org:8080/12495
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I5e4b53d695de04d31d39b6909572907944713ba4
Gerrit-Change-Number: 12495
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Feb 2019 17:45:49 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR](2.x) IMPALA-8204: checkout to the right commit of Impala-lzo in buildall.sh
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12495 )
Change subject: IMPALA-8204: checkout to the right commit of Impala-lzo in buildall.sh
......................................................................
Patch Set 1: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3779/
--
To view, visit http://gerrit.cloudera.org:8080/12495
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I5e4b53d695de04d31d39b6909572907944713ba4
Gerrit-Change-Number: 12495
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Feb 2019 06:00:25 +0000
Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-8204: checkout to the right commit of Impala-lzo in buildall.sh
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12495
to look at the new patch set (#2).
Change subject: IMPALA-8204: checkout to the right commit of Impala-lzo in buildall.sh
......................................................................
IMPALA-8204: checkout to the right commit of Impala-lzo in buildall.sh
Impala depends on the Impala-lzo project in builds. However, Impala-lzo
is evolving. Different branches of Impala depends on different versions
of Impala-lzo. This patch adds a variable in bin/impala-config-branch.sh
to specify the depended version for Impala-lzo.
Using this variable(IMPALA_LZO_VERSION), buildall.sh will checkout to
the right version at each run.
Change-Id: I5e4b53d695de04d31d39b6909572907944713ba4
---
M bin/impala-config-branch.sh
M buildall.sh
2 files changed, 9 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/12495/2
--
To view, visit http://gerrit.cloudera.org:8080/12495
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5e4b53d695de04d31d39b6909572907944713ba4
Gerrit-Change-Number: 12495
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR](2.x) IMPALA-8204: Add IMPALA LZO VERSION to pin Impala-lzo commit
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12495 )
Change subject: IMPALA-8204: Add IMPALA_LZO_VERSION to pin Impala-lzo commit
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2151/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/12495
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I5e4b53d695de04d31d39b6909572907944713ba4
Gerrit-Change-Number: 12495
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 18 Feb 2019 04:45:04 +0000
Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-8204: checkout to the right commit of Impala-lzo in buildall.sh
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12495 )
Change subject: IMPALA-8204: checkout to the right commit of Impala-lzo in buildall.sh
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2134/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/12495
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I5e4b53d695de04d31d39b6909572907944713ba4
Gerrit-Change-Number: 12495
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Feb 2019 02:47:39 +0000
Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-8204: checkout to the right commit of Impala-lzo in buildall.sh
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/12495 )
Change subject: IMPALA-8204: checkout to the right commit of Impala-lzo in buildall.sh
......................................................................
Patch Set 2:
(1 comment)
> Patch Set 2:
>
> > I still think we should write down the depended Impala-lzo version
> > somewhere. Once I want to build an older version of Impala, I don't
> > need to guess which Impala-lzo commit should I check out to.
>
> it's in bin/bootstrap_development.sh, no?
bootstrap_development.sh just chooses branches. I think we can also checkout to the right commit(version) there. But we need to source impala-config.sh beffore it.
Two year agos when I wanted to build Impala-2.5-cdh5.7.3, I encountered build errors from Impala-lzo too. At that time Impala-2.5 was also a fairly old version, so I can just guessing the right commit of Impala-lzo based on the committed time.
>
>
> >
> > There's a benifit to do the checkout in buildall.sh. In the future
> > when I want to build an older version (but >2.13, >3.2), I just
> > need to checkout to the right tag in Impala and then run
> > buildall.sh. It'll bootstrap dependencies including the Impala-lzo
> > to the corresponding versions.
> >
> > If you're developing Impala-lzo, you can first commit it locally,
> > and then change the depended commit id in impala-config-branch.sh.
> > I think this is the same with other dependencies. For example, if
> > we want to bump the version of ORC lib, we need to upgrade it in
> > native-toolchain project and then change the corresponding version
> > in impala-config.sh.
>
> I understand your argument, but I don't think it's acceptable for a "build" script to change branches of git repos. We could treat impala-lzo like we treat the toolchain (i.e., checked out in bootstrap_toolchain.sh), and then I'd sort of see it as a "build artifact" which can be overridden rather than my source code.
Sure. I'm ok to move it to bootstrap_toolchain.sh.
> (BTW, if you choose to cherrypick the num_unqueued_files that drove this mess, we'll be back to using the same impala-lzo code base in both branches...)
Sure. That would fix the mess quickly! Let's at least pin the Impala-lzo version(tag or commit id) to ease builds of older version from now on.
http://gerrit.cloudera.org:8080/#/c/12495/2/bin/impala-config-branch.sh
File bin/impala-config-branch.sh:
http://gerrit.cloudera.org:8080/#/c/12495/2/bin/impala-config-branch.sh@26
PS2, Line 26: IMPALA_LZO_VERSION=8a984e0
> Do you want to use a long git hash or a tag or a branch name instead? The s
Sure. Creating tags for each Impala version may be better but that may be tedious for your management. I change it to long git hash.
--
To view, visit http://gerrit.cloudera.org:8080/12495
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I5e4b53d695de04d31d39b6909572907944713ba4
Gerrit-Change-Number: 12495
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sat, 16 Feb 2019 01:00:34 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR](2.x) IMPALA-8204: checkout to the right commit of Impala-lzo in buildall.sh
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/12495 )
Change subject: IMPALA-8204: checkout to the right commit of Impala-lzo in buildall.sh
......................................................................
Patch Set 2:
I still think we should write down the depended Impala-lzo version somewhere. Once I want to build an older version of Impala, I don't need to guess which Impala-lzo commit should I check out to.
There's a benifit to do the checkout in buildall.sh. In the future when I want to build an older version (but >2.13, >3.2), I just need to checkout to the right tag in Impala and then run buildall.sh. It'll bootstrap dependencies including the Impala-lzo to the corresponding versions.
If you're developing Impala-lzo, you can first commit it locally, and then change the depended commit id in impala-config-branch.sh. I think this is the same with other dependencies. For example, if we want to bump the version of ORC lib, we need to upgrade it in native-toolchain project and then change the corresponding version in impala-config.sh.
--
To view, visit http://gerrit.cloudera.org:8080/12495
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I5e4b53d695de04d31d39b6909572907944713ba4
Gerrit-Change-Number: 12495
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sat, 16 Feb 2019 00:21:07 +0000
Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-8204: checkout to the right commit of Impala-lzo in buildall.sh
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12495 )
Change subject: IMPALA-8204: checkout to the right commit of Impala-lzo in buildall.sh
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2137/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/12495
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I5e4b53d695de04d31d39b6909572907944713ba4
Gerrit-Change-Number: 12495
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Feb 2019 07:01:48 +0000
Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-8204: Add IMPALA LZO VERSION to pin Impala-lzo commit
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/12495 )
Change subject: IMPALA-8204: Add IMPALA_LZO_VERSION to pin Impala-lzo commit
......................................................................
Patch Set 3:
Hi Philip, bootstrap_toolchain.py is in Python and mainly developed for native-toolchain dependencies. So I still let the codes in buildall.sh but wrapped with a new variable and default to doing nothing.
--
To view, visit http://gerrit.cloudera.org:8080/12495
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I5e4b53d695de04d31d39b6909572907944713ba4
Gerrit-Change-Number: 12495
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 18 Feb 2019 05:50:09 +0000
Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-8204: checkout to the right commit of Impala-lzo in buildall.sh
Posted by "Philip Zeyliger (Code Review)" <ge...@cloudera.org>.
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/12495 )
Change subject: IMPALA-8204: checkout to the right commit of Impala-lzo in buildall.sh
......................................................................
Patch Set 2:
> I still think we should write down the depended Impala-lzo version
> somewhere. Once I want to build an older version of Impala, I don't
> need to guess which Impala-lzo commit should I check out to.
it's in bin/bootstrap_development.sh, no?
>
> There's a benifit to do the checkout in buildall.sh. In the future
> when I want to build an older version (but >2.13, >3.2), I just
> need to checkout to the right tag in Impala and then run
> buildall.sh. It'll bootstrap dependencies including the Impala-lzo
> to the corresponding versions.
>
> If you're developing Impala-lzo, you can first commit it locally,
> and then change the depended commit id in impala-config-branch.sh.
> I think this is the same with other dependencies. For example, if
> we want to bump the version of ORC lib, we need to upgrade it in
> native-toolchain project and then change the corresponding version
> in impala-config.sh.
I understand your argument, but I don't think it's acceptable for a "build" script to change branches of git repos. We could treat impala-lzo like we treat the toolchain (i.e., checked out in bootstrap_toolchain.sh), and then I'd sort of see it as a "build artifact" which can be overridden rather than my source code.
(BTW, if you choose to cherrypick the num_unqueued_files that drove this mess, we'll be back to using the same impala-lzo code base in both branches...)
--
To view, visit http://gerrit.cloudera.org:8080/12495
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I5e4b53d695de04d31d39b6909572907944713ba4
Gerrit-Change-Number: 12495
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sat, 16 Feb 2019 00:34:23 +0000
Gerrit-HasComments: No