You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Attila Bukor (Code Review)" <ge...@cloudera.org> on 2018/08/14 20:00:36 UTC
[kudu-CR] [build] Switch to sha512 checksum for releases
Hello Grant Henke, Todd Lipcon,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/11217
to review the following change.
Change subject: [build] Switch to sha512 checksum for releases
......................................................................
[build] Switch to sha512 checksum for releases
According to the current release distribution guide[1] the checksum file
SHOULD be SHA-256 and/or SHA-512 and SHOULD NOT be SHA-1
SHA-512 seemed more future-proof than SHA-256
[1] http://www.apache.org/dev/release-distribution#sigs-and-sums
Change-Id: I7646b4559bf39d2415b32a1dcdd4cd7ecde41531
---
M build-support/build_source_release.py
1 file changed, 4 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/17/11217/1
--
To view, visit http://gerrit.cloudera.org:8080/11217
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7646b4559bf39d2415b32a1dcdd4cd7ecde41531
Gerrit-Change-Number: 11217
Gerrit-PatchSet: 1
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [build] Switch to sha512 checksum for releases
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/11217 )
Change subject: [build] Switch to sha512 checksum for releases
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11217/1/build-support/build_source_release.py
File build-support/build_source_release.py:
http://gerrit.cloudera.org:8080/#/c/11217/1/build-support/build_source_release.py@134
PS1, Line 134:
why switch away from a tab here? I think the tabs made things nicely line up in the output. Do we need two tabs elsewhere so it lines up?
--
To view, visit http://gerrit.cloudera.org:8080/11217
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7646b4559bf39d2415b32a1dcdd4cd7ecde41531
Gerrit-Change-Number: 11217
Gerrit-PatchSet: 1
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 14 Aug 2018 20:07:59 +0000
Gerrit-HasComments: Yes
[kudu-CR] [build] Switch to sha512 checksum for releases
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/11217 )
Change subject: [build] Switch to sha512 checksum for releases
......................................................................
Patch Set 1:
(1 comment)
> Patch Set 1:
>
> (1 comment)
http://gerrit.cloudera.org:8080/#/c/11217/1/build-support/build_source_release.py
File build-support/build_source_release.py:
http://gerrit.cloudera.org:8080/#/c/11217/1/build-support/build_source_release.py@134
PS1, Line 134:
> Sorry, should've clarified in the commit message...
Should I put this in the commit message by the way?
--
To view, visit http://gerrit.cloudera.org:8080/11217
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7646b4559bf39d2415b32a1dcdd4cd7ecde41531
Gerrit-Change-Number: 11217
Gerrit-PatchSet: 1
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 14 Aug 2018 20:50:30 +0000
Gerrit-HasComments: Yes
[kudu-CR] [build] Switch to sha512 checksum for releases
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11217 )
Change subject: [build] Switch to sha512 checksum for releases
......................................................................
[build] Switch to sha512 checksum for releases
According to the current release distribution guide[1] the checksum file
SHOULD be SHA-256 and/or SHA-512 and SHOULD NOT be SHA-1
SHA-512 seemed more future-proof than SHA-256
Another change is the separator between the checksum and the filename.
Previously it was a <TAB> character, but at least on MacOS the tab
version doesn't work when trying to verify the checksum with `shasum -c
<checksum_file>`
This commit changes it to double-space which works on both Linux using
`sha512sum` and MacOS using `shasum`.
Looking at a checksum created by `sha512sum` on Linux, it also uses
double-space as the separator, so this seems to be the de-facto
standard:
$ sha512sum apache-kudu-1.8.0-SNAPSHOT.tar.gz | xxd
0000070: 3065 3132 3666 3463 3664 6332 3065 6338 0e126f4c6dc20ec8
0000080: 2020 6170 6163 6865 2d6b 7564 752d 312e apache-kudu-1.
(0x20 is <SPACE>[2])
[1] http://www.apache.org/dev/release-distribution#sigs-and-sums
[2] man 7 ascii
Change-Id: I7646b4559bf39d2415b32a1dcdd4cd7ecde41531
Reviewed-on: http://gerrit.cloudera.org:8080/11217
Tested-by: Kudu Jenkins
Reviewed-by: Grant Henke <gr...@apache.org>
Reviewed-by: Todd Lipcon <to...@apache.org>
---
M build-support/build_source_release.py
1 file changed, 4 insertions(+), 4 deletions(-)
Approvals:
Kudu Jenkins: Verified
Grant Henke: Looks good to me, but someone else must approve
Todd Lipcon: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/11217
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I7646b4559bf39d2415b32a1dcdd4cd7ecde41531
Gerrit-Change-Number: 11217
Gerrit-PatchSet: 3
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [build] Switch to sha512 checksum for releases
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/11217 )
Change subject: [build] Switch to sha512 checksum for releases
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11217/1/build-support/build_source_release.py
File build-support/build_source_release.py:
http://gerrit.cloudera.org:8080/#/c/11217/1/build-support/build_source_release.py@134
PS1, Line 134:
> why switch away from a tab here? I think the tabs made things nicely line u
Sorry, should've clarified in the commit message...
On MacOS shasum the tab version doesn't work. I verified the double white space on Linux as well and it works on both OSes. The Linux version of sha512sum also uses double-space to separate the checksum from the filename:
$ sha512sum apache-kudu-1.8.0-SNAPSHOT.tar.gz | xxd
# ...
0000070: 3065 3132 3666 3463 3664 6332 3065 6338 0e126f4c6dc20ec8
0000080: 2020 6170 6163 6865 2d6b 7564 752d 312e apache-kudu-1.
# ...
--
To view, visit http://gerrit.cloudera.org:8080/11217
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7646b4559bf39d2415b32a1dcdd4cd7ecde41531
Gerrit-Change-Number: 11217
Gerrit-PatchSet: 1
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 14 Aug 2018 20:50:06 +0000
Gerrit-HasComments: Yes
[kudu-CR] [build] Switch to sha512 checksum for releases
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/11217 )
Change subject: [build] Switch to sha512 checksum for releases
......................................................................
Patch Set 2: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/11217
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7646b4559bf39d2415b32a1dcdd4cd7ecde41531
Gerrit-Change-Number: 11217
Gerrit-PatchSet: 2
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 15 Aug 2018 14:16:45 +0000
Gerrit-HasComments: No
[kudu-CR] [build] Switch to sha512 checksum for releases
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Grant Henke, Todd Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11217
to look at the new patch set (#2).
Change subject: [build] Switch to sha512 checksum for releases
......................................................................
[build] Switch to sha512 checksum for releases
According to the current release distribution guide[1] the checksum file
SHOULD be SHA-256 and/or SHA-512 and SHOULD NOT be SHA-1
SHA-512 seemed more future-proof than SHA-256
Another change is the separator between the checksum and the filename.
Previously it was a <TAB> character, but at least on MacOS the tab
version doesn't work when trying to verify the checksum with `shasum -c
<checksum_file>`
This commit changes it to double-space which works on both Linux using
`sha512sum` and MacOS using `shasum`.
Looking at a checksum created by `sha512sum` on Linux, it also uses
double-space as the separator, so this seems to be the de-facto
standard:
$ sha512sum apache-kudu-1.8.0-SNAPSHOT.tar.gz | xxd
0000070: 3065 3132 3666 3463 3664 6332 3065 6338 0e126f4c6dc20ec8
0000080: 2020 6170 6163 6865 2d6b 7564 752d 312e apache-kudu-1.
(0x20 is <SPACE>[2])
[1] http://www.apache.org/dev/release-distribution#sigs-and-sums
[2] man 7 ascii
Change-Id: I7646b4559bf39d2415b32a1dcdd4cd7ecde41531
---
M build-support/build_source_release.py
1 file changed, 4 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/17/11217/2
--
To view, visit http://gerrit.cloudera.org:8080/11217
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7646b4559bf39d2415b32a1dcdd4cd7ecde41531
Gerrit-Change-Number: 11217
Gerrit-PatchSet: 2
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [build] Switch to sha512 checksum for releases
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/11217 )
Change subject: [build] Switch to sha512 checksum for releases
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11217
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7646b4559bf39d2415b32a1dcdd4cd7ecde41531
Gerrit-Change-Number: 11217
Gerrit-PatchSet: 2
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 15 Aug 2018 18:49:05 +0000
Gerrit-HasComments: No