[PATCH v2 0/3] RFC: Integrating gitpkgv functionality

[PATCH v2 0/3] RFC: Integrating gitpkgv functionality

[Mike Looijmans]

Follow-up on the discussion here:
http://lists.openembedded.org/pipermail/openembedded-core/2015-January/100345.html

This is an implementation of gitpkgv in OE-core. It modifies the fetcher
to optionally provide an incremental numbering scheme for GIT repositories.

These three patches implement this, and provide the functionality in three stages.
1 __init__.py: Allows fetchers to provide alternatives to formatting a version string.
2 git.py: Implements such alternative formatting in gitpkgv style.
3 bitbake.conf: Sets GITPKGV so that it becomes backwards compatible with gitpkgv.

v2: Rebased and added bitbake.conf patch to set GITPKGV

Mike Looijmans (3):
  fetch2/__init__.py: Make get_srcrev output configurable
  fetch2/git.py: Add gitpkgv_revision alternative version information
  bitbake.conf: Set GITPKGV variable

 conf/bitbake.conf         |  1 +
 lib/bb/fetch2/__init__.py |  9 ++++++---
 lib/bb/fetch2/git.py      | 25 +++++++++++++++++++++++++
 3 files changed, 32 insertions(+), 3 deletions(-)

-- 
1.9.1

[PATCH v2 1/3] fetch2/__init__.py: Make get_srcrev output configurable

[Mike Looijmans]

From: Mike Looijmans <milo-software@users.sourceforge.net>

The idea here is to support package version numbering similar to gitpkgv in
meta-openembedded. This commit is the first step towards such functionality.

The original plan was to add a "get_pretty_srcrev" method to the fetcher, as
per Richard's suggestion [1]. While writing this, I noticed that it would
become a copy of get_srcrev with only two lines changed. So to create something
more Pythonic than a boolean argument and conditionals around the calls to the
fetcher's sortable_revision, I just made the method to be called on the fetcher
an argument to the method. Defaulting to 'sortable_revision' prevents affecting
existing code.

Now if the git fetcher were to implement, say 'gitpkgv_revision' one could
set the following in a recipe:
  PKGV="1.2+${@bb.fetch2.get_srcrev(d, 'gitpkgv_revision')}"
and this would yield the same result as gitpkgv's GITPKGV variable.

See for the discussion leading to this change:
[1] http://lists.openembedded.org/pipermail/openembedded-core/2015-January/100345.html

Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
---
 lib/bb/fetch2/__init__.py | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
index 31d9f01..958469d 100644
--- a/lib/bb/fetch2/__init__.py
+++ b/lib/bb/fetch2/__init__.py
@@ -720,7 +720,7 @@ def get_autorev(d):
         d.setVar('__BB_DONT_CACHE', '1')
     return "AUTOINC"
 
-def get_srcrev(d):
+def get_srcrev(d, method_name='sortable_revision'):
     """
     Return the revsion string, usually for use in the version string (PV) of the current package
     Most packages usually only have one SCM so we just pass on the call.
@@ -729,6 +729,9 @@ def get_srcrev(d):
 
     The idea here is that we put the string "AUTOINC+" into return value if the revisions are not 
     incremental, other code is then responsible for turning that into an increasing value (if needed)
+
+    A method_name can be supplied to retrieve an alternatively formatted revision from a fetcher, if
+    that fetcher provides a method with the given name and the same signature as sortable_revision.
     """
 
     scms = []
@@ -742,7 +745,7 @@ def get_srcrev(d):
         raise FetchError("SRCREV was used yet no valid SCM was found in SRC_URI")
 
     if len(scms) == 1 and len(urldata[scms[0]].names) == 1:
-        autoinc, rev = urldata[scms[0]].method.sortable_revision(urldata[scms[0]], d, urldata[scms[0]].names[0])
+        autoinc, rev = getattr(urldata[scms[0]].method, method_name)(urldata[scms[0]], d, urldata[scms[0]].names[0])
         if len(rev) > 10:
             rev = rev[:10]
         if autoinc:
@@ -760,7 +763,7 @@ def get_srcrev(d):
     for scm in scms:
         ud = urldata[scm]
         for name in ud.names:
-            autoinc, rev = ud.method.sortable_revision(ud, d, name)
+            autoinc, rev = getattr(ud.method, method_name)(ud, d, name)
             seenautoinc = seenautoinc or autoinc
             if len(rev) > 10:
                 rev = rev[:10]
-- 
1.9.1

[PATCH v2 2/3] fetch2/git.py: Add gitpkgv_revision alternative version information

[Mike Looijmans]

From: Mike Looijmans <milo-software@users.sourceforge.net>

gitpkgv_revision returns a sortable revision number that can be used
in the PKGV variable for example. To mimic meta-openembedded gitpkgv
behaviour to provide a sortable revision numner, one could set the
following:

PKGV = "1.0+${@bb.fetch2.get_srcrev(d, 'gitpkgv_revision')}"

