Use hermetic timestamps in apkbuilder.py and refactor helper function

Use hermetic timestamps in apkbuilder.py and refactor helper function

This makes it so that the command will always produce the same output
given the same inputs.

BUG=

Committed: https://crrev.com/b37a3d86d55cb781acdaf4aa64c1ea4cadae3271
Cr-Commit-Position: refs/heads/master@{#361198}

[agrieve, 2015-11-16 21:38:07]

Description was changed from

==========
Use hermetic timestamps in apkbuilder.py and refactor helper function

This makes it so that the command will always produce the same output
given the same inputs.

BUG=
==========

to

==========
Use hermetic timestamps in apkbuilder.py and refactor helper function

This makes it so that the command will always produce the same output
given the same inputs.

BUG=
==========

[agrieve, 2015-11-16 21:38:07]

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

[agrieve, 2015-11-16 21:38:27]

On 2015/11/16 21:38:07, agrieve wrote:
> mailto:agrieve@chromium.org changed reviewers:
> + mailto:jbudorick@chromium.org

Will hold off on submitting until after your multidex commit lands.

[jbudorick, 2015-11-17 03:27:17]

https://codereview.chromium.org/1452843002/diff/20001/build/android/gyp/util/...
File build/android/gyp/util/build_utils.py (right):

https://codereview.chromium.org/1452843002/diff/20001/build/android/gyp/util/...
build/android/gyp/util/build_utils.py:239: assert (not src_path) != (not data),
(
I was really confused by the original version of this assert.

Could this be

  assert bool(src_path) != bool(data)

?

https://codereview.chromium.org/1452843002/diff/20001/build/android/gyp/util/...
build/android/gyp/util/build_utils.py:255: if compress is None:
why isn't this just

  zip_file.writestr(zipinfo, data, zipfile.ZIP_DEFLATED if compress else
zipfile.ZIP_STORED)

why is the compress is None case handled differently than the compress is False
case?

[agrieve, 2015-11-17 03:43:01]

https://codereview.chromium.org/1452843002/diff/20001/build/android/gyp/util/...
File build/android/gyp/util/build_utils.py (right):

https://codereview.chromium.org/1452843002/diff/20001/build/android/gyp/util/...
build/android/gyp/util/build_utils.py:239: assert (not src_path) != (not data),
(
On 2015/11/17 03:27:17, jbudorick wrote:
> I was really confused by the original version of this assert.
> 
> Could this be
> 
>   assert bool(src_path) != bool(data)
> 
> ?

embarrassingly, didn't know about bool() :P

https://codereview.chromium.org/1452843002/diff/20001/build/android/gyp/util/...
build/android/gyp/util/build_utils.py:255: if compress is None:
On 2015/11/17 03:27:17, jbudorick wrote:
> why isn't this just
> 
>   zip_file.writestr(zipinfo, data, zipfile.ZIP_DEFLATED if compress else
> zipfile.ZIP_STORED)
> 
> why is the compress is None case handled differently than the compress is
False
> case?

Updated this logic to be a bit more clever and elaborated in the comment.

[jbudorick, 2015-11-17 14:15:15]

lgtm w/ nit

https://codereview.chromium.org/1452843002/diff/40001/build/android/gyp/apkbu...
File build/android/gyp/apkbuilder.py (right):

https://codereview.chromium.org/1452843002/diff/40001/build/android/gyp/apkbu...
build/android/gyp/apkbuilder.py:94: compress = not disable_compression and (
nit: Can we flip this to enable_compression? I think it'd make this easier to
read than the double negative here.

https://codereview.chromium.org/1452843002/diff/40001/build/android/gyp/util/...
File build/android/gyp/util/build_utils.py (right):

https://codereview.chromium.org/1452843002/diff/40001/build/android/gyp/util/...
build/android/gyp/util/build_utils.py:236: compress: Whether to enable
compression. Default is take from ZipFile
oh, I get it now.

[agrieve, 2015-11-18 20:36:34]

https://codereview.chromium.org/1452843002/diff/40001/build/android/gyp/apkbu...
File build/android/gyp/apkbuilder.py (right):

https://codereview.chromium.org/1452843002/diff/40001/build/android/gyp/apkbu...
build/android/gyp/apkbuilder.py:94: compress = not disable_compression and (
On 2015/11/17 14:15:15, jbudorick wrote:
> nit: Can we flip this to enable_compression? I think it'd make this easier to
> read than the double negative here.

Normally agree, but in this case I think it's a bit more clear that the result
is to disable all compression, whereas when it's false (e.g. enable_compression)
- compression is still sometimes disabled based on file type.

[agrieve, 2015-11-23 20:29:22]

The CQ bit was checked by agrieve@chromium.org

[agrieve, 2015-11-23 20:29:23]

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

[commit-bot: I haz the power, 2015-11-23 20:31:12]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1452843002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1452843002/60001

[commit-bot: I haz the power, 2015-11-23 22:07:35]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-11-23 22:08:19]

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