[Android] Lint build/android/gyp/.

[Android] Lint build/android/gyp/.

BUG=168518
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=258451

[cjhopman, 2014-03-12 17:43:09]

It looks like pylint is assuming the wrong sys.path.

When these scripts actually run, sys.path[0] is build/android/gyp and sys.path
doesn't include build/android.

I would assume that we can do something to make pylint aware of the fact that
build/android/gyp is in sys.path and then get rid of all the f0401 suppression.

I don't know if we can get it to understand that build/android is not in
sys.path. So may need to suppress warnings about not needing to
sys.path.append(BUILD_ANDROID_DIR).

https://codereview.chromium.org/197693002/diff/1/build/android/gyp/get_device...
File build/android/gyp/get_device_configuration.py (left):

https://codereview.chromium.org/197693002/diff/1/build/android/gyp/get_device...
build/android/gyp/get_device_configuration.py:23:
sys.path.append(BUILD_ANDROID_DIR)
Removing this is wrong.

[jbudorick, 2014-03-12 18:24:41]

I think I should be able to get sys.path set up appropriately with a pylintrc
file in build/android/gyp.

https://codereview.chromium.org/197693002/diff/1/build/android/gyp/get_device...
File build/android/gyp/get_device_configuration.py (left):

https://codereview.chromium.org/197693002/diff/1/build/android/gyp/get_device...
build/android/gyp/get_device_configuration.py:23:
sys.path.append(BUILD_ANDROID_DIR)
On 2014/03/12 17:43:10, cjhopman wrote:
> Removing this is wrong.

Even though, after removing the unused import below, this file no longer
includes anything that would require build/android being in the path?

[jbudorick, 2014-03-12 19:28:29]

On 2014/03/12 18:24:41, jbudorick wrote:
> I think I should be able to get sys.path set up appropriately with a pylintrc
> file in build/android/gyp.
> 
>
https://codereview.chromium.org/197693002/diff/1/build/android/gyp/get_device...
> File build/android/gyp/get_device_configuration.py (left):
> 
>
https://codereview.chromium.org/197693002/diff/1/build/android/gyp/get_device...
> build/android/gyp/get_device_configuration.py:23:
> sys.path.append(BUILD_ANDROID_DIR)
> On 2014/03/12 17:43:10, cjhopman wrote:
> > Removing this is wrong.
> 
> Even though, after removing the unused import below, this file no longer
> includes anything that would require build/android being in the path?

