Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(336)

Issue 657163003: Swarming|deterministic: Run zap_timestamp on the Windows binaries. (Closed)

Created:
6 years, 2 months ago by Sébastien Marchand
Modified:
6 years, 2 months ago
Reviewers:
M-A Ruel
CC:
chromium-reviews, pgervais+watch_chromium.org, kjellander-cc_chromium.org, cmp-cc_chromium.org, stip+watch_chromium.org
Project:
tools
Visibility:
Public.

Description

Swarming|deterministic: Run zap_timestamp on the Windows binaries. BUG=330260 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=292492

Patch Set 1 : #

Total comments: 2

Patch Set 2 : Simplify the RunZapTimestamp function. #

Patch Set 3 : Call zap_timestamp from a script rather than in the recipe. #

Total comments: 8

Patch Set 4 : Address maruel's comments. #

Patch Set 5 : Fix a bug in the test expectations #

Messages

Total messages: 23 (10 generated)
Sébastien Marchand
PTAL. The zap_timestamp step appear only once in the expectations, but it run twice when ...
6 years, 2 months ago (2014-10-15 19:21:29 UTC) #4
M-A Ruel
https://codereview.chromium.org/657163003/diff/40001/scripts/slave/recipes/swarming/deterministic_build.py File scripts/slave/recipes/swarming/deterministic_build.py (right): https://codereview.chromium.org/657163003/diff/40001/scripts/slave/recipes/swarming/deterministic_build.py#newcode98 scripts/slave/recipes/swarming/deterministic_build.py:98: proc = subprocess.Popen(command, Why not call the executable directly?
6 years, 2 months ago (2014-10-15 19:25:59 UTC) #5
Sébastien Marchand
Thanks, PTAnL. https://codereview.chromium.org/657163003/diff/40001/scripts/slave/recipes/swarming/deterministic_build.py File scripts/slave/recipes/swarming/deterministic_build.py (right): https://codereview.chromium.org/657163003/diff/40001/scripts/slave/recipes/swarming/deterministic_build.py#newcode98 scripts/slave/recipes/swarming/deterministic_build.py:98: proc = subprocess.Popen(command, On 2014/10/15 19:25:58, M-A ...
6 years, 2 months ago (2014-10-15 19:51:56 UTC) #7
M-A Ruel
In practice, it's better to glob *.exe, *.dll and *.pdb. So it should be done ...
6 years, 2 months ago (2014-10-15 20:00:30 UTC) #8
Sébastien Marchand
PTAnL. I've added a script to run zap_timestamp on all the PE files in the ...
6 years, 2 months ago (2014-10-16 18:43:37 UTC) #11
M-A Ruel
lgtm with nits https://codereview.chromium.org/657163003/diff/140001/scripts/slave/swarming/remove_build_metadata.py File scripts/slave/swarming/remove_build_metadata.py (right): https://codereview.chromium.org/657163003/diff/140001/scripts/slave/swarming/remove_build_metadata.py#newcode25 scripts/slave/swarming/remove_build_metadata.py:25: # Only run zap_timestamp on the ...
6 years, 2 months ago (2014-10-16 18:56:41 UTC) #12
Sébastien Marchand
Thanks, committing. https://codereview.chromium.org/657163003/diff/140001/scripts/slave/swarming/remove_build_metadata.py File scripts/slave/swarming/remove_build_metadata.py (right): https://codereview.chromium.org/657163003/diff/140001/scripts/slave/swarming/remove_build_metadata.py#newcode25 scripts/slave/swarming/remove_build_metadata.py:25: # Only run zap_timestamp on the PE ...
6 years, 2 months ago (2014-10-16 19:08:31 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/657163003/150001
6 years, 2 months ago (2014-10-16 19:09:38 UTC) #15
commit-bot: I haz the power
Presubmit check for 657163003-150001 failed and returned exit status 1. Running presubmit commit checks ...
6 years, 2 months ago (2014-10-16 19:10:49 UTC) #17
Sébastien Marchand
there's a bug with the recipe training script on Windows apparently... The bug is that ...
6 years, 2 months ago (2014-10-16 19:14:42 UTC) #18
M-A Ruel
On 2014/10/16 19:14:42, Sébastien Marchand wrote: > there's a bug with the recipe training script ...
6 years, 2 months ago (2014-10-16 19:19:14 UTC) #19
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/657163003/180001
6 years, 2 months ago (2014-10-16 19:29:20 UTC) #22
commit-bot: I haz the power
6 years, 2 months ago (2014-10-16 19:30:53 UTC) #23
Message was sent while issue was closed.
Committed patchset #5 (id:180001) as 292492

Powered by Google App Engine
This is Rietveld 408576698