Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 l0rinc commented on pull request "ci: Print inner env, Make ccache config more flexible":
(https://github.com/bitcoin/bitcoin/pull/30869#discussion_r1756384586)
the dot is dropped since it's not a [home dir](https://ccache.dev/manual/latest.html#config_cache_dir), right?
💬 l0rinc commented on pull request "ci: Print inner env, Make ccache config more flexible":
(https://github.com/bitcoin/bitcoin/pull/30869#discussion_r1756381105)
This is sourced usually through `test_run_all.sh`, right?
What was the reason for such low values before?
💬 l0rinc commented on pull request "ci: Print inner env, Make ccache config more flexible":
(https://github.com/bitcoin/bitcoin/pull/30869#discussion_r1756386761)
nit:
> the build path**s** are constant
💬 l0rinc commented on pull request "ci: Print inner env, Make ccache config more flexible":
(https://github.com/bitcoin/bitcoin/pull/30869#discussion_r1755666556)
nit: are you using the two separate variable reference styles because the second one doesn't have any characters after it? To be fair, the first also ends with a `,` so I think we can simplify this:
```suggestion
CI_CCACHE_MOUNT="type=bind,src=$CCACHE_DIR,dst=$CCACHE_DIR"
```
📝 fanquake locked a pull request: "26.2 final changes"
(https://github.com/bitcoin/bitcoin/pull/30376)
bins were uploaded 2 weeks ago on June 18
website PR: https://github.com/bitcoin-core/bitcoincore.org/pull/1039
🚀 fanquake merged a pull request: "build: Minimize I/O operations in `GenerateHeaderFrom{Json,Raw}.cmake`"
(https://github.com/bitcoin/bitcoin/pull/30842)
💬 maflcko commented on pull request "util: Use consteval checked format string in FatalErrorf, LogConnectFailure":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1756411531)
> > The verbosity in tests is a bit annoying, but seems acceptable to me.
>
> I agree.

I pushed a test-only change in the latest push to reduce the verbosity to a minimum.
👍 fanquake approved a pull request: "ci: Post CMake-migration fixes and amendments"
(https://github.com/bitcoin/bitcoin/pull/30841#pullrequestreview-2299649592)
ACK c45186ca548362b75a6640393ccf79b11ff727da
🚀 fanquake merged a pull request: "ci: Post CMake-migration fixes and amendments"
(https://github.com/bitcoin/bitcoin/pull/30841)
💬 0xB10C commented on issue "Continuous benchmark tracking":
(https://github.com/bitcoin/bitcoin/issues/27284#issuecomment-2345677280)
I recently came across @aureleoules's https://corecheck.dev/benchmarks which tracks execution time, CPU instructions, and CPU cycles of the Bitcoin Core benchmarks.
💬 maflcko commented on issue "GUI event loop should be block free":
(https://github.com/bitcoin/bitcoin/issues/17145#issuecomment-2345680377)
What is the status of the QML GUI? IIRC it was put on hold, because the Android APK in depends couldn't be updated for some reason, but I may be misremembering.
💬 hodlinator commented on pull request "util: Use consteval checked format string in FatalErrorf, LogConnectFailure":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1756443830)
<details>
<summary>

#### Diff on top of fae8c25d07cf003df1470699df4ef475055bb885

</summary>

```diff
diff --git a/src/test/util_string_tests.cpp b/src/test/util_string_tests.cpp
index 4c61632fce..ed383e9dad 100644
--- a/src/test/util_string_tests.cpp
+++ b/src/test/util_string_tests.cpp
@@ -17,7 +17,53 @@ template <unsigned NumArgs>
inline void PassFmt(util::ConstevalFormatString<NumArgs> fmt)
{
decltype(fmt)::Detail_CheckNumFormatSpecifiers(fmt.fmt); // This was run at c
...
👍 fanquake approved a pull request: "build: Use CMake's default permissions in macOS `deploy` target"
(https://github.com/bitcoin/bitcoin/pull/30838#pullrequestreview-2299668588)
ACK 5ba03e7d35e1b47ba864c9ae3c94af97cd3ae10b - I'm going to merge this now so we return to usable (comparable) guix builds.
💬 maflcko commented on pull request "util: Use consteval checked format string in FatalErrorf, LogConnectFailure":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1756448598)
> Still dropping a few negative tests, since they are proven by positive tests, even if less straightforward without the free function.

I don't think this is true. Without the free function, the tests are required to catch bugs, such as removing a check. E.g. see this diff that removes a check and the tests that you want to remove:

```diff
diff --git a/src/test/util_string_tests.cpp b/src/test/util_string_tests.cpp
index 4c61632fce..42facd4dc1 100644
--- a/src/test/util_string_tests.cpp
...
💬 hodlinator commented on pull request "util: Use consteval checked format string in FatalErrorf, LogConnectFailure":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1756457285)
> > Still dropping a few negative tests, since they are proven by positive tests, even if less straightforward without the free function.
>
> I don't think this is true. Without the free function

True, forgot that that last condition was back in the non-free function. Negative tests are not removed in [latest diff](https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1756443830).
💬 maflcko commented on issue "Continuous benchmark tracking":
(https://github.com/bitcoin/bitcoin/issues/27284#issuecomment-2345700640)
Yeah, corecheck is nice, but I think it went down after `cmake`. See also https://github.com/corecheck/corecheck/issues/10
💬 maflcko commented on issue "Continuous benchmark tracking":
(https://github.com/bitcoin/bitcoin/issues/27284#issuecomment-2345720633)
Another thing to add (back) to the benchmarks would be compile memory usage, which is easy to achieve with `$(which time) -f "%M;%S;%U" ...`, c.f. https://github.com/chaincodelabs/bitcoinperf/blob/0a1d6f54263cca78e645544ab8247d106d5ff59c/runner/sh.py#L110
fanquake closed an issue: "build: reproducibility issue with macOS Guix builds"
(https://github.com/bitcoin/bitcoin/issues/30815)
🚀 fanquake merged a pull request: "build: Use CMake's default permissions in macOS `deploy` target"
(https://github.com/bitcoin/bitcoin/pull/30838)
💬 hodlinator commented on pull request "wallet: Remove IsMine from migration code":
(https://github.com/bitcoin/bitcoin/pull/30328#issuecomment-2345733518)
https://github.com/bitcoin/bitcoin/pull/30328#pullrequestreview-2201704324 still applies to 705d9eb0aef8831891e1cce80c33615440547e90 instead of d5d994c02bb54db395da457724ec45539f1c10a8.

(Clarified linked comment a bit).