You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jeff Coffler <je...@taltos.com> on 2017/10/11 23:32:41 UTC
Re: Review Request 60628: Enabled fetcher_tests.cpp unit test module
on Windows platform.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60628/
-----------------------------------------------------------
(Updated Oct. 11, 2017, 11:32 p.m.)
Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.
Summary (updated)
-----------------
Enabled fetcher_tests.cpp unit test module on Windows platform.
Bugs: MESOS-6705
https://issues.apache.org/jira/browse/MESOS-6705
Repository: mesos
Description (updated)
-------
Tests for tar, gzip, and such won't be working on Windows for
the time being. Thoughts are to provide this capability to the
Fetcher in a cross-platform manner via a programmatic code library
rather than Linux-specific command line tools (tar, gzip, etc).
See MESOS-8064 for more details.
In the short term, zip and unzip will work since PowerShell can
support that natively.
Diffs (updated)
-----
src/launcher/CMakeLists.txt c7a83d476efe13d65fa529e7676b6488eb48f44b
src/launcher/fetcher.cpp 5a9e93adbf2e4f0b8ff64a69e62fea5f6a0b8f2d
src/slave/containerizer/fetcher.cpp ba5b0979aa427c5b3dbacf39661d6027da13e0d6
src/tests/CMakeLists.txt 386e0473c93d0a993248c7818067071d0c761c76
src/tests/fetcher_tests.cpp df9d2d9586a6457004506c4e2a972ccfc912c7c5
Diff: https://reviews.apache.org/r/60628/diff/3/
Changes: https://reviews.apache.org/r/60628/diff/2-3/
Testing
-------
See upstream.
Thanks,
Jeff Coffler
Re: Review Request 60628: Enabled fetcher_tests.cpp unit test module
on Windows platform.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60628/#review187735
-----------------------------------------------------------
FAIL: Failed to apply the dependent review: 60292.
Failed command: `python.exe .\support\apply-reviews.py -n -r 60292`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/60628
Relevant logs:
- [apply-review-60292-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/60628/logs/apply-review-60292-stdout.log):
```
error: patch failed: 3rdparty/stout/include/stout/stringify.hpp:57
error: 3rdparty/stout/include/stout/stringify.hpp: patch does not apply
```
- Mesos Reviewbot Windows
On Oct. 11, 2017, 11:32 p.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60628/
> -----------------------------------------------------------
>
> (Updated Oct. 11, 2017, 11:32 p.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.
>
>
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Tests for tar, gzip, and such won't be working on Windows for
> the time being. Thoughts are to provide this capability to the
> Fetcher in a cross-platform manner via a programmatic code library
> rather than Linux-specific command line tools (tar, gzip, etc).
> See MESOS-8064 for more details.
>
> In the short term, zip and unzip will work since PowerShell can
> support that natively.
>
>
> Diffs
> -----
>
> src/launcher/CMakeLists.txt c7a83d476efe13d65fa529e7676b6488eb48f44b
> src/launcher/fetcher.cpp 5a9e93adbf2e4f0b8ff64a69e62fea5f6a0b8f2d
> src/slave/containerizer/fetcher.cpp ba5b0979aa427c5b3dbacf39661d6027da13e0d6
> src/tests/CMakeLists.txt 386e0473c93d0a993248c7818067071d0c761c76
> src/tests/fetcher_tests.cpp df9d2d9586a6457004506c4e2a972ccfc912c7c5
>
>
> Diff: https://reviews.apache.org/r/60628/diff/3/
>
>
> Testing
> -------
>
> See upstream.
>
>
> Thanks,
>
> Jeff Coffler
>
>
Re: Review Request 60628: Enabled fetcher_tests.cpp unit test module
on Windows platform.
Posted by Jeff Coffler <je...@taltos.com>.
> On Oct. 13, 2017, 7:28 p.m., Andrew Schwartzmeyer wrote:
> > src/slave/containerizer/fetcher.cpp
> > Lines 202-213 (original), 202-210 (patched)
> > <https://reviews.apache.org/r/60628/diff/3/?file=1852896#file1852896line202>
> >
> > This logic should be refactored into `Try<string> path::from_uri`.
Refactored into std::string path::from_uri (since that function can't return an error per our discussion). Today, URIs and paths are used interchangably within MESOS, not not prepending a "file://" URI prefix will not trigger an error.
- Jeff
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60628/#review188003
-----------------------------------------------------------
On Oct. 17, 2017, 1:19 a.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60628/
> -----------------------------------------------------------
>
> (Updated Oct. 17, 2017, 1:19 a.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.
>
>
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Tests for tar, gzip, and such won't be working on Windows for
> the time being. Thoughts are to provide this capability to the
> Fetcher in a cross-platform manner via a programmatic code library
> rather than Linux-specific command line tools (tar, gzip, etc).
> See MESOS-8064 for more details.
>
> In the short term, zip and unzip will work since PowerShell can
> support that natively.
>
>
> Diffs
> -----
>
> src/launcher/CMakeLists.txt c7a83d476efe13d65fa529e7676b6488eb48f44b
> src/launcher/fetcher.cpp 5a9e93adbf2e4f0b8ff64a69e62fea5f6a0b8f2d
> src/slave/containerizer/fetcher.cpp ba5b0979aa427c5b3dbacf39661d6027da13e0d6
> src/tests/CMakeLists.txt 386e0473c93d0a993248c7818067071d0c761c76
> src/tests/fetcher_tests.cpp df9d2d9586a6457004506c4e2a972ccfc912c7c5
>
>
> Diff: https://reviews.apache.org/r/60628/diff/4/
>
>
> Testing
> -------
>
> See upstream.
>
>
> Thanks,
>
> Jeff Coffler
>
>
Re: Review Request 60628: Enabled fetcher_tests.cpp unit test module
on Windows platform.
Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
> On Oct. 13, 2017, 12:28 p.m., Andrew Schwartzmeyer wrote:
> > src/tests/fetcher_tests.cpp
> > Line 483 (original), 487 (patched)
> > <https://reviews.apache.org/r/60628/diff/3/?file=1852898#file1852898line487>
> >
> > Was this URL using backslashes for some reason? This doesn't appear to be necessary.
Found it: the root of the problem was the use of `path::join(http.process->self().id, "test"));` in the constrution of the URL. The `path` class shouldn't be used for URLs; it really should just be `http.process->self().id + "/test"` (well, perhaps we need a `url::join` to handle adding one and only one `/`).
But anyway, `path` is for _filesystem_ paths, not URLs. If the same problem is the cause of the other uses of `partial_path`, then we've identified how to get rid of it.
- Andrew
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60628/#review188003
-----------------------------------------------------------
On Oct. 11, 2017, 4:32 p.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60628/
> -----------------------------------------------------------
>
> (Updated Oct. 11, 2017, 4:32 p.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.
>
>
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Tests for tar, gzip, and such won't be working on Windows for
> the time being. Thoughts are to provide this capability to the
> Fetcher in a cross-platform manner via a programmatic code library
> rather than Linux-specific command line tools (tar, gzip, etc).
> See MESOS-8064 for more details.
>
> In the short term, zip and unzip will work since PowerShell can
> support that natively.
>
>
> Diffs
> -----
>
> src/launcher/CMakeLists.txt c7a83d476efe13d65fa529e7676b6488eb48f44b
> src/launcher/fetcher.cpp 5a9e93adbf2e4f0b8ff64a69e62fea5f6a0b8f2d
> src/slave/containerizer/fetcher.cpp ba5b0979aa427c5b3dbacf39661d6027da13e0d6
> src/tests/CMakeLists.txt 386e0473c93d0a993248c7818067071d0c761c76
> src/tests/fetcher_tests.cpp df9d2d9586a6457004506c4e2a972ccfc912c7c5
>
>
> Diff: https://reviews.apache.org/r/60628/diff/3/
>
>
> Testing
> -------
>
> See upstream.
>
>
> Thanks,
>
> Jeff Coffler
>
>
Re: Review Request 60628: Enabled fetcher_tests.cpp unit test module
on Windows platform.
Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60628/#review188003
-----------------------------------------------------------
As far as I can tell, this review is enabling the `mesos-fetcher` itself, not just the unit tests. The title is misleading.
src/launcher/fetcher.cpp
Line 293 (original), 291 (patched)
<https://reviews.apache.org/r/60628/#comment265064>
We should replace `path::normalize` with `path::from_uri`, which fixes the forward slashes from a URI into backslashes, and verifies that it is indeed a file URI (i.e. starts with `file://`).
The reason is that we are not performing normalization. We are instead converting to (and here, from) a file URI.
src/slave/containerizer/fetcher.cpp
Lines 202-213 (original), 202-210 (patched)
<https://reviews.apache.org/r/60628/#comment265065>
This logic should be refactored into `Try<string> path::from_uri`.
src/slave/containerizer/fetcher.cpp
Line 228 (original), 224 (patched)
<https://reviews.apache.org/r/60628/#comment265066>
Doing so would then also eliminate this.
src/tests/fetcher_tests.cpp
Line 188 (original), 189 (patched)
<https://reviews.apache.org/r/60628/#comment265067>
This probably isn't necessary, as `output_file` on `CommandInfo::URI` is just a relative path, not a URI.
src/tests/fetcher_tests.cpp
Line 483 (original), 487 (patched)
<https://reviews.apache.org/r/60628/#comment265068>
Was this URL using backslashes for some reason? This doesn't appear to be necessary.
src/tests/fetcher_tests.cpp
Line 531 (original), 535 (patched)
<https://reviews.apache.org/r/60628/#comment265069>
Ditto. We should look at this closer.
- Andrew Schwartzmeyer
On Oct. 11, 2017, 4:32 p.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60628/
> -----------------------------------------------------------
>
> (Updated Oct. 11, 2017, 4:32 p.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.
>
>
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Tests for tar, gzip, and such won't be working on Windows for
> the time being. Thoughts are to provide this capability to the
> Fetcher in a cross-platform manner via a programmatic code library
> rather than Linux-specific command line tools (tar, gzip, etc).
> See MESOS-8064 for more details.
>
> In the short term, zip and unzip will work since PowerShell can
> support that natively.
>
>
> Diffs
> -----
>
> src/launcher/CMakeLists.txt c7a83d476efe13d65fa529e7676b6488eb48f44b
> src/launcher/fetcher.cpp 5a9e93adbf2e4f0b8ff64a69e62fea5f6a0b8f2d
> src/slave/containerizer/fetcher.cpp ba5b0979aa427c5b3dbacf39661d6027da13e0d6
> src/tests/CMakeLists.txt 386e0473c93d0a993248c7818067071d0c761c76
> src/tests/fetcher_tests.cpp df9d2d9586a6457004506c4e2a972ccfc912c7c5
>
>
> Diff: https://reviews.apache.org/r/60628/diff/3/
>
>
> Testing
> -------
>
> See upstream.
>
>
> Thanks,
>
> Jeff Coffler
>
>
Re: Review Request 60628: Enabled fetcher_tests.cpp unit test module
on Windows platform.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60628/#review188260
-----------------------------------------------------------
FAIL: Failed to apply the dependent review: 60292.
Failed command: `python.exe .\support\apply-reviews.py -n -r 60292`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/60628
Relevant logs:
- [apply-review-60292-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/60628/logs/apply-review-60292-stdout.log):
```
error: patch failed: 3rdparty/stout/include/stout/stringify.hpp:57
error: 3rdparty/stout/include/stout/stringify.hpp: patch does not apply
```
- Mesos Reviewbot Windows
On Oct. 17, 2017, 1:19 a.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60628/
> -----------------------------------------------------------
>
> (Updated Oct. 17, 2017, 1:19 a.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.
>
>
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Tests for tar, gzip, and such won't be working on Windows for
> the time being. Thoughts are to provide this capability to the
> Fetcher in a cross-platform manner via a programmatic code library
> rather than Linux-specific command line tools (tar, gzip, etc).
> See MESOS-8064 for more details.
>
> In the short term, zip and unzip will work since PowerShell can
> support that natively.
>
>
> Diffs
> -----
>
> src/launcher/CMakeLists.txt c7a83d476efe13d65fa529e7676b6488eb48f44b
> src/launcher/fetcher.cpp 5a9e93adbf2e4f0b8ff64a69e62fea5f6a0b8f2d
> src/slave/containerizer/fetcher.cpp ba5b0979aa427c5b3dbacf39661d6027da13e0d6
> src/tests/CMakeLists.txt 386e0473c93d0a993248c7818067071d0c761c76
> src/tests/fetcher_tests.cpp df9d2d9586a6457004506c4e2a972ccfc912c7c5
>
>
> Diff: https://reviews.apache.org/r/60628/diff/4/
>
>
> Testing
> -------
>
> See upstream.
>
>
> Thanks,
>
> Jeff Coffler
>
>
Re: Review Request 60628: Enabled fetcher_tests.cpp unit test module
on Windows platform.
Posted by Jeff Coffler <je...@taltos.com>.
> On Oct. 18, 2017, 1:36 a.m., Andrew Schwartzmeyer wrote:
> > src/launcher/fetcher.cpp
> > Lines 192-211 (original), 198-209 (patched)
> > <https://reviews.apache.org/r/60628/diff/4/?file=1858875#file1858875line198>
> >
> > I've mentioned this a few times, but this function at least appears super redundant.
> >
> > It's only used in the fetcher's `download()` and `fetchFromCache()`. It's true that these are dumb and expect `Try<string> copy(a, b)` where the result is always `b` (or the error), and I just wish we could fix the use of it instead.
> >
> > Maybe at least leave a `// TODO: Refactor code to eliminate redundnant function.` Since it looks so funny to have `copyFile` return `copyfile`.
The problem is that this is NOT a redundant function. This is called in several places where other methods in those places also return a Try<string>, and stout's copyfile function doesn't return that (it did initially, but that was factored out in prior changes).
I don't see the need to refactor other, unrelated code, just to get rid of this Try<string> handler. Note, by the way, that this function also implements logging that existed in the original fetcher code but does not exist in stout's copyfile function either, so it's not really redundant (in case the logging doesn't matter and, in general, I tend to think (within reason): the more logging the better).
In any case, I've added a TODO as per your request.
> On Oct. 18, 2017, 1:36 a.m., Andrew Schwartzmeyer wrote:
> > src/launcher/fetcher.cpp
> > Lines 267-272 (original), 265-273 (patched)
> > <https://reviews.apache.org/r/60628/diff/4/?file=1858875#file1858875line273>
> >
> > I've mentioned in other reviews, but we _do know_ how to handle `chmod +x` for Windows: we explicitly don't do anything. I don't think this is a `TODO` but a `NOTE: Windows does not have the concept of executable permissions.` It's handled instead by file extensions (and a list in the registrary that says which extensions are "executable").
> >
> > Correct me if I'm wrong, but I don't think we'll _ever_ do anything other than not execute this code on Windows.
That's not true as I stated in an earlier review. I have some ideas on how to handle this (given existing positive AND negative tests), but I need to distribute something and get buy-in from the community. Until then, this isn't clear.
As for as specifically handling chmod, I'd like to completely change the interface into something that is Mesos-specific (there are only a few use cases of this in Mesos today), rather than specifying Linux flags. That will be sure to keep both Linux and Windows in-sync with one another. That's what this bug is all about.
If we need to chmod code and deal with ownership, we should refactor that into what Mesos actually needs and uses, and not be based on Linux specifically, since Mesos runs on more than just Linux now.
My vote is to leave this as is. When MESOS-3176 is resolved, this code will be fixed in a permanent way.
> On Oct. 18, 2017, 1:36 a.m., Andrew Schwartzmeyer wrote:
> > src/launcher/fetcher.cpp
> > Lines 482-489 (original), 483-494 (patched)
> > <https://reviews.apache.org/r/60628/diff/4/?file=1858875#file1858875line491>
> >
> > This one is `chown`, not `chmod`, and really more of a revisit of MESOS-4310 (and MESOS-8063). It is possible to impersonate a user on Windows, so we probably should open a new issue to implement `chown` and then `runas` like code-paths.
A runas like code path is much like su. I've updated the comment to reer to chown and MESOS-8063. I think this can be handled with that change, most likely. If not, we can create a new issue at that time.
> On Oct. 18, 2017, 1:36 a.m., Andrew Schwartzmeyer wrote:
> > src/tests/fetcher_tests.cpp
> > Line 531 (original), 535 (patched)
> > <https://reviews.apache.org/r/60628/diff/4/?file=1858878#file1858878line535>
> >
> > Glad this worked out :)
Me too! I was happy to see the test pass.
> On Oct. 18, 2017, 1:36 a.m., Andrew Schwartzmeyer wrote:
> > src/tests/fetcher_tests.cpp
> > Lines 925 (patched)
> > <https://reviews.apache.org/r/60628/diff/4/?file=1858878#file1858878line925>
> >
> > Nit: s/doesns't/doesn't/
Ok.
- Jeff
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60628/#review188433
-----------------------------------------------------------
On Oct. 17, 2017, 1:19 a.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60628/
> -----------------------------------------------------------
>
> (Updated Oct. 17, 2017, 1:19 a.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.
>
>
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Tests for tar, gzip, and such won't be working on Windows for
> the time being. Thoughts are to provide this capability to the
> Fetcher in a cross-platform manner via a programmatic code library
> rather than Linux-specific command line tools (tar, gzip, etc).
> See MESOS-8064 for more details.
>
> In the short term, zip and unzip will work since PowerShell can
> support that natively.
>
>
> Diffs
> -----
>
> src/launcher/CMakeLists.txt c7a83d476efe13d65fa529e7676b6488eb48f44b
> src/launcher/fetcher.cpp 5a9e93adbf2e4f0b8ff64a69e62fea5f6a0b8f2d
> src/slave/containerizer/fetcher.cpp ba5b0979aa427c5b3dbacf39661d6027da13e0d6
> src/tests/CMakeLists.txt 386e0473c93d0a993248c7818067071d0c761c76
> src/tests/fetcher_tests.cpp df9d2d9586a6457004506c4e2a972ccfc912c7c5
>
>
> Diff: https://reviews.apache.org/r/60628/diff/4/
>
>
> Testing
> -------
>
> See upstream.
>
>
> Thanks,
>
> Jeff Coffler
>
>
Re: Review Request 60628: Enabled fetcher_tests.cpp unit test module
on Windows platform.
Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60628/#review188433
-----------------------------------------------------------
src/launcher/fetcher.cpp
Lines 192-211 (original), 198-209 (patched)
<https://reviews.apache.org/r/60628/#comment265427>
I've mentioned this a few times, but this function at least appears super redundant.
It's only used in the fetcher's `download()` and `fetchFromCache()`. It's true that these are dumb and expect `Try<string> copy(a, b)` where the result is always `b` (or the error), and I just wish we could fix the use of it instead.
Maybe at least leave a `// TODO: Refactor code to eliminate redundnant function.` Since it looks so funny to have `copyFile` return `copyfile`.
src/launcher/fetcher.cpp
Lines 267-272 (original), 265-273 (patched)
<https://reviews.apache.org/r/60628/#comment265428>
I've mentioned in other reviews, but we _do know_ how to handle `chmod +x` for Windows: we explicitly don't do anything. I don't think this is a `TODO` but a `NOTE: Windows does not have the concept of executable permissions.` It's handled instead by file extensions (and a list in the registrary that says which extensions are "executable").
Correct me if I'm wrong, but I don't think we'll _ever_ do anything other than not execute this code on Windows.
src/launcher/fetcher.cpp
Lines 482-489 (original), 483-494 (patched)
<https://reviews.apache.org/r/60628/#comment265429>
This one is `chown`, not `chmod`, and really more of a revisit of MESOS-4310 (and MESOS-8063). It is possible to impersonate a user on Windows, so we probably should open a new issue to implement `chown` and then `runas` like code-paths.
src/tests/fetcher_tests.cpp
Line 531 (original), 535 (patched)
<https://reviews.apache.org/r/60628/#comment265431>
Glad this worked out :)
src/tests/fetcher_tests.cpp
Lines 925 (patched)
<https://reviews.apache.org/r/60628/#comment265430>
Nit: s/doesns't/doesn't/
- Andrew Schwartzmeyer
On Oct. 16, 2017, 6:19 p.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60628/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2017, 6:19 p.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.
>
>
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Tests for tar, gzip, and such won't be working on Windows for
> the time being. Thoughts are to provide this capability to the
> Fetcher in a cross-platform manner via a programmatic code library
> rather than Linux-specific command line tools (tar, gzip, etc).
> See MESOS-8064 for more details.
>
> In the short term, zip and unzip will work since PowerShell can
> support that natively.
>
>
> Diffs
> -----
>
> src/launcher/CMakeLists.txt c7a83d476efe13d65fa529e7676b6488eb48f44b
> src/launcher/fetcher.cpp 5a9e93adbf2e4f0b8ff64a69e62fea5f6a0b8f2d
> src/slave/containerizer/fetcher.cpp ba5b0979aa427c5b3dbacf39661d6027da13e0d6
> src/tests/CMakeLists.txt 386e0473c93d0a993248c7818067071d0c761c76
> src/tests/fetcher_tests.cpp df9d2d9586a6457004506c4e2a972ccfc912c7c5
>
>
> Diff: https://reviews.apache.org/r/60628/diff/4/
>
>
> Testing
> -------
>
> See upstream.
>
>
> Thanks,
>
> Jeff Coffler
>
>
Re: Review Request 60628: Enabled fetcher_tests.cpp unit test module
on Windows platform.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60628/#review188798
-----------------------------------------------------------
PASS: Mesos patch 60628 was successfully built and tested.
Reviews applied: `['60620', '60621', '60622', '60623', '60624', '60626', '60628']`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/60628
- Mesos Reviewbot Windows
On Oct. 20, 2017, 12:33 a.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60628/
> -----------------------------------------------------------
>
> (Updated Oct. 20, 2017, 12:33 a.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.
>
>
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Tests for tar, gzip, and such won't be working on Windows for
> the time being. Thoughts are to provide this capability to the
> Fetcher in a cross-platform manner via a programmatic code library
> rather than Linux-specific command line tools (tar, gzip, etc).
> See MESOS-8064 for more details.
>
> In the short term, zip and unzip will work since PowerShell can
> support that natively.
>
>
> Diffs
> -----
>
> src/launcher/CMakeLists.txt c7a83d476efe13d65fa529e7676b6488eb48f44b
> src/launcher/fetcher.cpp 5a9e93adbf2e4f0b8ff64a69e62fea5f6a0b8f2d
> src/slave/containerizer/fetcher.cpp ba5b0979aa427c5b3dbacf39661d6027da13e0d6
> src/tests/CMakeLists.txt 386e0473c93d0a993248c7818067071d0c761c76
> src/tests/fetcher_tests.cpp df9d2d9586a6457004506c4e2a972ccfc912c7c5
>
>
> Diff: https://reviews.apache.org/r/60628/diff/6/
>
>
> Testing
> -------
>
> See upstream.
>
>
> Thanks,
>
> Jeff Coffler
>
>
Re: Review Request 60628: Enabled fetcher_tests.cpp unit test module
on Windows platform.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60628/#review188768
-----------------------------------------------------------
Patch looks great!
Reviews applied: [60291, 60292, 60293, 60294, 60295, 60296, 60297, 60298, 60299, 60300, 60302, 60304, 60305, 60307, 60308, 60310, 60311, 60313, 60314, 60315, 60316, 60317, 60318, 60319, 60320, 60321, 60322, 60323, 60324, 60325, 60326, 60327, 60328, 60329, 60330, 60331, 60332, 60333, 60334, 60335, 60336, 60337, 60338, 60339, 60340, 60341, 60342, 60343, 60344, 60345, 60620, 60621, 60622, 60623, 60624, 60626, 60628]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On Oct. 19, 2017, 9:33 p.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60628/
> -----------------------------------------------------------
>
> (Updated Oct. 19, 2017, 9:33 p.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.
>
>
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Tests for tar, gzip, and such won't be working on Windows for
> the time being. Thoughts are to provide this capability to the
> Fetcher in a cross-platform manner via a programmatic code library
> rather than Linux-specific command line tools (tar, gzip, etc).
> See MESOS-8064 for more details.
>
> In the short term, zip and unzip will work since PowerShell can
> support that natively.
>
>
> Diffs
> -----
>
> src/launcher/CMakeLists.txt c7a83d476efe13d65fa529e7676b6488eb48f44b
> src/launcher/fetcher.cpp 5a9e93adbf2e4f0b8ff64a69e62fea5f6a0b8f2d
> src/slave/containerizer/fetcher.cpp ba5b0979aa427c5b3dbacf39661d6027da13e0d6
> src/tests/CMakeLists.txt 386e0473c93d0a993248c7818067071d0c761c76
> src/tests/fetcher_tests.cpp df9d2d9586a6457004506c4e2a972ccfc912c7c5
>
>
> Diff: https://reviews.apache.org/r/60628/diff/6/
>
>
> Testing
> -------
>
> See upstream.
>
>
> Thanks,
>
> Jeff Coffler
>
>
Re: Review Request 60628: Enabled fetcher_tests.cpp unit test module
on Windows platform.
Posted by Jeff Coffler <je...@taltos.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60628/
-----------------------------------------------------------
(Updated Oct. 19, 2017, 9:33 p.m.)
Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.
Bugs: MESOS-6705
https://issues.apache.org/jira/browse/MESOS-6705
Repository: mesos
Description
-------
Tests for tar, gzip, and such won't be working on Windows for
the time being. Thoughts are to provide this capability to the
Fetcher in a cross-platform manner via a programmatic code library
rather than Linux-specific command line tools (tar, gzip, etc).
See MESOS-8064 for more details.
In the short term, zip and unzip will work since PowerShell can
support that natively.
Diffs (updated)
-----
src/launcher/CMakeLists.txt c7a83d476efe13d65fa529e7676b6488eb48f44b
src/launcher/fetcher.cpp 5a9e93adbf2e4f0b8ff64a69e62fea5f6a0b8f2d
src/slave/containerizer/fetcher.cpp ba5b0979aa427c5b3dbacf39661d6027da13e0d6
src/tests/CMakeLists.txt 386e0473c93d0a993248c7818067071d0c761c76
src/tests/fetcher_tests.cpp df9d2d9586a6457004506c4e2a972ccfc912c7c5
Diff: https://reviews.apache.org/r/60628/diff/6/
Changes: https://reviews.apache.org/r/60628/diff/5-6/
Testing
-------
See upstream.
Thanks,
Jeff Coffler
Re: Review Request 60628: Enabled fetcher_tests.cpp unit test module
on Windows platform.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60628/#review188739
-----------------------------------------------------------
FAIL: Failed to apply the dependent review: 60292.
Failed command: `python.exe .\support\apply-reviews.py -n -r 60292`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/60628
Relevant logs:
- [apply-review-60292-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/60628/logs/apply-review-60292-stdout.log):
```
error: patch failed: 3rdparty/stout/include/stout/stringify.hpp:57
error: 3rdparty/stout/include/stout/stringify.hpp: patch does not apply
```
- Mesos Reviewbot Windows
On Oct. 19, 2017, 11:18 a.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60628/
> -----------------------------------------------------------
>
> (Updated Oct. 19, 2017, 11:18 a.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.
>
>
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Tests for tar, gzip, and such won't be working on Windows for
> the time being. Thoughts are to provide this capability to the
> Fetcher in a cross-platform manner via a programmatic code library
> rather than Linux-specific command line tools (tar, gzip, etc).
> See MESOS-8064 for more details.
>
> In the short term, zip and unzip will work since PowerShell can
> support that natively.
>
>
> Diffs
> -----
>
> src/launcher/CMakeLists.txt c7a83d476efe13d65fa529e7676b6488eb48f44b
> src/launcher/fetcher.cpp 5a9e93adbf2e4f0b8ff64a69e62fea5f6a0b8f2d
> src/slave/containerizer/fetcher.cpp ba5b0979aa427c5b3dbacf39661d6027da13e0d6
> src/tests/CMakeLists.txt 386e0473c93d0a993248c7818067071d0c761c76
> src/tests/fetcher_tests.cpp df9d2d9586a6457004506c4e2a972ccfc912c7c5
>
>
> Diff: https://reviews.apache.org/r/60628/diff/5/
>
>
> Testing
> -------
>
> See upstream.
>
>
> Thanks,
>
> Jeff Coffler
>
>
Re: Review Request 60628: Enabled fetcher_tests.cpp unit test module
on Windows platform.
Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60628/#review188764
-----------------------------------------------------------
Ship it!
src/launcher/fetcher.cpp
Lines 99 (patched)
<https://reviews.apache.org/r/60628/#comment265775>
Nit: // __WINDOWS__
- Andrew Schwartzmeyer
On Oct. 19, 2017, 11:18 a.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60628/
> -----------------------------------------------------------
>
> (Updated Oct. 19, 2017, 11:18 a.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.
>
>
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Tests for tar, gzip, and such won't be working on Windows for
> the time being. Thoughts are to provide this capability to the
> Fetcher in a cross-platform manner via a programmatic code library
> rather than Linux-specific command line tools (tar, gzip, etc).
> See MESOS-8064 for more details.
>
> In the short term, zip and unzip will work since PowerShell can
> support that natively.
>
>
> Diffs
> -----
>
> src/launcher/CMakeLists.txt c7a83d476efe13d65fa529e7676b6488eb48f44b
> src/launcher/fetcher.cpp 5a9e93adbf2e4f0b8ff64a69e62fea5f6a0b8f2d
> src/slave/containerizer/fetcher.cpp ba5b0979aa427c5b3dbacf39661d6027da13e0d6
> src/tests/CMakeLists.txt 386e0473c93d0a993248c7818067071d0c761c76
> src/tests/fetcher_tests.cpp df9d2d9586a6457004506c4e2a972ccfc912c7c5
>
>
> Diff: https://reviews.apache.org/r/60628/diff/5/
>
>
> Testing
> -------
>
> See upstream.
>
>
> Thanks,
>
> Jeff Coffler
>
>
Re: Review Request 60628: Enabled fetcher_tests.cpp unit test module
on Windows platform.
Posted by Jeff Coffler <je...@taltos.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60628/
-----------------------------------------------------------
(Updated Oct. 19, 2017, 6:18 p.m.)
Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.
Bugs: MESOS-6705
https://issues.apache.org/jira/browse/MESOS-6705
Repository: mesos
Description
-------
Tests for tar, gzip, and such won't be working on Windows for
the time being. Thoughts are to provide this capability to the
Fetcher in a cross-platform manner via a programmatic code library
rather than Linux-specific command line tools (tar, gzip, etc).
See MESOS-8064 for more details.
In the short term, zip and unzip will work since PowerShell can
support that natively.
Diffs (updated)
-----
src/launcher/CMakeLists.txt c7a83d476efe13d65fa529e7676b6488eb48f44b
src/launcher/fetcher.cpp 5a9e93adbf2e4f0b8ff64a69e62fea5f6a0b8f2d
src/slave/containerizer/fetcher.cpp ba5b0979aa427c5b3dbacf39661d6027da13e0d6
src/tests/CMakeLists.txt 386e0473c93d0a993248c7818067071d0c761c76
src/tests/fetcher_tests.cpp df9d2d9586a6457004506c4e2a972ccfc912c7c5
Diff: https://reviews.apache.org/r/60628/diff/5/
Changes: https://reviews.apache.org/r/60628/diff/4-5/
Testing
-------
See upstream.
Thanks,
Jeff Coffler
Re: Review Request 60628: Enabled fetcher_tests.cpp unit test module
on Windows platform.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60628/#review188273
-----------------------------------------------------------
Patch looks great!
Reviews applied: [60291, 60292, 60293, 60294, 60295, 60296, 60297, 60298, 60299, 60300, 60302, 60304, 60305, 60307, 60308, 60310, 60311, 60313, 60314, 60315, 60316, 60317, 60318, 60319, 60320, 60321, 60322, 60323, 60324, 60325, 60326, 60327, 60328, 60329, 60330, 60331, 60332, 60333, 60334, 60335, 60336, 60337, 60338, 60339, 60340, 60341, 60342, 60343, 60344, 60345, 60620, 60621, 60622, 60623, 60624, 60626, 60628]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On Oct. 17, 2017, 1:19 a.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60628/
> -----------------------------------------------------------
>
> (Updated Oct. 17, 2017, 1:19 a.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.
>
>
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Tests for tar, gzip, and such won't be working on Windows for
> the time being. Thoughts are to provide this capability to the
> Fetcher in a cross-platform manner via a programmatic code library
> rather than Linux-specific command line tools (tar, gzip, etc).
> See MESOS-8064 for more details.
>
> In the short term, zip and unzip will work since PowerShell can
> support that natively.
>
>
> Diffs
> -----
>
> src/launcher/CMakeLists.txt c7a83d476efe13d65fa529e7676b6488eb48f44b
> src/launcher/fetcher.cpp 5a9e93adbf2e4f0b8ff64a69e62fea5f6a0b8f2d
> src/slave/containerizer/fetcher.cpp ba5b0979aa427c5b3dbacf39661d6027da13e0d6
> src/tests/CMakeLists.txt 386e0473c93d0a993248c7818067071d0c761c76
> src/tests/fetcher_tests.cpp df9d2d9586a6457004506c4e2a972ccfc912c7c5
>
>
> Diff: https://reviews.apache.org/r/60628/diff/4/
>
>
> Testing
> -------
>
> See upstream.
>
>
> Thanks,
>
> Jeff Coffler
>
>
Re: Review Request 60628: Enabled fetcher_tests.cpp unit test module
on Windows platform.
Posted by Jeff Coffler <je...@taltos.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60628/
-----------------------------------------------------------
(Updated Oct. 17, 2017, 1:19 a.m.)
Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.
Bugs: MESOS-6705
https://issues.apache.org/jira/browse/MESOS-6705
Repository: mesos
Description
-------
Tests for tar, gzip, and such won't be working on Windows for
the time being. Thoughts are to provide this capability to the
Fetcher in a cross-platform manner via a programmatic code library
rather than Linux-specific command line tools (tar, gzip, etc).
See MESOS-8064 for more details.
In the short term, zip and unzip will work since PowerShell can
support that natively.
Diffs (updated)
-----
src/launcher/CMakeLists.txt c7a83d476efe13d65fa529e7676b6488eb48f44b
src/launcher/fetcher.cpp 5a9e93adbf2e4f0b8ff64a69e62fea5f6a0b8f2d
src/slave/containerizer/fetcher.cpp ba5b0979aa427c5b3dbacf39661d6027da13e0d6
src/tests/CMakeLists.txt 386e0473c93d0a993248c7818067071d0c761c76
src/tests/fetcher_tests.cpp df9d2d9586a6457004506c4e2a972ccfc912c7c5
Diff: https://reviews.apache.org/r/60628/diff/4/
Changes: https://reviews.apache.org/r/60628/diff/3-4/
Testing
-------
See upstream.
Thanks,
Jeff Coffler