bb.fetch2: SRCREV logic enhancement V2
cover bb.fetch2: SRCREV logic enhancement V2
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 <[email protected]>
---
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
--- meta/conf/bitbake.conf | 2 +- meta/lib/oe/patch.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-)
meta/conf/bitbake.conf +1 -1
@@ -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}"
meta/lib/oe/patch.py +3 -3
@@ -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:
Signature · 1 line
1.7.0.4
PATCH 2/8 bb.fetch2: add urldata_init call for Fetch class
In reply to Yu KeFetchData 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.
--- bitbake/lib/bb/fetch2/__init__.py | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
bitbake/lib/bb/fetch2/__init__.py +2 -0
@@ -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)
Signature · 1 line
1.7.0.4
PATCH 3/8 bb.fetch2.git.py: add git urldata_init
In reply to Yu Kemove the git specific urldata init from localpath to urldata_init so that it can be called early
--- bitbake/lib/bb/fetch2/git.py | 13 +++++++++---- 1 files changed, 9 insertions(+), 4 deletions(-)
bitbake/lib/bb/fetch2/git.py +9 -4
@@ -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
Signature · 1 line
1.7.0.4
PATCH 4/8 bb.fetch2.svn.py: add urldata_init
In reply to Yu Kemove the svn specific urldata init from localpath to urldata_init so that it can be called early
--- bitbake/lib/bb/fetch2/svn.py | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-)
bitbake/lib/bb/fetch2/svn.py +5 -1
@@ -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']
Signature · 1 line
1.7.0.4
PATCH 5/8 bb.fetch2.hg: add hg urldata_init
In reply to Yu Kemove the hg specific urldata init from localpath to urldata_init so that it can be called early
--- bitbake/lib/bb/fetch2/hg.py | 14 +++++++++----- 1 files changed, 9 insertions(+), 5 deletions(-)
bitbake/lib/bb/fetch2/hg.py +9 -5
@@ -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:
Signature · 1 line
1.7.0.4
PATCH 6/8 bb.fetch2.bzr: add bzr urldata_init
In reply to Yu Kemove the bzr specific urldata init from localpath to urldata_init so that it can be called early
--- bitbake/lib/bb/fetch2/bzr.py | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-)
bitbake/lib/bb/fetch2/bzr.py +5 -2
@@ -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)
Signature · 1 line
1.7.0.4
PATCH 7/8 Fetcher: break the "SRCREVINACTION" deadlock
In reply to Yu KeCurrent fetcher has annoying "SRCREVINACTION" deadlock, which occurs when SRCREV=${AUTOREV}[email protected]_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.
--- 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(-)
bitbake/lib/bb/fetch/__init__.py +3 -0
@@ -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
bitbake/lib/bb/fetch2/__init__.py +10 -24
@@ -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)
bitbake/lib/bb/fetch2/bzr.py +0 -6
@@ -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)
bitbake/lib/bb/fetch2/git.py +1 -7
@@ -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)
bitbake/lib/bb/fetch2/hg.py +2 -8
@@ -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)
bitbake/lib/bb/fetch2/svn.py +2 -8
@@ -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)
meta/conf/bitbake.conf +1 -1
@@ -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}"
Signature · 1 line
1.7.0.4
PATCH 8/8 Fetcher: only set __BB_DONT_CACHE when SRCREV = "${AUTOREV}"
In reply to Yu Kewe 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
--- bitbake/lib/bb/fetch2/__init__.py | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
bitbake/lib/bb/fetch2/__init__.py +3 -3
@@ -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)
Signature · 1 line
1.7.0.4
Re: [PATCH 8/8] Fetcher: only set __BB_DONT_CACHE when SRCREV = "${AUTOREV}"
Quoted 24 lines · 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_CACHEat SRCREV = "${AUTOREV}" caseSigned-off-by Yu Ke <[email protected]>--- bitbake/lib/bb/fetch2/__init__.py | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)bitbake/lib/bb/fetch2/__init__.py +3
-0@@ -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):
bitbake/lib/bb/fetch2/__init__.py def get_srcrev(d): · line 389
Quoted 8 lines
bitbake/lib/bb/fetch2/__init__.py +0
-3@@ -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}"
Quoted 28 lines · Richard Purdie
Quoted 27 lines · 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_CACHEat SRCREV = "${AUTOREV}" caseSigned-off-by Yu Ke <[email protected]>--- bitbake/lib/bb/fetch2/__init__.py | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)bitbake/lib/bb/fetch2/__init__.py +3
-0@@ -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
Quoted 13 lines
bitbake/lib/bb/fetch2/__init__.py def get_srcrev(d): · line 389
Quoted 5 lines
bitbake/lib/bb/fetch2/__init__.py +0
-3- 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
Quoted 9 lines
Cheers,
Richard
_______________________________________________
poky mailing list
[email protected]
https://lists.yoctoproject.org/listinfo/poky
Hi Ke,
Quoted 29 lines · 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
- callurldata_initin 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, MIRRORPull 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-v2Thanks,
Yu Ke <[email protected]>
---Yu Ke (8):
meta: replace bb.fetch with bb.fetcher.instance bb.fetch2: addurldata_initcall for Fetch classbb.fetch2.git.py: add giturldata_initbb.fetch2.svn.py: addurldata_init
bb.fetch2.hg: add hgurldata_init
bb.fetch2.bzr: add bzrurldata_init
Fetcher: break the "SRCREVINACTION" deadlock
Fetcher: only set__BB_DONT_CACHEwhen SRCREV = "${AUTOREV}"
Quoted 9 lines
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