You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Lars Volker (Code Review)" <ge...@cloudera.org> on 2018/07/10 16:31:38 UTC

[Impala-ASF-CR] Fix zsh issue in set-pythonpath.sh

Lars Volker has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10901


Change subject: Fix zsh issue in set-pythonpath.sh
......................................................................

Fix zsh issue in set-pythonpath.sh

When sourcing set-pythonpath.sh on a partially checked out or partially
built source tree (e.g. when running perf tests or after build errors),
an empty shell glob pattern would return an empty list in bash but
trigger an error in zsh. This would lead to the script aborting before
exporting the PYTHONPATH variable, leading to hard to debug test
failures.

To fix this, this change replaces the glob pattern with a call to
'find', which works independent of the shell being used.

Although we would only hit this in somewhat contrived edge cases, it
seems beneficial to make the code work as expected on all shells.

Change-Id: Ia902891ab36f3aee96a53aa105cc5775321d0058
---
M bin/set-pythonpath.sh
1 file changed, 1 insertion(+), 1 deletion(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia902891ab36f3aee96a53aa105cc5775321d0058
Gerrit-Change-Number: 10901
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker <lv...@cloudera.com>

[Impala-ASF-CR] Fix zsh issue in set-pythonpath.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/10901 )

Change subject: Fix zsh issue in set-pythonpath.sh
......................................................................


Patch Set 2:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2786/ DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia902891ab36f3aee96a53aa105cc5775321d0058
Gerrit-Change-Number: 10901
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Jul 2018 17:05:17 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] Fix zsh issue in set-pythonpath.sh

Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Hello David Knupp, 

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#2).

Change subject: Fix zsh issue in set-pythonpath.sh
......................................................................

Fix zsh issue in set-pythonpath.sh

When sourcing set-pythonpath.sh on a partially checked out or partially
built source tree (e.g. when running perf tests or after build errors),
an empty shell glob pattern would return an empty list in bash but
trigger an error in zsh. This would lead to the script aborting before
exporting the PYTHONPATH variable, leading to hard to debug test
failures.

To fix this, this change replaces the glob pattern with a call to
'find', which works independent of the shell being used.

Although we would only hit this in somewhat contrived edge cases, it
seems beneficial to make the code work as expected on all shells.

Testing:
I built Impala, sourced impala-config.sh in both new zsh and bash
shells, and observed that PYTHONPATH was exported correctly.

Then I deleted the .egg files in question, sourced impala-config.sh in
both new zsh and bash shells, and observed that PYTHONPATH was exported
correctly, too.

Change-Id: Ia902891ab36f3aee96a53aa105cc5775321d0058
---
M bin/set-pythonpath.sh
1 file changed, 1 insertion(+), 1 deletion(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia902891ab36f3aee96a53aa105cc5775321d0058
Gerrit-Change-Number: 10901
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>

[Impala-ASF-CR] Fix zsh issue in set-pythonpath.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/10901 )

Change subject: Fix zsh issue in set-pythonpath.sh
......................................................................


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2787/ DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia902891ab36f3aee96a53aa105cc5775321d0058
Gerrit-Change-Number: 10901
Gerrit-PatchSet: 3
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Jul 2018 22:24:54 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] Fix zsh issue in set-pythonpath.sh

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10901 )

Change subject: Fix zsh issue in set-pythonpath.sh
......................................................................

Fix zsh issue in set-pythonpath.sh

When sourcing set-pythonpath.sh on a partially checked out or partially
built source tree (e.g. when running perf tests or after build errors),
an empty shell glob pattern would return an empty list in bash but
trigger an error in zsh. This would lead to the script aborting before
exporting the PYTHONPATH variable, leading to hard to debug test
failures.

To fix this, this change replaces the glob pattern with a call to
'find', which works independent of the shell being used.

Although we would only hit this in somewhat contrived edge cases, it
seems beneficial to make the code work as expected on all shells.

Testing:
I built Impala, sourced impala-config.sh in both new zsh and bash
shells, and observed that PYTHONPATH was exported correctly.

Then I deleted the .egg files in question, sourced impala-config.sh in
both new zsh and bash shells, and observed that PYTHONPATH was exported
correctly, too.

Change-Id: Ia902891ab36f3aee96a53aa105cc5775321d0058
Reviewed-on: http://gerrit.cloudera.org:8080/10901
Reviewed-by: Lars Volker <lv...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M bin/set-pythonpath.sh
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  Lars Volker: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia902891ab36f3aee96a53aa105cc5775321d0058
Gerrit-Change-Number: 10901
Gerrit-PatchSet: 4
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>

[Impala-ASF-CR] Fix zsh issue in set-pythonpath.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/10901 )

Change subject: Fix zsh issue in set-pythonpath.sh
......................................................................


Patch Set 3: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia902891ab36f3aee96a53aa105cc5775321d0058
Gerrit-Change-Number: 10901
Gerrit-PatchSet: 3
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Jul 2018 01:43:41 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] Fix zsh issue in set-pythonpath.sh

Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
David Knupp has posted comments on this change. ( http://gerrit.cloudera.org:8080/10901 )

Change subject: Fix zsh issue in set-pythonpath.sh
......................................................................


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia902891ab36f3aee96a53aa105cc5775321d0058
Gerrit-Change-Number: 10901
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Jul 2018 16:51:57 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] Fix zsh issue in set-pythonpath.sh

Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/10901 )

Change subject: Fix zsh issue in set-pythonpath.sh
......................................................................


Patch Set 3: Code-Review+2

Rebased, carrying David's +2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia902891ab36f3aee96a53aa105cc5775321d0058
Gerrit-Change-Number: 10901
Gerrit-PatchSet: 3
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Jul 2018 22:24:46 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] Fix zsh issue in set-pythonpath.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/10901 )

Change subject: Fix zsh issue in set-pythonpath.sh
......................................................................


Patch Set 2: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/2786/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia902891ab36f3aee96a53aa105cc5775321d0058
Gerrit-Change-Number: 10901
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Jul 2018 20:28:01 +0000
Gerrit-HasComments: No