[PATCH v2 0/2] netcf build fail / gnulib handling fix.

[PATCH v2 0/2] netcf build fail / gnulib handling fix.

[Paul Gortmaker]

In the previous version[1] of the attempt to fix the netcf build on
newer hosts, Martin noted that my change broke fontforge.

The reason for that is the ./bootstrap in fontforge is older and
doesn't support the "--no-git" option I used for netcf.

This highlighted the fact that we will want to be able to use
different args to the ./bootstrap for different packages.

The first commit moves the configure_prepend step as-is out to
the packages, so we can do the above, and thus making
autotools-bootstrap unused in the process.

The second commit fixes the netcf prepend to not overwrite
contents from the netcf package itself, which was the source
of the build failure.

The functionality of the prepend for fontforge is left as-is
so that it will continue to build w/o issue.

Both packages build tested on a meta-overc host [which used to
fail] and an Ubuntu host [v15.10 - worked before ; still works].

Paul.

[1] http://patchwork.openembedded.org/patch/119183/

---

Paul Gortmaker (2):
  autotools-bootstrap: make bootstrap package specific.
  netcf: fix mishandling of gnulib submodule causing build fail

 meta-networking/recipes-support/netcf/netcf_git.bb | 21 ++++++++++++++++++-
 meta-oe/classes/autotools-bootstrap.bbclass        | 24 ----------------------
 .../fontforge/fontforge_20150824.bb                | 18 +++++++++++++++-
 3 files changed, 37 insertions(+), 26 deletions(-)
 delete mode 100644 meta-oe/classes/autotools-bootstrap.bbclass

-- 
2.7.4

[PATCH 1/2] autotools-bootstrap: make bootstrap package specific.

[Paul Gortmaker]

The theory behind this bbclass was reasonable, with the primary
goal being to avoid multiple downloads of gnulib, but it neglected
the fact that packages would be shipping a specific version of the
./bootstrap which will support some flags but maybe not all the
latest ones from the latest gnulib/build-aux/bootstrap file.

I attempted to simply update the two pkgs to use the latest copy
of bootstrap from gnulib but this of course triggers the descent
into autoconf hell that we all know and love.  Rather than futzing
with the packages configure.ac and deviating from what the pkg
maintainers intended and tested, we can just let the packages have
independent calls to ./bootstrap with whatever flags are needed.

The goal of this commit is to move the prepend out to the packages
and then delete the class without any real functional change ; i.e.
a purely mechanical change.  Then we can adjust each package to
ensure it will still build with a modern host, in an independent
fashion, while keeping the main advantage of not fetching gnulib
two extra times for netcf and fontforge.

Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 meta-networking/recipes-support/netcf/netcf_git.bb | 22 +++++++++++++++++++-
 meta-oe/classes/autotools-bootstrap.bbclass        | 24 ----------------------
 .../fontforge/fontforge_20150824.bb                | 18 +++++++++++++++-
 3 files changed, 38 insertions(+), 26 deletions(-)
 delete mode 100644 meta-oe/classes/autotools-bootstrap.bbclass

diff --git a/meta-networking/recipes-support/netcf/netcf_git.bb b/meta-networking/recipes-support/netcf/netcf_git.bb
index 1bc6baef0d34..f67f4ec6082e 100644
--- a/meta-networking/recipes-support/netcf/netcf_git.bb
+++ b/meta-networking/recipes-support/netcf/netcf_git.bb
@@ -16,13 +16,33 @@ DEPENDS += "augeas libnl libxslt libxml2 gnulib"
 
 S = "${WORKDIR}/git"
 
-inherit gettext autotools-bootstrap pkgconfig systemd
+inherit gettext autotools pkgconfig systemd
 
 EXTRA_OECONF_append_class-target = " --with-driver=redhat"
 
 PACKAGECONFIG ??= "${@bb.utils.contains("DISTRO_FEATURES", "systemd", "systemd", "", d)}"
 PACKAGECONFIG[systemd] = "--with-sysinit=systemd,--with-sysinit=initscripts,"
 
