Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2284453002:
Make compiles fail when __DATE__, __TIME__, or __TIMESTAMP_ are used. (Closed)
|
Created:
4 years, 3 months ago by Dirk Pranke Modified:
4 years, 3 months ago CC:
chromium-reviews, yyanagisawa, mithro, justincohen Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionMake compiles fail when __DATE__, __TIME__, or __TIMESTAMP_ are used.
Using these compiler defines makes builds nondeterministic,
which we don't want. The -Wdate-time flag (plus -Werror) should
make use of these flags a fatal error. The flag should be supported
by both gcc and clang, so even though we can't check this w/ visual
studio, we should largely still get coverage on windows via win clang.
R=thakis@chromium.org
BUG=314403
Patch Set 1 #
Total comments: 1
Patch Set 2 : disable for nacl, improve comment #Messages
Total messages: 17 (10 generated)
The CQ bit was checked by dpranke@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm if it builds https://codereview.chromium.org/2284453002/diff/1/build/config/compiler/BUILD.gn File build/config/compiler/BUILD.gn (right): https://codereview.chromium.org/2284453002/diff/1/build/config/compiler/BUILD... build/config/compiler/BUILD.gn:970: # Disable __DATE__ and __TIME__ to help make builds deterministic. Maybe add a note along the lines of "If this fires in third-party code you're adding, make that third-party code not use __DATE__ / __TIME__. Do not disable this warning, we must have deterministic builds"
On 2016/08/25 19:20:04, Nico wrote: > lgtm if it builds > > https://codereview.chromium.org/2284453002/diff/1/build/config/compiler/BUILD.gn > File build/config/compiler/BUILD.gn (right): > > https://codereview.chromium.org/2284453002/diff/1/build/config/compiler/BUILD... > build/config/compiler/BUILD.gn:970: # Disable __DATE__ and __TIME__ to help make > builds deterministic. > Maybe add a note along the lines of "If this fires in third-party code you're > adding, make that third-party code not use __DATE__ / __TIME__. Do not disable > this warning, we must have deterministic builds" Seems like a good idea. I'll at least add a comment about why this is a common warning and not just part of chromium code.
rohitrao@chromium.org changed reviewers: + rohitrao@chromium.org
Will this cause the third_party/class-dump code to throw errors again? +Justin and I can run this through the internal iOS trybots and make sure we don't have any additional errors.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: chromeos_amd64-generic_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...) chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...) linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
On 2016/08/25 19:28:59, rohitrao wrote: > Will this cause the third_party/class-dump code to throw errors again? > > +Justin and I can run this through the internal iOS trybots and make sure we > don't have any additional errors. Yes before enabling this CL we'll have to fix all cases first. I think it's a good thing to do. This will require patches in third parties like flatbuffers.
On 2016/08/25 19:45:13, M-A Ruel wrote: > On 2016/08/25 19:28:59, rohitrao wrote: > > Will this cause the third_party/class-dump code to throw errors again? > > > > +Justin and I can run this through the internal iOS trybots and make sure we > > don't have any additional errors. > > Yes before enabling this CL we'll have to fix all cases first. I think it's a > good thing to do. This will require patches in third parties like flatbuffers. Yup. And clearly I need to disable this for NaCl :).
Description was changed from ========== Add a compiler warning to check for __DATE__ and __TIME__ usage. Using the __DATE__ and __TIME__ (and __TIMESTAMP__) compiler defines can make builds nondeterministic, which we don't want. The -Wdate-time flag (plus -Werror) should make use of these flags a fatal error. The flag should be supported by both gcc and clang, so even though we can't check this w/ visual studio, we should largely still get coverage on windows via win clang. R=thakis@chromium.org BUG=314403 ========== to ========== Make compiles fail when __DATE__, __TIME__, or __TIMESTAMP_ are used. Using these compiler defines makes builds nondeterministic, which we don't want. The -Wdate-time flag (plus -Werror) should make use of these flags a fatal error. The flag should be supported by both gcc and clang, so even though we can't check this w/ visual studio, we should largely still get coverage on windows via win clang. R=thakis@chromium.org BUG=314403 ==========
The CQ bit was checked by dpranke@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...) chromeos_x86-generic_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...) linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
Message was sent while issue was closed.
Okay, this clearly isn't going to work at the moment, and I'm not signing up to fix the errors, so I'm going to close this for now. We can always try to reland once the problems have been addressed. |
