Download server #1084

NeroBurner · 2017-10-18T08:54:57Z

add cmake option HUNTER_DOWNLOAD_SERVER
if set the packages are downloaded from the specified download server
instead of the original URL
the following charactes in the package filename are replaced by '_':
!@#$%^&*?
the new URL has the following setup:
"${HUNTER_DOWNLOAD_SERVER}/${HUNTER_PACKAGE_NAME}/${HUNTER_PACKAGE_VERSION}/${package_filename}"
HUNTER_DOWNLOAD_SERVER_BLACKLIST can be used to provide a list of URLs
to NOT use the download server. For example
-DHUNTER_DOWNLOAD_SERVER_BLACKLIST="github.com"

also adds a shell script to download a package and put it into the expected directory structure

ruslo

Please add documentation for this variable.

ruslo · 2017-10-18T13:21:33Z

cmake/schemes/url_sha1_cmake.cmake.in

@@ -150,6 +150,8 @@ foreach(configuration @HUNTER_PACKAGE_CONFIGURATION_TYPES@)
      "${build_type_opts}"
      "-DCMAKE_INSTALL_PREFIX=@HUNTER_PACKAGE_INSTALL_PREFIX@"
      "-DCMAKE_TOOLCHAIN_FILE=${CMAKE_TOOLCHAIN_FILE}"
+      "-DHUNTER_DOWNLOAD_SERVER=@HUNTER_DOWNLOAD_SERVER@"


Move them to HUNTER_DOWNLOAD_TOOLCHAIN.

I don't understand what I should move where. Do you mean I should rename HUNTER_DOWNLOAD_SERVER to HUNTER_DOWNLOAD_TOOLCHAIN?

See

hunter/cmake/modules/hunter_download.cmake

Lines 393 to 432 in 93dc8ab

file(

APPEND

"${HUNTER_DOWNLOAD_TOOLCHAIN}"

"set(HUNTER_PARENT_PACKAGE \"${HUNTER_PACKAGE_NAME};${HUNTER_PACKAGE_COMPONENT}\" CACHE INTERNAL \"\")\n"

)

file(

APPEND

"${HUNTER_DOWNLOAD_TOOLCHAIN}"

"set(HUNTER_ALREADY_LOCKED_DIRECTORIES \"${HUNTER_ALREADY_LOCKED_DIRECTORIES}\" CACHE INTERNAL \"\")\n"

)

file(

APPEND

"${HUNTER_DOWNLOAD_TOOLCHAIN}"

"set(HUNTER_DISABLE_BUILDS \"${HUNTER_DISABLE_BUILDS}\" CACHE INTERNAL \"\")\n"

)

file(

APPEND

"${HUNTER_DOWNLOAD_TOOLCHAIN}"

"set(HUNTER_USE_CACHE_SERVERS \"${HUNTER_USE_CACHE_SERVERS}\" CACHE INTERNAL \"\")\n"

)

file(

APPEND

"${HUNTER_DOWNLOAD_TOOLCHAIN}"

"set(HUNTER_CACHE_SERVERS \"${HUNTER_CACHE_SERVERS}\" CACHE INTERNAL \"\")\n"

)

file(

APPEND

"${HUNTER_DOWNLOAD_TOOLCHAIN}"

"set(HUNTER_PASSWORDS_PATH \"${HUNTER_PASSWORDS_PATH}\" CACHE INTERNAL \"\")\n"

)

file(

APPEND

"${HUNTER_DOWNLOAD_TOOLCHAIN}"

"set(HUNTER_KEEP_PACKAGE_SOURCES \"${HUNTER_KEEP_PACKAGE_SOURCES}\" CACHE INTERNAL \"\")\n"

)

file(

APPEND

"${HUNTER_DOWNLOAD_TOOLCHAIN}"

"set(HUNTER_SUPPRESS_LIST_OF_FILES \"${HUNTER_SUPPRESS_LIST_OF_FILES}\" CACHE INTERNAL \"\")\n"

)

can I append the two variables at once to the file, or should I use two file(APPEND) calls?

I think you can use one file(APPEND) command, but to keep things consistent it make sense to use two calls to fit current style. This part should be refactored anyway, so I will not be too picky about it here.

ruslo · 2017-10-18T13:22:19Z

cmake/modules/hunter_download.cmake

@@ -79,6 +80,35 @@ function(hunter_download)
  set(HUNTER_PACKAGE_VERSION "${HUNTER_${h_name}_VERSION}")
  set(ver "${HUNTER_PACKAGE_VERSION}")
  set(HUNTER_PACKAGE_URL "${HUNTER_${h_name}_URL}")
