[PATCH 0/8] bb.fetch2: SRCREV logic enhancement V2

[PATCH 0/8] bb.fetch2: SRCREV logic enhancement V2

[Yu Ke]

This is the V2 patch for the SRCREV logic enhancement.

there are several changes compared with the V1:
- rebase it to bb.fetch2 code, and all code are applied to bb.fetch2 instead of bb.fetch
- call urldata_init in FetchData:__init__ according to Richard's suggestion.

this patch has been tested with
- fetch world packages with DISTRO = "poky-bleeding", empty ${DL_DIR},
  and null PREMIRROR, MIRROR

Pull URL: git://git.pokylinux.org/poky-contrib.git
  Branch: kyu3/srcrev-v2
  Browse: http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=kyu3/srcrev-v2

Thanks,
    Yu Ke <ke.yu@intel.com>
---


Yu Ke (8):
  meta: replace bb.fetch with bb.fetcher.instance
  bb.fetch2: add urldata_init call for Fetch class
  bb.fetch2.git.py: add git urldata_init
  bb.fetch2.svn.py: add urldata_init
  bb.fetch2.hg: add hg urldata_init
  bb.fetch2.bzr: add bzr urldata_init
  Fetcher: break the "SRCREVINACTION" deadlock
  Fetcher: only set __BB_DONT_CACHE when SRCREV = "${AUTOREV}"

 bitbake/lib/bb/fetch/__init__.py  |    3 ++
 bitbake/lib/bb/fetch2/__init__.py |   42 +++++++++++++-----------------------
 bitbake/lib/bb/fetch2/bzr.py      |   13 ++++-------
 bitbake/lib/bb/fetch2/git.py      |   17 +++++++--------
 bitbake/lib/bb/fetch2/hg.py       |   24 +++++++++-----------
 bitbake/lib/bb/fetch2/svn.py      |   16 ++++++--------
 meta/conf/bitbake.conf            |    4 +-
 meta/lib/oe/patch.py              |    6 ++--
 8 files changed, 54 insertions(+), 71 deletions(-)

[PATCH 1/8] meta: replace bb.fetch with bb.fetcher.instance

[Yu Ke]

Signed-off-by: Yu Ke <ke.yu@intel.com>
---
 meta/conf/bitbake.conf |    2 +-
 meta/lib/oe/patch.py   |    6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
index 30fcc79..794adc4 100644
--- a/meta/conf/bitbake.conf
+++ b/meta/conf/bitbake.conf
@@ -537,7 +537,7 @@ UPDATECOMMAND_svn = "/usr/bin/env svn update ${SVNCOOPTS}"
 SRCDATE = "${DATE}"
 SRCREV = "INVALID"
 AUTOREV = "${SRCPV}"
-SRCPV = "${@bb.fetch.get_srcrev(d)}"
+SRCPV = "${@bb.fetcher.instance.get_srcrev(d)}"
 
 SRC_URI = "file://${FILE}"
 
diff --git a/meta/lib/oe/patch.py b/meta/lib/oe/patch.py
index f203d68..86ae4e2 100644
--- a/meta/lib/oe/patch.py
+++ b/meta/lib/oe/patch.py
@@ -72,14 +72,14 @@ class PatchSet(object):
             if not patch.get("remote"):
                 raise PatchError("Patch file must be specified in patch import.")
             else:
-                patch["file"] = bb.fetch.localpath(patch["remote"], self.d)
+                patch["file"] = bb.fetcher.instance.localpath(patch["remote"], self.d)
 
         for param in PatchSet.defaults:
             if not patch.get(param):
                 patch[param] = PatchSet.defaults[param]
 
         if patch.get("remote"):
-            patch["file"] = bb.data.expand(bb.fetch.localpath(patch["remote"], self.d), self.d)
+            patch["file"] = bb.data.expand(bb.fetcher.instance.localpath(patch["remote"], self.d), self.d)
 
         patch["filemd5"] = bb.utils.md5_file(patch["file"])
 
@@ -293,7 +293,7 @@ class QuiltTree(PatchSet):
             if type == "file":
                 import shutil
                 if not patch.get("file") and patch.get("remote"):
-                    patch["file"] = bb.fetch.localpath(patch["remote"], self.d)
+                    patch["file"] = bb.fetcher.instance.localpath(patch["remote"], self.d)
 
                 shutil.copyfile(patch["quiltfile"], patch["file"])
             else:
