Bitcoin Core Github
44 subscribers
120K links
Download Telegram
๐Ÿ’ฌ Ayush170-Future commented on issue "ci: Cleanup ci/test/01_base_install.sh":
(https://github.com/bitcoin/bitcoin/issues/27321#issuecomment-1484123445)
Hey @vstoyanov!
No problem at all. You seem to understand this far better than I do, so no worries. I will at the very least do a code review for your PR (as I mentioned in my comment). And will look for some other issues to contribute to.

P.S. Thanks for clearing my test/04_install.sh doubt. I got a little confused there.
๐Ÿ’ฌ vstoyanov commented on issue "ci: Cleanup ci/test/01_base_install.sh":
(https://github.com/bitcoin/bitcoin/issues/27321#issuecomment-1484135897)
@Ayush170-Future Thanks for understanding. Also thanks for the complement, but maybe this is not the case :) - I got to the point where `$CI_BASE_PACKAGES `and `$CI_PACKAGES` should pass lint. This means either convert them to bash arrays or use SuppressWarnings to switch lint of. The first seems much cleaner to do, the second is a smaller change and I am confused as to what to do with that.

In about 45 minutes I will push the array conversion in a second commit I could squash later if we dec
...
๐Ÿ’ฌ Ferrydh commented on issue "ci: Cleanup ci/test/01_base_install.sh":
(https://github.com/bitcoin/bitcoin/issues/27321#issuecomment-1484136940)
่ฟ™ๆ˜ฏๆฅ่‡ชQQ้‚ฎ็ฎฑ็š„ๅ‡ๆœŸ่‡ชๅŠจๅ›žๅค้‚ฎไปถใ€‚
 
ๆ‚จๅฅฝ๏ผŒๆˆ‘ๆœ€่ฟ‘ๆญฃๅœจไผ‘ๅ‡ไธญ๏ผŒๆ— ๆณ•ไบฒ่‡ชๅ›žๅคๆ‚จ็š„้‚ฎไปถใ€‚ๆˆ‘ๅฐ†ๅœจๅ‡ๆœŸ็ป“ๆŸๅŽ๏ผŒๅฐฝๅฟซ็ป™ๆ‚จๅ›žๅคใ€‚
๐Ÿ’ฌ hebasto commented on pull request "util: implement `noexcept` move assignment & move ctor for `prevector`":
(https://github.com/bitcoin/bitcoin/pull/27334#issuecomment-1484141370)
Concept ACK. Thanks for picking [this](https://github.com/bitcoin/bitcoin/pull/26642#discussion_r1108186877) up.
๐Ÿ’ฌ martinus commented on pull request "log: Check that the timestamp string is non-empty to avoid undefined behavior":
(https://github.com/bitcoin/bitcoin/pull/27317#discussion_r1148585918)
I've seen this happening to computers before, but never when bitcoind was running. Usually that happens when time synchronization goes wrong for whatever reason.
๐Ÿ’ฌ martinus commented on pull request "clang-tidy: Add more `performance-*` checks and related fixes":
(https://github.com/bitcoin/bitcoin/pull/26642#discussion_r1148594367)
nit: maybe add `+ 2` to the reserve because when the next `if` is true 2 more will be added. Otherwise the next `emplace_back` will most likely cause a capacity change
๐Ÿ’ฌ vstoyanov commented on issue "ci: Cleanup ci/test/01_base_install.sh":
(https://github.com/bitcoin/bitcoin/issues/27321#issuecomment-1484167539)
False alarm. The `bash -c` hack also worked for dnf the same way it did for apt, so now it is both not a big change and has no SuppressWarnings BS. I could still push the arrayification of $PACKAGES and $CI_BASE_PACKAGES though?
๐Ÿ’ฌ martinus commented on pull request "fix initialization in FastRandomContext: removes undefined behavior & uninitialized read":
(https://github.com/bitcoin/bitcoin/pull/23169#issuecomment-1484180043)
Now that #26153 was merged this is no relevant
โœ… martinus closed a pull request: "fix initialization in FastRandomContext: removes undefined behavior & uninitialized read"
(https://github.com/bitcoin/bitcoin/pull/23169)
๐Ÿ’ฌ hebasto commented on pull request "clang-tidy: Add more `performance-*` checks and related fixes":
(https://github.com/bitcoin/bitcoin/pull/26642#issuecomment-1484197246)
Updated 09383b612216b63f2aec0f1fffc889989c66f212 -> 03ec5b6f9ca3af28c9ce25cf2393e28ae852d808 ([pr26642.12](https://github.com/hebasto/bitcoin/commits/pr26642.12) -> [pr26642.13](https://github.com/hebasto/bitcoin/commits/pr26642.13), [diff](https://github.com/hebasto/bitcoin/compare/pr26642.12..pr26642.13)):

- addressed @martinus's [comment](https://github.com/bitcoin/bitcoin/pull/26642#discussion_r1148594367)
๐Ÿ’ฌ hebasto commented on pull request "clang-tidy: Add more `performance-*` checks and related fixes":
(https://github.com/bitcoin/bitcoin/pull/26642#discussion_r1148626538)
Thanks! [Updated](https://github.com/bitcoin/bitcoin/pull/26642#issuecomment-1484197246).
๐Ÿ’ฌ Ayush170-Future commented on issue "ci: Cleanup ci/test/01_base_install.sh":
(https://github.com/bitcoin/bitcoin/issues/27321#issuecomment-1484198989)
I guess SuppressWarnings was not a good practice. It's great that you found a workaround.

Instead of giving package strings as an array, I believe the bash -c technique should be sufficient here. @MarcoFalke, I think you wanted it that way as well.
๐Ÿ’ฌ theuni commented on pull request "depends: fix osx build with clang 16":
(https://github.com/bitcoin/bitcoin/pull/27328#issuecomment-1484213695)
> It fixes building `make -C depends HOST=x86_64-apple-darwin FORCE_USE_SYSTEM_CLANG=1` with clang-16.

Yes, exactly this thanks.

This can be verified very simply outside of depends:
Broken:
> $ /opt/clang+llvm-16.0.0-x86_64-linux-gnu-ubuntu-18.04/bin/clang -c test.c -o test.o -Xclang -internal-externc-isystem/tmp
> error: unknown argument: '-internal-externc-isystem/tmp'

Working:
> $ /opt/clang+llvm-16.0.0-x86_64-linux-gnu-ubuntu-18.04/bin/clang -c test.c -o test.o -Xclang -intern
...
๐Ÿ‘ TheCharlatan approved a pull request: "depends: fix osx build with clang 16"
(https://github.com/bitcoin/bitcoin/pull/27328)
ACK 87afcb0029b8dab933c122fb8f7263c2e7272731
๐Ÿ’ฌ Empact commented on pull request "Torcontrol opt check":
(https://github.com/bitcoin/bitcoin/pull/25136#issuecomment-1484225313)
@vstoyanov No need to ask for permission to contribute. I'd say go for it.
๐Ÿ“ EthanHeilman opened a pull request: "Fixes compile errors in MSVC build #27332"
(https://github.com/bitcoin/bitcoin/pull/27335)
This PR is designed to address the issue https://github.com/bitcoin/bitcoin/issues/27332. The MSVC build is failing because of two bugs in how the build is configured.

The issue
====

When running `msbuild build_msvc\bitcoin.sln -property:Configuration=Release -maxCpuCount -verbosity:minima`l the build fails with following two errors.

* `C:\Users\e0\Documents\GitHub\bitcoin\src\httpserver.cpp(637,9): error C2664: 'void evhttp_connection_get_peer(evhttp_connection *,const char **,uint16_
...
๐Ÿ’ฌ hernanmarino commented on pull request "Fixes compile errors in MSVC build #27332":
(https://github.com/bitcoin/bitcoin/pull/27335#discussion_r1148657332)
Is there a chance that this is not the case in any situation ?
๐Ÿ’ฌ hernanmarino commented on pull request "ci: cleanup of CI_EXEC & CI_EXEC_ROOT (refs #27321)":
(https://github.com/bitcoin/bitcoin/pull/27333#discussion_r1148659470)
Aren't there other scripts relying on the exported aliases ? Won't they fail ?
๐Ÿ‘ hernanmarino approved a pull request: "build, qt: Fix handling of `CXX=clang++` when building `qt` package"
(https://github.com/bitcoin/bitcoin/pull/27314)
tested ACK
๐Ÿ‘ hernanmarino approved a pull request: "log: Check that the timestamp string is non-empty to avoid undefined behavior"
(https://github.com/bitcoin/bitcoin/pull/27317)