+do_configure_prepend() {
+    currdir=`pwd`
+    cd ${S}
+
+    # avoid bootstrap cloning gnulib on every configure
+    cat >.gitmodules <<EOF
+[submodule "gnulib"]
+       path = gnulib
+       url = git://git.sv.gnu.org/gnulib
+EOF
+    cp -rf ${STAGING_DATADIR}/gnulib ${S}
+
+    # --force to avoid errors on reconfigure e.g if recipes changed we depend on
+    # | bootstrap: running: libtoolize --quiet
+    # | libtoolize:   error: 'libltdl/COPYING.LIB' exists: use '--force' to overwrite
+    # | ...
+    ./bootstrap --force
+    cd $currdir
+}
+
 do_install_append() {
     if ${@bb.utils.contains('DISTRO_FEATURES', 'systemd', 'true', 'false', d)}; then
        install -d ${D}${systemd_unitdir}/system
diff --git a/meta-oe/classes/autotools-bootstrap.bbclass b/meta-oe/classes/autotools-bootstrap.bbclass
deleted file mode 100644
index 8d7af1b47222..000000000000
--- a/meta-oe/classes/autotools-bootstrap.bbclass
+++ /dev/null
@@ -1,24 +0,0 @@
-# Class to inherit when you want to build with autotools after running bootstrap
-inherit autotools
-
-DEPENDS += "gnulib"
-
-do_configure_prepend() {
-    currdir=`pwd`
-    cd ${S}
-
-    # avoid bootstrap cloning gnulib on every configure
-    cat >.gitmodules <<EOF
-[submodule "gnulib"]
-	path = gnulib
-	url = git://git.sv.gnu.org/gnulib
-EOF
-    cp -rf ${STAGING_DATADIR}/gnulib ${S}
-
-    # --force to avoid errors on reconfigure e.g if recipes changed we depend on
-    # | bootstrap: running: libtoolize --quiet
-    # | libtoolize:   error: 'libltdl/COPYING.LIB' exists: use '--force' to overwrite
-    # | ...
-    ./bootstrap --force
-    cd $currdir
-}
diff --git a/meta-oe/recipes-graphics/fontforge/fontforge_20150824.bb b/meta-oe/recipes-graphics/fontforge/fontforge_20150824.bb
index b50202d0a660..53899f13cdc2 100644
--- a/meta-oe/recipes-graphics/fontforge/fontforge_20150824.bb
+++ b/meta-oe/recipes-graphics/fontforge/fontforge_20150824.bb
@@ -9,7 +9,7 @@ LIC_FILES_CHKSUM = " \
 DEPENDS = "glib-2.0 pango giflib tiff libxml2 jpeg python libtool uthash"
 DEPENDS_append_class-target = " libxi"
 
-inherit autotools-bootstrap pkgconfig pythonnative distro_features_check
+inherit autotools pkgconfig pythonnative distro_features_check
 
 REQUIRED_DISTRO_FEATURES_append_class-target = " x11"
 
@@ -24,8 +24,24 @@ do_configure_prepend() {
     # uthash sources are expected in uthash/src
     currdir=`pwd`
     cd ${S}
+
     mkdir -p uthash/src
     cp ${STAGING_INCDIR}/ut*.h uthash/src
+
+    # avoid bootstrap cloning gnulib on every configure
+    cat >.gitmodules <<EOF
+[submodule "gnulib"]
+       path = gnulib
+       url = git://git.sv.gnu.org/gnulib
+EOF
+    cp -rf ${STAGING_DATADIR}/gnulib ${S}
+
+    # --force to avoid errors on reconfigure e.g if recipes changed we depend on
+    # | bootstrap: running: libtoolize --quiet
+    # | libtoolize:   error: 'libltdl/COPYING.LIB' exists: use '--force' to overwrite
+    # | ...
+    ./bootstrap --force
+
     cd $currdir
 }
 
-- 
2.7.4

[PATCH 2/2] netcf: fix mishandling of gnulib submodule causing build fail

[Paul Gortmaker]

netcf fails to build on certain hosts with newer versions
of git installed as follows:

| ./bootstrap: Bootstrapping from checked-out netcf sources...
| ./bootstrap: consider installing git-merge-changelog from gnulib
| ./bootstrap: getting gnulib files...
| error: pathspec 'gnulib' did not match any file(s) known to git.

If we do a devshell we will see that our configure prepend that
intended to _create_ the .gitmodules has instead _modified_ it
and left us with this change present:

  sh-4.3# git diff
  diff --git a/.gitmodules b/.gitmodules
  index 7acb1ea19ca7..2d10b0e0e0fe 100644
  --- a/.gitmodules
  +++ b/.gitmodules
  @@ -1,3 +1,3 @@
   [submodule "gnulib"]
  -       path = .gnulib
  -       url = git://git.sv.gnu.org/gnulib.git
  +       path = gnulib
  +       url = git://git.sv.gnu.org/gnulib
  sh-4.3#

What happens is that the newer git does not respect uncommitted
changes to the .gitmodules file, and hence the path ".gnulib" is
still considered valid vs. the in tree updated path "gnulib".  It
doesn't help any that the package has its own tracked files in
gnulib/ that we stomp over, but the real fail is just uncommitted
changes to the .gitmodule as this insertion of a random path shows:

  sh-4.3# git diff
  diff --git a/.gitmodules b/.gitmodules
  index 7acb1ea19ca7..91bd45f8e4d4 100644
  --- a/.gitmodules
  +++ b/.gitmodules
  @@ -1,3 +1,3 @@
   [submodule "gnulib"]
  -       path = .gnulib
  +       path = gnulibaaa
          url = git://git.sv.gnu.org/gnulib.git
  sh-4.3# git --version
  git version 2.7.4
  sh-4.3# git submodule init
  fatal: no submodule mapping found in .gitmodules for path '.gnulib'
  sh-4.3#

Since the original bbclass simply assumed there was no .gitmodules
file to begin with, we can easily solve this by not clobbering it
and respect the path choice used by the package itself.

As the version of ./bootstrap shipped with netcf supports this:

     --no-git       do not use git to update gnulib.  Requires that
                    --gnulib-srcdir point to a correct gnulib snapshot

we can use it in conjunction with the pathspec since we know the
gnulib was just copied in from the sysroot, and does not need
to try and pull any further updates.

Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 meta-networking/recipes-support/netcf/netcf_git.bb | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/meta-networking/recipes-support/netcf/netcf_git.bb b/meta-networking/recipes-support/netcf/netcf_git.bb
index f67f4ec6082e..8d6d32a60895 100644
--- a/meta-networking/recipes-support/netcf/netcf_git.bb
+++ b/meta-networking/recipes-support/netcf/netcf_git.bb
@@ -28,18 +28,17 @@ do_configure_prepend() {
     cd ${S}
 
     # avoid bootstrap cloning gnulib on every configure
-    cat >.gitmodules <<EOF
-[submodule "gnulib"]
-       path = gnulib
-       url = git://git.sv.gnu.org/gnulib
-EOF
-    cp -rf ${STAGING_DATADIR}/gnulib ${S}
+    # the rmdir acts as a sentinel to let us know if the pkg ever changes
+    # the path for GNUlib or populates the dir making it non-empty.
+    rmdir ${S}/.gnulib
+    cp -rf ${STAGING_DATADIR}/gnulib ${S}/.gnulib
 
     # --force to avoid errors on reconfigure e.g if recipes changed we depend on
     # | bootstrap: running: libtoolize --quiet
     # | libtoolize:   error: 'libltdl/COPYING.LIB' exists: use '--force' to overwrite
     # | ...
-    ./bootstrap --force
+    ./bootstrap --force --no-git --gnulib-srcdir=.gnulib
+
     cd $currdir
 }
 
-- 
2.7.4

Re: [PATCH v2 0/2] netcf build fail / gnulib handling fix.

[Joe MacDonald]

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

[[PATCH v2 0/2] netcf build fail / gnulib handling fix.] On 16.05.08 (Sun 15:08) Paul Gortmaker wrote:

> In the previous version[1] of the attempt to fix the netcf build on
> newer hosts, Martin noted that my change broke fontforge.
> 
> The reason for that is the ./bootstrap in fontforge is older and
> doesn't support the "--no-git" option I used for netcf.
> 
> This highlighted the fact that we will want to be able to use
> different args to the ./bootstrap for different packages.
> 
> The first commit moves the configure_prepend step as-is out to
> the packages, so we can do the above, and thus making
> autotools-bootstrap unused in the process.
> 
> The second commit fixes the netcf prepend to not overwrite
> contents from the netcf package itself, which was the source
> of the build failure.
> 
> The functionality of the prepend for fontforge is left as-is
> so that it will continue to build w/o issue.

I've no objection to this change, it seems sensible and low-impact to
me.  That said, if there's any objection to removing the
autotools-bootstrap class, I'd also support a version of this that
leaves the class but divorces netcf from it.

I wonder, though, now that I'm thinking about it, if the crux of the
change is going from:

 21     # | ...
 22     ./bootstrap --force
 23     cd $currdir
 24 }