-- 
1.7.0.4

[PATCH 2/8] bb.fetch2: add urldata_init call for Fetch class

[Yu Ke]

FetchData has some fetch method specific data, and only fetch method knows how
to initialize it. originally it is mostly initialized in Fetch.localpath().
But now there is requirement to call Fetch.latest_revision() before
Fetch.localpath(), thus require another earlier place for initialization. so
urldata_init is introduced for this purpose. it will be called in FetchData:__init__
and make all the Fetch functions useable after that.

Signed-off-by: Yu Ke <ke.yu@intel.com>
---
 bitbake/lib/bb/fetch2/__init__.py |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/bitbake/lib/bb/fetch2/__init__.py b/bitbake/lib/bb/fetch2/__init__.py
index 3a209d3..52c8d97 100644
--- a/bitbake/lib/bb/fetch2/__init__.py
+++ b/bitbake/lib/bb/fetch2/__init__.py
@@ -543,6 +543,8 @@ class FetchData(object):
         for m in methods:
             if m.supports(url, self, d):
                 self.method = m
+                if hasattr(m,"urldata_init"):
+                    m.urldata_init(self, d)
                 return
         raise NoMethodError("Missing implementation for url %s" % url)
 
-- 
1.7.0.4

[PATCH 3/8] bb.fetch2.git.py: add git urldata_init

[Yu Ke]

move the git specific urldata init from localpath to urldata_init
so that it can be called early

Signed-off-by: Yu Ke <ke.yu@intel.com>
---
 bitbake/lib/bb/fetch2/git.py |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/bitbake/lib/bb/fetch2/git.py b/bitbake/lib/bb/fetch2/git.py
index e8ad3b4..58ed1f4 100644
--- a/bitbake/lib/bb/fetch2/git.py
+++ b/bitbake/lib/bb/fetch2/git.py
@@ -41,8 +41,11 @@ class Git(Fetch):
         """
         return ud.type in ['git']
 
-    def localpath(self, url, ud, d):
-
+    def urldata_init(self, ud, d):
+        """
+        init git specific variable within url data
+        so that the git method like latest_revision() can work
+        """
         if 'protocol' in ud.parm:
             ud.proto = ud.parm['protocol']
         elif not ud.host:
@@ -56,6 +59,10 @@ class Git(Fetch):
         ud.mirrortarball = 'git_%s.tar.gz' % (gitsrcname)
         ud.clonedir = os.path.join(data.expand('${GITDIR}', d), gitsrcname)
 
+        ud.basecmd = data.getVar("FETCHCMD_git", d, True) or "git"
+
+    def localpath(self, url, ud, d):
+
         tag = Fetch.srcrev_internal_helper(ud, d)
         if tag is True:
             ud.tag = self.latest_revision(url, ud, d)
@@ -78,8 +85,6 @@ class Git(Fetch):
         else:
             ud.localfile = data.expand('git_%s%s_%s.tar.gz' % (ud.host, subdirpath.replace('/', '.'), ud.tag), d)
 
-        ud.basecmd = data.getVar("FETCHCMD_git", d, True) or "git"
-
         if 'noclone' in ud.parm:
             ud.localfile = None
             return None
-- 
1.7.0.4

[PATCH 4/8] bb.fetch2.svn.py: add urldata_init

[Yu Ke]

move the svn specific urldata init from localpath to urldata_init
so that it can be called early

Signed-off-by: Yu Ke <ke.yu@intel.com>
---
 bitbake/lib/bb/fetch2/svn.py |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/bitbake/lib/bb/fetch2/svn.py b/bitbake/lib/bb/fetch2/svn.py
index 3315b15..c0a7a54 100644
--- a/bitbake/lib/bb/fetch2/svn.py
+++ b/bitbake/lib/bb/fetch2/svn.py
@@ -42,7 +42,10 @@ class Svn(Fetch):
         """
         return ud.type in ['svn']
 
-    def localpath(self, url, ud, d):
+    def urldata_init(self, ud, d):
+        """
+        init svn specific variable within url data
+        """
         if not "module" in ud.parm:
             raise MissingParameterError("svn method needs a 'module' parameter")
 
@@ -53,6 +56,7 @@ class Svn(Fetch):
         ud.pkgdir = os.path.join(data.expand('${SVNDIR}', d), ud.host, relpath)
         ud.moddir = os.path.join(ud.pkgdir, ud.module)
 
