Fix a few typos and style in checkdeps

Fix a few typos in checkdeps

R=Jói

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

[Nils Barth (inactive), 2014-04-21 03:40:48]

Hi Jói, a few typos, notably warning that talks about a directory when the error
is that there is *not* a directory!

[Nils Barth (inactive), 2014-04-21 08:36:08]

Looked at more and turned into lots of Python style fixes.
Also think I found one bug (noted); hope this is helpful!

https://codereview.chromium.org/244313005/diff/20001/tools/checkdeps/checkdep...
File tools/checkdeps/checkdeps.py (right):

https://codereview.chromium.org/244313005/diff/20001/tools/checkdeps/checkdep...
tools/checkdeps/checkdeps.py:128: continue
This |pass| looks like a bug, as the |if| does nothing.
I think this is supposed to be |continue|, since we don't check
non-C++ files.

[Jói, 2014-04-22 14:23:31]

LGTM

https://codereview.chromium.org/244313005/diff/20001/tools/checkdeps/checkdep...
File tools/checkdeps/checkdeps.py (right):

https://codereview.chromium.org/244313005/diff/20001/tools/checkdeps/checkdep...
tools/checkdeps/checkdeps.py:128: continue
On 2014/04/21 08:36:09, Nils Barth wrote:
> This |pass| looks like a bug, as the |if| does nothing.
> I think this is supposed to be |continue|, since we don't check
> non-C++ files.

I think you're right.

[Nils Barth (inactive), 2014-04-23 08:59:23]

https://codereview.chromium.org/244313005/diff/20001/tools/checkdeps/checkdep...
File tools/checkdeps/checkdeps.py (right):

https://codereview.chromium.org/244313005/diff/20001/tools/checkdeps/checkdep...
tools/checkdeps/checkdeps.py:128: continue
On 2014/04/22 14:23:31, Jói wrote:
> I think you're right.

Thanks for checking!

[Nils Barth (inactive), 2014-04-23 09:01:19]

The CQ bit was checked by nbarth@chromium.org

[commit-bot: I haz the power, 2014-04-23 09:01:32]

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

[commit-bot: I haz the power, 2014-04-23 09:04:44]

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

[commit-bot: I haz the power, 2014-04-23 09:04:46]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_chromeos_clang_dbg
  tryserver.chromium on linux_chromium_chromeos_rel

[Nils Barth (inactive), 2014-04-23 09:19:21]

The CQ bit was checked by nbarth@chromium.org

[commit-bot: I haz the power, 2014-04-23 09:23:36]

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

[commit-bot: I haz the power, 2014-04-23 09:31:03]

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

[commit-bot: I haz the power, 2014-04-23 09:31:04]

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

[Nils Barth (inactive), 2014-04-24 00:47:14]

The CQ bit was checked by nbarth@chromium.org

[commit-bot: I haz the power, 2014-04-24 00:47:53]

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

[commit-bot: I haz the power, 2014-04-24 03:08:20]

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

[commit-bot: I haz the power, 2014-04-24 03:08:21]

Try jobs failed on following builders:
  tryserver.chromium on win_chromium_rel
  tryserver.chromium on win_chromium_x64_rel

[Nils Barth (inactive), 2014-04-24 03:45:57]

On 2014/04/24 03:08:21, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   tryserver.chromium on win_chromium_rel
>   tryserver.chromium on win_chromium_x64_rel

Turns out there was a real issue on Windows, due to normalizing one dir but not
another, and then comparing them.
Fixed this by normalizing both, and also noticed that NormalizePath() was just
duplicating os.path.normcase(),
so replaced it with the library function.

[Nils Barth (inactive), 2014-04-24 06:08:18]

The CQ bit was checked by nbarth@chromium.org