to something like:

 21     # | ...
 22     ./bootstrap --force --no-git --gnulib-srcdir=.gnulib
 23     cd $currdir
 24 }

with the extra logic around the "rmdir .gnulib" being somewhat
peripheral, would it be possible to extend the class instead with a new
varaible that would be set in the respective recipe file, using, say,
breakpad.bbclass as an example?

What I'm thinking of here is something like this:

>>> do_configure_prepend() {
>>>     currdir=`pwd`
>>>     cd ${S}
>>> 
>>>     rmdir ${S}/.gnulib || cat >.gitmodules <<EOF
>>> [submodule "gnulib"]
>>>    path = gnulib
>>>    url = git://git.sv.gnu.org/gnulib
>>> EOF
>>>     if [ -d ${S}/.gnulib ] ; then
>>>         cp -rf ${STAGING_DATADIR}/gnulib ${S}
>>>     else
>>>         cp -rf ${STAGING_DATADIR}/gnulib ${S}
>>>     fi
>>> 
>>>     # --force to avoid errors on reconfigure e.g if recipes changed we depend on
>>>     # | bootstrap: running: libtoolize --quiet
>>>     # | libtoolize:   error: 'libltdl/COPYING.LIB' exists: use '--force' to overwrite
>>>     # | ...
>>>     ./bootstrap --force ${BOOTSTRAP_EXTRA}
>>>     cd $currdir
>>> }

