[PATCH 0/4] fetch2: fixes and optimizations

[PATCH 0/4] fetch2: fixes and optimizations

[Jason Wessel]

* The fetcher logging is broken in a way where it doesnt return errors.
* There is a regression in the URI handling
* Local git mirrors do not work correctly as PRE-MIRRORS
* The fall back fetch can be optimized to use clone -s
  if it is a local file url with a PRE-MIRROR

[PATCH 1/4] fetch2: Fix missing output from stderr in fetcher logs

[Jason Wessel]

There are actually two problems to fix

1) The exception for bb.process.ExecutionError must be processed first
   because it is a derived from the bb.process.CmdError class and
   we never reach the ExecutionError otherwise.

2) The stderr needs to be printed as well as stdout to determine
   the root cause of a fetch failure.

The example I have is that I got a log that looked like:

--
ERROR: Function failed: Network access disabled through
  BB_NO_NETWORK but access requested with command
   /usr/bin/env wget -t 5 -nv --passive-ftp --no-check-certificate -P
   /localds 'http://downloads.yoctoproject.org/[...CLIPPED...] url None)
--

That really didn't tell me much, but with this patch I get error above
plus the following:

--
STDERR: /net/[...CLIPPED...]kernel-tools.git: Read-only file system
--

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 lib/bb/fetch2/__init__.py |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
index 83050e4..324eef2 100644
--- a/lib/bb/fetch2/__init__.py
+++ b/lib/bb/fetch2/__init__.py
@@ -437,11 +437,10 @@ def runfetchcmd(cmd, d, quiet = False, cleanup = []):
         success = True
     except bb.process.NotFoundError as e:
         error_message = "Fetch command %s" % (e.command)
+    except bb.process.ExecutionError as e:
+        error_message = "Fetch command %s failed with exit code %s, output:\nSTDOUT: %s\nSTDERR: %s" % (e.command, e.exitcode, e.stdout, e.stderr)
     except bb.process.CmdError as e:
         error_message = "Fetch command %s could not be run:\n%s" % (e.command, e.msg)
-    except bb.process.ExecutionError as e:
-        error_message = "Fetch command %s failed with exit code %s, output:\n%s" % (e.command, e.exitcode, e.stderr)
-
     if not success:
         for f in cleanup:
             try:
-- 
1.7.10

[PATCH 2/4] fetch2: Fix URI encode / decode regression

[Jason Wessel]

The commit 420eb112a4f (fetch2: quote/unquote url paths) caused a
regression on Ubuntu 11.10's default python.

The problem is a result of the uri_replace() returning an encoded uri
and the local.py's localpath() routine not decoding the uri before
executing the local checks.

The problem was found using an updated version of the screen patches where there is a local copy of the download.  Example:

| DEBUG: For url ftp://ftp.debian.org/debian/pool/main/s/screen/screen_4.0.3-11+lenny1.diff.gz;name=patch returning file:///opt/dl/downloads/screen_4.0.3-11%2Blenny1.diff.gz;name=patch

The %2B was not getting decoded by the localpath() routine.

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 lib/bb/fetch2/local.py |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/bb/fetch2/local.py b/lib/bb/fetch2/local.py
index a0ed444..fdbd9fc 100644
--- a/lib/bb/fetch2/local.py
+++ b/lib/bb/fetch2/local.py
@@ -43,10 +43,12 @@ class Local(FetchMethod):
         ud.basename = os.path.basename(ud.url.split("://")[1].split(";")[0])
         return
 
-    def localpath(self, url, urldata, d):
+    def localpath(self, tryurl, urldata, d):
         """
         Return the local filename of a given url assuming a successful fetch.
         """
+        import urllib;
+        url = urllib.unquote(tryurl)
         path = url.split("://")[1]
         path = path.split(";")[0]
         newpath = path
-- 
1.7.10

[PATCH 3/4] fetch2: Allow local git trees as pre-mirrors

[Jason Wessel]

There is a strong desire to store the raw git trees in a pre-mirror
such that they can easily get updated externally as well as to contain
local branches.

This patch adds another type of pre-mirror rule that is "git" based.

The syntax looks like:
  PREMIRRORS_append := "\
     git://.*/.* git://${LAYERDIR}/downloads/git/ \n"

The syntax for typical tar.gz compressed git tree is:
  PREMIRRORS_append := "\
     git://.*/.* file://${LAYERDIR}/downloads/git/ \n"

