💬 fanquake commented on pull request "doc: Add note for building on macOS (Intel) with CMake ≥ 4.0":
(https://github.com/bitcoin/bitcoin/pull/32289#issuecomment-2812293989)
Not sure. I see this is just copied from the CMake release notes, but it doesn't really explain anything, and it's not clear what warnings that were being supressed no-longer are. Also seems like the warnings are a sideeffect of some other issues, that should probably be fixed *properly*, in some way.

💬 fanquake commented on pull request "build: Restore cross-compilation for Android":
(https://github.com/bitcoin/bitcoin/pull/32262#issuecomment-2812297608)
> as CI integration still needs to be added.

Not sure. As long as we aren't shipping this in releases, I don't think it needs to be added to the CI (the same could also be said for some other jobs), and shouldn't be a blocker for making changes (iirc the previous android job used to sporadically fail quite often, failing to download JDKs and related things). Especially when there are other more important things, which we do support, which we would be better off spending the CPU on (guix build
...

💬 fanquake commented on pull request "guix: Remove no longer necessary `file` package":
(https://github.com/bitcoin/bitcoin/pull/32242#issuecomment-2812301514)
> My only guess is that it was used by autotools detection code indirectly, somehow.

Yea, `file` was checked-for by Autotools, but maybe non of the related checks affected our build.

If the conclusion is that this was never used, can the PR title, commit message, and PR description be fixed to reflect that.

💬 hebasto commented on pull request "ci: drop -priority-level from bench in win cross CI":
(https://github.com/bitcoin/bitcoin/pull/32288#discussion_r2048583154)
Why not drop `-priority-level=high` here as well:https://github.com/bitcoin/bitcoin/blob/7a3afe6787bc36fd98684eac020f9abdbfae6f0a/src/bench/CMakeLists.txt#L82-L84 ?

Will it have a significant impact on execution time?

💬 fanquake commented on pull request "ci: drop -priority-level from bench in win cross CI":
(https://github.com/bitcoin/bitcoin/pull/32288#discussion_r2048585578)
I'm just changing CI, so that we catch regressions. Someone could follow up with changing this for all developers & users.

💬 maflcko commented on pull request "build: Restore cross-compilation for Android":
(https://github.com/bitcoin/bitcoin/pull/32262#issuecomment-2812310449)
I'd say a CI task config should exist. Otherwise, there is no way to test the build easily for non-android people. No opinion on adding it to the live CI matrix here in this repo.

💬 vasild commented on pull request "rpc: add cpu_load to getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/31672#issuecomment-2812322401)
`bb822a5ee1...ee16345af3`: address suggestions

💬 vasild commented on pull request "rpc: add cpu_load to getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/31672#discussion_r2048596931)
Done.

💬 vasild commented on pull request "rpc: add cpu_load to getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/31672#discussion_r2048597720)
Done. This removes all build system changes from this PR, thanks!

💬 vasild commented on pull request "rpc: add cpu_load to getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/31672#discussion_r2048598114)
Removed.

💬 hebasto commented on pull request "guix: Remove unused `file` package":
(https://github.com/bitcoin/bitcoin/pull/32242#issuecomment-2812325561)
> > My only guess is that it was used by autotools detection code indirectly, somehow.
>
> Yea, `file` was checked-for by Autotools, but maybe non of the related checks affected our build.
>
> If the conclusion is that this was never used, can the PR title, commit message, and PR description be fixed to reflect that.

Thanks! Done.

💬 fanquake commented on pull request "rpc: add cpu_load to getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/31672#discussion_r2048601918)
Same here, as the `clock_gettime` alternative comment.

💬 vasild commented on pull request "rpc: add cpu_load to getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/31672#discussion_r2048602251)
The linked article is long and convoluted and here I want to highlight only a short part of it that is relevant. Also having the text here is helpful if the web page disappears in the future.

💬 l0rinc commented on pull request "ci: drop -priority-level from bench in win cross CI":
(https://github.com/bitcoin/bitcoin/pull/32288#issuecomment-2812335126)
utACK 27f11217ca63e0f8f78f14db139150052dcd9962

💬 hebasto commented on pull request "build: Restore cross-compilation for Android":
(https://github.com/bitcoin/bitcoin/pull/32262#issuecomment-2812338837)
> I'd say a CI task config should exist. Otherwise, there is no way to test the build easily for non-android people. No opinion on adding it to the live CI matrix here in this repo.

I meant this ^ without adding to the live CI matrix in this repo.

💬 fanquake commented on pull request "rpc: add cpu_load to getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/31672#discussion_r2048607821)
Then I'd suggest atleast trying to consolidate what is here, given you already link to the same place above, with just "GetThreadTimes():" as a comment (that seems unecessary).

💬 hebasto commented on pull request "ci: drop -priority-level from bench in win cross CI":
(https://github.com/bitcoin/bitcoin/pull/32288#discussion_r2048612501)
But this introduces another inconsistency between Windows and other platforms. Initially, `run: ./bin/bench_bitcoin.exe -sanity-check -priority-level=high` was written to mimic the `bench_sanity_check_high_priority` test task behaviour.

💬 fanquake commented on pull request "ci: drop -priority-level from bench in win cross CI":
(https://github.com/bitcoin/bitcoin/pull/32288#discussion_r2048614265)
Can you explain your issue with running more tests in this CI?

💬 hebasto commented on pull request "doc: Add note for building on macOS (Intel) with CMake ≥ 4.0":
(https://github.com/bitcoin/bitcoin/pull/32289#issuecomment-2812355350)
> ... that should probably be fixed _properly_, in some way.

I haven't found a way to do it that covers all use cases, is user-friendly, and doesn't introduce excessive complexity.

💬 hebasto commented on pull request "doc: Add note for building on macOS (Intel) with CMake ≥ 4.0":
(https://github.com/bitcoin/bitcoin/pull/32289#discussion_r2048621583)
> This says it applies to users with Homebrew installed, when _not_ building with homebrew-provided tools? What are the "tools" here?

For example, LLVM toolchain.