You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@impala.apache.org by "Jim Apple (Code Review)" <ge...@cloudera.org> on 2016/09/08 16:02:56 UTC

[Impala-ASF-CR] Allow buildall.sh to work from snapshots (not just git checkouts) 

Jim Apple has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/4336

Change subject: Allow buildall.sh to work from snapshots (not just git checkouts)
......................................................................

Allow buildall.sh to work from snapshots (not just git checkouts)

buildall.sh calls bin/clean.sh, which fails when not in a git
directory. This skips the "git clean" steps when not in a git
checkout.

Todd Lipcon found this when testing a snapshot created using git
archive.

Change-Id: I12dd9035298151557491009680d66d25c8f58c1d
---
M bin/clean.sh
1 file changed, 11 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/36/4336/1
-- 
To view, visit http://gerrit.cloudera.org:8080/4336
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I12dd9035298151557491009680d66d25c8f58c1d
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@cloudera.com>

[Impala-ASF-CR] IMPALA-4096: Allow clean.sh to work from snapshots

Posted by "Michael Brown (Code Review)" <ge...@cloudera.org>.
Michael Brown has posted comments on this change.

Change subject: IMPALA-4096: Allow clean.sh to work from snapshots
......................................................................


Patch Set 3: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I12dd9035298151557491009680d66d25c8f58c1d
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4096: Allow clean.sh to work from snapshots

Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change.

Change subject: IMPALA-4096: Allow clean.sh to work from snapshots
......................................................................