[commit-bot: I haz the power, 2014-04-24 06:08:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nbarth@chromium.org/244313005/80001

[commit-bot: I haz the power, 2014-04-24 08:27:31]

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

[commit-bot: I haz the power, 2014-04-24 08:27:31]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_chromeos_rel
  tryserver.chromium on win_chromium_rel
  tryserver.chromium on win_chromium_x64_rel

[Nils Barth (inactive), 2014-04-24 09:05:28]

The CQ bit was checked by nbarth@chromium.org

[commit-bot: I haz the power, 2014-04-24 09:05:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nbarth@chromium.org/244313005/80001

[commit-bot: I haz the power, 2014-04-24 14:18:25]

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

[commit-bot: I haz the power, 2014-04-24 14:18:26]

Try jobs failed on following builders:
  tryserver.chromium on win_chromium_rel
  tryserver.chromium on win_chromium_x64_rel

[Nils Barth (inactive), 2014-04-25 03:09:35]

On 2014/04/24 14:18:26, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   tryserver.chromium on win_chromium_rel
>   tryserver.chromium on win_chromium_x64_rel

This is a real failure; problem was using os.path.reldir to get relative paths,
which fails on Windows.
We're using POSIX-style paths, so this should be posixpath.reldir instead.

Also flattened the logic in GetDirectoryRules, as it was redundantly nested and
confusing.
(Existing comment was correct and clarified it, but expanded a bit.)
Added a few comments on exactly where mutation is happening.

[Nils Barth (inactive), 2014-04-25 03:09:40]

The CQ bit was checked by nbarth@chromium.org

[commit-bot: I haz the power, 2014-04-25 03:10:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nbarth@chromium.org/244313005/100001

[commit-bot: I haz the power, 2014-04-25 08:27:44]

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

[commit-bot: I haz the power, 2014-04-25 08:27:45]

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

[Nils Barth (inactive), 2014-04-25 09:01:39]

The CQ bit was checked by nbarth@chromium.org

[commit-bot: I haz the power, 2014-04-25 09:02:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nbarth@chromium.org/244313005/100001

[Paweł Hajdan Jr., 2014-04-25 09:30:56]

The CQ bit was unchecked by phajdan.jr@chromium.org

[Paweł Hajdan Jr., 2014-04-25 12:10:24]

Please see the following builds:

http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...

http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...

It seems a bit suspicious checkeps is taking so long there and printing so many
lines, and this CL is modifying checkdeps.

[Nils Barth (inactive), 2014-04-28 05:32:55]

On 2014/04/25 12:10:24, Paweł Hajdan Jr. wrote:
> Please see the following builds:
> 
>
http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...
> 
>
http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...
> 
> It seems a bit suspicious checkeps is taking so long there and printing so
many
> lines, and this CL is modifying checkdeps.

Hi Paweł, thanks for checking on this.

Yup, the try jobs had not only been failing, but checkdeps had been taking a
very long time.
This is because I was fixing the path handling (making it more regular), which
of course is
different on Windows.
My initial fixes didn't work, and resulted in a recursive check (climbing up a
directory tree
and looking for memoized results) going failing to find a match, hence doing
far, far too much work.

I couldn't test this normally, since I don't have a Windows machine, so I was
running this on the try bots;
sorry if this loaded them excessively!
(Is there something else I should do in future? Get a Windows account or login
somewhere?)

I've since fixed it, so this should work now.

[Nils Barth (inactive), 2014-04-28 05:35:28]

I got normcase & normpath backwards: these convert / to \ on Windows (POSIX to
Win),
but we wanted to convert \ to / (Win to POSIX), which has to be done manually.

I've fixed this and made the code explicitly distinguish local paths from
normalized paths,
which should avoid such problems in future, and allows us to remove several
excess normalizations.

This is working now on Windows, per:
http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...
(Running in 3 minutes, not 60 minutes.)
...so committing.

[Nils Barth (inactive), 2014-04-28 06:17:22]

The CQ bit was checked by nbarth@chromium.org

[commit-bot: I haz the power, 2014-04-28 06:17:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nbarth@chromium.org/244313005/120001

[commit-bot: I haz the power, 2014-04-28 07:55:57]

Change committed as 266477