Fix download functionality under Windows #3099
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sarlinpe
approved these changes
Jan 5, 2025
| if(CURL_FOUND AND OpenSSL_FOUND) | ||
| set(CRYPTO_FOUND FALSE) | ||
| if(IS_MSVC AND IS_ARM64) | ||
| # OpenSSL crashes for ARM64 under Windows. We therefore fall back to |
Member
There was a problem hiding this comment.
Is this a temporary issue that we can expect to see fixed soon? If not, why using OpenSSL at all instead of Crypto++ on all platforms? Is it because Crypto++ is more difficult to install? It's unideal to have such complex dependency logic for such a small feature.
Contributor
Author
There was a problem hiding this comment.
I would expect this to be fixed eventually and then we can hopefully remove this workaround again. The reason for not defaulting to CryptoPP is that I wanted to keep dependencies minimal on all other platforms. We already transitively depend on OpenSSL on all platforms. I expect essentially nobody to use colmap under Windows ARM except myself in a VM on my MacBook for debugging.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes two issues:
curl[openssl]does not pick up certificates from the system automatically. Now, we instead use Windows native schannel for this purpose.OpenSSL::Cryptoleads to segfaults under Windows/ARM64. Here, we switch to CryptoPP as an alternative.