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/07/03 19:35:29 UTC
Review Request 60628: Enable 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/
-----------------------------------------------------------
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
-------
Document that some FetcherTest tests won't work on Windows platform.
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).
In the short term, zip and unzip will work since PowerShell can
support that natively.
Fix FetcherTest.AbsoluteCustomSubdirectoryFails on Windows platform.
Test FetcherTest.InvalidUser can't work on Windows for now.
Fix test FetcherTest.NonExistingFile on Windows platform.
Fix test FetcherTest.AbsolutePath on Windows platform.
Fix test FetcherTest.RelativeFilePath on Windows platform.
Fix test FetcherTest.OSNetUriTest on Windows platform.
Fix test FetcherTest.OSNetUriSpaceTest on Windows platform.
Fix test FetcherTest.FileLocalhostURI on Windows platform.
Fix test FetcherTest.Unzip_ExtractFile on Windows platform.
Fix test FetcherTest.Unzip_ExtractInvalidFile on Windows platform.
Fix test Unzip_ExtractFileWithDuplicatedEntries on Windows platform.
Fix test FetcherTest.UseCustomOutputFile on Windows platform.
Diffs
-----
src/launcher/CMakeLists.txt 97edc4a28a995da8a5e963aaa62c4c955cc59719
src/launcher/fetcher.cpp 42980f5a4a40b72f754156469e9fe60a952d1d87
src/slave/containerizer/fetcher.cpp e3c786b36ad16b33ef9d3eed15f722890e80f0bb
src/tests/fetcher_tests.cpp 99149baa1c7abfabf572a0d0f4512a8e84d1e5be
Diff: https://reviews.apache.org/r/60628/diff/1/
Testing
-------
See upstream.
Thanks,
Jeff Coffler
Re: Review Request 60628: Enable 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/#review182978
-----------------------------------------------------------
src/launcher/CMakeLists.txt
Lines 38-65 (original), 38-57 (patched)
<https://reviews.apache.org/r/60628/#comment258962>
I rewrote this file. Poor Joe :(
src/slave/containerizer/fetcher.cpp
Line 780 (original), 778-783 (patched)
<https://reviews.apache.org/r/60628/#comment258953>
I don't think this change is necessary. On Windows, when this eventually reaches `CreateProcess`, it tries the various suffixes such as `.exe` to launch the process.
Furthermore, we don't do this anywhere else, so it's inconsistent.
src/tests/fetcher_tests.cpp
Line 355 (original), 359 (patched)
<https://reviews.apache.org/r/60628/#comment258954>
Ah. Precisely one use of this odd boolean flag to `path::uri`. I would definitely suggest this just be `path::normalize`. Or make `path::uri` idempotent (i.e. only add the prefix if it's not already there, if that's the logic you're needing here.).
src/tests/fetcher_tests.cpp
Line 466 (original), 470 (patched)
<https://reviews.apache.org/r/60628/#comment258956>
Huh?
src/tests/fetcher_tests.cpp
Line 512 (original), 516 (patched)
<https://reviews.apache.org/r/60628/#comment258963>
Another, huh?
src/tests/fetcher_tests.cpp
Line 565 (original), 569 (patched)
<https://reviews.apache.org/r/60628/#comment258964>
This looks like it dropped the `localhost` part.
src/tests/fetcher_tests.cpp
Line 593 (original), 598 (patched)
<https://reviews.apache.org/r/60628/#comment258965>
Good place for `path::normalize`.
src/tests/fetcher_tests.cpp
Line 739 (original), 754 (patched)
<https://reviews.apache.org/r/60628/#comment258966>
Ditto.
src/tests/fetcher_tests.cpp
Line 827 (original), 845 (patched)
<https://reviews.apache.org/r/60628/#comment258967>
Ditto.
src/tests/fetcher_tests.cpp
Line 880 (original), 898 (patched)
<https://reviews.apache.org/r/60628/#comment258968>
Ditto.
src/tests/fetcher_tests.cpp
Line 937 (original), 969 (patched)
<https://reviews.apache.org/r/60628/#comment258969>
Ditto.
src/tests/fetcher_tests.cpp
Line 979 (original), 1011 (patched)
<https://reviews.apache.org/r/60628/#comment258970>
Ditto.
- Andrew Schwartzmeyer
On July 3, 2017, 12:35 p.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60628/
> -----------------------------------------------------------
>
> (Updated July 3, 2017, 12:35 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
> -------
>
> Document that some FetcherTest tests won't work on Windows platform.
> 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).
>
> In the short term, zip and unzip will work since PowerShell can
> support that natively.
>
> Fix FetcherTest.AbsoluteCustomSubdirectoryFails on Windows platform.
> Test FetcherTest.InvalidUser can't work on Windows for now.
> Fix test FetcherTest.NonExistingFile on Windows platform.
> Fix test FetcherTest.AbsolutePath on Windows platform.
> Fix test FetcherTest.RelativeFilePath on Windows platform.
> Fix test FetcherTest.OSNetUriTest on Windows platform.
> Fix test FetcherTest.OSNetUriSpaceTest on Windows platform.
> Fix test FetcherTest.FileLocalhostURI on Windows platform.
> Fix test FetcherTest.Unzip_ExtractFile on Windows platform.
> Fix test FetcherTest.Unzip_ExtractInvalidFile on Windows platform.
> Fix test Unzip_ExtractFileWithDuplicatedEntries on Windows platform.
> Fix test FetcherTest.UseCustomOutputFile on Windows platform.
>
>
> Diffs
> -----
>
> src/launcher/CMakeLists.txt 97edc4a28a995da8a5e963aaa62c4c955cc59719
> src/launcher/fetcher.cpp 42980f5a4a40b72f754156469e9fe60a952d1d87
> src/slave/containerizer/fetcher.cpp e3c786b36ad16b33ef9d3eed15f722890e80f0bb
> src/tests/fetcher_tests.cpp 99149baa1c7abfabf572a0d0f4512a8e84d1e5be
>
>
> Diff: https://reviews.apache.org/r/60628/diff/1/
>
>
> 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 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 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 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 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
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 Jeff Coffler <je...@taltos.com>.
-----------------------------------------------------------
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: Enable 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/#review187232
-----------------------------------------------------------
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. 6, 2017, 4:04 a.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60628/
> -----------------------------------------------------------
>
> (Updated Oct. 6, 2017, 4:04 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
> -------
>
> Document that some FetcherTest tests won't work on Windows platform.
> 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).
>
> In the short term, zip and unzip will work since PowerShell can
> support that natively.
>
> Fix FetcherTest.AbsoluteCustomSubdirectoryFails on Windows platform.
> Test FetcherTest.InvalidUser can't work on Windows for now.
> Fix test FetcherTest.NonExistingFile on Windows platform.
> Fix test FetcherTest.AbsolutePath on Windows platform.
> Fix test FetcherTest.RelativeFilePath on Windows platform.
> Fix test FetcherTest.OSNetUriTest on Windows platform.
> Fix test FetcherTest.OSNetUriSpaceTest on Windows platform.
> Fix test FetcherTest.FileLocalhostURI on Windows platform.
> Fix test FetcherTest.Unzip_ExtractFile on Windows platform.
> Fix test FetcherTest.Unzip_ExtractInvalidFile on Windows platform.
> Fix test Unzip_ExtractFileWithDuplicatedEntries on Windows platform.
> Fix test FetcherTest.UseCustomOutputFile on Windows platform.
>
>
> Diffs
> -----
>
> src/launcher/CMakeLists.txt c7a83d476efe13d65fa529e7676b6488eb48f44b
> src/launcher/fetcher.cpp 5a9e93adbf2e4f0b8ff64a69e62fea5f6a0b8f2d
> src/slave/containerizer/fetcher.cpp ba5b0979aa427c5b3dbacf39661d6027da13e0d6
> src/tests/CMakeLists.txt def233d13f627f58362fdc61ac3e9c104d65fe7e
> src/tests/fetcher_tests.cpp df9d2d9586a6457004506c4e2a972ccfc912c7c5
>
>
> Diff: https://reviews.apache.org/r/60628/diff/2/
>
>
> Testing
> -------
>
> See upstream.
>
>
> Thanks,
>
> Jeff Coffler
>
>
Re: Review Request 60628: Enable 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/#review187238
-----------------------------------------------------------
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, 60625, 60626, 60628]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On Oct. 6, 2017, 4:04 a.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60628/
> -----------------------------------------------------------
>
> (Updated Oct. 6, 2017, 4:04 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
> -------
>
> Document that some FetcherTest tests won't work on Windows platform.
> 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).
>
> In the short term, zip and unzip will work since PowerShell can
> support that natively.
>
> Fix FetcherTest.AbsoluteCustomSubdirectoryFails on Windows platform.
> Test FetcherTest.InvalidUser can't work on Windows for now.
> Fix test FetcherTest.NonExistingFile on Windows platform.
> Fix test FetcherTest.AbsolutePath on Windows platform.
> Fix test FetcherTest.RelativeFilePath on Windows platform.
> Fix test FetcherTest.OSNetUriTest on Windows platform.
> Fix test FetcherTest.OSNetUriSpaceTest on Windows platform.
> Fix test FetcherTest.FileLocalhostURI on Windows platform.
> Fix test FetcherTest.Unzip_ExtractFile on Windows platform.
> Fix test FetcherTest.Unzip_ExtractInvalidFile on Windows platform.
> Fix test Unzip_ExtractFileWithDuplicatedEntries on Windows platform.
> Fix test FetcherTest.UseCustomOutputFile on Windows platform.
>
>
> Diffs
> -----
>
> src/launcher/CMakeLists.txt c7a83d476efe13d65fa529e7676b6488eb48f44b
> src/launcher/fetcher.cpp 5a9e93adbf2e4f0b8ff64a69e62fea5f6a0b8f2d
> src/slave/containerizer/fetcher.cpp ba5b0979aa427c5b3dbacf39661d6027da13e0d6
> src/tests/CMakeLists.txt def233d13f627f58362fdc61ac3e9c104d65fe7e
> src/tests/fetcher_tests.cpp df9d2d9586a6457004506c4e2a972ccfc912c7c5
>
>
> Diff: https://reviews.apache.org/r/60628/diff/2/
>
>
> 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. 8, 2017, 4:31 a.m., Andrew Schwartzmeyer wrote:
> > src/launcher/fetcher.cpp
> > Lines 192-211 (original), 198-208 (patched)
> > <https://reviews.apache.org/r/60628/diff/2/?file=1847166#file1847166line198>
> >
> > What's the point of this entire function now, since we have `os::copyfile`? It appears redundant. The only difference is the return value of `destinationPath`, but does the user even need this?
There are a number of call paths, all expecting a Try<string>. `os::copyfile` doesn't return that. This makes the caller more compatible with other functions that are called (that don't involve `os::copyfile`). See callers, having this minimizes code churn.
> On Oct. 8, 2017, 4:31 a.m., Andrew Schwartzmeyer wrote:
> > src/slave/containerizer/fetcher.cpp
> > Line 228 (original), 226 (patched)
> > <https://reviews.apache.org/r/60628/diff/2/?file=1847167#file1847167line228>
> >
> > Ah ha! The one place in the code base that we should actually need to normalize back from `/` to `\` for Windows: `Fetcher::uriToLocalPath`.
> >
> > Is logic like `uriToLocalPath` anywhere else, too? If so, it's a great chance to refactor into your new `uri::` namespace.
I don't think so. I'm planning on getting rid of internal calls to `path::normalize` from `path::` functions and see what pops up.
> On Oct. 8, 2017, 4:31 a.m., Andrew Schwartzmeyer wrote:
> > src/tests/fetcher_tests.cpp
> > Line 132 (original), 132 (patched)
> > <https://reviews.apache.org/r/60628/diff/2/?file=1847169#file1847169line132>
> >
> > Your Review Board name is `coffler` :P
Groan, probably should have made it consistent with github.
> On Oct. 8, 2017, 4:31 a.m., Andrew Schwartzmeyer wrote:
> > src/tests/fetcher_tests.cpp
> > Line 469 (original), 473 (patched)
> > <https://reviews.apache.org/r/60628/diff/2/?file=1847169#file1847169line473>
> >
> > What was changed that required this to change?
I think this was due to normalizing in the path object. Removed, will test again.
> On Oct. 8, 2017, 4:31 a.m., Andrew Schwartzmeyer wrote:
> > src/tests/fetcher_tests.cpp
> > Line 913 (original), 935-940 (patched)
> > <https://reviews.apache.org/r/60628/diff/2/?file=1847169#file1847169line937>
> >
> > Can we explain this a little further? I'm guessetimated that `verifyMetrics(x, y)` here is testing error codes, maybe? Except above it says `2` instead of `1`. What's going on here?
Yeah. That last parameter is the number of errors. It's zero on Windows because PowerShell doesn't consider CRC errors to be an error.
- Jeff
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60628/#review187357
-----------------------------------------------------------
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: Enable 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/#review187357
-----------------------------------------------------------
Summary and description need to follow Mesos standards (past-tense, full-sentence summary, with description in the body).
(This description in particular needs to be cleaned up.)
src/launcher/CMakeLists.txt
Lines 25-28 (original), 25-26 (patched)
<https://reviews.apache.org/r/60628/#comment264317>
Yay!
src/launcher/fetcher.cpp
Lines 99 (patched)
<https://reviews.apache.org/r/60628/#comment264318>
This needs to be `#endif // __WINDOWS__`.
src/launcher/fetcher.cpp
Lines 192-211 (original), 198-208 (patched)
<https://reviews.apache.org/r/60628/#comment264320>
What's the point of this entire function now, since we have `os::copyfile`? It appears redundant. The only difference is the return value of `destinationPath`, but does the user even need this?
src/launcher/fetcher.cpp
Lines 196-197 (original), 202-203 (patched)
<https://reviews.apache.org/r/60628/#comment264319>
I think our style is a line between the assignment statement and the expression.
src/slave/containerizer/fetcher.cpp
Line 210 (original), 209 (patched)
<https://reviews.apache.org/r/60628/#comment264321>
Let's just replace this with `!path::absolute(path)`. No need for the intermediate `isRelativePath` variable. It's just a boolean. "if not absolute path" reads just fine.
src/slave/containerizer/fetcher.cpp
Line 228 (original), 226 (patched)
<https://reviews.apache.org/r/60628/#comment264322>
Ah ha! The one place in the code base that we should actually need to normalize back from `/` to `\` for Windows: `Fetcher::uriToLocalPath`.
Is logic like `uriToLocalPath` anywhere else, too? If so, it's a great chance to refactor into your new `uri::` namespace.
src/tests/CMakeLists.txt
Lines 270-280 (original), 270-280 (patched)
<https://reviews.apache.org/r/60628/#comment264323>
This is fine for now, MESOS-8035 covers fixing these dependencies (i.e. `mesos-agent` should depend on `mesos-fetcher` instead of `mesos-tests` doing it).
src/tests/fetcher_tests.cpp
Line 132 (original), 132 (patched)
<https://reviews.apache.org/r/60628/#comment264324>
Your Review Board name is `coffler` :P
src/tests/fetcher_tests.cpp
Line 157 (original), 158 (patched)
<https://reviews.apache.org/r/60628/#comment264325>
(Same comment as in prior reviews, ignoring this for the rest of the review.)
src/tests/fetcher_tests.cpp
Lines 244-245 (patched)
<https://reviews.apache.org/r/60628/#comment264326>
Let's make sure we mention the issue.
src/tests/fetcher_tests.cpp
Line 469 (original), 473 (patched)
<https://reviews.apache.org/r/60628/#comment264327>
What was changed that required this to change?
src/tests/fetcher_tests.cpp
Line 515 (original), 519 (patched)
<https://reviews.apache.org/r/60628/#comment264328>
Ditto.
src/tests/fetcher_tests.cpp
Lines 586 (patched)
<https://reviews.apache.org/r/60628/#comment264329>
Mention the issue (or resolve this now as "executable" permissions don't map to Windows, at all).
src/tests/fetcher_tests.cpp
Lines 627 (patched)
<https://reviews.apache.org/r/60628/#comment264330>
Ditto.
src/tests/fetcher_tests.cpp
Lines 670 (patched)
<https://reviews.apache.org/r/60628/#comment264331>
Ditto.
src/tests/fetcher_tests.cpp
Lines 731-732 (patched)
<https://reviews.apache.org/r/60628/#comment264332>
Open an issue for this and mention it here.
src/tests/fetcher_tests.cpp
Lines 782-783 (patched)
<https://reviews.apache.org/r/60628/#comment264333>
Ditto.
src/tests/fetcher_tests.cpp
Lines 806-807 (original)
<https://reviews.apache.org/r/60628/#comment264334>
What happened here?
src/tests/fetcher_tests.cpp
Lines 919-924 (patched)
<https://reviews.apache.org/r/60628/#comment264337>
PowerShell* (casing)
"so it succeeds, whereas the zip utility errors."
src/tests/fetcher_tests.cpp
Lines 928 (patched)
<https://reviews.apache.org/r/60628/#comment264335>
Style.
src/tests/fetcher_tests.cpp
Line 913 (original), 935-940 (patched)
<https://reviews.apache.org/r/60628/#comment264339>
Can we explain this a little further? I'm guessetimated that `verifyMetrics(x, y)` here is testing error codes, maybe? Except above it says `2` instead of `1`. What's going on here?
src/tests/fetcher_tests.cpp
Lines 941 (patched)
<https://reviews.apache.org/r/60628/#comment264338>
Style again.
- Andrew Schwartzmeyer
On Oct. 5, 2017, 9:04 p.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60628/
> -----------------------------------------------------------
>
> (Updated Oct. 5, 2017, 9:04 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
> -------
>
> Document that some FetcherTest tests won't work on Windows platform.
> 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).
>
> In the short term, zip and unzip will work since PowerShell can
> support that natively.
>
> Fix FetcherTest.AbsoluteCustomSubdirectoryFails on Windows platform.
> Test FetcherTest.InvalidUser can't work on Windows for now.
> Fix test FetcherTest.NonExistingFile on Windows platform.
> Fix test FetcherTest.AbsolutePath on Windows platform.
> Fix test FetcherTest.RelativeFilePath on Windows platform.
> Fix test FetcherTest.OSNetUriTest on Windows platform.
> Fix test FetcherTest.OSNetUriSpaceTest on Windows platform.
> Fix test FetcherTest.FileLocalhostURI on Windows platform.
> Fix test FetcherTest.Unzip_ExtractFile on Windows platform.
> Fix test FetcherTest.Unzip_ExtractInvalidFile on Windows platform.
> Fix test Unzip_ExtractFileWithDuplicatedEntries on Windows platform.
> Fix test FetcherTest.UseCustomOutputFile on Windows platform.
>
>
> Diffs
> -----
>
> src/launcher/CMakeLists.txt c7a83d476efe13d65fa529e7676b6488eb48f44b
> src/launcher/fetcher.cpp 5a9e93adbf2e4f0b8ff64a69e62fea5f6a0b8f2d
> src/slave/containerizer/fetcher.cpp ba5b0979aa427c5b3dbacf39661d6027da13e0d6
> src/tests/CMakeLists.txt def233d13f627f58362fdc61ac3e9c104d65fe7e
> src/tests/fetcher_tests.cpp df9d2d9586a6457004506c4e2a972ccfc912c7c5
>
>
> Diff: https://reviews.apache.org/r/60628/diff/2/
>
>
> Testing
> -------
>
> See upstream.
>
>
> Thanks,
>
> Jeff Coffler
>
>
Re: Review Request 60628: Enable 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/#review187231
-----------------------------------------------------------
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. 6, 2017, 4:04 a.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60628/
> -----------------------------------------------------------
>
> (Updated Oct. 6, 2017, 4:04 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
> -------
>
> Document that some FetcherTest tests won't work on Windows platform.
> 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).
>
> In the short term, zip and unzip will work since PowerShell can
> support that natively.
>
> Fix FetcherTest.AbsoluteCustomSubdirectoryFails on Windows platform.
> Test FetcherTest.InvalidUser can't work on Windows for now.
> Fix test FetcherTest.NonExistingFile on Windows platform.
> Fix test FetcherTest.AbsolutePath on Windows platform.
> Fix test FetcherTest.RelativeFilePath on Windows platform.
> Fix test FetcherTest.OSNetUriTest on Windows platform.
> Fix test FetcherTest.OSNetUriSpaceTest on Windows platform.
> Fix test FetcherTest.FileLocalhostURI on Windows platform.
> Fix test FetcherTest.Unzip_ExtractFile on Windows platform.
> Fix test FetcherTest.Unzip_ExtractInvalidFile on Windows platform.
> Fix test Unzip_ExtractFileWithDuplicatedEntries on Windows platform.
> Fix test FetcherTest.UseCustomOutputFile on Windows platform.
>
>
> Diffs
> -----
>
> src/launcher/CMakeLists.txt c7a83d476efe13d65fa529e7676b6488eb48f44b
> src/launcher/fetcher.cpp 5a9e93adbf2e4f0b8ff64a69e62fea5f6a0b8f2d
> src/slave/containerizer/fetcher.cpp ba5b0979aa427c5b3dbacf39661d6027da13e0d6
> src/tests/CMakeLists.txt def233d13f627f58362fdc61ac3e9c104d65fe7e
> src/tests/fetcher_tests.cpp df9d2d9586a6457004506c4e2a972ccfc912c7c5
>
>
> Diff: https://reviews.apache.org/r/60628/diff/2/
>
>
> Testing
> -------
>
> See upstream.
>
>
> Thanks,
>
> Jeff Coffler
>
>
Re: Review Request 60628: Enable 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. 6, 2017, 4:04 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
-------
Document that some FetcherTest tests won't work on Windows platform.
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).
In the short term, zip and unzip will work since PowerShell can
support that natively.
Fix FetcherTest.AbsoluteCustomSubdirectoryFails on Windows platform.
Test FetcherTest.InvalidUser can't work on Windows for now.
Fix test FetcherTest.NonExistingFile on Windows platform.
Fix test FetcherTest.AbsolutePath on Windows platform.
Fix test FetcherTest.RelativeFilePath on Windows platform.
Fix test FetcherTest.OSNetUriTest on Windows platform.
Fix test FetcherTest.OSNetUriSpaceTest on Windows platform.
Fix test FetcherTest.FileLocalhostURI on Windows platform.
Fix test FetcherTest.Unzip_ExtractFile on Windows platform.
Fix test FetcherTest.Unzip_ExtractInvalidFile on Windows platform.
Fix test Unzip_ExtractFileWithDuplicatedEntries on Windows platform.
Fix test FetcherTest.UseCustomOutputFile on Windows platform.
Diffs (updated)
-----
src/launcher/CMakeLists.txt c7a83d476efe13d65fa529e7676b6488eb48f44b
src/launcher/fetcher.cpp 5a9e93adbf2e4f0b8ff64a69e62fea5f6a0b8f2d
src/slave/containerizer/fetcher.cpp ba5b0979aa427c5b3dbacf39661d6027da13e0d6
src/tests/CMakeLists.txt def233d13f627f58362fdc61ac3e9c104d65fe7e
src/tests/fetcher_tests.cpp df9d2d9586a6457004506c4e2a972ccfc912c7c5
Diff: https://reviews.apache.org/r/60628/diff/2/
Changes: https://reviews.apache.org/r/60628/diff/1-2/
Testing
-------
See upstream.
Thanks,
Jeff Coffler