then in fontforge, just leave everything as-is (until it gets a review /
update / whatever) and in netcf's recipe we just add a line:

   BOOTSTRAP_EXTRA = "--no-git --gnulib-srcdir=.gnulib"

Obviously the above is all un-tested and mostly just off-the-top-of-
my-head thinking, so maybe it's completely unworkable.  If Martin is
okay with the patches as sent, I'm fine with them too.

-- 
-Joe MacDonald.
:wq

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 484 bytes --]

Re: [PATCH v2 0/2] netcf build fail / gnulib handling fix.

[Martin Jansa]

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

On Mon, May 09, 2016 at 10:08:33AM -0400, Joe MacDonald wrote:
> [[PATCH v2 0/2] netcf build fail / gnulib handling fix.] On 16.05.08 (Sun 15:08) Paul Gortmaker wrote:
> 
> > In the previous version[1] of the attempt to fix the netcf build on
> > newer hosts, Martin noted that my change broke fontforge.
> > 
> > The reason for that is the ./bootstrap in fontforge is older and
> > doesn't support the "--no-git" option I used for netcf.
> > 
> > This highlighted the fact that we will want to be able to use
> > different args to the ./bootstrap for different packages.
> > 
> > The first commit moves the configure_prepend step as-is out to
> > the packages, so we can do the above, and thus making
> > autotools-bootstrap unused in the process.
> > 
> > The second commit fixes the netcf prepend to not overwrite
> > contents from the netcf package itself, which was the source
> > of the build failure.
> > 
> > The functionality of the prepend for fontforge is left as-is
> > so that it will continue to build w/o issue.
> 
> I've no objection to this change, it seems sensible and low-impact to
> me.  That said, if there's any objection to removing the
> autotools-bootstrap class, I'd also support a version of this that
> leaves the class but divorces netcf from it.
> 
> I wonder, though, now that I'm thinking about it, if the crux of the
> change is going from:
> 
>  21     # | ...
>  22     ./bootstrap --force
>  23     cd $currdir
>  24 }
> 
> to something like:
> 
>  21     # | ...
>  22     ./bootstrap --force --no-git --gnulib-srcdir=.gnulib
>  23     cd $currdir
>  24 }
> 
> with the extra logic around the "rmdir .gnulib" being somewhat
> peripheral, would it be possible to extend the class instead with a new
> varaible that would be set in the respective recipe file, using, say,
> breakpad.bbclass as an example?
> 
> What I'm thinking of here is something like this:
> 
> >>> do_configure_prepend() {
> >>>     currdir=`pwd`
> >>>     cd ${S}
> >>> 
> >>>     rmdir ${S}/.gnulib || cat >.gitmodules <<EOF
> >>> [submodule "gnulib"]
> >>>    path = gnulib
> >>>    url = git://git.sv.gnu.org/gnulib
> >>> EOF
> >>>     if [ -d ${S}/.gnulib ] ; then
> >>>         cp -rf ${STAGING_DATADIR}/gnulib ${S}
> >>>     else
> >>>         cp -rf ${STAGING_DATADIR}/gnulib ${S}
> >>>     fi
> >>> 
> >>>     # --force to avoid errors on reconfigure e.g if recipes changed we depend on
> >>>     # | bootstrap: running: libtoolize --quiet
> >>>     # | libtoolize:   error: 'libltdl/COPYING.LIB' exists: use '--force' to overwrite
> >>>     # | ...
> >>>     ./bootstrap --force ${BOOTSTRAP_EXTRA}
> >>>     cd $currdir
> >>> }
> 
> then in fontforge, just leave everything as-is (until it gets a review /
> update / whatever) and in netcf's recipe we just add a line:
> 
>    BOOTSTRAP_EXTRA = "--no-git --gnulib-srcdir=.gnulib"
> 
> Obviously the above is all un-tested and mostly just off-the-top-of-
> my-head thinking, so maybe it's completely unworkable.  If Martin is
> okay with the patches as sent, I'm fine with them too.

