New issue
Advanced search Search tips

Issue 804926 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 7
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Bug

Blocked on:
issue 792131

Blocking:
issue 330260



Sign in to add a comment

Remove zap_timestamp once we switch to LLD

Project Member Reported by sebmarchand@chromium.org, Jan 23 2018

Issue description

zap_timestamp is currently used to remove the timestamps embedded in the PE files produced by the MSVC linker, this will because useless once we switch to LLD.
 

Comment 1 by r...@chromium.org, Feb 14 2018

Cc: brucedaw...@chromium.org
I noticed that this bug conflicts with  https://crbug.com/801349 .

Maybe we should just add a flag that lets the build system tell us if it wants a timestamp, and then remove zap_timestamp.
Thanks for the heads up. I am hopeful that we can get determinism in ways other than zeroing out the timestamp. I've frequently found it useful to have the link time embedded in binaries and I was disappointed when Windows started repurposing that field in order to get determinism. However that (putting a hash in the timestamp field) is better than just zeroing it out. I do sometimes have to add my locally built binaries to my local symbol server and that breaks if the timestamp is zero.

Here's my understanding of the situation:

a) Official builds definitely need non-zero (timestamp or hash) time stamps so that they can be added to symbol servers
b) Local builds *occasionally* need non-zero time stamps and there are some modest advantages to these recording the actual time
c) Swarming needs a deterministic timestamp (zero or a hash) to avoid running tests unnecessarily

If this accurately reflects the situation then my preference (and the "least surprising thing" for Windows developers) would be for lld to default to a non-zero timestamp, either a hash of the binary or the actual link time, with a linker option to do a timestamp of zero if that is necessary for determinism.

In particular, if the linker defaults to a zero (or other constant) timestamp then all users of lld will have to adjust their build scripts to enable non-constant timestamps where appropriate. This will be an ongoing disruption that would impeded and complicate the switch to lld.

A hash timestamp would avoid the need for a linker switch.

Yep, I agree that a hash seems like the best solution here. 

This bug is mostly about removing zap_timestamp.exe from the pipeline as it's not maintained anymore and there's no guarantee that it'll work on LLD generated binaries.
Cc: mar...@chromium.org
I don't think that there's anything blocking this as we moved to LLD, I've started working on this in https://chromium-review.googlesource.com/c/chromium/src/+/1108185

Comment 5 by thakis@chromium.org, Jun 20 2018

We had to make lld write real timestamps due to win7 appcompat, see  issue 843199  -- so we probably can't remove this quite yet.
zap_timestamp can't process binaries produced by LLD afaik? 
See the remove_build_metadata step of this build: https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Win%20Builder/56213 , it's silently failing on all the binaries.

Comment 8 by thakis@chromium.org, Jun 20 2018

Oh, then I guess we can remove it :-P Why can't it though, isn't it working on the coff header and that format is pretty fixed?

We could pass /Brepro to ldflags get the old hash-as-timestamp behavior back when we're not doing official builds. We should probably do that. (And pass a /TIMESTAMP using the __DATE__ algorithm in official builds, as discussed in  issue 843199  -- I don't think we have a bug for that yet.)
zap_timestamp is based on Syzygy, which isn't staffed anymore so I don't think that we'll be able to fix this, and even if we did fix this it'd be limited to 32-bit builds, so it's probably better to use the /Brepro approach anyway. We should open a separate crbug for that if deterministic builds on the continuous waterfall are still needed (the fact that we can't get deterministic PDBs probably make this less useful).

Removing zap_timestamp will allow us to finally remove the Syzygy DEPS from Chrome.
There's still value in having the EXE be reproducible. We do loose all the task deduplication due to the PDB but still gain reduction in EXE uploads.
Project Member

Comment 11 by bugdroid1@chromium.org, Aug 2

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/96d692b3f755e51519b4d7c37dc0ea2491b6ac68

commit 96d692b3f755e51519b4d7c37dc0ea2491b6ac68
Author: Sebastien Marchand <sebmarchand@chromium.org>
Date: Thu Aug 02 17:47:59 2018

Remove zap_timestamp and the Syzygy deps

