[Android] Fix checksum handling when md5sum generates unexpected output.

[Android] Fix checksum handling when md5sum generates unexpected output.

BUG=479966
Committed: https://crrev.com/d2e31764fd89c457741330c430beaf89b3c40f49
Cr-Commit-Position: refs/heads/master@{#326814}

[jbudorick, 2015-04-22 23:32:29]

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

[jbudorick, 2015-04-22 23:32:30]

- md5sum will no longer emit device HashAndPath tuples that contain dumb things
like linker warnings.
- device_utils will now look for the correct host and device paths within the
returned hash tuples for single-file change detection.

Both fixes independently fixed the issue (and they work together, too).

[perezju, 2015-04-23 09:54:59]

https://codereview.chromium.org/1106573002/diff/1/build/android/pylib/device/...
File build/android/pylib/device/device_utils.py (right):

https://codereview.chromium.org/1106573002/diff/1/build/android/pylib/device/...
build/android/pylib/device/device_utils.py:874: device_hash = h
really sounds like md5sum functions should return a dictionary of {path: hash,
...}?

[jbudorick, 2015-04-23 16:20:15]

https://codereview.chromium.org/1106573002/diff/1/build/android/pylib/device/...
File build/android/pylib/device/device_utils.py (right):

https://codereview.chromium.org/1106573002/diff/1/build/android/pylib/device/...
build/android/pylib/device/device_utils.py:874: device_hash = h
On 2015/04/23 09:54:59, perezju wrote:
> really sounds like md5sum functions should return a dictionary of {path: hash,
> ...}?

I suppose so. Done.

[perezju, 2015-04-24 08:06:33]

https://codereview.chromium.org/1106573002/diff/20001/build/android/pylib/dev...
File build/android/pylib/device/device_utils.py (right):

https://codereview.chromium.org/1106573002/diff/20001/build/android/pylib/dev...
build/android/pylib/device/device_utils.py:861: for p, _ in
host_checksums.iteritems()))
for p in host_checksums.iterkeys()

https://codereview.chromium.org/1106573002/diff/20001/build/android/pylib/dev...
build/android/pylib/device/device_utils.py:866: host_checksum =
host_checksums.get(real_host_path)
I guess here the host_checksum *must* exist? I'm wondering what happens if both
host_checksum and device_checksum end up being None?

https://codereview.chromium.org/1106573002/diff/20001/build/android/pylib/dev...
build/android/pylib/device/device_utils.py:870: else [])
nit: I think the expanded version was easier to read.

https://codereview.chromium.org/1106573002/diff/20001/build/android/pylib/dev...
build/android/pylib/device/device_utils.py:877: or
device_checksums[device_abs_path] != host_checksum):
maybe just:

  if device_checksums.get(device_abs_path) != host_checksum:

https://codereview.chromium.org/1106573002/diff/20001/build/android/pylib/ins...
File build/android/pylib/instrumentation/instrumentation_test_instance.py
(right):

https://codereview.chromium.org/1106573002/diff/20001/build/android/pylib/ins...
build/android/pylib/instrumentation/instrumentation_test_instance.py:338:
jar_md5 = md5sum.CalculateHostMd5Sums(jar_path)[jar_path]
This usage seems pretty common. Wondering if we should add a function in md5sum,
something like:

def CalculateHostMd5Sum(path):
  return CalculateHostMd5Sums(path).get(path)

https://codereview.chromium.org/1106573002/diff/20001/build/android/pylib/uti...
File build/android/pylib/utils/md5sum.py (right):

https://codereview.chromium.org/1106573002/diff/20001/build/android/pylib/uti...
build/android/pylib/utils/md5sum.py:16: HashAndPath =
collections.namedtuple('HashAndPath', ['hash', 'path'])
this is not needed any more

https://codereview.chromium.org/1106573002/diff/20001/build/android/pylib/uti...
build/android/pylib/utils/md5sum.py:85: return dict((p, h) for h, p in
hash_and_path)
nit: can be written as:

  return {p: h for h, p in hash_and_path}

[jbudorick, 2015-04-24 13:56:36]

https://codereview.chromium.org/1106573002/diff/20001/build/android/pylib/dev...
File build/android/pylib/device/device_utils.py (right):

