Remove all DEPS files under //mojo.

Remove all DEPS files under //mojo.

In Mojo, we are using "gn check" as the primary mechanism for checking validity
of includes. "gn check" is run over //mojo.

R=jamesr@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/5ddeb46264fe8ba308a36dd3e69b110a42aa591f

[blundell, 2015-01-23 13:25:11]

blundell@chromium.org changed reviewers:
+ jamesr@chromium.org

[blundell, 2015-01-23 13:25:12]

https://codereview.chromium.org/872783002/diff/1/DEPS
File DEPS (right):

https://codereview.chromium.org/872783002/diff/1/DEPS#newcode208
DEPS:208: # include_rules in Mojo are more restrictive than in Chromium, so
checkdeps
FYI, the specific problem here is the removal of "library_loaders" as a DEPS
entry that is allowed for everyone; I assume that that removal was intentional?

[jamesr, 2015-01-23 16:44:35]

Since we have gn check I don't really care about checkdeps.

[blundell, 2015-01-26 16:38:24]

On 2015/01/23 16:44:35, jamesr wrote:
> Since we have gn check I don't really care about checkdeps.

I'm not super-familiar with "gn check". It checks that the graph of dependencies
in the build system properly reflects includes, correct?

I think that that people have internalized checkdeps and DEPS files as the means
of specifying what code in a given directory is *allowed* to depend on. -People
could be trained to apply the same scrutiny to deps additions in BUILD.gn files.
There are a few features of DEPS files that I really like though:

- The ability to specify "!" dependencies that will raise a warning when new
includes are added
- The ability to specify "-" dependencies that serve to explicitly forbid
certain dependencies from being added that might naively seem safe. Otherwise
people have to keep these kinds of rules in their heads, which seems a little
dangerous to me especially since we don't use OWNERS in Mojo.

It sounds like you think the benefits of maintaining DEPS files aren't worth the
overhead?

[jamesr, 2015-01-26 23:58:28]

Both DEPS and gn check look at your #includes and decide if they are good or
not.  DEPS works by comparing the strings in the #include path against the
strings in DEPS in the ancestors to see if there are any matches.  It doesn't
know anything about the build graph or include paths, so it just does substring
comparisons.

gn check compares includes to the actual build graph. it can figure out the
actual target an #include is coming from and is being included from and compare
them.  This lets it check files in the same directory and files that use
different include path conventions.

We have to maintain the build graph in order to build.  It's a lot of overhead
to maintain a sparse subset of the build graph in DEPS files especially since it
can't really understand paths, and it doesn't generate anything positive for us
other than the thing that 'gn check' can do more precisely.

The '-' syntax in DEPS is really a way to hack around its lack of understanding,
generally.  In gn this can be better accomplished by creating additional targets
and controlling visibility to match what's actually happening.

The '!' syntax is nice for long-lived cleanups, but we don't have a big enough
repo for that to be a real issue.  If we want to remove #includes from something
we can just do it.

[jamesr, 2015-01-27 00:02:09]

The other issue with checkdeps is that it's really slow, so it's not run by
default as part of the edit/compile/test cycle.  gn check is fast enough to run
whenever we edit the build files so it actually stays green.

[blundell, 2015-01-27 09:01:45]

On 2015/01/26 23:58:28, jamesr wrote:
> Both DEPS and gn check look at your #includes and decide if they are good or
> not.  DEPS works by comparing the strings in the #include path against the
> strings in DEPS in the ancestors to see if there are any matches.  It doesn't
> know anything about the build graph or include paths, so it just does
substring
> comparisons.
> 
> gn check compares includes to the actual build graph. it can figure out the
> actual target an #include is coming from and is being included from and
compare
> them.  This lets it check files in the same directory and files that use
> different include path conventions.
> 
> We have to maintain the build graph in order to build.  It's a lot of overhead
> to maintain a sparse subset of the build graph in DEPS files especially since
it
> can't really understand paths, and it doesn't generate anything positive for
us
> other than the thing that 'gn check' can do more precisely.
> 
> The '-' syntax in DEPS is really a way to hack around its lack of
understanding,
> generally.  In gn this can be better accomplished by creating additional
targets
> and controlling visibility to match what's actually happening.
> 

This all sounds good; there's basically just one thing I'm having trouble
grokking.
Is there a way to express constraints like "the Mojo public SDK shouldn't depend
on //base"
or "services' client libs should depend only on the public SDK and other
interfaces/client libs"
succinctly via GN (outside of the work we've been doing in using templates and
presubmit
checks)? 

> The '!' syntax is nice for long-lived cleanups, but we don't have a big enough
> repo for that to be a real issue.  If we want to remove #includes from
something
> we can just do it.

[jamesr, 2015-01-27 09:13:22]

Code review

[blundell, 2015-01-28 16:00:44]

OK, PTAL then ;)

[jamesr, 2015-01-28 16:06:58]

not lgtm

checkdeps still needs to pass in chromium when we roll the sdk so we can't
delete those DEPS files.

[blundell, 2015-01-28 16:07:58]

On 2015/01/28 16:06:58, jamesr wrote:
> not lgtm
> 
> checkdeps still needs to pass in chromium when we roll the sdk so we can't
> delete those DEPS files.

d'oh, right.

[blundell, 2015-01-28 16:09:55]

Actually none of //third_party is run through checkdeps in Chromium so once we
have all the Mojo code getting pulled under //third_party we can do this.

[blundell, 2015-02-02 20:39:44]

We should be good to go on this now that all code from //mojo is pulled under
//third_party in Chromium.

[jamesr, 2015-02-02 20:42:42]

lgtm

[blundell, 2015-02-03 08:37:41]

Committed patchset #3 (id:40001) manually as
5ddeb46264fe8ba308a36dd3e69b110a42aa591f (presubmit successful).