💬 vasild commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-2514198467)
Ready for review. I updated the OP with some details.
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-2514198467)
Ready for review. I updated the OP with some details.
👋 vasild's pull request is ready for review: "ci: detect outbound internet traffic generated while running tests"
(https://github.com/bitcoin/bitcoin/pull/31349)
(https://github.com/bitcoin/bitcoin/pull/31349)
💬 hodlinator commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1867426740)
I prefer having a "Config file: "-prefix to grep for in logs, so following that pattern for added messages here in this PR (but not going to touch the InitWarning as it might be shown in the GUI).
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1867426740)
I prefer having a "Config file: "-prefix to grep for in logs, so following that pattern for added messages here in this PR (but not going to touch the InitWarning as it might be shown in the GUI).
💬 hodlinator commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1867422543)
Thanks! Fixed in latest push.
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1867422543)
Thanks! Fixed in latest push.
💬 hodlinator commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1867434512)
Current way is clearer & safer to me.
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1867434512)
Current way is clearer & safer to me.
💬 hodlinator commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1867430106)
Would rather not change to quote paths in all log messages of this function as it would touch even more lines.
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1867430106)
Would rather not change to quote paths in all log messages of this function as it would touch even more lines.
💬 hodlinator commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1867438656)
I've updated that commit to better document that I'm overriding the 2 methods (unfortunately our *.python-version* is 3.10.14, python 3.12 includes `@override`).
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1867438656)
I've updated that commit to better document that I'm overriding the 2 methods (unfortunately our *.python-version* is 3.10.14, python 3.12 includes `@override`).
💬 hodlinator commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1867460705)
PR does not contain a test for this as I thought it was straightforward enough to skip and I didn't want to add more things to review/maintain. If we pass `-nopid` we simply do not:
- Create the file
- Write the PID
- Set the `bool` to remember deleting the file
- (Delete the file)
https://github.com/bitcoin/bitcoin/blob/95a0104f2e9869799db84add108ae8c57b56d360/src/init.cpp#L165-L192
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1867460705)
PR does not contain a test for this as I thought it was straightforward enough to skip and I didn't want to add more things to review/maintain. If we pass `-nopid` we simply do not:
- Create the file
- Write the PID
- Set the `bool` to remember deleting the file
- (Delete the file)
https://github.com/bitcoin/bitcoin/blob/95a0104f2e9869799db84add108ae8c57b56d360/src/init.cpp#L165-L192
💬 hodlinator commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1867433277)
Thanks, agree that it's a bit redundant in an assert, leftover from previous comment. Updated.
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1867433277)
Thanks, agree that it's a bit redundant in an assert, leftover from previous comment. Updated.
💬 hodlinator commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1867461728)
Moved back the log message in latest push to clarify.
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1867461728)
Moved back the log message in latest push to clarify.
💬 hodlinator commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1867463418)
Cheers, replaced hashes with commit message titles.
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1867463418)
Cheers, replaced hashes with commit message titles.
💬 hodlinator commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1867468510)
Having a latent *bitcoin.conf* is not ideal and something the user should correct if they want to run with `-noconf` long-term to reduce potential for confusion ("Why is my change to *bitcoin.conf* not getting picked up?!!" until realizing `-noconf` was used). But changed to `LogInfo` for now.
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1867468510)
Having a latent *bitcoin.conf* is not ideal and something the user should correct if they want to run with `-noconf` long-term to reduce potential for confusion ("Why is my change to *bitcoin.conf* not getting picked up?!!" until realizing `-noconf` was used). But changed to `LogInfo` for now.
✅ willcl-ark closed a pull request: "build: special instruction check script"
(https://github.com/bitcoin/bitcoin/pull/26693)
(https://github.com/bitcoin/bitcoin/pull/26693)
💬 willcl-ark commented on pull request "build: special instruction check script":
(https://github.com/bitcoin/bitcoin/pull/26693#issuecomment-2514215309)
Closing for now, as there's not much interest currently. Would be happy to re-open in the future.
(https://github.com/bitcoin/bitcoin/pull/26693#issuecomment-2514215309)
Closing for now, as there's not much interest currently. Would be happy to re-open in the future.
💬 maflcko commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#discussion_r1867506582)
I don't think this works reliably? See:
```
This output will be buffered if written to a file or pipe, so a program reading from the file or pipe may not see packets for an arbitrary amount of time after they are received. Use the -U flag to cause packets to be
written as soon as they are received.
(https://github.com/bitcoin/bitcoin/pull/31349#discussion_r1867506582)
I don't think this works reliably? See:
```
This output will be buffered if written to a file or pipe, so a program reading from the file or pipe may not see packets for an arbitrary amount of time after they are received. Use the -U flag to cause packets to be
written as soon as they are received.
💬 maflcko commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#discussion_r1867509291)
Why is this needed? The file isn't cleared anyway (and exited if it isn't clear), so might as well just run the processes and never kill them?
Also, it doesn't seem to be working anyway on some of the tasks?
(https://github.com/bitcoin/bitcoin/pull/31349#discussion_r1867509291)
Why is this needed? The file isn't cleared anyway (and exited if it isn't clear), so might as well just run the processes and never kill them?
Also, it doesn't seem to be working anyway on some of the tasks?
💬 maflcko commented on issue "scripts: check for .text.startup sections":
(https://github.com/bitcoin/bitcoin/issues/18603#issuecomment-2514249729)
https://github.com/bitcoin/bitcoin/pull/26693 was closed
(https://github.com/bitcoin/bitcoin/issues/18603#issuecomment-2514249729)
https://github.com/bitcoin/bitcoin/pull/26693 was closed
⚠️ hebasto opened an issue: "qa: Broken `wallet_multiwallet.py`"
(https://github.com/bitcoin/bitcoin/issues/31409)
On the master branch @ ebe4cac38bf6c510b00b8e080acab079c54016d6, the `wallet_multiwallet.py` test has several issues:
1. This code: https://github.com/bitcoin/bitcoin/blob/ebe4cac38bf6c510b00b8e080acab079c54016d6/test/functional/wallet_multiwallet.py#L132-L140 checks for the "Error scanning" message in the `debug.log` caused by processing the `no_access` directory. However, the same message can also be generated when parsing the `self_walletdat_symlink` directory. As a result, the current imp
...
(https://github.com/bitcoin/bitcoin/issues/31409)
On the master branch @ ebe4cac38bf6c510b00b8e080acab079c54016d6, the `wallet_multiwallet.py` test has several issues:
1. This code: https://github.com/bitcoin/bitcoin/blob/ebe4cac38bf6c510b00b8e080acab079c54016d6/test/functional/wallet_multiwallet.py#L132-L140 checks for the "Error scanning" message in the `debug.log` caused by processing the `no_access` directory. However, the same message can also be generated when parsing the `self_walletdat_symlink` directory. As a result, the current imp
...
💬 laanwj commented on pull request "build: special instruction check script":
(https://github.com/bitcoin/bitcoin/pull/26693#issuecomment-2514359474)
What was the blocker here?
Is the problem "how to check just-generated .o files" in the CI/GUIX build?
> This sounds very cool (and more practically-useful than the check here perhaps).
Revisiting this, i'm not sure. Checking the final binary would be more practical with integration in the process (it could run with the other symbol/security checks), but with optimizations like LTO it's not necessarily true that the different init functions end up in different places. So there's somethi
...
(https://github.com/bitcoin/bitcoin/pull/26693#issuecomment-2514359474)
What was the blocker here?
Is the problem "how to check just-generated .o files" in the CI/GUIX build?
> This sounds very cool (and more practically-useful than the check here perhaps).
Revisiting this, i'm not sure. Checking the final binary would be more practical with integration in the process (it could run with the other symbol/security checks), but with optimizations like LTO it's not necessarily true that the different init functions end up in different places. So there's somethi
...
💬 Sjors commented on pull request "guix: Notarize MacOS app bundle and codesign all MacOS and Windows binaries":
(https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2514411310)
> It's unclear to me whether the standalone binaries need to be notarized too.
Do you mean the binaries in `unsigned.{zip,tar.gz}` archives? I think it's fine not to.
(https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2514411310)
> It's unclear to me whether the standalone binaries need to be notarized too.
Do you mean the binaries in `unsigned.{zip,tar.gz}` archives? I think it's fine not to.