This would yield a package version like "1.0+69+fb5eb80".

Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
---
 lib/bb/fetch2/git.py | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/lib/bb/fetch2/git.py b/lib/bb/fetch2/git.py
index 45bdec0..2e53882 100644
--- a/lib/bb/fetch2/git.py
+++ b/lib/bb/fetch2/git.py
@@ -398,6 +398,31 @@ class Git(FetchMethod):
     def _build_revision(self, ud, d, name):
         return ud.revisions[name]
 
+    def gitpkgv_revision(self, ud, d, name):
+        """
+        Return a sortable revision number by counting commits in the history
+        Based on gitpkgv.bblass in meta-openembedded
+        """
+        rev = self._build_revision(ud, d, name)
+        localpath = ud.localpath
+        rev_file = os.path.join(localpath, "oe-gitpkgv_" + rev)
+        if not os.path.exists(localpath):
+            commits = None
+        else:
+            if not os.path.exists(rev_file) or not os.path.getsize(rev_file):
+                from pipes import quote
+                commits = bb.fetch2.runfetchcmd(
+                        "git rev-list %s -- | wc -l" % (quote(rev)),
+                        d, quiet=True).strip().lstrip('0')
+                if commits:
+                    open(rev_file, "w").write("%d\n" % int(commits))
+            else:
+                commits = open(rev_file, "r").readline(128).strip()
+        if commits:
+            return False, "%s+%s" % (commits, rev[:7])
+        else:
+            return True, str(rev)
+
     def checkstatus(self, ud, d):
         try:
             self._lsremote(ud, d, "")
-- 
1.9.1

[PATCH v2 3/3] bitbake.conf: Set GITPKGV variable

[Mike Looijmans]

Set GITPKGV to mimic the meta-openembedded 'gitpkgv' class. This allows
a recipe to simply set:
PKGV="1.2+${GITPKGV}"
And no longer needs to inherit gitpkgv class.
---
 conf/bitbake.conf | 1 +
 1 file changed, 1 insertion(+)

diff --git a/conf/bitbake.conf b/conf/bitbake.conf
index a35219d..5dafd52 100644
--- a/conf/bitbake.conf
+++ b/conf/bitbake.conf
@@ -47,3 +47,4 @@ TARGET_ARCH = "${BUILD_ARCH}"
 TMPDIR = "${TOPDIR}/tmp"
 WORKDIR = "${TMPDIR}/work/${PF}"
 PERSISTENT_DIR = "${TMPDIR}/cache"
+GITPKGV = "${@bb.fetch2.get_srcrev(d, 'gitpkgv_revision')}"
-- 
1.9.1

Re: [PATCH v2 0/3] RFC: Integrating gitpkgv functionality

[Richard Purdie]

On Fri, 2015-05-22 at 08:29 +0200, Mike Looijmans wrote:
> Follow-up on the discussion here:
> http://lists.openembedded.org/pipermail/openembedded-core/2015-January/100345.html
> 
> This is an implementation of gitpkgv in OE-core. It modifies the fetcher
> to optionally provide an incremental numbering scheme for GIT repositories.
> 
> These three patches implement this, and provide the functionality in three stages.
> 1 __init__.py: Allows fetchers to provide alternatives to formatting a version string.
> 2 git.py: Implements such alternative formatting in gitpkgv style.
> 3 bitbake.conf: Sets GITPKGV so that it becomes backwards compatible with gitpkgv.
> 
> v2: Rebased and added bitbake.conf patch to set GITPKGV
> 
> Mike Looijmans (3):
>   fetch2/__init__.py: Make get_srcrev output configurable
>   fetch2/git.py: Add gitpkgv_revision alternative version information
>   bitbake.conf: Set GITPKGV variable

FWIW this looks quite neat to me, thanks!

Cheers,

Richard

Re: [PATCH v2 3/3] bitbake.conf: Set GITPKGV variable

[Mike Looijmans]

On 22-05-15 08:29, Mike Looijmans wrote:
> Set GITPKGV to mimic the meta-openembedded 'gitpkgv' class. This allows
> a recipe to simply set:
> PKGV="1.2+${GITPKGV}"
> And no longer needs to inherit gitpkgv class.
> ---
>   conf/bitbake.conf | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/conf/bitbake.conf b/conf/bitbake.conf
> index a35219d..5dafd52 100644
> --- a/conf/bitbake.conf
> +++ b/conf/bitbake.conf
> @@ -47,3 +47,4 @@ TARGET_ARCH = "${BUILD_ARCH}"
>   TMPDIR = "${TOPDIR}/tmp"
>   WORKDIR = "${TMPDIR}/work/${PF}"
>   PERSISTENT_DIR = "${TMPDIR}/cache"
> +GITPKGV = "${@bb.fetch2.get_srcrev(d, 'gitpkgv_revision')}"
>

This looked okay, but when put to a practical test, I noticed that this did 
not work at all. The GITPKGV variable is not set when building recipes, 
resulting in the literal string ${GITPKGV} appearing in version numbers of 
packages that try to use it.

