ndimiduk commented on a change in pull request #2056: URL: https://github.com/apache/hbase/pull/2056#discussion_r455177715
########## File path: hbase-common/src/saveVersion.sh ########## @@ -30,34 +30,34 @@ nativeOutputDirectory="$2/native/utils/" pushd . cd .. -user=`whoami | sed -n -e 's/\\\/\\\\\\\\/p'` +user=$(whoami | sed -n -e "s/\\\/\\\\\\\\/p") if [ "$user" == "" ] then - user=`whoami` + user=$(whoami) fi -date=`date` -cwd=`pwd` +date=$(date) +cwd=$(pwd) if [ -d .svn ]; then - revision=`(svn info | sed -n -e 's/Last Changed Rev: \(.*\)/\1/p') || true` - url=`(svn info | sed -n -e 's/^URL: \(.*\)/\1/p') || true` + revision=$( (svn info | sed -n -e "s/Last Changed Rev: \(.*\)/\1/p") || true) + url=$( (svn info | sed -n -e 's/^URL: \(.*\)/\1/p') || true) elif [ -d .git ]; then - revision=`git log -1 --no-show-signature --pretty=format:"%H" || true` - hostname=`hostname` + revision=$(git log -1 --no-show-signature --pretty=format:"%H" || true) + hostname=$(hostname) url="git://${hostname}${cwd}" fi if [ -z "${revision}" ]; then echo "[WARN] revision info is empty! either we're not in VCS or VCS commands failed." >&2 revision="Unknown" url="file://$cwd" fi -if ! [ -x "$(command -v openssl)" ]; then +if ! [ -x "$(command -v openssl)" ]; then if ! [ -x "$(command -v gpg)" ]; then srcChecksum="Unknown" else - srcChecksum=`find hbase-*/src/main/ | grep -e "\.java" -e "\.proto" | LC_ALL=C sort | xargs gpg --print-md sha512 | gpg --print-md sha512 | cut -d ' ' -f 1` Review comment: Previous use of `cut` here at the end is surprising. It took only the first block... ########## File path: hbase-common/src/saveVersion.sh ########## @@ -30,34 +30,34 @@ nativeOutputDirectory="$2/native/utils/" pushd . cd .. -user=`whoami | sed -n -e 's/\\\/\\\\\\\\/p'` +user=$(whoami | sed -n -e "s/\\\/\\\\\\\\/p") if [ "$user" == "" ] then - user=`whoami` + user=$(whoami) fi -date=`date` -cwd=`pwd` +date=$(date) +cwd=$(pwd) if [ -d .svn ]; then - revision=`(svn info | sed -n -e 's/Last Changed Rev: \(.*\)/\1/p') || true` - url=`(svn info | sed -n -e 's/^URL: \(.*\)/\1/p') || true` + revision=$( (svn info | sed -n -e "s/Last Changed Rev: \(.*\)/\1/p") || true) + url=$( (svn info | sed -n -e 's/^URL: \(.*\)/\1/p') || true) elif [ -d .git ]; then - revision=`git log -1 --no-show-signature --pretty=format:"%H" || true` - hostname=`hostname` + revision=$(git log -1 --no-show-signature --pretty=format:"%H" || true) + hostname=$(hostname) url="git://${hostname}${cwd}" fi if [ -z "${revision}" ]; then echo "[WARN] revision info is empty! either we're not in VCS or VCS commands failed." >&2 revision="Unknown" url="file://$cwd" fi -if ! [ -x "$(command -v openssl)" ]; then +if ! [ -x "$(command -v openssl)" ]; then if ! [ -x "$(command -v gpg)" ]; then srcChecksum="Unknown" else - srcChecksum=`find hbase-*/src/main/ | grep -e "\.java" -e "\.proto" | LC_ALL=C sort | xargs gpg --print-md sha512 | gpg --print-md sha512 | cut -d ' ' -f 1` + srcChecksum=$(find hbase-*/src/main/ | grep -e "\.java" -e "\.proto" | LC_ALL=C sort xargs gpg --print-md sha512 | gpg --print-md sha512 | tr '\n' ' ' | sed 's/[[:space:]]*//g') Review comment: I think dropping the `|` between `sort` and `xargs` is a bug. I believe this calculates the sha512 of the sort error message, not the sha512 of the sha512s of the contents of all the files. On a mac anyway, I get ``` $ find hbase-common/src/main/ | grep -e "\.java" | LC_ALL=C sort xargs gpg --print-md sha512 sort: unrecognized option `--print-md' Usage: sort [-bcCdfigMmnrsuz] [-kPOS1[,POS2] ... ] [+POS1 [-POS2]] [-S memsize] [-T tmpdir] [-t separator] [-o outfile] [--batch-size size] [--files0-from file] [--heapsort] [--mergesort] [--radixsort] [--qsort] [--mmap] [--parallel thread_no] [--human-numeric-sort] [--version-sort] [--random-sort [--random-source file]] [--compress-program program] [file ...] $ find hbase-common/src/main/ | grep -e "\.java" | LC_ALL=C sort xargs openssl dgst -sha512 sort: invalid option -- a Usage: sort [-bcCdfigMmnrsuz] [-kPOS1[,POS2] ... ] [+POS1 [-POS2]] [-S memsize] [-T tmpdir] [-t separator] [-o outfile] [--batch-size size] [--files0-from file] [--heapsort] [--mergesort] [--radixsort] [--qsort] [--mmap] [--parallel thread_no] [--human-numeric-sort] [--version-sort] [--random-sort [--random-source file]] [--compress-program program] [file ...] ``` ########## File path: hbase-common/src/saveVersion.sh ########## @@ -30,34 +30,34 @@ nativeOutputDirectory="$2/native/utils/" pushd . cd .. -user=`whoami | sed -n -e 's/\\\/\\\\\\\\/p'` +user=$(whoami | sed -n -e "s/\\\/\\\\\\\\/p") if [ "$user" == "" ] then - user=`whoami` + user=$(whoami) fi -date=`date` -cwd=`pwd` +date=$(date) +cwd=$(pwd) if [ -d .svn ]; then - revision=`(svn info | sed -n -e 's/Last Changed Rev: \(.*\)/\1/p') || true` - url=`(svn info | sed -n -e 's/^URL: \(.*\)/\1/p') || true` + revision=$( (svn info | sed -n -e "s/Last Changed Rev: \(.*\)/\1/p") || true) + url=$( (svn info | sed -n -e 's/^URL: \(.*\)/\1/p') || true) elif [ -d .git ]; then - revision=`git log -1 --no-show-signature --pretty=format:"%H" || true` - hostname=`hostname` + revision=$(git log -1 --no-show-signature --pretty=format:"%H" || true) + hostname=$(hostname) url="git://${hostname}${cwd}" fi if [ -z "${revision}" ]; then echo "[WARN] revision info is empty! either we're not in VCS or VCS commands failed." >&2 revision="Unknown" url="file://$cwd" fi -if ! [ -x "$(command -v openssl)" ]; then +if ! [ -x "$(command -v openssl)" ]; then if ! [ -x "$(command -v gpg)" ]; then srcChecksum="Unknown" else - srcChecksum=`find hbase-*/src/main/ | grep -e "\.java" -e "\.proto" | LC_ALL=C sort | xargs gpg --print-md sha512 | gpg --print-md sha512 | cut -d ' ' -f 1` + srcChecksum=$(find hbase-*/src/main/ | grep -e "\.java" -e "\.proto" | LC_ALL=C sort xargs gpg --print-md sha512 | gpg --print-md sha512 | tr '\n' ' ' | sed 's/[[:space:]]*//g') fi else - srcChecksum=`find hbase-*/src/main/ | grep -e "\.java" -e "\.proto" | LC_ALL=C sort | xargs openssl dgst -sha512 | openssl dgst -sha512 | cut -d ' ' -f 1` + srcChecksum=$(find hbase-*/src/main/ | grep -e "\.java" -e "\.proto" | LC_ALL=C sort xargs openssl dgst -sha512 | openssl dgst -sha512 | sed 's/^.* //') Review comment: what is the purpose of the final `sed`? I see no difference. ``` $ echo "foo" | openssl dgst -sha512 | sed 's/^.* //' 0cf9180a764aba863a67b6d72f0918bc131c6772642cb2dce5a34f0a702f9470ddc2bf125c12198b1995c233c34b4afd346c54a2334c350a948a51b6e8b4e6b6 $ echo "foo" | openssl dgst -sha512 0cf9180a764aba863a67b6d72f0918bc131c6772642cb2dce5a34f0a702f9470ddc2bf125c12198b1995c233c34b4afd346c54a2334c350a948a51b6e8b4e6b6 ``` ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org