https://codereview.chromium.org/1106573002/diff/20001/build/android/pylib/dev...
build/android/pylib/device/device_utils.py:861: for p, _ in
host_checksums.iteritems()))
On 2015/04/24 08:06:32, perezju wrote:
> for p in host_checksums.iterkeys()

Done.

https://codereview.chromium.org/1106573002/diff/20001/build/android/pylib/dev...
build/android/pylib/device/device_utils.py:866: host_checksum =
host_checksums.get(real_host_path)
On 2015/04/24 08:06:32, perezju wrote:
> I guess here the host_checksum *must* exist? I'm wondering what happens if
both
> host_checksum and device_checksum end up being None?

If we can't determine a checksum for a file, it's probably ok if we don't try to
push that file.

https://codereview.chromium.org/1106573002/diff/20001/build/android/pylib/dev...
build/android/pylib/device/device_utils.py:870: else [])
On 2015/04/24 08:06:32, perezju wrote:
> nit: I think the expanded version was easier to read.

Done.

https://codereview.chromium.org/1106573002/diff/20001/build/android/pylib/dev...
build/android/pylib/device/device_utils.py:877: or
device_checksums[device_abs_path] != host_checksum):
On 2015/04/24 08:06:32, perezju wrote:
> maybe just:
> 
>   if device_checksums.get(device_abs_path) != host_checksum:

Done.

https://codereview.chromium.org/1106573002/diff/20001/build/android/pylib/ins...
File build/android/pylib/instrumentation/instrumentation_test_instance.py
(right):

https://codereview.chromium.org/1106573002/diff/20001/build/android/pylib/ins...
build/android/pylib/instrumentation/instrumentation_test_instance.py:338:
jar_md5 = md5sum.CalculateHostMd5Sums(jar_path)[jar_path]
On 2015/04/24 08:06:32, perezju wrote:
> This usage seems pretty common. Wondering if we should add a function in
md5sum,
> something like:
> 
> def CalculateHostMd5Sum(path):
>   return CalculateHostMd5Sums(path).get(path)

It only seems common because our instrumentation code is temporarily duplicated.

https://codereview.chromium.org/1106573002/diff/20001/build/android/pylib/uti...
File build/android/pylib/utils/md5sum.py (right):

https://codereview.chromium.org/1106573002/diff/20001/build/android/pylib/uti...
build/android/pylib/utils/md5sum.py:16: HashAndPath =
collections.namedtuple('HashAndPath', ['hash', 'path'])
On 2015/04/24 08:06:33, perezju wrote:
> this is not needed any more

Removed.

https://codereview.chromium.org/1106573002/diff/20001/build/android/pylib/uti...
build/android/pylib/utils/md5sum.py:85: return dict((p, h) for h, p in
hash_and_path)
On 2015/04/24 08:06:32, perezju wrote:
> nit: can be written as:
> 
>   return {p: h for h, p in hash_and_path}

We have generally avoided dictionary comprehensions for reasons I don't
remember.

[perezju, 2015-04-24 14:13:25]

lgtm

https://codereview.chromium.org/1106573002/diff/20001/build/android/pylib/uti...
File build/android/pylib/utils/md5sum.py (right):

https://codereview.chromium.org/1106573002/diff/20001/build/android/pylib/uti...
build/android/pylib/utils/md5sum.py:85: return dict((p, h) for h, p in
hash_and_path)
On 2015/04/24 13:56:36, jbudorick wrote:
> On 2015/04/24 08:06:32, perezju wrote:
> > nit: can be written as:
> > 
> >   return {p: h for h, p in hash_and_path}
> 
> We have generally avoided dictionary comprehensions for reasons I don't
> remember.

hmmm, strange, but ok.

[jbudorick, 2015-04-24 15:00:48]

The CQ bit was checked by jbudorick@chromium.org

[commit-bot: I haz the power, 2015-04-24 15:00:58]

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

[commit-bot: I haz the power, 2015-04-24 16:18:30]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-04-24 16:19:28]

Patchset 3 (id:??) landed as
https://crrev.com/d2e31764fd89c457741330c430beaf89b3c40f49
Cr-Commit-Position: refs/heads/master@{#326814}