💬 hebasto commented on issue "macOS 15.0.1 can't build Qt (Intel, without Xcode)":
(https://github.com/bitcoin/bitcoin/issues/31054#issuecomment-2402733760)
I cannot neither confirm nor reproduce the issue on my MacBook Pro (2019, Intel).
The following scenarios were tested:
- Sonoma 14.6.1 + Command-Line Tools 15.0.
- Sonoma 14.7 + Command-Line Tools 16.0.
- Sequoia 15.0.1 + Command-Line Tools 16.0.
- Sequoia 15.0.1 + Xcode 16.0.
- Sequoia 15.0.1 + Xcode 16.0 installed and then deleted.
For all scenarios, here is the list of the installed Homebrew's packages:
```
% brew list
==> Formulae
cmake pkg-config
==> Casks
```
> But i
...
(https://github.com/bitcoin/bitcoin/issues/31054#issuecomment-2402733760)
I cannot neither confirm nor reproduce the issue on my MacBook Pro (2019, Intel).
The following scenarios were tested:
- Sonoma 14.6.1 + Command-Line Tools 15.0.
- Sonoma 14.7 + Command-Line Tools 16.0.
- Sequoia 15.0.1 + Command-Line Tools 16.0.
- Sequoia 15.0.1 + Xcode 16.0.
- Sequoia 15.0.1 + Xcode 16.0 installed and then deleted.
For all scenarios, here is the list of the installed Homebrew's packages:
```
% brew list
==> Formulae
cmake pkg-config
==> Casks
```
> But i
...
📝 maflcko opened a pull request: "lint: commit-script-check.sh: echo to stderr"
(https://github.com/bitcoin/bitcoin/pull/31063)
This makes it easier to redirect the produced `git diff` on failure. On success, it shouldn't hurt, because the same output is still present, just on stderr.
Can be tested by introducing a fault in any scripted diff and then calling `commit-script-check.sh HEAD~..HEAD > any_file.txt`. Previously the file contained the full output, now it contains just the diff.
(https://github.com/bitcoin/bitcoin/pull/31063)
This makes it easier to redirect the produced `git diff` on failure. On success, it shouldn't hurt, because the same output is still present, just on stderr.
Can be tested by introducing a fault in any scripted diff and then calling `commit-script-check.sh HEAD~..HEAD > any_file.txt`. Previously the file contained the full output, now it contains just the diff.
💬 maflcko commented on pull request "doc: cmake: prepend "build" to functional/test_runner.py":
(https://github.com/bitcoin/bitcoin/pull/30859#issuecomment-2402756550)
lgtm ACK c15ef096ce317ae911dd756593adeac9d9a0b5a1
(https://github.com/bitcoin/bitcoin/pull/30859#issuecomment-2402756550)
lgtm ACK c15ef096ce317ae911dd756593adeac9d9a0b5a1
✅ maflcko closed a pull request: "bench: Adds a benchmark for CheckInputScripts"
(https://github.com/bitcoin/bitcoin/pull/29745)
(https://github.com/bitcoin/bitcoin/pull/29745)
💬 theuni commented on pull request "RFC: build: support for pre-compiled headers.":
(https://github.com/bitcoin/bitcoin/pull/31053#issuecomment-2402769455)
> > Combining with #30911 produces even more of a speedup (with Make, ninja is about the same).
>
> Why are ninja builds not affected?
Ninja builds are sped up significantly by pch (this PR), it's the combo with #30911 doesn't seem to have much effect.
(https://github.com/bitcoin/bitcoin/pull/31053#issuecomment-2402769455)
> > Combining with #30911 produces even more of a speedup (with Make, ninja is about the same).
>
> Why are ninja builds not affected?
Ninja builds are sped up significantly by pch (this PR), it's the combo with #30911 doesn't seem to have much effect.
💬 naumenkogs commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1793800548)
2658d2b2dfaacbfb2ddf2f1305ed8c3c9e134b71
won't this fail in `DIFFERENT_WITNESS` case?
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1793800548)
2658d2b2dfaacbfb2ddf2f1305ed8c3c9e134b71
won't this fail in `DIFFERENT_WITNESS` case?
💬 maflcko commented on pull request "bench: Adds a benchmark for CheckInputScripts":
(https://github.com/bitcoin/bitcoin/pull/29745#issuecomment-2402770771)
Closing for now due to inactivity since the day it was opened. Leave a comment below if you want this re-opened, or open a new pull.
(https://github.com/bitcoin/bitcoin/pull/29745#issuecomment-2402770771)
Closing for now due to inactivity since the day it was opened. Leave a comment below if you want this re-opened, or open a new pull.
✅ maflcko closed a pull request: "validation: Use witness maleation flag for non-segwit blocks"
(https://github.com/bitcoin/bitcoin/pull/29540)
(https://github.com/bitcoin/bitcoin/pull/29540)
💬 maflcko commented on pull request "validation: Use witness maleation flag for non-segwit blocks":
(https://github.com/bitcoin/bitcoin/pull/29540#issuecomment-2402780104)
Closing for now due to inactivity (lack of reply to a question about the motivation for this change). Please leave a comment, if you want this re-opened.
(https://github.com/bitcoin/bitcoin/pull/29540#issuecomment-2402780104)
Closing for now due to inactivity (lack of reply to a question about the motivation for this change). Please leave a comment, if you want this re-opened.
🤔 stickies-v reviewed a pull request: "refactor: Check translatable format strings at compile-time"
(https://github.com/bitcoin/bitcoin/pull/31061#pullrequestreview-2357070300)
Concept ACK, and this is a smaller diff than I'd expected to implement this, nice!
I need to think about it more and work out some code, but I feel like the new interface is not as intuitive yet as it could be. Will follow up with that in next review.
(https://github.com/bitcoin/bitcoin/pull/31061#pullrequestreview-2357070300)
Concept ACK, and this is a smaller diff than I'd expected to implement this, nice!
I need to think about it more and work out some code, but I feel like the new interface is not as intuitive yet as it could be. Will follow up with that in next review.
💬 stickies-v commented on pull request "refactor: Check translatable format strings at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1793488009)
in aaaa4fb20156b4375d92e1eca4acc90a425a1896: seems more straightforward to just extend `ConstevalStringLiteral` instead of creating a new `util::Original`? This would 1) minimize the diff and 2) I find `ConstevalStringLiteral` a much more helpful name than `Original`.
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1793488009)
in aaaa4fb20156b4375d92e1eca4acc90a425a1896: seems more straightforward to just extend `ConstevalStringLiteral` instead of creating a new `util::Original`? This would 1) minimize the diff and 2) I find `ConstevalStringLiteral` a much more helpful name than `Original`.
💬 stickies-v commented on pull request "refactor: Check translatable format strings at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1793707636)
```suggestion
QMessageBox::information(m_parent_widget, tr("Restore wallet message"), QString::fromStdString("Wallet restored successfully \n"));
```
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1793707636)
```suggestion
QMessageBox::information(m_parent_widget, tr("Restore wallet message"), QString::fromStdString("Wallet restored successfully \n"));
```
💬 stickies-v commented on pull request "refactor: Check translatable format strings at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1793744937)
This function is currently unused, and it seems incorrectly implemented as it doesn't compile when actually used:
```
In file included from ../../src/clientversion.cpp:9:
../../src/util/translation.h:42:48: error: too many arguments to function call, expected 0, have 1
std::string translate() { return translate(original.fmt); }
~~~~~~~~~ ^~~~~~~~~~~~
../../src/util/translation.h:106:42: note: in instantiation of member function 'util::bilingual_fm
...
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1793744937)
This function is currently unused, and it seems incorrectly implemented as it doesn't compile when actually used:
```
In file included from ../../src/clientversion.cpp:9:
../../src/util/translation.h:42:48: error: too many arguments to function call, expected 0, have 1
std::string translate() { return translate(original.fmt); }
~~~~~~~~~ ^~~~~~~~~~~~
../../src/util/translation.h:106:42: note: in instantiation of member function 'util::bilingual_fm
...
💬 stickies-v commented on pull request "refactor: Check translatable format strings at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1793473436)
typo nit
```suggestion
/** Type to denote whether an original string literal is translatable */
```
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1793473436)
typo nit
```suggestion
/** Type to denote whether an original string literal is translatable */
```
💬 stickies-v commented on pull request "refactor: Check translatable format strings at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1793829614)
nit: I would prefer templating this on an enum `{Translateable,Untranslateable}` for readability over the non-obvious `<{true,false}>` boolean
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1793829614)
nit: I would prefer templating this on an enum `{Translateable,Untranslateable}` for readability over the non-obvious `<{true,false}>` boolean
💬 kevkevinpal commented on pull request "bench: Adds a benchmark for CheckInputScripts":
(https://github.com/bitcoin/bitcoin/pull/29745#issuecomment-2402804271)
> Closing for now due to inactivity since the day it was opened. Leave a comment below if you want this re-opened, or open a new pull.
ya sorry about that I can leave it up for grabs if anyone else wants to look at it otherwise I can reopen when I have a more finalized PR open, again sorry for leaving it open but inactive
(https://github.com/bitcoin/bitcoin/pull/29745#issuecomment-2402804271)
> Closing for now due to inactivity since the day it was opened. Leave a comment below if you want this re-opened, or open a new pull.
ya sorry about that I can leave it up for grabs if anyone else wants to look at it otherwise I can reopen when I have a more finalized PR open, again sorry for leaving it open but inactive
💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2402903171)
@Sjors
I cannot reproduce the issue on macOS Intel w/o Xcode you mentioned in:
- https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2388568119
- https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2396235386
On my MacBook Pro (2019, Intel) + macOS 15.0.1, no Xcode, I can successfully build 86837bc75c4c1282fc652dfd755273db956222db as follows:
```
% brew list
==> Formulae
cmake ninja pkg-config
==> Casks
% make -j 16 -C depends NO_WALLET=1 NO_UPNP=1 NO_ZMQ=1
% cm
...
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2402903171)
@Sjors
I cannot reproduce the issue on macOS Intel w/o Xcode you mentioned in:
- https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2388568119
- https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2396235386
On my MacBook Pro (2019, Intel) + macOS 15.0.1, no Xcode, I can successfully build 86837bc75c4c1282fc652dfd755273db956222db as follows:
```
% brew list
==> Formulae
cmake ninja pkg-config
==> Casks
% make -j 16 -C depends NO_WALLET=1 NO_UPNP=1 NO_ZMQ=1
% cm
...
💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2402950941)
> They'd have to be cleared on all machines. Let me known if you want that to happen.
At some point, we may have to do this anyway.
> If you are just looking for a temporary workaround for testing, you can pick a different qt version, like `6.7.1`.
Not really, as it'll break patches.
> Or you can temporarily use a new sources volume, similar to the diff in [#30851 (comment)](https://github.com/bitcoin/bitcoin/issues/30851#issuecomment-2337816419).
Thanks for a hint. I've added a t
...
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2402950941)
> They'd have to be cleared on all machines. Let me known if you want that to happen.
At some point, we may have to do this anyway.
> If you are just looking for a temporary workaround for testing, you can pick a different qt version, like `6.7.1`.
Not really, as it'll break patches.
> Or you can temporarily use a new sources volume, similar to the diff in [#30851 (comment)](https://github.com/bitcoin/bitcoin/issues/30851#issuecomment-2337816419).
Thanks for a hint. I've added a t
...
👍 itornaza approved a pull request: "build: Bump minimum supported macOS to 12.0"
(https://github.com/bitcoin/bitcoin/pull/31048#pullrequestreview-2357857323)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/31048#pullrequestreview-2357857323)
Concept ACK
🤔 mzumsande reviewed a pull request: "init: Some small chainstate load improvements"
(https://github.com/bitcoin/bitcoin/pull/31046#pullrequestreview-2357858524)
Code Review / lightly tested ACK 31cc5006c3de4dd6a1f7a238684163956604df45
(https://github.com/bitcoin/bitcoin/pull/31046#pullrequestreview-2357858524)
Code Review / lightly tested ACK 31cc5006c3de4dd6a1f7a238684163956604df45