refactor: Remove gmtime* #29081

maflcko · 2023-12-14T17:22:06Z

Now that the ChronoSanityCheck has passed for everyone with C++17 and is guaranteed by C++20 to always pass, remove it.

Also, remove gmtime_r and gmtime_s and replace them with year_month_day+hh_mm_ss from C++20.

DrahtBot · 2023-12-14T17:22:09Z

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

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	sipa, fanquake
Concept ACK	theuni, hebasto

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#29494 (build: Assume HAVE_CONFIG_H, Add IWYU pragma keep to bitcoin-config.h includes by maflcko)

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.

theuni · 2023-12-14T18:46:37Z

Finally :)

Concept ACK.

theuni · 2023-12-14T18:56:04Z

util/time.cpp:60:24: error: ‘year_month_day’ in namespace ‘std::chrono’ does not name a type

It would have been too easy if it had just worked in the real world :p

Edit: looks like this is first available with libstdc++ 11.1: gcc-mirror/gcc@03d5044 . I'm assuming our mingw builder is using something older?

maflcko · 2023-12-14T19:01:49Z

Jup, a bump to g++-11 should be fine, but currently not possible because the guix build is still on g++-10 🫠

DrahtBot · 2023-12-15T06:24:29Z

Guix builds (on x86_64)

File	commit `1b2dedb` (master)	commit `e652b60` (master and this pull)
SHA256SUMS.part	`e26cbf7c47c1e628...`
*-aarch64-linux-gnu-debug.tar.gz	`751497dd9987c54a...`
*-aarch64-linux-gnu.tar.gz	`139b5e7f8534df57...`
*-arm-linux-gnueabihf-debug.tar.gz	`ff31610bf056d1f6...`
*-arm-linux-gnueabihf.tar.gz	`57b94de21440b37e...`
*-arm64-apple-darwin-unsigned.tar.gz	`a5ca39be7a16056d...`
*-arm64-apple-darwin-unsigned.zip	`25b24e68f83cd916...`
*-arm64-apple-darwin.tar.gz	`9bb85e4892af91c4...`
*-powerpc64-linux-gnu-debug.tar.gz	`afcfd38b7b9fddd8...`
*-powerpc64-linux-gnu.tar.gz	`3035b844df9e0ee4...`
*-powerpc64le-linux-gnu-debug.tar.gz	`639ce27bb7a7531a...`
*-powerpc64le-linux-gnu.tar.gz	`827691729935bbce...`
*-riscv64-linux-gnu-debug.tar.gz	`2fa0f3bfb31b38cc...`
*-riscv64-linux-gnu.tar.gz	`eb9b434686d85d15...`
*-x86_64-apple-darwin-unsigned.tar.gz	`4afc08d1ce24a2fb...`
*-x86_64-apple-darwin-unsigned.zip	`f407031433f298c9...`
*-x86_64-apple-darwin.tar.gz	`ef6501565607adbb...`
*-x86_64-linux-gnu-debug.tar.gz	`189ec3e6b94673ba...`
*-x86_64-linux-gnu.tar.gz	`012164ecdb96c0be...`
*.tar.gz	`22783dcb8deddd29...`	`7a4458f64b88fc64...`
guix_build.log	`e04a367c86d8bc6b...`	`a2fb30e702314e36...`
guix_build.log.diff		`8718267e44cbcde2...`

fanquake

Concept ACK - can we also drop the Boost date_time usage from ParseISO8601DateTime in favour of the std lib?

configure.ac

maflcko · 2023-12-15T10:56:40Z

Concept ACK - can we also drop the Boost date_time usage from ParseISO8601DateTime in favour of the std lib?

Probably no. std::chrono::parse isn't released yet in gcc at all, or did you mean something else?

fanquake · 2023-12-15T11:04:36Z

Probably no. std::chrono::parse isn't released yet in gcc at all, or did you mean something else?

Damn. I just meant any possibility to get rid of Boost.

maflcko · 2023-12-15T11:15:26Z

Probably no. std::chrono::parse isn't released yet in gcc at all, or did you mean something else?

Damn. I just meant any possibility to get rid of Boost.

At least this one is header-only in a single translation unit, so should be fine to stay around for another 3 years

fanquake · 2024-03-18T14:57:49Z

Want to rebase now that we've got 11.

This reverts commit 3c2e16b.

hebasto · 2024-03-18T16:17:22Z

Concept ACK.

DrahtBot · 2024-03-19T17:50:16Z

Guix builds (on x86_64)

