Bitcoin Core Github
43 subscribers
123K links
Download Telegram
💬 Maxzy000 commented on issue "Tracepoint Interface Tracking Issue":
(https://github.com/bitcoin/bitcoin/issues/31274#issuecomment-2497919023)
Oh can see can you text me in my telegram account
👍 l0rinc approved a pull request: "ci: Update Clang in "tidy" job"
(https://github.com/bitcoin/bitcoin/pull/31306#pullrequestreview-2458343388)
ACK 3e039b7a03bea6e11e994aebe038ebd6ca80aeb8

The latest change:
* Reverted the `CI_IMAGE_NAME_TAG` and added `APT_LLVM_V`.
* Reverted the `CheckAddCoin` and `ExpectSuccess` fixes to avoid collisions with other PRs.
💬 laanwj commented on issue "guix: Linux and macOS builds are not cross-arch reproducible with powerpc64le build arch":
(https://github.com/bitcoin/bitcoin/issues/31207#issuecomment-2497932788)
i've compared against the binaries from bitcoincore.org and looked at the ELF metadata and disassembly and it's a significant difference, not just a few different instructions. The entire `.text` section ends up with a different size, which shifts the offsets, making it very hard to determine what the source of the discrepancy is.

Debug symbols could help but ideally i'd need a linker map.

Can you please build the following branch: https://github.com/laanwj/bitcoin/tree/2024-11-linker-map-
...
💬 maflcko commented on pull request "ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#discussion_r1856563510)
Correct, but it isn't using the Ubuntu mirror. It is using the GHA mirror. Using a GHA cache to sidestep the GHA mirror seems a no-op, no?
💬 l0rinc commented on pull request "refactor: Fix remaining clang-tidy performance-unnecessary-copy-initialization errors":
(https://github.com/bitcoin/bitcoin/pull/31364#issuecomment-2497942820)
Rebased on top of https://github.com/bitcoin/bitcoin/pull/31306 and updated commit message
💬 maflcko commented on pull request "ci: Update Clang in "tidy" job":
(https://github.com/bitcoin/bitcoin/pull/31306#discussion_r1856568054)
Seems easier to just fix it right away, instead of touching the line to say to fix it and then fix it in the next commit?
💬 l0rinc commented on pull request "refactor: Fix remaining clang-tidy performance-inefficient-vector errors":
(https://github.com/bitcoin/bitcoin/pull/31305#issuecomment-2497947459)
Rebased on top of https://github.com/bitcoin/bitcoin/pull/31306 and updated commit messages
💬 maflcko commented on pull request "ci: Skip broken Wine64 tests by default":
(https://github.com/bitcoin/bitcoin/pull/31284#issuecomment-2497949320)
> The false positives keep coming in, so it would be good to make progress here:

https://cirrus-ci.com/task/5175499276681216
💬 l0rinc commented on pull request "ci: Update Clang in "tidy" job":
(https://github.com/bitcoin/bitcoin/pull/31306#discussion_r1856576778)
I'm fine either way, but I guess this way it was easier to selectively revert a conflicting fix and leave only the suppression.

But I'd like to understand the reason for the supressions: would the lint problem break CI without them? Since this will still invalidate ACKed PRs if we merge it like this.
maflcko closed an issue: "Segmentation fault when ./bitcoind"
(https://github.com/bitcoin/bitcoin/issues/31321)
💬 maflcko commented on issue "Segmentation fault when ./bitcoind":
(https://github.com/bitcoin/bitcoin/issues/31321#issuecomment-2497970472)
Closing for now, due to lack of details/follow-up
maflcko closed an issue: "Maximum dbcache too small"
(https://github.com/bitcoin/bitcoin/issues/31326)
💬 maflcko commented on issue "Maximum dbcache too small":
(https://github.com/bitcoin/bitcoin/issues/31326#issuecomment-2497978288)
> Seems like the issue is also here: #27599

Closing for now as duplicate. Leave a comment, if this is not a duplicate, or if you have any other questions.

If you have more relevant details to add to #27599, please do so.
💬 hebasto commented on pull request "ci: Update Clang in "tidy" job":
(https://github.com/bitcoin/bitcoin/pull/31306#discussion_r1856612322)
> Seems easier to just fix it right away, instead of touching the line to say to fix it and then fix it in the next commit?

The commits have been squashed.

> But I'd like to understand the reason for the supressions: would the lint problem break CI without them?

Yes, it would.
💬 l0rinc commented on pull request "ci: Update Clang in "tidy" job":
(https://github.com/bitcoin/bitcoin/pull/31306#issuecomment-2498022212)
ACK f90fddfabd646c3f1d21de7bb004707a072e753e
💬 maflcko commented on pull request "ci: Update Clang in "tidy" job":
(https://github.com/bitcoin/bitcoin/pull/31306#discussion_r1856621868)
it would be good to understand why this is needed. If this is a bug, it would be good to fix it.
💬 brunoerg commented on pull request "fuzz: set the output argument of FuzzedSock::Accept()":
(https://github.com/bitcoin/bitcoin/pull/31316#issuecomment-2498034392)
So, it would be better to cherry-pick this into #28584?
💬 hodlinator commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#issuecomment-2498039344)
Can understand the checks failing on MacOS or Windows, but surprised that what I assume to be Linux on Cirrus CI is also failing. Will move to Windows and see how it behaves there. Might have to revert to explicitly checking `is_directory()`.
💬 hebasto commented on pull request "ci: Update Clang in "tidy" job":
(https://github.com/bitcoin/bitcoin/pull/31306#discussion_r1856627172)
Right. I outlined this issue for follow-ups in the PR description.
💬 maflcko commented on pull request "ci: Update Clang in "tidy" job":
(https://github.com/bitcoin/bitcoin/pull/31306#discussion_r1856627249)
See b32845cb94a81e6fd8b01a8631e3d276c9fc9e35, so I think this is intentional.

I think it would be better to fix the bug, instead of silencing it.