Re: [cmake-developers] [patch] One more pattern for extracting file name from URL

2016-07-05 Thread Ruslan Baratov via cmake-developers 

On 05-Jul-16 23:26, Brad King wrote:

On 07/05/2016 04:13 PM, Ruslan Baratov wrote:

Works fine on my tests.

Thanks for testing!


However I still vote for using default value for 'fname' instead
of FATAL_ERROR.

I think that behavior would be surprising.
Not such surprising as some error in internal CMake module with no 
detail description how it can be fixed. My point is that if it will work 
in most cases then why not? Most users will not even notice this.





If something bad will happen user always can use DOWNLOAD_NAME.

Example of the test that will fail with current version but works
fine with default name:

 URL "http://tinyurl.com/jn86pmc;

In that case one can just use DOWNLOAD_NAME.  Auto-extraction of
the filename from the URL is a convenience for a common case, but
in principle DOWNLOAD_NAME could always be used.
The problem with DOWNLOAD_NAME is about generalizing code. If I have 100 
ExternalProject_Add commands with such URLs then I have to add 100 
DOWNLOAD_NAME. In my case I have a bunch of templates for 
ExternalProject_Add and to introduce this feature either I have:
* test URL for the pattern in similar way CMake do and predict the fact 
that CMake will fail to extract name (not stable, what if CMake logic 
changed? DRY violation)
* force some name unconditionally like archive.tar (most of the projects 
works fine and now for no reason archives will have another names)
* add about 20-30 lines of code to core modules, for each 
ExternalProject_Add template add parsing logic and 1 line for each 
package version declaration on user side


And all this without introducing any useful features, just to suppress 
CMake error for 0.01% of cases.


Anyway I've found workaround that I will use for such URLs. According to 
my tests most of the servers ignore fragments in URL and CMake can 
extract name from them. E.g. this works:


   cmake_minimum_required(VERSION 3.5)

   include(ExternalProject)

   ExternalProject_Add(
foo
URL "http://tinyurl.com/jn86pmc#a.tar;
URL_HASH SHA1=9c29c30fff99b7c1479b2e9d4926dcc3f8d364e0
CONFIGURE_COMMAND ""
BUILD_COMMAND ""
INSTALL_COMMAND ""
   )

Ruslo
-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Re: [cmake-developers] [patch] One more pattern for extracting file name from URL

2016-07-05 Thread Brad King 
On 07/05/2016 04:13 PM, Ruslan Baratov wrote:
> Works fine on my tests.

Thanks for testing!

> However I still vote for using default value for 'fname' instead
> of FATAL_ERROR.

I think that behavior would be surprising.

> If something bad will happen user always can use DOWNLOAD_NAME.
> 
> Example of the test that will fail with current version but works
> fine with default name:
> 
> URL "http://tinyurl.com/jn86pmc;

In that case one can just use DOWNLOAD_NAME.  Auto-extraction of
the filename from the URL is a convenience for a common case, but
in principle DOWNLOAD_NAME could always be used.

-Brad

-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Re: [cmake-developers] [patch] One more pattern for extracting file name from URL

2016-07-05 Thread Ruslan Baratov via cmake-developers 

On 05-Jul-16 22:03, Brad King wrote:

On 07/01/2016 03:35 PM, Brad King wrote:

I don't think we should duplicate the

   "(\\.|=)(7z|tar|tar\\.bz2|tar\\.gz|tar\\.xz|tbz2|tgz|txz|zip)$"

expression.  The stripping of ?.* can be done earlier, or done as
part of the main match.

Please try this commit:

  ExternalProject: Match filenames in URLs with query strings and anchors
  https://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=57c337e2

Thanks,
-Brad

Works fine on my tests. However I still vote for using default value for 
'fname' instead of FATAL_ERROR. If something bad will happen user always 
can use DOWNLOAD_NAME.


Example of the test that will fail with current version but works fine 
with default name:


   cmake_minimum_required(VERSION 3.5)

   include(ExternalProject)

   ExternalProject_Add(
foo
URL "http://tinyurl.com/jn86pmc;
URL_HASH SHA1=9c29c30fff99b7c1479b2e9d4926dcc3f8d364e0
CONFIGURE_COMMAND ""
BUILD_COMMAND ""
INSTALL_COMMAND ""
   )

Ruslo
-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Re: [cmake-developers] [patch] One more pattern for extracting file name from URL

