Skip to content

Samza 1114 #69

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
renato2099 wants to merge 6 commits into from
Closed

Samza 1114 #69

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
52e7242
SAMZA-489
renato2099 Feb 11, 2016
3d77f2b
Merging
renato2099 Feb 11, 2016
393e46c
Merging Local
renato2099 Feb 25, 2017
84277c6
Merge branch 'master' of https://github.com/apache/samza
renato2099 Mar 1, 2017
77de76c
Sync with apache_master
renato2099 Mar 1, 2017
4cac3ae
SAMZA-1114: Improving script
renato2099 Mar 1, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions bin/check-all.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,9 @@ fi
if [ -e "${base_dir}/gradlew" ]; then
gradle_file="${base_dir}/gradlew"
else
gradle_base_file=$(which gradle)
gradle_base_file=$(which gradle) && chk_gradle=$? || chk_gradle=$?
Copy link
Copy Markdown
Contributor

@vjagadish1989 vjagadish1989 Mar 2, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

chk_gradle=$?

Is this is just a way of checking chk_gradle when set -e is set? Otherwise it would cause the shell to exit if any subcommand or pipeline returns a non-zero status?

Wondering what's the need for this additional check using a chk_gradle flag? It seems that -f should already do the trick? (if gradle_base_file does not exist)

Copy link
Copy Markdown
Contributor Author

@renato2099 renato2099 Mar 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hey @vjagadish1989 thanks for checking this out. The problem is that in my case the $(which gradle) command failed, and then the script exits without prompting anything, not even getting to the <-f> check.
So this line puts the result of executing $(which gradle) into the variable chk_gradle so we can check if the $(which gradle) didn't fail.

Copy link
Copy Markdown
Contributor

@vjagadish1989 vjagadish1989 Mar 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Firstly, Thanks for the patch @renato2099 👍

The which gradle failed

QQ: Did the command fail because gradle was not installed or which was not installed? For the former, wouldn't the -f in the current implementation would catch it?

I wrote a small test to verify this:

Check with valid gradle path:

sh-3.2$ gradle_base_file=$(which gradle)
sh-3.2$ if [ -f "$gradle_base_file" ]; then    echo "gradle_base_file $gradle_base_file exists."; else    echo "gradle_base_file $gradle_base_file does not exist."; fi
gradle_base_file /usr/local/bin/gradle exists.

Check with invalid gradle path:

sh-3.2$ gradle_base_file=$(which gradle-notfound)
sh-3.2$ if [ -f "$gradle_base_file" ]; then    echo "gradle_base_file $gradle_base_file exists."; else    echo "gradle_base_file $gradle_base_file does not exist."; fi
gradle_base_file  does not exist.

Copy link
Copy Markdown
Contributor Author

@renato2099 renato2099 Mar 8, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @vjagadish1989 thanks again for checking this out with me.
So on my env (Debian Lenny) the script exited when executing which gradle and it didn't make it to the if statement. which was installed because I didn't get a command not found error.
I have also tried to reproduce the issue on different servers now and everything works as expected i.e. which doesn't crash. I guess it was just something wrong with my local env. You can close this PR as this only affected me ;)


if [ -f "${gradle_base_file}" ]; then
if [[ -f "${gradle_base_file}" && $chk_gradle -eq 0 ]]; then
gradle_file="${gradle_base_file}"
echo "Gradlew is not found. Using currently installed gradle, found on: ${gradle_file}."
else
Expand Down