+    def localpath(self, url, ud, d):
         if 'rev' in ud.parm:
             ud.date = ""
             ud.revision = ud.parm['rev']
-- 
1.7.0.4

[PATCH 5/8] bb.fetch2.hg: add hg urldata_init

[Yu Ke]

move the hg specific urldata init from localpath to urldata_init
so that it can be called early

Signed-off-by: Yu Ke <ke.yu@intel.com>
---
 bitbake/lib/bb/fetch2/hg.py |   14 +++++++++-----
 1 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/bitbake/lib/bb/fetch2/hg.py b/bitbake/lib/bb/fetch2/hg.py
index 9e91bec..80a1551 100644
--- a/bitbake/lib/bb/fetch2/hg.py
+++ b/bitbake/lib/bb/fetch2/hg.py
@@ -43,11 +43,10 @@ class Hg(Fetch):
         """
         return ud.type in ['hg']
 
-    def forcefetch(self, url, ud, d):
-        revTag = ud.parm.get('rev', 'tip')
-        return revTag == "tip"
-
-    def localpath(self, url, ud, d):
+    def urldata_init(self, ud, d):
+        """
+        init hg specific variable within url data
+        """
         if not "module" in ud.parm:
             raise MissingParameterError("hg method needs a 'module' parameter")
 
@@ -58,6 +57,11 @@ class Hg(Fetch):
         ud.pkgdir = os.path.join(data.expand('${HGDIR}', d), ud.host, relpath)
         ud.moddir = os.path.join(ud.pkgdir, ud.module)
 
+    def forcefetch(self, url, ud, d):
+        revTag = ud.parm.get('rev', 'tip')
+        return revTag == "tip"
+
+    def localpath(self, url, ud, d):
         if 'rev' in ud.parm:
             ud.revision = ud.parm['rev']
         else:
-- 
1.7.0.4

[PATCH 6/8] bb.fetch2.bzr: add bzr urldata_init

[Yu Ke]

move the bzr specific urldata init from localpath to urldata_init
so that it can be called early

Signed-off-by: Yu Ke <ke.yu@intel.com>
---
 bitbake/lib/bb/fetch2/bzr.py |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/bitbake/lib/bb/fetch2/bzr.py b/bitbake/lib/bb/fetch2/bzr.py
index 3d23b4d..1368f17 100644
--- a/bitbake/lib/bb/fetch2/bzr.py
+++ b/bitbake/lib/bb/fetch2/bzr.py
@@ -34,12 +34,15 @@ class Bzr(Fetch):
     def supports(self, url, ud, d):
         return ud.type in ['bzr']
 
-    def localpath (self, url, ud, d):
-
+    def urldata_init(self, ud, d):
+        """
+        init bzr specific variable within url data
+        """
         # Create paths to bzr checkouts
         relpath = self._strip_leading_slashes(ud.path)
         ud.pkgdir = os.path.join(data.expand('${BZRDIR}', d), ud.host, relpath)
 
+    def localpath (self, url, ud, d):
         revision = Fetch.srcrev_internal_helper(ud, d)
         if revision is True:
             ud.revision = self.latest_revision(url, ud, d)
-- 
1.7.0.4

[PATCH 7/8] Fetcher: break the "SRCREVINACTION" deadlock

[Yu Ke]

Current fetcher has annoying "SRCREVINACTION" deadlock,
which occurs when SRCREV=${AUTOREV}=@bb.fetch.get_srcrev():
get_srcrev()->setup_localpath()->srcrev_internal_helper()
->evaluate SRCREV->get_srcrev()

current fetcher resolve the deadlock by introducing a
"SRCREVINACTION" condition check. Althoguh it works, it is
indeed not clean.

This patch use antoehr idea to break the deadlock: break
the dependency among SRCREV and get_srcrev(), i.e. assign
a specific keyword "AUTOINC" to AUTOREV. when Fetcher meet
this keyword, it will check and set the latest revision to
urldata.revision. get_srcrev later can use the urldata.revision
for value evaluation(SRCPV etc). In this case, SRCREV no longer
depends on get_srcrev, and there is not deadlock anymore.

Signed-off-by: Yu Ke <ke.yu@intel.com>
---
 bitbake/lib/bb/fetch/__init__.py  |    3 +++
 bitbake/lib/bb/fetch2/__init__.py |   34 ++++++++++------------------------
 bitbake/lib/bb/fetch2/bzr.py      |    6 ------
 bitbake/lib/bb/fetch2/git.py      |    8 +-------
 bitbake/lib/bb/fetch2/hg.py       |   10 ++--------
 bitbake/lib/bb/fetch2/svn.py      |   10 ++--------
 meta/conf/bitbake.conf            |    2 +-
 7 files changed, 19 insertions(+), 54 deletions(-)

diff --git a/bitbake/lib/bb/fetch/__init__.py b/bitbake/lib/bb/fetch/__init__.py
index 67e5add..4bd3888 100644
--- a/bitbake/lib/bb/fetch/__init__.py
+++ b/bitbake/lib/bb/fetch/__init__.py
@@ -357,6 +357,9 @@ def localpaths(d):
 
 srcrev_internal_call = False
 
+def get_autorev(d):
+    return get_srcrev(d)
+
 def get_srcrev(d):
     """
     Return the version string for the current package
