💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1742931121)
I'd rather not include Wine installation instructions in this build doc. It is not a trivial process and could create an additional maintenance burden in the future.

> it wasn't present previously with autotools.

Let's just remove it.

⚠️ hebasto opened an issue: "guix: Build fails for `x86_64-apple-darwin`"
(https://github.com/bitcoin/bitcoin/issues/30810)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

Guix build for `x86_64-apple-darwin` fails.

### Expected behaviour

The build succeeds.

### Steps to reproduce

```
$ git checkout 94c307b3c0746a08c6135ff38d6f1ad1ed6693bb
$ env SDK_PATH=/home/hebasto HOSTS=x86_64-apple-darwin ./contrib/guix/guix-build
```

### Relevant log output

```
$ env SDK_PATH=/home/hebasto HOSTS=x86_64-apple-darwin ./contrib/guix/guix-build
Found macOS SDK a
...

💬 hebasto commented on pull request "build: Fix / improve coverage scripts":
(https://github.com/bitcoin/bitcoin/pull/30772#discussion_r1742980659)
You mean for the `lcov` command?

💬 romanz commented on pull request "http: set TCP_NODELAY when creating HTTP server":
(https://github.com/bitcoin/bitcoin/pull/30675#issuecomment-2327863260)
Many thanks!

💬 romanz commented on pull request "http: set TCP_NODELAY when creating HTTP server":
(https://github.com/bitcoin/bitcoin/pull/30675#issuecomment-2327863312)
Many thanks!

💬 davidgumberg commented on pull request "test: remove unused src_dir param from run_tests after CMake migration":
(https://github.com/bitcoin/bitcoin/pull/30733#issuecomment-2327921561)
ACK https://github.com/bitcoin/bitcoin/commit/76a28306f1e2c0828590eded0bf6b178f6b889da

💬 maflcko commented on pull request "build: Fix / improve coverage scripts":
(https://github.com/bitcoin/bitcoin/pull/30772#discussion_r1743085660)
I mean in the test_runner command

💬 maflcko commented on issue "guix: Build fails for `x86_64-apple-darwin`":
(https://github.com/bitcoin/bitcoin/issues/30810#issuecomment-2327944439)
Has this ever worked on your system? `guix shell: error` sounds like it is an error unrelated to Bitcoin Core?

Can you use guix and guix shell normally?

💬 maflcko commented on pull request "build: Minor build system fixes and amendments":
(https://github.com/bitcoin/bitcoin/pull/30803#issuecomment-2327952563)
review ACK dee36a6746f18a3db748ef7bf52bfebddb546307

CI output:

* Before: https://cirrus-ci.com/task/6415951824945152?logs=ci#L1057

```
C++ compiler flags .................... -Wno-error=documentation -O0 -ftrapv -O1 -g3 -g3 -std=c++20 -fPIC -fdebug-prefix-map=/ci_container_base=. -fmacro-prefix-map=/ci_container_base=. -Werror -Wall -Wextra -Wgnu -Wformat -Wformat-security -Wvla -Wshadow-field -Wthread-safety -Wloop-analysis -Wredundant-decls -Wunused-member-function -Wdate-time -Wcond
...

💬 hebasto commented on pull request "build: Fix / improve coverage scripts":
(https://github.com/bitcoin/bitcoin/pull/30772#discussion_r1743093952)
Thanks! Added.

💬 jadijadi commented on pull request "test: fixing failing system_tests/run_command under some Locales":
(https://github.com/bitcoin/bitcoin/pull/30788#issuecomment-2327990907)
> review ACK [8289373](https://github.com/bitcoin/bitcoin/commit/8289373ebf089d0719a8edecb8bfbd52c244f4ee)
>
> Please adjust the pull request description, now that the approach has changed: [#30788 (comment)](https://github.com/bitcoin/bitcoin/pull/30788#issue-2500949706)

Done. The PR description (https://github.com/bitcoin/bitcoin/pull/30788#issue-2500949706) is updated to reflect the changes in the commit message.

💬 maflcko commented on pull request "test: fixing failing system_tests/run_command under some Locales":
(https://github.com/bitcoin/bitcoin/pull/30788#issuecomment-2327996238)
Please update the pull request description properly. There is no need to include incorrect stuff (`echo success` isn't translated), or outdated stuff of a previous commit. Please focus on the important and correct details only.

👍 hebasto approved a pull request: "test: fixing failing system_tests/run_command under some Locales"
(https://github.com/bitcoin/bitcoin/pull/30788#pullrequestreview-2279109392)
ACK ae48a22a3df086fb59843b7b814619ed5df7557b, tested on Ubuntu 24.04 by switching locale.

https://github.com/bitcoin/bitcoin/pull/30788#issue-2500949706

> the run_command test under system_tests fails if the locale is anything other than US/UK/C (English ones) because it checks for things like `result.find_value("success");`
>
> To prevent this, a `setenv("LC_ALL", "C", 1);` is added to make sure that the tests are being run under C locale even if something else is set in the terminal.
...

💬 maflcko commented on pull request "test: fixing failing system_tests/run_command under some Locales":
(https://github.com/bitcoin/bitcoin/pull/30788#issuecomment-2328028855)
The commit description looks fine, so why not copy-paste it and replace the pull request description?

💬 maflcko commented on pull request "test: fixing failing system_tests/run_command under some Locales":
(https://github.com/bitcoin/bitcoin/pull/30788#issuecomment-2328029615)
review ACK ae48a22a3df086fb59843b7b814619ed5df7557b

💬 maflcko commented on pull request "build: Minor build system fixes and amendments":
(https://github.com/bitcoin/bitcoin/pull/30803#issuecomment-2328044971)
Could include https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1742931121 to avoid having to open another pull for that?

📝 hebasto opened a pull request: "build: Unify `-logsourcelocations` format"
(https://github.com/bitcoin/bitcoin/pull/30811)
Closes https://github.com/bitcoin/bitcoin/issues/30799:

```
$ ./build/src/bitcoind -logsourcelocations -asmap=/tmp/no_file 2>&1 | head -1
2024-09-04T07:01:03Z [init/common.cpp:149] [LogPackageVersion] Bitcoin Core version v28.99.0-b94fe85fbe2f (release build)
```

💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1743189780)
Fixed in https://github.com/bitcoin/bitcoin/pull/30803.

💬 hebasto commented on pull request "build: Minor build system fixes and amendments":
(https://github.com/bitcoin/bitcoin/pull/30803#issuecomment-2328087131)
> Could include [#30454 (comment)](https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1742931121) to avoid having to open another pull for that?

Sure! Included.

💬 maflcko commented on pull request "build: Unify `-logsourcelocations` format":
(https://github.com/bitcoin/bitcoin/pull/30811#discussion_r1743191660)
This isn't the only reason. The other is ccache, see commit 7abac98d3e3c1bc8ad66cb5c05184b9c5cc674d5. It would be good to mention this.

Ref: https://gcc.gnu.org/onlinedocs/gcc/Overall-Options.html#index-ffile-prefix-map

💬 jadijadi commented on pull request "test: fixing failing system_tests/run_command under some Locales":
(https://github.com/bitcoin/bitcoin/pull/30788#issuecomment-2328091025)
> The commit description looks fine, so why not copy-paste it and replace the pull request description?

just did. thanks.