This isn't needed anymore as we're now using LLD.

Bug:  804926 ,  821764 
Change-Id: I3c6e4ce0793997823b89978cf7eda5b3a26399b8
Reviewed-on: https://chromium-review.googlesource.com/1108185
Reviewed-by: Sigurður Ásgeirsson <siggi@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Sébastien Marchand <sebmarchand@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580239}
[modify] https://crrev.com/96d692b3f755e51519b4d7c37dc0ea2491b6ac68/DEPS
[delete] https://crrev.com/1fffcaa3a17ce8a428343bd633f5f9583108dc79/build/get_syzygy_binaries.py
[modify] https://crrev.com/96d692b3f755e51519b4d7c37dc0ea2491b6ac68/tools/determinism/deterministic_build_blacklist.json
[modify] https://crrev.com/96d692b3f755e51519b4d7c37dc0ea2491b6ac68/tools/determinism/remove_build_metadata.py

Project Member

Comment 12 by bugdroid1@chromium.org, Aug 3

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ef36dc19986a90f49d0d31520c88d310d72bd038

commit ef36dc19986a90f49d0d31520c88d310d72bd038
Author: Nico Weber <thakis@chromium.org>
Date: Fri Aug 03 17:33:15 2018

win: write a deterministic-ish timestamp into the PE/COFF header instead of the current time

We used to set the timestamp to a hash of the binary, similar to
https://blogs.msdn.microsoft.com/oldnewthing/20180103-00/?p=97705
However, that caused an appcompat warning on Windows 7 to appear, which
interpreted the hash as a timestamp. (It's possible that https://llvm.org/PR38429
could help with that, but my guess it won't have an effect on Windows 7,
which likely always believes that the the coff timestamp field always stores
a timestamp).

So currently we write the current time during linking in that field, but that's
bad for build determinism and that in turn is bad for swarming test result cachability.

build/write_build_date_header.py already creates a deterministic BUILD_DATE
with several tradeoffs. Cachability wants this to change infrequently, but
things like HSTS need a "real" build date and want this to change frequently.
The compromise is: The date changes once per day in official builds, and
once a month in regular builds.

(We could use /Brepro in ldflags instead of /TIMESTAMP for unofficial builds to get
the binary hash in the timestamp, but having the header timestamp match the BUILD_DATE
define seems nice.)

So let's use that same time as timestamp in the PE/COFF header. lld-link has a
/TIMESTAMP: flag we can use to pass in an explicit timestamp.

Since tools can't have deps, we need to compute the timestamp at gn time,
so split write_build_date_header.py in two pieces: build/compute_build_timestamp.py
that just prints the timestamp we want to use, and the old write_build_date_header.py, which
now takes that timestamp and writes the header file.

Call compute_build_timestamp.py at gn time so that we can pass it in ldflags, and
pass the resultl to write_build_date_header.py which keeps running as an action
during build time (so that we at least don't need to write a file at gn time).

An additional wrinkle here is that the PE/COFF timestamp is used as one of just two
keys per binary for uploading PE binaries to the symbol server, the other being file size.
https://bugs.llvm.org/show_bug.cgi?id=35914#c0 has a good description of this, and
tools/symsrc/img_fingerprint.py's GetImgFingerprint() is our implementation of it.
But since we only upload binaries with symbols for official chrome builds to the symbol server,
a timestamp that changes once a day should be still enough. (32-bit and 64-bit chromes
have the same filename, and we might rarely build canary and beta and stable all on the
same day, but them all being the same size seems highly unlikely.)

Bug:  843199 , 804926 ,330260
Change-Id: I1d4193cc537ae0c4b2d6ac9281fad29de754dd6c
Reviewed-on: https://chromium-review.googlesource.com/1161104
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Hans Wennborg <hans@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580585}
[modify] https://crrev.com/ef36dc19986a90f49d0d31520c88d310d72bd038/base/BUILD.gn
[add] https://crrev.com/ef36dc19986a90f49d0d31520c88d310d72bd038/build/compute_build_timestamp.py
[modify] https://crrev.com/ef36dc19986a90f49d0d31520c88d310d72bd038/build/config/win/BUILD.gn
[modify] https://crrev.com/ef36dc19986a90f49d0d31520c88d310d72bd038/build/dotfile_settings.gni
[add] https://crrev.com/ef36dc19986a90f49d0d31520c88d310d72bd038/build/timestamp.gni
[modify] https://crrev.com/ef36dc19986a90f49d0d31520c88d310d72bd038/build/write_build_date_header.py