diff --git a/bitbake/lib/bb/fetch2/__init__.py b/bitbake/lib/bb/fetch2/__init__.py
index 52c8d97..44189ee 100644
--- a/bitbake/lib/bb/fetch2/__init__.py
+++ b/bitbake/lib/bb/fetch2/__init__.py
@@ -355,7 +355,8 @@ def localpaths(d):
 
     return local
 
-srcrev_internal_call = False
+def get_autorev(d):
+    return "AUTOINC"
 
 def get_srcrev(d):
     """
@@ -366,18 +367,6 @@ def get_srcrev(d):
     have been set.
     """
 
-    #
-    # Ugly code alert. localpath in the fetchers will try to evaluate SRCREV which
-    # could translate into a call to here. If it does, we need to catch this
-    # and provide some way so it knows get_srcrev is active instead of being
-    # some number etc. hence the srcrev_internal_call tracking and the magic
-    # "SRCREVINACTION" return value.
-    #
-    # Neater solutions welcome!
-    #
-    if bb.fetch2.srcrev_internal_call:
-        return "SRCREVINACTION"
-
     scms = []
 
     # Only call setup_localpath on URIs which supports_srcrev()
@@ -545,6 +534,8 @@ class FetchData(object):
                 self.method = m
                 if hasattr(m,"urldata_init"):
                     m.urldata_init(self, d)
+                if m.supports_srcrev():
+                    self.revision = Fetch.srcrev_internal_helper(self, d);
                 return
         raise NoMethodError("Missing implementation for url %s" % url)
 
@@ -569,11 +560,7 @@ class FetchData(object):
                             local = ""
                 self.localpath = local
             if not local:
-                try:
-                    bb.fetch2.srcrev_internal_call = True
-                    self.localpath = self.method.localpath(self.url, self, d)
-                finally:
-                    bb.fetch2.srcrev_internal_call = False
+                self.localpath = self.method.localpath(self.url, self, d)
                 # We have to clear data's internal caches since the cached value of SRCREV is now wrong.
                 # Horrible...
                 bb.data.delVar("ISHOULDNEVEREXIST", d)
@@ -679,8 +666,8 @@ class Fetch(object):
         """
         Return:
             a) a source revision if specified
-            b) True if auto srcrev is in action
-            c) False otherwise
+            b) latest revision if SREREV="AUTOINC"
+            c) None if not specified
         """
 
         if 'rev' in ud.parm:
@@ -701,10 +688,9 @@ class Fetch(object):
             rev = data.getVar("SRCREV", d, 1)
         if rev == "INVALID":
             raise InvalidSRCREV("Please set SRCREV to a valid value")
-        if not rev:
-            return False
-        if rev is "SRCREVINACTION":
-            return True
+        if rev == "AUTOINC":
+            rev = ud.method.latest_revision(ud.url, ud, d)
+
         return rev
 
     srcrev_internal_helper = staticmethod(srcrev_internal_helper)
diff --git a/bitbake/lib/bb/fetch2/bzr.py b/bitbake/lib/bb/fetch2/bzr.py
index 1368f17..97b042b 100644
--- a/bitbake/lib/bb/fetch2/bzr.py
+++ b/bitbake/lib/bb/fetch2/bzr.py
@@ -43,12 +43,6 @@ class Bzr(Fetch):
         ud.pkgdir = os.path.join(data.expand('${BZRDIR}', d), ud.host, relpath)
 
     def localpath (self, url, ud, d):
