convert source-download to fetchcontent
Vollstrecker opened this issue · 6 comments
As FC is used for URL download, it should also be used for git clones to reduce complexity. gitclone.cmake can then be dropped.
Using FC everywhere, now that we require a modern version of cmake, makes sense. However it is still preferred to get crypto++ from its git repo rather than from a URL download, still using FetchContent.
Please refer to GetCryptoppSource.cmake and adjust accordingly based on whether git is available on the system or not.
Note: git is not required for the build, but when available it is preferred.
Exactly that's the plan
I now uploaded a first version for this and stumpled across 2 things.
-
CRYPTOPP_PROJECT_DIR (which is replaced) is referred to as an option to set for searching the files, but it's not listed as option. I could add this one with the replacement handling when doing the option-renaming which starts now, or can just ignore it as it's not really a documented one.
-
When using URL-dowload, downloading master is just completely ignored. Is this intentionally?
- CRYPTOPP_PROJECT_DIR (which is replaced) is referred to as an option to set for searching the files, but it's not listed as option. I could add this one with the replacement handling when doing the option-renaming which starts now, or can just ignore it as it's not really a documented one.
It is important to allow the use of locally provided version of crypto++, and this variable could/should become a documented option. It works fine without being an option as the CMakeLists.txt checks if it is pointing to an existing location and uses it instead of fetching.
- When using URL-dowload, downloading master is just completely ignored. Is this intentionally?
URL download gets a zip file corresponding to the released version of crypto++, therefore it does not do a git clone. Not that if the builder has git on her system, git clone will always be preferred to URL download, and if you set CRYPTOPP_USE_MASTER_BRANCH it will clone master not a released tag.
URL download gets a zip file corresponding to the released version of crypto++, therefore it does not do a git clone. Not that if the builder has git on her system, git clone will always be preferred to URL download, and if you set CRYPTOPP_USE_MASTER_BRANCH it will clone master not a released tag.
Sure, the question was if we to support the zip for master also, or if we should tell the user that this combination isn't working. By now just nothing happens which I would call undefined behaviour.
It is important to allow the use of locally provided version of crypto++, and this variable could/should become a documented option. It works fine without being an option as the CMakeLists.txt checks if it is pointing to an existing location and uses it instead of fetching.
k, then I'll add it with the old name as deprecated and double check if the new name really does as it should.
Sure, the question was if we to support the zip for master also, or if we should tell the user that this combination isn't working. By now just nothing happens which I would call undefined behaviour.
If you can reliably get a URL for master from github, you can support it. Otherwise, report an error.