The new pre-mirror type also requires the creation of the GITDIR at
the pre-mirror check time or the git clone and git checks will fail
with obscure errors.

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 lib/bb/fetch2/__init__.py |    8 +++++++-
 lib/bb/fetch2/git.py      |    4 ++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
index 324eef2..68025a3 100644
--- a/lib/bb/fetch2/__init__.py
+++ b/lib/bb/fetch2/__init__.py
@@ -202,7 +202,13 @@ def uri_replace(ud, uri_find, uri_replace, d):
                 if uri_find_decoded.index(i) == 2:
                     basename = None
                     if ud.mirrortarball:
-                        basename = os.path.basename(ud.mirrortarball)
+                        # Transpose a git uri to a file uri and check if there
+                        # is a local mirror durring the premirror checks
+                        if ud.mirror and uri_find_decoded[0] == "git":
+                            result_decoded[0] = "file"
+                            basename = os.path.basename(ud.mirror)
+                        else:
+                            basename = os.path.basename(ud.mirrortarball)
                     elif ud.localpath:
                         basename = os.path.basename(ud.localpath)
                     if basename and result_decoded[loc].endswith("/"):
diff --git a/lib/bb/fetch2/git.py b/lib/bb/fetch2/git.py
index ecc5e0d..1ad9213 100644
--- a/lib/bb/fetch2/git.py
+++ b/lib/bb/fetch2/git.py
@@ -138,9 +138,11 @@ class Git(FetchMethod):
         if ud.rebaseable:
             for name in ud.names:
                 gitsrcname = gitsrcname + '_' + ud.revisions[name]
+        ud.mirror = gitsrcname
         ud.mirrortarball = 'git2_%s.tar.gz' % (gitsrcname)
         ud.fullmirror = os.path.join(d.getVar("DL_DIR", True), ud.mirrortarball)
         gitdir = d.getVar("GITDIR", True) or (d.getVar("DL_DIR", True) + "/git2/")
+        ud.gitdir = gitdir
         ud.clonedir = os.path.join(gitdir, gitsrcname)
 
         ud.localfile = ud.clonedir
@@ -166,6 +168,8 @@ class Git(FetchMethod):
             return True
         if os.path.exists(ud.clonedir):
             return False
+        if not os.path.exists(ud.gitdir):
+            bb.utils.mkdirhier(ud.gitdir)
         return True
 
     def download(self, loc, ud, d):
-- 
1.7.10

[PATCH 4/4] fetch2/git.py: Optimize clone fall back when it is local

[Jason Wessel]

A file:// url should use "clone -s" to greatly speed
up the clone in the case of a kernel when it is local.

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 lib/bb/fetch2/git.py |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/lib/bb/fetch2/git.py b/lib/bb/fetch2/git.py
index 1ad9213..f5a3983 100644
--- a/lib/bb/fetch2/git.py
+++ b/lib/bb/fetch2/git.py
@@ -192,7 +192,13 @@ class Git(FetchMethod):
 
         # If the repo still doesn't exist, fallback to cloning it
         if not os.path.exists(ud.clonedir):
-            clone_cmd = "%s clone --bare --mirror %s %s" % (ud.basecmd, repourl, ud.clonedir)
+            if repourl.startswith("file://"):
+                use_s = "-s"
+                repo = repourl[7:]
+            else:
+                use_s = ""
+                repo = repourl
+            clone_cmd = "%s clone %s --bare --mirror %s %s" % (ud.basecmd, use_s, repo, ud.clonedir)
             if ud.proto.lower() != 'file':
                 bb.fetch2.check_network_access(d, clone_cmd)
             runfetchcmd(clone_cmd, d)
-- 
1.7.10

Re: [PATCH 4/4] fetch2/git.py: Optimize clone fall back when it is local

[Richard Purdie]

