Bitcoin Core Github
43 subscribers
123K links
Download Telegram
💬 hebasto commented on issue "Cmake build system breaks with symbolic links":
(https://github.com/bitcoin/bitcoin/issues/31145#issuecomment-2506192717)
@willcl-ark
> So I agree, symlinked build dirs seem generally broken currently with cmake.

Does https://github.com/bitcoin/bitcoin/pull/31361 fix it?
👍 willcl-ark approved a pull request: "cmake, qt: Use absolute paths for includes in MOC-generated files"
(https://github.com/bitcoin/bitcoin/pull/31361#pullrequestreview-2468275553)
tACK 6f4128e3a838d03f46d397c15bc5333287e14863

Tested building using a symlinked build dir using Ninja. Failures I experienced in #31145 build successfully with this patch.
💬 willcl-ark commented on issue "Cmake build system breaks with symbolic links":
(https://github.com/bitcoin/bitcoin/issues/31145#issuecomment-2506211739)
It does indeed.
💬 vasild commented on pull request "Fuzz: extend CConnman tests":
(https://github.com/bitcoin/bitcoin/pull/28584#issuecomment-2506252596)
`c97d49628a...687a9af2a8`: include #31316 as first commit here. It fixes a problem in `master` with `FuzzedSock::Accept()` which might be triggered by the tests added in this PR.
💬 vasild commented on pull request "fuzz: set the output argument of FuzzedSock::Accept()":
(https://github.com/bitcoin/bitcoin/pull/31316#issuecomment-2506260829)
Included this in #28584.

Leaving this open for an independent review/merge because it is smaller change and fixes a problem in `master`.
willcl-ark closed an issue: "estimateSmartFee error: "Insufficient data or no feerate found""
(https://github.com/bitcoin/bitcoin/issues/31116)
💬 willcl-ark commented on issue "estimateSmartFee error: "Insufficient data or no feerate found"":
(https://github.com/bitcoin/bitcoin/issues/31116#issuecomment-2506318310)
In the absence of any followup from @ella-quicknode in a few weeks, and no reproduction available (as well as no other reports) I'm going to close this as not reproducible.

If you have an excerpt from a debug.log with the `estimatesmartfee` logging turned on that you could provide, or think this should be re-opened for any other reason, please drop a comment below and we can re-open this.

If this has been continuing it would be good also to know if you see the same behaviour with 28.0.
⚠️ Sjors reopened an issue: "macOS 13.7 depends build can't find qt"
(https://github.com/bitcoin/bitcoin/issues/31050)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

```
cd depends
make NO_BDB=1
...
copying packages: boost libevent qt qrencode sqlite miniupnpc zeromq
to: /Volumes/SSD/Dev/bitcoin/depends/x86_64-apple-darwin22.6.0

cd ..
cmake -B build --toolchain depends/x86_64-apple-darwin22.6.0/toolchain.cmake
```

This fails, see below

(building BDB hasn't work for me for a while, but that's going away anyway)

### Expected behaviour

T
...
💬 Sjors commented on issue "macOS 13.7 depends build can't find qt":
(https://github.com/bitcoin/bitcoin/issues/31050#issuecomment-2506334687)
I tried https://github.com/bitcoin/bitcoin/pull/31361 and it makes no difference.

```
% cmake -B build --toolchain /Volumes/SSD/Dev/bitcoin/depends/x86_64-apple-darwin22.6.0/toolchain.cmake
-- The CXX compiler identification is AppleClang 15.0.0.15000100
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /Applications/Xcode15.2.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang++ - skipped
-- Detecting CX
...
👍 willcl-ark approved a pull request: "ci: Skip broken Wine64 tests by default"
(https://github.com/bitcoin/bitcoin/pull/31284#pullrequestreview-2468417480)
ACK fa5e7064597fc51366f082e3d07a4591e576db38

I agree with disabling these tests, and then pursuing a followup e.g. #31176 to run them natively on windows. There's no point in debugging false positives (and no? real positives).
💬 Sjors commented on issue "macOS 13.7 depends build can't find qt":
(https://github.com/bitcoin/bitcoin/issues/31050#issuecomment-2506339307)
It does indeed seem related to symlinks though, maybe something slightly different than what #31361 tries to fix. If I go from the symlinked to `~/dev` to its target (`cd /Volumes/SSD/Dev/bitcoin`), and then do the above `cmake -B ...` it works fine.
💬 Sjors commented on pull request "cmake, qt: Use absolute paths for includes in MOC-generated files":
(https://github.com/bitcoin/bitcoin/pull/31361#issuecomment-2506345496)
See #31050, this doesn't fix the scenario where the whole source dir is a symlink for me on macOS 13.7. But it's good that it does fix other things.
💬 maflcko commented on pull request "util: Drop boost posix_time in ParseISO8601DateTime":
(https://github.com/bitcoin/bitcoin/pull/31391#issuecomment-2506357477)
Looks like MSVC is broken, according to the GHA run:

```
D:/a/bitcoin/bitcoin/src/test/util_tests.cpp(334): error: in "util_tests/util_ParseISO8601DateTime": check ParseISO8601DateTime("9999-12-31T23:59:59Z").value() == 253402300799 has failed [-4295736961 != 253402300799]
```

It would be good if a someone using Windows reproduced this locally and then reported the bug to MSVC. Godbolt draft: https://godbolt.org/z/4WYn6E61W
💬 0xB10C commented on something "":
(https://github.com/bitcoin/bitcoin/commit/e6ff612fe6a41978d2c9dba7af0da36db24b9d85#r149713050)
The name should probably include `ci` somewhere
💬 hebasto commented on pull request "guix: use GCC 13 to build releases":
(https://github.com/bitcoin/bitcoin/pull/29881#issuecomment-2506436251)
My Guix build:
```
aarch64
827361e6e7a412ee6178440519b0716551c22fca9799a0a6cf17155790bfeef8 guix-build-09c2323e751f/output/aarch64-linux-gnu/SHA256SUMS.part
8cd3752d49cd087d8bc39947788b9bc2af9830063786b974debe651bc0fde99f guix-build-09c2323e751f/output/aarch64-linux-gnu/bitcoin-09c2323e751f-aarch64-linux-gnu-debug.tar.gz
56b76fcd39c8cb424ae04a5405ac95d836955334e53f7ff57c7bfe2219b05091 guix-build-09c2323e751f/output/aarch64-linux-gnu/bitcoin-09c2323e751f-aarch64-linux-gnu.tar.gz
62edbcf5
...
💬 hodlinator commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1862455177)
Reverted in later pushes.
👍 TheCharlatan approved a pull request: "Remove `src/config` directory"
(https://github.com/bitcoin/bitcoin/pull/31390#pullrequestreview-2468548777)
ACK 935973b315fa3b99f5e79b360bcb66f473bb7b95
💬 hodlinator commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1862479984)
To me this is a good pattern:
1. Document former *correct behavior* by adding tests.
2. Change implementation and tests to conform to *new behavior*.

However, if 1) is about documenting *buggy behavior*, committing a test for that in a way that makes them succeed feels off to me. In that case I would prefer adding tests that *prove failure* with some special annotation in the commit message for CI to expect failure like what @l0rinc [linked here (Spock)](https://github.com/bitcoin/bitcoin/p
...
📝 vasild converted_to_draft a pull request: "ci: detect outbound internet traffic generated while running tests"
(https://github.com/bitcoin/bitcoin/pull/31349)
Make sure that CI fails if some of the tests generate an outbound traffic on the non-loopback interface.

Resolves https://github.com/bitcoin/bitcoin/issues/31339
💬 vasild commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-2506560308)
Converted to draft for a while, testing docker with full privileges (need cirrus which does not run in my personal fork).