💬 TheCharlatan commented on pull request "kernel: Add interrupt class":
(https://github.com/bitcoin/bitcoin/pull/27861#issuecomment-1589076014)
Rebased d60ab3b5099d3a348f3122b5d3207ac12f4a751c -> 3e60c02ef3759e16a762bd193e0b9291bf46b73c ([kernelInterrupt_0](https://github.com/TheCharlatan/bitcoin/commits/kernelInterrupt_0) -> [kernelInterrupt_1](https://github.com/TheCharlatan/bitcoin/commits/kernelInterrupt_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelInterrupt_0..kernelInterrupt_1)).
* Added `src/node/abort.*` to allow both the notification code and `index/base.cpp` to use a common abort definition as introduce
...
(https://github.com/bitcoin/bitcoin/pull/27861#issuecomment-1589076014)
Rebased d60ab3b5099d3a348f3122b5d3207ac12f4a751c -> 3e60c02ef3759e16a762bd193e0b9291bf46b73c ([kernelInterrupt_0](https://github.com/TheCharlatan/bitcoin/commits/kernelInterrupt_0) -> [kernelInterrupt_1](https://github.com/TheCharlatan/bitcoin/commits/kernelInterrupt_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelInterrupt_0..kernelInterrupt_1)).
* Added `src/node/abort.*` to allow both the notification code and `index/base.cpp` to use a common abort definition as introduce
...
💬 fanquake commented on pull request "wallet: Give deprecation warning when loading a legacy wallet":
(https://github.com/bitcoin/bitcoin/pull/27869#issuecomment-1589076049)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/27869#issuecomment-1589076049)
Concept ACK
📝 fanquake opened a pull request: "lint: suppress pip spam"
(https://github.com/bitcoin/bitcoin/pull/27871)
The logs are [currently full of](https://cirrus-ci.com/task/5380096597426176?logs=lint#L240):
```bash
WARNING: Running pip as the 'root' user can result in broken permissions and conflicting behaviour with the system package manager. It is recommended to use a virtual environment instead: https://pip.pypa.io/warnings/venv
WARNING: You are using pip version 22.0.4; however, version 23.1.2 is available.
You should consider upgrading via the '/tmp/python/bin/python3.8 -m pip install --upgrade p
...
(https://github.com/bitcoin/bitcoin/pull/27871)
The logs are [currently full of](https://cirrus-ci.com/task/5380096597426176?logs=lint#L240):
```bash
WARNING: Running pip as the 'root' user can result in broken permissions and conflicting behaviour with the system package manager. It is recommended to use a virtual environment instead: https://pip.pypa.io/warnings/venv
WARNING: You are using pip version 22.0.4; however, version 23.1.2 is available.
You should consider upgrading via the '/tmp/python/bin/python3.8 -m pip install --upgrade p
...
📝 fanquake opened a pull request: "build: suppress external warnings by default"
(https://github.com/bitcoin/bitcoin/pull/27872)
I think we are at the point where it make more sense to make this the default, than not. It's already used in the CI, and I assume most building locally are also utilising it.
(https://github.com/bitcoin/bitcoin/pull/27872)
I think we are at the point where it make more sense to make this the default, than not. It's already used in the CI, and I assume most building locally are also utilising it.
💬 fanquake commented on pull request "doc: clarify full Xcode download is not needed":
(https://github.com/bitcoin/bitcoin/pull/27835#issuecomment-1589083587)
> Some background, it should indeed not be necessary to install Xcode:
It hasn't been required for at least 7 years, that's why we removed the instructions to install in: https://github.com/bitcoin/bitcoin/commit/2692e1b10bd7a0be644ed8a69c54152bc741e855.
(https://github.com/bitcoin/bitcoin/pull/27835#issuecomment-1589083587)
> Some background, it should indeed not be necessary to install Xcode:
It hasn't been required for at least 7 years, that's why we removed the instructions to install in: https://github.com/bitcoin/bitcoin/commit/2692e1b10bd7a0be644ed8a69c54152bc741e855.
💬 fjahr commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1227953145)
@ajtowns Right, this is the example I thought of as well. But I don't understand why you think the check is unnecessary. From my understanding, this is the point where the subpackage A+B would fail then so I think it is necessary. It just depends on the fact that the function is called with each subpackage.
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1227953145)
@ajtowns Right, this is the example I thought of as well. But I don't understand why you think the check is unnecessary. From my understanding, this is the point where the subpackage A+B would fail then so I think it is necessary. It just depends on the fact that the function is called with each subpackage.
💬 0xB10C commented on pull request "build: Use newest `config.{guess,sub}` available":
(https://github.com/bitcoin/bitcoin/pull/26422#issuecomment-1589094552)
fwiw: This broke the `autogen.sh` part of my development builds on NixOS - found this PR by bisecting. The error is:
```
$ ./autogen.sh
libtoolize: putting auxiliary files in AC_CONFIG_AUX_DIR, 'build-aux'.
libtoolize: copying file 'build-aux/ltmain.sh'
libtoolize: putting macros in AC_CONFIG_MACRO_DIRS, 'build-aux/m4'.
libtoolize: copying file 'build-aux/m4/libtool.m4'
libtoolize: copying file 'build-aux/m4/ltoptions.m4'
libtoolize: copying file 'build-aux/m4/ltsugar.m4'
libtoolize:
...
(https://github.com/bitcoin/bitcoin/pull/26422#issuecomment-1589094552)
fwiw: This broke the `autogen.sh` part of my development builds on NixOS - found this PR by bisecting. The error is:
```
$ ./autogen.sh
libtoolize: putting auxiliary files in AC_CONFIG_AUX_DIR, 'build-aux'.
libtoolize: copying file 'build-aux/ltmain.sh'
libtoolize: putting macros in AC_CONFIG_MACRO_DIRS, 'build-aux/m4'.
libtoolize: copying file 'build-aux/m4/libtool.m4'
libtoolize: copying file 'build-aux/m4/ltoptions.m4'
libtoolize: copying file 'build-aux/m4/ltsugar.m4'
libtoolize:
...
💬 MarcoFalke commented on pull request "build: suppress external warnings by default":
(https://github.com/bitcoin/bitcoin/pull/27872#issuecomment-1589095764)
I think there is still a benefit to developers being notified about warnings in headers. Otherwise no one may notice the false warnings and report them upstream. Also, if they point to real problems, it will be harder to track them down.
(https://github.com/bitcoin/bitcoin/pull/27872#issuecomment-1589095764)
I think there is still a benefit to developers being notified about warnings in headers. Otherwise no one may notice the false warnings and report them upstream. Also, if they point to real problems, it will be harder to track them down.
💬 MarcoFalke commented on pull request "lint: suppress pip spam":
(https://github.com/bitcoin/bitcoin/pull/27871#discussion_r1227961509)
not sure about another place to hardcode the python version, especially if the only reason is to silence a simple single warning.
If you want the warning to be printed less often, you can modify the lines below to be a single call to pip, instead of multiple.
(https://github.com/bitcoin/bitcoin/pull/27871#discussion_r1227961509)
not sure about another place to hardcode the python version, especially if the only reason is to silence a simple single warning.
If you want the warning to be printed less often, you can modify the lines below to be a single call to pip, instead of multiple.
💬 fanquake commented on pull request "build: suppress external warnings by default":
(https://github.com/bitcoin/bitcoin/pull/27872#issuecomment-1589115669)
> I think there is still a benefit to developers being notified about warnings in headers.
They can still be notified, by disabling the suppression.
> Otherwise no one may notice the false warnings and report them upstream.
Then we should stop suppressing them globally in the CI, so we are more likely to see (and then report) the warnings across all relevant platforms / arches. Although then we can't use `-Werror` (which is also why some devs would always be suppressing locally).
(https://github.com/bitcoin/bitcoin/pull/27872#issuecomment-1589115669)
> I think there is still a benefit to developers being notified about warnings in headers.
They can still be notified, by disabling the suppression.
> Otherwise no one may notice the false warnings and report them upstream.
Then we should stop suppressing them globally in the CI, so we are more likely to see (and then report) the warnings across all relevant platforms / arches. Although then we can't use `-Werror` (which is also why some devs would always be suppressing locally).
💬 MarcoFalke commented on pull request "build: suppress external warnings by default":
(https://github.com/bitcoin/bitcoin/pull/27872#issuecomment-1589136728)
Also, it may be good to give some more background. IIRC this is only needed for bdb4 and qt5, so someone compiling bitcoind with sqlite shouldn't have to suppress warnings, no?
(https://github.com/bitcoin/bitcoin/pull/27872#issuecomment-1589136728)
Also, it may be good to give some more background. IIRC this is only needed for bdb4 and qt5, so someone compiling bitcoind with sqlite shouldn't have to suppress warnings, no?
⚠️ 0xB10C opened an issue: "Can't copy to 'build-aux/config.guess' in autoconf.sh: Permission denied"
(https://github.com/bitcoin/bitcoin/issues/27873)
https://github.com/bitcoin/bitcoin/issues/26422 broke the `autogen.sh` part of my development builds on NixOS - found #26422 by bisecting. The error is:
```
$ ./autogen.sh
libtoolize: putting auxiliary files in AC_CONFIG_AUX_DIR, 'build-aux'.
libtoolize: copying file 'build-aux/ltmain.sh'
libtoolize: putting macros in AC_CONFIG_MACRO_DIRS, 'build-aux/m4'.
libtoolize: copying file 'build-aux/m4/libtool.m4'
libtoolize: copying file 'build-aux/m4/ltoptions.m4'
libtoolize: copying file 'bu
...
(https://github.com/bitcoin/bitcoin/issues/27873)
https://github.com/bitcoin/bitcoin/issues/26422 broke the `autogen.sh` part of my development builds on NixOS - found #26422 by bisecting. The error is:
```
$ ./autogen.sh
libtoolize: putting auxiliary files in AC_CONFIG_AUX_DIR, 'build-aux'.
libtoolize: copying file 'build-aux/ltmain.sh'
libtoolize: putting macros in AC_CONFIG_MACRO_DIRS, 'build-aux/m4'.
libtoolize: copying file 'build-aux/m4/libtool.m4'
libtoolize: copying file 'build-aux/m4/ltoptions.m4'
libtoolize: copying file 'bu
...
💬 0xB10C commented on issue "Can't copy to 'build-aux/config.guess' in autoconf.sh: Permission denied":
(https://github.com/bitcoin/bitcoin/issues/27873#issuecomment-1589157862)
https://github.com/bitcoin/bitcoin/blob/8de9bb7a5ab04669369e2bb59ea92a5c1a91a8d2/autogen.sh#L18 evaluates to true for me. The `depends/config.guess` file has a timestamp of `2023-01-01` while the generated `build-aux/config.guess` file has a timestamp of `2021-06-03`.
The generated `build-aux/config.guess` file has the file permissions `-r-xr-xr-x` so overwriting it fails.
```
$ autoreconf --version
autoreconf (GNU Autoconf) 2.71
```
(https://github.com/bitcoin/bitcoin/issues/27873#issuecomment-1589157862)
https://github.com/bitcoin/bitcoin/blob/8de9bb7a5ab04669369e2bb59ea92a5c1a91a8d2/autogen.sh#L18 evaluates to true for me. The `depends/config.guess` file has a timestamp of `2023-01-01` while the generated `build-aux/config.guess` file has a timestamp of `2021-06-03`.
The generated `build-aux/config.guess` file has the file permissions `-r-xr-xr-x` so overwriting it fails.
```
$ autoreconf --version
autoreconf (GNU Autoconf) 2.71
```
💬 0xB10C commented on issue "Can't copy to 'build-aux/config.guess' in autoconf.sh: Permission denied":
(https://github.com/bitcoin/bitcoin/issues/27873#issuecomment-1589198065)
> The generated `build-aux/config.guess` file has the file permissions `-r-xr-xr-x` so overwriting it fails.
`autoreconf --install --force --warnings=all` copies from the nix-store (in this case `/nix/store/3bvdlza5sxy6kjs9y7v0kbx4gxgp0wg8-autoconf-2.71/share/autoconf/build-aux/config.guess`) to `build-aux/config.guess`. The file has `-r-xr-xr-x` permissions in the nix-store. These permissions are preserved when copying. So the problem is a combination of how the nix-store works, how `autorec
...
(https://github.com/bitcoin/bitcoin/issues/27873#issuecomment-1589198065)
> The generated `build-aux/config.guess` file has the file permissions `-r-xr-xr-x` so overwriting it fails.
`autoreconf --install --force --warnings=all` copies from the nix-store (in this case `/nix/store/3bvdlza5sxy6kjs9y7v0kbx4gxgp0wg8-autoconf-2.71/share/autoconf/build-aux/config.guess`) to `build-aux/config.guess`. The file has `-r-xr-xr-x` permissions in the nix-store. These permissions are preserved when copying. So the problem is a combination of how the nix-store works, how `autorec
...
💬 0xB10C commented on issue "Can't copy to 'build-aux/config.guess' in autoconf.sh: Permission denied":
(https://github.com/bitcoin/bitcoin/issues/27873#issuecomment-1589220664)
Changing the permissions for build-aux/config.{guess, sub} with `chmod ug+w` allows us to overwrite the files. Will PR this.
(https://github.com/bitcoin/bitcoin/issues/27873#issuecomment-1589220664)
Changing the permissions for build-aux/config.{guess, sub} with `chmod ug+w` allows us to overwrite the files. Will PR this.
💬 willcl-ark commented on pull request "Mempool: persist mempoolminfee accross restarts":
(https://github.com/bitcoin/bitcoin/pull/27859#discussion_r1228062756)
nit:
```suggestion
// How often to flush fee estimates to disk.
```
(https://github.com/bitcoin/bitcoin/pull/27859#discussion_r1228062756)
nit:
```suggestion
// How often to flush fee estimates to disk.
```
💬 willcl-ark commented on pull request "Mempool: persist mempoolminfee accross restarts":
(https://github.com/bitcoin/bitcoin/pull/27859#discussion_r1228063658)
nit: Perhaps we can give this a slightly more descriptive name? `FEE_ESTIMATE_MAX_AGE` or something?
Would need to update comment below if you do change
(https://github.com/bitcoin/bitcoin/pull/27859#discussion_r1228063658)
nit: Perhaps we can give this a slightly more descriptive name? `FEE_ESTIMATE_MAX_AGE` or something?
Would need to update comment below if you do change
💬 fanquake commented on pull request "build: suppress external warnings by default":
(https://github.com/bitcoin/bitcoin/pull/27872#issuecomment-1589239464)
> IIRC this is only needed for bdb4 and qt5,
There are likely warnings produced for all dependencies (includeing Boost and libevent),
because warning output is dependant on:
* architecture
* operating system (& version)
* compiler (& version)
* compiler flags being used
* other compile options i.e LTO
* packages being used (& version) (& system vs depends)
Where a number of the above continue to change over time.
> so someone compiling bitcoind with sqlite shouldn't have to sup
...
(https://github.com/bitcoin/bitcoin/pull/27872#issuecomment-1589239464)
> IIRC this is only needed for bdb4 and qt5,
There are likely warnings produced for all dependencies (includeing Boost and libevent),
because warning output is dependant on:
* architecture
* operating system (& version)
* compiler (& version)
* compiler flags being used
* other compile options i.e LTO
* packages being used (& version) (& system vs depends)
Where a number of the above continue to change over time.
> so someone compiling bitcoind with sqlite shouldn't have to sup
...
📝 0xB10C opened a pull request: "build: make sure we can overwrite config.{guess,sub} before doing so"
(https://github.com/bitcoin/bitcoin/pull/27875)
Since ea7b8528 (#26422), `autogen.sh` overwrites the `build-aux/config.{guess, sub}` files (installed there by `autoreconf`) with the `depends/config.{guess, sub}` files if these are newer.
The `autoreconf` tool copies them from it's `share/autoconf/build-aux/` directory. Specifically on NixOS, the `share/autoconf/build-aux/` files are located in the nix-store and are read-only. `autoreconf` preserves the read-only permissions when copying. Overwriting them with our `depends/config.{guess, su
...
(https://github.com/bitcoin/bitcoin/pull/27875)
Since ea7b8528 (#26422), `autogen.sh` overwrites the `build-aux/config.{guess, sub}` files (installed there by `autoreconf`) with the `depends/config.{guess, sub}` files if these are newer.
The `autoreconf` tool copies them from it's `share/autoconf/build-aux/` directory. Specifically on NixOS, the `share/autoconf/build-aux/` files are located in the nix-store and are read-only. `autoreconf` preserves the read-only permissions when copying. Overwriting them with our `depends/config.{guess, su
...
💬 hebasto commented on pull request "build: make sure we can overwrite config.{guess,sub} before doing so":
(https://github.com/bitcoin/bitcoin/pull/27875#issuecomment-1589290446)
Concept ACK. However, I'm not a NixOS user and cannot test this PR, unfortunately.
(https://github.com/bitcoin/bitcoin/pull/27875#issuecomment-1589290446)
Concept ACK. However, I'm not a NixOS user and cannot test this PR, unfortunately.
💬 Xekyo commented on pull request "[coinselection] Increase SRD target by change_fee":
(https://github.com/bitcoin/bitcoin/pull/27846#issuecomment-1589306983)
I cannot reproduce the issue with `test/functional/interface_usdt_mempool.py` locally. Any idea what’s up here?
(https://github.com/bitcoin/bitcoin/pull/27846#issuecomment-1589306983)
I cannot reproduce the issue with `test/functional/interface_usdt_mempool.py` locally. Any idea what’s up here?