+  if(use_download_server)


Refactor this into separate CMake module.

ruslo · 2017-10-18T13:24:18Z

https://github.com/ruslo/hunter/issues/521

NeroBurner · 2017-10-30T14:17:28Z

sorry for the long delay. Incorporated your requested changes

move url replacing to new module
revert changes in url_cmake scheme and add it by HUNTER_TOOLCHAIN

edit: if you approve of changes I'd like to rebase and squash before merging

NeroBurner · 2017-10-30T15:20:10Z

added documentation for new variables

ruslo · 2017-10-30T15:23:47Z

cmake/modules/hunter_download.cmake

@@ -79,6 +81,15 @@ function(hunter_download)
  set(HUNTER_PACKAGE_VERSION "${HUNTER_${h_name}_VERSION}")
  set(ver "${HUNTER_PACKAGE_VERSION}")
  set(HUNTER_PACKAGE_URL "${HUNTER_${h_name}_URL}")
+  if(use_download_server)


You can check if(use_download_server) inside hunter_download_server_url funtion.

ruslo · 2017-10-30T15:26:44Z

cmake/modules/hunter_download_server_url.cmake

+
+  # No free arguments allowed
+  list(LENGTH x_UNPARSED_ARGUMENTS x_len)
+  if(NOT x_len EQUAL 0)


Add quotes: if(NOT x_len EQUAL "0")

will do
just curious: why?

There can be variable with name 0 defined in user's space, it will leak to this code.

http://cgold.readthedocs.io/en/latest/tutorials/variables/regular.html#names

http://cgold.readthedocs.io/en/latest/tutorials/control-structures.html#cmp0054

wow, I did not know that 0 could be a variable. Very good to know, thanks!

ruslo · 2017-10-30T15:31:35Z

cmake/modules/hunter_download_server_url.cmake

+  # update download url to point to HUNTER_DOWNLOAD_SERVER
+  # - blacklist specifies server urls, which are not mapped to the download server
+  set(pkg_url_in_blacklist FALSE)
+  foreach(blacklist_item ${HUNTER_DOWNLOAD_SERVER_BLACKLIST})


HUNTER_DOWNLOAD_SERVER_BLACKLIST

The name is confusing, it's not blacklisting anything.

Also I'm not sure what is the point of this feature. If you're using your own server why not download everything?

The reason for this feature is, that I have an internal GitLab server with some private packages. Without this blacklist I would have to create a tag and then copy the file to the download server.

I chose blacklist, because it removes package URLs from the list of URLs to redirect to the download server

Without this blacklist I would have to create a tag and then copy the file to the download server

How do you use HUNTER_DOWNLOAD_SERVER? I was thinking that you've collected all source archives and put it to server.

I chose blacklist, because it removes package URLs from the list of URLs to redirect to the download server

From https://en.oxforddictionaries.com/definition/blacklist :

A list of people or things that are regarded as unacceptable or untrustworthy and should be excluded or avoided

HUNTER_DOWNLOAD_SERVER_BLACKLIST sounds like list of servers that you don't want to trust. I expect error while trying to download sources from there.

hmm, then maybe whitelist would be better? or do you maybe have a better name?

maybe whitelist would be better?

Sounds like an opposite to blacklist. I don't think it's something about trusted/untrusted.

do you maybe have a better name?

NO_REDIRECT, ALWAYS_DIRECT? Not sure, still confusing - unrelated URL redirection came to mind.

However like I said already I don't fully understand the use case. Can you add few details?

then just one variable is confusing

Using more than one download server is confusing anyway. I was thinking that HUNTER_DOWNLOAD_SERVER will be used as the only server to download sources from. It can be maintained outside, not in Hunter.

Package name: foo

Package URL: https://foo.com/downloads/foo-1.0.tar.gz

Package name: boo

Package URL: https://server-2.com/downloads/boo-3.0.tar.gz

If HUNTER_DOWNLOAD_SERVER is empty we use URLs https://foo.com/downloads/foo-1.0.tar.gz and https://server-2.com/downloads/boo-3.0.tar.gz to download sources. Retry logic implemented in ExternalProject_Add.

If HUNTER_DOWNLOAD_SERVER is a list of servers https://server-1.com;https://server-2.com;https://server-3.com we do analyze original package URL. If original URL matches one of the server we leaving it untouched and set as a server with high priority.

For package foo we pass next URLs to ExternalProject_Add (original URL is not used):

https://server-1.com/foo/1.0/SHASUM/foo-1.0.tar.gz