Following up: pylint uses the directory in which PRESUBMIT.py as sys.path[0],
the pylintrc file is set up in depot_tools (i.e., we can't modify or extend it),
and pylint only handles message enabling and disabling inline. The only option
to avoid these F0401s, then, is to lint b/a/gyp in a separate PRESUBMIT.py file
in b/a/gyp and blacklist it in b/a/PRESUBMIT.py. We wind up with F0401s for the
stuff in pylib anyway, though, so I'm not sure if it's worth the separate
presubmit script.

[cjhopman, 2014-03-12 19:34:11]

https://codereview.chromium.org/197693002/diff/1/build/android/gyp/get_device...
File build/android/gyp/get_device_configuration.py (left):

https://codereview.chromium.org/197693002/diff/1/build/android/gyp/get_device...
build/android/gyp/get_device_configuration.py:23:
sys.path.append(BUILD_ANDROID_DIR)
On 2014/03/12 18:24:42, jbudorick wrote:
> On 2014/03/12 17:43:10, cjhopman wrote:
> > Removing this is wrong.
> 
> Even though, after removing the unused import below, this file no longer
> includes anything that would require build/android being in the path?

You've convinced me. Removing this is right.

[cjhopman, 2014-03-13 23:23:28]

On 2014/03/12 19:28:29, jbudorick wrote:
> On 2014/03/12 18:24:41, jbudorick wrote:
> > I think I should be able to get sys.path set up appropriately with a
pylintrc
> > file in build/android/gyp.
> > 
> >
>
https://codereview.chromium.org/197693002/diff/1/build/android/gyp/get_device...
> > File build/android/gyp/get_device_configuration.py (left):
> > 
> >
>
https://codereview.chromium.org/197693002/diff/1/build/android/gyp/get_device...
> > build/android/gyp/get_device_configuration.py:23:
> > sys.path.append(BUILD_ANDROID_DIR)
> > On 2014/03/12 17:43:10, cjhopman wrote:
> > > Removing this is wrong.
> > 
> > Even though, after removing the unused import below, this file no longer
> > includes anything that would require build/android being in the path?
> 
> Following up: pylint uses the directory in which PRESUBMIT.py as sys.path[0],
> the pylintrc file is set up in depot_tools (i.e., we can't modify or extend
it),
> and pylint only handles message enabling and disabling inline. The only option
> to avoid these F0401s, then, is to lint b/a/gyp in a separate PRESUBMIT.py
file
> in b/a/gyp and blacklist it in b/a/PRESUBMIT.py. We wind up with F0401s for
the
> stuff in pylib anyway, though, so I'm not sure if it's worth the separate
> presubmit script.

Oh, it's not important that build/android/gyp is sys.path[0], just that it is in
sys.path. It looks like you can do that simply by adding it to extra_paths_list.

[jbudorick, 2014-03-14 00:38:15]

On 2014/03/13 23:23:28, cjhopman wrote:
> On 2014/03/12 19:28:29, jbudorick wrote:
> > On 2014/03/12 18:24:41, jbudorick wrote:
> > > I think I should be able to get sys.path set up appropriately with a
> pylintrc
> > > file in build/android/gyp.
> > > 
> > >
> >
>
https://codereview.chromium.org/197693002/diff/1/build/android/gyp/get_device...
> > > File build/android/gyp/get_device_configuration.py (left):
> > > 
> > >
> >
>
https://codereview.chromium.org/197693002/diff/1/build/android/gyp/get_device...
> > > build/android/gyp/get_device_configuration.py:23:
> > > sys.path.append(BUILD_ANDROID_DIR)
> > > On 2014/03/12 17:43:10, cjhopman wrote:
> > > > Removing this is wrong.
> > > 
> > > Even though, after removing the unused import below, this file no longer
> > > includes anything that would require build/android being in the path?
> > 
> > Following up: pylint uses the directory in which PRESUBMIT.py as
sys.path[0],
> > the pylintrc file is set up in depot_tools (i.e., we can't modify or extend
> it),
> > and pylint only handles message enabling and disabling inline. The only
option
> > to avoid these F0401s, then, is to lint b/a/gyp in a separate PRESUBMIT.py
> file
> > in b/a/gyp and blacklist it in b/a/PRESUBMIT.py. We wind up with F0401s for
> the
> > stuff in pylib anyway, though, so I'm not sure if it's worth the separate
> > presubmit script.
> 
> Oh, it's not important that build/android/gyp is sys.path[0], just that it is
in
> sys.path. It looks like you can do that simply by adding it to
extra_paths_list.

Good call, I'd missed extra_paths_list.

[jbudorick, 2014-03-20 00:21:53]

Now F0401-free.

I'm not sure about choosing to do a separate pylint call vs. adding b/a/gyp to
the extra_paths_list for everything.

[cjhopman, 2014-03-20 01:20:11]

lgtm. This is great, thanks.

[jbudorick, 2014-03-20 13:51:45]

The CQ bit was checked by jbudorick@chromium.org

[commit-bot: I haz the power, 2014-03-20 13:51:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jbudorick@chromium.org/197693002/20001

[commit-bot: I haz the power, 2014-03-20 13:53:24]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-20 13:53:25]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_clang_dbg

[jbudorick, 2014-03-20 15:30:55]

The CQ bit was checked by jbudorick@chromium.org

[commit-bot: I haz the power, 2014-03-20 15:31:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jbudorick@chromium.org/197693002/20001

[commit-bot: I haz the power, 2014-03-20 15:34:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-20 15:34:07]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_clang_dbg

[jbudorick, 2014-03-20 20:56:30]

The CQ bit was checked by jbudorick@chromium.org

[commit-bot: I haz the power, 2014-03-20 20:57:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jbudorick@chromium.org/197693002/20001

[commit-bot: I haz the power, 2014-03-20 23:09:32]

Change committed as 258451