On Wed, 2012-06-13 at 22:09 -0500, Jason Wessel wrote:
> A file:// url should use "clone -s" to greatly speed
> up the clone in the case of a kernel when it is local.
> 
> Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
> ---
>  lib/bb/fetch2/git.py |    8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/bb/fetch2/git.py b/lib/bb/fetch2/git.py
> index 1ad9213..f5a3983 100644
> --- a/lib/bb/fetch2/git.py
> +++ b/lib/bb/fetch2/git.py
> @@ -192,7 +192,13 @@ class Git(FetchMethod):
>  
>          # If the repo still doesn't exist, fallback to cloning it
>          if not os.path.exists(ud.clonedir):
> -            clone_cmd = "%s clone --bare --mirror %s %s" % (ud.basecmd, repourl, ud.clonedir)
> +            if repourl.startswith("file://"):
> +                use_s = "-s"
> +                repo = repourl[7:]
> +            else:
> +                use_s = ""
> +                repo = repourl
> +            clone_cmd = "%s clone %s --bare --mirror %s %s" % (ud.basecmd, use_s, repo, ud.clonedir)
>              if ud.proto.lower() != 'file':
>                  bb.fetch2.check_network_access(d, clone_cmd)
>              runfetchcmd(clone_cmd, d)

How about always using the -l option instead?

This avoids the disadvantage of -s where you could corrupt the central
mirror store with local changes in the checkout.

Cheers,

Richard

Re: [PATCH 0/4] fetch2: fixes and optimizations

[Richard Purdie]

On Wed, 2012-06-13 at 22:09 -0500, Jason Wessel wrote:
> * The fetcher logging is broken in a way where it doesnt return errors.

I merged this.

> * There is a regression in the URI handling

I've merged an alternative patch for this that was being discussed which
fixes another part of the problem.

> * Local git mirrors do not work correctly as PRE-MIRRORS

This isn't the correct fix for this problem IMO, this needs needs a bit
more work. I'll try and take a look at this problem.

> * The fall back fetch can be optimized to use clone -s
>   if it is a local file url with a PRE-MIRROR

I've replied to this.

Cheers,

Richard

Re: [PATCH 4/4] fetch2/git.py: Optimize clone fall back when it is local

[Jason Wessel]

