Fix markupsafe pydeps import issue.

Fix markupsafe pydeps import issue.

Pydeps would use markupsafe system module over the one in
third_party causing presubmit issues.

Review-Url: https://codereview.chromium.org/2896583003
Cr-Commit-Position: refs/heads/master@{#473229}
Committed: https://chromium.googlesource.com/chromium/src/+/79ae465115ced930a6daeb8a58f0aa7dfa07f6af

[mikecase (-- gone --), 2017-05-19 16:01:22]

The CQ bit was checked by mikecase@chromium.org to run a CQ dry run

[mikecase (-- gone --), 2017-05-19 16:01:56]

mikecase@chromium.org changed reviewers:
+ jbudorick@chromium.org

[commit-bot: I haz the power, 2017-05-19 16:02:31]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Nico, 2017-05-19 16:03:43]

thakis@chromium.org changed reviewers:
+ thakis@chromium.org

[Nico, 2017-05-19 16:03:43]

https://codereview.chromium.org/2896583003/diff/1/build/android/pylib/local/d...
File build/android/pylib/local/device/local_device_instrumentation_test_run.py
(right):

https://codereview.chromium.org/2896583003/diff/1/build/android/pylib/local/d...
build/android/pylib/local/device/local_device_instrumentation_test_run.py:32:
os.path.join(host_paths.DIR_SOURCE_ROOT, 'third_party'), 1):
Why is this better than just sys.path.insert(0, ...) (which we use all over the
place and which people can understand without having to read
host_paths.SysPath())?

[mikecase (-- gone --), 2017-05-19 16:06:30]

The CQ bit was unchecked by mikecase@chromium.org

[mikecase (-- gone --), 2017-05-19 16:09:44]

On 2017/05/19 at 16:03:43, thakis wrote:
>
https://codereview.chromium.org/2896583003/diff/1/build/android/pylib/local/d...
> File build/android/pylib/local/device/local_device_instrumentation_test_run.py
(right):
> 
>
https://codereview.chromium.org/2896583003/diff/1/build/android/pylib/local/d...
> build/android/pylib/local/device/local_device_instrumentation_test_run.py:32:
os.path.join(host_paths.DIR_SOURCE_ROOT, 'third_party'), 1):
> Why is this better than just sys.path.insert(0, ...) (which we use all over
the place and which people can understand without having to read
host_paths.SysPath())?

idk, I kind of like that it removes the path from sys.path right after import.
It can prevent us from importing modules from the wrong locations (like what
caused this pydeps issue). It is used quite a bit in build/android/

[jbudorick, 2017-05-19 16:17:07]

lgtm

On 2017/05/19 16:09:44, mikecase wrote:
> On 2017/05/19 at 16:03:43, thakis wrote:
> >
>
https://codereview.chromium.org/2896583003/diff/1/build/android/pylib/local/d...
> > File
build/android/pylib/local/device/local_device_instrumentation_test_run.py
> (right):
> > 
> >
>
https://codereview.chromium.org/2896583003/diff/1/build/android/pylib/local/d...
> >
build/android/pylib/local/device/local_device_instrumentation_test_run.py:32:
> os.path.join(host_paths.DIR_SOURCE_ROOT, 'third_party'), 1):
> > Why is this better than just sys.path.insert(0, ...) (which we use all over
> the place and which people can understand without having to read
> host_paths.SysPath())?
> 
> idk, I kind of like that it removes the path from sys.path right after import.
> It can prevent us from importing modules from the wrong locations (like what
> caused this pydeps issue). It is used quite a bit in build/android/

That is the primary benefit.

[mikecase (-- gone --), 2017-05-19 16:18:24]

The CQ bit was checked by mikecase@chromium.org

[Nico, 2017-05-19 16:18:32]