2016-07-05 Thread Ben Boeckel 
On Tue, Jul 05, 2016 at 13:46:06 -0400, Brad King wrote:
> On 07/05/2016 01:31 PM, Ben Boeckel wrote:
> > Here's another idea: add an `OUTPUT_FILE ` keyword argument to
> > `file(DOWNLOAD)` to get the actual filename after content disposition
> > resolution (probably similar for 30x rewrites). This would also be
> > useful for things other than ExternalProject.
> 
> This may be a reasonable idea for the future but we are getting
> way off from the original patch here.

https://gitlab.kitware.com/cmake/cmake/issues/16187

--Ben
-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Re: [cmake-developers] [patch] One more pattern for extracting file name from URL

2016-07-05 Thread Ben Boeckel 
On Tue, Jul 05, 2016 at 21:01:36 +0300, Ruslan Baratov wrote:
> Well it's never too late to learn something new. Also `tar` is not 
> widely used on Windows so to write nice code you should convert it to 
> `cmake -E tar`. That the whole point of `cmake -E` feature.
> I don't see the problem here, it was used internally, unpack archive for 
> you and you can do what you want with **unpacked** sources. Anyway you 
> can always force the name with |DOWNLOAD_NAME.|

I'm more considering the case of using the files as they exist on the
disk for other purposes, not necessarily through code (which probably
should be using `cmake -E tar`). For example, I will do:

  - tar xf $tarball.tar.gz
  - cp -a $tarball $tarball.to-patch
  - $edit .to-patch
  - diff -U5 -r $tarball $tarball.to-patch

to create patches for ExternalProject tarballs we use.

--Ben
-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Re: [cmake-developers] [patch] One more pattern for extracting file name from URL

2016-07-05 Thread Ruslan Baratov via cmake-developers 

On 05-Jul-16 20:31, Ben Boeckel wrote:

On Tue, Jul 05, 2016 at 19:49:46 +0300, Ruslan Baratov wrote:

Archive can be unpacked by "cmake -E tar xf" which detect type
automatically as far as I understand. At least this one used internally
by ExternalProject (see the testing I've made before).

CMake is not my go-to tool for extracting files though (and similarly
for others I imagine), so running `tar xf archive.tar` and getting an
error because it is actually a .tar.gz (or even a .zip) file is not
nice.
Well it's never too late to learn something new. Also `tar` is not 
widely used on Windows so to write nice code you should convert it to 
`cmake -E tar`. That the whole point of `cmake -E` feature.
I don't see the problem here, it was used internally, unpack archive for 
you and you can do what you want with **unpacked** sources. Anyway you 
can always force the name with |DOWNLOAD_NAME.|



So even if it seems that archive name is `|archive.tar.gz` it should be
downloaded to
`||gitlab-ce-v8.9.3-5e546d9b4728fc9c9623992a678cbea9eb2098f1.tar.gz`. So
I think it's easily can be file with another extension.|

Here's another idea: add an `OUTPUT_FILE ` keyword argument to
`file(DOWNLOAD)` to get the actual filename after content disposition
resolution (probably similar for 30x rewrites). This would also be
useful for things other than ExternalProject.

--Ben

Good idea, I'll be happy to see this feature in future CMake releases.

Ruslo
-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Re: [cmake-developers] [patch] One more pattern for extracting file name from URL

2016-07-05 Thread Brad King 
On 07/05/2016 01:31 PM, Ben Boeckel wrote:
> Here's another idea: add an `OUTPUT_FILE ` keyword argument to
> `file(DOWNLOAD)` to get the actual filename after content disposition
> resolution (probably similar for 30x rewrites). This would also be
> useful for things other than ExternalProject.

This may be a reasonable idea for the future but we are getting
way off from the original patch here.

I will likely commit something from this thread when I get a chance
to review it.

-Brad

-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Re: [cmake-developers] [patch] One more pattern for extracting file name from URL

2016-07-05 Thread Ben Boeckel 
On Tue, Jul 05, 2016 at 19:49:46 +0300, Ruslan Baratov wrote:
> Archive can be unpacked by "cmake -E tar xf" which detect type 
> automatically as far as I understand. At least this one used internally 
> by ExternalProject (see the testing I've made before).

CMake is not my go-to tool for extracting files though (and similarly
for others I imagine), so running `tar xf archive.tar` and getting an
error because it is actually a .tar.gz (or even a .zip) file is not
nice.

> So even if it seems that archive name is `|archive.tar.gz` it should be 
> downloaded to 
> `||gitlab-ce-v8.9.3-5e546d9b4728fc9c9623992a678cbea9eb2098f1.tar.gz`. So 
> I think it's easily can be file with another extension.|

Here's another idea: add an `OUTPUT_FILE ` keyword argument to
`file(DOWNLOAD)` to get the actual filename after content disposition
resolution (probably similar for 30x rewrites). This would also be
useful for things other than ExternalProject.

--Ben
-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Re: [cmake-developers] [patch] One more pattern for extracting file name from URL

2016-07-05 Thread Ruslan Baratov via cmake-developers 

On 05-Jul-16 18:50, Ben Boeckel wrote:

On Tue, Jul 05, 2016 at 18:43:25 +0300, Ruslan Baratov wrote:

By default not, goes to -prefix/src/archive.tar .

Right, but if a download directory is set, this won't work. Can the
project name at least be added to the name?
If user are forcing directory then it is user's responsibility I think. 
For example GitHub releases goes without name of the project so both:


* https://github.com/ruslo/polly/archive/v0.10.1.tar.gz
* https://github.com/ruslo/hunter/archive/v0.10.1.tar.gz

will be downloaded to v0.10.1.tar.gz - this filename extracted from URL 
by CMake. In general patch made in a code branch where was FATAL_ERROR 
before, so it doesn't affect backward compatibility.




Though I'm partial to keeping the extension still; if I need to add a
patch to a project, it'd be nice if the extension weren't a lie when I
go to extract the tarball.
Archive can be unpacked by "cmake -E tar xf" which detect type 
automatically as far as I understand. At least this one used internally 
by ExternalProject (see the testing I've made before).


Also note that in fact name of the file extracted from URL can differ 
from real:

|
> curl --head 
https://gitlab.com/gitlab-org/gitlab-ce/repository/archive.tar.gz\?ref\=v8.9.3 
| grep file
Content-Disposition: attachment; 
filename="gitlab-ce-v8.9.3-5e546d9b4728fc9c9623992a678cbea9eb2098f1.tar.gz"|


So even if it seems that archive name is `|archive.tar.gz` it should be 
downloaded to 
`||gitlab-ce-v8.9.3-5e546d9b4728fc9c9623992a678cbea9eb2098f1.tar.gz`. So 
I think it's easily can be file with another extension.|


Ruslo
-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Re: [cmake-developers] [patch] One more pattern for extracting file name from URL

2016-07-05 Thread Ben Boeckel 
On Tue, Jul 05, 2016 at 18:43:25 +0300, Ruslan Baratov wrote:
> By default not, goes to -prefix/src/archive.tar .

Right, but if a download directory is set, this won't work. Can the
project name at least be added to the name?

Though I'm partial to keeping the extension still; if I need to add a
patch to a project, it'd be nice if the extension weren't a lie when I
go to extract the tarball.

--Ben
-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Re: [cmake-developers] [patch] One more pattern for extracting file name from URL

2016-07-05 Thread Ben Boeckel 
On Tue, Jul 05, 2016 at 16:26:54 +0300, Ruslan Baratov via cmake-developers 
wrote:
> +  # Do not put warning message here. If everything works OK it will
> +  # only distract. If unpack will fail the standard name can be 
> found in logs.
> +  # (see _ep_write_extractfile_script function)
> +  set(fname "archive.tar")

Is this going to cause multiple tarball downloads to overwrite each
other in the download cache directory?

--Ben
-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Re: [cmake-developers] [patch] One more pattern for extracting file name from URL

2016-07-05 Thread Ruslan Baratov via cmake-developers 
Got another idea. We can use default name like 'archive.tar'. Seems that 
internal type of archive determined automatically and doesn't depends on 
the filename. I've tried one example for each extension: 7z, tar.bz2, 
tar.gz, tar.xz, tbz2, tgz, txz, zip and it just works. Patch attached.


Ruslo

On 01-Jul-16 15:01, Ruslan Baratov via cmake-developers wrote:

Hi,

With attached patch it's possible to extract name from URLs like 
'https://.../archive.tar.gz?a=x=y'.


Ruslo 



>From 5284eedbd1b038aab2eab73252eb41914afb14e1 Mon Sep 17 00:00:00 2001
From: Ruslan Baratov 
Date: Tue, 5 Jul 2016 16:17:12 +0300
Subject: [PATCH] Use default filename if extracting from URL failed

---
 Modules/ExternalProject.cmake | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Modules/ExternalProject.cmake b/Modules/ExternalProject.cmake
index 2ff18fc..4c35c8b 100644
--- a/Modules/ExternalProject.cmake
+++ b/Modules/ExternalProject.cmake
@@ -1880,7 +1880,10 @@ function(_ep_add_download_command name)
 if (no_extract)
   get_filename_component(fname "${url}" NAME)
 elseif(NOT "${fname}" MATCHES "(\\.|=)(7z|tar|tar\\.bz2|tar\\.gz|tar\\.xz|tbz2|tgz|txz|zip)$")
-  message(FATAL_ERROR "Could not extract tarball filename from url:\n  ${url}")
+  # Do not put warning message here. If everything works OK it will
+  # only distract. If unpack will fail the standard name can be found in logs.
+  # (see _ep_write_extractfile_script function)
+  set(fname "archive.tar")
 endif()
 string(REPLACE ";" "-" fname "${fname}")
 set(file ${download_dir}/${fname})
-- 
1.9.1

-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Re: [cmake-developers] [patch] One more pattern for extracting file name from URL

2016-07-01 Thread Brad King 
On 07/01/2016 11:48 AM, Ruslan Baratov wrote:
> On 01-Jul-16 15:58, Brad King wrote:
>> Please try to structure the logic right in the if/elseif part.
>> Matching in those also sets `CMAKE_MATCH_*` variables so one
>> does not need to double-match.
> 'elseif' part try to find 'archive.tar.gz' in '${fname}', then 'string(REGEX' 
> try to match it in '${url}'. There is no reusing of 'CMAKE_MATCH_*'.

I don't think we should duplicate the

  "(\\.|=)(7z|tar|tar\\.bz2|tar\\.gz|tar\\.xz|tbz2|tgz|txz|zip)$"

expression.  The stripping of ?.* can be done earlier, or done as
part of the main match.

>> Also, the `([^/]*)\\?.*` part of the regex should be more
>> like `([^/?]*)\\?.*` to avoid eagerly matching early `?`.
> We can't have question mark ('?') in path as far as I understand, it should 
> be percent-encoded, will be |'%3F'.|||
> https://en.wikipedia.org/wiki/Uniform_Resource_Locator#Syntax
> https://en.wikipedia.org/wiki/Percent-encoding

In that case it won't hurt to exclude `?` and `#` from the
allowed matching.

Thanks,
-Brad

-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Re: [cmake-developers] [patch] One more pattern for extracting file name from URL

2016-07-01 Thread Ben Boeckel 
On Fri, Jul 01, 2016 at 18:48:41 +0300, Ruslan Baratov wrote:
> On 01-Jul-16 16:04, Ben Boeckel wrote:
> > On Fri, Jul 01, 2016 at 08:58:16 -0400, Brad King wrote:
> >> Also, the `([^/]*)\\?.*` part of the regex should be more
> >> like `([^/?]*)\\?.*` to avoid eagerly matching early `?`.
> > This should also probably skip '#' characters for URLs with anchors.
> >
> > --Ben
> 'fragement' goes after 'query' so as far as I understand it's not necessary.
> https://en.wikipedia.org/wiki/Uniform_Resource_Locator#Syntax

Right, but if there isn't a query part:

foo.tar.gz#some_anchor

--Ben
-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Re: [cmake-developers] [patch] One more pattern for extracting file name from URL

2016-07-01 Thread Ruslan Baratov via cmake-developers 

On 01-Jul-16 16:04, Ben Boeckel wrote:

On Fri, Jul 01, 2016 at 08:58:16 -0400, Brad King wrote:

Also, the `([^/]*)\\?.*` part of the regex should be more
like `([^/?]*)\\?.*` to avoid eagerly matching early `?`.

This should also probably skip '#' characters for URLs with anchors.

--Ben

'fragement' goes after 'query' so as far as I understand it's not necessary.
https://en.wikipedia.org/wiki/Uniform_Resource_Locator#Syntax

Ruslo
--

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Re: [cmake-developers] [patch] One more pattern for extracting file name from URL

2016-07-01 Thread Ruslan Baratov via cmake-developers 

On 01-Jul-16 15:58, Brad King wrote:

On 07/01/2016 08:01 AM, Ruslan Baratov via cmake-developers wrote:

With attached patch it's possible to extract name from URLs like
'https://.../archive.tar.gz?a=x=y'.

Thanks.


  elseif(NOT "${fname}" MATCHES 
"(\\.|=)(7z|tar|tar\\.bz2|tar\\.gz|tar\\.xz|tbz2|tgz|txz|zip)$")
-  message(FATAL_ERROR "Could not extract tarball filename from url:\n  
${url}")
+  # Try: https://.../archive.tar.gz?a=x=y
+  string(REGEX MATCH "^.*/([^/]*)\\?.*$" match_result "${url}")
+  set(fname "${CMAKE_MATCH_1}")
+  if(NOT "${fname}" MATCHES 
"(\\.|=)(7z|tar|tar\\.bz2|tar\\.gz|tar\\.xz|tbz2|tgz|txz|zip)$")
+message(FATAL_ERROR "Could not extract tarball filename from url:\n  
${url}")
+  endif()

Please try to structure the logic right in the if/elseif part.
Matching in those also sets `CMAKE_MATCH_*` variables so one
does not need to double-match.
'elseif' part try to find 'archive.tar.gz' in '${fname}', then 
'string(REGEX' try to match it in '${url}'. There is no reusing of 
'CMAKE_MATCH_*'.




Also, the `([^/]*)\\?.*` part of the regex should be more
like `([^/?]*)\\?.*` to avoid eagerly matching early `?`.
We can't have question mark ('?') in path as far as I understand, it 
should be percent-encoded, will be |'%3F'.|||

https://en.wikipedia.org/wiki/Uniform_Resource_Locator#Syntax
https://en.wikipedia.org/wiki/Percent-encoding

Ruslo
-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Re: [cmake-developers] [patch] One more pattern for extracting file name from URL

2016-07-01 Thread Brad King 
On 07/01/2016 08:01 AM, Ruslan Baratov via cmake-developers wrote:
> With attached patch it's possible to extract name from URLs like 
> 'https://.../archive.tar.gz?a=x=y'.

Thanks.

>  elseif(NOT "${fname}" MATCHES 
> "(\\.|=)(7z|tar|tar\\.bz2|tar\\.gz|tar\\.xz|tbz2|tgz|txz|zip)$")
> -  message(FATAL_ERROR "Could not extract tarball filename from 
> url:\n  ${url}")
> +  # Try: https://.../archive.tar.gz?a=x=y
> +  string(REGEX MATCH "^.*/([^/]*)\\?.*$" match_result "${url}")
> +  set(fname "${CMAKE_MATCH_1}")
> +  if(NOT "${fname}" MATCHES 
> "(\\.|=)(7z|tar|tar\\.bz2|tar\\.gz|tar\\.xz|tbz2|tgz|txz|zip)$")
> +message(FATAL_ERROR "Could not extract tarball filename from 
> url:\n  ${url}")
> +  endif()

Please try to structure the logic right in the if/elseif part.
Matching in those also sets `CMAKE_MATCH_*` variables so one
does not need to double-match.

Also, the `([^/]*)\\?.*` part of the regex should be more
like `([^/?]*)\\?.*` to avoid eagerly matching early `?`.

Thanks,
-Brad

-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

[cmake-developers] [patch] One more pattern for extracting file name from URL

2016-07-01 Thread Ruslan Baratov via cmake-developers 

Hi,

With attached patch it's possible to extract name from URLs like 
'https://.../archive.tar.gz?a=x=y'.


Ruslo
>From 0bab30e859541dde51e9e21d3a22ec80a649a30a Mon Sep 17 00:00:00 2001
From: Ruslan Baratov 
Date: Fri, 1 Jul 2016 14:55:29 +0300
Subject: [PATCH] Add pattern for extracting file name from URL

Add one more pattern like 'https://.../archive.tar.gz?a=x=y'
to try to extract name of the archive from URL
---
 Modules/ExternalProject.cmake | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Modules/ExternalProject.cmake b/Modules/ExternalProject.cmake
index 2ff18fc..96f315f 100644
--- a/Modules/ExternalProject.cmake
+++ b/Modules/ExternalProject.cmake
@@ -1880,7 +1880,12 @@ function(_ep_add_download_command name)
 if (no_extract)
   get_filename_component(fname "${url}" NAME)
 elseif(NOT "${fname}" MATCHES "(\\.|=)(7z|tar|tar\\.bz2|tar\\.gz|tar\\.xz|tbz2|tgz|txz|zip)$")
-  message(FATAL_ERROR "Could not extract tarball filename from url:\n  ${url}")
+  # Try: https://.../archive.tar.gz?a=x=y
+  string(REGEX MATCH "^.*/([^/]*)\\?.*$" match_result "${url}")
+  set(fname "${CMAKE_MATCH_1}")
+  if(NOT "${fname}" MATCHES "(\\.|=)(7z|tar|tar\\.bz2|tar\\.gz|tar\\.xz|tbz2|tgz|txz|zip)$")
+message(FATAL_ERROR "Could not extract tarball filename from url:\n  ${url}")
+  endif()
 endif()
 string(REPLACE ";" "-" fname "${fname}")
 set(file ${download_dir}/${fname})
-- 
1.9.1

-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers