-
Notifications
You must be signed in to change notification settings - Fork 3.4k
HBASE-24710 Incorrect checksum calculation in saveVersion.sh #2056
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.
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
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
ndimiduk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the general cleanup.
| 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` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previous use of cut here at the end is surprising. It took only the first block...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on history it was used for md5 and md5sum commands but it wasn't removed when SHA512 was introduced in saveVersion.sh.
root@8224f4c69e6f:/# echo "foo" | md5sum
d3b07384d113edec49eaa6238ad5ff00 -
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does tr -d '[:space:]' work? It's simpler and shorter than tr '\n' ' ' | sed 's/[[:space:]]*//g'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I get a different output from your command, an extra %.
find hbase-*/src/main/ | grep -e "\.java" -e "\.proto" | LC_ALL=C sort | xargs gpg --print-md sha512 | gpg --print-md sha512 | tr -d '[:space:]'
3BF4CCC275112769C7AB0EDC302793FB81005BDC9681E1604BD965918363FC93311FC60B24000563DD4D3EFDA10685F549721A28428F23487813A7397FBC9BD0%
ind 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'
3BF4CCC275112769C7AB0EDC302793FB81005BDC9681E1604BD965918363FC93311FC60B24000563DD4D3EFDA10685F549721A28428F23487813A7397FBC9BD0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's weird. I did not see in my macOS and Ubuntu Linux. But since this is not reliable, I guess we can stay with your script tr and sed - which works for me on both macOS and Ubuntu.
hbase-common/src/saveVersion.sh
Outdated
| 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/^.* //') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, this is the example from your comment on JIRA, a difference between the mac and linux versions of openssl dgst. Disregard.
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
LGTM |
Signed-off-by: Nick Dimiduk <[email protected]> Signed-off-by: Mingliang Liu <[email protected]>
Signed-off-by: Nick Dimiduk <[email protected]> Signed-off-by: Mingliang Liu <[email protected]>
…2056) Signed-off-by: Nick Dimiduk <[email protected]> Signed-off-by: Mingliang Liu <[email protected]>
No description provided.