You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Joseph Wu <jo...@mesosphere.io> on 2016/12/16 06:43:17 UTC
Review Request 54807: Windows: Disable CHECK-failing on symlink
creation.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54807/
-----------------------------------------------------------
Review request for mesos, Andrew Schwartzmeyer, Alex Clemmer, and Michael Park.
Repository: mesos
Description
-------
This fixes the 500 or so tests that are enabled on Windows builds.
With the exception of some parts of the agent recovery pathway,
creating symlinks is not necessary for the agent to function.
The current set of tests will pass with or without the ability to
create symlinks. In the case of the ASF CI, we will never have the
ability to create symlinks on Windows.
Diffs
-----
src/slave/paths.cpp 8792cee43d94e7b0bbd7b80aebbe501236244621
Diff: https://reviews.apache.org/r/54807/diff/
Testing
-------
.\support\windows-build.bat (Win10)
Thanks,
Joseph Wu
Re: Review Request 54807: Windows: Disable CHECK-failing on symlink
creation.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54807/#review159421
-----------------------------------------------------------
Patch looks great!
Reviews applied: [54807]
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 Dec. 16, 2016, 6:43 a.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54807/
> -----------------------------------------------------------
>
> (Updated Dec. 16, 2016, 6:43 a.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, Alex Clemmer, and Michael Park.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This fixes the 500 or so tests that are enabled on Windows builds.
>
> With the exception of some parts of the agent recovery pathway,
> creating symlinks is not necessary for the agent to function.
> The current set of tests will pass with or without the ability to
> create symlinks. In the case of the ASF CI, we will never have the
> ability to create symlinks on Windows.
>
>
> Diffs
> -----
>
> src/slave/paths.cpp 8792cee43d94e7b0bbd7b80aebbe501236244621
>
> Diff: https://reviews.apache.org/r/54807/diff/
>
>
> Testing
> -------
>
> .\support\windows-build.bat (Win10)
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 54807: Windows: Disable CHECK-failing on symlink
creation.
Posted by Joseph Wu <jo...@mesosphere.io>.
> On Dec. 16, 2016, 10:22 a.m., Alex Clemmer wrote:
> > While not ideal, in the event that we were wrong about whether this symlink is required, it seems likely that it would at least make visible, obvious errors, instead of subtle ones. But, can we please create an issue to track our divestment from symlinks here, and tag it with the `microsoft` label so I can track it on our dashboard?
Note: I don't think this is the right thing to do, but it does show how little we actually rely on symlinks (on Windows, and only on Windows). I'm working on a short design doc for removing the need for symlinks on Windows (tests) entirely. In non-test environments, we'll still want symlinks (especially for the long-path support).
- Joseph
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54807/#review159470
-----------------------------------------------------------
On Dec. 15, 2016, 10:43 p.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54807/
> -----------------------------------------------------------
>
> (Updated Dec. 15, 2016, 10:43 p.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, Alex Clemmer, and Michael Park.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This fixes the 500 or so tests that are enabled on Windows builds.
>
> With the exception of some parts of the agent recovery pathway,
> creating symlinks is not necessary for the agent to function.
> The current set of tests will pass with or without the ability to
> create symlinks. In the case of the ASF CI, we will never have the
> ability to create symlinks on Windows.
>
>
> Diffs
> -----
>
> src/slave/paths.cpp 8792cee43d94e7b0bbd7b80aebbe501236244621
>
> Diff: https://reviews.apache.org/r/54807/diff/
>
>
> Testing
> -------
>
> .\support\windows-build.bat (Win10)
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 54807: Windows: Disable CHECK-failing on symlink
creation.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54807/#review159470
-----------------------------------------------------------
Fix it, then Ship it!
While not ideal, in the event that we were wrong about whether this symlink is required, it seems likely that it would at least make visible, obvious errors, instead of subtle ones. But, can we please create an issue to track our divestment from symlinks here, and tag it with the `microsoft` label so I can track it on our dashboard?
src/slave/paths.cpp (line 520)
<https://reviews.apache.org/r/54807/#comment230475>
If I'm reading this code correctly, it looks like we always log a warning saying we failed to symlink. It seems like we want to have a different error message for the Windows code path?
- Alex Clemmer
On Dec. 16, 2016, 6:43 a.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54807/
> -----------------------------------------------------------
>
> (Updated Dec. 16, 2016, 6:43 a.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, Alex Clemmer, and Michael Park.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This fixes the 500 or so tests that are enabled on Windows builds.
>
> With the exception of some parts of the agent recovery pathway,
> creating symlinks is not necessary for the agent to function.
> The current set of tests will pass with or without the ability to
> create symlinks. In the case of the ASF CI, we will never have the
> ability to create symlinks on Windows.
>
>
> Diffs
> -----
>
> src/slave/paths.cpp 8792cee43d94e7b0bbd7b80aebbe501236244621
>
> Diff: https://reviews.apache.org/r/54807/diff/
>
>
> Testing
> -------
>
> .\support\windows-build.bat (Win10)
>
>
> Thanks,
>
> Joseph Wu
>
>