-        revision = Fetch.srcrev_internal_helper(ud, d)
-        if revision is True:
-            ud.revision = self.latest_revision(url, ud, d)
-        elif revision:
-            ud.revision = revision
-
         if not ud.revision:
             ud.revision = self.latest_revision(url, ud, d)
 
diff --git a/bitbake/lib/bb/fetch2/git.py b/bitbake/lib/bb/fetch2/git.py
index 58ed1f4..c621457 100644
--- a/bitbake/lib/bb/fetch2/git.py
+++ b/bitbake/lib/bb/fetch2/git.py
@@ -62,13 +62,7 @@ class Git(Fetch):
         ud.basecmd = data.getVar("FETCHCMD_git", d, True) or "git"
 
     def localpath(self, url, ud, d):
-
-        tag = Fetch.srcrev_internal_helper(ud, d)
-        if tag is True:
-            ud.tag = self.latest_revision(url, ud, d)
-        elif tag:
-            ud.tag = tag
-
+        ud.tag = ud.revision
         if not ud.tag or ud.tag == "master":
             ud.tag = self.latest_revision(url, ud, d)
 
diff --git a/bitbake/lib/bb/fetch2/hg.py b/bitbake/lib/bb/fetch2/hg.py
index 80a1551..0ba8433 100644
--- a/bitbake/lib/bb/fetch2/hg.py
+++ b/bitbake/lib/bb/fetch2/hg.py
@@ -64,14 +64,8 @@ class Hg(Fetch):
     def localpath(self, url, ud, d):
         if 'rev' in ud.parm:
             ud.revision = ud.parm['rev']
-        else:
-            tag = Fetch.srcrev_internal_helper(ud, d)
-            if tag is True:
-                ud.revision = self.latest_revision(url, ud, d)
-            elif tag:
-                ud.revision = tag
-            else:
-                ud.revision = self.latest_revision(url, ud, d)
+        elif not ud.revision:
+            ud.revision = self.latest_revision(url, ud, d)
 
         ud.localfile = data.expand('%s_%s_%s_%s.tar.gz' % (ud.module.replace('/', '.'), ud.host, ud.path.replace('/', '.'), ud.revision), d)
 
diff --git a/bitbake/lib/bb/fetch2/svn.py b/bitbake/lib/bb/fetch2/svn.py
index c0a7a54..1116795 100644
--- a/bitbake/lib/bb/fetch2/svn.py
+++ b/bitbake/lib/bb/fetch2/svn.py
@@ -73,15 +73,9 @@ class Svn(Fetch):
             if "DATE" in pv:
                 ud.revision = ""
             else:
-                rev = Fetch.srcrev_internal_helper(ud, d)
-                if rev is True:
-                    ud.revision = self.latest_revision(url, ud, d)
+                # use the initizlied revision
+                if ud.revision:
                     ud.date = ""
-                elif rev:
-                    ud.revision = rev
-                    ud.date = ""
-                else:
-                    ud.revision = ""
 
         ud.localfile = data.expand('%s_%s_%s_%s_%s.tar.gz' % (ud.module.replace('/', '.'), ud.host, ud.path.replace('/', '.'), ud.revision, ud.date), d)
 
diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
index 794adc4..e994438 100644
--- a/meta/conf/bitbake.conf
+++ b/meta/conf/bitbake.conf
@@ -536,7 +536,7 @@ UPDATECOMMAND_cvs = "/usr/bin/env cvs -d${CVSROOT} update -d -P ${CVSCOOPTS}"
 UPDATECOMMAND_svn = "/usr/bin/env svn update ${SVNCOOPTS}"
 SRCDATE = "${DATE}"
 SRCREV = "INVALID"
-AUTOREV = "${SRCPV}"
+AUTOREV = "${@bb.fetcher.instance.get_autorev(d)}"
 SRCPV = "${@bb.fetcher.instance.get_srcrev(d)}"
 
 SRC_URI = "file://${FILE}"
-- 
1.7.0.4

[PATCH 8/8] Fetcher: only set __BB_DONT_CACHE when SRCREV = "${AUTOREV}"

[Yu Ke]

we should cache SRCREV whenever possible, the only exception is
when SREREV is auto rev. so change the logic to only set __BB_DONT_CACHE
at SRCREV = "${AUTOREV}" case