File	commit `5d045c3` (master)	commit `515f85a` (master and this pull)
SHA256SUMS.part	`809aed301ff8b9e5...`	`0c736297b04cb556...`
*-aarch64-linux-gnu-debug.tar.gz	`0e3b07f59f0f4c99...`	`956b14935601ae50...`
*-aarch64-linux-gnu.tar.gz	`fa7a1044e52953da...`	`3b783449b88c58b9...`
*-arm-linux-gnueabihf-debug.tar.gz	`e9f2fe00f9c22706...`	`ef6b8a0091d2063f...`
*-arm-linux-gnueabihf.tar.gz	`0946d8ec6468c624...`	`518489c7c9256386...`
*-arm64-apple-darwin-unsigned.tar.gz	`9b9672f33e488b30...`	`c799e3094f66b8c3...`
*-arm64-apple-darwin-unsigned.zip	`3bf9c92b1d64ff9c...`	`c2d2f8a3ece50a5b...`
*-arm64-apple-darwin.tar.gz	`0c29f8b9a161ce17...`	`89a0f08363652a32...`
*-powerpc64-linux-gnu-debug.tar.gz	`a2d56a46dd325fe2...`	`f66f55f15781e574...`
*-powerpc64-linux-gnu.tar.gz	`1db308988c8f7ee5...`	`2da946cf9a109852...`
*-riscv64-linux-gnu-debug.tar.gz	`debd4a6b170e3262...`	`e55815c330d9e940...`
*-riscv64-linux-gnu.tar.gz	`1d4ac8875fc68c7e...`	`0bc9a0a075401061...`
*-x86_64-apple-darwin-unsigned.tar.gz	`31f94a6ff65a5616...`	`4348dc9559fdddeb...`
*-x86_64-apple-darwin-unsigned.zip	`4f2ea7f3ce709d41...`	`1a094269c16c131e...`
*-x86_64-apple-darwin.tar.gz	`e1a925b74649b303...`	`2af9366dd0a5125f...`
*-x86_64-linux-gnu-debug.tar.gz	`935d6a71805a4032...`	`e19f2c2d77524b2f...`
*-x86_64-linux-gnu.tar.gz	`48053325e45ae5e7...`	`bb585c6b2bafb02f...`
*.tar.gz	`fbfea13acd4d867a...`	`da8ea0dea59e8a25...`
guix_build.log	`5b388141802142e6...`	`8eb1ac5d9cef005e...`
guix_build.log.diff		`e62f1da5db1a3bd0...`

fanquake · 2024-04-04T12:34:48Z

cc @theuni you might want to circle back here now that this is unblocked.

sipa · 2024-04-04T13:37:29Z

utACK fa9f36b

fanquake

ACK fa9f36b - more std lib & even less stuff to port.

theuni · 2024-04-05T16:09:16Z

Post-merge utACK fa9f36b

Backport changes from bitcoin#29081.

6d4584b fixup! cmake: Check system symbols (Hennadii Stepanov) Pull request description: Backport changes from bitcoin#29081. ACKs for top commit: theuni: utACK 6d4584b Tree-SHA512: 079f14184db01c2a90acca435330d1b19b95dc21b9e37d5b4ba3c628d2f205c6590f53e6768c32ca633323a8ab7ed10e87a3f87fdb7b7a27378b4c3f5fa0519b

fa9f36b build: Remove HAVE_GMTIME_R (MarcoFalke) fa72dcb refactor: FormatISO8601* without gmtime* (MarcoFalke) fa2c486 Revert "time: add runtime sanity check" (MarcoFalke) Pull request description: Now that the `ChronoSanityCheck` has passed for everyone with C++17 and is guaranteed by C++20 to always pass, remove it. Also, remove `gmtime_r` and `gmtime_s` and replace them with `year_month_day`+`hh_mm_ss` from C++20. ACKs for top commit: sipa: utACK fa9f36b fanquake: ACK fa9f36b - more std lib & even less stuff to port. Tree-SHA512: a9e7e805b757b7dade0bcc3f95273a7dc4f68622630d74838339789dd203ad7542d36b2e090a93b2bc5a7ecc383207dd7ec82c68147108bdac7ce44f088c8c9a

DrahtBot added the Refactoring label Dec 14, 2023

maflcko mentioned this pull request Dec 14, 2023

refactor: Replace GetTimeMicros by SystemClock #27233

Merged

maflcko marked this pull request as draft December 14, 2023 17:42

maflcko added the DrahtBot Guix build requested label Dec 14, 2023

DrahtBot added the CI failed label Dec 14, 2023

theuni mentioned this pull request Dec 14, 2023