Status: Fixed (was: Assigned)
Blocking: 330260
Project Member

Comment 15 by bugdroid1@chromium.org, Aug 7

Labels: merge-merged-3515
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c234baa7e80a72e2dbd89bedd7abd047a12f68d4

commit c234baa7e80a72e2dbd89bedd7abd047a12f68d4
Author: Dirk Pranke <dpranke@chromium.org>
Date: Tue Aug 07 22:38:06 2018

Revert "win: write a deterministic-ish timestamp into the PE/COFF header instead of the current time"

This reverts commit ef36dc19986a90f49d0d31520c88d310d72bd038.

Reason for revert: This turns out to break the official android build, which apparently was relying on a broken aspect of the build that this fixed :(. See https://crbug.com/871173.

Original change's description:
> win: write a deterministic-ish timestamp into the PE/COFF header instead of the current time
> 
> We used to set the timestamp to a hash of the binary, similar to
> https://blogs.msdn.microsoft.com/oldnewthing/20180103-00/?p=97705
> However, that caused an appcompat warning on Windows 7 to appear, which
> interpreted the hash as a timestamp. (It's possible that https://llvm.org/PR38429
> could help with that, but my guess it won't have an effect on Windows 7,
> which likely always believes that the the coff timestamp field always stores
> a timestamp).
> 
> So currently we write the current time during linking in that field, but that's
> bad for build determinism and that in turn is bad for swarming test result cachability.
> 
> build/write_build_date_header.py already creates a deterministic BUILD_DATE
> with several tradeoffs. Cachability wants this to change infrequently, but
> things like HSTS need a "real" build date and want this to change frequently.
> The compromise is: The date changes once per day in official builds, and
> once a month in regular builds.
> 
> (We could use /Brepro in ldflags instead of /TIMESTAMP for unofficial builds to get
> the binary hash in the timestamp, but having the header timestamp match the BUILD_DATE
> define seems nice.)
> 
> So let's use that same time as timestamp in the PE/COFF header. lld-link has a
> /TIMESTAMP: flag we can use to pass in an explicit timestamp.
> 
> Since tools can't have deps, we need to compute the timestamp at gn time,
> so split write_build_date_header.py in two pieces: build/compute_build_timestamp.py
> that just prints the timestamp we want to use, and the old write_build_date_header.py, which
> now takes that timestamp and writes the header file.
> 
> Call compute_build_timestamp.py at gn time so that we can pass it in ldflags, and
> pass the resultl to write_build_date_header.py which keeps running as an action
> during build time (so that we at least don't need to write a file at gn time).
> 
> An additional wrinkle here is that the PE/COFF timestamp is used as one of just two
> keys per binary for uploading PE binaries to the symbol server, the other being file size.
> https://bugs.llvm.org/show_bug.cgi?id=35914#c0 has a good description of this, and
> tools/symsrc/img_fingerprint.py's GetImgFingerprint() is our implementation of it.
> But since we only upload binaries with symbols for official chrome builds to the symbol server,
> a timestamp that changes once a day should be still enough. (32-bit and 64-bit chromes
> have the same filename, and we might rarely build canary and beta and stable all on the
> same day, but them all being the same size seems highly unlikely.)
> 
> Bug:  843199 , 804926 ,330260
> Change-Id: I1d4193cc537ae0c4b2d6ac9281fad29de754dd6c
> Reviewed-on: https://chromium-review.googlesource.com/1161104
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> Reviewed-by: Hans Wennborg <hans@chromium.org>
> Commit-Queue: Nico Weber <thakis@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#580585}

TBR=thakis@chromium.org,hans@chromium.org,dpranke@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  843199 ,  804926 , 330260
Change-Id: Ib93697a82f8a9d3fb303b763609e82e0612887cd
Reviewed-on: https://chromium-review.googlesource.com/1166005
Reviewed-by: Alex Mineer <amineer@chromium.org>
Cr-Commit-Position: refs/branch-heads/3515@{#6}
Cr-Branched-From: 87042be6b9b11f1fe4f18fb565cadfea71f256e7-refs/heads/master@{#581084}
[modify] https://crrev.com/c234baa7e80a72e2dbd89bedd7abd047a12f68d4/base/BUILD.gn
[delete] https://crrev.com/26c10db8bff36a8b6fc073c0f38b1e9493cabb04/build/compute_build_timestamp.py
[modify] https://crrev.com/c234baa7e80a72e2dbd89bedd7abd047a12f68d4/build/config/win/BUILD.gn
[modify] https://crrev.com/c234baa7e80a72e2dbd89bedd7abd047a12f68d4/build/dotfile_settings.gni
[delete] https://crrev.com/26c10db8bff36a8b6fc073c0f38b1e9493cabb04/build/timestamp.gni
[modify] https://crrev.com/c234baa7e80a72e2dbd89bedd7abd047a12f68d4/build/write_build_date_header.py

Project Member

Comment 16 by bugdroid1@chromium.org, Aug 8

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2df0c83aee0a00bc0f7c26a22b68b4ea3eefef80

commit 2df0c83aee0a00bc0f7c26a22b68b4ea3eefef80
Author: Dirk Pranke <dpranke@chromium.org>
Date: Wed Aug 08 06:26:08 2018

Revert "win: write a deterministic-ish timestamp into the PE/COFF header instead of the current time"

This reverts commit ef36dc19986a90f49d0d31520c88d310d72bd038.

Reason for revert: This turns out to break the official android build, which apparently was relying on a broken aspect of the build that this fixed :(. See https://crbug.com/871173.

Original change's description:
> win: write a deterministic-ish timestamp into the PE/COFF header instead of the current time
>
> We used to set the timestamp to a hash of the binary, similar to
> https://blogs.msdn.microsoft.com/oldnewthing/20180103-00/?p=97705
> However, that caused an appcompat warning on Windows 7 to appear, which
> interpreted the hash as a timestamp. (It's possible that https://llvm.org/PR38429
> could help with that, but my guess it won't have an effect on Windows 7,
> which likely always believes that the the coff timestamp field always stores
> a timestamp).
>
> So currently we write the current time during linking in that field, but that's
> bad for build determinism and that in turn is bad for swarming test result cachability.
>
> build/write_build_date_header.py already creates a deterministic BUILD_DATE
> with several tradeoffs. Cachability wants this to change infrequently, but
> things like HSTS need a "real" build date and want this to change frequently.
> The compromise is: The date changes once per day in official builds, and
> once a month in regular builds.
>
> (We could use /Brepro in ldflags instead of /TIMESTAMP for unofficial builds to get
> the binary hash in the timestamp, but having the header timestamp match the BUILD_DATE
> define seems nice.)
>
> So let's use that same time as timestamp in the PE/COFF header. lld-link has a
> /TIMESTAMP: flag we can use to pass in an explicit timestamp.
>
> Since tools can't have deps, we need to compute the timestamp at gn time,
> so split write_build_date_header.py in two pieces: build/compute_build_timestamp.py
> that just prints the timestamp we want to use, and the old write_build_date_header.py, which
> now takes that timestamp and writes the header file.
>
> Call compute_build_timestamp.py at gn time so that we can pass it in ldflags, and
> pass the resultl to write_build_date_header.py which keeps running as an action
> during build time (so that we at least don't need to write a file at gn time).
>
> An additional wrinkle here is that the PE/COFF timestamp is used as one of just two
> keys per binary for uploading PE binaries to the symbol server, the other being file size.
> https://bugs.llvm.org/show_bug.cgi?id=35914#c0 has a good description of this, and
> tools/symsrc/img_fingerprint.py's GetImgFingerprint() is our implementation of it.
> But since we only upload binaries with symbols for official chrome builds to the symbol server,
> a timestamp that changes once a day should be still enough. (32-bit and 64-bit chromes
> have the same filename, and we might rarely build canary and beta and stable all on the
> same day, but them all being the same size seems highly unlikely.)
>
> Bug:  843199 , 804926 ,330260
> Change-Id: I1d4193cc537ae0c4b2d6ac9281fad29de754dd6c
> Reviewed-on: https://chromium-review.googlesource.com/1161104
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> Reviewed-by: Hans Wennborg <hans@chromium.org>
> Commit-Queue: Nico Weber <thakis@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#580585}

TBR=thakis@chromium.org,hans@chromium.org,dpranke@chromium.org
NOTRY=true

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  843199 ,  804926 , 330260
Change-Id: Ib93697a82f8a9d3fb303b763609e82e0612887cd
Reviewed-on: https://chromium-review.googlesource.com/1166203
Commit-Queue: Hans Wennborg <hans@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581485}
[modify] https://crrev.com/2df0c83aee0a00bc0f7c26a22b68b4ea3eefef80/base/BUILD.gn
[delete] https://crrev.com/35ec8d11347ddcdc425f63b8da279f3c3fcfaf92/build/compute_build_timestamp.py
[modify] https://crrev.com/2df0c83aee0a00bc0f7c26a22b68b4ea3eefef80/build/config/win/BUILD.gn
[modify] https://crrev.com/2df0c83aee0a00bc0f7c26a22b68b4ea3eefef80/build/dotfile_settings.gni
[delete] https://crrev.com/35ec8d11347ddcdc425f63b8da279f3c3fcfaf92/build/timestamp.gni
[modify] https://crrev.com/2df0c83aee0a00bc0f7c26a22b68b4ea3eefef80/build/write_build_date_header.py

Project Member

Comment 17 by bugdroid1@chromium.org, Aug 8

Labels: merge-merged-3516
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/72c03041cad06f204c6e1c09af414381e032e999

commit 72c03041cad06f204c6e1c09af414381e032e999
Author: Dirk Pranke <dpranke@chromium.org>
Date: Wed Aug 08 11:56:31 2018

Revert "win: write a deterministic-ish timestamp into the PE/COFF header instead of the current time"

This reverts commit ef36dc19986a90f49d0d31520c88d310d72bd038.

Reason for revert: This turns out to break the official android build, which apparently was relying on a broken aspect of the build that this fixed :(. See https://crbug.com/871173.

Original change's description:
> win: write a deterministic-ish timestamp into the PE/COFF header instead of the current time
>
> We used to set the timestamp to a hash of the binary, similar to
> https://blogs.msdn.microsoft.com/oldnewthing/20180103-00/?p=97705
> However, that caused an appcompat warning on Windows 7 to appear, which
> interpreted the hash as a timestamp. (It's possible that https://llvm.org/PR38429
> could help with that, but my guess it won't have an effect on Windows 7,
> which likely always believes that the the coff timestamp field always stores
> a timestamp).
>
> So currently we write the current time during linking in that field, but that's
> bad for build determinism and that in turn is bad for swarming test result cachability.
>
> build/write_build_date_header.py already creates a deterministic BUILD_DATE
> with several tradeoffs. Cachability wants this to change infrequently, but
> things like HSTS need a "real" build date and want this to change frequently.
> The compromise is: The date changes once per day in official builds, and
> once a month in regular builds.
>
> (We could use /Brepro in ldflags instead of /TIMESTAMP for unofficial builds to get
> the binary hash in the timestamp, but having the header timestamp match the BUILD_DATE
> define seems nice.)
>
> So let's use that same time as timestamp in the PE/COFF header. lld-link has a
> /TIMESTAMP: flag we can use to pass in an explicit timestamp.
>
> Since tools can't have deps, we need to compute the timestamp at gn time,
> so split write_build_date_header.py in two pieces: build/compute_build_timestamp.py
> that just prints the timestamp we want to use, and the old write_build_date_header.py, which
> now takes that timestamp and writes the header file.
>
> Call compute_build_timestamp.py at gn time so that we can pass it in ldflags, and
> pass the resultl to write_build_date_header.py which keeps running as an action
> during build time (so that we at least don't need to write a file at gn time).
>
> An additional wrinkle here is that the PE/COFF timestamp is used as one of just two
> keys per binary for uploading PE binaries to the symbol server, the other being file size.
> https://bugs.llvm.org/show_bug.cgi?id=35914#c0 has a good description of this, and
> tools/symsrc/img_fingerprint.py's GetImgFingerprint() is our implementation of it.
> But since we only upload binaries with symbols for official chrome builds to the symbol server,
> a timestamp that changes once a day should be still enough. (32-bit and 64-bit chromes
> have the same filename, and we might rarely build canary and beta and stable all on the
> same day, but them all being the same size seems highly unlikely.)
>
> Bug:  843199 , 804926 ,330260
> Change-Id: I1d4193cc537ae0c4b2d6ac9281fad29de754dd6c
> Reviewed-on: https://chromium-review.googlesource.com/1161104
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> Reviewed-by: Hans Wennborg <hans@chromium.org>
> Commit-Queue: Nico Weber <thakis@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#580585}

TBR=thakis@chromium.org,hans@chromium.org,dpranke@chromium.org
NOTRY=true

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  843199 ,  804926 , 330260
Change-Id: Ib93697a82f8a9d3fb303b763609e82e0612887cd
Reviewed-on: https://chromium-review.googlesource.com/1166203
Commit-Queue: Hans Wennborg <hans@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#581485}(cherry picked from commit 2df0c83aee0a00bc0f7c26a22b68b4ea3eefef80)
Reviewed-on: https://chromium-review.googlesource.com/1167162
Reviewed-by: Boris Sazonov <bsazonov@chromium.org>
Cr-Commit-Position: refs/branch-heads/3516@{#3}
Cr-Branched-From: 9cc04d3302503238bd6a70ac8f0402351e1916a1-refs/heads/master@{#581409}
[modify] https://crrev.com/72c03041cad06f204c6e1c09af414381e032e999/base/BUILD.gn
[delete] https://crrev.com/c10cfa8427d4c07ef786eaf2053dee663f6150c4/build/compute_build_timestamp.py
[modify] https://crrev.com/72c03041cad06f204c6e1c09af414381e032e999/build/config/win/BUILD.gn
[modify] https://crrev.com/72c03041cad06f204c6e1c09af414381e032e999/build/dotfile_settings.gni
[delete] https://crrev.com/c10cfa8427d4c07ef786eaf2053dee663f6150c4/build/timestamp.gni
[modify] https://crrev.com/72c03041cad06f204c6e1c09af414381e032e999/build/write_build_date_header.py

Project Member

Comment 18 by bugdroid1@chromium.org, Aug 17

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/967d1f1e6adc9b7641a5b0174c48f0883e293880

commit 967d1f1e6adc9b7641a5b0174c48f0883e293880
Author: Nico Weber <thakis@chromium.org>
Date: Fri Aug 17 02:42:02 2018

Reland "win: write a deterministic-ish timestamp into the PE/COFF header instead of the current time"

This reverts commit 2df0c83aee0a00bc0f7c26a22b68b4ea3eefef80.

Reason for revert: official android should be fine with this after #583420

Original change's description:
> Revert "win: write a deterministic-ish timestamp into the PE/COFF header instead of the current time"
> 
> This reverts commit ef36dc19986a90f49d0d31520c88d310d72bd038.
> 
> Reason for revert: This turns out to break the official android build, which apparently was relying on a broken aspect of the build that this fixed :(. See https://crbug.com/871173.
> 
> Original change's description:
> > win: write a deterministic-ish timestamp into the PE/COFF header instead of the current time
> >
> > We used to set the timestamp to a hash of the binary, similar to
> > https://blogs.msdn.microsoft.com/oldnewthing/20180103-00/?p=97705
> > However, that caused an appcompat warning on Windows 7 to appear, which
> > interpreted the hash as a timestamp. (It's possible that https://llvm.org/PR38429
> > could help with that, but my guess it won't have an effect on Windows 7,
> > which likely always believes that the the coff timestamp field always stores
> > a timestamp).
> >
> > So currently we write the current time during linking in that field, but that's
> > bad for build determinism and that in turn is bad for swarming test result cachability.
> >
> > build/write_build_date_header.py already creates a deterministic BUILD_DATE
> > with several tradeoffs. Cachability wants this to change infrequently, but
> > things like HSTS need a "real" build date and want this to change frequently.
> > The compromise is: The date changes once per day in official builds, and
> > once a month in regular builds.
> >
> > (We could use /Brepro in ldflags instead of /TIMESTAMP for unofficial builds to get
> > the binary hash in the timestamp, but having the header timestamp match the BUILD_DATE
> > define seems nice.)
> >
> > So let's use that same time as timestamp in the PE/COFF header. lld-link has a
> > /TIMESTAMP: flag we can use to pass in an explicit timestamp.
> >
> > Since tools can't have deps, we need to compute the timestamp at gn time,
> > so split write_build_date_header.py in two pieces: build/compute_build_timestamp.py
> > that just prints the timestamp we want to use, and the old write_build_date_header.py, which
> > now takes that timestamp and writes the header file.
> >
> > Call compute_build_timestamp.py at gn time so that we can pass it in ldflags, and
> > pass the resultl to write_build_date_header.py which keeps running as an action
> > during build time (so that we at least don't need to write a file at gn time).
> >
> > An additional wrinkle here is that the PE/COFF timestamp is used as one of just two
> > keys per binary for uploading PE binaries to the symbol server, the other being file size.
> > https://bugs.llvm.org/show_bug.cgi?id=35914#c0 has a good description of this, and
> > tools/symsrc/img_fingerprint.py's GetImgFingerprint() is our implementation of it.
> > But since we only upload binaries with symbols for official chrome builds to the symbol server,
> > a timestamp that changes once a day should be still enough. (32-bit and 64-bit chromes
> > have the same filename, and we might rarely build canary and beta and stable all on the
> > same day, but them all being the same size seems highly unlikely.)
> >
> > Bug:  843199 , 804926 ,330260
> > Change-Id: I1d4193cc537ae0c4b2d6ac9281fad29de754dd6c
> > Reviewed-on: https://chromium-review.googlesource.com/1161104
> > Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> > Reviewed-by: Hans Wennborg <hans@chromium.org>
> > Commit-Queue: Nico Weber <thakis@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#580585}
> 
> TBR=thakis@chromium.org,hans@chromium.org,dpranke@chromium.org
> NOTRY=true
> 
> # Not skipping CQ checks because original CL landed > 1 day ago.
> 
> Bug:  843199 ,  804926 , 330260
> Change-Id: Ib93697a82f8a9d3fb303b763609e82e0612887cd
> Reviewed-on: https://chromium-review.googlesource.com/1166203
> Commit-Queue: Hans Wennborg <hans@chromium.org>
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> Reviewed-by: Nico Weber <thakis@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#581485}

TBR=thakis@chromium.org,hans@chromium.org,dpranke@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  843199 ,  804926 , 330260
Change-Id: I136e405c84eba3f61a4ac96b2017a34ade0cfba6
Reviewed-on: https://chromium-review.googlesource.com/1179281
Commit-Queue: Nico Weber <thakis@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583945}
[modify] https://crrev.com/967d1f1e6adc9b7641a5b0174c48f0883e293880/base/BUILD.gn
[add] https://crrev.com/967d1f1e6adc9b7641a5b0174c48f0883e293880/build/compute_build_timestamp.py
[modify] https://crrev.com/967d1f1e6adc9b7641a5b0174c48f0883e293880/build/config/win/BUILD.gn
[modify] https://crrev.com/967d1f1e6adc9b7641a5b0174c48f0883e293880/build/dotfile_settings.gni
[add] https://crrev.com/967d1f1e6adc9b7641a5b0174c48f0883e293880/build/timestamp.gni
[modify] https://crrev.com/967d1f1e6adc9b7641a5b0174c48f0883e293880/build/write_build_date_header.py

Sign in to add a comment