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/23 10:57:48 UTC
Review Request 39584: Windows: Implemented `os::rmdir.hpp`.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39584/
-----------------------------------------------------------
Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
Repository: mesos
Description
-------
Windows: Implemented `os::rmdir.hpp`.
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/posix/rmdir.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.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/39584/diff/
Testing
-------
`make check` from autotools on Ubuntu 15.
`make check` from CMake on OS X 10.10.
Ran `check` project in VS on Windows 10.
Thanks,
Alex Clemmer
Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39584/#review103858
-----------------------------------------------------------
Patch looks great!
Reviews applied: [39537, 39538, 39539, 39540, 39541, 39383, 39559, 39219, 39560, 39583, 39584]
All tests passed.
- Mesos ReviewBot
On Oct. 23, 2015, 4:30 p.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39584/
> -----------------------------------------------------------
>
> (Updated Oct. 23, 2015, 4:30 p.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Implemented `os::rmdir.hpp`.
>
>
> 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/posix/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.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/39584/diff/
>
>
> Testing
> -------
>
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39584/#review104771
-----------------------------------------------------------
Ship it!
Double-checked non-Windows builds.
- Joseph Wu
On Oct. 29, 2015, 11:11 p.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39584/
> -----------------------------------------------------------
>
> (Updated Oct. 29, 2015, 11:11 p.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Implemented `os::rmdir.hpp`.
>
>
> 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/posix/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.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/39584/diff/
>
>
> Testing
> -------
>
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.
Posted by Daniel Pravat <dp...@outlook.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39584/#review112669
-----------------------------------------------------------
Ship it!
Ship It!
- Daniel Pravat
On Jan. 4, 2016, 12:02 p.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39584/
> -----------------------------------------------------------
>
> (Updated Jan. 4, 2016, 12:02 p.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Implemented `os::rmdir.hpp`.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/Makefile.am b2dea9b93adfa3ea0415a0b5c81a369dd29b6cfe
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 14fbca6d222bdfc0e8be301050b4ea1a8a6e7758
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 4cf693fb7e8c6bb3ad1920ebe90d61f0adb5dc99
> 3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp e738cdbf5846950c475c159fb9a770acc45159f5
>
> Diff: https://reviews.apache.org/r/39584/diff/
>
>
> Testing
> -------
>
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.
Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39584/#review114875
-----------------------------------------------------------
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (line 32)
<https://reviews.apache.org/r/39584/#comment175712>
also `snake_case` for this function name as we did in `stat`.
- Joris Van Remoortere
On Jan. 15, 2016, 7:29 p.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39584/
> -----------------------------------------------------------
>
> (Updated Jan. 15, 2016, 7:29 p.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Implemented `os::rmdir.hpp`.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/Makefile.am ec851dcb08d938defeb6066810011e003d594b29
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 14fbca6d222bdfc0e8be301050b4ea1a8a6e7758
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 4cf693fb7e8c6bb3ad1920ebe90d61f0adb5dc99
> 3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp e738cdbf5846950c475c159fb9a770acc45159f5
>
> Diff: https://reviews.apache.org/r/39584/diff/
>
>
> Testing
> -------
>
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39584/
-----------------------------------------------------------
(Updated Jan. 16, 2016, 8:44 p.m.)
Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
Bugs: MESOS-4415
https://issues.apache.org/jira/browse/MESOS-4415
Repository: mesos
Description
-------
Windows: Implemented `os::rmdir.hpp`.
Diffs
-----
3rdparty/libprocess/3rdparty/stout/include/Makefile.am ec851dcb08d938defeb6066810011e003d594b29
3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 14fbca6d222bdfc0e8be301050b4ea1a8a6e7758
3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 4cf693fb7e8c6bb3ad1920ebe90d61f0adb5dc99
3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp e738cdbf5846950c475c159fb9a770acc45159f5
Diff: https://reviews.apache.org/r/39584/diff/
Testing
-------
`make check` from autotools on Ubuntu 15.
`make check` from CMake on OS X 10.10.
Ran `check` project in VS on Windows 10.
Thanks,
Alex Clemmer
Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39584/
-----------------------------------------------------------
(Updated Jan. 15, 2016, 7:29 p.m.)
Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
Repository: mesos
Description
-------
Windows: Implemented `os::rmdir.hpp`.
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/stout/include/Makefile.am ec851dcb08d938defeb6066810011e003d594b29
3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 14fbca6d222bdfc0e8be301050b4ea1a8a6e7758
3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 4cf693fb7e8c6bb3ad1920ebe90d61f0adb5dc99
3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp e738cdbf5846950c475c159fb9a770acc45159f5
Diff: https://reviews.apache.org/r/39584/diff/
Testing
-------
`make check` from autotools on Ubuntu 15.
`make check` from CMake on OS X 10.10.
Ran `check` project in VS on Windows 10.
Thanks,
Alex Clemmer
Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.
Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39584/#review114562
-----------------------------------------------------------
Ship it!
snake_case
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (line 37)
<https://reviews.apache.org/r/39584/#comment175407>
Can you switch to `snake_case` as per the other reviews.
- Joris Van Remoortere
On Jan. 7, 2016, 8:29 p.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39584/
> -----------------------------------------------------------
>
> (Updated Jan. 7, 2016, 8:29 p.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Implemented `os::rmdir.hpp`.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/Makefile.am b2dea9b93adfa3ea0415a0b5c81a369dd29b6cfe
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 14fbca6d222bdfc0e8be301050b4ea1a8a6e7758
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 4cf693fb7e8c6bb3ad1920ebe90d61f0adb5dc99
> 3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp e738cdbf5846950c475c159fb9a770acc45159f5
>
> Diff: https://reviews.apache.org/r/39584/diff/
>
>
> Testing
> -------
>
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39584/
-----------------------------------------------------------
(Updated Jan. 7, 2016, 8:29 p.m.)
Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
Repository: mesos
Description
-------
Windows: Implemented `os::rmdir.hpp`.
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/stout/include/Makefile.am b2dea9b93adfa3ea0415a0b5c81a369dd29b6cfe
3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 14fbca6d222bdfc0e8be301050b4ea1a8a6e7758
3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 4cf693fb7e8c6bb3ad1920ebe90d61f0adb5dc99
3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp e738cdbf5846950c475c159fb9a770acc45159f5
Diff: https://reviews.apache.org/r/39584/diff/
Testing
-------
`make check` from autotools on Ubuntu 15.
`make check` from CMake on OS X 10.10.
Ran `check` project in VS on Windows 10.
Thanks,
Alex Clemmer
Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.
Posted by Yi Sun <yi...@hotmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39584/#review112953
-----------------------------------------------------------
Ship it!
Ship It!
- Yi Sun
On Jan. 5, 2016, 11:04 p.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39584/
> -----------------------------------------------------------
>
> (Updated Jan. 5, 2016, 11:04 p.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Implemented `os::rmdir.hpp`.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/Makefile.am b2dea9b93adfa3ea0415a0b5c81a369dd29b6cfe
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 14fbca6d222bdfc0e8be301050b4ea1a8a6e7758
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 4cf693fb7e8c6bb3ad1920ebe90d61f0adb5dc99
> 3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp e738cdbf5846950c475c159fb9a770acc45159f5
>
> Diff: https://reviews.apache.org/r/39584/diff/
>
>
> Testing
> -------
>
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39584/
-----------------------------------------------------------
(Updated Jan. 5, 2016, 11:04 p.m.)
Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
Repository: mesos
Description
-------
Windows: Implemented `os::rmdir.hpp`.
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/stout/include/Makefile.am b2dea9b93adfa3ea0415a0b5c81a369dd29b6cfe
3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 14fbca6d222bdfc0e8be301050b4ea1a8a6e7758
3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 4cf693fb7e8c6bb3ad1920ebe90d61f0adb5dc99
3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp e738cdbf5846950c475c159fb9a770acc45159f5
Diff: https://reviews.apache.org/r/39584/diff/
Testing
-------
`make check` from autotools on Ubuntu 15.
`make check` from CMake on OS X 10.10.
Ran `check` project in VS on Windows 10.
Thanks,
Alex Clemmer
Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.
Posted by Yi Sun <yi...@hotmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39584/#review112717
-----------------------------------------------------------
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (line 54)
<https://reviews.apache.org/r/39584/#comment173203>
the current function is not os::rmdir.
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (line 64)
<https://reviews.apache.org/r/39584/#comment173202>
What does "Previous" mean here? Should it be "Parent"?
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (line 87)
<https://reviews.apache.org/r/39584/#comment173204>
os::rmdir is not the right func name. Also, "found it was not a valid directory" does not make sense because we only hit here when it's not a directory.
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (line 96)
<https://reviews.apache.org/r/39584/#comment173205>
os::rmdir
- Yi Sun
On Jan. 4, 2016, 11:26 p.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39584/
> -----------------------------------------------------------
>
> (Updated Jan. 4, 2016, 11:26 p.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Implemented `os::rmdir.hpp`.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/Makefile.am b2dea9b93adfa3ea0415a0b5c81a369dd29b6cfe
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 14fbca6d222bdfc0e8be301050b4ea1a8a6e7758
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 4cf693fb7e8c6bb3ad1920ebe90d61f0adb5dc99
> 3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp e738cdbf5846950c475c159fb9a770acc45159f5
>
> Diff: https://reviews.apache.org/r/39584/diff/
>
>
> Testing
> -------
>
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39584/
-----------------------------------------------------------
(Updated Jan. 4, 2016, 11:26 p.m.)
Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
Repository: mesos
Description
-------
Windows: Implemented `os::rmdir.hpp`.
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/stout/include/Makefile.am b2dea9b93adfa3ea0415a0b5c81a369dd29b6cfe
3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 14fbca6d222bdfc0e8be301050b4ea1a8a6e7758
3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 4cf693fb7e8c6bb3ad1920ebe90d61f0adb5dc99
3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp e738cdbf5846950c475c159fb9a770acc45159f5
Diff: https://reviews.apache.org/r/39584/diff/
Testing
-------
`make check` from autotools on Ubuntu 15.
`make check` from CMake on OS X 10.10.
Ran `check` project in VS on Windows 10.
Thanks,
Alex Clemmer
Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.
Posted by Alex Clemmer <cl...@gmail.com>.
> On Jan. 4, 2016, 10:33 p.m., Daniel Pravat wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp, line 94
> > <https://reviews.apache.org/r/39584/diff/9/?file=1180823#file1180823line94>
> >
> > Can you use _rmdir() instead to avoid another deprecation warning ?
Just a brief comment here: in windows.hpp there are aliases mapping POSIX functions to their underscore versions (_e.g._, `open` -> `_open`), but this is for compatibility in files where POSIX and Windows implementations are the same, so that we don't have to fork the implementation just because the names are slightly different.
BUT, now that you point it out, I totally agree that we should call the actual function `_rmdir` instead when the file is Windows-specific.
- Alex
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39584/#review112667
-----------------------------------------------------------
On Jan. 4, 2016, 11:26 p.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39584/
> -----------------------------------------------------------
>
> (Updated Jan. 4, 2016, 11:26 p.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Implemented `os::rmdir.hpp`.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/Makefile.am b2dea9b93adfa3ea0415a0b5c81a369dd29b6cfe
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 14fbca6d222bdfc0e8be301050b4ea1a8a6e7758
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 4cf693fb7e8c6bb3ad1920ebe90d61f0adb5dc99
> 3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp e738cdbf5846950c475c159fb9a770acc45159f5
>
> Diff: https://reviews.apache.org/r/39584/diff/
>
>
> Testing
> -------
>
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.
Posted by Daniel Pravat <dp...@outlook.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39584/#review112667
-----------------------------------------------------------
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (line 94)
<https://reviews.apache.org/r/39584/#comment173176>
Can you use _rmdir() instead to avoid another deprecation warning ?
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (line 127)
<https://reviews.apache.org/r/39584/#comment173178>
Can you use _rmdir() instead to avoid another deprecation warning ?
- Daniel Pravat
On Jan. 4, 2016, 12:02 p.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39584/
> -----------------------------------------------------------
>
> (Updated Jan. 4, 2016, 12:02 p.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Implemented `os::rmdir.hpp`.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/Makefile.am b2dea9b93adfa3ea0415a0b5c81a369dd29b6cfe
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 14fbca6d222bdfc0e8be301050b4ea1a8a6e7758
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 4cf693fb7e8c6bb3ad1920ebe90d61f0adb5dc99
> 3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp e738cdbf5846950c475c159fb9a770acc45159f5
>
> Diff: https://reviews.apache.org/r/39584/diff/
>
>
> Testing
> -------
>
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.
Posted by Alex Naparu <al...@outlook.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39584/#review112618
-----------------------------------------------------------
Ship it!
Ship It!
- Alex Naparu
On Jan. 4, 2016, 12:02 p.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39584/
> -----------------------------------------------------------
>
> (Updated Jan. 4, 2016, 12:02 p.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Implemented `os::rmdir.hpp`.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/Makefile.am b2dea9b93adfa3ea0415a0b5c81a369dd29b6cfe
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 14fbca6d222bdfc0e8be301050b4ea1a8a6e7758
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 4cf693fb7e8c6bb3ad1920ebe90d61f0adb5dc99
> 3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp e738cdbf5846950c475c159fb9a770acc45159f5
>
> Diff: https://reviews.apache.org/r/39584/diff/
>
>
> Testing
> -------
>
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39584/
-----------------------------------------------------------
(Updated Jan. 4, 2016, 12:02 p.m.)
Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
Repository: mesos
Description
-------
Windows: Implemented `os::rmdir.hpp`.
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/stout/include/Makefile.am b2dea9b93adfa3ea0415a0b5c81a369dd29b6cfe
3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 14fbca6d222bdfc0e8be301050b4ea1a8a6e7758
3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 4cf693fb7e8c6bb3ad1920ebe90d61f0adb5dc99
3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp e738cdbf5846950c475c159fb9a770acc45159f5
Diff: https://reviews.apache.org/r/39584/diff/
Testing
-------
`make check` from autotools on Ubuntu 15.
`make check` from CMake on OS X 10.10.
Ran `check` project in VS on Windows 10.
Thanks,
Alex Clemmer
Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39584/
-----------------------------------------------------------
(Updated Jan. 4, 2016, 11:59 a.m.)
Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
Repository: mesos
Description
-------
Windows: Implemented `os::rmdir.hpp`.
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/stout/include/Makefile.am b2dea9b93adfa3ea0415a0b5c81a369dd29b6cfe
3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 14fbca6d222bdfc0e8be301050b4ea1a8a6e7758
3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 4cf693fb7e8c6bb3ad1920ebe90d61f0adb5dc99
3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp e738cdbf5846950c475c159fb9a770acc45159f5
Diff: https://reviews.apache.org/r/39584/diff/
Testing
-------
`make check` from autotools on Ubuntu 15.
`make check` from CMake on OS X 10.10.
Ran `check` project in VS on Windows 10.
Thanks,
Alex Clemmer
Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39584/
-----------------------------------------------------------
(Updated Jan. 4, 2016, 11:18 a.m.)
Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
Repository: mesos
Description
-------
Windows: Implemented `os::rmdir.hpp`.
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/posix/rmdir.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.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/39584/diff/
Testing
-------
`make check` from autotools on Ubuntu 15.
`make check` from CMake on OS X 10.10.
Ran `check` project in VS on Windows 10.
Thanks,
Alex Clemmer
Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.
Posted by Alex Clemmer <cl...@gmail.com>.
> On Dec. 10, 2015, 4:12 a.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp, line 109
> > <https://reviews.apache.org/r/39584/diff/7/?file=1125983#file1125983line109>
> >
> > Can you explain / comment why here (as well as on L139) we use `ErrnoError`, yet elsewhere we use `WindowsError`?
`errno` captures errors that have happened in the C standard library, and `ErrnoError` captures those errors and their messages and lifts them into an `Error` type.
For Windows API functions (which, of course, are not a part of the C standard library) have their own `errno`-like error reporting mechanism, called `GetLastError()`. `WindowsError` captures these errors and lifts them into an `Error` type.
Thus, when we call Windows API functions, we capture errors with `WindowsError`, and when we call C standard library functions, we capture errors with `ErrnoError`.
Does that make sense?
- Alex
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39584/#review109681
-----------------------------------------------------------
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/39584/
> -----------------------------------------------------------
>
> (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::rmdir.hpp`.
>
>
> 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/posix/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.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/39584/diff/
>
>
> Testing
> -------
>
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.
Posted by Alex Clemmer <cl...@gmail.com>.
> On Dec. 10, 2015, 4:12 a.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp, line 109
> > <https://reviews.apache.org/r/39584/diff/7/?file=1125983#file1125983line109>
> >
> > Can you explain / comment why here (as well as on L139) we use `ErrnoError`, yet elsewhere we use `WindowsError`?
>
> Alex Clemmer wrote:
> `errno` captures errors that have happened in the C standard library, and `ErrnoError` captures those errors and their messages and lifts them into an `Error` type.
>
> For Windows API functions (which, of course, are not a part of the C standard library) have their own `errno`-like error reporting mechanism, called `GetLastError()`. `WindowsError` captures these errors and lifts them into an `Error` type.
>
> Thus, when we call Windows API functions, we capture errors with `WindowsError`, and when we call C standard library functions, we capture errors with `ErrnoError`.
>
> Does that make sense?
As far as comments go, this is commented in the `WindowsError` code itself. It seems impractical to add this comment at every call site, but maybe you meant something else?
- Alex
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39584/#review109681
-----------------------------------------------------------
On Jan. 4, 2016, 12:02 p.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39584/
> -----------------------------------------------------------
>
> (Updated Jan. 4, 2016, 12:02 p.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Implemented `os::rmdir.hpp`.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/Makefile.am b2dea9b93adfa3ea0415a0b5c81a369dd29b6cfe
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 14fbca6d222bdfc0e8be301050b4ea1a8a6e7758
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 4cf693fb7e8c6bb3ad1920ebe90d61f0adb5dc99
> 3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp e738cdbf5846950c475c159fb9a770acc45159f5
>
> Diff: https://reviews.apache.org/r/39584/diff/
>
>
> Testing
> -------
>
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.
Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39584/#review109681
-----------------------------------------------------------
3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp (lines 1 - 13)
<https://reviews.apache.org/r/39584/#comment169276>
header format change.
3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp (line 18)
<https://reviews.apache.org/r/39584/#comment169278>
`#include <unistd.h>`
`#include <string>`
`#include <stout/error.hpp>`
3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp (lines 1 - 13)
<https://reviews.apache.org/r/39584/#comment169275>
header format change.
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (lines 1 - 13)
<https://reviews.apache.org/r/39584/#comment169277>
header format change.
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (lines 53 - 54)
<https://reviews.apache.org/r/39584/#comment169279>
indentation.
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (line 58)
<https://reviews.apache.org/r/39584/#comment169280>
`because folder guaranteed`
can you fix the grammar here?
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (lines 79 - 80)
<https://reviews.apache.org/r/39584/#comment169281>
indentation
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (lines 90 - 91)
<https://reviews.apache.org/r/39584/#comment169282>
indentation.
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (lines 93 - 94)
<https://reviews.apache.org/r/39584/#comment169283>
let's keep the else on the same line as the previous closing brace.
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (lines 100 - 101)
<https://reviews.apache.org/r/39584/#comment169284>
indentation.
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (line 109)
<https://reviews.apache.org/r/39584/#comment169286>
Can you explain / comment why here (as well as on L139) we use `ErrnoError`, yet elsewhere we use `WindowsError`?
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (lines 110 - 111)
<https://reviews.apache.org/r/39584/#comment169285>
indentation.
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (lines 133 - 134)
<https://reviews.apache.org/r/39584/#comment169287>
indentation.
- 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/39584/
> -----------------------------------------------------------
>
> (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::rmdir.hpp`.
>
>
> 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/posix/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.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/39584/diff/
>
>
> Testing
> -------
>
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.
Posted by Alex Clemmer <cl...@gmail.com>.
> On Dec. 15, 2015, 3:54 a.m., Alex Naparu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp, line 39
> > <https://reviews.apache.org/r/39584/diff/7/?file=1125983#file1125983line39>
> >
> > What if the path ends with multiple '\' chars to begin with?
Good question. In general, our externally-facing API (like `os::rmdir`) will take any path and normalize that before processing, but our internal functions assume that paths are already normalized. Practically speaking, this simplifies our code quite a bit, and it's an easy rule to remember.
In this case, `os::realpath` will strip the trailing extraneous slashes from the filename (I've verified this), and since `os::rmdir` preforms this normalization before `recursiveRemoveDirectory` is called, I believe this code is correct.
So the short answer is, "we should never encounter such a situation", assuming there is no normalized path with multiple
> On Dec. 15, 2015, 3:54 a.m., Alex Naparu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp, line 47
> > <https://reviews.apache.org/r/39584/diff/7/?file=1125983#file1125983line47>
> >
> > What happens if the input path already contains a wildcard? Might be a good idea to check that "path" is a directory before moving forward. Also might be a good idea to use realpath here.
The short answer is that paths end with a star yield correct behavior -- _i.e._, it will treat the path ending in a wildcard as an absolute path, and since that path doesn't exist, it will then return an error. So, a call to `isdir` is not necessary here, unless I'm missing something else.
The longer answer is: we assume the path passed into the function is a directory, and append either a `*` or a `*` to the end of the path (depending on whether it already has a trailing `\ `), and ask `FindFirstFile` to find the first file that matches that pattern. Since this will result in a path like `C:\example**`, clearly this is not a directory, and the Windows API it will fail to find any directory entries matching the pattern, returning an error.
For the normalization issue, as noted above, we do assume that the path is normalized when passed into this function.
> On Dec. 15, 2015, 3:54 a.m., Alex Naparu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp, line 62
> > <https://reviews.apache.org/r/39584/diff/7/?file=1125983#file1125983line62>
> >
> > Nit: tou're not reusing these, so might as well inline the callse.
I'd like to keep these actually, unless you really are opposed to them. I chose them because they make the code self-documenting.
> On Dec. 15, 2015, 3:54 a.m., Alex Naparu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp, line 73
> > <https://reviews.apache.org/r/39584/diff/7/?file=1125983#file1125983line73>
> >
> > Attributes should already be available in WIN32_FIND_DATA (see https://msdn.microsoft.com/en-us/library/windows/desktop/aa365740(v=vs.85).aspx), you might not need to call GetFileAttribiutes here. If you do need to get the attributes, consider using CreateFile and GetFileInformationByHandle, which will always give the up-to-date information even on NTFS (see https://msdn.microsoft.com/en-us/library/windows/desktop/aa364952(v=vs.85).aspx for details)
This code can probably actually transition to `stat::isdir`; it wasn't written to use that code because I hadn't written that function yet. Now that I have, we can reverse the order of these commits, and simply use that function.
> On Dec. 15, 2015, 3:54 a.m., Alex Naparu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp, line 77
> > <https://reviews.apache.org/r/39584/diff/7/?file=1125983#file1125983line77>
> >
> > Consider using a smart pointer for closing the handle.
Per our conversation some months ago, we'll transition to a new type, `shared_handle`, which is a typedef to `shared_ptr<void>`. This looks the same but avoids returning pointer to void everywhere, which is gross.
> On Dec. 15, 2015, 3:54 a.m., Alex Naparu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp, line 97
> > <https://reviews.apache.org/r/39584/diff/7/?file=1125983#file1125983line97>
> >
> > Failure of ::remove alone might not be reason enough for aborting the entire call. For instance, if ::remove fails with "file not found", we're still good to go.
I believe the POSIX spec actually says that you should stop after you fail to delete one of the things you tried to delete. So I think that we should keep this behavior, unless you think I missed something else?
> On Dec. 15, 2015, 3:54 a.m., Alex Naparu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp, line 138
> > <https://reviews.apache.org/r/39584/diff/7/?file=1125983#file1125983line138>
> >
> > Consider using ::RemoveDirectory here, which will delete the directory when the last handle is closed. Unless that's not the desired behavior...
I believe `rmdir` does the same thing, actually. In fact, I remember being unaware of this fact until I spent a coule hours trying to debug why things weren't deleting.
- Alex
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39584/#review110385
-----------------------------------------------------------
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/39584/
> -----------------------------------------------------------
>
> (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::rmdir.hpp`.
>
>
> 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/posix/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.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/39584/diff/
>
>
> Testing
> -------
>
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.
Posted by Alex Clemmer <cl...@gmail.com>.
> On Dec. 15, 2015, 3:54 a.m., Alex Naparu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp, line 51
> > <https://reviews.apache.org/r/39584/diff/7/?file=1125983#file1125983line51>
> >
> > Calling FindClose on an invalid handle is not the best idea. At best it will be a no-op.
>
> Alex Clemmer wrote:
> So, would you suggest checking if the handle is `INVALID_HANDLE_VALUE` and closing it only if that's not true?
I bring this up mainly because it seems like if we go the RAII route, we'll end up calling this anyway. So if it's a big deal, we need to beef up the shared_ptr dispose logic for all our shared handles, right?
> On Dec. 15, 2015, 3:54 a.m., Alex Naparu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp, lines 110-111
> > <https://reviews.apache.org/r/39584/diff/7/?file=1125983#file1125983line110>
> >
> > Nit: error message is the same as the one on line 100, which makes it hard to tell what happened, even for someone with access to the source code.
We don't really need to re-box the error on (what is now) line 81. So let's just remove that one.
- Alex
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39584/#review110385
-----------------------------------------------------------
On Jan. 4, 2016, 11:59 a.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39584/
> -----------------------------------------------------------
>
> (Updated Jan. 4, 2016, 11:59 a.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Implemented `os::rmdir.hpp`.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/Makefile.am b2dea9b93adfa3ea0415a0b5c81a369dd29b6cfe
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 14fbca6d222bdfc0e8be301050b4ea1a8a6e7758
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 4cf693fb7e8c6bb3ad1920ebe90d61f0adb5dc99
> 3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp e738cdbf5846950c475c159fb9a770acc45159f5
>
> Diff: https://reviews.apache.org/r/39584/diff/
>
>
> Testing
> -------
>
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.
Posted by Alex Clemmer <cl...@gmail.com>.
> On Dec. 15, 2015, 3:54 a.m., Alex Naparu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp, line 51
> > <https://reviews.apache.org/r/39584/diff/7/?file=1125983#file1125983line51>
> >
> > Calling FindClose on an invalid handle is not the best idea. At best it will be a no-op.
So, would you suggest checking if the handle is `INVALID_HANDLE_VALUE` and closing it only if that's not true?
- Alex
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39584/#review110385
-----------------------------------------------------------
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/39584/
> -----------------------------------------------------------
>
> (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::rmdir.hpp`.
>
>
> 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/posix/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.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/39584/diff/
>
>
> Testing
> -------
>
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.
Posted by Alex Clemmer <cl...@gmail.com>.
> On Dec. 15, 2015, 3:54 a.m., Alex Naparu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp, line 51
> > <https://reviews.apache.org/r/39584/diff/7/?file=1125983#file1125983line51>
> >
> > Calling FindClose on an invalid handle is not the best idea. At best it will be a no-op.
>
> Alex Clemmer wrote:
> So, would you suggest checking if the handle is `INVALID_HANDLE_VALUE` and closing it only if that's not true?
>
> Alex Clemmer wrote:
> I bring this up mainly because it seems like if we go the RAII route, we'll end up calling this anyway. So if it's a big deal, we need to beef up the shared_ptr dispose logic for all our shared handles, right?
Per our discussion on Slack, and our consulting of the documentation[1][2], and also inspection of the source code, we have concluded it is safe to call `*Close` on an invalid handle after all. Hence, let's close the comment as "drop".
[1] https://msdn.microsoft.com/en-us/library/windows/desktop/aa364413(v=vs.85).aspx
[2] https://msdn.microsoft.com/en-us/library/windows/desktop/ms724211(v=vs.85).aspx
- Alex
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39584/#review110385
-----------------------------------------------------------
On Jan. 7, 2016, 8:29 p.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39584/
> -----------------------------------------------------------
>
> (Updated Jan. 7, 2016, 8:29 p.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Implemented `os::rmdir.hpp`.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/Makefile.am b2dea9b93adfa3ea0415a0b5c81a369dd29b6cfe
> 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 14fbca6d222bdfc0e8be301050b4ea1a8a6e7758
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 4cf693fb7e8c6bb3ad1920ebe90d61f0adb5dc99
> 3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp e738cdbf5846950c475c159fb9a770acc45159f5
>
> Diff: https://reviews.apache.org/r/39584/diff/
>
>
> Testing
> -------
>
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.
Posted by Alex Naparu <al...@outlook.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39584/#review110385
-----------------------------------------------------------
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (line 39)
<https://reviews.apache.org/r/39584/#comment170194>
What if the path ends with multiple '\' chars to begin with?
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (line 47)
<https://reviews.apache.org/r/39584/#comment170206>
What happens if the input path already contains a wildcard? Might be a good idea to check that "path" is a directory before moving forward. Also might be a good idea to use realpath here.
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (line 51)
<https://reviews.apache.org/r/39584/#comment170192>
Calling FindClose on an invalid handle is not the best idea. At best it will be a no-op.
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (line 62)
<https://reviews.apache.org/r/39584/#comment170195>
Nit: tou're not reusing these, so might as well inline the callse.
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (line 73)
<https://reviews.apache.org/r/39584/#comment170196>
Attributes should already be available in WIN32_FIND_DATA (see https://msdn.microsoft.com/en-us/library/windows/desktop/aa365740(v=vs.85).aspx), you might not need to call GetFileAttribiutes here. If you do need to get the attributes, consider using CreateFile and GetFileInformationByHandle, which will always give the up-to-date information even on NTFS (see https://msdn.microsoft.com/en-us/library/windows/desktop/aa364952(v=vs.85).aspx for details)
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (line 77)
<https://reviews.apache.org/r/39584/#comment170197>
Consider using a smart pointer for closing the handle.
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (line 97)
<https://reviews.apache.org/r/39584/#comment170213>
Failure of ::remove alone might not be reason enough for aborting the entire call. For instance, if ::remove fails with "file not found", we're still good to go.
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (line 108)
<https://reviews.apache.org/r/39584/#comment170214>
Same comment as for line 108.
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (lines 110 - 111)
<https://reviews.apache.org/r/39584/#comment170199>
Nit: error message is the same as the one on line 100, which makes it hard to tell what happened, even for someone with access to the source code.
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (line 138)
<https://reviews.apache.org/r/39584/#comment170200>
Consider using ::RemoveDirectory here, which will delete the directory when the last handle is closed. Unless that's not the desired behavior...
- Alex Naparu
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/39584/
> -----------------------------------------------------------
>
> (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::rmdir.hpp`.
>
>
> 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/posix/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.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/39584/diff/
>
>
> Testing
> -------
>
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.
Posted by Alex Clemmer <cl...@gmail.com>.
> On Dec. 15, 2015, 3:55 a.m., Alex Naparu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp, line 62
> > <https://reviews.apache.org/r/39584/diff/7/?file=1125983#file1125983line62>
> >
> > Nit: You're not reusing these, so might as well inline the calls.
You mean, just call `currentFile.compare` inline? I actualyl purposefully expanded this to make it "self-documenting" (to the extent anything can be). I'd like to keep them unless you're really opposed to this.
- Alex
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39584/#review110391
-----------------------------------------------------------
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/39584/
> -----------------------------------------------------------
>
> (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::rmdir.hpp`.
>
>
> 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/posix/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.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/39584/diff/
>
>
> Testing
> -------
>
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.
Posted by Alex Naparu <al...@outlook.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39584/#review110391
-----------------------------------------------------------
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (line 62)
<https://reviews.apache.org/r/39584/#comment170217>
Nit: You're not reusing these, so might as well inline the calls.
- Alex Naparu
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/39584/
> -----------------------------------------------------------
>
> (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::rmdir.hpp`.
>
>
> 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/posix/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.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/39584/diff/
>
>
> Testing
> -------
>
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39584/#review111775
-----------------------------------------------------------
Bad patch!
Reviews applied: [39621, 39620, 39583, 39584]
Failed command: ./support/apply-review.sh -n -r 39584
Error:
2015-12-23 11:50:08 URL:https://reviews.apache.org/r/39584/diff/raw/ [14011/14011] -> "39584.patch" [1]
error: patch failed: 3rdparty/libprocess/3rdparty/stout/include/Makefile.am:95
error: 3rdparty/libprocess/3rdparty/stout/include/Makefile.am: patch does not apply
- Mesos ReviewBot
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/39584/
> -----------------------------------------------------------
>
> (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::rmdir.hpp`.
>
>
> 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/posix/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.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/39584/diff/
>
>
> Testing
> -------
>
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39584/
-----------------------------------------------------------
(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::rmdir.hpp`.
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/posix/rmdir.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.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/39584/diff/
Testing
-------
`make check` from autotools on Ubuntu 15.
`make check` from CMake on OS X 10.10.
Ran `check` project in VS on Windows 10.
Thanks,
Alex Clemmer
Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39584/
-----------------------------------------------------------
(Updated Oct. 30, 2015, 6:11 a.m.)
Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
Repository: mesos
Description
-------
Windows: Implemented `os::rmdir.hpp`.
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/posix/rmdir.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.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/39584/diff/
Testing
-------
`make check` from autotools on Ubuntu 15.
`make check` from CMake on OS X 10.10.
Ran `check` project in VS on Windows 10.
Thanks,
Alex Clemmer
Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39584/
-----------------------------------------------------------
(Updated Oct. 27, 2015, 8:33 a.m.)
Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
Repository: mesos
Description
-------
Windows: Implemented `os::rmdir.hpp`.
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/posix/rmdir.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.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/39584/diff/
Testing
-------
`make check` from autotools on Ubuntu 15.
`make check` from CMake on OS X 10.10.
Ran `check` project in VS on Windows 10.
Thanks,
Alex Clemmer
Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39584/
-----------------------------------------------------------
(Updated Oct. 27, 2015, 8:30 a.m.)
Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
Repository: mesos
Description
-------
Windows: Implemented `os::rmdir.hpp`.
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/posix/rmdir.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.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/39584/diff/
Testing
-------
`make check` from autotools on Ubuntu 15.
`make check` from CMake on OS X 10.10.
Ran `check` project in VS on Windows 10.
Thanks,
Alex Clemmer
Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39584/#review104156
-----------------------------------------------------------
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (line 138)
<https://reviews.apache.org/r/39584/#comment162433>
note that actually `::rmdir` here will deal appropriately with Unix-style paths, so there's no real need to normalize them.
- Alex Clemmer
On Oct. 27, 2015, 8:23 a.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39584/
> -----------------------------------------------------------
>
> (Updated Oct. 27, 2015, 8:23 a.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Implemented `os::rmdir.hpp`.
>
>
> 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/posix/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.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/39584/diff/
>
>
> Testing
> -------
>
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39584/
-----------------------------------------------------------
(Updated Oct. 27, 2015, 8:23 a.m.)
Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
Repository: mesos
Description
-------
Windows: Implemented `os::rmdir.hpp`.
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/posix/rmdir.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.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/39584/diff/
Testing
-------
`make check` from autotools on Ubuntu 15.
`make check` from CMake on OS X 10.10.
Ran `check` project in VS on Windows 10.
Thanks,
Alex Clemmer
Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.
Posted by Joseph Wu <jo...@mesosphere.io>.
> On Oct. 26, 2015, 1:43 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp, lines 35-40
> > <https://reviews.apache.org/r/39584/diff/2/?file=1105044#file1105044line35>
> >
> > Use `strings::endsWith`.
> >
> > And you might also want to use a ternary conditional:
> > `currentPath = path + (strings::endsWith("\") ? "" : "\");`
>
> Alex Clemmer wrote:
> The `strings::endsWith` suggestion is really good! I actually would like to avoid the choice of the ternary operator if you don't mind. Even though I used it in a recent review in general my personal opinion is that I tend to find it too opaque and not much more readble.
That's fine too. I just personally like using ternary for short statements like this. (Marking as fixed.)
> On Oct. 26, 2015, 1:43 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp, line 42
> > <https://reviews.apache.org/r/39584/diff/2/?file=1105044#file1105044line42>
> >
> > `X:\blah*` isn't clear. Did you mean `currentPath` + `*`?
>
> Alex Clemmer wrote:
> I actually specifically avoided mentioning the variable name because I thought it was not clear what the relationship of the Kleene star was to the variable. I've expanded the path to something more clear. What do you think of this solution? Is it more clear? Is it worse?
I think it's better.
You could also say that the "X" is an arbitrary drive letter. I've seen "Z" drives before, so I'm guessing "X" drives aren't unheard of.
- Joseph
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39584/#review104054
-----------------------------------------------------------
On Oct. 27, 2015, 1:33 a.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39584/
> -----------------------------------------------------------
>
> (Updated Oct. 27, 2015, 1:33 a.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Implemented `os::rmdir.hpp`.
>
>
> 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/posix/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.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/39584/diff/
>
>
> Testing
> -------
>
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.
Posted by Alex Clemmer <cl...@gmail.com>.
> On Oct. 26, 2015, 8:43 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp, line 42
> > <https://reviews.apache.org/r/39584/diff/2/?file=1105044#file1105044line42>
> >
> > `X:\blah*` isn't clear. Did you mean `currentPath` + `*`?
I actually specifically avoided mentioning the variable name because I thought it was not clear what the relationship of the Kleene star was to the variable. I've expanded the path to something more clear. What do you think of this solution? Is it more clear? Is it worse?
> On Oct. 26, 2015, 8:43 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp, lines 35-40
> > <https://reviews.apache.org/r/39584/diff/2/?file=1105044#file1105044line35>
> >
> > Use `strings::endsWith`.
> >
> > And you might also want to use a ternary conditional:
> > `currentPath = path + (strings::endsWith("\") ? "" : "\");`
The `strings::endsWith` suggestion is really good! I actually would like to avoid the choice of the ternary operator if you don't mind. Even though I used it in a recent review in general my personal opinion is that I tend to find it too opaque and not much more readble.
> On Oct. 26, 2015, 8:43 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp, line 134
> > <https://reviews.apache.org/r/39584/diff/2/?file=1105044#file1105044line134>
> >
> > You seem to be missing an `if (recursive)` here...
Wow, major +1 awesome catch I meant to go back and add this but totally spaced it. Great job!
- Alex
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39584/#review104054
-----------------------------------------------------------
On Oct. 27, 2015, 8:23 a.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39584/
> -----------------------------------------------------------
>
> (Updated Oct. 27, 2015, 8:23 a.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Implemented `os::rmdir.hpp`.
>
>
> 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/posix/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.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/39584/diff/
>
>
> Testing
> -------
>
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39584/#review104054
-----------------------------------------------------------
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (lines 35 - 40)
<https://reviews.apache.org/r/39584/#comment162258>
Use `strings::endsWith`.
And you might also want to use a ternary conditional:
`currentPath = path + (strings::endsWith("\") ? "" : "\");`
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (line 42)
<https://reviews.apache.org/r/39584/#comment162259>
`X:\blah*` isn't clear. Did you mean `currentPath` + `*`?
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (line 51)
<https://reviews.apache.org/r/39584/#comment162261>
Nit: Errors don't end with periods. Ditto for the other error messages.
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (line 134)
<https://reviews.apache.org/r/39584/#comment162265>
You seem to be missing an `if (recursive)` here...
- Joseph Wu
On Oct. 23, 2015, 9:30 a.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39584/
> -----------------------------------------------------------
>
> (Updated Oct. 23, 2015, 9:30 a.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Implemented `os::rmdir.hpp`.
>
>
> 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/posix/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.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/39584/diff/
>
>
> Testing
> -------
>
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39584/
-----------------------------------------------------------
(Updated Oct. 23, 2015, 4:30 p.m.)
Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
Repository: mesos
Description
-------
Windows: Implemented `os::rmdir.hpp`.
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/posix/rmdir.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.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/39584/diff/
Testing
-------
`make check` from autotools on Ubuntu 15.
`make check` from CMake on OS X 10.10.
Ran `check` project in VS on Windows 10.
Thanks,
Alex Clemmer
Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39584/#review103749
-----------------------------------------------------------
Bad patch!
Reviews applied: [39537, 39538, 39539, 39540, 39541, 39383, 39559, 39219, 39560, 39583, 39584]
Failed command: ./support/apply-review.sh -n -r 39584
Error:
2015-10-23 09:51:38 URL:https://reviews.apache.org/r/39584/diff/raw/ [13792/13792] -> "39584.patch" [1]
Successfully applied: Windows: Implemented `os::rmdir.hpp`.
Windows: Implemented `os::rmdir.hpp`.
Review: https://reviews.apache.org/r/39584
Checking 6 files using filter --filter=-,+build/class,+build/deprecated,+build/endif_comment,+readability/todo,+readability/namespace,+runtime/vlog,+whitespace/blank_line,+whitespace/comma,+whitespace/end_of_line,+whitespace/ending_newline,+whitespace/forcolon,+whitespace/indent,+whitespace/line_length,+whitespace/operators,+whitespace/semicolon,+whitespace/tab,+whitespace/todo
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp:28: Lines should very rarely be longer than 100 characters [whitespace/line_length] [4]
Total errors found: 1
Failed to commit patch
- Mesos ReviewBot
On Oct. 23, 2015, 8:57 a.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39584/
> -----------------------------------------------------------
>
> (Updated Oct. 23, 2015, 8:57 a.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Implemented `os::rmdir.hpp`.
>
>
> 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/posix/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.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/39584/diff/
>
>
> Testing
> -------
>
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
>
>
> Thanks,
>
> Alex Clemmer
>
>