Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: add -Wdate-time to Werror flags #17880

Merged
merged 1 commit into from Jan 8, 2020

Conversation

fanquake
Copy link
Member

@fanquake fanquake commented Jan 6, 2020

-Wdate-time
Warn when macros TIME, DATE or TIMESTAMP are encountered as
they might prevent bit-wise-identical reproducible compilations.

This is supported by GCC and Clang.

Example output:

  CXX      bitcoind-bitcoind.o
bitcoind.cpp:48:20: warning: expansion of date or time macro is not reproducible [-Wdate-time]
    printf("%s\n", __TIMESTAMP__);
                   ^
bitcoind.cpp:49:20: warning: expansion of date or time macro is not reproducible [-Wdate-time]
    printf("%s\n", __TIME__);
                   ^
bitcoind.cpp:50:20: warning: expansion of date or time macro is not reproducible [-Wdate-time]
    printf("%s\n", __DATE__);
                   ^
3 warnings generated.

-Wdate-time

    Warn when macros __TIME__, __DATE__ or __TIMESTAMP__ are encountered as 
    they might prevent bit-wise-identical reproducible compilations.

This is supported by GCC and Clang.
https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html
https://clang.llvm.org/docs/DiagnosticsReference.html#wdate-time
@dongcarl
Copy link
Contributor

dongcarl commented Jan 6, 2020

Concept ACK!

@practicalswift
Copy link
Contributor

ACK b0a2540 -- diff looks correct and guarding against potential non-reproducibility is good :)

@DrahtBot
Copy link
Contributor

DrahtBot commented Jan 6, 2020

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #15382 (util: add runCommandParseJSON by Sjors)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@promag
Copy link
Member

promag commented Jan 6, 2020

Concept ACK.

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK b0a2540

@maflcko
Copy link
Member

maflcko commented Jan 6, 2020

Last use was removed in d096d22. I can't imagine they ever get added back in, but I don't mind this warning either if others find it useful.

@DrahtBot
Copy link
Contributor

DrahtBot commented Jan 6, 2020

Gitian builds

File commit b949ac9
(master)
commit 1964fe6
(master and this pull)
bitcoin-0.19.99-aarch64-linux-gnu-debug.tar.gz a14207517d25fece... 97978b864c832627...
bitcoin-0.19.99-aarch64-linux-gnu.tar.gz 51fd58ca7f14c4bb... 03b2d7fcdb1ed36c...
bitcoin-0.19.99-arm-linux-gnueabihf-debug.tar.gz 0c13785557c412cb... c36722c9c1782f44...
bitcoin-0.19.99-arm-linux-gnueabihf.tar.gz d717d387941a6f52... 97c21e223f28af42...
bitcoin-0.19.99-i686-pc-linux-gnu-debug.tar.gz 485858210efe05c2... 71625a6c514c33cc...
bitcoin-0.19.99-i686-pc-linux-gnu.tar.gz 69a1cf3b9bf79fbe... 0461c4dce9f66d5d...
bitcoin-0.19.99-osx-unsigned.dmg c876fb9db3d4d189... 979c194e7b7941c2...
bitcoin-0.19.99-osx64.tar.gz dd8a3ebd540cc66c... a8f368d8437eb08a...
bitcoin-0.19.99-riscv64-linux-gnu-debug.tar.gz 6642fc1c8d1635d1... a697223a5950dc2a...
bitcoin-0.19.99-riscv64-linux-gnu.tar.gz 6cccb448a4262e7b... 63b9b2ba5ef6a51c...
bitcoin-0.19.99-win64-debug.zip f94ba1033e8cc317... 4106d6ffeac35c5c...
bitcoin-0.19.99-win64-setup-unsigned.exe 8b59c3214bfb6a46... f7edd163f772677c...
bitcoin-0.19.99-win64.zip a23d7c88ff460d78... 29ec1d072e952d4b...
bitcoin-0.19.99-x86_64-linux-gnu-debug.tar.gz 62a5918ebf22fc21... 30fe566c60efad33...
bitcoin-0.19.99-x86_64-linux-gnu.tar.gz 98cd19fe59bc6eff... 9b7f191b2d7a5284...
bitcoin-0.19.99.tar.gz 5a211d09899b034c... 7cbc81a7b15f898d...
bitcoin-core-linux-0.20-res.yml 7ed024b81ed9b639... 8e38087d74e030d3...
bitcoin-core-osx-0.20-res.yml 5947a980f2b74084... ad20c4cb76cfea3d...
bitcoin-core-win-0.20-res.yml 9e64f2d02f143a21... dec85693cdd1649b...
linux-build.log ef1f26008e10e842... b90d34bc115d72b9...
osx-build.log 4ab5188b79e2e50c... 3914214927f38de3...
win-build.log 0567536136b41af8... 64c3472f30743fd6...
bitcoin-core-linux-0.20-res.yml.diff dcbda83ade9604ac...
bitcoin-core-osx-0.20-res.yml.diff 6cc626e6ab14c364...
bitcoin-core-win-0.20-res.yml.diff ffa28ab12ba9f0f1...
linux-build.log.diff c9b200f7caf63a10...
osx-build.log.diff 8f5138075948e6ce...
win-build.log.diff 28422ee61fb64c33...