https://server-2.com/foo/1.0/SHASUM/foo-1.0.tar.gz

https://server-3.com/foo/1.0/SHASUM/foo-1.0.tar.gz

For package boo we pass next URLs to ExternalProject_Add:

https://server-2.com/downloads/boo-3.0.tar.gz (take priority, original URL used)

https://server-1.com/boo/3.0/SHASUM/boo-3.0.tar.gz

https://server-3.com/boo/3.0/SHASUM/boo-3.0.tar.gz

very nice proposal! I like it!

one last question I have is how external_project_add of older cmake versions (<3.7) handle a list of URLs instead of a single URL

3.6 documentation https://cmake.org/cmake/help/v3.6/module/ExternalProject.html

URL /.../src.tgz
Full path or URL of source

3.7 documentation https://cmake.org/cmake/help/v3.7/module/ExternalProject.html

URL /.../src.tgz [/.../src.tgz]...
Full path or URL(s) of source. Multiple URLs are allowed as mirrors.

one last question I have is how external_project_add of older cmake versions (<3.7) handle a list of URLs instead of a single URL

You can check CMAKE_VERSION and allow multiple URLs only for CMake 3.7+. Mention in here:

https://docs.hunter.sh/en/latest/quick-start/cmake.html

Anyway let's finish the first part without multiple URLs feature. You can add it in next pull request.

ruslo · 2017-10-30T15:37:31Z

cmake/modules/hunter_download_server_url.cmake

+    # replace special characters from filename
+    string(REGEX REPLACE "[!@#$%^&*?]" "_" package_filename "${package_filename_raw}")
+    # create new URL
+    set(hunter_package_new_url "${HUNTER_DOWNLOAD_SERVER}/${x_PACKAGE}/${x_VERSION}/${package_filename}")


May be it make sense to use this layout:

https://docs.hunter.sh/en/latest/reference/layouts/deployed.html#download

It will be possible to share ~/.hunter/_Base/Download as far as I understand. E.g. in terms of Nginx set ~/.hunter/_Base/Download as a root location.

where is archive-ID calculated? I think it was the first 7 digits of the sha1 of the package archive?

I think it was the first 7 digits of the sha1 of the package archive?

Yes, see https://docs.hunter.sh/en/latest/reference/layouts/deployed.html#common

NeroBurner · 2017-10-31T16:17:56Z

new version with changes

NeroBurner · 2017-11-02T10:33:05Z

rebased and squashed (also fixed HUNTER_DOWNLOAD_SERVER_NO_REDIRECT variable passing)

ruslo · 2017-11-09T20:56:37Z

cmake/modules/hunter_download_server_url.cmake

+# - HUNTER_DOWNLOAD_SERVER
+#   - if given, URLs are redirected to the download server
+# - HUNTER_DOWNLOAD_SERVER_NO_REDIRECT
+#   - blacklist specifies server urls, which are not mapped to the download server


I'm still not happy with "blacklist" term, I think something about "except" or "exclude" is more appropriate.

I did miss that one. Will fix

ruslo · 2017-11-09T20:59:23Z

docs/reference/user-variables.rst

+HUNTER_DOWNLOAD_SERVER
+======================
+
+When using Hunter without internet access in an internal LAN a download server


without internet access

I don't think it's an only case. Can be useful even with normal internet access, e.g. for speeding up download if regular server is slow.

NeroBurner · 2017-11-10T14:29:03Z

updated documentation, but did not have time to update the module

script synopsis: download_package_for_server.sh [options] -h, --help print this help message and exit. --URL original URL to download from" --SHA1 SHA1 of the archive to download" --PACKAGE name of the package to download" --VERSION version of the package to download" -o,--output-dir output directory to create structure and download file into

- only active if global variable HUNTER_DOWNLOAD_SERVER is set - for cmake << 3.7 - return one URL to try to download from - if the original URL matches one of the download server try to download from the original URL - otherwise download from the first download server - for cmake >= 3.7 - return list of URLs to try to download from - if the original URL matches one of the download server try to download from the original URL first, then from the other listed download server - try to download from the listed download server, one after another

- add documentation for HUNTER_DOWNLOAD_SERVER

NeroBurner · 2017-11-13T13:52:48Z

updated module with full functionality (multiple download server if cmake >= 3.7)
rebased on current master and squashed

ruslo · 2017-11-13T13:59:51Z

cmake/modules/hunter_download.cmake

  set(HUNTER_PACKAGE_SHA1 "${HUNTER_${h_name}_SHA1}")
+  set(ver "${HUNTER_PACKAGE_VERSION}")


