Update the VS toolchain hash computation caching.

Update the VS toolchain hash computation caching.


R=hans@chromium.org, scottmg@chromium.org
BUG=495204
Committed: https://chromium.googlesource.com/chromium/tools/depot_tools/+/e9013dbe6f7822560d7cc1dbc25f1127333f86a2

[Sébastien Marchand, 2016-06-08 16:51:01]

PTAL.

[scottmg, 2016-06-08 17:06:39]

Sorry about that, thanks, lgtm.

[Sébastien Marchand, 2016-06-08 17:07:17]

The CQ bit was checked by sebmarchand@chromium.org

[commit-bot: I haz the power, 2016-06-08 17:07:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2052533002/1

[hans, 2016-06-08 17:07:55]

https://codereview.chromium.org/2052533002/diff/1/win_toolchain/get_toolchain...
File win_toolchain/get_toolchain_if_necessary.py (right):

https://codereview.chromium.org/2052533002/diff/1/win_toolchain/get_toolchain...
win_toolchain/get_toolchain_if_necessary.py:91: file_list.append(p.lower())
This would essentially undo https://codereview.chromium.org/2039563002/

The point of that patch was to keep the correct case in the list from
GetFileList(), because otherwise many files (e.g. DIA SDK/) won't be found on
case-sensitive filesystems.

Maybe what we want to do is make the comparison on line 130 case-insensitive.

Or, maybe better, if we do the full hash computation on line 156, call
SaveTimestampsAndHash to cache it. Currently it seems the cache is only written
after downloading a new toolchain.

[hans, 2016-06-08 17:08:08]

The CQ bit was unchecked by hans@chromium.org

[Sébastien Marchand, 2016-06-08 17:14:55]

Oops, let me try something else then :)

[Sébastien Marchand, 2016-06-08 17:17:07]

Ok, I'm now updating the timestamp cache file if needed, PTAnL

[hans, 2016-06-08 17:18:30]

LGTM. The patch description needs an update, though.

Thanks for digging into this!

[Sébastien Marchand, 2016-06-08 17:22:04]

Description was changed from

==========
Add a missing .lower() to the win toolchain script.

The list of files returned by the GetFileList function is cased, while the
list of files in the cache file is all lowercased, so the file names
comparisons done in CalculateHash were always wrong as soon as a path
contained somme upper case characters (e.g. 'DIA SDK').

R=hans@chromium.org, scottmg@chromium.org
BUG=495204
==========

to

==========
Update the VS toolchain hash computation caching.


R=hans@chromium.org, scottmg@chromium.org
BUG=495204
==========

[Sébastien Marchand, 2016-06-08 17:22:14]

The CQ bit was checked by sebmarchand@chromium.org

[Sébastien Marchand, 2016-06-08 17:22:15]

The patchset sent to the CQ was uploaded after l-g-t-m from scottmg@chromium.org
Link to the patchset: https://codereview.chromium.org/2052533002/#ps20001
(title: " ")

[commit-bot: I haz the power, 2016-06-08 17:22:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2052533002/20001

[commit-bot: I haz the power, 2016-06-08 17:25:57]

Description was changed from

==========
Update the VS toolchain hash computation caching.


R=hans@chromium.org, scottmg@chromium.org
BUG=495204
==========

to

==========
Update the VS toolchain hash computation caching.


R=hans@chromium.org, scottmg@chromium.org
BUG=495204

Committed:
https://chromium.googlesource.com/chromium/tools/depot_tools/+/e9013dbe6f7822...
==========

[commit-bot: I haz the power, 2016-06-08 17:25:58]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/tools/depot_tools/+/e9013dbe6f7822...