refactor: Remove gmtime* #29081

maflcko commented at 5:22 PM on December 14, 2023: member

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 commented at 5:22 PM on December 14, 2023: contributor

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.

DrahtBot added the label Refactoring on Dec 14, 2023

maflcko marked this as a draft on Dec 14, 2023

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

DrahtBot added the label CI failed on Dec 14, 2023

theuni commented at 6:46 PM on December 14, 2023: member

Finally :)

Concept ACK.

theuni commented at 6:56 PM on December 14, 2023: member

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: https://github.com/gcc-mirror/gcc/commit/03d5044b31f7bf94fcda4136b4ed87a5fee7735d . I'm assuming our mingw builder is using something older?

maflcko commented at 7:01 PM on December 14, 2023: member

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

DrahtBot commented at 6:24 AM on December 15, 2023: contributor

Guix builds (on x86_64)

File	commit 1b2dedbf5c4520774df020d06d848f29e3439b52<br>(master)	commit e652b600be5ef4b0feb9695b1eabdedb4f64c58d<br>(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...`

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

in configure.ac:1110 in fa53215d04 outdated

1143 | -       [ AC_MSG_RESULT([yes])],
1144 | -       [ AC_MSG_RESULT([no]); AC_MSG_ERROR([Both gmtime_r and gmtime_s are unavailable]) ]
1145 | -    )
1146 | -  ]
1147 | -)
1148 | -

fanquake commented at 10:20 AM on December 15, 2023:

Can also drop AC_SUBST(HAVE_GMTIME_R).

maflcko commented at 10:54 AM on December 15, 2023:

Thx, done

fanquake commented at 10:21 AM on December 15, 2023: member

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

maflcko force-pushed on Dec 15, 2023

maflcko commented at 10:56 AM on December 15, 2023: member

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 commented at 11:04 AM on December 15, 2023: member

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 commented at 11:15 AM on December 15, 2023: member

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

maflcko added this to the milestone 28.0 on Dec 15, 2023

maflcko force-pushed on Mar 11, 2024

DrahtBot removed the label CI failed on Mar 12, 2024

DrahtBot added the label CI failed on Mar 13, 2024

maflcko force-pushed on Mar 14, 2024

DrahtBot removed the label CI failed on Mar 14, 2024

fanquake commented at 2:57 PM on March 18, 2024: member

Want to rebase now that we've got 11.

Revert "time: add runtime sanity check"

This reverts commit 3c2e16be22ae04bf56663ee5ec1554d0d569741b.

fa2c486afc

maflcko removed this from the milestone 28.0 on Mar 18, 2024

maflcko marked this as ready for review on Mar 18, 2024

refactor: FormatISO8601* without gmtime* fa72dcbfa5

build: Remove HAVE_GMTIME_R fa9f36baba

maflcko force-pushed on Mar 18, 2024

hebasto commented at 4:17 PM on March 18, 2024: member

Concept ACK.

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

DrahtBot commented at 5:50 PM on March 19, 2024: contributor

Guix builds (on x86_64)

File	commit 5d045c31a537b417fe840271c6ed961f1d5cb130<br>(master)	commit 515f85ab07d6eabd4afa8d78080e16b6b12489ed<br>(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...`

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

fanquake commented at 12:34 PM on April 4, 2024: member

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

sipa commented at 1:37 PM on April 4, 2024: member

utACK fa9f36babaceba6ab2f88e64bc4bc2956f58871f

DrahtBot requested review from fanquake on Apr 4, 2024

DrahtBot requested review from hebasto on Apr 4, 2024

DrahtBot requested review from theuni on Apr 4, 2024

fanquake approved

fanquake commented at 3:56 PM on April 5, 2024: member

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

fanquake merged this on Apr 5, 2024

fanquake closed this on Apr 5, 2024

theuni commented at 4:09 PM on April 5, 2024: member

Post-merge utACK fa9f36babaceba6ab2f88e64bc4bc2956f58871f

maflcko deleted the branch on Apr 8, 2024

Pttn referenced this in commit 4169b51b12 on Apr 13, 2024

hebasto referenced this in commit 6d4584b22f on Apr 18, 2024

hebasto referenced this in commit e79ce6f75c on Apr 18, 2024

PastaPastaPasta referenced this in commit e5ee3ee0b5 on Oct 4, 2024

PastaPastaPasta referenced this in commit 1fdf6dc456 on Oct 8, 2024

PastaPastaPasta referenced this in commit 7d8d7483d8 on Oct 30, 2024

bitcoin locked this on Apr 8, 2025