Re: Review Request 60356: Tried to compile Mesos with Boost 1.6.4

[David Carlier]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60356/
---

(Updated June 22, 2017, 5:09 a.m.)


Review request for mesos.


Changes
---

Linked to JIRA ticket.


Bugs: MESOS-7707
https://issues.apache.org/jira/browse/MESOS-7707


Repository: mesos


Description
---

Making it compilable with newer Boost versions


Diffs
-

  3rdparty/stout/include/stout/json.hpp 4f9ea1b34 


Diff: https://reviews.apache.org/r/60356/diff/1/


Testing
---


Thanks,

David Carlier

Re: Review Request 60281: Added the adjusted `TaskInfo` into the `Operation` to be sent out.

[Neil Conway]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60281/#review178623
---




src/master/master.cpp
Line 4558 (original), 4558 (patched)


If we're already copying the operation (see `foreach` a few lines up), do 
we need to make an additional copy of the `TaskInfo`? If we mutate w/o copying, 
we'd avoid the need to introduce a second variable.


- Neil Conway


On June 21, 2017, 7:06 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60281/
> ---
> 
> (Updated June 21, 2017, 7:06 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since we drop invalid tasks on a `LAUNCH` operation, we send a new
> `Operation` with only the valid tasks appended. At the same time,
> we adjust the `TaskInfo` a little bit to perform validation, and
> to maintain backwards compatibility. In the end, we were appending
> the unadjusted task to the new `Operation` rather than the adjusted
> task. This patch changes this to append the adjusted task instead.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp ec594a8f4fa95e77fc38103c5561d1797fe2b133 
> 
> 
> Diff: https://reviews.apache.org/r/60281/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 60283: Fixed `convertResourceFormat` uses with `validateAndUpgradeResources`.

[Neil Conway]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60283/#review178622
---




src/master/master.cpp
Line 4221 (original), 4221 (patched)


Update this comment.


- Neil Conway


On June 21, 2017, 7:08 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60283/
> ---
> 
> (Updated June 21, 2017, 7:08 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Initially, it seemed like calling `convertResourceFormat` after
> operation validation seemed safe since the operation validation
> themselves performed `Resources::validate` within them.
> 
> However, the rest of the operation validation code relies on
> the fact that the resources have been validated, and uses
> functions such as `isDynamicallyReserved`. Since functions such as
> `isDynamicallyReserved` now requires "post-reservation-refinement"
> format, we must perform this conversion earlier.
> 
> In this patch, we use `upgradeResources` to perform resources
> validation __and__ convert the resources before going into the
> operation and task validation.
> 
> We really need a better plan for this going forward. MESOS-7702.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp ec594a8f4fa95e77fc38103c5561d1797fe2b133 
>   src/tests/master_tests.cpp 652f37ab6f640ac87c48e419680501b705e11394 
>   src/tests/resource_offers_tests.cpp 
> c2bbf834c1d46079af492887b9dd40e57f3f2ac7 
> 
> 
> Diff: https://reviews.apache.org/r/60283/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Review Request 60356: Tried to compile Mesos with Boost 1.6.4

[David Carlier]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60356/
---

Review request for mesos.


Repository: mesos


Description
---

Making it compilable with newer Boost versions


Diffs
-

  3rdparty/stout/include/stout/json.hpp 4f9ea1b34 


Diff: https://reviews.apache.org/r/60356/diff/1/


Testing
---


Thanks,

David Carlier

Re: Review Request 60356: Tried to compile Mesos with Boost 1.6.4

[Mesos Reviewbot Windows]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60356/#review178621
---



Bad review!

Reviews applied: []

Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing 
list.

- Mesos Reviewbot Windows


On June 21, 2017, 9:28 p.m., David Carlier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60356/
> ---
> 
> (Updated June 21, 2017, 9:28 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Making it compilable with newer Boost versions
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/json.hpp 4f9ea1b34 
> 
> 
> Diff: https://reviews.apache.org/r/60356/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Carlier
> 
>

Re: Review Request 60284: Removed unused `convertResourceFormat` for `Operation`.

[Mesos Reviewbot Windows]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60284/#review178618
---



Patch looks great!

Reviews applied: [60281, 60282, 60351, 60352, 60283, 60255, 60284]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On June 21, 2017, 7:08 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60284/
> ---
> 
> (Updated June 21, 2017, 7:08 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed unused `convertResourceFormat` for `Operation`.
> 
> 
> Diffs
> -
> 
>   src/common/resources_utils.hpp 4b033687fa7224ffa36523cb3ca366f4384fd4f8 
>   src/common/resources_utils.cpp 751194c5ff5c794c77be3bcf16892af39de6d4d9 
> 
> 
> Diff: https://reviews.apache.org/r/60284/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 60353: Allowed dashes in Python modules.

[Mesos Reviewbot Windows]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60353/#review178619
---



Bad patch!

Reviews applied: [60353]

Failed command: python support/apply-reviews.py -n -r 60353

Error:
error: patch failed: support/mesos-style.py:279
error: support/mesos-style.py: patch does not apply

Full log: http://mesos-winbot.westus.cloudapp.azure.com/logs/111/console

- Mesos Reviewbot Windows


On June 22, 2017, 12:57 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60353/
> ---
> 
> (Updated June 22, 2017, 12:57 a.m.)
> 
> 
> Review request for mesos, Armand Grillet and Benjamin Bannier.
> 
> 
> Bugs: MESOS-6390
> https://issues.apache.org/jira/browse/MESOS-6390
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> None of our support scripts are intended to be used as modules, so
> the usage of dashes in the filenames are acceptable.  PyLint however
> considers dashes in module names (same as the filename) to be an error.
> 
> This commit also adds the `support` directory to the list of linted
> sources.
> 
> 
> Diffs
> -
> 
>   src/cli_new/pylint.config 63fbb94fbf47cccd1053b4c1eafac18b88dbc818 
>   support/mesos-style.py 48d816f72df65d77364769f812fc06afcd6b5aec 
> 
> 
> Diff: https://reviews.apache.org/r/60353/diff/1/
> 
> 
> Testing
> ---
> 
> This shouldn't be commited yet as a few support scripts don't pass the linter 
> yet.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 60279: Add constructor for ObjectApprover::Object.

[Mesos Reviewbot Windows]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60279/#review178615
---



Patch looks great!

Reviews applied: [60279]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On June 21, 2017, 11:35 a.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60279/
> ---
> 
> (Updated June 21, 2017, 11:35 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Greg Mann, and Vinod Kone.
> 
> 
> Bugs: MESOS-7630
> https://issues.apache.org/jira/browse/MESOS-7630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add constructor for ObjectApprover::Object.
> 
> Add constructors for ObjectApprover::Object, making the caller side cleaner.
> Only one case is ignored, it's in "src/slave/slave.cpp:6709", since it's 
> setting parameters after the construction.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.hpp 
> 95cbcf3f9a962a896c9c23e961f02eb16dff36f8 
>   src/common/http.cpp 2f7718cbc2e449b4f7c89754e8f84eac2f3c35b6 
>   src/master/http.cpp 4dd43fd7c3fb986f4eed78bce574b6d3af156b67 
>   src/slave/http.cpp cbbc1dc27cc90bac8d48cbbc84266c3d87490a3c 
> 
> 
> Diff: https://reviews.apache.org/r/60279/diff/1/
> 
> 
> Testing
> ---
> 
> Passed "make check -j48"
> No specific test case related.
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Re: Review Request 60345: Windows: Set Unicode compilation flags.

[Mesos Reviewbot Windows]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60345/#review178614
---



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]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On June 21, 2017, 3:17 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60345/
> ---
> 
> (Updated June 21, 2017, 3:17 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
> Joseph Wu, Li Li, and Michael Park.
> 
> 
> Bugs: MESOS-6817
> https://issues.apache.org/jira/browse/MESOS-6817
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Set Unicode compilation flags.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 3fa2e2f3b51a7a8dfe05085298f240019ff7b9c4 
> 
> 
> Diff: https://reviews.apache.org/r/60345/diff/1/
> 
> 
> Testing
> ---
> 
> # Windows
> 
> This is with Visual Studio 2017 tools, _without_ the long path registry key 
> set.
> 
> ## `stout-tests`
> ```
> [--] Global test environment tear-down
> [==] 253 tests from 42 test cases ran. (4910 ms total)
> [  PASSED  ] 253 tests.
> 
>   YOU HAVE 12 DISABLED TESTS
> ```
> 
> ## `libprocess-tests`
> ```
> [--] Global test environment tear-down
> [==] 148 tests from 27 test cases ran. (1232 ms total)
> [  PASSED  ] 148 tests.
> 
>   YOU HAVE 15 DISABLED TESTS
> ```
> 
> ## `mesos-tests`
> ```
> [--] Global test environment tear-down
> [==] 596 tests from 61 test cases ran. (115278 ms total)
> [  PASSED  ] 596 tests.
> 
>   YOU HAVE 166 DISABLED TESTS
> ```
> 
> # Linux
> 
> This is using `make check` on CentOS 7.
> 
> ## `stout-tests`
> ```
> [--] Global test environment tear-down
> [==] 310 tests from 50 test cases ran. (392 ms total)
> [  PASSED  ] 310 tests.
> ```
> 
> ## `libprocess-tests`
> ```
> [--] Global test environment tear-down
> [==] 193 tests from 33 test cases ran. (960 ms total)
> [  PASSED  ] 193 tests.
> 
>   YOU HAVE 2 DISABLED TESTS
> ```
> 
> ## `mesos-tests`
> ```
> [--] Global test environment tear-down
> [==] 1611 tests from 177 test cases ran. (481344 ms total)
> [  PASSED  ] 1611 tests.
> 
>   YOU HAVE 20 DISABLED TESTS
> ```
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 60280: Provide full path to the custom executor.

[Aaron Wood via Review Board]

> On June 21, 2017, 11:07 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/launch.cpp
> > Lines 671-674 (patched)
> > 
> >
> > Sorry, I got confused. User should be the one setting argv[0]. So we 
> > don't need to change the code here.

No problem.
So every time someone wants to make use of `argv[0]` in their executor 
somewhere they'll have to construct that argument and send it along in the 
protos. Why not take care of it here so that it's less to do (and less going 
over the wire) from the scheduler's side?


> On June 21, 2017, 11:07 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/launch.cpp
> > Lines 801 (patched)
> > 
> >
> > OK, what if 'executable' has an absolute path?

I guess this would break in that case. My thought was that no one would know 
the absolute path ahead of time. For example, how would a user know the 
container ID (not even generated yet since the executor/task has not been sent 
to Mesos) to be used in the full absolute path to their executor binary in the 
sandbox?


- Aaron


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60280/#review178586
---


On June 21, 2017, 10:26 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60280/
> ---
> 
> (Updated June 21, 2017, 10:26 p.m.)
> 
> 
> Review request for mesos, Jie Yu, James Peach, and Zhitao Li.
> 
> 
> Bugs: MESOS-7703
> https://issues.apache.org/jira/browse/MESOS-7703
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If a framework specifies use of its own executor and sets shell to false the 
> executor is never found. Additionally, the name of the binary is never passed 
> as an argument so executors making use of argv[0] will fail. This provides 
> the full path to the executor so that the `execvp` or `execvpe` is 
> successful. The name of the binary is also passed as the first argument for 
> cases where there is no shell used.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 162ca1c2e 
> 
> 
> Diff: https://reviews.apache.org/r/60280/diff/2/
> 
> 
> Testing
> ---
> 
> `cd build && cmake .. -DCMAKE_BUILD_TYPE=Release && make -j4`
> Also spun up a master and agent, connected and sent a task using the UCR 
> (both with and without the use of an OCI image) via our own framework, and 
> checked the sandbox to verify that things went accordingly.
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 60280: Provide full path to the custom executor.

[Mesos Reviewbot Windows]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60280/#review178612
---



Patch looks great!

Reviews applied: [60280]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On June 21, 2017, 10:26 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60280/
> ---
> 
> (Updated June 21, 2017, 10:26 p.m.)
> 
> 
> Review request for mesos, Jie Yu, James Peach, and Zhitao Li.
> 
> 
> Bugs: MESOS-7703
> https://issues.apache.org/jira/browse/MESOS-7703
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If a framework specifies use of its own executor and sets shell to false the 
> executor is never found. Additionally, the name of the binary is never passed 
> as an argument so executors making use of argv[0] will fail. This provides 
> the full path to the executor so that the `execvp` or `execvpe` is 
> successful. The name of the binary is also passed as the first argument for 
> cases where there is no shell used.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 162ca1c2e 
> 
> 
> Diff: https://reviews.apache.org/r/60280/diff/2/
> 
> 
> Testing
> ---
> 
> `cd build && cmake .. -DCMAKE_BUILD_TYPE=Release && make -j4`
> Also spun up a master and agent, connected and sent a task using the UCR 
> (both with and without the use of an OCI image) via our own framework, and 
> checked the sandbox to verify that things went accordingly.
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Review Request 60353: Allowed dashes in Python modules.

[Joseph Wu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60353/
---

Review request for mesos, Armand Grillet and Benjamin Bannier.


Bugs: MESOS-6390
https://issues.apache.org/jira/browse/MESOS-6390


Repository: mesos


Description
---

None of our support scripts are intended to be used as modules, so
the usage of dashes in the filenames are acceptable.  PyLint however
considers dashes in module names (same as the filename) to be an error.

This commit also adds the `support` directory to the list of linted
sources.


Diffs
-

  src/cli_new/pylint.config 63fbb94fbf47cccd1053b4c1eafac18b88dbc818 
  support/mesos-style.py 48d816f72df65d77364769f812fc06afcd6b5aec 


Diff: https://reviews.apache.org/r/60353/diff/1/


Testing
---

This shouldn't be commited yet as a few support scripts don't pass the linter 
yet.


Thanks,

Joseph Wu

Re: Review Request 60252: Fixed a bug that causes segfault in ProcessManager::finalize.

[Joseph Wu]

> On June 21, 2017, 11:42 a.m., Joseph Wu wrote:
> > I agree this makes the finalization logic safer, but I don't see a way to 
> > add a `nullptr` to the `ProcessManager::processes` map.
> > 
> > `ProcessManager::spawn` is the only location where the `processes` map is 
> > inserted, and this method CHECK-fails if you give it a `nullptr`.  Two 
> > other locations use the map's `operator[]`, but those are guarded by a 
> > mutex so they will never empty-initialize a map entry.
> > 
> > Can you note how you ran into a segfault here?  It's more likely that the 
> > segfault was caused by a dereferencing a non-`nullptr` process that had 
> > already been deallocated (which would not be fixed by this review).
> 
> Jiang Yan Xu wrote:
> The use case is something like this: 
> 
> https://github.com/apache/mesos/blob/65152413836b58d01ace3a40bdc9056f9a489c6b/3rdparty/libprocess/src/tests/process_tests.cpp#L691
> 
> Arguably I made an error in doing:
> 
> ```
> ProcessBase process;
> UPID pid = spawn(, true);
> 
> ...
> 
> terminate(process);
> wait(process);
> ```
> 
> And the process (in a test method) goes out of scope before libprocess 
> finalize() so it segfaulted.
> 
> Just felt it's not really necessary to fail (and it used to not) in this 
> way?

I see.  That error will still segfault, even with this patch.

---

The tests (and otherwise invalid/risky uses of libprocess methods) did not 
segfault in the past because we didn't clean up libprocess in the past.


- Joseph


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60252/#review178522
---


On June 20, 2017, 4:16 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60252/
> ---
> 
> (Updated June 20, 2017, 4:16 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We don't need and shouldn't assume pointers in `processes` are
> non-nullptr.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> 7ce6d2b13baa68906e091a95c0dd58ee1ca2bc7d 
> 
> 
> Diff: https://reviews.apache.org/r/60252/diff/1/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Review Request 60352: Relaxed the resource format restriction for frameworks.

[Michael Park]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60352/
---

Review request for mesos and Neil Conway.


Repository: mesos


Description (updated)
---

This patch relaxes the validation such that a framework with
RESERVATION_REFINEMENT capability can send the old resources format as
well. This is more of a temporary solution due to the complexity of
the validation code, and we will revisit whether we want to restrict
RESERVATION_REFINEMENT-capable frameworks to the new format.


Diffs (updated)
-

  src/master/validation.hpp 129505948c0fad28743e290f24df55918c8d601c 
  src/master/validation.cpp 4d0af26fedb6ef3039536cb42d2ac9112c9f4bdd 


Diff: https://reviews.apache.org/r/60352/diff/1/


Testing
---


Thanks,

Michael Park

Review Request 60351: Used `JSON::protobuf` to print resources not validated nor converted.

[Michael Park]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60351/
---

Review request for mesos and Neil Conway.


Repository: mesos


Description (updated)
---

During the authorization phase, the resources have not been validated
not converted to the "post-reservation-refinement" format.
We can't rely on any operations that require valid resources and/or
"post-reservation-refinement" format. `operator<<` is one of those
functions, and here we print out the JSON representation of the
resources instead.


Diffs (updated)
-

  src/master/master.cpp ec594a8f4fa95e77fc38103c5561d1797fe2b133 


Diff: https://reviews.apache.org/r/60351/diff/1/


Testing
---


Thanks,

Michael Park

Re: Review Request 60279: Add constructor for ObjectApprover::Object.

[Greg Mann]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60279/#review178605
---




include/mesos/authorizer/authorizer.hpp
Lines 74 (patched)


We would prefer to accept these parameters by const reference, not as 
pointers. Also, for function parameters we use leading underscores instead of 
trailing underscores, like `_machine_id`.

So the constructors should look like: `Object(const MachineID& _machine_id)`


- Greg Mann


On June 21, 2017, 6:35 p.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60279/
> ---
> 
> (Updated June 21, 2017, 6:35 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Greg Mann, and Vinod Kone.
> 
> 
> Bugs: MESOS-7630
> https://issues.apache.org/jira/browse/MESOS-7630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add constructor for ObjectApprover::Object.
> 
> Add constructors for ObjectApprover::Object, making the caller side cleaner.
> Only one case is ignored, it's in "src/slave/slave.cpp:6709", since it's 
> setting parameters after the construction.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.hpp 
> 95cbcf3f9a962a896c9c23e961f02eb16dff36f8 
>   src/common/http.cpp 2f7718cbc2e449b4f7c89754e8f84eac2f3c35b6 
>   src/master/http.cpp 4dd43fd7c3fb986f4eed78bce574b6d3af156b67 
>   src/slave/http.cpp cbbc1dc27cc90bac8d48cbbc84266c3d87490a3c 
> 
> 
> Diff: https://reviews.apache.org/r/60279/diff/1/
> 
> 
> Testing
> ---
> 
> Passed "make check -j48"
> No specific test case related.
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Re: Review Request 60331: Windows: Disable code using `os::shell()`.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60331/
---

(Updated June 21, 2017, 11:52 p.m.)


Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Changes
---

Update after rebase so Review Bot can apply review.


Bugs: MESOS-6817, MESOS-7371 and MESOS-7407
https://issues.apache.org/jira/browse/MESOS-6817
https://issues.apache.org/jira/browse/MESOS-7371
https://issues.apache.org/jira/browse/MESOS-7407


Repository: mesos


Description
---

The disabled tests were already disabled, but at runtime instead of
compile time. Deleting `os::shell()` forces us to temporarily disable
the IP discovery command flag until we can move the implementation of
`subprocess()` into stout.


Diffs (updated)
-

  src/slave/main.cpp 66ae7db4039eedba7ff4eff51495384317c09354 
  src/tests/fetcher_tests.cpp 99149baa1c7abfabf572a0d0f4512a8e84d1e5be 


Diff: https://reviews.apache.org/r/60331/diff/2/

Changes: https://reviews.apache.org/r/60331/diff/1-2/


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 60252: Fixed a bug that causes segfault in ProcessManager::finalize.

[Mesos Reviewbot Windows]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60252/#review178603
---



Patch looks great!

Reviews applied: [60252]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On June 20, 2017, 11:16 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60252/
> ---
> 
> (Updated June 20, 2017, 11:16 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We don't need and shouldn't assume pointers in `processes` are
> non-nullptr.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> 7ce6d2b13baa68906e091a95c0dd58ee1ca2bc7d 
> 
> 
> Diff: https://reviews.apache.org/r/60252/diff/1/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 60345: Windows: Set Unicode compilation flags.

[Andrew Schwartzmeyer]

> On June 21, 2017, 11:05 p.m., Mesos Reviewbot wrote:
> > Bad patch!
> > 
> > Reviews applied: [60345, 60344, 60343, 60342, 60341, 60340, 60339, 60338, 
> > 60337, 60336, 60335, 60334, 60333, 60332, 60331, 60330, 60329, 60328, 
> > 60327, 60326, 60325, 60324, 60323, 60322, 60321, 60320, 60319, 60318, 
> > 60317, 60316, 60315, 60314, 60313, 60311, 60310, 60308, 60307, 60305, 
> > 60304, 60302, 60300, 60299, 60298, 60297, 60296, 60295, 60294, 60293, 
> > 60292, 60291]
> > 
> > Failed command: python support/apply-reviews.py -n -r 60331
> > 
> > Error:
> > 2017-06-21 22:53:24 URL:https://reviews.apache.org/r/60331/diff/raw/ 
> > [3996/3996] -> "60331.patch" [1]
> > error: patch failed: src/tests/fetcher_tests.cpp:650
> > error: src/tests/fetcher_tests.cpp: patch does not apply
> > 
> > Full log: https://builds.apache.org/job/Mesos-Reviewbot/18425/console

Fixing.


- Andrew


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60345/#review178587
---


On June 21, 2017, 10:17 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60345/
> ---
> 
> (Updated June 21, 2017, 10:17 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
> Joseph Wu, Li Li, and Michael Park.
> 
> 
> Bugs: MESOS-6817
> https://issues.apache.org/jira/browse/MESOS-6817
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Set Unicode compilation flags.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 3fa2e2f3b51a7a8dfe05085298f240019ff7b9c4 
> 
> 
> Diff: https://reviews.apache.org/r/60345/diff/1/
> 
> 
> Testing
> ---
> 
> # Windows
> 
> This is with Visual Studio 2017 tools, _without_ the long path registry key 
> set.
> 
> ## `stout-tests`
> ```
> [--] Global test environment tear-down
> [==] 253 tests from 42 test cases ran. (4910 ms total)
> [  PASSED  ] 253 tests.
> 
>   YOU HAVE 12 DISABLED TESTS
> ```
> 
> ## `libprocess-tests`
> ```
> [--] Global test environment tear-down
> [==] 148 tests from 27 test cases ran. (1232 ms total)
> [  PASSED  ] 148 tests.
> 
>   YOU HAVE 15 DISABLED TESTS
> ```
> 
> ## `mesos-tests`
> ```
> [--] Global test environment tear-down
> [==] 596 tests from 61 test cases ran. (115278 ms total)
> [  PASSED  ] 596 tests.
> 
>   YOU HAVE 166 DISABLED TESTS
> ```
> 
> # Linux
> 
> This is using `make check` on CentOS 7.
> 
> ## `stout-tests`
> ```
> [--] Global test environment tear-down
> [==] 310 tests from 50 test cases ran. (392 ms total)
> [  PASSED  ] 310 tests.
> ```
> 
> ## `libprocess-tests`
> ```
> [--] Global test environment tear-down
> [==] 193 tests from 33 test cases ran. (960 ms total)
> [  PASSED  ] 193 tests.
> 
>   YOU HAVE 2 DISABLED TESTS
> ```
> 
> ## `mesos-tests`
> ```
> [--] Global test environment tear-down
> [==] 1611 tests from 177 test cases ran. (481344 ms total)
> [  PASSED  ] 1611 tests.
> 
>   YOU HAVE 20 DISABLED TESTS
> ```
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 60104: Added rebooted flag to State.

[Megha Sharma]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60104/#review178601
---




src/slave/state.cpp
Line 96 (original), 95 (patched)


If this line is moved to 60105 then this patch will be independently 
mergeable.


- Megha Sharma


On June 21, 2017, 11:32 p.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60104/
> ---
> 
> (Updated June 21, 2017, 11:32 p.m.)
> 
> 
> Review request for mesos, Neil Conway, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6223
> https://issues.apache.org/jira/browse/MESOS-6223
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added rebooted flag to State to remember whether or not
> the agent has rebooted.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp f808458849bb9667a91abe18868751d377d36e0c 
>   src/slave/state.hpp 537358cb54393dd02b655050c26504808a1505ad 
>   src/slave/state.cpp 5dd8b1cc29cf1809e948015e146691596a8202b8 
> 
> 
> Diff: https://reviews.apache.org/r/60104/diff/6/
> 
> 
> Testing
> ---
> 
> Latest make check results pending.
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Re: Review Request 60232: Linted support/mesos-split.py.

[Joseph Wu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60232/#review178600
---


Fix it, then Ship it!




I'll fixup before committing.


support/mesos-split.py
Lines 41-45 (patched)


It would make more sense to all-CAPS these variables and keep them where 
they were.  Otherwise, we'll be re-initializing them for every single filename.



support/mesos-split.py
Lines 59-61 (patched)


Instead of explaining why we have a main method, it would be better to 
mention what it does, or what it expects.  For example:
```
Expects a list of filenames on the command line.

See `support/hooks/pre-commit` for the canonical usage of this method.
```


- Joseph Wu


On June 20, 2017, 3:15 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60232/
> ---
> 
> (Updated June 20, 2017, 3:15 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Joseph Wu.
> 
> 
> Bugs: MESOS-6390
> https://issues.apache.org/jira/browse/MESOS-6390
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This will allow us to use PyLint on the
> entire support directory in the future.
> 
> 
> Diffs
> -
> 
>   support/mesos-split.py 92084f15d125a4b4fb2369c385a74999efdf982c 
> 
> 
> Diff: https://reviews.apache.org/r/60232/diff/1/
> 
> 
> Testing
> ---
> 
> Added `support` to `source_dirs` in the PyLinter defined
> in `mesos-style.py`. Linted until only acceptable errors
> where displayed (e.g. due to the name of the file containing
> a dash instead of an underscore).
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 60231: Linted support/mesos-gtest-runner.py.

[Joseph Wu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60231/#review178598
---


Ship it!




Ship It!

- Joseph Wu


On June 20, 2017, 3:14 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60231/
> ---
> 
> (Updated June 20, 2017, 3:14 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Joseph Wu.
> 
> 
> Bugs: MESOS-6390
> https://issues.apache.org/jira/browse/MESOS-6390
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This will allow us to use PyLint on the
> entire support directory in the future.
> 
> 
> Diffs
> -
> 
>   support/mesos-gtest-runner.py 101dea8b6319c4f0106d7ace247554661cb6d248 
> 
> 
> Diff: https://reviews.apache.org/r/60231/diff/1/
> 
> 
> Testing
> ---
> 
> Added `support` to `source_dirs` in the PyLinter defined
> in `mesos-style.py`. Linted until only acceptable errors
> where displayed (e.g. due to the name of the file containing
> a dash instead of an underscore).
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 60104: Added rebooted flag to State.

[Megha Sharma]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60104/
---

(Updated June 21, 2017, 11:32 p.m.)


Review request for mesos, Neil Conway, Vinod Kone, and Jiang Yan Xu.


Summary (updated)
-

Added rebooted flag to State.


Bugs: MESOS-6223
https://issues.apache.org/jira/browse/MESOS-6223


Repository: mesos


Description (updated)
---

Added rebooted flag to State to remember whether or not
the agent has rebooted.


Diffs (updated)
-

  src/slave/slave.cpp f808458849bb9667a91abe18868751d377d36e0c 
  src/slave/state.hpp 537358cb54393dd02b655050c26504808a1505ad 
  src/slave/state.cpp 5dd8b1cc29cf1809e948015e146691596a8202b8 


Diff: https://reviews.apache.org/r/60104/diff/6/

Changes: https://reviews.apache.org/r/60104/diff/5-6/


Testing (updated)
---

Latest make check results pending.


Thanks,

Megha Sharma

Re: Review Request 60105: Clean rebooted slave's state if slaveInfo mismatches.

[Megha Sharma]

> On June 21, 2017, 4:39 p.m., Jiang Yan Xu wrote:
> > src/slave/slave.cpp
> > Lines 5798-5867 (original), 5798-5867 (patched)
> > 
> >
> > So all of this work is only useful for recovering frameworks, it looks 
> > like we don't need to do them before we decide we actually are going to 
> > recover the frameworks (i.e., not continue as a new agent)?
> > 
> > This reasoning certainly also applies to the current HEAD but since we 
> > are donig refactor here, we should probably address this (in a separate 
> > preceding review perhaps)

I strongly agree we should defer doing this work until we start to recover the 
frameworks. I was thinking of doing this may be as another review for slave 
recovery improvements after this change is in.


- Megha


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60105/#review178388
---


On June 21, 2017, 11:22 p.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60105/
> ---
> 
> (Updated June 21, 2017, 11:22 p.m.)
> 
> 
> Review request for mesos, Neil Conway, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6223
> https://issues.apache.org/jira/browse/MESOS-6223
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prior to Mesos 1.4 we bypass the state recovery and
> start as a new agent upon reboot. Starting in Mesos 1.4
> we'll attempt to recover the slave state even after reboot
> except for when there is a SlaveInfo mismatch.
> Here, we cleanup the slave state for a rebooted agent if
> there's been a SlaveInfo mismatch during recovery to
> ensure that no other state is recovered and the
> agent enventually registers as a new agent.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp f808458849bb9667a91abe18868751d377d36e0c 
> 
> 
> Diff: https://reviews.apache.org/r/60105/diff/5/
> 
> 
> Testing
> ---
> 
> Latest make check results pending.
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Re: Review Request 60230: Linted support/jsonurl.py.

[Joseph Wu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60230/#review178596
---


Ship it!




I can fix this up before committing.


support/jsonurl.py
Lines 37 (patched)


This comment doesn't seem right...



support/jsonurl.py
Line 13 (original), 38 (patched)


Since we're fixing these files up, might as well add a guard to this array 
access:
```
if len(sys.argv) < 2:
print >> sys.stderr, "USAGE: {} URL [KEY...]".format(sys.argv[0])
sys.exit(1)
```


- Joseph Wu


On June 20, 2017, 3:14 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60230/
> ---
> 
> (Updated June 20, 2017, 3:14 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Joseph Wu.
> 
> 
> Bugs: MESOS-6390
> https://issues.apache.org/jira/browse/MESOS-6390
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This will allow us to use PyLint on the
> entire support directory in the future.
> 
> 
> Diffs
> -
> 
>   support/jsonurl.py 72a91046c055831d469dde33b54d99a1b4170aef 
> 
> 
> Diff: https://reviews.apache.org/r/60230/diff/1/
> 
> 
> Testing
> ---
> 
> Added `support` to `source_dirs` in the PyLinter defined
> in `mesos-style.py`. Linted until only acceptable errors
> where displayed (e.g. due to the name of the file containing
> a dash instead of an underscore).
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 60105: Clean rebooted slave's state if slaveInfo mismatches.

[Megha Sharma]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60105/
---

(Updated June 21, 2017, 11:22 p.m.)


Review request for mesos, Neil Conway, Vinod Kone, and Jiang Yan Xu.


Summary (updated)
-

Clean rebooted slave's state if slaveInfo mismatches.


Bugs: MESOS-6223
https://issues.apache.org/jira/browse/MESOS-6223


Repository: mesos


Description
---

Prior to Mesos 1.4 we bypass the state recovery and
start as a new agent upon reboot. Starting in Mesos 1.4
we'll attempt to recover the slave state even after reboot
except for when there is a SlaveInfo mismatch.
Here, we cleanup the slave state for a rebooted agent if
there's been a SlaveInfo mismatch during recovery to
ensure that no other state is recovered and the
agent enventually registers as a new agent.


Diffs (updated)
-

  src/slave/slave.cpp f808458849bb9667a91abe18868751d377d36e0c 


Diff: https://reviews.apache.org/r/60105/diff/5/

Changes: https://reviews.apache.org/r/60105/diff/4-5/


Testing (updated)
---

Latest make check results pending.


Thanks,

Megha Sharma

Re: Review Request 60229: Linted support/generate-endpoint-help.py.

[Joseph Wu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60229/#review178582
---


Fix it, then Ship it!




I'll fixup before committing.


support/generate-endpoint-help.py
Lines 49-54 (original), 63-68 (patched)


The leading spaces in multi-line `"""` strings will be kept in the string.  
For markdown, this will generate some odd results, as a leading indent is 
sometimes how you start a code block.

We should go with:
```
MARKDOWN_HEADER = """---
title: %s
layout: documentation
---

"""
```
```



support/generate-endpoint-help.py
Lines 260-273 (original), 286-299 (patched)


This comment is indented unlike the others :)


- Joseph Wu


On June 20, 2017, 3:14 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60229/
> ---
> 
> (Updated June 20, 2017, 3:14 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Joseph Wu.
> 
> 
> Bugs: MESOS-6390
> https://issues.apache.org/jira/browse/MESOS-6390
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This will allow us to use PyLint on the
> entire support directory in the future.
> 
> 
> Diffs
> -
> 
>   support/generate-endpoint-help.py 6eb2d41b35c5f064839c29a0ba8326c590a33213 
> 
> 
> Diff: https://reviews.apache.org/r/60229/diff/1/
> 
> 
> Testing
> ---
> 
> Added `support` to `source_dirs` in the PyLinter defined
> in `mesos-style.py`. Linted until only acceptable errors
> where displayed (e.g. due to the name of the file containing
> a dash instead of an underscore).
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 60227: Linted support/mesos-style.py.

[Joseph Wu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60227/#review178589
---




support/mesos-style.py
Lines 305-306 (patched)


Oh, looks like this debug print snuck into the review.  I'll delete it 
before committing.


- Joseph Wu


On June 20, 2017, 8:41 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60227/
> ---
> 
> (Updated June 20, 2017, 8:41 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Joseph Wu.
> 
> 
> Bugs: MESOS-6390
> https://issues.apache.org/jira/browse/MESOS-6390
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This will allow us to use PyLint on the
> entire support directory in the future.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py f9c852806bfb2ead1552b5da4aa337edc5b9f947 
> 
> 
> Diff: https://reviews.apache.org/r/60227/diff/2/
> 
> 
> Testing
> ---
> 
> Added `support` to `source_dirs` in the PyLinter defined
> in `mesos-style.py`. Linted until only acceptable errors
> where displayed (e.g. due to the name of the file containing
> a dash instead of an underscore).
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 60105: Added helper method recoverSlaveState.

[Megha Sharma]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60105/
---

(Updated June 21, 2017, 11:11 p.m.)


Review request for mesos, Neil Conway, Vinod Kone, and Jiang Yan Xu.


Bugs: MESOS-6223
https://issues.apache.org/jira/browse/MESOS-6223


Repository: mesos


Description (updated)
---

Prior to Mesos 1.4 we bypass the state recovery and
start as a new agent upon reboot. Starting in Mesos 1.4
we'll attempt to recover the slave state even after reboot
except for when there is a SlaveInfo mismatch.
Here, we cleanup the slave state for a rebooted agent if
there's been a SlaveInfo mismatch during recovery to
ensure that no other state is recovered and the
agent enventually registers as a new agent.


Diffs (updated)
-

  src/slave/slave.cpp f808458849bb9667a91abe18868751d377d36e0c 


Diff: https://reviews.apache.org/r/60105/diff/4/

Changes: https://reviews.apache.org/r/60105/diff/3-4/


Testing
---


Thanks,

Megha Sharma

Re: Review Request 60280: Provide full path to the custom executor.

[Jie Yu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60280/#review178586
---




src/slave/containerizer/mesos/launch.cpp
Lines 671-674 (patched)


Sorry, I got confused. User should be the one setting argv[0]. So we don't 
need to change the code here.



src/slave/containerizer/mesos/launch.cpp
Lines 801 (patched)


OK, what if 'executable' has an absolute path?


- Jie Yu


On June 21, 2017, 10:26 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60280/
> ---
> 
> (Updated June 21, 2017, 10:26 p.m.)
> 
> 
> Review request for mesos, Jie Yu, James Peach, and Zhitao Li.
> 
> 
> Bugs: MESOS-7703
> https://issues.apache.org/jira/browse/MESOS-7703
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If a framework specifies use of its own executor and sets shell to false the 
> executor is never found. Additionally, the name of the binary is never passed 
> as an argument so executors making use of argv[0] will fail. This provides 
> the full path to the executor so that the `execvp` or `execvpe` is 
> successful. The name of the binary is also passed as the first argument for 
> cases where there is no shell used.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 162ca1c2e 
> 
> 
> Diff: https://reviews.apache.org/r/60280/diff/2/
> 
> 
> Testing
> ---
> 
> `cd build && cmake .. -DCMAKE_BUILD_TYPE=Release && make -j4`
> Also spun up a master and agent, connected and sent a task using the UCR 
> (both with and without the use of an OCI image) via our own framework, and 
> checked the sandbox to verify that things went accordingly.
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 60346: Improved the documentation of 'TASK_LOST'.

[Mesos Reviewbot Windows]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60346/#review178588
---



Bad patch!

Reviews applied: [60346]

Failed command: python support/apply-reviews.py -n -r 60346

Error:
Traceback (most recent call last):
  File "support/apply-reviews.py", line 381, in 
reviewboard()
  File "support/apply-reviews.py", line 360, in reviewboard
apply_review()
  File "support/apply-reviews.py", line 126, in apply_review
commit_patch()
  File "support/apply-reviews.py", line 225, in commit_patch
shell(cmd, options['dry_run'])
  File "support/apply-reviews.py", line 111, in shell
error_code = subprocess.call(command, stderr=subprocess.STDOUT, shell=True)
  File "C:\Python27\lib\subprocess.py", line 168, in call
return Popen(*popenargs, **kwargs).wait()
  File "C:\Python27\lib\subprocess.py", line 390, in __init__
errread, errwrite)
  File "C:\Python27\lib\subprocess.py", line 610, in _execute_child
args = '{} /c "{}"'.format (comspec, args)
UnicodeEncodeError: 'ascii' codec can't encode character u'\xf3' in position 
25: ordinal not in range(128)

Full log: http://mesos-winbot.westus.cloudapp.azure.com/logs/105/console

- Mesos Reviewbot Windows


On June 21, 2017, 3:17 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60346/
> ---
> 
> (Updated June 21, 2017, 3:17 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-7662
> https://issues.apache.org/jira/browse/MESOS-7662
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated the protobuf comments to clarify that 'TASK_LOST' is not
> always a terminal state.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 38449da44a5288afc7e2ef17ef5e1380ecabd5d2 
>   include/mesos/v1/mesos.proto 81eca47ce1074f5e522a0410f01ab2b2cfc76aa2 
> 
> 
> Diff: https://reviews.apache.org/r/60346/diff/1/
> 
> 
> Testing
> ---
> 
> None, this is not functional change.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 60345: Windows: Set Unicode compilation flags.

[Mesos Reviewbot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60345/#review178587
---



Bad patch!

Reviews applied: [60345, 60344, 60343, 60342, 60341, 60340, 60339, 60338, 
60337, 60336, 60335, 60334, 60333, 60332, 60331, 60330, 60329, 60328, 60327, 
60326, 60325, 60324, 60323, 60322, 60321, 60320, 60319, 60318, 60317, 60316, 
60315, 60314, 60313, 60311, 60310, 60308, 60307, 60305, 60304, 60302, 60300, 
60299, 60298, 60297, 60296, 60295, 60294, 60293, 60292, 60291]

Failed command: python support/apply-reviews.py -n -r 60331

Error:
2017-06-21 22:53:24 URL:https://reviews.apache.org/r/60331/diff/raw/ 
[3996/3996] -> "60331.patch" [1]
error: patch failed: src/tests/fetcher_tests.cpp:650
error: src/tests/fetcher_tests.cpp: patch does not apply

Full log: https://builds.apache.org/job/Mesos-Reviewbot/18425/console

- Mesos Reviewbot


On June 21, 2017, 10:17 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60345/
> ---
> 
> (Updated June 21, 2017, 10:17 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
> Joseph Wu, Li Li, and Michael Park.
> 
> 
> Bugs: MESOS-6817
> https://issues.apache.org/jira/browse/MESOS-6817
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Set Unicode compilation flags.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 3fa2e2f3b51a7a8dfe05085298f240019ff7b9c4 
> 
> 
> Diff: https://reviews.apache.org/r/60345/diff/1/
> 
> 
> Testing
> ---
> 
> # Windows
> 
> This is with Visual Studio 2017 tools, _without_ the long path registry key 
> set.
> 
> ## `stout-tests`
> ```
> [--] Global test environment tear-down
> [==] 253 tests from 42 test cases ran. (4910 ms total)
> [  PASSED  ] 253 tests.
> 
>   YOU HAVE 12 DISABLED TESTS
> ```
> 
> ## `libprocess-tests`
> ```
> [--] Global test environment tear-down
> [==] 148 tests from 27 test cases ran. (1232 ms total)
> [  PASSED  ] 148 tests.
> 
>   YOU HAVE 15 DISABLED TESTS
> ```
> 
> ## `mesos-tests`
> ```
> [--] Global test environment tear-down
> [==] 596 tests from 61 test cases ran. (115278 ms total)
> [  PASSED  ] 596 tests.
> 
>   YOU HAVE 166 DISABLED TESTS
> ```
> 
> # Linux
> 
> This is using `make check` on CentOS 7.
> 
> ## `stout-tests`
> ```
> [--] Global test environment tear-down
> [==] 310 tests from 50 test cases ran. (392 ms total)
> [  PASSED  ] 310 tests.
> ```
> 
> ## `libprocess-tests`
> ```
> [--] Global test environment tear-down
> [==] 193 tests from 33 test cases ran. (960 ms total)
> [  PASSED  ] 193 tests.
> 
>   YOU HAVE 2 DISABLED TESTS
> ```
> 
> ## `mesos-tests`
> ```
> [--] Global test environment tear-down
> [==] 1611 tests from 177 test cases ran. (481344 ms total)
> [  PASSED  ] 1611 tests.
> 
>   YOU HAVE 20 DISABLED TESTS
> ```
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 60105: Added helper method recoverSlaveState.

[Megha Sharma]

> On June 21, 2017, 4:39 p.m., Jiang Yan Xu wrote:
> > src/slave/slave.cpp
> > Lines 5945-5946 (patched)
> > 
> >
> > This comment seems to be misplaced, why does the code here cares? It 
> > just processes the `slaveState` which could be optional regardless of what 
> > we have done for the reboot case.

Some of the comments I added were for my own undrestanding (like this one) 
which I forgot to clean up before pushing


> On June 21, 2017, 4:39 p.m., Jiang Yan Xu wrote:
> > src/slave/slave.cpp
> > Line 5950 (original), 5969-5974 (patched)
> > 
> >
> > As suggested by related comments, we are not removing the symlink here 
> > so we shouldn't need to comment about it here. 
> > 
> > "This is being done ...": here the comment describes the low-level 
> > mechanics but perhaps we need a high level comment about "this" instead, 
> > which is that:
> > 
> > Prior to Mesos 1.4 we bypass the state recovery and start as a new 
> > agent upon reboot (introduced in MESOS-844); starting in Mesos 1.4 we'll 
> > attempt to recover the slave state even after reboot (so we don't discard 
> > the agent ID unnecessarily) but in case of SlaveInfo mismatch we'll still 
> > continue as a new agent so we don't introduce a regression for the reboot 
> > scenario.
> > 
> > We can go on to explain the rationale: agent resource and attribute 
> > changes are more often associated with host reboots and we don't want to 
> > agent to flap in such cases.
> > 
> > But it's fine if we focus on improving the comments after we first get 
> > the code rigth.

Makes sense, updated the comment.


> On June 21, 2017, 4:39 p.m., Jiang Yan Xu wrote:
> > src/slave/slave.cpp
> > Lines 5987-5989 (patched)
> > 
> >
> > You don't need to do this here. If you continue as a new agent, it's 
> > going to update the latest symlink when it's registered:
> > 
> > 
> > https://github.com/apache/mesos/blob/3bea3fcb4eef00ce469ba4f27fcfd2d3eec05aea/src/slave/paths.cpp#L621-L622

Good Point, removed


- Megha


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60105/#review178388
---


On June 15, 2017, 7:31 p.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60105/
> ---
> 
> (Updated June 15, 2017, 7:31 p.m.)
> 
> 
> Review request for mesos, Neil Conway, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6223
> https://issues.apache.org/jira/browse/MESOS-6223
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Here, a helper method recoverSlaveState is added to
> determine if the slave state is to be recovered or not.
> During agent recovery, if the SlaveInfo compatibility
> check fails and the agent is rebooted then to be
> backwards compatible with MESOS <= 1.3, since a
> a rebooted agent didn't recover state, we clear
> slave state and slaveId so the agent registers with the
> master as a new agent.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp 7ffaed14035a05259ec72c70532ee4f0affa1f5d 
>   src/slave/slave.cpp 7d147ac6609933ac884bfc29032dba572a0952c6 
> 
> 
> Diff: https://reviews.apache.org/r/60105/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Re: Review Request 60280: Provide full path to the custom executor.

[Aaron Wood via Review Board]

> On June 21, 2017, 9:54 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/launch.cpp
> > Lines 801 (patched)
> > 
> >
> > Why this is necessary? I think execvp will look into the current 
> > working directory?
> 
> Aaron Wood wrote:
> I found that this was not true. This is the real core of the fix. It 
> seems that exec will not look in the current directory, only in PATH.

`The file is sought in the colon-separated list of directory pathnames 
specified in the PATH environment variable. If this variable isn't defined, the 
path list defaults to the current directory followed by the list of directories 
returned by confstr(_CS_PATH). (This confstr(3) call typically returns the 
value "/bin:/usr/bin".)`


- Aaron


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60280/#review178562
---


On June 21, 2017, 10:26 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60280/
> ---
> 
> (Updated June 21, 2017, 10:26 p.m.)
> 
> 
> Review request for mesos, Jie Yu, James Peach, and Zhitao Li.
> 
> 
> Bugs: MESOS-7703
> https://issues.apache.org/jira/browse/MESOS-7703
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If a framework specifies use of its own executor and sets shell to false the 
> executor is never found. Additionally, the name of the binary is never passed 
> as an argument so executors making use of argv[0] will fail. This provides 
> the full path to the executor so that the `execvp` or `execvpe` is 
> successful. The name of the binary is also passed as the first argument for 
> cases where there is no shell used.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 162ca1c2e 
> 
> 
> Diff: https://reviews.apache.org/r/60280/diff/2/
> 
> 
> Testing
> ---
> 
> `cd build && cmake .. -DCMAKE_BUILD_TYPE=Release && make -j4`
> Also spun up a master and agent, connected and sent a task using the UCR 
> (both with and without the use of an OCI image) via our own framework, and 
> checked the sandbox to verify that things went accordingly.
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 60228: Linted support/apply-reviews.py.

[Joseph Wu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60228/#review178570
---


Fix it, then Ship it!





support/apply-reviews.py
Lines 173-176 (original), 208-211 (patched)


One indent too many here.



support/apply-reviews.py
Line 180 (original), 215 (patched)


This should have triple double-quotes.


- Joseph Wu


On June 20, 2017, 8:46 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60228/
> ---
> 
> (Updated June 20, 2017, 8:46 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Joseph Wu.
> 
> 
> Bugs: MESOS-6390
> https://issues.apache.org/jira/browse/MESOS-6390
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This will allow us to use PyLint on the
> entire support directory in the future.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py 0b1028c66e6b0ba8883d6f4580a95060445df7ac 
> 
> 
> Diff: https://reviews.apache.org/r/60228/diff/2/
> 
> 
> Testing
> ---
> 
> Added `support` to `source_dirs` in the PyLinter defined
> in `mesos-style.py`. Linted until only acceptable errors
> where displayed (e.g. due to the name of the file containing
> a dash instead of an underscore).
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 60280: Provide full path to the custom executor.

[Aaron Wood via Review Board]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60280/
---

(Updated June 21, 2017, 10:26 p.m.)


Review request for mesos, Jie Yu, James Peach, and Zhitao Li.


Bugs: MESOS-7703
https://issues.apache.org/jira/browse/MESOS-7703


Repository: mesos


Description
---

If a framework specifies use of its own executor and sets shell to false the 
executor is never found. Additionally, the name of the binary is never passed 
as an argument so executors making use of argv[0] will fail. This provides the 
full path to the executor so that the `execvp` or `execvpe` is successful. The 
name of the binary is also passed as the first argument for cases where there 
is no shell used.


Diffs (updated)
-

  src/slave/containerizer/mesos/launch.cpp 162ca1c2e 


Diff: https://reviews.apache.org/r/60280/diff/2/

Changes: https://reviews.apache.org/r/60280/diff/1-2/


Testing
---

`cd build && cmake .. -DCMAKE_BUILD_TYPE=Release && make -j4`
Also spun up a master and agent, connected and sent a task using the UCR (both 
with and without the use of an OCI image) via our own framework, and checked 
the sandbox to verify that things went accordingly.


Thanks,

Aaron Wood

Re: Review Request 60252: Fixed a bug that causes segfault in ProcessManager::finalize.

[Jiang Yan Xu]

> On June 21, 2017, 11:42 a.m., Joseph Wu wrote:
> > I agree this makes the finalization logic safer, but I don't see a way to 
> > add a `nullptr` to the `ProcessManager::processes` map.
> > 
> > `ProcessManager::spawn` is the only location where the `processes` map is 
> > inserted, and this method CHECK-fails if you give it a `nullptr`.  Two 
> > other locations use the map's `operator[]`, but those are guarded by a 
> > mutex so they will never empty-initialize a map entry.
> > 
> > Can you note how you ran into a segfault here?  It's more likely that the 
> > segfault was caused by a dereferencing a non-`nullptr` process that had 
> > already been deallocated (which would not be fixed by this review).

The use case is something like this: 
https://github.com/apache/mesos/blob/65152413836b58d01ace3a40bdc9056f9a489c6b/3rdparty/libprocess/src/tests/process_tests.cpp#L691

Arguably I made an error in doing:

```
ProcessBase process;
UPID pid = spawn(, true);

...

terminate(process);
wait(process);
```

And the process (in a test method) goes out of scope before libprocess 
finalize() so it segfaulted.

Just felt it's not really necessary to fail (and it used to not) in this way?


- Jiang Yan


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60252/#review178522
---


On June 20, 2017, 4:16 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60252/
> ---
> 
> (Updated June 20, 2017, 4:16 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We don't need and shouldn't assume pointers in `processes` are
> non-nullptr.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> 7ce6d2b13baa68906e091a95c0dd58ee1ca2bc7d 
> 
> 
> Diff: https://reviews.apache.org/r/60252/diff/1/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 60345: Windows: Set Unicode compilation flags.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60345/
---

(Updated June 21, 2017, 10:17 p.m.)


Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817
https://issues.apache.org/jira/browse/MESOS-6817


Repository: mesos


Description
---

Windows: Set Unicode compilation flags.


Diffs
-

  cmake/CompilationConfigure.cmake 3fa2e2f3b51a7a8dfe05085298f240019ff7b9c4 


Diff: https://reviews.apache.org/r/60345/diff/1/


Testing (updated)
---

# Windows

This is with Visual Studio 2017 tools, _without_ the long path registry key set.

## `stout-tests`
```
[--] Global test environment tear-down
[==] 253 tests from 42 test cases ran. (4910 ms total)
[  PASSED  ] 253 tests.

  YOU HAVE 12 DISABLED TESTS
```

## `libprocess-tests`
```
[--] Global test environment tear-down
[==] 148 tests from 27 test cases ran. (1232 ms total)
[  PASSED  ] 148 tests.

  YOU HAVE 15 DISABLED TESTS
```

## `mesos-tests`
```
[--] Global test environment tear-down
[==] 596 tests from 61 test cases ran. (115278 ms total)
[  PASSED  ] 596 tests.

  YOU HAVE 166 DISABLED TESTS
```

# Linux

This is using `make check` on CentOS 7.

## `stout-tests`
```
[--] Global test environment tear-down
[==] 310 tests from 50 test cases ran. (392 ms total)
[  PASSED  ] 310 tests.
```

## `libprocess-tests`
```
[--] Global test environment tear-down
[==] 193 tests from 33 test cases ran. (960 ms total)
[  PASSED  ] 193 tests.

  YOU HAVE 2 DISABLED TESTS
```

## `mesos-tests`
```
[--] Global test environment tear-down
[==] 1611 tests from 177 test cases ran. (481344 ms total)
[  PASSED  ] 1611 tests.

  YOU HAVE 20 DISABLED TESTS
```


Thanks,

Andrew Schwartzmeyer

Review Request 60346: Improved the documentation of 'TASK_LOST'.

[Gastón Kleiman]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60346/
---

Review request for mesos and Neil Conway.


Bugs: MESOS-7662
https://issues.apache.org/jira/browse/MESOS-7662


Repository: mesos


Description
---

Updated the protobuf comments to clarify that 'TASK_LOST' is not
always a terminal state.


Diffs
-

  include/mesos/mesos.proto 38449da44a5288afc7e2ef17ef5e1380ecabd5d2 
  include/mesos/v1/mesos.proto 81eca47ce1074f5e522a0410f01ab2b2cfc76aa2 


Diff: https://reviews.apache.org/r/60346/diff/1/


Testing
---

None, this is not functional change.


Thanks,

Gastón Kleiman

Review Request 60341: Windows: Changed job object names to `wstring`.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60341/
---

Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817
https://issues.apache.org/jira/browse/MESOS-6817


Repository: mesos


Description
---

Windows: Changed job object names to `wstring`.


Diffs
-

  3rdparty/libprocess/include/process/windows/jobobject.hpp 
5fb41c41b3f6242ed04a3274b9a3f27f705db26e 


Diff: https://reviews.apache.org/r/60341/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 60342: Windows: Made version info use Unicode.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60342/
---

Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817
https://issues.apache.org/jira/browse/MESOS-6817


Repository: mesos


Description
---

The `GetVersionEx` family of functions have been marked deprecated; not
just the non-Unicode version as the previous comment implied. Thus we do
not hide the valid deprecation warning, and should replace the use of
this API.


Diffs
-

  3rdparty/stout/include/stout/windows/os.hpp 
f1722d5e18af97d89c6ca7aaf002fd58cb0d21ae 


Diff: https://reviews.apache.org/r/60342/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 60345: Windows: Set Unicode compilation flags.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60345/
---

Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817
https://issues.apache.org/jira/browse/MESOS-6817


Repository: mesos


Description
---

Windows: Set Unicode compilation flags.


Diffs
-

  cmake/CompilationConfigure.cmake 3fa2e2f3b51a7a8dfe05085298f240019ff7b9c4 


Diff: https://reviews.apache.org/r/60345/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 60280: Provide full path to the custom executor.

[Aaron Wood via Review Board]

> On June 21, 2017, 7:28 p.m., Aaron Wood wrote:
> > Someone correct me if I'm wrong, but I don't think we need this anymore:
> > ```
> >   if (launchInfo.has_working_directory()) {
> > Try chdir = os::chdir(launchInfo.working_directory());
> > if (chdir.isError()) {
> >   cerr << "Failed to chdir into current working directory "
> ><< "'" << launchInfo.working_directory() << "': "
> ><< chdir.error() << endl;
> >   exitWithStatus(EXIT_FAILURE);
> > }
> >   }
> > ```
> > Everyone okay with me removing this?
> 
> Jie Yu wrote:
> No, we cannot. We do want the process's cwd to be inside the sandbox.

Okay, will keep it as is then.


- Aaron


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60280/#review178534
---


On June 21, 2017, 7:15 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60280/
> ---
> 
> (Updated June 21, 2017, 7:15 p.m.)
> 
> 
> Review request for mesos, Jie Yu, James Peach, and Zhitao Li.
> 
> 
> Bugs: MESOS-7703
> https://issues.apache.org/jira/browse/MESOS-7703
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If a framework specifies use of its own executor and sets shell to false the 
> executor is never found. Additionally, the name of the binary is never passed 
> as an argument so executors making use of argv[0] will fail. This provides 
> the full path to the executor so that the `execvp` or `execvpe` is 
> successful. The name of the binary is also passed as the first argument for 
> cases where there is no shell used.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 162ca1c2e 
> 
> 
> Diff: https://reviews.apache.org/r/60280/diff/1/
> 
> 
> Testing
> ---
> 
> `cd build && cmake .. -DCMAKE_BUILD_TYPE=Release && make -j4`
> Also spun up a master and agent, connected and sent a task using the UCR 
> (both with and without the use of an OCI image) via our own framework, and 
> checked the sandbox to verify that things went accordingly.
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Review Request 60340: Windows: Made job object APIs use Unicode.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60340/
---

Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817
https://issues.apache.org/jira/browse/MESOS-6817


Repository: mesos


Description
---

Changed the name to be a `wstring` for easier use.


Diffs
-

  3rdparty/stout/include/stout/os/windows/killtree.hpp 
267a9a0db6c5e077f0da617c572884ea86b26932 
  3rdparty/stout/include/stout/windows/os.hpp 
f1722d5e18af97d89c6ca7aaf002fd58cb0d21ae 


Diff: https://reviews.apache.org/r/60340/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 60344: Windows: Require building with `_UNICODE` and `UNICODE`.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60344/
---

Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817
https://issues.apache.org/jira/browse/MESOS-6817


Repository: mesos


Description
---

Windows: Require building with `_UNICODE` and `UNICODE`.


Diffs
-

  3rdparty/stout/include/stout/windows.hpp 
2fcf27f627e19d82f99547417bd6578817624127 


Diff: https://reviews.apache.org/r/60344/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 60343: Windows: Made `process_entry()` use Unicode.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60343/
---

Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817
https://issues.apache.org/jira/browse/MESOS-6817


Repository: mesos


Description
---

Windows: Made `process_entry()` use Unicode.


Diffs
-

  3rdparty/stout/include/stout/windows/os.hpp 
f1722d5e18af97d89c6ca7aaf002fd58cb0d21ae 


Diff: https://reviews.apache.org/r/60343/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 60339: Windows: Deleted `os::hstrerror()` as it is unused.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60339/
---

Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817
https://issues.apache.org/jira/browse/MESOS-6817


Repository: mesos


Description
---

Windows: Deleted `os::hstrerror()` as it is unused.


Diffs
-

  3rdparty/stout/include/stout/windows/os.hpp 
f1722d5e18af97d89c6ca7aaf002fd58cb0d21ae 
  3rdparty/stout/tests/os/strerror_tests.cpp 
f69a4d3dc2f2ecfdbad7fb82510ffbf0d04f6afb 


Diff: https://reviews.apache.org/r/60339/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 60280: Provide full path to the custom executor.

[Aaron Wood via Review Board]

> On June 21, 2017, 9:54 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/launch.cpp
> > Lines 801 (patched)
> > 
> >
> > Why this is necessary? I think execvp will look into the current 
> > working directory?

I found that this was not true. This is the real core of the fix. It seems that 
exec will not look in the current directory, only in PATH.


- Aaron


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60280/#review178562
---


On June 21, 2017, 7:15 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60280/
> ---
> 
> (Updated June 21, 2017, 7:15 p.m.)
> 
> 
> Review request for mesos, Jie Yu, James Peach, and Zhitao Li.
> 
> 
> Bugs: MESOS-7703
> https://issues.apache.org/jira/browse/MESOS-7703
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If a framework specifies use of its own executor and sets shell to false the 
> executor is never found. Additionally, the name of the binary is never passed 
> as an argument so executors making use of argv[0] will fail. This provides 
> the full path to the executor so that the `execvp` or `execvpe` is 
> successful. The name of the binary is also passed as the first argument for 
> cases where there is no shell used.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 162ca1c2e 
> 
> 
> Diff: https://reviews.apache.org/r/60280/diff/1/
> 
> 
> Testing
> ---
> 
> `cd build && cmake .. -DCMAKE_BUILD_TYPE=Release && make -j4`
> Also spun up a master and agent, connected and sent a task using the UCR 
> (both with and without the use of an OCI image) via our own framework, and 
> checked the sandbox to verify that things went accordingly.
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Review Request 60337: Windows: Made `os::user()` use Unicode.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60337/
---

Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817
https://issues.apache.org/jira/browse/MESOS-6817


Repository: mesos


Description
---

Windows: Made `os::user()` use Unicode.


Diffs
-

  3rdparty/stout/include/stout/os/windows/su.hpp 
573fdefdf38ae8560059c6a0c512ef561ca617b2 


Diff: https://reviews.apache.org/r/60337/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 60338: Windows: Made `dynamiclibrary.hpp` use Unicode.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60338/
---

Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817
https://issues.apache.org/jira/browse/MESOS-6817


Repository: mesos


Description
---

Windows: Made `dynamiclibrary.hpp` use Unicode.


Diffs
-

  3rdparty/stout/include/stout/windows/dynamiclibrary.hpp 
28b8e32ecc6748449d2623637e58bab25b2a6590 


Diff: https://reviews.apache.org/r/60338/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 60336: Windows: Made environment abstractions use Unicode.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60336/
---

Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817
https://issues.apache.org/jira/browse/MESOS-6817


Repository: mesos


Description
---

Windows: Made environment abstractions use Unicode.


Diffs
-

  3rdparty/stout/include/stout/os/windows/environment.hpp 
96f0eb4f56a73f4058eeaf68502e441bd00147a8 
  3rdparty/stout/include/stout/os/windows/getenv.hpp 
2200f8527da0c8d5a5ffee287eb86e4d0cb58b8b 
  3rdparty/stout/include/stout/windows/os.hpp 
f1722d5e18af97d89c6ca7aaf002fd58cb0d21ae 


Diff: https://reviews.apache.org/r/60336/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 60335: Windows: Made `net.hpp` use Unicode.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60335/
---

Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817
https://issues.apache.org/jira/browse/MESOS-6817


Repository: mesos


Description
---

This commit moves most of the shared `os::net` code into similar but
separate POSIX and Windows implementations, where the latter use the
explicit wide / Unicode Windows APIs.


Diffs
-

  3rdparty/stout/include/stout/net.hpp 425544e1d428b1c65a15221c34c9d29e7dc31f65 
  3rdparty/stout/include/stout/posix/net.hpp 
59463a05a62559cea3964c2b058a2205db24fa9b 
  3rdparty/stout/include/stout/windows/net.hpp 
987ab7075fe3c1127cab7e14831ca22030c35e45 


Diff: https://reviews.apache.org/r/60335/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 60334: Windows: Made `WindowsError` use Unicode.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60334/
---

Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817
https://issues.apache.org/jira/browse/MESOS-6817


Repository: mesos


Description
---

Windows: Made `WindowsError` use Unicode.


Diffs
-

  3rdparty/stout/include/stout/windows/error.hpp 
2f2cbd4d40275495561be4c9391b16cfa82762dd 


Diff: https://reviews.apache.org/r/60334/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 60331: Windows: Disable code using `os::shell()`.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60331/
---

(Updated June 21, 2017, 10:13 p.m.)


Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817, MESOS-7371 and MESOS-7407
https://issues.apache.org/jira/browse/MESOS-6817
https://issues.apache.org/jira/browse/MESOS-7371
https://issues.apache.org/jira/browse/MESOS-7407


Repository: mesos


Description
---

The disabled tests were already disabled, but at runtime instead of
compile time. Deleting `os::shell()` forces us to temporarily disable
the IP discovery command flag until we can move the implementation of
`subprocess()` into stout.


Diffs
-

  src/slave/main.cpp 66ae7db4039eedba7ff4eff51495384317c09354 
  src/tests/fetcher_tests.cpp b4124158a0e92f289f0edc0c8cb9394350e1cbb5 


Diff: https://reviews.apache.org/r/60331/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 60332: Windows: Fixed `fs::usage()` to support long paths.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60332/
---

(Updated June 21, 2017, 10:13 p.m.)


Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817, MESOS-7371 and MESOS-7407
https://issues.apache.org/jira/browse/MESOS-6817
https://issues.apache.org/jira/browse/MESOS-7371
https://issues.apache.org/jira/browse/MESOS-7407


Repository: mesos


Description
---

Windows: Fixed `fs::usage()` to support long paths.


Diffs
-

  3rdparty/stout/include/stout/windows/fs.hpp 
7c3413b2399d90be3739eaeeb50e0ebf3abac04c 


Diff: https://reviews.apache.org/r/60332/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 60331: Windows: Disable code using `os::shell()`.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60331/
---

Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817, MESOS-7371 and MESOS-7407
https://issues.apache.org/jira/browse/MESOS-6817
https://issues.apache.org/jira/browse/MESOS-7371
https://issues.apache.org/jira/browse/MESOS-7407


Repository: mesos


Description
---

The disabled tests were already disabled, but at runtime instead of
compile time. Deleting `os::shell()` forces us to temporarily disable
the IP discovery command flag until we can move the implementation of
`subprocess()` into stout.


Diffs
-

  src/slave/main.cpp 66ae7db4039eedba7ff4eff51495384317c09354 
  src/tests/fetcher_tests.cpp b4124158a0e92f289f0edc0c8cb9394350e1cbb5 


Diff: https://reviews.apache.org/r/60331/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 60330: Windows: Fixed use of `get_system_env()` in containerizer.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60330/
---

Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817, MESOS-7371 and MESOS-7407
https://issues.apache.org/jira/browse/MESOS-6817
https://issues.apache.org/jira/browse/MESOS-7371
https://issues.apache.org/jira/browse/MESOS-7407


Repository: mesos


Description
---

Windows: Fixed use of `get_system_env()` in containerizer.


Diffs
-

  src/slave/containerizer/docker.cpp 5cd3b6d95ff78fb114a06d49122b7161a6688646 
  src/slave/containerizer/mesos/launch.cpp 
162ca1c2ee5e5ef2a901f5ecca5016c933710541 


Diff: https://reviews.apache.org/r/60330/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 60327: Added `map<string, string>` explict cast to `Envp`.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60327/
---

Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817, MESOS-7371 and MESOS-7407
https://issues.apache.org/jira/browse/MESOS-6817
https://issues.apache.org/jira/browse/MESOS-7371
https://issues.apache.org/jira/browse/MESOS-7407


Repository: mesos


Description
---

This is needed for implementations that expect a `map`
over a `char **`.


Diffs
-

  3rdparty/stout/include/stout/os/raw/environment.hpp 
c7f889f0caa78d9f82dc9337f2bcf205d5a0174a 


Diff: https://reviews.apache.org/r/60327/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 60332: Windows: Fixed `fs::usage()` to support long paths.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60332/
---

Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817, MESOS-7371 and MESOS-7407
https://issues.apache.org/jira/browse/MESOS-6817
https://issues.apache.org/jira/browse/MESOS-7371
https://issues.apache.org/jira/browse/MESOS-7407


Repository: mesos


Description
---

Windows: Fixed `fs::usage()` to support long paths.


Diffs
-

  3rdparty/stout/include/stout/windows/fs.hpp 
7c3413b2399d90be3739eaeeb50e0ebf3abac04c 


Diff: https://reviews.apache.org/r/60332/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 60329: Windows: Replaced `CreateProcess()` with `create_process()` helper.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60329/
---

Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817, MESOS-7371 and MESOS-7407
https://issues.apache.org/jira/browse/MESOS-6817
https://issues.apache.org/jira/browse/MESOS-7371
https://issues.apache.org/jira/browse/MESOS-7407


Repository: mesos


Description
---

This uses the new `ProcessData create_process()` abstraction added to
stout, reducing the use of the Windows API in libprocess. The
`ProcessData` struct eliminates the need to manually call
`CloseHandle()`.


Diffs
-

  3rdparty/libprocess/cmake/ProcessConfigure.cmake 
2c46d43ed8418f128f12fad10151700611e81b1e 
  3rdparty/libprocess/include/process/subprocess_base.hpp 
a3edae6c37e5336912d92b406d934c58bdf386d9 
  3rdparty/libprocess/include/process/windows/subprocess.hpp 
5459955fb3b7b07b976ece61bf59bc237896e2de 
  3rdparty/libprocess/src/subprocess.cpp 
0f1532b294d6d6b1e017468cfde47362f3faa84d 


Diff: https://reviews.apache.org/r/60329/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 60328: Windows: Added `create_process()` wrapper to `shell.hpp`.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60328/
---

Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817, MESOS-7371 and MESOS-7407
https://issues.apache.org/jira/browse/MESOS-6817
https://issues.apache.org/jira/browse/MESOS-7371
https://issues.apache.org/jira/browse/MESOS-7407


Repository: mesos


Description
---

This removes the use of unsupported CRT functions from the `os::`
namespace, and reimplements them using `CreateProcess()`.

The CRT APIs behave unpredictably in the context of long paths, and are
not necessary. Furthermore, it is not possible to use the Visual Studio
Child Process Debugger for processes spawned by any API other than
`CreateProcess()`.

Some unused functions were explicitly deleted to avoid future bugs.


Diffs
-

  3rdparty/stout/cmake/StoutConfigure.cmake 
6b1d27fc53e49ac3378c17017c72ecfb2e3739ac 
  3rdparty/stout/include/stout/os/windows/shell.hpp 
b93f33771302ebbb1ce0c8515a3101a33f9956f5 
  3rdparty/stout/tests/os/process_tests.cpp 
09f749528fa34935a7ac69921a6d7acc73af16e8 
  3rdparty/stout/tests/os_tests.cpp 7f785b07feaed28e272b4195362eb83842813e2b 


Diff: https://reviews.apache.org/r/60328/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 60326: Added `vector` explicit cast to `Argv`.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60326/
---

Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817, MESOS-7371 and MESOS-7407
https://issues.apache.org/jira/browse/MESOS-6817
https://issues.apache.org/jira/browse/MESOS-7371
https://issues.apache.org/jira/browse/MESOS-7407


Repository: mesos


Description
---

This is needed for implementations that expect a `vector`
instead of a `char **`.


Diffs
-

  3rdparty/stout/include/stout/os/raw/argv.hpp 
58e5ee2a0ae6a68b3221124f196f0e650c32a8c0 


Diff: https://reviews.apache.org/r/60326/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 60325: Windows: Replaced `ping` with `Start-Sleep`.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60325/
---

Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-4467
https://issues.apache.org/jira/browse/MESOS-4467


Repository: mesos


Description
---

This `SLEEP_COMMAND` macro represents a command that should sleep. On
POSIX systems, a `sleep` binary is available, but on Windows, `sleep` is
a `cmd.exe` feature that requires a TTY. Previously, this was replaced
with `ping` as it approximates sleep, except that it produces noisy
output. We replace it with the `Start-Sleep` PowerShell command, which
does not require a TTY, and is always available.


Diffs
-

  3rdparty/stout/include/stout/gtest.hpp 
a004a378cb467495234d77a0c56fbea6e7bec420 


Diff: https://reviews.apache.org/r/60325/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 60324: Windows: Implemented `WSTRINGIFY()` to print exit codes.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60324/
---

(Updated June 21, 2017, 10:08 p.m.)


Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-5417
https://issues.apache.org/jira/browse/MESOS-5417


Repository: mesos


Description
---

Windows: Implemented `WSTRINGIFY()` to print exit codes.


Diffs
-

  src/common/status_utils.hpp 36df84d0112dd824e001df010a5f4ccbf2229006 


Diff: https://reviews.apache.org/r/60324/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 60324: Windows: Implemented `WSTRINGIFY()` to print exit codes.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60324/
---

(Updated June 21, 2017, 10:08 p.m.)


Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-5417
https://issues.apache.org/jira/browse/MESOS-5417


Repository: mesos


Description
---

Windows: Implemented `WSTRINGIFY()` to print exit codes.


Diffs
-

  src/common/status_utils.hpp 36df84d0112dd824e001df010a5f4ccbf2229006 


Diff: https://reviews.apache.org/r/60324/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 60324: Windows: Implemented `WSTRINGIFY()` to print exit codes.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60324/
---

Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817, MESOS-7371 and MESOS-7407
https://issues.apache.org/jira/browse/MESOS-6817
https://issues.apache.org/jira/browse/MESOS-7371
https://issues.apache.org/jira/browse/MESOS-7407


Repository: mesos


Description
---

Windows: Implemented `WSTRINGIFY()` to print exit codes.


Diffs
-

  src/common/status_utils.hpp 36df84d0112dd824e001df010a5f4ccbf2229006 


Diff: https://reviews.apache.org/r/60324/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 60322: Windows: Fixed `os::rename()` to support long paths.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60322/
---

Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817, MESOS-7371 and MESOS-7407
https://issues.apache.org/jira/browse/MESOS-6817
https://issues.apache.org/jira/browse/MESOS-7371
https://issues.apache.org/jira/browse/MESOS-7407


Repository: mesos


Description
---

Windows: Fixed `os::rename()` to support long paths.


Diffs
-

  3rdparty/stout/include/stout/os/windows/rename.hpp 
544ff90df8828196f8188877a8e7f8aca8b6445e 


Diff: https://reviews.apache.org/r/60322/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 60321: Windows: Fixed `fs::list()` to support long paths.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60321/
---

Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817, MESOS-7371 and MESOS-7407
https://issues.apache.org/jira/browse/MESOS-6817
https://issues.apache.org/jira/browse/MESOS-7371
https://issues.apache.org/jira/browse/MESOS-7407


Repository: mesos


Description
---

Windows: Fixed `fs::list()` to support long paths.


Diffs
-

  3rdparty/stout/include/stout/windows/fs.hpp 
7c3413b2399d90be3739eaeeb50e0ebf3abac04c 


Diff: https://reviews.apache.org/r/60321/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 60323: Windows: Fixed `process::createIoPath()` to support long paths.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60323/
---

Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817, MESOS-7371 and MESOS-7407
https://issues.apache.org/jira/browse/MESOS-6817
https://issues.apache.org/jira/browse/MESOS-7371
https://issues.apache.org/jira/browse/MESOS-7407


Repository: mesos


Description
---

Windows: Fixed `process::createIoPath()` to support long paths.


Diffs
-

  3rdparty/libprocess/src/subprocess_windows.cpp 
cc71fbd7a43c88a24602540cd65e2073473ddb2c 


Diff: https://reviews.apache.org/r/60323/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 60318: Windows: Made `os::temp()` use UTF-16 system calls.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60318/
---

Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817, MESOS-7371 and MESOS-7407
https://issues.apache.org/jira/browse/MESOS-6817
https://issues.apache.org/jira/browse/MESOS-7371
https://issues.apache.org/jira/browse/MESOS-7407


Repository: mesos


Description
---

Windows: Made `os::temp()` use UTF-16 system calls.


Diffs
-

  3rdparty/stout/include/stout/os/windows/temp.hpp 
639f1aafbc3de3aa7d4752ad89a6f585dc23846d 


Diff: https://reviews.apache.org/r/60318/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 60320: Windows: Cleaned up `os::var()`.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60320/
---

Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817, MESOS-7371 and MESOS-7407
https://issues.apache.org/jira/browse/MESOS-6817
https://issues.apache.org/jira/browse/MESOS-7371
https://issues.apache.org/jira/browse/MESOS-7407


Repository: mesos


Description
---

Windows: Cleaned up `os::var()`.


Diffs
-

  3rdparty/stout/include/stout/windows/os.hpp 
f1722d5e18af97d89c6ca7aaf002fd58cb0d21ae 


Diff: https://reviews.apache.org/r/60320/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 60317: Windows: Fixed `os::access()` to support long paths.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60317/
---

(Updated June 21, 2017, 10:05 p.m.)


Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817, MESOS-7371 and MESOS-7407
https://issues.apache.org/jira/browse/MESOS-6817
https://issues.apache.org/jira/browse/MESOS-7371
https://issues.apache.org/jira/browse/MESOS-7407


Repository: mesos


Description
---

Windows: Fixed `os::access()` to support long paths.


Diffs
-

  3rdparty/stout/include/stout/os/access.hpp 
d87762a97152d55c58f6e54a9560c5fa0d051473 
  3rdparty/stout/include/stout/windows.hpp 
2fcf27f627e19d82f99547417bd6578817624127 


Diff: https://reviews.apache.org/r/60317/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 60317: Windows: Fixed `os::access()` to support long paths.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60317/
---

Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817, MESOS-7371 and MESOS-7407
https://issues.apache.org/jira/browse/MESOS-6817
https://issues.apache.org/jira/browse/MESOS-7371
https://issues.apache.org/jira/browse/MESOS-7407


Repository: mesos


Description
---

Windows: Fixed `os::access()` to support long paths.


Diffs
-

  3rdparty/stout/include/stout/os/access.hpp 
d87762a97152d55c58f6e54a9560c5fa0d051473 
  3rdparty/stout/include/stout/windows.hpp 
2fcf27f627e19d82f99547417bd6578817624127 


Diff: https://reviews.apache.org/r/60317/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 60314: Windows: Fixed `os::rmdir()` to support long paths.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60314/
---

Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817, MESOS-7371 and MESOS-7407
https://issues.apache.org/jira/browse/MESOS-6817
https://issues.apache.org/jira/browse/MESOS-7371
https://issues.apache.org/jira/browse/MESOS-7407


Repository: mesos


Description
---

Windows: Fixed `os::rmdir()` to support long paths.


Diffs
-

  3rdparty/stout/include/stout/os/windows/rmdir.hpp 
05eceea94d2be71173584c71570f5f2f95fab744 


Diff: https://reviews.apache.org/r/60314/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 60315: Windows: Fixed `os::open()` to support long paths.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60315/
---

Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817, MESOS-7371 and MESOS-7407
https://issues.apache.org/jira/browse/MESOS-6817
https://issues.apache.org/jira/browse/MESOS-7371
https://issues.apache.org/jira/browse/MESOS-7407


Repository: mesos


Description
---

The CRT library can support long paths on Windows, if the Unicode
versions of the APIs are used, and the long path prefix is prepended to
the path.


Diffs
-

  3rdparty/stout/include/stout/os/open.hpp 
ceff89f0c9f265d410078c7e6e6bbc230ce8cf98 


Diff: https://reviews.apache.org/r/60315/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 60313: Windows: Fixed `os::rm()` to support long paths.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60313/
---

Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817, MESOS-7371 and MESOS-7407
https://issues.apache.org/jira/browse/MESOS-6817
https://issues.apache.org/jira/browse/MESOS-7371
https://issues.apache.org/jira/browse/MESOS-7407


Repository: mesos


Description
---

Windows: Fixed `os::rm()` to support long paths.


Diffs
-

  3rdparty/stout/include/stout/os/windows/rm.hpp 
6460b526fd81389653d93ea222c1d7f3e6bbc22f 


Diff: https://reviews.apache.org/r/60313/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 60311: Windows: Replaced use of `_stat()` with `os::stat::isdir()`.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60311/
---

Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817, MESOS-7371 and MESOS-7407
https://issues.apache.org/jira/browse/MESOS-6817
https://issues.apache.org/jira/browse/MESOS-7371
https://issues.apache.org/jira/browse/MESOS-7407


Repository: mesos


Description
---

This removes hairy, duplicated code which did not support long paths.


Diffs
-

  3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp 
a9228437bc1b2b287dd19cb9903ce048db2a90ef 


Diff: https://reviews.apache.org/r/60311/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 60310: Fixed fetcher to use `enum class FollowSymlink`.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60310/
---

Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817, MESOS-7371 and MESOS-7407
https://issues.apache.org/jira/browse/MESOS-6817
https://issues.apache.org/jira/browse/MESOS-7371
https://issues.apache.org/jira/browse/MESOS-7407


Repository: mesos


Description
---

Fixed fetcher to use `enum class FollowSymlink`.


Diffs
-

  src/slave/containerizer/fetcher.cpp 770cad3e046e8a6d58b6bc9176eb7ecdbd340db4 


Diff: https://reviews.apache.org/r/60310/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 60290: Handle EINVAL from the capabilities version check.

[Jie Yu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60290/#review178568
---


Ship it!




Ship It!

- Jie Yu


On June 21, 2017, 8:52 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60290/
> ---
> 
> (Updated June 21, 2017, 8:52 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Bannier, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> capget will fail with the error EINVAL, and set the version field
> to the kernel preferred value of _LINUX_CAPABILITY_VERSION_?
> when an unsupported version value is specified.
> 
> So if we get EINVAL, we can treat that as success because we can
> continue to verify the capabilities version.
> 
> 
> Diffs
> -
> 
>   src/linux/capabilities.cpp b362759c06637a3a2b4359b95e86d6d3b87352dd 
> 
> 
> Diff: https://reviews.apache.org/r/60290/diff/2/
> 
> 
> Testing
> ---
> 
> make check (CentOS 6)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Review Request 60308: Refactored `enum FollowSymlink` to an `enum class FollowSymlink`.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60308/
---

Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817, MESOS-7371 and MESOS-7407
https://issues.apache.org/jira/browse/MESOS-6817
https://issues.apache.org/jira/browse/MESOS-7371
https://issues.apache.org/jira/browse/MESOS-7407


Repository: mesos


Description
---

Using `enum class` is preferred as it provides stronger type-safety.

The `enum class FollowSymlink` definition was duplicated into
the POSIX and Windows `stat.hpp` headers to avoid a circular
dependency otherwise caused by `reparsepoint.hpp`.


Diffs
-

  3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp 
a9228437bc1b2b287dd19cb9903ce048db2a90ef 
  3rdparty/stout/include/stout/os/posix/stat.hpp 
2762c41fdebc28c412126e39029cd03345677a73 
  3rdparty/stout/include/stout/os/stat.hpp 
d002c98ac3afbcdc8886a8ba6657919d011f2b46 
  3rdparty/stout/include/stout/os/windows/stat.hpp 
b2ff43696a6b4172a2ebcf08ae36aa9e143ad101 
  3rdparty/stout/tests/os_tests.cpp 7f785b07feaed28e272b4195362eb83842813e2b 


Diff: https://reviews.apache.org/r/60308/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 60227: Linted support/mesos-style.py.

[Joseph Wu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60227/#review178566
---


Ship it!




Ship It!

- Joseph Wu


On June 20, 2017, 8:41 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60227/
> ---
> 
> (Updated June 20, 2017, 8:41 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Joseph Wu.
> 
> 
> Bugs: MESOS-6390
> https://issues.apache.org/jira/browse/MESOS-6390
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This will allow us to use PyLint on the
> entire support directory in the future.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py f9c852806bfb2ead1552b5da4aa337edc5b9f947 
> 
> 
> Diff: https://reviews.apache.org/r/60227/diff/2/
> 
> 
> Testing
> ---
> 
> Added `support` to `source_dirs` in the PyLinter defined
> in `mesos-style.py`. Linted until only acceptable errors
> where displayed (e.g. due to the name of the file containing
> a dash instead of an underscore).
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Review Request 60307: Windows: Fixed `CreateSymbolicLink()` to support long paths.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60307/
---

Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817, MESOS-7371 and MESOS-7407
https://issues.apache.org/jira/browse/MESOS-6817
https://issues.apache.org/jira/browse/MESOS-7371
https://issues.apache.org/jira/browse/MESOS-7407


Repository: mesos


Description
---

Windows: Fixed `CreateSymbolicLink()` to support long paths.


Diffs
-

  3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp 
a9228437bc1b2b287dd19cb9903ce048db2a90ef 


Diff: https://reviews.apache.org/r/60307/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 60305: Windows: Refactored `reparse_point_attribute_set()`.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60305/
---

Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817, MESOS-7371 and MESOS-7407
https://issues.apache.org/jira/browse/MESOS-6817
https://issues.apache.org/jira/browse/MESOS-7371
https://issues.apache.org/jira/browse/MESOS-7407


Repository: mesos


Description
---

It now takes a `std::wstring` and uses `get_file_attributes()`.


Diffs
-

  3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp 
a9228437bc1b2b287dd19cb9903ce048db2a90ef 
  3rdparty/stout/include/stout/internal/windows/symlink.hpp 
5dc022b22a73efc2d868d01ce0a27c93bd9148f2 


Diff: https://reviews.apache.org/r/60305/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 60288: Fix ambient capability tests on older systems.

[Jie Yu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60288/#review178565
---


Ship it!




Ship It!

- Jie Yu


On June 21, 2017, 8:25 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60288/
> ---
> 
> (Updated June 21, 2017, 8:25 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Bannier, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If the kernel doesn't support ambinent capabilities we were leaving
> stale capabilities in the ambient set from a precious check, causing
> the later tests to fail.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/capabilities_tests.cpp 
> 3070e1d57c2f211c46c0435349ac352bc875ee27 
> 
> 
> Diff: https://reviews.apache.org/r/60288/diff/1/
> 
> 
> Testing
> ---
> 
> make check (CentOS 6)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Review Request 60300: Windows: Fixed `isdir()` and `isfile()` to support long paths.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60300/
---

Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817, MESOS-7371 and MESOS-7407
https://issues.apache.org/jira/browse/MESOS-6817
https://issues.apache.org/jira/browse/MESOS-7371
https://issues.apache.org/jira/browse/MESOS-7407


Repository: mesos


Description
---

Now uses Windows file attributes instead of `::_stat()`.


Diffs
-

  3rdparty/stout/include/stout/os/windows/stat.hpp 
b2ff43696a6b4172a2ebcf08ae36aa9e143ad101 


Diff: https://reviews.apache.org/r/60300/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 60299: Windows: Fixed `os::exists()` to support long paths.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60299/
---

Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817, MESOS-7371 and MESOS-7407
https://issues.apache.org/jira/browse/MESOS-6817
https://issues.apache.org/jira/browse/MESOS-7371
https://issues.apache.org/jira/browse/MESOS-7407


Repository: mesos


Description
---

Windows: Fixed `os::exists()` to support long paths.


Diffs
-

  3rdparty/stout/include/stout/os/windows/exists.hpp 
42b8a9d6cca7e13c8feac997f5d6de8cdcc007f4 


Diff: https://reviews.apache.org/r/60299/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 60287: Remove unnecessary test logging.

[Jie Yu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60287/#review178564
---


Ship it!




Ship It!

- Jie Yu


On June 21, 2017, 8:24 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60287/
> ---
> 
> (Updated June 21, 2017, 8:24 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Bannier, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The glog expectations already print the capability sets, so we don't
> need to add them to the failure log again.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/capabilities_tests.cpp 
> 3070e1d57c2f211c46c0435349ac352bc875ee27 
> 
> 
> Diff: https://reviews.apache.org/r/60287/diff/1/
> 
> 
> Testing
> ---
> 
> make check (CentOS 6)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Review Request 60296: Windows: Added long path test.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60296/
---

Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817, MESOS-7371 and MESOS-7407
https://issues.apache.org/jira/browse/MESOS-6817
https://issues.apache.org/jira/browse/MESOS-7371
https://issues.apache.org/jira/browse/MESOS-7407


Repository: mesos


Description
---

Windows: Added long path test.


Diffs
-

  3rdparty/stout/tests/os/filesystem_tests.cpp 
ee5a1a1f7e1aa4e2838a7df8b39266d5bdcb3db7 


Diff: https://reviews.apache.org/r/60296/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 60280: Provide full path to the custom executor.

[Jie Yu]

> On June 21, 2017, 7:28 p.m., Aaron Wood wrote:
> > Someone correct me if I'm wrong, but I don't think we need this anymore:
> > ```
> >   if (launchInfo.has_working_directory()) {
> > Try chdir = os::chdir(launchInfo.working_directory());
> > if (chdir.isError()) {
> >   cerr << "Failed to chdir into current working directory "
> ><< "'" << launchInfo.working_directory() << "': "
> ><< chdir.error() << endl;
> >   exitWithStatus(EXIT_FAILURE);
> > }
> >   }
> > ```
> > Everyone okay with me removing this?

No, we cannot. We do want the process's cwd to be inside the sandbox.


- Jie


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60280/#review178534
---


On June 21, 2017, 7:15 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60280/
> ---
> 
> (Updated June 21, 2017, 7:15 p.m.)
> 
> 
> Review request for mesos, Jie Yu, James Peach, and Zhitao Li.
> 
> 
> Bugs: MESOS-7703
> https://issues.apache.org/jira/browse/MESOS-7703
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If a framework specifies use of its own executor and sets shell to false the 
> executor is never found. Additionally, the name of the binary is never passed 
> as an argument so executors making use of argv[0] will fail. This provides 
> the full path to the executor so that the `execvp` or `execvpe` is 
> successful. The name of the binary is also passed as the first argument for 
> cases where there is no shell used.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 162ca1c2e 
> 
> 
> Diff: https://reviews.apache.org/r/60280/diff/1/
> 
> 
> Testing
> ---
> 
> `cd build && cmake .. -DCMAKE_BUILD_TYPE=Release && make -j4`
> Also spun up a master and agent, connected and sent a task using the UCR 
> (both with and without the use of an OCI image) via our own framework, and 
> checked the sandbox to verify that things went accordingly.
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 60280: Provide full path to the custom executor.

[Jie Yu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60280/#review178562
---




src/slave/containerizer/mesos/launch.cpp
Lines 671-674 (patched)


I would do the following to make it more symmetric:
```
vector args;
if (launchInfo.command().shell()) {
  args = {
  os::Shell::arg0,
  os::Shell::arg1,
  launchInfo.command().value()
  };
} else {
  args = { launchInfo.command().value().c_str()) };
  
  args.insert(
  args.end(),
  launchInfo.command().arguments().begin(),
  launchInfo.command().arguments().end());
}

os::raw::Argv argv(args);
```



src/slave/containerizer/mesos/launch.cpp
Lines 801 (patched)


Why this is necessary? I think execvp will look into the current working 
directory?


- Jie Yu


On June 21, 2017, 7:15 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60280/
> ---
> 
> (Updated June 21, 2017, 7:15 p.m.)
> 
> 
> Review request for mesos, Jie Yu, James Peach, and Zhitao Li.
> 
> 
> Bugs: MESOS-7703
> https://issues.apache.org/jira/browse/MESOS-7703
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If a framework specifies use of its own executor and sets shell to false the 
> executor is never found. Additionally, the name of the binary is never passed 
> as an argument so executors making use of argv[0] will fail. This provides 
> the full path to the executor so that the `execvp` or `execvpe` is 
> successful. The name of the binary is also passed as the first argument for 
> cases where there is no shell used.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 162ca1c2e 
> 
> 
> Diff: https://reviews.apache.org/r/60280/diff/1/
> 
> 
> Testing
> ---
> 
> `cd build && cmake .. -DCMAKE_BUILD_TYPE=Release && make -j4`
> Also spun up a master and agent, connected and sent a task using the UCR 
> (both with and without the use of an OCI image) via our own framework, and 
> checked the sandbox to verify that things went accordingly.
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Review Request 60294: Windows: Added `os::LONGPATH_PREFIX` constant.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60294/
---

Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817, MESOS-7371 and MESOS-7407
https://issues.apache.org/jira/browse/MESOS-6817
https://issues.apache.org/jira/browse/MESOS-7371
https://issues.apache.org/jira/browse/MESOS-7407


Repository: mesos


Description
---

The string `\\?\` is the prefix used by Windows to indicate the path may
be longer than `MAX_PATH`.


Diffs
-

  3rdparty/stout/include/stout/os/constants.hpp 
f20041ce8b7be5821d48b4a7d3c2fc8834fbad18 


Diff: https://reviews.apache.org/r/60294/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 59930: Support RO mode for bind mount volumes with filesystem/linux isolator

[Charles Raimbert]

> On June 10, 2017, 8:09 a.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/isolators/filesystem/linux.cpp
> > Lines 607 (patched)
> > 
> >
> > I found this might be related. Are we able to test on Debian Wheezy?

Yes exactly, from the link you pointed out: "the read-only attribute can only 
be added with a remount operation afterward".

I have tested on Debian Jessie only.


- Charles


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59930/#review177543
---


On June 8, 2017, 11:06 p.m., Charles Raimbert wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59930/
> ---
> 
> (Updated June 8, 2017, 11:06 p.m.)
> 
> 
> Review request for mesos, Jason Lai, Jie Yu, and Zhitao Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The filesystem/linux isolator currently creates all bind mount volumes as RW, 
> even if a volume mode is set as RO.
> 
> The TODO in the isolator code helps to spot the missing capability:
> https://github.com/apache/mesos/blob/master/src/slave/containerizer/mesos/isolators/filesystem/linux.cpp#L587
> `
> // TODO(jieyu): Consider the mode in the volume.
> `
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> 69804eec61467ae0fd95dfdf53a08875e27a0bca 
>   src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
> 803758c1437b21df9f25ad7e994298e89de44cbe 
> 
> 
> Diff: https://reviews.apache.org/r/59930/diff/1/
> 
> 
> Testing
> ---
> 
> Test cases for RW and RO bind mount volumes have been added, by following the 
> filesystem/linux isolator's existing testing pattern.
> 
> 
> Thanks,
> 
> Charles Raimbert
> 
>

Re: Review Request 60293: Windows: Switched from `gethostname` to `GetComputerName`.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60293/
---

(Updated June 21, 2017, 9:53 p.m.)


Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.


Bugs: MESOS-6817 and MESOS-7548
https://issues.apache.org/jira/browse/MESOS-6817
https://issues.apache.org/jira/browse/MESOS-7548


Repository: mesos


Description
---

The former can inordinately slow on Windows (about 5 seconds),
whereas the latter is guaranteed to return instantly.

Because `os::internal::nodename()` and `net::hostname()` implemented the
exact same feature, we just reuse the first in the second.


Diffs
-

  3rdparty/stout/include/stout/net.hpp 425544e1d428b1c65a15221c34c9d29e7dc31f65 
  3rdparty/stout/include/stout/posix/net.hpp 
59463a05a62559cea3964c2b058a2205db24fa9b 
  3rdparty/stout/include/stout/windows/net.hpp 
987ab7075fe3c1127cab7e14831ca22030c35e45 
  3rdparty/stout/include/stout/windows/os.hpp 
f1722d5e18af97d89c6ca7aaf002fd58cb0d21ae 


Diff: https://reviews.apache.org/r/60293/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 60292: Windows: Added a `wstring wide_stringify(string)` helper.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60292/
---

(Updated June 21, 2017, 9:53 p.m.)


Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817, MESOS-7371 and MESOS-7407
https://issues.apache.org/jira/browse/MESOS-6817
https://issues.apache.org/jira/browse/MESOS-7371
https://issues.apache.org/jira/browse/MESOS-7407


Repository: mesos


Description
---

When dealing with Windows APIs, often `string` -> `wstring` conversions
are required. This addition makes the process cleaner.

Also added explicit errors messages, which are written to the buffer
instead of throwing an exception if an error occurs.


Diffs
-

  3rdparty/stout/include/stout/stringify.hpp 
44da506e5eb3c04f6727683ccc346e08f128d265 


Diff: https://reviews.apache.org/r/60292/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 60293: Windows: Switched from `gethostname` to `GetComputerName`.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60293/
---

Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.


Bugs: MESOS-6817 and MESOS-7548
https://issues.apache.org/jira/browse/MESOS-6817
https://issues.apache.org/jira/browse/MESOS-7548


Repository: mesos


Description
---

The former can inordinately slow on Windows (about 5 seconds),
whereas the latter is guaranteed to return instantly.

Because `os::internal::nodename()` and `net::hostname()` implemented the
exact same feature, we just reuse the first in the second.


Diffs
-

  3rdparty/stout/include/stout/net.hpp 425544e1d428b1c65a15221c34c9d29e7dc31f65 
  3rdparty/stout/include/stout/posix/net.hpp 
59463a05a62559cea3964c2b058a2205db24fa9b 
  3rdparty/stout/include/stout/windows/net.hpp 
987ab7075fe3c1127cab7e14831ca22030c35e45 
  3rdparty/stout/include/stout/windows/os.hpp 
f1722d5e18af97d89c6ca7aaf002fd58cb0d21ae 


Diff: https://reviews.apache.org/r/60293/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 60292: Windows: Added a `wstring wide_stringify(string)` helper.

[Andrew Schwartzmeyer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60292/
---

Review request for mesos, Alexander Rukletsov, Jeff Coffler, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


Bugs: MESOS-6817, MESOS-7371 and MESOS-7407
https://issues.apache.org/jira/browse/MESOS-6817
https://issues.apache.org/jira/browse/MESOS-7371
https://issues.apache.org/jira/browse/MESOS-7407


Repository: mesos


Description
---

When dealing with Windows APIs, often `string` -> `wstring` conversions
are required. This addition makes the process cleaner.

Also added explicit errors messages, which are written to the buffer
instead of throwing an exception if an error occurs.


Diffs
-

  3rdparty/stout/include/stout/stringify.hpp 
44da506e5eb3c04f6727683ccc346e08f128d265 


Diff: https://reviews.apache.org/r/60292/diff/1/


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 60124: Added a name to ballon framework to distinguish between instances.

[Joseph Wu]

> On June 15, 2017, 12:07 p.m., Armand Grillet wrote:
> > src/examples/balloon_framework.cpp
> > Line 464 (original), 469 (patched)
> > 
> >
> > Consistency wise, other tests have just `executor.set_name("Framework 
> > Name Executor (C++)");` to set the name of the executor (e.g. in 
> > `long_lived_framework.cpp`). More generally, what is the reason to add a 
> > `name` flag for this test?
> 
> Alexander Rukletsov wrote:
> If we have multiple instance of the same framework, it can be difficult 
> to distinguish between them in the WebUI, hence this change. I agree, this is 
> not only relevant to this framework, but actually to all example frameworks.

You might as well let the flag control the entire name, rather than appending 
`--name` to the end of `Balloon Framework`.  Or update the flag description to 
mention that it will be appended to the canonical framework name.


- Joseph


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60124/#review178030
---


On June 15, 2017, 10:09 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60124/
> ---
> 
> (Updated June 15, 2017, 10:09 a.m.)
> 
> 
> Review request for mesos, Armand Grillet and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/examples/balloon_framework.cpp bfaae3796afb426459ef5cff3a37634a8242032e 
> 
> 
> Diff: https://reviews.apache.org/r/60124/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 60123: Minor clean up of the balloon framework.

[Joseph Wu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60123/#review178558
---


Ship it!




Ship It!

- Joseph Wu


On June 20, 2017, 6:17 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60123/
> ---
> 
> (Updated June 20, 2017, 6:17 a.m.)
> 
> 
> Review request for mesos, Armand Grillet and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/examples/balloon_framework.cpp bfaae3796afb426459ef5cff3a37634a8242032e 
> 
> 
> Diff: https://reviews.apache.org/r/60123/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 60122: Printed the reason why offer is declined in disk full framework.

[Joseph Wu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60122/#review178557
---


Ship it!




Ship It!

- Joseph Wu


On June 15, 2017, 10:08 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60122/
> ---
> 
> (Updated June 15, 2017, 10:08 a.m.)
> 
> 
> Review request for mesos, Armand Grillet and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/examples/disk_full_framework.cpp 
> a73e6cf285b9a74492d476a624915235e079051f 
> 
> 
> Diff: https://reviews.apache.org/r/60122/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 59464: Add Fetcher task total and failed fetch metrics.

[Joseph Wu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59464/#review178543
---


Ship it!




LGTM.  I can add the comment mentioned below before committing.


src/slave/containerizer/fetcher.hpp
Lines 327-328 (patched)


It's important enough to add a comment here about the counter resolution.  
This number will go up once per task, rather than once per artifact.

So if 1 task asks for 10 artifacts, the total number of fetches is 1.  If 1 
to 10 of those artifacts fail to fetch, the total number of failures is also 1.



src/slave/containerizer/fetcher.cpp
Lines 68-78 (original), 70-78 (patched)


This change isn't strictly related to the metrics, so it could be moved 
elsewhere.


- Joseph Wu


On June 9, 2017, 11:55 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59464/
> ---
> 
> (Updated June 9, 2017, 11:55 a.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-7524
> https://issues.apache.org/jira/browse/MESOS-7524
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add the Fetcher metrics to track the number of fetch requests
> sent to the Fetcher (`containerizer/fetcher/task_fetches_total`)
> and the number of errors reported by the Fetcher
> (`containerizer/fetcher/task_fetches_failed`).
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/fetcher.hpp 
> efeadbf4b7804ea4c1e443d1e5212e303796ace4 
>   src/slave/containerizer/fetcher.cpp 
> 770cad3e046e8a6d58b6bc9176eb7ecdbd340db4 
> 
> 
> Diff: https://reviews.apache.org/r/59464/diff/4/
> 
> 
> Testing
> ---
> 
> make check (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>