You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Alex Clemmer <cl...@gmail.com> on 2015/10/22 20:23:35 UTC
Review Request 39559: Windows: Implemented `os::mkdtemp`.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39559/
-----------------------------------------------------------
Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
Repository: mesos
Description
-------
Windows: Implemented `os::mkdtemp`.
Diffs
-----
3rdparty/libprocess/3rdparty/stout/include/Makefile.am ba2836a72ceee948cb43364e80ada9f132f33d04
3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 7f70c9ea7d57634b5bfd40523ba37561ec92a09a
3rdparty/libprocess/3rdparty/stout/include/stout/os/mkdtemp.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/mkdtemp.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp b6afe0e76366d0bc68d37097ced83a1e14828d84
3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 3e6f2aafd0f541f512025dfa683ab4178701f7c4
Diff: https://reviews.apache.org/r/39559/diff/
Testing
-------
Thanks,
Alex Clemmer
Re: Review Request 39559: Windows: Implemented `os::mkdtemp`.
Posted by Joseph Wu <jo...@mesosphere.io>.
> On Oct. 23, 2015, 11:45 a.m., Joseph Wu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp, line 34
> > <https://reviews.apache.org/r/39559/diff/1/?file=1103501#file1103501line34>
> >
> > `strlen()` might be better/more-readable.
>
> Alex Clemmer wrote:
> So, I could definitely be mistaken, but it looks like `strlen` would have to be recomputed every time we call the function, right? And it looks like this code is normally run at initialization time?
>
> If so it seems like this way has a clear benefit over the `strlen` approach. What do you think? Maybe there is another way to make `strlen` an init-time expression? (I already looked at `constexpr`.)
>
> Alex Clemmer wrote:
> Post script: this exchange actually convinced me that we should compute the size of `alphabet` statically to! Let me know if you have a problem with this given the constraints I mentioned. :)
After taking a second pass at your code, I'd say `strlen` isn't really more readable. Dropping.
- Joseph
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39559/#review103815
-----------------------------------------------------------
On Oct. 29, 2015, 10:53 p.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39559/
> -----------------------------------------------------------
>
> (Updated Oct. 29, 2015, 10:53 p.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Implemented `os::mkdtemp`.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/Makefile.am 67b142bbc2d80f40a1e893cc5813d58dd2aa8381
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp fc2df6831ae2cb1a91c7a8cc92965939576e575d
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp e26df59d9b837e1f4a4b92577f0a3de4b9076cb4
> 3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp edf17d5ad8efbc988e909bfb8ffa5a015ecdc89d
>
> Diff: https://reviews.apache.org/r/39559/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39559: Windows: Implemented `os::mkdtemp`.
Posted by Alex Clemmer <cl...@gmail.com>.
> On Oct. 23, 2015, 6:45 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp, lines 36-41
> > <https://reviews.apache.org/r/39559/diff/1/?file=1103501#file1103501line36>
> >
> > The posix spec says that `XXXXXX` suffix is required, so `strings::endsWith` would be a better check.
Oh, good tip, I didn't know about that function.
- Alex
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39559/#review103815
-----------------------------------------------------------
On Oct. 22, 2015, 6:23 p.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39559/
> -----------------------------------------------------------
>
> (Updated Oct. 22, 2015, 6:23 p.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Implemented `os::mkdtemp`.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/Makefile.am ba2836a72ceee948cb43364e80ada9f132f33d04
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 7f70c9ea7d57634b5bfd40523ba37561ec92a09a
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp b6afe0e76366d0bc68d37097ced83a1e14828d84
> 3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 3e6f2aafd0f541f512025dfa683ab4178701f7c4
>
> Diff: https://reviews.apache.org/r/39559/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39559: Windows: Implemented `os::mkdtemp`.
Posted by Alex Clemmer <cl...@gmail.com>.
> On Oct. 23, 2015, 6:45 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp, line 34
> > <https://reviews.apache.org/r/39559/diff/1/?file=1103501#file1103501line34>
> >
> > `strlen()` might be better/more-readable.
>
> Alex Clemmer wrote:
> So, I could definitely be mistaken, but it looks like `strlen` would have to be recomputed every time we call the function, right? And it looks like this code is normally run at initialization time?
>
> If so it seems like this way has a clear benefit over the `strlen` approach. What do you think? Maybe there is another way to make `strlen` an init-time expression? (I already looked at `constexpr`.)
Post script: this exchange actually convinced me that we should compute the size of `alphabet` statically to! Let me know if you have a problem with this given the constraints I mentioned. :)
> On Oct. 23, 2015, 6:45 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp, lines 36-41
> > <https://reviews.apache.org/r/39559/diff/1/?file=1103501#file1103501line36>
> >
> > The posix spec says that `XXXXXX` suffix is required, so `strings::endsWith` would be a better check.
>
> Alex Clemmer wrote:
> Oh, good tip, I didn't know about that function.
Thus turns out to have been a great suggestion. Simplified the code greatly, so thanks!
- Alex
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39559/#review103815
-----------------------------------------------------------
On Oct. 27, 2015, 8:14 a.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39559/
> -----------------------------------------------------------
>
> (Updated Oct. 27, 2015, 8:14 a.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Implemented `os::mkdtemp`.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/Makefile.am ba2836a72ceee948cb43364e80ada9f132f33d04
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 7f70c9ea7d57634b5bfd40523ba37561ec92a09a
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp b6afe0e76366d0bc68d37097ced83a1e14828d84
> 3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 3e6f2aafd0f541f512025dfa683ab4178701f7c4
>
> Diff: https://reviews.apache.org/r/39559/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39559: Windows: Implemented `os::mkdtemp`.
Posted by Alex Clemmer <cl...@gmail.com>.
> On Oct. 23, 2015, 6:45 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp, line 48
> > <https://reviews.apache.org/r/39559/diff/1/?file=1103501#file1103501line48>
> >
> > That `- 1` doesn't match the "size" described by the variable name. Seems like you're describing the `max_alphabet_index`.
Marking as fixed, but, I just want to verify: You meant `maxAlphabetIndex`, right? It looks like we're not using snake case. Have I missed something important?
- Alex
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39559/#review103815
-----------------------------------------------------------
On Oct. 27, 2015, 8:14 a.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39559/
> -----------------------------------------------------------
>
> (Updated Oct. 27, 2015, 8:14 a.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Implemented `os::mkdtemp`.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/Makefile.am ba2836a72ceee948cb43364e80ada9f132f33d04
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 7f70c9ea7d57634b5bfd40523ba37561ec92a09a
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp b6afe0e76366d0bc68d37097ced83a1e14828d84
> 3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 3e6f2aafd0f541f512025dfa683ab4178701f7c4
>
> Diff: https://reviews.apache.org/r/39559/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39559: Windows: Implemented `os::mkdtemp`.
Posted by Alex Clemmer <cl...@gmail.com>.
> On Oct. 23, 2015, 6:45 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp, line 34
> > <https://reviews.apache.org/r/39559/diff/1/?file=1103501#file1103501line34>
> >
> > `strlen()` might be better/more-readable.
So, I could definitely be mistaken, but it looks like `strlen` would have to be recomputed every time we call the function, right? And it looks like this code is normally run at initialization time?
If so it seems like this way has a clear benefit over the `strlen` approach. What do you think? Maybe there is another way to make `strlen` an init-time expression? (I already looked at `constexpr`.)
- Alex
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39559/#review103815
-----------------------------------------------------------
On Oct. 22, 2015, 6:23 p.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39559/
> -----------------------------------------------------------
>
> (Updated Oct. 22, 2015, 6:23 p.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Implemented `os::mkdtemp`.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/Makefile.am ba2836a72ceee948cb43364e80ada9f132f33d04
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 7f70c9ea7d57634b5bfd40523ba37561ec92a09a
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp b6afe0e76366d0bc68d37097ced83a1e14828d84
> 3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 3e6f2aafd0f541f512025dfa683ab4178701f7c4
>
> Diff: https://reviews.apache.org/r/39559/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39559: Windows: Implemented `os::mkdtemp`.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39559/#review103815
-----------------------------------------------------------
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp (line 34)
<https://reviews.apache.org/r/39559/#comment161905>
`strlen()` might be better/more-readable.
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp (lines 36 - 41)
<https://reviews.apache.org/r/39559/#comment161904>
The posix spec says that `XXXXXX` suffix is required, so `strings::endsWith` would be a better check.
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp (line 48)
<https://reviews.apache.org/r/39559/#comment161906>
That `- 1` doesn't match the "size" described by the variable name. Seems like you're describing the `max_alphabet_index`.
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp (lines 62 - 74)
<https://reviews.apache.org/r/39559/#comment161908>
If you use `strings::endsWith` above, you won't need this validation.
- Joseph Wu
On Oct. 22, 2015, 11:23 a.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39559/
> -----------------------------------------------------------
>
> (Updated Oct. 22, 2015, 11:23 a.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Implemented `os::mkdtemp`.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/Makefile.am ba2836a72ceee948cb43364e80ada9f132f33d04
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 7f70c9ea7d57634b5bfd40523ba37561ec92a09a
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp b6afe0e76366d0bc68d37097ced83a1e14828d84
> 3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 3e6f2aafd0f541f512025dfa683ab4178701f7c4
>
> Diff: https://reviews.apache.org/r/39559/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39559: Windows: Implemented `os::mkdtemp`.
Posted by Alex Clemmer <cl...@gmail.com>.
> On Oct. 22, 2015, 6:33 p.m., Alex Clemmer wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp, line 31
> > <https://reviews.apache.org/r/39559/diff/1/?file=1103501#file1103501line31>
> >
> > Just so I catch it before it comes up: this is new code that needs to be tested, but we wrote it so that we could pull the FS tests out of `os_test.hpp`.
> >
> > So my brief answer to "yo, are you stupid, you can't commit this stuff with tests" is "yes, I agree, but I have to write this before we can open up the tests!" :)
(And yes, I did test it _ad hoc_ before I pushed it. But that is not repeatable, we just need it to be "good enough" to run the actual tests.)
- Alex
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39559/#review103636
-----------------------------------------------------------
On Oct. 22, 2015, 6:23 p.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39559/
> -----------------------------------------------------------
>
> (Updated Oct. 22, 2015, 6:23 p.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Implemented `os::mkdtemp`.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/Makefile.am ba2836a72ceee948cb43364e80ada9f132f33d04
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 7f70c9ea7d57634b5bfd40523ba37561ec92a09a
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp b6afe0e76366d0bc68d37097ced83a1e14828d84
> 3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 3e6f2aafd0f541f512025dfa683ab4178701f7c4
>
> Diff: https://reviews.apache.org/r/39559/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39559: Windows: Implemented `os::mkdtemp`.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39559/#review103636
-----------------------------------------------------------
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp (line 31)
<https://reviews.apache.org/r/39559/#comment161696>
Just so I catch it before it comes up: this is new code that needs to be tested, but we wrote it so that we could pull the FS tests out of `os_test.hpp`.
So my brief answer to "yo, are you stupid, you can't commit this stuff with tests" is "yes, I agree, but I have to write this before we can open up the tests!" :)
- Alex Clemmer
On Oct. 22, 2015, 6:23 p.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39559/
> -----------------------------------------------------------
>
> (Updated Oct. 22, 2015, 6:23 p.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Implemented `os::mkdtemp`.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/Makefile.am ba2836a72ceee948cb43364e80ada9f132f33d04
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 7f70c9ea7d57634b5bfd40523ba37561ec92a09a
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp b6afe0e76366d0bc68d37097ced83a1e14828d84
> 3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 3e6f2aafd0f541f512025dfa683ab4178701f7c4
>
> Diff: https://reviews.apache.org/r/39559/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39559: Windows: Implemented `os::mkdtemp`.
Posted by Joseph Wu <jo...@mesosphere.io>.
> On Nov. 2, 2015, 12:38 p.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp, lines 34-71
> > <https://reviews.apache.org/r/39559/diff/3/?file=1112902#file1112902line34>
> >
> > Can you please explain why we can't just use `mktemp` here?
> > I acknowledge that the idea behind `mktemp` is to guarantee that the directory name can't collide with an existing one, but the custom implementation you provide also doesn't do that while generating the name, correct?
Windows doesn't have a temp-directory function. Are you suggesting calling `mktemp` and then using the file's name as the argument to `mkdir`?
- Joseph
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39559/#review104778
-----------------------------------------------------------
On Oct. 29, 2015, 10:53 p.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39559/
> -----------------------------------------------------------
>
> (Updated Oct. 29, 2015, 10:53 p.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Implemented `os::mkdtemp`.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/Makefile.am 67b142bbc2d80f40a1e893cc5813d58dd2aa8381
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp fc2df6831ae2cb1a91c7a8cc92965939576e575d
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp e26df59d9b837e1f4a4b92577f0a3de4b9076cb4
> 3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp edf17d5ad8efbc988e909bfb8ffa5a015ecdc89d
>
> Diff: https://reviews.apache.org/r/39559/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39559: Windows: Implemented `os::mkdtemp`.
Posted by Joris Van Remoortere <jo...@gmail.com>.
> On Nov. 2, 2015, 8:38 p.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp, lines 34-71
> > <https://reviews.apache.org/r/39559/diff/3/?file=1112902#file1112902line34>
> >
> > Can you please explain why we can't just use `mktemp` here?
> > I acknowledge that the idea behind `mktemp` is to guarantee that the directory name can't collide with an existing one, but the custom implementation you provide also doesn't do that while generating the name, correct?
>
> Joseph Wu wrote:
> Windows doesn't have a temp-directory function. Are you suggesting calling `mktemp` and then using the file's name as the argument to `mkdir`?
Indeed. It seems like that's what we are doing manually right now?
- Joris
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39559/#review104778
-----------------------------------------------------------
On Oct. 30, 2015, 5:53 a.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39559/
> -----------------------------------------------------------
>
> (Updated Oct. 30, 2015, 5:53 a.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Implemented `os::mkdtemp`.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/Makefile.am 67b142bbc2d80f40a1e893cc5813d58dd2aa8381
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp fc2df6831ae2cb1a91c7a8cc92965939576e575d
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp e26df59d9b837e1f4a4b92577f0a3de4b9076cb4
> 3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp edf17d5ad8efbc988e909bfb8ffa5a015ecdc89d
>
> Diff: https://reviews.apache.org/r/39559/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39559: Windows: Implemented `os::mkdtemp`.
Posted by Alex Clemmer <cl...@gmail.com>.
> On Nov. 2, 2015, 8:38 p.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp, lines 34-71
> > <https://reviews.apache.org/r/39559/diff/3/?file=1112902#file1112902line34>
> >
> > Can you please explain why we can't just use `mktemp` here?
> > I acknowledge that the idea behind `mktemp` is to guarantee that the directory name can't collide with an existing one, but the custom implementation you provide also doesn't do that while generating the name, correct?
>
> Joseph Wu wrote:
> Windows doesn't have a temp-directory function. Are you suggesting calling `mktemp` and then using the file's name as the argument to `mkdir`?
>
> Joris Van Remoortere wrote:
> Indeed. It seems like that's what we are doing manually right now?
>
> Alex Clemmer wrote:
> I'm not a C programmer, so feel free to tell me if there is faulty reasoning here, but a colleague looked at an early version of this code and told me that in some versions of the C standard library on Windows `_mktemp` and `_mktemp_s` will replace the template `XXXXXX` with the process ID, and append a single letter to the end, meaning that there are 26 unique filenames possible. And, while `mktemp` and `mkstemp` seem to be used only in the test harness, `mkdtemp` is in a few places that seem important. So, I figured, it will take me not-too-long to just write the function myself, so why not. I looked at some other implementations (_e.g._ that of Postgres) but they were mostly quite complex due to trying to be cryptographically secure. So I opted for my simple approach.
>
> I think you are right that we should retry if there is a name collision though, so based on my read, I suggest that I make that change.
>
> Does this seem reasonable to you?
Ok, per the extended discussion, I've decided to leave this alone. The reasons, summarized, are:
(1) We agree we can't use `_mktemp_s` and `_mktemp` due to the documentation explicitly saying only 26 filenames are possible
(2) This is only used in tests, unlike `mkstemp` (which, by the way, does not suffer from this problem), which means it is ok for this not to be a completely robust solution. (BTW, we should also _never_ use `mktemp` on Linux either, for the same reason as above; this is just a general problem with implementations of this routine, especially on older systems.)
(3) The existing solutions that are Apache v2 compatible are dramatically more complicated, and not worth the code review overhead
- Alex
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39559/#review104778
-----------------------------------------------------------
On Nov. 9, 2015, 5:58 p.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39559/
> -----------------------------------------------------------
>
> (Updated Nov. 9, 2015, 5:58 p.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Implemented `os::mkdtemp`.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/Makefile.am 741639a942971e48e2dac42db238d423e61cac21
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp fc2df6831ae2cb1a91c7a8cc92965939576e575d
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp e26df59d9b837e1f4a4b92577f0a3de4b9076cb4
> 3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp edf17d5ad8efbc988e909bfb8ffa5a015ecdc89d
>
> Diff: https://reviews.apache.org/r/39559/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39559: Windows: Implemented `os::mkdtemp`.
Posted by Alex Clemmer <cl...@gmail.com>.
> On Nov. 2, 2015, 8:38 p.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/mkdtemp.hpp, line 16
> > <https://reviews.apache.org/r/39559/diff/3/?file=1112901#file1112901line16>
> >
> > `<stdlib.h>`?
Just for my own education, what parts of `stdlib.h` are used in here? I must have missed them.
> On Nov. 2, 2015, 8:38 p.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp, line 34
> > <https://reviews.apache.org/r/39559/diff/3/?file=1112902#file1112902line34>
> >
> > Let's try to be consistent about capitalizing after `NOTE:`
Just to be explicit about this fix: I have grep'd the codebase and found that we capitalize the first letter. If you meant for me to lowercase it, feel free to bother me about it again.
> On Nov. 2, 2015, 8:38 p.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp, lines 34-71
> > <https://reviews.apache.org/r/39559/diff/3/?file=1112902#file1112902line34>
> >
> > Can you please explain why we can't just use `mktemp` here?
> > I acknowledge that the idea behind `mktemp` is to guarantee that the directory name can't collide with an existing one, but the custom implementation you provide also doesn't do that while generating the name, correct?
>
> Joseph Wu wrote:
> Windows doesn't have a temp-directory function. Are you suggesting calling `mktemp` and then using the file's name as the argument to `mkdir`?
>
> Joris Van Remoortere wrote:
> Indeed. It seems like that's what we are doing manually right now?
I'm not a C programmer, so feel free to tell me if there is faulty reasoning here, but a colleague looked at an early version of this code and told me that in some versions of the C standard library on Windows `_mktemp` and `_mktemp_s` will replace the template `XXXXXX` with the process ID, and append a single letter to the end, meaning that there are 26 unique filenames possible. And, while `mktemp` and `mkstemp` seem to be used only in the test harness, `mkdtemp` is in a few places that seem important. So, I figured, it will take me not-too-long to just write the function myself, so why not. I looked at some other implementations (_e.g._ that of Postgres) but they were mostly quite complex due to trying to be cryptographically secure. So I opted for my simple approach.
I think you are right that we should retry if there is a name collision though, so based on my read, I suggest that I make that change.
Does this seem reasonable to you?
- Alex
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39559/#review104778
-----------------------------------------------------------
On Oct. 30, 2015, 5:53 a.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39559/
> -----------------------------------------------------------
>
> (Updated Oct. 30, 2015, 5:53 a.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Implemented `os::mkdtemp`.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/Makefile.am 67b142bbc2d80f40a1e893cc5813d58dd2aa8381
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp fc2df6831ae2cb1a91c7a8cc92965939576e575d
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp e26df59d9b837e1f4a4b92577f0a3de4b9076cb4
> 3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp edf17d5ad8efbc988e909bfb8ffa5a015ecdc89d
>
> Diff: https://reviews.apache.org/r/39559/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39559: Windows: Implemented `os::mkdtemp`.
Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39559/#review104778
-----------------------------------------------------------
3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/mkdtemp.hpp (line 16)
<https://reviews.apache.org/r/39559/#comment163018>
`<stdlib.h>`?
3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/mkdtemp.hpp (line 18)
<https://reviews.apache.org/r/39559/#comment163017>
`<string>`
`<stout/error.hpp>`
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp (lines 19 - 21)
<https://reviews.apache.org/r/39559/#comment163027>
order?
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp (line 34)
<https://reviews.apache.org/r/39559/#comment163020>
Let's try to be consistent about capitalizing after `NOTE:`
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp (lines 34 - 71)
<https://reviews.apache.org/r/39559/#comment163026>
Can you please explain why we can't just use `mktemp` here?
I acknowledge that the idea behind `mktemp` is to guarantee that the directory name can't collide with an existing one, but the custom implementation you provide also doesn't do that while generating the name, correct?
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp (line 52)
<https://reviews.apache.org/r/39559/#comment163021>
Let's try to be consistent about capitalizing after `NOTE:`
- Joris Van Remoortere
On Oct. 30, 2015, 5:53 a.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39559/
> -----------------------------------------------------------
>
> (Updated Oct. 30, 2015, 5:53 a.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Implemented `os::mkdtemp`.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/Makefile.am 67b142bbc2d80f40a1e893cc5813d58dd2aa8381
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp fc2df6831ae2cb1a91c7a8cc92965939576e575d
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp e26df59d9b837e1f4a4b92577f0a3de4b9076cb4
> 3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp edf17d5ad8efbc988e909bfb8ffa5a015ecdc89d
>
> Diff: https://reviews.apache.org/r/39559/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39559: Windows: Implemented `os::mkdtemp`.
Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39559/#review109646
-----------------------------------------------------------
Ship it!
3rdparty/libprocess/3rdparty/stout/include/Makefile.am (lines 67 - 68)
<https://reviews.apache.org/r/39559/#comment169238>
alphabetic
3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp (lines 85 - 86)
<https://reviews.apache.org/r/39559/#comment169239>
alphabetic.
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp (lines 47 - 48)
<https://reviews.apache.org/r/39559/#comment169245>
indentation.
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp (lines 65 - 68)
<https://reviews.apache.org/r/39559/#comment169247>
rand() won't be threadsafe.
Let's use stout's thread_locals and do something like:
```
static THREAD_LOCAL std::mt19937 generator;
int index = generator() % maxAlphabetIndex;
```
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp (line 77)
<https://reviews.apache.org/r/39559/#comment169248>
Can we call this variable `mkdir`?
- Joris Van Remoortere
On Nov. 16, 2015, 9:13 a.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39559/
> -----------------------------------------------------------
>
> (Updated Nov. 16, 2015, 9:13 a.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Implemented `os::mkdtemp`.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/Makefile.am a8c35c086ecae21701f6a720f25231c1b0d4e329
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 5c1df81193b4b888d2ed5c7dbfa0b5e2fae48467
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp e26df59d9b837e1f4a4b92577f0a3de4b9076cb4
> 3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp edf17d5ad8efbc988e909bfb8ffa5a015ecdc89d
>
> Diff: https://reviews.apache.org/r/39559/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39559: Windows: Implemented `os::mkdtemp`.
Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39559/#review110020
-----------------------------------------------------------
Ship it!
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp (lines 71 - 72)
<https://reviews.apache.org/r/39559/#comment169775>
Let's seed this with std::random_device inline.
- Joris Van Remoortere
On Dec. 11, 2015, 8:10 p.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39559/
> -----------------------------------------------------------
>
> (Updated Dec. 11, 2015, 8:10 p.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Bugs: MESOS-4108
> https://issues.apache.org/jira/browse/MESOS-4108
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Implemented `os::mkdtemp`.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/Makefile.am a8c35c086ecae21701f6a720f25231c1b0d4e329
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 03e6f75850561b5eb92da4771fbe18e4057ad520
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 99e0ae40971bd547dc55c42c090df4206cd41a17
> 3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp e2d7424d01b8076c584370ac7c628284ca7ae54f
>
> Diff: https://reviews.apache.org/r/39559/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39559: Windows: Implemented `os::mkdtemp`.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39559/
-----------------------------------------------------------
(Updated Dec. 11, 2015, 8:59 p.m.)
Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
Bugs: MESOS-4108
https://issues.apache.org/jira/browse/MESOS-4108
Repository: mesos
Description
-------
Windows: Implemented `os::mkdtemp`.
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/stout/include/Makefile.am a8c35c086ecae21701f6a720f25231c1b0d4e329
3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 03e6f75850561b5eb92da4771fbe18e4057ad520
3rdparty/libprocess/3rdparty/stout/include/stout/os/mkdtemp.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/mkdtemp.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 99e0ae40971bd547dc55c42c090df4206cd41a17
3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp e2d7424d01b8076c584370ac7c628284ca7ae54f
Diff: https://reviews.apache.org/r/39559/diff/
Testing
-------
Thanks,
Alex Clemmer
Re: Review Request 39559: Windows: Implemented `os::mkdtemp`.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39559/
-----------------------------------------------------------
(Updated Dec. 11, 2015, 8:10 p.m.)
Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
Bugs: MESOS-4108
https://issues.apache.org/jira/browse/MESOS-4108
Repository: mesos
Description
-------
Windows: Implemented `os::mkdtemp`.
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/stout/include/Makefile.am a8c35c086ecae21701f6a720f25231c1b0d4e329
3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 03e6f75850561b5eb92da4771fbe18e4057ad520
3rdparty/libprocess/3rdparty/stout/include/stout/os/mkdtemp.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/mkdtemp.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 99e0ae40971bd547dc55c42c090df4206cd41a17
3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp e2d7424d01b8076c584370ac7c628284ca7ae54f
Diff: https://reviews.apache.org/r/39559/diff/
Testing
-------
Thanks,
Alex Clemmer
Re: Review Request 39559: Windows: Implemented `os::mkdtemp`.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39559/
-----------------------------------------------------------
(Updated Dec. 11, 2015, 8:03 p.m.)
Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
Bugs: MESOS-4108
https://issues.apache.org/jira/browse/MESOS-4108
Repository: mesos
Description
-------
Windows: Implemented `os::mkdtemp`.
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/stout/include/Makefile.am a8c35c086ecae21701f6a720f25231c1b0d4e329
3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 03e6f75850561b5eb92da4771fbe18e4057ad520
3rdparty/libprocess/3rdparty/stout/include/stout/os/mkdtemp.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/mkdtemp.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 99e0ae40971bd547dc55c42c090df4206cd41a17
3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp e2d7424d01b8076c584370ac7c628284ca7ae54f
Diff: https://reviews.apache.org/r/39559/diff/
Testing
-------
Thanks,
Alex Clemmer
Re: Review Request 39559: Windows: Implemented `os::mkdtemp`.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39559/
-----------------------------------------------------------
(Updated Dec. 11, 2015, 7:56 p.m.)
Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
Bugs: MESOS-4108
https://issues.apache.org/jira/browse/MESOS-4108
Repository: mesos
Description
-------
Windows: Implemented `os::mkdtemp`.
Diffs
-----
3rdparty/libprocess/3rdparty/stout/include/Makefile.am a8c35c086ecae21701f6a720f25231c1b0d4e329
3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 5c1df81193b4b888d2ed5c7dbfa0b5e2fae48467
3rdparty/libprocess/3rdparty/stout/include/stout/os/mkdtemp.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/mkdtemp.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp e26df59d9b837e1f4a4b92577f0a3de4b9076cb4
3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp edf17d5ad8efbc988e909bfb8ffa5a015ecdc89d
Diff: https://reviews.apache.org/r/39559/diff/
Testing
-------
Thanks,
Alex Clemmer
Re: Review Request 39559: Windows: Implemented `os::mkdtemp`.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39559/
-----------------------------------------------------------
(Updated Dec. 11, 2015, 6:12 p.m.)
Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
Bugs: MESOS-4108
https://issues.apache.org/jira/browse/MESOS-4108
Repository: mesos
Description
-------
Windows: Implemented `os::mkdtemp`.
Diffs
-----
3rdparty/libprocess/3rdparty/stout/include/Makefile.am a8c35c086ecae21701f6a720f25231c1b0d4e329
3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 5c1df81193b4b888d2ed5c7dbfa0b5e2fae48467
3rdparty/libprocess/3rdparty/stout/include/stout/os/mkdtemp.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/mkdtemp.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp e26df59d9b837e1f4a4b92577f0a3de4b9076cb4
3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp edf17d5ad8efbc988e909bfb8ffa5a015ecdc89d
Diff: https://reviews.apache.org/r/39559/diff/
Testing
-------
Thanks,
Alex Clemmer
Re: Review Request 39559: Windows: Implemented `os::mkdtemp`.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39559/
-----------------------------------------------------------
(Updated Dec. 10, 2015, 1:14 a.m.)
Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
Bugs: MESOS-4108
https://issues.apache.org/jira/browse/MESOS-4108
Repository: mesos
Description
-------
Windows: Implemented `os::mkdtemp`.
Diffs
-----
3rdparty/libprocess/3rdparty/stout/include/Makefile.am a8c35c086ecae21701f6a720f25231c1b0d4e329
3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 5c1df81193b4b888d2ed5c7dbfa0b5e2fae48467
3rdparty/libprocess/3rdparty/stout/include/stout/os/mkdtemp.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/mkdtemp.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp e26df59d9b837e1f4a4b92577f0a3de4b9076cb4
3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp edf17d5ad8efbc988e909bfb8ffa5a015ecdc89d
Diff: https://reviews.apache.org/r/39559/diff/
Testing
-------
Thanks,
Alex Clemmer
Re: Review Request 39559: Windows: Implemented `os::mkdtemp`.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39559/
-----------------------------------------------------------
(Updated Nov. 16, 2015, 9:13 a.m.)
Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
Repository: mesos
Description
-------
Windows: Implemented `os::mkdtemp`.
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/stout/include/Makefile.am a8c35c086ecae21701f6a720f25231c1b0d4e329
3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 5c1df81193b4b888d2ed5c7dbfa0b5e2fae48467
3rdparty/libprocess/3rdparty/stout/include/stout/os/mkdtemp.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/mkdtemp.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp e26df59d9b837e1f4a4b92577f0a3de4b9076cb4
3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp edf17d5ad8efbc988e909bfb8ffa5a015ecdc89d
Diff: https://reviews.apache.org/r/39559/diff/
Testing
-------
Thanks,
Alex Clemmer
Re: Review Request 39559: Windows: Implemented `os::mkdtemp`.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39559/
-----------------------------------------------------------
(Updated Nov. 9, 2015, 5:58 p.m.)
Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
Repository: mesos
Description
-------
Windows: Implemented `os::mkdtemp`.
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/stout/include/Makefile.am 741639a942971e48e2dac42db238d423e61cac21
3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp fc2df6831ae2cb1a91c7a8cc92965939576e575d
3rdparty/libprocess/3rdparty/stout/include/stout/os/mkdtemp.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/mkdtemp.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp e26df59d9b837e1f4a4b92577f0a3de4b9076cb4
3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp edf17d5ad8efbc988e909bfb8ffa5a015ecdc89d
Diff: https://reviews.apache.org/r/39559/diff/
Testing
-------
Thanks,
Alex Clemmer
Re: Review Request 39559: Windows: Implemented `os::mkdtemp`.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39559/#review104739
-----------------------------------------------------------
Ship it!
LGTM.
Note: I didn't actually run the code on Windows.
3rdparty/libprocess/3rdparty/stout/include/Makefile.am (line 84)
<https://reviews.apache.org/r/39559/#comment162988>
Missing tab for alignment.
- Joseph Wu
On Oct. 29, 2015, 10:53 p.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39559/
> -----------------------------------------------------------
>
> (Updated Oct. 29, 2015, 10:53 p.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Implemented `os::mkdtemp`.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/Makefile.am 67b142bbc2d80f40a1e893cc5813d58dd2aa8381
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp fc2df6831ae2cb1a91c7a8cc92965939576e575d
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp e26df59d9b837e1f4a4b92577f0a3de4b9076cb4
> 3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp edf17d5ad8efbc988e909bfb8ffa5a015ecdc89d
>
> Diff: https://reviews.apache.org/r/39559/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39559: Windows: Implemented `os::mkdtemp`.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39559/
-----------------------------------------------------------
(Updated Oct. 30, 2015, 5:53 a.m.)
Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
Repository: mesos
Description
-------
Windows: Implemented `os::mkdtemp`.
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/stout/include/Makefile.am 67b142bbc2d80f40a1e893cc5813d58dd2aa8381
3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp fc2df6831ae2cb1a91c7a8cc92965939576e575d
3rdparty/libprocess/3rdparty/stout/include/stout/os/mkdtemp.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/mkdtemp.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp e26df59d9b837e1f4a4b92577f0a3de4b9076cb4
3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp edf17d5ad8efbc988e909bfb8ffa5a015ecdc89d
Diff: https://reviews.apache.org/r/39559/diff/
Testing
-------
Thanks,
Alex Clemmer
Re: Review Request 39559: Windows: Implemented `os::mkdtemp`.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39559/
-----------------------------------------------------------
(Updated Oct. 27, 2015, 8:14 a.m.)
Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
Repository: mesos
Description
-------
Windows: Implemented `os::mkdtemp`.
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/stout/include/Makefile.am ba2836a72ceee948cb43364e80ada9f132f33d04
3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 7f70c9ea7d57634b5bfd40523ba37561ec92a09a
3rdparty/libprocess/3rdparty/stout/include/stout/os/mkdtemp.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/mkdtemp.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/mkdtemp.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp b6afe0e76366d0bc68d37097ced83a1e14828d84
3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 3e6f2aafd0f541f512025dfa683ab4178701f7c4
Diff: https://reviews.apache.org/r/39559/diff/
Testing
-------
Thanks,
Alex Clemmer