build: remove configure checks for win libraries we don't link against #17740
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
|
ACK 1778e20. Note the appveyor build failed on the python functional tests which is nothing to do with this PR. |
As for The only one that slightly scares me (that it can be dropped, not the change itself) is |
|
Oh, I missed the "none of these libs actually seem to be passed to the linker"—ok in that case, I don't see why it's checking for the library So this is safe. These checks don't cause linkage. They just check whether the library exists. I suppose the explicit check for |
fanquake
force-pushed
the
remove_windll_configure_checks
branch
from
1778e20 to
fd33d90
Compare
|
Added some additional changes for the MSVC build system. |
|
fgep'ing for the defines below does not get any results for the It does actually seem a bit strange for any defines not to be used in the code no matter what the build system. I'd be surprised if the mingw makefile parses |
| @@ -115,7 +115,7 @@ | |||
| <Link> | |||
| <SubSystem>Console</SubSystem> | |||
| <GenerateDebugInformation>true</GenerateDebugInformation> | |||
| <AdditionalDependencies>crypt32.lib;Iphlpapi.lib;ws2_32.lib;Shlwapi.lib;kernel32.lib;user32.lib;gdi32.lib;winspool.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies)</AdditionalDependencies> | |||
| <AdditionalDependencies>Iphlpapi.lib;ws2_32.lib;Shlwapi.lib;kernel32.lib;user32.lib;gdi32.lib;comdlg32.lib;advapi32.lib;shell32.lib;ole32.lib;oleaut32.lib;uuid.lib;odbc32.lib;odbccp32.lib;%(AdditionalDependencies)</AdditionalDependencies> | |||
There was a problem hiding this comment.
Can confirm removing those two libraries doesn't break the build.
|
Discussed with @theuni. Going to add specific symbols to each of the |
Right. |
fanquake
force-pushed
the
remove_windll_configure_checks
branch
from
fd33d90 to
890baac
Compare
|
Incorporated suggestions from @sipsorcery and added specific symbols to some of the |
|
tACK (msvc and mingw on Win10) 890baac. |
Gitian builds
|
|
Concept ACK Thanks for removing all this old cruft. |
| Needs rebase |
While cross compiling, HOST=x86_64-w64-mingw32, none of these libs actually seem to be passed to the linker.
fanquake
force-pushed
the
remove_windll_configure_checks
branch
from
890baac to
2525c09
Compare
|
ACK 2525c09. |
|
ACK 2525c09 -- diff looks correct |
…'t link against 2525c09 build: remove configure checks for win libraries we don't link against (fanquake) Pull request description: While cross compiling, `HOST=x86_64-w64-mingw32`, none of these libs actually seem to be passed to the linker. i.e tailing a build with `make -j5 V=1 | rg -i 'mingwthrd|winspool|rpcrt4|crypt32'`. I'm not 100% sure about `crypt32`, even though the majority of our Windows cryptography usage, i.e [`CryptAcquireContextW`](https://docs.microsoft.com/en-us/windows/win32/api/wincrypt/nf-wincrypt-cryptacquirecontextw) or [`CryptGenRandom`](https://docs.microsoft.com/en-us/windows/win32/api/wincrypt/nf-wincrypt-cryptgenrandom) is provided by `advapi32`. Note that `rpcrt4` and `mingwthrd` are already missing from the MSVC build, so we can sync the remainder once it's clear what's actually needed. Hopefully sipsorcery can add some MSVC insight. ACKs for top commit: practicalswift: ACK 2525c09 -- diff looks correct sipsorcery: ACK 2525c09. Tree-SHA512: c756618f85ce2ab1e14e5514dbdc490d94c1c6dfd7a3e3d3b16344ae302fb789585dd10b5c2d784f961f3115bec1d914615051b3184bea00dfbcc3c23884ab4a
… we don't link against 2525c09 build: remove configure checks for win libraries we don't link against (fanquake) Pull request description: While cross compiling, `HOST=x86_64-w64-mingw32`, none of these libs actually seem to be passed to the linker. i.e tailing a build with `make -j5 V=1 | rg -i 'mingwthrd|winspool|rpcrt4|crypt32'`. I'm not 100% sure about `crypt32`, even though the majority of our Windows cryptography usage, i.e [`CryptAcquireContextW`](https://docs.microsoft.com/en-us/windows/win32/api/wincrypt/nf-wincrypt-cryptacquirecontextw) or [`CryptGenRandom`](https://docs.microsoft.com/en-us/windows/win32/api/wincrypt/nf-wincrypt-cryptgenrandom) is provided by `advapi32`. Note that `rpcrt4` and `mingwthrd` are already missing from the MSVC build, so we can sync the remainder once it's clear what's actually needed. Hopefully sipsorcery can add some MSVC insight. ACKs for top commit: practicalswift: ACK 2525c09 -- diff looks correct sipsorcery: ACK 2525c09. Tree-SHA512: c756618f85ce2ab1e14e5514dbdc490d94c1c6dfd7a3e3d3b16344ae302fb789585dd10b5c2d784f961f3115bec1d914615051b3184bea00dfbcc3c23884ab4a
This is incorrect. I don't see how this didn't just reintroduce CVE-2012-1910. |
…n libraries we don't link against" This partially reverts commit 2525c09.
… we don't link against 2525c09 build: remove configure checks for win libraries we don't link against (fanquake) Pull request description: While cross compiling, `HOST=x86_64-w64-mingw32`, none of these libs actually seem to be passed to the linker. i.e tailing a build with `make -j5 V=1 | rg -i 'mingwthrd|winspool|rpcrt4|crypt32'`. I'm not 100% sure about `crypt32`, even though the majority of our Windows cryptography usage, i.e [`CryptAcquireContextW`](https://docs.microsoft.com/en-us/windows/win32/api/wincrypt/nf-wincrypt-cryptacquirecontextw) or [`CryptGenRandom`](https://docs.microsoft.com/en-us/windows/win32/api/wincrypt/nf-wincrypt-cryptgenrandom) is provided by `advapi32`. Note that `rpcrt4` and `mingwthrd` are already missing from the MSVC build, so we can sync the remainder once it's clear what's actually needed. Hopefully sipsorcery can add some MSVC insight. ACKs for top commit: practicalswift: ACK 2525c09 -- diff looks correct sipsorcery: ACK 2525c09. Tree-SHA512: c756618f85ce2ab1e14e5514dbdc490d94c1c6dfd7a3e3d3b16344ae302fb789585dd10b5c2d784f961f3115bec1d914615051b3184bea00dfbcc3c23884ab4a
…n libraries we don't link against" This partially reverts commit 2525c09.
…n libraries we don't link against" This partially reverts commit 2525c09.
…n libraries we don't link against" This partially reverts commit 2525c09.
While cross compiling,
HOST=x86_64-w64-mingw32, none of these libs actually seem to be passed to the linker. i.e tailing a build withmake -j5 V=1 | rg -i 'mingwthrd|winspool|rpcrt4|crypt32'.I'm not 100% sure about
crypt32, even though the majority of our Windows cryptography usage, i.eCryptAcquireContextWorCryptGenRandomis provided byadvapi32.Note that
rpcrt4andmingwthrdare already missing from the MSVC build, so we can sync the remainder once it's clear what's actually needed. Hopefully sipsorcery can add some MSVC insight.