Line haven't changed, can be moved up to beautify diff.

ruslo · 2017-11-13T14:02:34Z

cmake/modules/hunter_download_server_url.cmake

+  set(hunter_url_list) # list of URLs to try downloading from
+
+  # check if download server is given
+  string(COMPARE NOTEQUAL "${HUNTER_DOWNLOAD_SERVER}" "" use_download_server)


Since we are inside function you can use return to avoid indentation in next part:

if(NOT use_download_server) set(${x_OUTPUT} ${hunter_package_new_url} PARENT_SCOPE) return() endif()

ruslo · 2017-11-13T14:08:59Z

docs/reference/user-variables.rst

+- ``https://server-2.com/downloads/boo-3.0.tar.gz`` (take priority, original URL used)
+- ``https://server-1.com/boo/3.0/SHASUM/boo-3.0.tar.gz``
+- ``https://server-3.com/boo/3.0/SHASUM/boo-3.0.tar.gz``
+


Add a .. note:: section about multiple URLs feature availability only for CMake 3.7+.

And mention it here: https://docs.hunter.sh/en/latest/quick-start/cmake.html

ruslo · 2017-11-13T14:10:33Z

docs/reference/user-variables.rst

+
+- The characters ``!@#$%^&*?`` occurring in ``${filename}`` are replaced with ``_``.
+- ``${ARCHIVE_ID}`` is the first 7 characters of the package archive ``SHA1`` sum.
+- Note: This is the same structure as Hunter uses for its own ``Download`` directory.


Use builtin .. note:: directive here.

You can use link

:doc:`Download </reference/layouts/deployed>`

instead of just

``Download``

ruslo · 2017-11-13T14:14:30Z

cmake/modules/hunter_download_server_url.cmake

+
+      # check if package URL is in the download server list
+      string(FIND "${x_URL}" "${list_item}" found)
+      if(NOT (found EQUAL -1))


Use quotes: EQUAL "-1".

ruslo · 2017-11-13T14:21:53Z

cmake/modules/hunter_download_server_url.cmake

+        list(APPEND hunter_url_list "${download_server_url}")
+      endif()
+    endforeach()
+


Probably it make sense to add warning:

set(multiple_urls_allowed TRUE) if(CMAKE_VERSION VERSION_LESS 3.7) hunter_status_debug("Multiple URLs are not allowed, use CMake 3.7+ (current version: ${CMAKE_VERSION})") set(multiple_urls_allowed FALSE) endif() if(...) if(multiple_urls_allowed) # ... endif() # ... endif()

NeroBurner · 2017-11-14T07:56:22Z

incorporated changes

NeroBurner · 2017-11-20T12:54:41Z

@ruslo please review

ruslo

NeroBurner commented 6 days ago

Not sure why I missed that, sorry.

Almost there :)

ruslo · 2017-11-20T13:01:00Z

cmake/modules/hunter_download_server_url.cmake

+
+  # check version to determine if external_project_add can handle multiple download URLs
+  set(multiple_urls_allowed TRUE)
+  if(CMAKE_VERSION VERSION_LESS 3.7)


for 3.7 or for TRUE ?

if(CMAKE_VERSION VERSION_LESS "3.7")

did the 3.7 part in "" now

ruslo · 2017-11-20T13:02:42Z

cmake/modules/hunter_download_server_url.cmake

+  # check version to determine if external_project_add can handle multiple download URLs
+  set(multiple_urls_allowed TRUE)
+  if(CMAKE_VERSION VERSION_LESS 3.7)
+    hunter_status_debug("DOWNLOAD_SERVER: Multiple URLs are not supported, only one URL will be used as download server. Use CMake 3.7+ for multiple URLs (current version: ${CMAKE_VERSION})")


Line is too long, split it for readability. Note that you can do:

hunter_status_debug("message 1") hunter_status_debug("message 2")

I did split the creation of the message into multiple lines, or should the real output be broken up into lines?

Looks fine.

ruslo · 2017-11-20T15:29:34Z

Released: https://github.com/ruslo/hunter/releases/tag/v0.19.167

NeroBurner · 2017-11-20T15:33:58Z

Not sure why I missed that, sorry.

No problem at all. I'm always amazed by your responsiveness! Thanks for your hard work 👍

ruslo suggested changes Oct 18, 2017

View reviewed changes

ruslo suggested changes Oct 30, 2017

View reviewed changes

NeroBurner force-pushed the download_server branch from 4ee2e0a to 70df479 Compare November 2, 2017 10:32

ruslo suggested changes Nov 9, 2017