It does work when I add this line to local.conf, but apparently bitbake.conf 
never actually gets parsed?




Kind regards,

Mike Looijmans
System Expert

TOPIC Embedded Products
Eindhovenseweg 32-C, NL-5683 KH Best
Postbus 440, NL-5680 AK Best
Telefoon: +31 (0) 499 33 69 79
Telefax: +31 (0) 499 33 69 70
E-mail: mike.looijmans@topicproducts.com
Website: www.topicproducts.com

Please consider the environment before printing this e-mail

Re: [PATCH v2 3/3] bitbake.conf: Set GITPKGV variable

[Paul Eggleton]

Hi Mike,

On Tuesday 09 June 2015 10:50:55 Mike Looijmans wrote:
> On 22-05-15 08:29, Mike Looijmans wrote:
> > Set GITPKGV to mimic the meta-openembedded 'gitpkgv' class. This allows
> > a recipe to simply set:
> > PKGV="1.2+${GITPKGV}"
> > And no longer needs to inherit gitpkgv class.
> > ---
> > 
> >   conf/bitbake.conf | 1 +
> >   1 file changed, 1 insertion(+)
> > 
> > diff --git a/conf/bitbake.conf b/conf/bitbake.conf
> > index a35219d..5dafd52 100644
> > --- a/conf/bitbake.conf
> > +++ b/conf/bitbake.conf
> > @@ -47,3 +47,4 @@ TARGET_ARCH = "${BUILD_ARCH}"
> > 
> >   TMPDIR = "${TOPDIR}/tmp"
> >   WORKDIR = "${TMPDIR}/work/${PF}"
> >   PERSISTENT_DIR = "${TMPDIR}/cache"
> > 
> > +GITPKGV = "${@bb.fetch2.get_srcrev(d, 'gitpkgv_revision')}"
> 
> This looked okay, but when put to a practical test, I noticed that this did
> not work at all. The GITPKGV variable is not set when building recipes,
> resulting in the literal string ${GITPKGV} appearing in version numbers of
> packages that try to use it.
> 
> It does work when I add this line to local.conf, but apparently bitbake.conf
> never actually gets parsed?

This is because you patched the bitbake.conf in bitbake rather than OE-Core - 
the bitbake one is really just an example and you're right, in an OE context 
it never gets used. I think Richard skipped this one as a result, so what we'd 
need is a patch against OE-Core's bitbake.conf to finish this off.

Cheers,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre

Re: [PATCH v2 3/3] bitbake.conf: Set GITPKGV variable

[Richard Purdie]

On Tue, 2015-06-09 at 10:56 +0100, Paul Eggleton wrote:
> Hi Mike,
> 
> On Tuesday 09 June 2015 10:50:55 Mike Looijmans wrote:
> > On 22-05-15 08:29, Mike Looijmans wrote:
> > > Set GITPKGV to mimic the meta-openembedded 'gitpkgv' class. This allows
> > > a recipe to simply set:
> > > PKGV="1.2+${GITPKGV}"
> > > And no longer needs to inherit gitpkgv class.
> > > ---
> > > 
> > >   conf/bitbake.conf | 1 +
> > >   1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/conf/bitbake.conf b/conf/bitbake.conf
> > > index a35219d..5dafd52 100644
> > > --- a/conf/bitbake.conf
> > > +++ b/conf/bitbake.conf
> > > @@ -47,3 +47,4 @@ TARGET_ARCH = "${BUILD_ARCH}"
> > > 
> > >   TMPDIR = "${TOPDIR}/tmp"
> > >   WORKDIR = "${TMPDIR}/work/${PF}"
> > >   PERSISTENT_DIR = "${TMPDIR}/cache"
> > > 
> > > +GITPKGV = "${@bb.fetch2.get_srcrev(d, 'gitpkgv_revision')}"
> > 
> > This looked okay, but when put to a practical test, I noticed that this did
> > not work at all. The GITPKGV variable is not set when building recipes,
> > resulting in the literal string ${GITPKGV} appearing in version numbers of
> > packages that try to use it.
> > 
> > It does work when I add this line to local.conf, but apparently bitbake.conf
> > never actually gets parsed?
> 
> This is because you patched the bitbake.conf in bitbake rather than OE-Core - 
> the bitbake one is really just an example and you're right, in an OE context 
> it never gets used. I think Richard skipped this one as a result, so what we'd 
> need is a patch against OE-Core's bitbake.conf to finish this off.

I just applied it to the one in bitbake since that was the one the patch
was written for.

To make this work in OE, we'd need one for OE-Core.

My only reluctance to put this into OE's bitbake.conf is that we can run
the risk of tracebacks/errors in the "bitbake -e" output. I think we
already get this for SRCREV.

Toaster is also generating errors on the autobuilder when it iterates
through the datastore and tries to expand variables like SRCREV, this
would add another such error there too :(.

Cheers,

Richard