build: Require libc++-16 or later #29077

Closed

DrahtBot removed the DrahtBot Guix build requested label Dec 15, 2023

fanquake reviewed Dec 15, 2023

View reviewed changes

configure.ac Show resolved Hide resolved

maflcko force-pushed the 2312-no-gmtime- branch from fa53215 to e712364 Compare December 15, 2023 10:53

maflcko added this to the 28.0 milestone Dec 15, 2023

fanquake mentioned this pull request Jan 5, 2024

util: remove Boost posix_time usage from GetTime* #21110

Merged

maflcko force-pushed the 2312-no-gmtime- branch 2 times, most recently from faedf56 to d53a1f8 Compare March 11, 2024 15:41

DrahtBot removed the CI failed label Mar 12, 2024

This was referenced Mar 12, 2024

build: Assume HAVE_CONFIG_H, Add IWYU pragma keep to bitcoin-config.h includes #29494

Merged

C++20 std::views::reverse #28687

Merged

build: no-longer disable WARN_CXXFLAGS when CXXFLAGS is set #25972

Merged

DrahtBot added the CI failed label Mar 13, 2024

maflcko force-pushed the 2312-no-gmtime- branch from 8905a5d to 0950849 Compare March 14, 2024 11:55

DrahtBot removed the CI failed label Mar 14, 2024

Revert "time: add runtime sanity check"

fa2c486

This reverts commit 3c2e16b.

maflcko removed this from the 28.0 milestone Mar 18, 2024

maflcko marked this pull request as ready for review March 18, 2024 15:01

MarcoFalke added 2 commits March 18, 2024 16:01

refactor: FormatISO8601* without gmtime*

fa72dcb

build: Remove HAVE_GMTIME_R

fa9f36b

maflcko force-pushed the 2312-no-gmtime- branch from 0950849 to fa9f36b Compare March 18, 2024 15:01

maflcko added the DrahtBot Guix build requested label Mar 19, 2024

DrahtBot removed the DrahtBot Guix build requested label Mar 19, 2024

DrahtBot requested review from fanquake, hebasto and theuni April 4, 2024 13:37

fanquake approved these changes Apr 5, 2024

View reviewed changes

fanquake merged commit c353025 into bitcoin:master Apr 5, 2024

maflcko deleted the 2312-no-gmtime- branch April 8, 2024 10:39

Pttn added a commit to RiecoinTeam/Riecoin that referenced this pull request Apr 13, 2024

Merge bitcoin#29081

4169b51

hebasto mentioned this pull request Apr 16, 2024

cmake: Regular rebasing of the cmake-staging branch hebasto/bitcoin#148

Closed

hebasto added a commit to hebasto/bitcoin that referenced this pull request Apr 18, 2024

fixup! cmake: Check system symbols

6d4584b

Backport changes from bitcoin#29081.

hebasto mentioned this pull request Apr 18, 2024

cmake: Backport changes from PR29081 hebasto/bitcoin#150

Merged

rkrux mentioned this pull request May 7, 2024

test: Assumeutxo: snapshots with less work should not be loaded #29428

Merged

edilmedeiros mentioned this pull request Jul 30, 2024

v27.1 won't compile on Debian 12. #30550

Closed

1 task

bitcoin locked and limited conversation to collaborators Apr 8, 2025

refactor: Remove gmtime* #29081

refactor: Remove gmtime* #29081

Uh oh!

Conversation

maflcko commented Dec 14, 2023

Uh oh!

DrahtBot commented Dec 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage

Reviews

Conflicts

Uh oh!

theuni commented Dec 14, 2023

Uh oh!

theuni commented Dec 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maflcko commented Dec 14, 2023

Uh oh!

DrahtBot commented Dec 15, 2023

Guix builds (on x86_64)

Uh oh!

fanquake left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

maflcko commented Dec 15, 2023

Uh oh!

fanquake commented Dec 15, 2023

Uh oh!

maflcko commented Dec 15, 2023

Uh oh!

fanquake commented Mar 18, 2024

Uh oh!

hebasto commented Mar 18, 2024

Uh oh!

DrahtBot commented Mar 19, 2024

Guix builds (on x86_64)

Uh oh!

fanquake commented Apr 4, 2024

Uh oh!

sipa commented Apr 4, 2024

Uh oh!

fanquake left a comment

Choose a reason for hiding this comment

Uh oh!

theuni commented Apr 5, 2024

Uh oh!

Uh oh!

DrahtBot commented Dec 14, 2023 •

edited

Loading

theuni commented Dec 14, 2023 •

edited

Loading