On 2017/05/19 16:17:07, jbudorick wrote:
> lgtm
> 
> On 2017/05/19 16:09:44, mikecase wrote:
> > On 2017/05/19 at 16:03:43, thakis wrote:
> > >
> >
>
https://codereview.chromium.org/2896583003/diff/1/build/android/pylib/local/d...
> > > File
> build/android/pylib/local/device/local_device_instrumentation_test_run.py
> > (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2896583003/diff/1/build/android/pylib/local/d...
> > >
> build/android/pylib/local/device/local_device_instrumentation_test_run.py:32:
> > os.path.join(host_paths.DIR_SOURCE_ROOT, 'third_party'), 1):
> > > Why is this better than just sys.path.insert(0, ...) (which we use all
over
> > the place and which people can understand without having to read
> > host_paths.SysPath())?
> > 
> > idk, I kind of like that it removes the path from sys.path right after
import.
> > It can prevent us from importing modules from the wrong locations (like what
> > caused this pydeps issue). It is used quite a bit in build/android/
> 
> That is the primary benefit.

Is that useful? When? Is the benefit large enough to make the android python
code harder to read for non-android folks?

[commit-bot: I haz the power, 2017-05-19 16:18:57]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jbudorick, 2017-05-19 16:25:09]

On 2017/05/19 16:18:32, Nico wrote:
> On 2017/05/19 16:17:07, jbudorick wrote:
> > lgtm
> > 
> > On 2017/05/19 16:09:44, mikecase wrote:
> > > On 2017/05/19 at 16:03:43, thakis wrote:
> > > >
> > >
> >
>
https://codereview.chromium.org/2896583003/diff/1/build/android/pylib/local/d...
> > > > File
> > build/android/pylib/local/device/local_device_instrumentation_test_run.py
> > > (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/2896583003/diff/1/build/android/pylib/local/d...
> > > >
> >
build/android/pylib/local/device/local_device_instrumentation_test_run.py:32:
> > > os.path.join(host_paths.DIR_SOURCE_ROOT, 'third_party'), 1):
> > > > Why is this better than just sys.path.insert(0, ...) (which we use all
> over
> > > the place and which people can understand without having to read
> > > host_paths.SysPath())?
> > > 
> > > idk, I kind of like that it removes the path from sys.path right after
> import.
> > > It can prevent us from importing modules from the wrong locations (like
what
> > > caused this pydeps issue). It is used quite a bit in build/android/
> > 
> > That is the primary benefit.
> 
> Is that useful? When? Is the benefit large enough to make the android python
> code harder to read for non-android folks?

It is. Clients importing modules that pop the entry out of the sys.path don't
have to worry about modules resolving to an unexpected implementation (or an
entirely different module with the same name). This is something that quite a
bit of our code does:
https://cs.chromium.org/search/?q=sys.path.pop+package:%5Echromium$&p=2&type=cs

iirc I wrote SysPath because:
 - it behaves well by default; we don't have to remember to remove the entry
from the sys.path
 - it's less clunky than having

  sys.path.append(...)
  try:
    import foo
  finally:
    sys.path.pop()

all over the place.

I suppose it could be slightly better named as "ScopedSysPath" or something
along those lines.

[commit-bot: I haz the power, 2017-05-19 17:29:25]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1495210704954670,
"parent_rev": "d5d2c899dd034c7f3fb6294a67a73d7a08ae1a05", "commit_rev":
"79ae465115ced930a6daeb8a58f0aa7dfa07f6af"}

[commit-bot: I haz the power, 2017-05-19 17:29:43]

Description was changed from

==========
Fix markupsafe pydeps import issue.

Pydeps would use markupsafe system module over the one in
third_party causing presubmit issues.
==========

to

==========
Fix markupsafe pydeps import issue.

Pydeps would use markupsafe system module over the one in
third_party causing presubmit issues.

Review-Url: https://codereview.chromium.org/2896583003
Cr-Commit-Position: refs/heads/master@{#473229}
Committed:
https://chromium.googlesource.com/chromium/src/+/79ae465115ced930a6daeb8a58f0...
==========

[commit-bot: I haz the power, 2017-05-19 17:29:44]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/79ae465115ced930a6daeb8a58f0...