Copy link
Member

@promag promag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to confirm, this change makes the compiler emit just a warning?

@fanquake
Copy link
Member Author

fanquake commented Jan 7, 2020

Just to confirm, this change makes the compiler emit just a warning?

Yes. Unless you —enable-werror.

@promag
Copy link
Member

promag commented Jan 8, 2020

Tested ACK b0a2540 on macos with clang. Already had --enable-werror, added a wild printf("%s\n", __TIMESTAMP__) and got the following error:

error: expansion of date or time macro is not reproducible [-Werror,-Wdate-time]
printf("%s\n", __TIMESTAMP__);
               ^
1 error generated.

@laanwj
Copy link
Member

laanwj commented Jan 8, 2020

ACK b0a2540

laanwj added a commit that referenced this pull request Jan 8, 2020
b0a2540 build: add Wdate-time to Werror flags (fanquake)

Pull request description:

  `-Wdate-time`
  Warn when macros __TIME__, __DATE__ or __TIMESTAMP__ are encountered as
  they might prevent bit-wise-identical reproducible compilations.

  This is supported by [GCC](https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html) and [Clang](https://clang.llvm.org/docs/DiagnosticsReference.html#wdate-time).

  Example output:
  ```bash
    CXX      bitcoind-bitcoind.o
  bitcoind.cpp:48:20: warning: expansion of date or time macro is not reproducible [-Wdate-time]
      printf("%s\n", __TIMESTAMP__);
                     ^
  bitcoind.cpp:49:20: warning: expansion of date or time macro is not reproducible [-Wdate-time]
      printf("%s\n", __TIME__);
                     ^
  bitcoind.cpp:50:20: warning: expansion of date or time macro is not reproducible [-Wdate-time]
      printf("%s\n", __DATE__);
                     ^
  3 warnings generated.
  ```

ACKs for top commit:
  practicalswift:
    ACK b0a2540 -- diff looks correct and guarding against potential non-reproducibility is good :)
  promag:
    Tested ACK b0a2540 on macos with clang. Already had `--enable-werror`, added a wild `printf("%s\n", __TIMESTAMP__)` and got the following error:
  laanwj:
    ACK b0a2540
  hebasto:
    ACK b0a2540

Tree-SHA512: b3a0b426e06dcd0c0baa94118c31158760b9690a8d0a15b5a2d544cb0879522e02817e134ef7346c707de09719818fc7e4bad1b3ad6b2dfe5e3c4169cdf5cb0d
@laanwj laanwj merged commit b0a2540 into bitcoin:master Jan 8, 2020
@fanquake fanquake deleted the add_Werror_date_time branch January 8, 2020 13:48
kwvg added a commit to kwvg/dash that referenced this pull request Jun 17, 2021
kwvg added a commit to kwvg/dash that referenced this pull request Jun 17, 2021
kwvg added a commit to kwvg/dash that referenced this pull request Jun 17, 2021
UdjinM6 added a commit to dashpay/dash that referenced this pull request Jun 23, 2021
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants