Patchwork [bitbake-devel,20/20] fetch: allow regexps in mirror protocol

login
register
mail settings
Submitter Enrico Scholz
Date June 27, 2012, 2:52 p.m.
Message ID <1340808774-24884-1-git-send-email-enrico.scholz@sigma-chemnitz.de>
Download mbox | patch
Permalink /patch/30711/
State New
Headers show

Comments

Enrico Scholz - June 27, 2012, 2:52 p.m.
Last mirror rewrite caused a regression not accepting

   .*://.*/.* file://${DL_DIR}/../local/

like specifications anymore. Patch restores old behavior by using regexp
matching when checking protocol.

Signed-off-by: Enrico Scholz <enrico.scholz@sigma-chemnitz.de>
---
 lib/bb/fetch2/__init__.py |    2 +-
 1 Datei geändert, 1 Zeile hinzugefügt(+), 1 Zeile entfernt(-)
Robert P. J. Day - June 27, 2012, 2:59 p.m.
On Wed, 27 Jun 2012, Enrico Scholz wrote:

> Last mirror rewrite caused a regression not accepting
>
>    .*://.*/.* file://${DL_DIR}/../local/
>
> like specifications anymore. Patch restores old behavior by using regexp
> matching when checking protocol.
>
> Signed-off-by: Enrico Scholz <enrico.scholz@sigma-chemnitz.de>
> ---
>  lib/bb/fetch2/__init__.py |    2 +-
>  1 Datei geändert, 1 Zeile hinzugefügt(+), 1 Zeile entfernt(-)
>
> diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
> index 6f3d88c..75ce01b 100644
> --- a/lib/bb/fetch2/__init__.py
> +++ b/lib/bb/fetch2/__init__.py
> @@ -207,7 +207,7 @@ def uri_replace(ud, uri_find, uri_replace, d):
>          elif loc == 0:
>              # Principle of least surprise. We could end up with https matching against http and
>              # generating "files://" urls if we use the regexp engine below.
> -            if i != uri_decoded[loc]:
> +            if not re.match(i, uri_decoded[loc]):
>                  return None
>              result_decoded[loc] = uri_replace_decoded[loc]
>          elif (re.match(i, uri_decoded[loc])):
> --
> 1.7.10.2

  that certainly seems to have solved my issue with using
"own-mirrors".

rday
Richard Purdie - June 27, 2012, 8:35 p.m.
On Wed, 2012-06-27 at 16:52 +0200, Enrico Scholz wrote:
> Last mirror rewrite caused a regression not accepting
> 
>    .*://.*/.* file://${DL_DIR}/../local/
> 
> like specifications anymore. Patch restores old behavior by using regexp
> matching when checking protocol.
> 
> Signed-off-by: Enrico Scholz <enrico.scholz@sigma-chemnitz.de>
> ---
>  lib/bb/fetch2/__init__.py |    2 +-
>  1 Datei geändert, 1 Zeile hinzugefügt(+), 1 Zeile entfernt(-)
> 
> diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
> index 6f3d88c..75ce01b 100644
> --- a/lib/bb/fetch2/__init__.py
> +++ b/lib/bb/fetch2/__init__.py
> @@ -207,7 +207,7 @@ def uri_replace(ud, uri_find, uri_replace, d):
>          elif loc == 0:
>              # Principle of least surprise. We could end up with https matching against http and 
>              # generating "files://" urls if we use the regexp engine below.
> -            if i != uri_decoded[loc]:
> +            if not re.match(i, uri_decoded[loc]):
>                  return None
>              result_decoded[loc] = uri_replace_decoded[loc]
>          elif (re.match(i, uri_decoded[loc])):

I'm not picking on you (Enrico) here, there is a problem with my change
and we probably should revert my change in this area.

However the above patch fails "bitbake-selftest" and introduces a
regression. I wrote regression tests for the fetcher for a reason.

The trouble is its sees:

https://.*/.* file:///someplace/

but returns a url like files:// which the fetcher doesn't understand.
Suggestions on how to fix this are welcome.

Cheers,

Richard
Enrico Scholz - June 28, 2012, 12:18 a.m.
Richard Purdie <richard.purdie@linuxfoundation.org> writes:

>>              # Principle of least surprise. We could end up with https matching against http and 
>>              # generating "files://" urls if we use the regexp engine below.
>> -            if i != uri_decoded[loc]:
>> +            if not re.match(i, uri_decoded[loc]):
>>                  return None
>>              result_decoded[loc] = uri_replace_decoded[loc]
>>          elif (re.match(i, uri_decoded[loc])):
>
> I'm not picking on you (Enrico) here, there is a problem with my change
> and we probably should revert my change in this area.
>
> However the above patch fails "bitbake-selftest" and introduces a
> regression.

selftest fails here in

| ERROR: test_gitfetch_premirror3 (bb.tests.fetch.FetcherTest)
| CalledProcessError: Command 'git clone git://git.openembedded.org/bitbake /tmp/tmpt_dDVD/sourcemirror.git 2> /dev/null' returned non-zero exit status 128

and

| FAIL: test_urilist2 (bb.tests.fetch.FetcherTest)
| AssertionError: Lists differ: ['file:///somepath/downloads/b... != ['file:///someotherpath/downlo...
| - ['file:///somepath/downloads/bitbake-1.0.tar.gz',
| -  'file:///someotherpath/downloads/bitbake-1.0.tar.gz']
| ? ^
| 
| + ['file:///someotherpath/downloads/bitbake-1.0.tar.gz']
| ? ^

(I am some days behind current bitbake master, so these might be fixed
in the meantime)


Nothing which seems to be related to the patch.


> The trouble is its sees:
>
> https://.*/.* file:///someplace/
>
> but returns a url like files:// which the fetcher doesn't understand.
> Suggestions on how to fix this are welcome.

I do not see how my patch can cause such a rewrite. The troublesome
're.sub()' is done in a branch for loc != 0 only and won't be applied on
the protocol part.


Enrico
Enrico Scholz - June 28, 2012, 12:27 a.m.
Enrico Scholz <enrico.scholz@sigma-chemnitz.de> writes:

>>> -            if i != uri_decoded[loc]:
>>> +            if not re.match(i, uri_decoded[loc]):
>
> | FAIL: test_urilist2 (bb.tests.fetch.FetcherTest)
> | AssertionError: Lists differ: ['file:///somepath/downloads/b... != ['file:///someotherpath/downlo...
> | - ['file:///somepath/downloads/bitbake-1.0.tar.gz',
> | -  'file:///someotherpath/downloads/bitbake-1.0.tar.gz']
> | ? ^
> | 
> | + ['file:///someotherpath/downloads/bitbake-1.0.tar.gz']
> | ? ^
>
> Nothing which seems to be related to the patch.

I have to correct me... the new line should be

| +            if not re.match(i + '$', uri_decoded[loc]):

to match the complete protocol.



Enrico
Richard Purdie - June 28, 2012, 11:42 a.m.
On Thu, 2012-06-28 at 02:27 +0200, Enrico Scholz wrote:
> Enrico Scholz <enrico.scholz@sigma-chemnitz.de> writes:
> 
> >>> -            if i != uri_decoded[loc]:
> >>> +            if not re.match(i, uri_decoded[loc]):
> >
> > | FAIL: test_urilist2 (bb.tests.fetch.FetcherTest)
> > | AssertionError: Lists differ: ['file:///somepath/downloads/b... != ['file:///someotherpath/downlo...
> > | - ['file:///somepath/downloads/bitbake-1.0.tar.gz',
> > | -  'file:///someotherpath/downloads/bitbake-1.0.tar.gz']
> > | ? ^
> > | 
> > | + ['file:///someotherpath/downloads/bitbake-1.0.tar.gz']
> > | ? ^
> >
> > Nothing which seems to be related to the patch.
> 
> I have to correct me... the new line should be
> 
> | +            if not re.match(i + '$', uri_decoded[loc]):
> 
> to match the complete protocol.

Agreed, this is going to make most sense. I've merged a patch which
reverts my original change and adds something like this (but checks the
pattern doesn't end with this already).

Hopefully this resolves the issues everyone was having.

Cheers,

Richard

Patch

diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
index 6f3d88c..75ce01b 100644
--- a/lib/bb/fetch2/__init__.py
+++ b/lib/bb/fetch2/__init__.py
@@ -207,7 +207,7 @@  def uri_replace(ud, uri_find, uri_replace, d):
         elif loc == 0:
             # Principle of least surprise. We could end up with https matching against http and 
             # generating "files://" urls if we use the regexp engine below.
-            if i != uri_decoded[loc]:
+            if not re.match(i, uri_decoded[loc]):
                 return None
             result_decoded[loc] = uri_replace_decoded[loc]
         elif (re.match(i, uri_decoded[loc])):