Signed-off-by: Yu Ke <ke.yu@intel.com>
---
 bitbake/lib/bb/fetch2/__init__.py |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/bitbake/lib/bb/fetch2/__init__.py b/bitbake/lib/bb/fetch2/__init__.py
index 44189ee..fd489a1 100644
--- a/bitbake/lib/bb/fetch2/__init__.py
+++ b/bitbake/lib/bb/fetch2/__init__.py
@@ -356,6 +356,9 @@ def localpaths(d):
     return local
 
 def get_autorev(d):
+    #  only not cache src rev in autorev case
+    if bb.data.getVar('BB_SRCREV_POLICY', d, True) != "cache":
+        bb.data.setVar('__BB_DONT_CACHE', '1', d)
     return "AUTOINC"
 
 def get_srcrev(d):
@@ -382,9 +385,6 @@ def get_srcrev(d):
         logger.error("SRCREV was used yet no valid SCM was found in SRC_URI")
         raise ParameterError
 
-    if bb.data.getVar('BB_SRCREV_POLICY', d, True) != "cache":
-        bb.data.setVar('__BB_DONT_CACHE', '1', d)
-
     if len(scms) == 1:
         return urldata[scms[0]].method.sortable_revision(scms[0], urldata[scms[0]], d)
 
-- 
1.7.0.4

Re: [PATCH 0/8] bb.fetch2: SRCREV logic enhancement V2

[Richard Purdie]

Hi Ke,

On Sun, 2011-01-09 at 19:48 +0800, Yu Ke wrote:
> This is the V2 patch for the SRCREV logic enhancement.
> 
> there are several changes compared with the V1:
> - rebase it to bb.fetch2 code, and all code are applied to bb.fetch2 instead of bb.fetch
> - call urldata_init in FetchData:__init__ according to Richard's suggestion.
> 
> this patch has been tested with
> - fetch world packages with DISTRO = "poky-bleeding", empty ${DL_DIR},
>   and null PREMIRROR, MIRROR
> 
> Pull URL: git://git.pokylinux.org/poky-contrib.git
>   Branch: kyu3/srcrev-v2
>   Browse: http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=kyu3/srcrev-v2
> 
> Thanks,
>     Yu Ke <ke.yu@intel.com>
> ---
> 
> 
> Yu Ke (8):
>   meta: replace bb.fetch with bb.fetcher.instance
>   bb.fetch2: add urldata_init call for Fetch class
>   bb.fetch2.git.py: add git urldata_init
>   bb.fetch2.svn.py: add urldata_init
>   bb.fetch2.hg: add hg urldata_init
>   bb.fetch2.bzr: add bzr urldata_init
>   Fetcher: break the "SRCREVINACTION" deadlock
>   Fetcher: only set __BB_DONT_CACHE when SRCREV = "${AUTOREV}"
> 
>  bitbake/lib/bb/fetch/__init__.py  |    3 ++
>  bitbake/lib/bb/fetch2/__init__.py |   42 +++++++++++++-----------------------
>  bitbake/lib/bb/fetch2/bzr.py      |   13 ++++-------
>  bitbake/lib/bb/fetch2/git.py      |   17 +++++++--------
>  bitbake/lib/bb/fetch2/hg.py       |   24 +++++++++-----------
>  bitbake/lib/bb/fetch2/svn.py      |   16 ++++++--------
>  meta/conf/bitbake.conf            |    4 +-
>  meta/lib/oe/patch.py              |    6 ++--
>  8 files changed, 54 insertions(+), 71 deletions(-)

To keep this work moving, I've altered some of the initial patches a
bit, as per my comments elsewhere in the thread. I've then merged the
mid section of the patches into master.

The only patch left which I've not merged is the last one, 'Fetcher:
only set __BB_DONT_CACHE when SRCREV = "${AUTOREV}"' which I'll comment
on separately.

Cheers,

Richard

Re: [PATCH 8/8] Fetcher: only set __BB_DONT_CACHE when SRCREV = "${AUTOREV}"

[Richard Purdie]

On Sun, 2011-01-09 at 19:50 +0800, Yu Ke wrote:
> we should cache SRCREV whenever possible, the only exception is
> when SREREV is auto rev. so change the logic to only set __BB_DONT_CACHE
> at SRCREV = "${AUTOREV}" case
> 
> Signed-off-by: Yu Ke <ke.yu@intel.com>
> ---
>  bitbake/lib/bb/fetch2/__init__.py |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/bitbake/lib/bb/fetch2/__init__.py b/bitbake/lib/bb/fetch2/__init__.py
> index 44189ee..fd489a1 100644
> --- a/bitbake/lib/bb/fetch2/__init__.py
> +++ b/bitbake/lib/bb/fetch2/__init__.py
> @@ -356,6 +356,9 @@ def localpaths(d):
>      return local
>  
>  def get_autorev(d):
> +    #  only not cache src rev in autorev case
> +    if bb.data.getVar('BB_SRCREV_POLICY', d, True) != "cache":
> +        bb.data.setVar('__BB_DONT_CACHE', '1', d)
>      return "AUTOINC"
>  
>  def get_srcrev(d):
> @@ -382,9 +385,6 @@ def get_srcrev(d):
>          logger.error("SRCREV was used yet no valid SCM was found in SRC_URI")
>          raise ParameterError
>  
> -    if bb.data.getVar('BB_SRCREV_POLICY', d, True) != "cache":
> -        bb.data.setVar('__BB_DONT_CACHE', '1', d)
> -
>      if len(scms) == 1:
>          return urldata[scms[0]].method.sortable_revision(scms[0], urldata[scms[0]], d)

I wasn't going to take this as I think its rather ugly. Looking at it
further, its not really any less ugly than the existing code so I'm
torn. I will take it as its an improvement and will make the user
experience better but this approach relies on the cache to expand PV and
call get_autorev() which is not obvious and the code should at least
have a comment attached to it about how it works. Behaviour change in
the cache could break this rather easily.

Cheers,

Richard

Re: [PATCH 8/8] Fetcher: only set __BB_DONT_CACHE when SRCREV = "${AUTOREV}"

[Yu Ke]

On Jan 10, 20:37, Richard Purdie wrote:
> On Sun, 2011-01-09 at 19:50 +0800, Yu Ke wrote:
> > we should cache SRCREV whenever possible, the only exception is
> > when SREREV is auto rev. so change the logic to only set __BB_DONT_CACHE
> > at SRCREV = "${AUTOREV}" case
> > 
> > Signed-off-by: Yu Ke <ke.yu@intel.com>
> > ---
> >  bitbake/lib/bb/fetch2/__init__.py |    6 +++---
> >  1 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/bitbake/lib/bb/fetch2/__init__.py b/bitbake/lib/bb/fetch2/__init__.py
> > index 44189ee..fd489a1 100644
> > --- a/bitbake/lib/bb/fetch2/__init__.py
> > +++ b/bitbake/lib/bb/fetch2/__init__.py
> > @@ -356,6 +356,9 @@ def localpaths(d):
> >      return local
> >  
> >  def get_autorev(d):
> > +    #  only not cache src rev in autorev case
> > +    if bb.data.getVar('BB_SRCREV_POLICY', d, True) != "cache":
> > +        bb.data.setVar('__BB_DONT_CACHE', '1', d)
> >      return "AUTOINC"
> >  
> >  def get_srcrev(d):
> > @@ -382,9 +385,6 @@ def get_srcrev(d):
> >          logger.error("SRCREV was used yet no valid SCM was found in SRC_URI")
> >          raise ParameterError
> >  
> > -    if bb.data.getVar('BB_SRCREV_POLICY', d, True) != "cache":
> > -        bb.data.setVar('__BB_DONT_CACHE', '1', d)
> > -
> >      if len(scms) == 1:
> >          return urldata[scms[0]].method.sortable_revision(scms[0], urldata[scms[0]], d)
> 
> I wasn't going to take this as I think its rather ugly. Looking at it
> further, its not really any less ugly than the existing code so I'm
> torn. I will take it as its an improvement and will make the user
> experience better but this approach relies on the cache to expand PV and
> call get_autorev() which is not obvious and the code should at least
> have a comment attached to it about how it works. Behaviour change in
> the cache could break this rather easily.

oh, yes. it is indeed not clean and fragile. I just blindly make the patch based on the orignal code and not think more.Idealy, if Cache.py has a hook that other component (e.g. fetcher) could register callback, and the callback will be executed in determed order, e.g. before Cache.add(), that will solve this problem. I will see if there is such place or any alternative.

Regards
Ke



> 
> Cheers,
> 
> Richard
> 
> _______________________________________________
> poky mailing list
> poky@yoctoproject.org
> https://lists.yoctoproject.org/listinfo/poky