I'm fine with current patches, but was wondering the same, why not
introduce variable so that the same bbclass can be used by both "new"
and "old" bootstrap versions.

But I don't know anything about this bootstrap, so cannot say if we can
expect only these 2 variants or if every bootstrap usage is really
specific to given recipe and not worth sharing the common part in bbclass.

-- 
Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 188 bytes --]

Re: [PATCH v2 0/2] netcf build fail / gnulib handling fix.

[Paul Gortmaker]

[Re: [PATCH v2 0/2] netcf build fail / gnulib handling fix.] On 09/05/2016 (Mon 16:26) Martin Jansa wrote:

> On Mon, May 09, 2016 at 10:08:33AM -0400, Joe MacDonald wrote:
> > [[PATCH v2 0/2] netcf build fail / gnulib handling fix.] On 16.05.08 (Sun 15:08) Paul Gortmaker wrote:
> > 
> > > In the previous version[1] of the attempt to fix the netcf build on
> > > newer hosts, Martin noted that my change broke fontforge.
> > > 
> > > The reason for that is the ./bootstrap in fontforge is older and
> > > doesn't support the "--no-git" option I used for netcf.
> > > 
> > > This highlighted the fact that we will want to be able to use
> > > different args to the ./bootstrap for different packages.
> > > 
> > > The first commit moves the configure_prepend step as-is out to
> > > the packages, so we can do the above, and thus making
> > > autotools-bootstrap unused in the process.
> > > 
> > > The second commit fixes the netcf prepend to not overwrite
> > > contents from the netcf package itself, which was the source
> > > of the build failure.
> > > 
> > > The functionality of the prepend for fontforge is left as-is
> > > so that it will continue to build w/o issue.
> > 
> > I've no objection to this change, it seems sensible and low-impact to
> > me.  That said, if there's any objection to removing the
> > autotools-bootstrap class, I'd also support a version of this that
> > leaves the class but divorces netcf from it.
> > 
> > I wonder, though, now that I'm thinking about it, if the crux of the
> > change is going from:
> > 
> >  21     # | ...
> >  22     ./bootstrap --force
> >  23     cd $currdir
> >  24 }
> > 
> > to something like:
> > 
> >  21     # | ...
> >  22     ./bootstrap --force --no-git --gnulib-srcdir=.gnulib
> >  23     cd $currdir
> >  24 }
> > 
> > with the extra logic around the "rmdir .gnulib" being somewhat
> > peripheral, would it be possible to extend the class instead with a new
> > varaible that would be set in the respective recipe file, using, say,
> > breakpad.bbclass as an example?
> > 
> > What I'm thinking of here is something like this:
> > 
> > >>> do_configure_prepend() {
> > >>>     currdir=`pwd`
> > >>>     cd ${S}
> > >>> 
> > >>>     rmdir ${S}/.gnulib || cat >.gitmodules <<EOF
> > >>> [submodule "gnulib"]
> > >>>    path = gnulib
> > >>>    url = git://git.sv.gnu.org/gnulib
> > >>> EOF
> > >>>     if [ -d ${S}/.gnulib ] ; then
> > >>>         cp -rf ${STAGING_DATADIR}/gnulib ${S}
> > >>>     else
> > >>>         cp -rf ${STAGING_DATADIR}/gnulib ${S}
> > >>>     fi
> > >>> 
> > >>>     # --force to avoid errors on reconfigure e.g if recipes changed we depend on
> > >>>     # | bootstrap: running: libtoolize --quiet
> > >>>     # | libtoolize:   error: 'libltdl/COPYING.LIB' exists: use '--force' to overwrite
> > >>>     # | ...
> > >>>     ./bootstrap --force ${BOOTSTRAP_EXTRA}
> > >>>     cd $currdir
> > >>> }
> > 
> > then in fontforge, just leave everything as-is (until it gets a review /
> > update / whatever) and in netcf's recipe we just add a line:
> > 
> >    BOOTSTRAP_EXTRA = "--no-git --gnulib-srcdir=.gnulib"
> > 
> > Obviously the above is all un-tested and mostly just off-the-top-of-
> > my-head thinking, so maybe it's completely unworkable.  If Martin is
> > okay with the patches as sent, I'm fine with them too.
> 
> I'm fine with current patches, but was wondering the same, why not
> introduce variable so that the same bbclass can be used by both "new"
> and "old" bootstrap versions.

I did consider a solution along the lines of the above, but the more I
thought about it, and looked for users outside OE (e.g. Yocto) the more
I came to the conclusion that the class was largely a solution looking
for a problem.  There are just these two users of the class now.  

Anyone trying to use gnulib (which BTW is meant to help build GNU source
on non-GNU systems -so hopefully it remains uncommon) is most likely going
to have to do some custom pre-config [e.g. create submodule file or not]
so the class boils down to infrastructure for one bootstrap line to be
"shared".

> 
> But I don't know anything about this bootstrap, so cannot say if we can
> expect only these 2 variants or if every bootstrap usage is really
> specific to given recipe and not worth sharing the common part in bbclass.

Per above, I think it is safe to say we won't see hundreds of gnulib
users, and speaking only for myself, the fact it was a class to begin
with led me down the wrong path thinking that I should fix the sharing
by standardizing on a common bootstrap etc. etc. which was ugly.

Here we retain the value of not downloading gnulib multiple times, and
we give the pkgs full independence on how they handle bootstrap without
trying to confine them into what we predict they might need.

Paul.
--

> 
> -- 
> Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com