You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by GitBox <gi...@apache.org> on 2020/07/15 16:32:45 UTC
[GitHub] [hbase] ndimiduk commented on a change in pull request #2056: HBASE-24710 Incorrect checksum calculation in saveVersion.sh
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:
users@infra.apache.org