----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45940/#review127916 -----------------------------------------------------------
Ship it! Ship It! - Michael Park On April 8, 2016, 7:59 p.m., Alex Clemmer wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45940/ > ----------------------------------------------------------- > > (Updated April 8, 2016, 7:59 p.m.) > > > Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, > Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun. > > > Repository: mesos > > > Description > ------- > > `qos_controllers/load.cpp` currently includes `stout/posix/os.hpp` > directly, which includes `killtree`, which includes `os.hpp`. When we > move `os::libraries` back to `os.hpp`, this will cause a problem, > because it will mean that `os::setenv` (which is defined further down in > `stout/posix/os.hpp` is not defined before > `os::libraries::Library::setPaths`, which uses it. > > In general, this problem is usually caused by including something like > `stout/posix/os.hpp` instead of `os.hpp`. The choice to include such a > header is somewhat fraught, because many files in `os.hpp` (particularly > those that are included into `os.hpp` via a derivative, like > `posix/os.hpp`) have implicit dependencies on functions they `#include` > being defined before their code is defined. This issue will probably get > worse in the future. Furthermore, it is worth noting that the plan is > to, at some point, remove `os.hpp` altogether. Depending on the specific > implementation, this could make the "transitive #include problem" either > dramatically worse, or much better. This is a discussion the community > needs to have more braodly. > > This commit will resovle the compile problem by removing `load.cpp`'s > explicit dependency on `os/os.hpp` and `posix/os.hpp`, and replacing > them with plain-old `stout/os.hpp`. As noted, however, this is really an > immediate solution to a larger problem, which we don't attempt to fully > address here. > > > Diffs > ----- > > src/slave/qos_controllers/load.cpp dd44f9209ad283bfea95f16a8c1017e309757f23 > > Diff: https://reviews.apache.org/r/45940/diff/ > > > Testing > ------- > > > Thanks, > > Alex Clemmer > >
