You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jojy Varghese <jo...@mesosphere.io> on 2015/09/25 05:07:27 UTC
Review Request 38747: Adding digest utilities
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38747/
-----------------------------------------------------------
Review request for mesos, Gilbert Song and Timothy Chen.
Repository: mesos
Description
-------
Added SHA256 implementation.
Diffs
-----
3rdparty/libprocess/Makefile.am 064310c61d8a0c5c5a87f305f1f62149dd3a6699
3rdparty/libprocess/include/Makefile.am fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e
3rdparty/libprocess/include/process/digest.hpp PRE-CREATION
Diff: https://reviews.apache.org/r/38747/diff/
Testing
-------
make check.
Thanks,
Jojy Varghese
Re: Review Request 38747: Adding digest utilities
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38747/#review100530
-----------------------------------------------------------
Bad patch!
Reviews applied: [38443, 38579, 38580, 38747]
Failed command: make -j3 distcheck
Error:
make dist-gzip am__post_remove_distdir='@:'
make[1]: Entering directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot'
if test -d "mesos-0.25.0"; then find "mesos-0.25.0" -type d ! -perm -200 -exec chmod u+w {} ';' && rm -rf "mesos-0.25.0" || { sleep 5 && rm -rf "mesos-0.25.0"; }; else :; fi
test -d "mesos-0.25.0" || mkdir "mesos-0.25.0"
(cd 3rdparty && make top_distdir=../mesos-0.25.0 distdir=../mesos-0.25.0/3rdparty \
am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[2]: Entering directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty'
(cd libprocess && make top_distdir=../../mesos-0.25.0 distdir=../../mesos-0.25.0/3rdparty/libprocess \
am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[3]: Entering directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess'
make[3]: *** No rule to make target `src/tests/digest_tests.cpp', needed by `distdir'. Stop.
make[3]: Leaving directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess'
make[2]: *** [distdir] Error 1
make[2]: Leaving directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty'
make[1]: *** [distdir] Error 1
make[1]: Leaving directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot'
make: *** [dist] Error 2
- Mesos ReviewBot
On Sept. 25, 2015, 3:07 a.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38747/
> -----------------------------------------------------------
>
> (Updated Sept. 25, 2015, 3:07 a.m.)
>
>
> Review request for mesos, Gilbert Song and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added SHA256 implementation.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/Makefile.am 064310c61d8a0c5c5a87f305f1f62149dd3a6699
> 3rdparty/libprocess/include/Makefile.am fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e
> 3rdparty/libprocess/include/process/digest.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/38747/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 38747: Adding digest utilities
Posted by Alex Clemmer <cl...@gmail.com>.
> On Sept. 25, 2015, 6:24 a.m., Alex Clemmer wrote:
> > Hey, it doesn't look like `src/tests/digest_tests.cpp` is being added to the `CMakeLists.txt` file in `3rdparty/libprocess/src/tests`. Is that correct? If not, could we please add it there as well?
> >
> > (I have forgotten to add a list of header files as dependencies, so you can't add that quite yet.)
Ah, after testing, we won't have to include the header lists at all, so you don't have to add that to a list. It turns out that if you pass the `include/` folder into the `include_directory` function, it automatically will monitor those files for changes. :) So we're good.
But, unless I'm mistaken, we'd still have to add that test to the `CMakeLists.txt`. Let me know if my thinking is wrong here.
- Alex
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38747/#review100539
-----------------------------------------------------------
On Sept. 25, 2015, 5:46 a.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38747/
> -----------------------------------------------------------
>
> (Updated Sept. 25, 2015, 5:46 a.m.)
>
>
> Review request for mesos, Gilbert Song and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added SHA256 implementation.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/Makefile.am 064310c61d8a0c5c5a87f305f1f62149dd3a6699
> 3rdparty/libprocess/include/Makefile.am fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e
> 3rdparty/libprocess/include/process/digest.hpp PRE-CREATION
> 3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/38747/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 38747: Adding digest utilities
Posted by Jojy Varghese <jo...@mesosphere.io>.
> On Sept. 25, 2015, 6:24 a.m., Alex Clemmer wrote:
> > Hey, it doesn't look like `src/tests/digest_tests.cpp` is being added to the `CMakeLists.txt` file in `3rdparty/libprocess/src/tests`. Is that correct? If not, could we please add it there as well?
> >
> > (I have forgotten to add a list of header files as dependencies, so you can't add that quite yet.)
>
> Alex Clemmer wrote:
> Ah, after testing, we won't have to include the header lists at all, so you don't have to add that to a list. It turns out that if you pass the `include/` folder into the `include_directory` function, it automatically will monitor those files for changes. :) So we're good.
>
> But, unless I'm mistaken, we'd still have to add that test to the `CMakeLists.txt`. Let me know if my thinking is wrong here.
Added. Didnt know this extra step existed :)
- Jojy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38747/#review100539
-----------------------------------------------------------
On Sept. 25, 2015, 4:45 p.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38747/
> -----------------------------------------------------------
>
> (Updated Sept. 25, 2015, 4:45 p.m.)
>
>
> Review request for mesos, Gilbert Song and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added SHA256 implementation.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/Makefile.am 064310c61d8a0c5c5a87f305f1f62149dd3a6699
> 3rdparty/libprocess/include/Makefile.am fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e
> 3rdparty/libprocess/include/process/digest.hpp PRE-CREATION
> 3rdparty/libprocess/src/tests/CMakeLists.txt a14b5b8fe22d3e75bef3c716ae7865ddaf132927
> 3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/38747/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 38747: Adding digest utilities
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38747/#review100539
-----------------------------------------------------------
Hey, it doesn't look like `src/tests/digest_tests.cpp` is being added to the `CMakeLists.txt` file in `3rdparty/libprocess/src/tests`. Is that correct? If not, could we please add it there as well?
(I have forgotten to add a list of header files as dependencies, so you can't add that quite yet.)
- Alex Clemmer
On Sept. 25, 2015, 5:46 a.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38747/
> -----------------------------------------------------------
>
> (Updated Sept. 25, 2015, 5:46 a.m.)
>
>
> Review request for mesos, Gilbert Song and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added SHA256 implementation.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/Makefile.am 064310c61d8a0c5c5a87f305f1f62149dd3a6699
> 3rdparty/libprocess/include/Makefile.am fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e
> 3rdparty/libprocess/include/process/digest.hpp PRE-CREATION
> 3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/38747/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 38747: Adding digest utilities
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38747/#review100619
-----------------------------------------------------------
Bad patch!
Reviews applied: [38443, 38579]
Failed command: ./support/apply-review.sh -n -r 38579
Error:
2015-09-25 17:21:42 URL:https://reviews.apache.org/r/38579/diff/raw/ [3389/3389] -> "38579.patch" [1]
error: patch failed: src/tests/containerizer/provisioner_docker_tests.cpp:48
error: src/tests/containerizer/provisioner_docker_tests.cpp: patch does not apply
Failed to apply patch
- Mesos ReviewBot
On Sept. 25, 2015, 4:45 p.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38747/
> -----------------------------------------------------------
>
> (Updated Sept. 25, 2015, 4:45 p.m.)
>
>
> Review request for mesos, Gilbert Song and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added SHA256 implementation.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/Makefile.am 064310c61d8a0c5c5a87f305f1f62149dd3a6699
> 3rdparty/libprocess/include/Makefile.am fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e
> 3rdparty/libprocess/include/process/digest.hpp PRE-CREATION
> 3rdparty/libprocess/src/tests/CMakeLists.txt a14b5b8fe22d3e75bef3c716ae7865ddaf132927
> 3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/38747/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 38747: Adding digest utilities
Posted by Jojy Varghese <jo...@mesosphere.io>.
> On Sept. 28, 2015, 6:32 p.m., Neil Conway wrote:
> > 3rdparty/libprocess/include/process/digest.hpp, line 191
> > <https://reviews.apache.org/r/38747/diff/7/?file=1084907#file1084907line191>
> >
> > I believe this leaks "fd".
good catch.
> On Sept. 28, 2015, 6:32 p.m., Neil Conway wrote:
> > 3rdparty/libprocess/include/process/digest.hpp, line 222
> > <https://reviews.apache.org/r/38747/diff/7/?file=1084907#file1084907line222>
> >
> > Why do we initialize this to `{0}`?
initialization of auto variables
- Jojy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38747/#review100835
-----------------------------------------------------------
On Sept. 25, 2015, 8:33 p.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38747/
> -----------------------------------------------------------
>
> (Updated Sept. 25, 2015, 8:33 p.m.)
>
>
> Review request for mesos, Gilbert Song and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added SHA256, SHA512 implementation.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1
> 3rdparty/libprocess/include/Makefile.am fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e
> 3rdparty/libprocess/include/process/digest.hpp PRE-CREATION
> 3rdparty/libprocess/src/tests/CMakeLists.txt a14b5b8fe22d3e75bef3c716ae7865ddaf132927
> 3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/38747/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 38747: Adding digest utilities
Posted by Neil Conway <ne...@gmail.com>.
> On Sept. 28, 2015, 6:32 p.m., Neil Conway wrote:
> > 3rdparty/libprocess/include/process/digest.hpp, line 222
> > <https://reviews.apache.org/r/38747/diff/7/?file=1084907#file1084907line222>
> >
> > Why do we initialize this to `{0}`?
>
> Jojy Varghese wrote:
> initialization of auto variables
IMO this is a little misleading: the array is actually initialized by the for loop that follows below. I think it would be a bit clearer to omit the initializer, and then explicitly NUL-terminate the array after the for loop.
- Neil
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38747/#review100835
-----------------------------------------------------------
On Sept. 28, 2015, 10:14 p.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38747/
> -----------------------------------------------------------
>
> (Updated Sept. 28, 2015, 10:14 p.m.)
>
>
> Review request for mesos, Gilbert Song and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added SHA256, SHA512 implementation.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1
> 3rdparty/libprocess/include/Makefile.am fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e
> 3rdparty/libprocess/include/process/digest.hpp PRE-CREATION
> 3rdparty/libprocess/src/tests/CMakeLists.txt a14b5b8fe22d3e75bef3c716ae7865ddaf132927
> 3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/38747/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 38747: Adding digest utilities
Posted by Neil Conway <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38747/#review100835
-----------------------------------------------------------
3rdparty/libprocess/include/process/digest.hpp (line 191)
<https://reviews.apache.org/r/38747/#comment158087>
I believe this leaks "fd".
3rdparty/libprocess/include/process/digest.hpp (line 205)
<https://reviews.apache.org/r/38747/#comment158086>
Probably "constexpr" is better than "static const".
3rdparty/libprocess/include/process/digest.hpp (line 219)
<https://reviews.apache.org/r/38747/#comment158088>
Seems like this duplicates some code above: can we refactor this into a function?
3rdparty/libprocess/include/process/digest.hpp (line 222)
<https://reviews.apache.org/r/38747/#comment158084>
Why do we initialize this to `{0}`?
3rdparty/libprocess/include/process/digest.hpp (line 225)
<https://reviews.apache.org/r/38747/#comment158085>
Need a space after "for".
Also, is sprintf() really the best choice here? There's std::hex + std::stringstream, although maybe that's not better overall.
3rdparty/libprocess/include/process/digest.hpp (line 237)
<https://reviews.apache.org/r/38747/#comment158081>
Typo
3rdparty/libprocess/include/process/digest.hpp (line 265)
<https://reviews.apache.org/r/38747/#comment158083>
Typo
3rdparty/libprocess/include/process/digest.hpp (line 315)
<https://reviews.apache.org/r/38747/#comment158082>
Typo
- Neil Conway
On Sept. 25, 2015, 8:33 p.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38747/
> -----------------------------------------------------------
>
> (Updated Sept. 25, 2015, 8:33 p.m.)
>
>
> Review request for mesos, Gilbert Song and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added SHA256, SHA512 implementation.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1
> 3rdparty/libprocess/include/Makefile.am fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e
> 3rdparty/libprocess/include/process/digest.hpp PRE-CREATION
> 3rdparty/libprocess/src/tests/CMakeLists.txt a14b5b8fe22d3e75bef3c716ae7865ddaf132927
> 3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/38747/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 38747: Adding digest utilities
Posted by Gilbert Song <gi...@mesoshere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38747/#review100839
-----------------------------------------------------------
3rdparty/libprocess/include/process/digest.hpp (lines 210 - 226)
<https://reviews.apache.org/r/38747/#comment158093>
digest printed out is different between digest(const char* bytes, size_t size) and digest(const std::string& filePath) with a corresponding same string.
Logic check is needed. (proprably is a logical error in my test case)
3rdparty/libprocess/include/process/digest.hpp (line 275)
<https://reviews.apache.org/r/38747/#comment158090>
This line is supposed to be deleted.
- Gilbert Song
On Sept. 25, 2015, 8:33 p.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38747/
> -----------------------------------------------------------
>
> (Updated Sept. 25, 2015, 8:33 p.m.)
>
>
> Review request for mesos, Gilbert Song and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added SHA256, SHA512 implementation.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1
> 3rdparty/libprocess/include/Makefile.am fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e
> 3rdparty/libprocess/include/process/digest.hpp PRE-CREATION
> 3rdparty/libprocess/src/tests/CMakeLists.txt a14b5b8fe22d3e75bef3c716ae7865ddaf132927
> 3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/38747/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 38747: Adding digest utilities
Posted by Jojy Varghese <jo...@mesosphere.io>.
> On Sept. 29, 2015, 9:17 p.m., Timothy Chen wrote:
> > 3rdparty/libprocess/src/tests/digest_tests.cpp, line 58
> > <https://reviews.apache.org/r/38747/diff/11/?file=1087208#file1087208line58>
> >
> > What's the motivation behind having both simple and dynamic verification?
"Simple" gives you the ability to call a digest (say SHA 256) directly if you already know the type. The dynamic part is so that the client code does not have to do the selection. Use cases like the docker registry where the sha type is dynamic, the client code will have to do the dynamic call. By keeping it central, this logic need not be repeated everywhere.
- Jojy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38747/#review101018
-----------------------------------------------------------
On Sept. 29, 2015, 9:40 p.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38747/
> -----------------------------------------------------------
>
> (Updated Sept. 29, 2015, 9:40 p.m.)
>
>
> Review request for mesos, Ben Mahler, Gilbert Song, and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added SHA256, SHA512 implementation.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1
> 3rdparty/libprocess/include/Makefile.am fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e
> 3rdparty/libprocess/include/process/digest.hpp PRE-CREATION
> 3rdparty/libprocess/src/tests/CMakeLists.txt a14b5b8fe22d3e75bef3c716ae7865ddaf132927
> 3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/38747/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 38747: Adding digest utilities
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38747/#review101018
-----------------------------------------------------------
3rdparty/libprocess/src/tests/digest_tests.cpp (line 58)
<https://reviews.apache.org/r/38747/#comment158324>
What's the motivation behind having both simple and dynamic verification?
- Timothy Chen
On Sept. 29, 2015, 6:41 p.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38747/
> -----------------------------------------------------------
>
> (Updated Sept. 29, 2015, 6:41 p.m.)
>
>
> Review request for mesos, Gilbert Song and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added SHA256, SHA512 implementation.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1
> 3rdparty/libprocess/include/Makefile.am fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e
> 3rdparty/libprocess/include/process/digest.hpp PRE-CREATION
> 3rdparty/libprocess/src/tests/CMakeLists.txt a14b5b8fe22d3e75bef3c716ae7865ddaf132927
> 3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/38747/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 38747: Adding digest utilities
Posted by Timothy Chen <tn...@apache.org>.
> On Sept. 29, 2015, 9:35 p.m., Ben Mahler wrote:
> > Tim, could you also get a libprocess maintainer to review this?
Volunteered you :)
- Timothy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38747/#review101026
-----------------------------------------------------------
On Sept. 29, 2015, 6:41 p.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38747/
> -----------------------------------------------------------
>
> (Updated Sept. 29, 2015, 6:41 p.m.)
>
>
> Review request for mesos, Gilbert Song and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added SHA256, SHA512 implementation.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1
> 3rdparty/libprocess/include/Makefile.am fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e
> 3rdparty/libprocess/include/process/digest.hpp PRE-CREATION
> 3rdparty/libprocess/src/tests/CMakeLists.txt a14b5b8fe22d3e75bef3c716ae7865ddaf132927
> 3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/38747/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 38747: Adding digest utilities
Posted by Ben Mahler <be...@gmail.com>.
> On Sept. 29, 2015, 9:35 p.m., Ben Mahler wrote:
> > Tim, could you also get a libprocess maintainer to review this?
>
> Timothy Chen wrote:
> Volunteered you :)
I chatted with Yan, he'll be helping review this.
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38747/#review101026
-----------------------------------------------------------
On Sept. 29, 2015, 9:40 p.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38747/
> -----------------------------------------------------------
>
> (Updated Sept. 29, 2015, 9:40 p.m.)
>
>
> Review request for mesos, Ben Mahler, Gilbert Song, and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added SHA256, SHA512 implementation.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1
> 3rdparty/libprocess/include/Makefile.am fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e
> 3rdparty/libprocess/include/process/digest.hpp PRE-CREATION
> 3rdparty/libprocess/src/tests/CMakeLists.txt a14b5b8fe22d3e75bef3c716ae7865ddaf132927
> 3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/38747/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 38747: Adding digest utilities
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38747/#review101026
-----------------------------------------------------------
Tim, could you also get a libprocess maintainer to review this?
- Ben Mahler
On Sept. 29, 2015, 6:41 p.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38747/
> -----------------------------------------------------------
>
> (Updated Sept. 29, 2015, 6:41 p.m.)
>
>
> Review request for mesos, Gilbert Song and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added SHA256, SHA512 implementation.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1
> 3rdparty/libprocess/include/Makefile.am fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e
> 3rdparty/libprocess/include/process/digest.hpp PRE-CREATION
> 3rdparty/libprocess/src/tests/CMakeLists.txt a14b5b8fe22d3e75bef3c716ae7865ddaf132927
> 3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/38747/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 38747: Adding digest utilities
Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38747/
-----------------------------------------------------------
(Updated Sept. 29, 2015, 6:41 p.m.)
Review request for mesos, Gilbert Song and Timothy Chen.
Changes
-------
style fixes.
Repository: mesos
Description
-------
Added SHA256, SHA512 implementation.
Diffs (updated)
-----
3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1
3rdparty/libprocess/include/Makefile.am fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e
3rdparty/libprocess/include/process/digest.hpp PRE-CREATION
3rdparty/libprocess/src/tests/CMakeLists.txt a14b5b8fe22d3e75bef3c716ae7865ddaf132927
3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/38747/diff/
Testing
-------
make check.
Thanks,
Jojy Varghese
Re: Review Request 38747: Adding digest utilities
Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38747/
-----------------------------------------------------------
(Updated Sept. 29, 2015, 6:30 p.m.)
Review request for mesos, Gilbert Song and Timothy Chen.
Changes
-------
eliminated need for ifdef
Repository: mesos
Description
-------
Added SHA256, SHA512 implementation.
Diffs (updated)
-----
3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1
3rdparty/libprocess/include/Makefile.am fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e
3rdparty/libprocess/include/process/digest.hpp PRE-CREATION
3rdparty/libprocess/src/tests/CMakeLists.txt a14b5b8fe22d3e75bef3c716ae7865ddaf132927
3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/38747/diff/
Testing
-------
make check.
Thanks,
Jojy Varghese
Re: Review Request 38747: Adding digest utilities
Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38747/
-----------------------------------------------------------
(Updated Sept. 29, 2015, 12:19 a.m.)
Review request for mesos, Gilbert Song and Timothy Chen.
Changes
-------
Review comments, fixed bug in test
Repository: mesos
Description
-------
Added SHA256, SHA512 implementation.
Diffs (updated)
-----
3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1
3rdparty/libprocess/include/Makefile.am fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e
3rdparty/libprocess/include/process/digest.hpp PRE-CREATION
3rdparty/libprocess/src/tests/CMakeLists.txt a14b5b8fe22d3e75bef3c716ae7865ddaf132927
3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/38747/diff/
Testing
-------
make check.
Thanks,
Jojy Varghese
Re: Review Request 38747: Adding digest utilities
Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38747/
-----------------------------------------------------------
(Updated Sept. 28, 2015, 10:14 p.m.)
Review request for mesos, Gilbert Song and Timothy Chen.
Changes
-------
review comments.
Repository: mesos
Description
-------
Added SHA256, SHA512 implementation.
Diffs (updated)
-----
3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1
3rdparty/libprocess/include/Makefile.am fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e
3rdparty/libprocess/include/process/digest.hpp PRE-CREATION
3rdparty/libprocess/src/tests/CMakeLists.txt a14b5b8fe22d3e75bef3c716ae7865ddaf132927
3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/38747/diff/
Testing
-------
make check.
Thanks,
Jojy Varghese
Re: Review Request 38747: Adding digest utilities
Posted by Jojy Varghese <jo...@mesosphere.io>.
> On Sept. 28, 2015, 7:17 p.m., Jiang Yan Xu wrote:
> > High-level comments:
> >
> > - This implementation relies on USE_SSL_SOCKET which is tied to `--enable_ssl`. Conceptually the two should be decoupled. Ian Downes has an implementation which uses the system commands that are widely distributed on *nix boxes: https://reviews.apache.org/r/34138/. Can such an implemenation be used as a fallback at least?
> > - Can the implentation be put in a cpp file and leave only public (the recommended way of using this utility) methods in the header (e.g. `process::verify(...)` and `process::digest()`)?
Thanks for the suggestion and history.
- digest.hpp's process::digest and process::verify has default implementations (when USE_SSL_SOCKET is not available). So thats the fallback for now.
- Since this implementation is dependent on using traits/templates, the implementation is in hpp file
- I can work on adding Ian's implementation in this patch or let Ian add it as an extension to this patch.
- Jojy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38747/#review100850
-----------------------------------------------------------
On Sept. 28, 2015, 10:14 p.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38747/
> -----------------------------------------------------------
>
> (Updated Sept. 28, 2015, 10:14 p.m.)
>
>
> Review request for mesos, Gilbert Song and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added SHA256, SHA512 implementation.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1
> 3rdparty/libprocess/include/Makefile.am fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e
> 3rdparty/libprocess/include/process/digest.hpp PRE-CREATION
> 3rdparty/libprocess/src/tests/CMakeLists.txt a14b5b8fe22d3e75bef3c716ae7865ddaf132927
> 3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/38747/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 38747: Adding digest utilities
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38747/#review100850
-----------------------------------------------------------
High-level comments:
- This implementation relies on USE_SSL_SOCKET which is tied to `--enable_ssl`. Conceptually the two should be decoupled. Ian Downes has an implementation which uses the system commands that are widely distributed on *nix boxes: https://reviews.apache.org/r/34138/. Can such an implemenation be used as a fallback at least?
- Can the implentation be put in a cpp file and leave only public (the recommended way of using this utility) methods in the header (e.g. `process::verify(...)` and `process::digest()`)?
- Jiang Yan Xu
On Sept. 25, 2015, 1:33 p.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38747/
> -----------------------------------------------------------
>
> (Updated Sept. 25, 2015, 1:33 p.m.)
>
>
> Review request for mesos, Gilbert Song and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added SHA256, SHA512 implementation.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1
> 3rdparty/libprocess/include/Makefile.am fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e
> 3rdparty/libprocess/include/process/digest.hpp PRE-CREATION
> 3rdparty/libprocess/src/tests/CMakeLists.txt a14b5b8fe22d3e75bef3c716ae7865ddaf132927
> 3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/38747/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 38747: Adding digest utilities
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38747/#review100686
-----------------------------------------------------------
Patch looks great!
Reviews applied: [38443, 38579, 38580, 38747]
All tests passed.
- Mesos ReviewBot
On Sept. 25, 2015, 8:33 p.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38747/
> -----------------------------------------------------------
>
> (Updated Sept. 25, 2015, 8:33 p.m.)
>
>
> Review request for mesos, Gilbert Song and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added SHA256, SHA512 implementation.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1
> 3rdparty/libprocess/include/Makefile.am fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e
> 3rdparty/libprocess/include/process/digest.hpp PRE-CREATION
> 3rdparty/libprocess/src/tests/CMakeLists.txt a14b5b8fe22d3e75bef3c716ae7865ddaf132927
> 3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/38747/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jojy Varghese
>
>
Re: Review Request 38747: Adding digest utilities
Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38747/
-----------------------------------------------------------
(Updated Sept. 25, 2015, 8:33 p.m.)
Review request for mesos, Gilbert Song and Timothy Chen.
Changes
-------
build fix.
Repository: mesos
Description (updated)
-------
Added SHA256, SHA512 implementation.
Diffs (updated)
-----
3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1
3rdparty/libprocess/include/Makefile.am fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e
3rdparty/libprocess/include/process/digest.hpp PRE-CREATION
3rdparty/libprocess/src/tests/CMakeLists.txt a14b5b8fe22d3e75bef3c716ae7865ddaf132927
3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/38747/diff/
Testing
-------
make check.
Thanks,
Jojy Varghese
Re: Review Request 38747: Adding digest utilities
Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38747/
-----------------------------------------------------------
(Updated Sept. 25, 2015, 6:33 p.m.)
Review request for mesos, Gilbert Song and Timothy Chen.
Changes
-------
rebased with master
Repository: mesos
Description
-------
Added SHA256 implementation.
Diffs (updated)
-----
3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1
3rdparty/libprocess/include/Makefile.am fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e
3rdparty/libprocess/include/process/digest.hpp PRE-CREATION
3rdparty/libprocess/src/tests/CMakeLists.txt a14b5b8fe22d3e75bef3c716ae7865ddaf132927
3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/38747/diff/
Testing
-------
make check.
Thanks,
Jojy Varghese
Re: Review Request 38747: Adding digest utilities
Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38747/
-----------------------------------------------------------
(Updated Sept. 25, 2015, 4:45 p.m.)
Review request for mesos, Gilbert Song and Timothy Chen.
Changes
-------
addressed review comments
Repository: mesos
Description
-------
Added SHA256 implementation.
Diffs (updated)
-----
3rdparty/libprocess/Makefile.am 064310c61d8a0c5c5a87f305f1f62149dd3a6699
3rdparty/libprocess/include/Makefile.am fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e
3rdparty/libprocess/include/process/digest.hpp PRE-CREATION
3rdparty/libprocess/src/tests/CMakeLists.txt a14b5b8fe22d3e75bef3c716ae7865ddaf132927
3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/38747/diff/
Testing
-------
make check.
Thanks,
Jojy Varghese
Re: Review Request 38747: Adding digest utilities
Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38747/
-----------------------------------------------------------
(Updated Sept. 25, 2015, 3:23 p.m.)
Review request for mesos, Gilbert Song and Timothy Chen.
Changes
-------
Chnged implementation to make it generic.
Repository: mesos
Description
-------
Added SHA256 implementation.
Diffs (updated)
-----
3rdparty/libprocess/Makefile.am 064310c61d8a0c5c5a87f305f1f62149dd3a6699
3rdparty/libprocess/include/Makefile.am fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e
3rdparty/libprocess/include/process/digest.hpp PRE-CREATION
3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/38747/diff/
Testing
-------
make check.
Thanks,
Jojy Varghese
Re: Review Request 38747: Adding digest utilities
Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38747/
-----------------------------------------------------------
(Updated Sept. 25, 2015, 5:46 a.m.)
Review request for mesos, Gilbert Song and Timothy Chen.
Changes
-------
build fix
Repository: mesos
Description
-------
Added SHA256 implementation.
Diffs (updated)
-----
3rdparty/libprocess/Makefile.am 064310c61d8a0c5c5a87f305f1f62149dd3a6699
3rdparty/libprocess/include/Makefile.am fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e
3rdparty/libprocess/include/process/digest.hpp PRE-CREATION
3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/38747/diff/
Testing
-------
make check.
Thanks,
Jojy Varghese
Re: Review Request 38747: Adding digest utilities
Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38747/
-----------------------------------------------------------
(Updated Sept. 25, 2015, 5:43 a.m.)
Review request for mesos, Gilbert Song and Timothy Chen.
Changes
-------
minor fixes.
Repository: mesos
Description
-------
Added SHA256 implementation.
Diffs (updated)
-----
3rdparty/libprocess/Makefile.am 064310c61d8a0c5c5a87f305f1f62149dd3a6699
3rdparty/libprocess/include/Makefile.am fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e
3rdparty/libprocess/include/process/digest.hpp PRE-CREATION
Diff: https://reviews.apache.org/r/38747/diff/
Testing
-------
make check.
Thanks,
Jojy Varghese