View reviewed changes

riegl-gc added 4 commits November 13, 2017 14:51

use hunter_download_server_url in hunter_download

3968950

add documentation for download server variable

4939464

- add documentation for HUNTER_DOWNLOAD_SERVER

NeroBurner force-pushed the download_server branch from d8a9dae to 4939464 Compare November 13, 2017 13:51

ruslo suggested changes Nov 13, 2017

View reviewed changes

riegl-gc added 7 commits November 14, 2017 08:20

download: minimize diff

8d26bfc

download_server: safer comparison with "-1"

f15ba2f

download_server: less indentation, message for <3.7

aac2488

docs: mention multiple URLs for ExternalProject_Add

faf8c49

docs: use .. note:: directive

1eb53ee

docs: add references for deployed layouts

45da5bb

doc: download_server: use reference

94f1c7a

ruslo suggested changes Nov 20, 2017

View reviewed changes

riegl-gc added 2 commits November 20, 2017 16:13

download_server: split 3.7 debug message in code

65d3ff5

download_server: quote version for security

2bc53dd

ruslo approved these changes Nov 20, 2017

View reviewed changes

ruslo merged commit b818355 into ruslo:master Nov 20, 2017

	file(
	APPEND
	"${HUNTER_DOWNLOAD_TOOLCHAIN}"
	"set(HUNTER_PARENT_PACKAGE \"${HUNTER_PACKAGE_NAME};${HUNTER_PACKAGE_COMPONENT}\" CACHE INTERNAL \"\")\n"
	)
	file(
	APPEND
	"${HUNTER_DOWNLOAD_TOOLCHAIN}"
	"set(HUNTER_ALREADY_LOCKED_DIRECTORIES \"${HUNTER_ALREADY_LOCKED_DIRECTORIES}\" CACHE INTERNAL \"\")\n"
	)
	file(
	APPEND
	"${HUNTER_DOWNLOAD_TOOLCHAIN}"
	"set(HUNTER_DISABLE_BUILDS \"${HUNTER_DISABLE_BUILDS}\" CACHE INTERNAL \"\")\n"
	)
	file(
	APPEND
	"${HUNTER_DOWNLOAD_TOOLCHAIN}"
	"set(HUNTER_USE_CACHE_SERVERS \"${HUNTER_USE_CACHE_SERVERS}\" CACHE INTERNAL \"\")\n"
	)
	file(
	APPEND
	"${HUNTER_DOWNLOAD_TOOLCHAIN}"
	"set(HUNTER_CACHE_SERVERS \"${HUNTER_CACHE_SERVERS}\" CACHE INTERNAL \"\")\n"
	)
	file(
	APPEND
	"${HUNTER_DOWNLOAD_TOOLCHAIN}"
	"set(HUNTER_PASSWORDS_PATH \"${HUNTER_PASSWORDS_PATH}\" CACHE INTERNAL \"\")\n"
	)
	file(
	APPEND
	"${HUNTER_DOWNLOAD_TOOLCHAIN}"
	"set(HUNTER_KEEP_PACKAGE_SOURCES \"${HUNTER_KEEP_PACKAGE_SOURCES}\" CACHE INTERNAL \"\")\n"
	)
	file(
	APPEND
	"${HUNTER_DOWNLOAD_TOOLCHAIN}"
	"set(HUNTER_SUPPRESS_LIST_OF_FILES \"${HUNTER_SUPPRESS_LIST_OF_FILES}\" CACHE INTERNAL \"\")\n"
	)

		set(HUNTER_PACKAGE_SHA1 "${HUNTER_${h_name}_SHA1}")
		set(ver "${HUNTER_PACKAGE_VERSION}")

Download server #1084

Download server #1084

Conversation

NeroBurner commented Oct 18, 2017

ruslo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruslo commented Oct 18, 2017

NeroBurner commented Oct 30, 2017 • edited

NeroBurner commented Oct 30, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruslo Nov 10, 2017 • edited

Choose a reason for hiding this comment

ruslo Nov 10, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NeroBurner commented Oct 31, 2017

NeroBurner commented Nov 2, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NeroBurner commented Nov 10, 2017

NeroBurner commented Nov 13, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NeroBurner commented Nov 14, 2017

NeroBurner commented Nov 20, 2017

ruslo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruslo commented Nov 20, 2017

NeroBurner commented Nov 20, 2017

NeroBurner commented Oct 30, 2017 •

edited

ruslo Nov 10, 2017 •

edited

ruslo Nov 10, 2017 •

edited

NeroBurner commented Nov 2, 2017 •

edited