Bitcoin Core Github
43 subscribers
123K links
Download Telegram
💬 l0rinc commented on pull request "net: Disconnect message follow-ups to #28521":
(https://github.com/bitcoin/bitcoin/pull/31633#discussion_r1910245234)
unrelated: do we really need this level of detail for a timeout (i.e. ms precision expressed in seconds)?
💬 hebasto commented on issue "depends: llvm-ranlib (etc): "No such file or directory" on Intel macOS 15.0":
(https://github.com/bitcoin/bitcoin/issues/30978#issuecomment-2582535849)
From my understanding, the issue arises because the `make` provided by Xcode does not set the `.SHELLFLAGS` variable properly. Here's an example of the output:
```
% whereis make
make: /usr/bin/make /Applications/Xcode.app/Contents/Developer/usr/share/man/man1/make.1
% make --version
GNU Make 3.81
Copyright (C) 2006 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.
There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A
PARTICULAR
...
💬 hebasto commented on pull request "depends: fix for llvm-ranlib (etc): 'No such file or directory' macOS 15.0":
(https://github.com/bitcoin/bitcoin/pull/30994#issuecomment-2582537242)
> > Not sure about this issue, or making this change. Looks like it needs more investigation. See my comment here: [#30978 (comment)](https://github.com/bitcoin/bitcoin/issues/30978#issuecomment-2388086723).
>
> Yes, switched to draft until we have a better understanding of the issue.

See https://github.com/bitcoin/bitcoin/issues/30978#issuecomment-2582535849.
💬 fanquake commented on pull request "build: Improve `ccache` performance for different build directories":
(https://github.com/bitcoin/bitcoin/pull/30861#issuecomment-2582556693)
What is the status of this?
💬 maflcko commented on pull request "test: importdescriptors is not available for non-descriptor wallets":
(https://github.com/bitcoin/bitcoin/pull/31609#issuecomment-2582574580)
Huh? Now you are adding a test for a previous release, which does not increase the test coverage, see also https://corecheck.dev/bitcoin/bitcoin/pulls/31609 .


If you want to test a previous release, it would be better to submit the test against the previous release branch. But I am not sure if this is worth it.

Also, I don't understand why the check should be kept after the bdb removal. Why would one want to guard against bdb at runtime when it is impossible to load bdb?
brunoerg closed a pull request: "test: importdescriptors is not available for non-descriptor wallets"
(https://github.com/bitcoin/bitcoin/pull/31609)
💬 brunoerg commented on pull request "test: importdescriptors is not available for non-descriptor wallets":
(https://github.com/bitcoin/bitcoin/pull/31609#issuecomment-2582613729)
> Huh? Now you are adding a test for a previous release, which does not increase the test coverage, see also https://corecheck.dev/bitcoin/bitcoin/pulls/31609 .

Nevermind, my bad, I did it quickly and did not notice it. I'm gonna close it because I believe this check could be removed within bdb removal.
💬 fanquake commented on pull request "depends: add *FLAGS to gen_id":
(https://github.com/bitcoin/bitcoin/pull/31125#discussion_r1910323797)
Dropped this out as well.
📝 NicolaLS opened a pull request: "doc: merge required dependency tables"
(https://github.com/bitcoin/bitcoin/pull/31634)
I was a bit confused why there are two tables in [dependencies.md](https://github.com/bitcoin/bitcoin/blob/master/doc/dependencies.md) please let me know if there a reason for this or if this change makes sense.

_rationale/motivation:_
Initially there was a distinction between the compiler dependencies and other required dependencies (refs #23565) but the distinction was removed (refs #24585) which is why having two distinct tables could lead to confusion now. This commit merges both tables
...
💬 maflcko commented on pull request "doc: merge required dependency tables":
(https://github.com/bitcoin/bitcoin/pull/31634#discussion_r1910358592)
this is optional
💬 maflcko commented on pull request "doc: merge required dependency tables":
(https://github.com/bitcoin/bitcoin/pull/31634#issuecomment-2582680383)
lgtm ACK 08c74105fd65b6356fcdb9d01972fd19f160b588, otherwise
💬 hebasto commented on pull request "cmake: Set top-level target output locations":
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2582681441)
A silent conflict has been fixed.
💬 hodlinator commented on pull request "doc: merge required dependency tables":
(https://github.com/bitcoin/bitcoin/pull/31634#discussion_r1910360393)
(systemtap) :+1:
💬 hodlinator commented on pull request "doc: merge required dependency tables":
(https://github.com/bitcoin/bitcoin/pull/31634#issuecomment-2582686766)
Would be good to move discussion/question-type commentary out of the PR summary into its own comment as it becomes part of the commit message, regarding the "I was a bit confused ... please let me know ...". Alternatively re-word it.
💬 NicolaLS commented on pull request "doc: merge required dependency tables":
(https://github.com/bitcoin/bitcoin/pull/31634#discussion_r1910364764)
should I add another sub-header (`### Tracing`) in the `## Optional` section and put it there ?
💬 fanquake commented on pull request "doc: merge required dependency tables":
(https://github.com/bitcoin/bitcoin/pull/31634#issuecomment-2582707462)
Not sure. Re-arranging this to put two different compilers under "required" doesn't seem less confusing. If anything, I think the tables should be left as-is, and a "developer tools" header added, if there is any confusion.
💬 maflcko commented on pull request "doc: merge required dependency tables":
(https://github.com/bitcoin/bitcoin/pull/31634#discussion_r1910376416)
Yeah, either a new section, or merge the tables below as well into one:

| Dependency | Releases | Version used | Minimum required | Runtime |
| --- | --- | --- | --- | --- |
| [Fontconfig](../depends/packages/fontconfig.mk) (GUI) | [link](https://www.freedesktop.org/wiki/Software/fontconfig/) | [2.12.6](https://github.com/bitcoin/bitcoin/pull/23495) | 2.6 | Yes |
| [FreeType](../depends/packages/freetype.mk) (GUI) | [link](https://freetype.org) | [2.11.0](https://github.com/bitcoin/bitcoin
...
🚀 fanquake merged a pull request: "ci: build msan's libc++ with _LIBCPP_ABI_BOUNDED_*"
(https://github.com/bitcoin/bitcoin/pull/31612)
🚀 fanquake merged a pull request: "doc: Clarify min macOS and Xcode version"
(https://github.com/bitcoin/bitcoin/pull/31608)
💬 NicolaLS commented on pull request "doc: merge required dependency tables":
(https://github.com/bitcoin/bitcoin/pull/31634#issuecomment-2582763764)
> Not sure. Re-arranging this to put two different compilers under "required" doesn't seem less confusing. If anything, I think the tables should be left as-is, and a "developer tools" header added, if there is any confusion.

With a "development tools" header it would still be unclear which development tools are required (`cmake`, `python`), optional (`systemtap`) or mutual substitutes (`gcc` _or_ `clang`) and since required ones would mixed with the two different compilers in the same table
...