Patch Set 3: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I12dd9035298151557491009680d66d25c8f58c1d
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4096: Allow clean.sh to work from snapshots

Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has uploaded a new patch set (#2).

Change subject: IMPALA-4096: Allow clean.sh to work from snapshots
......................................................................

IMPALA-4096: Allow clean.sh to work from snapshots

buildall.sh calls bin/clean.sh, which fails when not in a git
directory. This skips the "git clean" steps when not in a git
checkout.

Todd Lipcon found this when testing a snapshot created using git
archive.

Change-Id: I12dd9035298151557491009680d66d25c8f58c1d
---
M bin/clean.sh
1 file changed, 5 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/36/4336/2
-- 
To view, visit http://gerrit.cloudera.org:8080/4336
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I12dd9035298151557491009680d66d25c8f58c1d
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>

[Impala-ASF-CR] IMPALA-4096: Allow clean.sh to work from snapshots

Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change.

Change subject: IMPALA-4096: Allow clean.sh to work from snapshots
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4336/2/bin/clean.sh
File bin/clean.sh:

PS2, Line 77: -fdx
I just noticed this one is different from the other two. I think since you're touching the line anyways it might make sense to use Xdfq here, too, especially since we probably want to prefer -X (use gitignore) over -x (ignore gitignore) to avoid throwing away uncommitted work. -q is quiet.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I12dd9035298151557491009680d66d25c8f58c1d
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4096: Allow clean.sh to work from snapshots

Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has uploaded a new patch set (#3).

Change subject: IMPALA-4096: Allow clean.sh to work from snapshots
......................................................................

IMPALA-4096: Allow clean.sh to work from snapshots

buildall.sh calls bin/clean.sh, which fails when not in a git
directory. This skips the "git clean" steps when not in a git
checkout.

Todd Lipcon found this when testing a snapshot created using git
archive.

Change-Id: I12dd9035298151557491009680d66d25c8f58c1d
---
M bin/clean.sh
1 file changed, 5 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/36/4336/3
-- 
To view, visit http://gerrit.cloudera.org:8080/4336
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I12dd9035298151557491009680d66d25c8f58c1d
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>

[Impala-ASF-CR] IMPALA-4096: Allow clean.sh to work from snapshots

Posted by "Michael Brown (Code Review)" <ge...@cloudera.org>.
Michael Brown has posted comments on this change.

Change subject: IMPALA-4096: Allow clean.sh to work from snapshots
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4336/2/bin/clean.sh
File bin/clean.sh:

PS2, Line 55: git rev-parse &>dev/null
Here and elsewhere, you're missing the leading / for /dev/null unless this is some fancy bashism I don't know about.

Can you also make sure that the &> operator is supported on CentOS 5? &>> is definitely not (see https://gerrit.cloudera.org/#/c/3531/ , for example)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I12dd9035298151557491009680d66d25c8f58c1d
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4096: Allow clean.sh to work from snapshots

Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change.

Change subject: IMPALA-4096: Allow clean.sh to work from snapshots
......................................................................


Patch Set 3:

> Build failed: http://sandbox.jenkins.cloudera.com/job/impala-external-gerrit-verify-merge-ASF/181/

Waiting on https://gerrit.cloudera.org/#/c/4337/ https://issues.cloudera.org/browse/IMPALA-4097

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I12dd9035298151557491009680d66d25c8f58c1d
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4096: Allow clean.sh to work from snapshots

Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change.

Change subject: IMPALA-4096: Allow clean.sh to work from snapshots
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4336/2/bin/clean.sh
File bin/clean.sh:

PS2, Line 77: -fdx
> I am reluctant to change this in this commit, which I would like to be a no
I agree with that, let's keep it the way it is.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I12dd9035298151557491009680d66d25c8f58c1d
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4096: Allow clean.sh to work from snapshots

Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change.

Change subject: IMPALA-4096: Allow clean.sh to work from snapshots
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4336/2/bin/clean.sh
File bin/clean.sh:

PS2, Line 77: -fdx
> I just noticed this one is different from the other two. I think since you'
I am reluctant to change this in this commit, which I would like to be a no-op for current developers, without understanding why the options are written the way they are in the first place.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I12dd9035298151557491009680d66d25c8f58c1d
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4096: Allow clean.sh to work from snapshots

Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has posted comments on this change.

Change subject: IMPALA-4096: Allow clean.sh to work from snapshots
......................................................................


Patch Set 4: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I12dd9035298151557491009680d66d25c8f58c1d
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4096: Allow clean.sh to work from snapshots

Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has posted comments on this change.

Change subject: IMPALA-4096: Allow clean.sh to work from snapshots
......................................................................


Patch Set 3: Verified-1

Build failed: http://sandbox.jenkins.cloudera.com/job/impala-external-gerrit-verify-merge-ASF/181/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I12dd9035298151557491009680d66d25c8f58c1d
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4096: Allow clean.sh to work from snapshots

Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change.

Change subject: IMPALA-4096: Allow clean.sh to work from snapshots
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4336/1//COMMIT_MSG
Commit Message:

Line 7: Allow buildall.sh to work from snapshots (not just git checkouts)
> Can you create an upstream issue to track this?
Done


http://gerrit.cloudera.org:8080/#/c/4336/1/bin/clean.sh
File bin/clean.sh:

Line 55: if git rev-parse &>dev/null; then
> You could rewrite these as such
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I12dd9035298151557491009680d66d25c8f58c1d
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4096: Allow clean.sh to work from snapshots

Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change.

Change subject: IMPALA-4096: Allow clean.sh to work from snapshots
......................................................................


Patch Set 4: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I12dd9035298151557491009680d66d25c8f58c1d
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] Allow buildall.sh to work from snapshots (not just git checkouts)

Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change.

Change subject: Allow buildall.sh to work from snapshots (not just git checkouts)
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4336/1//COMMIT_MSG
Commit Message:

Line 7: Allow buildall.sh to work from snapshots (not just git checkouts)
Can you create an upstream issue to track this?


http://gerrit.cloudera.org:8080/#/c/4336/1/bin/clean.sh
File bin/clean.sh:

Line 55: if git rev-parse &>dev/null; then
You could rewrite these as such

git rev-parse &>dev/null && git clean -Xdfq


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I12dd9035298151557491009680d66d25c8f58c1d
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4096: Allow clean.sh to work from snapshots

Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has submitted this change and it was merged.

Change subject: IMPALA-4096: Allow clean.sh to work from snapshots
......................................................................


IMPALA-4096: Allow clean.sh to work from snapshots

buildall.sh calls bin/clean.sh, which fails when not in a git
directory. This skips the "git clean" steps when not in a git
checkout.

Todd Lipcon found this when testing a snapshot created using git
archive.

Change-Id: I12dd9035298151557491009680d66d25c8f58c1d
Reviewed-on: http://gerrit.cloudera.org:8080/4336
Tested-by: Internal Jenkins
Reviewed-by: Jim Apple <jb...@cloudera.com>
---
M bin/clean.sh
1 file changed, 5 insertions(+), 3 deletions(-)

Approvals:
  Jim Apple: Looks good to me, approved
  Internal Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I12dd9035298151557491009680d66d25c8f58c1d
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>

[Impala-ASF-CR] IMPALA-4096: Allow clean.sh to work from snapshots

Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change.

Change subject: IMPALA-4096: Allow clean.sh to work from snapshots
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4336/2/bin/clean.sh
File bin/clean.sh:

PS2, Line 55: git rev-parse &>dev/null
> Here and elsewhere, you're missing the leading / for /dev/null unless this 
1. Done

2. Changed to 2>, as (a) this is a more common idiom, (b) git rev-parse produces no output in my testing, and (c) I don't have easy access to a CentOS 5 installation.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I12dd9035298151557491009680d66d25c8f58c1d
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4096: Allow clean.sh to work from snapshots

Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change.

Change subject: IMPALA-4096: Allow clean.sh to work from snapshots
......................................................................


Patch Set 3: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I12dd9035298151557491009680d66d25c8f58c1d
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: No