build: add test_bitcoin.exe to win installer, don't install entire docs/ dir #25809
Conversation
|
GUIX hashes x86: arm64: |
|
Concept ACK I could test if it was a change possible to test with windows binaries built using instructions in https://github.com/bitcoin/bitcoin/blob/master/doc/build-windows.md No clue about guix build process for win installer |
| File @abs_top_builddir@/release/@BITCOIN_TX_NAME@@EXEEXT@ | ||
| File @abs_top_builddir@/release/@BITCOIN_WALLET_TOOL_NAME@@EXEEXT@ | ||
| SetOutPath $INSTDIR\doc | ||
| File /r /x Makefile* @abs_top_srcdir@/doc\*.* |
There was a problem hiding this comment.
Do we not want to install any docs?
There was a problem hiding this comment.
what would be docs that would be critical to include with the installation executable?
There was a problem hiding this comment.
Do we not want to install any docs?
Windows is the odd one out in this regard. I think the README and copyright is fine. I'd assume most users never even realised we were dumping a bumping of random/non-windows documentation onto their drives. If you think we should be installing some, then you should suggest a list.
There was a problem hiding this comment.
It's not a question of critical, just what's appropriate. I would think most of the docs would be. Exceptions are build docs, old release notes, and similar.
There was a problem hiding this comment.
I would think most of the docs would be. Exceptions are build docs, old release notes, and similar.
Again, feel free to propose a list / change. I think your assessment of "most" is incorrect. Of all the .md files in the 23.x docs/ folder, less than half would be relevent.
In any case don't think this something worth spending time to try and optimise. If removing the installation of docs is too contreversial, happy to just drop that commit from this PR, and we can close the second half of #17171 as wont fix.
It is. Run |
ghost
left a comment
ghost
left a comment
There was a problem hiding this comment.
ACK d755ffc
Tested on Win 10 and installer works as expected. Docs directory isn't added and test_bitcoin.exe is added in daemon directory.
PS C:\Program Files\Bitcoin> ls
Directory: C:\Program Files\Bitcoin
Mode LastWriteTime Length Name
---- ------------- ------ ----
d----- 10-08-2022 16:40 daemon
-a---- 10-08-2022 16:42 36432928 bitcoin-qt.exe
-a---- 10-08-2022 16:42 1142 COPYING.txt
-a---- 10-08-2022 16:42 846 readme.txt
-a---- 10-08-2022 16:42 152493 uninstall.exe
PS C:\Program Files\Bitcoin> ls .\daemon\
Directory: C:\Program Files\Bitcoin\daemon
Mode LastWriteTime Length Name
---- ------------- ------ ----
-a---- 10-08-2022 16:42 2176512 bitcoin-cli.exe
-a---- 10-08-2022 16:42 3810304 bitcoin-tx.exe
-a---- 10-08-2022 16:42 8976896 bitcoin-wallet.exe
-a---- 10-08-2022 16:42 14503424 bitcoind.exe
-a---- 10-08-2022 16:42 24411648 test_bitcoin.exe
Although bitcoin core (windows) users will have to manually delete docs directory because installer just ensures its not added with new docs, it does not delete the docs already present.
|
Guix build on |
I don't think this needed a milestone, but is going to be merged now in any case. I'll combine some other minor share additions into #25829. |
6 participants
Haven't tested other than checking that it Guix builds.
Fixes: #17171.
Guix build (x86_64):