[-- Attachment #1: Type: text/plain, Size: 1509 bytes --]

On 06/14/2012 08:03 AM, Richard Purdie wrote:
> On Wed, 2012-06-13 at 22:09 -0500, Jason Wessel wrote:
>> A file:// url should use "clone -s" to greatly speed
>> up the clone in the case of a kernel when it is local.
>>
>> Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
>> ---
>>  lib/bb/fetch2/git.py |    8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/bb/fetch2/git.py b/lib/bb/fetch2/git.py
>> index 1ad9213..f5a3983 100644
>> --- a/lib/bb/fetch2/git.py
>> +++ b/lib/bb/fetch2/git.py
>> @@ -192,7 +192,13 @@ class Git(FetchMethod):
>>  
>>          # If the repo still doesn't exist, fallback to cloning it
>>          if not os.path.exists(ud.clonedir):
>> -            clone_cmd = "%s clone --bare --mirror %s %s" % (ud.basecmd, repourl, ud.clonedir)
>> +            if repourl.startswith("file://"):
>> +                use_s = "-s"
>> +                repo = repourl[7:]
>> +            else:
>> +                use_s = ""
>> +                repo = repourl
>> +            clone_cmd = "%s clone %s --bare --mirror %s %s" % (ud.basecmd, use_s, repo, ud.clonedir)
>>              if ud.proto.lower() != 'file':
>>                  bb.fetch2.check_network_access(d, clone_cmd)
>>              runfetchcmd(clone_cmd, d)
> 
> How about always using the -l option instead?
> 

I just had to put it through some testing first.  Yes I agree it is fine to use -l as well.

Attached is the new version of the patch.

Jason.

[-- Attachment #2: 0001-fetch2-git.py-Optimize-clone-fall-back-when-it-is-lo.patch --]
[-- Type: text/x-diff, Size: 1370 bytes --]

From daa5cc78b294406fd2376697d44c4de1a42d6f34 Mon Sep 17 00:00:00 2001
From: Jason Wessel <jason.wessel@windriver.com>
Date: Tue, 12 Jun 2012 13:23:34 -0500
Subject: [PATCH] fetch2/git.py: Optimize clone fall back when it is local

A file:// url should use "clone -l" to greatly speed
up the clone in the case of a kernel when it is local.

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 lib/bb/fetch2/git.py |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/lib/bb/fetch2/git.py b/lib/bb/fetch2/git.py
index ecc5e0d..d6a08ac 100644
--- a/lib/bb/fetch2/git.py
+++ b/lib/bb/fetch2/git.py
@@ -188,7 +188,13 @@ class Git(FetchMethod):
 
         # If the repo still doesn't exist, fallback to cloning it
         if not os.path.exists(ud.clonedir):
-            clone_cmd = "%s clone --bare --mirror %s %s" % (ud.basecmd, repourl, ud.clonedir)
+            if repourl.startswith("file://"):
+                use_s = "-l"
+                repo = repourl[7:]
+            else:
+                use_s = ""
+                repo = repourl
+            clone_cmd = "%s clone %s --bare --mirror %s %s" % (ud.basecmd, use_s, repo, ud.clonedir)
             if ud.proto.lower() != 'file':
                 bb.fetch2.check_network_access(d, clone_cmd)
             runfetchcmd(clone_cmd, d)
-- 
1.7.10

Re: [PATCH 4/4] fetch2/git.py: Optimize clone fall back when it is local

[Richard Purdie]

On Thu, 2012-06-14 at 08:33 -0500, Jason Wessel wrote:
> On 06/14/2012 08:03 AM, Richard Purdie wrote:
> > On Wed, 2012-06-13 at 22:09 -0500, Jason Wessel wrote:
> >> A file:// url should use "clone -s" to greatly speed
> >> up the clone in the case of a kernel when it is local.
> >>
> >> Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
> >> ---
> >>  lib/bb/fetch2/git.py |    8 +++++++-
> >>  1 file changed, 7 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/lib/bb/fetch2/git.py b/lib/bb/fetch2/git.py
> >> index 1ad9213..f5a3983 100644
> >> --- a/lib/bb/fetch2/git.py
> >> +++ b/lib/bb/fetch2/git.py
> >> @@ -192,7 +192,13 @@ class Git(FetchMethod):
> >>  
> >>          # If the repo still doesn't exist, fallback to cloning it
> >>          if not os.path.exists(ud.clonedir):
> >> -            clone_cmd = "%s clone --bare --mirror %s %s" % (ud.basecmd, repourl, ud.clonedir)
> >> +            if repourl.startswith("file://"):
> >> +                use_s = "-s"
> >> +                repo = repourl[7:]
> >> +            else:
> >> +                use_s = ""
> >> +                repo = repourl
> >> +            clone_cmd = "%s clone %s --bare --mirror %s %s" % (ud.basecmd, use_s, repo, ud.clonedir)
> >>              if ud.proto.lower() != 'file':
> >>                  bb.fetch2.check_network_access(d, clone_cmd)
> >>              runfetchcmd(clone_cmd, d)
> > 

I think you can then simplify this to:

@@ -192,7 +192,9 @@ class Git(FetchMethod):
  
         # If the repo still doesn't exist, fallback to cloning it
         if not os.path.exists(ud.clonedir):
-            clone_cmd = "%s clone --bare --mirror %s %s" % (ud.basecmd, repourl, ud.clonedir)
+            if repourl.startswith("file://"):
+                repo = repourl[7:]
+            clone_cmd = "%s clone -l --bare --mirror %s %s" % (ud.basecmd, repo, ud.clonedir)
             if ud.proto.lower() != 'file':
                 bb.fetch2.check_network_access(d, clone_cmd)
             runfetchcmd(clone_cmd, d)
 
Cheers,

Richard

Re: [PATCH 4/4] fetch2/git.py: Optimize clone fall back when it is local

[Jason Wessel]

On 06/14/2012 09:26 AM, Richard Purdie wrote:
> On Thu, 2012-06-14 at 08:33 -0500, Jason Wessel wrote:
>> On 06/14/2012 08:03 AM, Richard Purdie wrote:
>>> On Wed, 2012-06-13 at 22:09 -0500, Jason Wessel wrote:
>>>> A file:// url should use "clone -s" to greatly speed
>>>> up the clone in the case of a kernel when it is local.
>>>>
>>>> Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
>>>> ---
>>>>  lib/bb/fetch2/git.py |    8 +++++++-
>>>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/lib/bb/fetch2/git.py b/lib/bb/fetch2/git.py
>>>> index 1ad9213..f5a3983 100644
>>>> --- a/lib/bb/fetch2/git.py
>>>> +++ b/lib/bb/fetch2/git.py
>>>> @@ -192,7 +192,13 @@ class Git(FetchMethod):
>>>>  
>>>>          # If the repo still doesn't exist, fallback to cloning it
>>>>          if not os.path.exists(ud.clonedir):
>>>> -            clone_cmd = "%s clone --bare --mirror %s %s" % (ud.basecmd, repourl, ud.clonedir)
>>>> +            if repourl.startswith("file://"):
>>>> +                use_s = "-s"
>>>> +                repo = repourl[7:]
>>>> +            else:
>>>> +                use_s = ""
>>>> +                repo = repourl
>>>> +            clone_cmd = "%s clone %s --bare --mirror %s %s" % (ud.basecmd, use_s, repo, ud.clonedir)
>>>>              if ud.proto.lower() != 'file':
>>>>                  bb.fetch2.check_network_access(d, clone_cmd)
>>>>              runfetchcmd(clone_cmd, d)
> I think you can then simplify this to:
>
> @@ -192,7 +192,9 @@ class Git(FetchMethod):
>   
>          # If the repo still doesn't exist, fallback to cloning it
>          if not os.path.exists(ud.clonedir):
> -            clone_cmd = "%s clone --bare --mirror %s %s" % (ud.basecmd, repourl, ud.clonedir)
> +            if repourl.startswith("file://"):
> +                repo = repourl[7:]
> +            clone_cmd = "%s clone -l --bare --mirror %s %s" % (ud.basecmd, repo, ud.clonedir)
>              if ud.proto.lower() != 'file':
>                  bb.fetch2.check_network_access(d, clone_cmd)
>              runfetchcmd(clone_cmd, d)
>  
>

That is fine with me, given that it does the same thing.

Jason.

Re: [PATCH 4/4] fetch2/git.py: Optimize clone fall back when it is local

[Jason Wessel]

On 06/14/2012 09:26 AM, Richard Purdie wrote:
> On Thu, 2012-06-14 at 08:33 -0500, Jason Wessel wrote:
>> On 06/14/2012 08:03 AM, Richard Purdie wrote:
>>> On Wed, 2012-06-13 at 22:09 -0500, Jason Wessel wrote:
>>>> A file:// url should use "clone -s" to greatly speed
>>>> up the clone in the case of a kernel when it is local.
>>>>
>>>> Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
>>>> ---
>>>>  lib/bb/fetch2/git.py |    8 +++++++-
>>>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/lib/bb/fetch2/git.py b/lib/bb/fetch2/git.py
>>>> index 1ad9213..f5a3983 100644
>>>> --- a/lib/bb/fetch2/git.py
>>>> +++ b/lib/bb/fetch2/git.py
>>>> @@ -192,7 +192,13 @@ class Git(FetchMethod):
>>>>  
>>>>          # If the repo still doesn't exist, fallback to cloning it
>>>>          if not os.path.exists(ud.clonedir):
>>>> -            clone_cmd = "%s clone --bare --mirror %s %s" % (ud.basecmd, repourl, ud.clonedir)
>>>> +            if repourl.startswith("file://"):
>>>> +                use_s = "-s"
>>>> +                repo = repourl[7:]
>>>> +            else:
>>>> +                use_s = ""
>>>> +                repo = repourl
>>>> +            clone_cmd = "%s clone %s --bare --mirror %s %s" % (ud.basecmd, use_s, repo, ud.clonedir)
>>>>              if ud.proto.lower() != 'file':
>>>>                  bb.fetch2.check_network_access(d, clone_cmd)
>>>>              runfetchcmd(clone_cmd, d)
> I think you can then simplify this to:
>
> @@ -192,7 +192,9 @@ class Git(FetchMethod):
>   
>          # If the repo still doesn't exist, fallback to cloning it
>          if not os.path.exists(ud.clonedir):
> -            clone_cmd = "%s clone --bare --mirror %s %s" % (ud.basecmd, repourl, ud.clonedir)
> +            if repourl.startswith("file://"):
> +                repo = repourl[7:]
> +            clone_cmd = "%s clone -l --bare --mirror %s %s" % (ud.basecmd, repo, ud.clonedir)
>              if ud.proto.lower() != 'file':
>                  bb.fetch2.check_network_access(d, clone_cmd)
>              runfetchcmd(clone_cmd, d)

This had one defect in it which I didn't notice until I tested it  "repo" should have been "repourl", so I'll send new version along with a respin of the local git:/// -> file:/// PREMIRROR patch.

Many thanks for your comments.

Cheers,
Jason.