PSA/request for advice: moving libvpx out of DEPS

[Johann Koenig]

I'd like to move libvpx from DEPS to the main src tree since there is

no technical reason for it to be there anymore and lots of reason for

it not to be:

https://code.google.com/p/chromium/issues/detail?id=481034

https://codereview.chromium.org/1158913006/

However, the move is doing some weird to the paths:

http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/69687/steps/checkdeps%20%28with%20patch%29/logs/stdio

ERROR in /b/build/slave/linux/build/src/third_party/libvpx/source/config/linux/arm/vp9_rtcd.h

  Illegal include: "vpx/vpx_integer.h"

    Because of no rule applying.

The includes *appear* to be set up correctly as the build is working

for all the targets that I've run. I can't figure out how to run

checkdeps.py because I don't know what's in the json input files.

In theory no paths have changed. I guessed that it might want full

paths instead of relative paths (third_party/libvpx/source/libvpx/ in

the case above, as opposed to source/libvpx/) but changing those

causes the build to fail (and I don't know if it fixes the deps script

because I'm not sure how to run it)

Any ideas?

[Scott Graham]

Yes, I believe that's it. It's checking vs the include_rules DEPS entries in src/DEPS, src/third_party/DEPS, etc.

You should be able run the checks with `git cl presubmit` locally.

 

Any ideas?

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

[Brett Wilson]

The checkdeps script should require no input:

  python buildtools/checkdeps/checkdeps.py

I'm guessing that checkdeps has a thing where it doesn't check DEPSed in directories, so was skipping your directory before.

Brett

On Fri, May 29, 2015 at 11:59 AM, Johann Koenig <johann...@google.com> wrote:

--

[Johann Koenig]

On Fri, May 29, 2015 at 12:09 PM, Scott Graham <sco...@chromium.org> wrote:
> Yes, I believe that's it. It's checking vs the include_rules DEPS entries in
> src/DEPS, src/third_party/DEPS, etc.
>
> You should be able run the checks with `git cl presubmit` locally.

Indeed I can, thanks! The presubmit doesn't like the a chunk of the
code in our googletest directory. I can probably make deleting the
directory part of the import, but is there a way to suppress warnings
like this:
New code should not use wstrings. If you are calling a cross-platform
API that accepts a wstring, fix the API.
third_party/libvpx/source/libvpx/third_party/googletest/src/include/gtest/gtest.h:152
third_party/libvpx/source/libvpx/third_party/googletest/src/include/gtest/gtest.h:160

Banned functions were used.
third_party/libvpx/source/libvpx/third_party/googletest/src/include/gtest/gtest.h:17031:
Chromium code should not use gtest's FRIEND_TEST() macro. Include
base/gtest_prod_util.h and use FRIEND_TEST_ALL_PREFIXES() instead.

On Fri, May 29, 2015 at 12:10 PM, Brett Wilson <bre...@chromium.org> wrote:
> The checkdeps script should require no input:
> python buildtools/checkdeps/checkdeps.py

Also convenient, thanks!

> I'm guessing that checkdeps has a thing where it doesn't check DEPSed in
> directories, so was skipping your directory before.

It does look that way. Yet another benefit of moving out of DEPS.

[Johann Koenig]

On Fri, May 29, 2015 at 12:46 PM, Johann Koenig <johann...@google.com> wrote:
> On Fri, May 29, 2015 at 12:09 PM, Scott Graham <sco...@chromium.org> wrote:
>> Yes, I believe that's it. It's checking vs the include_rules DEPS entries in
>> src/DEPS, src/third_party/DEPS, etc.

I have a very basic understanding of DEPS but this makes it pass the presubmit:
https://codereview.chromium.org/1158913006/patch/20001/5631496962441216

Anything look wrong with it? gtest, libyuv, and test are not strictly
necessary because they aren't actually compiled by chromium, but
adding them seemed easier than figuring out how to tell DEPS that
(they don't show up in any of the file lists, but the presubmit
appears to parse everything)

The gtest presubmit warnings are only warnings and appear to be for
new code, so once this is submitted I shouldn't have any trouble
ignoring them.

[Scott Graham]

My understanding is also limited (but commented there fwiw).