Hi All,
TLDR: We currently use "mesos::internal" namespace for almost everything
inside src/. However, in most cases, it is directly enclosing another
namespace. This makes the "internal" namespace redundant and we should
kill it.
I learned from Ben Hindman that the original motivation for introducing an
explicit internal namespace was to discourage people from exposing internal
symbols through "extern", etc. Since we don't seem to expose symbols
through "extern" in our codebase, I think it's safe to kill this namespace.
Here is a list of files that define classes directly in the mesos::internal
namespace (i.e. without enclosing a separate namespace) [1]:
authorizer/authorizer.hpp
common/http.hpp
common/attributes.hpp
common/lock.hpp
files/files.hpp
hook/manager.hpp
master/contender.hpp
master/detector.hpp
usage/usage.hpp
watcher/whitelist_watcher.hpp
messages/messages.pb.h
Of the above list, things like hook/manager.hpp and
master/{contender,detector}.hpp can be moved into their own namespaces. I
am sure, we can come up with a strategy for the rest as well.
One possible concern here might be messages.proto and the effects on
upgrades, etc., if we change the namespace/package for these protobufs. If
this turns out to be a real concern, we can possibly keep the internal
namespace for messages.proto.
If we kill the "internal" namespace altogether, it would make it much
easier to expose some headers as public headers for modularization, etc..
This will also help us get rid of "namespace internal" from some of the
public headers that we already have.
The motivation for killing the "internal" namespace comes from a
patch-set[2] that tries to expose slave/containerizer/isolator.hpp as
include/mesos/slave/isolator.hpp. We wanted to keep Isolator inside the
"mesos::slave" namespace instead of putting it directly in the "mesos"
namespace. This change causes a lot of conflicts due to "mesos::slave" and
"mesos::internal::slave" and we had to resort to using fully qualified
names for a large number of definition in src/slave, src/tests, etc.
Best,
Kapil
[1] List obtained by running the following command and then filtering out
the instances where "internal" was enclosed in something other than "mesos":
grep -nr "} // namespace internal" * -B1 |grep -v
"namespace\|\-\-\|\.cpp"|cut -d'-' -f1
[2] https://reviews.apache.org/r/30238/
https://reviews.apache.org/r/29602/
https://reviews.apache.org/r/29603/
