[PATCH 0/5] various fixes

[PATCH 0/5] various fixes

[Mikko Rapeli]

Hi,

Here are some patches we have to use on sumo. Developed and
tested heavily on sumo and only compile tested with core-image-minimal
on top of master branch.

Michael Ho (3):
  cmake.bbclass: append includedir to implicit include dirs
  sstate: add support for caching shared workdir tasks
  insane.bbclass: add package specific skips to sstate hash

Mikko Rapeli (1):
  RFC image_types.bbclass: add image size limit for tar image type

Ulf Magnusson (1):
  bitbake: fetch2/svn: Fix SVN repository concurrent update race

 bitbake/lib/bb/fetch2/svn.py     | 64 ++++++++++++++++++++++------------------
 meta/classes/cmake.bbclass       |  4 +++
 meta/classes/image.bbclass       |  2 +-
 meta/classes/image_types.bbclass | 11 ++++++-
 meta/classes/insane.bbclass      |  7 +++++
 meta/classes/sstate.bbclass      |  6 ++++
 6 files changed, 64 insertions(+), 30 deletions(-)

-- 
1.9.1

[PATCH 1/5] RFC image_types.bbclass: add image size limit for tar image type

[Mikko Rapeli]

If IMAGE_ROOTFS_SIZE_LIMIT is defined in image configuration, then
build will fail if for tar image type the uncompressed tar ball size
exceeds the value, as reported by 'du -ks'.

This check could be extended to other image types as well.

There already exists a check with IMAGE_ROOTFS_SIZE variable
but I could not figure out how to actually use it. It does
some quite complex overhead calculations which did not seem
to work for me on sumo.

When the image size is exceeded, build fails like this:

ERROR: image-1.0-r0 do_image_tar: Image size 170712 of /home/builder/src/base/build/tmp/work/linux/image/1.0-r0/deploy-image-complete/image.rootfs.tar reported by 'du -ks' is larger than limit IMAGE_ROOTFS_SIZE_LIMIT 170000

Signed-off-by: Mikko Rapeli <mikko.rapeli@bmw.de>
---
 meta/classes/image.bbclass       |  2 +-
 meta/classes/image_types.bbclass | 11 ++++++++++-
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
index 276d0d3..34a567f 100644
--- a/meta/classes/image.bbclass
+++ b/meta/classes/image.bbclass
@@ -127,7 +127,7 @@ def rootfs_variables(d):
                  'IMAGE_ROOTFS_MAXSIZE','IMAGE_NAME','IMAGE_LINK_NAME','IMAGE_MANIFEST','DEPLOY_DIR_IMAGE','IMAGE_FSTYPES','IMAGE_INSTALL_COMPLEMENTARY','IMAGE_LINGUAS',
                  'MULTILIBRE_ALLOW_REP','MULTILIB_TEMP_ROOTFS','MULTILIB_VARIANTS','MULTILIBS','ALL_MULTILIB_PACKAGE_ARCHS','MULTILIB_GLOBAL_VARIANTS','BAD_RECOMMENDATIONS','NO_RECOMMENDATIONS',
                  'PACKAGE_ARCHS','PACKAGE_CLASSES','TARGET_VENDOR','TARGET_ARCH','TARGET_OS','OVERRIDES','BBEXTENDVARIANT','FEED_DEPLOYDIR_BASE_URI','INTERCEPT_DIR','USE_DEVFS',
-                 'CONVERSIONTYPES', 'IMAGE_GEN_DEBUGFS', 'ROOTFS_RO_UNNEEDED', 'IMGDEPLOYDIR', 'PACKAGE_EXCLUDE_COMPLEMENTARY', 'REPRODUCIBLE_TIMESTAMP_ROOTFS', 'IMAGE_INSTALL_DEBUGFS']
+                 'CONVERSIONTYPES', 'IMAGE_GEN_DEBUGFS', 'ROOTFS_RO_UNNEEDED', 'IMGDEPLOYDIR', 'PACKAGE_EXCLUDE_COMPLEMENTARY', 'REPRODUCIBLE_TIMESTAMP_ROOTFS', 'IMAGE_INSTALL_DEBUGFS', 'IMAGE_ROOTFS_SIZE_LIMIT']
     variables.extend(rootfs_command_variables(d))
     variables.extend(variable_depends(d))
     return " ".join(variables)
diff --git a/meta/classes/image_types.bbclass b/meta/classes/image_types.bbclass
index 5c40648..0481703 100644
--- a/meta/classes/image_types.bbclass
+++ b/meta/classes/image_types.bbclass
@@ -125,7 +125,16 @@ IMAGE_CMD_squashfs-lz4 = "mksquashfs ${IMAGE_ROOTFS} ${IMGDEPLOYDIR}/${IMAGE_NAM
 # required when extracting, but it seems prudent to use it in both cases.
 IMAGE_CMD_TAR ?= "tar"
 # ignore return code 1 "file changed as we read it" as other tasks(e.g. do_image_wic) may be hardlinking rootfs
-IMAGE_CMD_tar = "${IMAGE_CMD_TAR} --numeric-owner -cf ${IMGDEPLOYDIR}/${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.tar -C ${IMAGE_ROOTFS} . || [ $? -eq 1 ]"
+IMAGE_CMD_tar() {
+	set -ex
+	"${IMAGE_CMD_TAR}" --numeric-owner -cf "${IMGDEPLOYDIR}/${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.tar" -C "${IMAGE_ROOTFS}" . || [ $? -eq 1 ]
+	# fail build if IMAGE_ROOTFS_SIZE_LIMIT is exceeded
+	if [ -n "${IMAGE_ROOTFS_SIZE_LIMIT}" ]; then
+		imagesize=$( du -ks "${IMGDEPLOYDIR}/${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.tar" | awk '{ print $1 }' )
+		[ "${imagesize}" -gt "${IMAGE_ROOTFS_SIZE_LIMIT}" ] && \
+		bberror "Image size ${imagesize} of ${IMGDEPLOYDIR}/${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.tar reported by 'du -ks' is larger than limit IMAGE_ROOTFS_SIZE_LIMIT ${IMAGE_ROOTFS_SIZE_LIMIT}"
+	fi
+}
 
 do_image_cpio[cleandirs] += "${WORKDIR}/cpio_append"
 IMAGE_CMD_cpio () {
-- 
1.9.1

[PATCH 2/5] bitbake: fetch2/svn: Fix SVN repository concurrent update race

[Mikko Rapeli]

From: Ulf Magnusson <Ulf.Magnusson@bmw.de>

The ${DL_DIR}/svn directory is used by BitBake to keep checked-out SVN
repositories from which tarballs are generated. These repositories were
protected from concurrent update with a lock on the tarballs. However,
the tarballs are specific to the SRCREV and module checked out (many
tarballs can come from the same repository), meaning a repository could
be modified concurrently if two recipes checked out two different
SRCREVs or modules from it in parallel. This caused errors like the
following:

ERROR: Fetcher failure: Fetch command failed with exit code 1, output:
svn: E155004: Run 'svn cleanup' to remove locks (type 'svn help cleanup' for details)
svn: E155004: Working copy '/home/foo/downloads/svn/repo/trunk' locked.
svn: E155004: '/home/foo/downloads/svn/repo/trunk' is already locked.

Fix it by adding a per-repository lock that's independent of the module
and SRCREV.

Signed-off-by: Ulf Magnusson <Ulf.Magnusson@bmw.de>
Signed-off-by: Michael Ho <Michael.Ho@bmw.de>
---
 bitbake/lib/bb/fetch2/svn.py | 64 +++++++++++++++++++++++++-------------------
 1 file changed, 36 insertions(+), 28 deletions(-)

diff --git a/bitbake/lib/bb/fetch2/svn.py b/bitbake/lib/bb/fetch2/svn.py
index ed70bcf..9dcf3eb 100644
--- a/bitbake/lib/bb/fetch2/svn.py
+++ b/bitbake/lib/bb/fetch2/svn.py
@@ -63,6 +63,9 @@ class Svn(FetchMethod):
         relpath = self._strip_leading_slashes(ud.path)
         ud.pkgdir = os.path.join(svndir, ud.host, relpath)
         ud.moddir = os.path.join(ud.pkgdir, ud.module)
+        # Protects the repository from concurrent updates, e.g. from two
+        # recipes fetching different revisions at the same time
+        ud.svnlock = os.path.join(ud.pkgdir, "svn.lock")
 
         ud.setup_revisions(d)
 
@@ -123,35 +126,40 @@ class Svn(FetchMethod):
 
         logger.debug(2, "Fetch: checking for module directory '" + ud.moddir + "'")
 
-        if os.access(os.path.join(ud.moddir, '.svn'), os.R_OK):
-            svnupdatecmd = self._buildsvncommand(ud, d, "update")
-            logger.info("Update " + ud.url)
-            # We need to attempt to run svn upgrade first in case its an older working format
-            try:
-                runfetchcmd(ud.basecmd + " upgrade", d, workdir=ud.moddir)
-            except FetchError:
-                pass
-            logger.debug(1, "Running %s", svnupdatecmd)
-            bb.fetch2.check_network_access(d, svnupdatecmd, ud.url)
-            runfetchcmd(svnupdatecmd, d, workdir=ud.moddir)
-        else:
-            svnfetchcmd = self._buildsvncommand(ud, d, "fetch")
-            logger.info("Fetch " + ud.url)
-            # check out sources there
-            bb.utils.mkdirhier(ud.pkgdir)
-            logger.debug(1, "Running %s", svnfetchcmd)
-            bb.fetch2.check_network_access(d, svnfetchcmd, ud.url)
-            runfetchcmd(svnfetchcmd, d, workdir=ud.pkgdir)
-
-        scmdata = ud.parm.get("scmdata", "")
-        if scmdata == "keep":
-            tar_flags = ""
-        else:
-            tar_flags = "--exclude='.svn'"
+        lf = bb.utils.lockfile(ud.svnlock)
+
+        try:
+            if os.access(os.path.join(ud.moddir, '.svn'), os.R_OK):
+                svnupdatecmd = self._buildsvncommand(ud, d, "update")
+                logger.info("Update " + ud.url)
+                # We need to attempt to run svn upgrade first in case its an older working format
+                try:
+                    runfetchcmd(ud.basecmd + " upgrade", d, workdir=ud.moddir)
+                except FetchError:
+                    pass
+                logger.debug(1, "Running %s", svnupdatecmd)
+                bb.fetch2.check_network_access(d, svnupdatecmd, ud.url)
+                runfetchcmd(svnupdatecmd, d, workdir=ud.moddir)
+            else:
+                svnfetchcmd = self._buildsvncommand(ud, d, "fetch")
+                logger.info("Fetch " + ud.url)
+                # check out sources there
+                bb.utils.mkdirhier(ud.pkgdir)
+                logger.debug(1, "Running %s", svnfetchcmd)
+                bb.fetch2.check_network_access(d, svnfetchcmd, ud.url)
+                runfetchcmd(svnfetchcmd, d, workdir=ud.pkgdir)
+
+            scmdata = ud.parm.get("scmdata", "")
+            if scmdata == "keep":
+                tar_flags = ""
+            else:
+                tar_flags = "--exclude='.svn'"
 
-        # tar them up to a defined filename
-        runfetchcmd("tar %s -czf %s %s" % (tar_flags, ud.localpath, ud.path_spec), d,
-                    cleanup=[ud.localpath], workdir=ud.pkgdir)
+            # tar them up to a defined filename
+            runfetchcmd("tar %s -czf %s %s" % (tar_flags, ud.localpath, ud.path_spec), d,
+                        cleanup=[ud.localpath], workdir=ud.pkgdir)
+        finally:
+            bb.utils.unlockfile(lf)
 
     def clean(self, ud, d):
         """ Clean SVN specific files and dirs """
-- 
1.9.1

[PATCH 3/5] cmake.bbclass: append includedir to implicit include dirs

[Mikko Rapeli]

From: Michael Ho <Michael.Ho@bmw.de>

This resolves issues with paths being marked as system includes that
differ from /usr/include but are considered implicit by the toolchain.
This enables developers to add directories to system includes
to supress compiler compiler warnings from them.

Signed-off-by: Michael Ho <Michael.Ho@bmw.de>
Cc: Pascal Bach <pascal.bach@siemens.com>
---
 meta/classes/cmake.bbclass | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/meta/classes/cmake.bbclass b/meta/classes/cmake.bbclass
index fd40a98..485aea6 100644
--- a/meta/classes/cmake.bbclass
+++ b/meta/classes/cmake.bbclass
@@ -108,6 +108,10 @@ list(APPEND CMAKE_MODULE_PATH "${STAGING_DATADIR}/cmake/Modules/")
 # add for non /usr/lib libdir, e.g. /usr/lib64
 set( CMAKE_LIBRARY_PATH ${libdir} ${base_libdir})
 
+# add include dir to implicit includes in case it differs from /usr/include
+list(APPEND CMAKE_C_IMPLICIT_INCLUDE_DIRECTORIES ${includedir})
+list(APPEND CMAKE_CXX_IMPLICIT_INCLUDE_DIRECTORIES ${includedir})
+
 EOF
 }
 
-- 
1.9.1

[PATCH 4/5] sstate: add support for caching shared workdir tasks

[Mikko Rapeli]

From: Michael Ho <Michael.Ho@bmw.de>

The sstate bbclass uses workdir as a hardcoded string in path
manipulations. This means that the sstate caching mechanism does
not work for the work-shared directory which the kernel uses to
share its build configuration and source files for out of tree
kernel modules.

This commit modifies the path manipulation mechanism to use the
work-shared directory if detected in the paths when handling the
sstate cache packages.

Signed-off-by: Michael Ho <Michael.Ho@bmw.de>
---
 meta/classes/sstate.bbclass | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/meta/classes/sstate.bbclass b/meta/classes/sstate.bbclass
index 8b48ab4..c856007 100644
--- a/meta/classes/sstate.bbclass
+++ b/meta/classes/sstate.bbclass
@@ -362,7 +362,10 @@ def sstate_installpkgdir(ss, d):
 
     for plain in ss['plaindirs']:
         workdir = d.getVar('WORKDIR')
+        sharedworkdir = os.path.join(d.getVar('TMPDIR', True), "work-shared")
         src = sstateinst + "/" + plain.replace(workdir, '')
+        if sharedworkdir in plain:
+            src = sstateinst + "/" + plain.replace(sharedworkdir, '')
         dest = plain
         bb.utils.mkdirhier(src)
         prepdir(dest)
@@ -620,8 +623,11 @@ def sstate_package(ss, d):
         os.rename(state[1], sstatebuild + state[0])
 
     workdir = d.getVar('WORKDIR')
+    sharedworkdir = os.path.join(d.getVar('TMPDIR', True), "work-shared")
     for plain in ss['plaindirs']:
         pdir = plain.replace(workdir, sstatebuild)
+        if sharedworkdir in plain:
+            pdir = plain.replace(sharedworkdir, sstatebuild)
         bb.utils.mkdirhier(plain)
         bb.utils.mkdirhier(pdir)
         os.rename(plain, pdir)
-- 
1.9.1

[PATCH 5/5] insane.bbclass: add package specific skips to sstate hash

[Mikko Rapeli]

From: Michael Ho <Michael.Ho@bmw.de>

The bbclass currently adds INSANE_SKIP to the sstate hash dependencies
however the package specific skips such as INSANE_SKIP_${PN} are
not added automatically because of how the class references them.

This causes the problem that modifying INSANE_SKIP_${PN} does not
invalidate the sstate cache and can mask build breaking warnings.

Add an anonymous python snippet to explicitly include these additional
relevant skips to the sstate hash.

Singed-off-by: Michael Ho <Michael.Ho@bmw.de>
---
 meta/classes/insane.bbclass | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass
index 4644221..e015c94 100644
--- a/meta/classes/insane.bbclass
+++ b/meta/classes/insane.bbclass
@@ -1017,6 +1017,13 @@ do_package_qa[vardepsexclude] = "BB_TASKDEPDATA"
 do_package_qa[rdeptask] = "do_packagedata"
 addtask do_package_qa after do_packagedata do_package before do_build
 
+# Add the package specific INSANE_SKIPs to the sstate dependencies
+python() {
+    pkgs = (d.getVar('PACKAGES') or '').split()
+    for pkg in pkgs:
+        d.appendVarFlag("do_package_qa", "vardeps", " INSANE_SKIP_{}".format(pkg))
+}
+
 SSTATETASKS += "do_package_qa"
 do_package_qa[sstate-inputdirs] = ""
 do_package_qa[sstate-outputdirs] = ""
-- 
1.9.1

✗ patchtest: failure for various fixes

[Patchwork]

== Series Details ==

Series: various fixes
Revision: 1
URL   : https://patchwork.openembedded.org/series/15136/
State : failure

== Summary ==


Thank you for submitting this patch series to OpenEmbedded Core. This is
an automated response. Several tests have been executed on the proposed
series by patchtest resulting in the following failures:



* Issue             Series sent to the wrong mailing list or some patches from the series correspond to different mailing lists [test_target_mailing_list] 
  Suggested fix    Send the series again to the correct mailing list (ML)
  Suggested ML     bitbake-devel@lists.openembedded.org [http://git.openembedded.org/bitbake/]
  Patch's path:    bitbake/lib/bb/fetch2/svn.py

* Issue             Series does not apply on top of target branch [test_series_merge_on_head] 
  Suggested fix    Rebase your series on top of targeted branch
  Targeted branch  master (currently at 21387613fe)

* Patch            [5/5] insane.bbclass: add package specific skips to sstate hash
 Issue             Patch is missing Signed-off-by [test_signed_off_by_presence] 
  Suggested fix    Sign off the patch (either manually or with "git commit --amend -s")



If you believe any of these test results are incorrect, please reply to the
mailing list (openembedded-core@lists.openembedded.org) raising your concerns.
Otherwise we would appreciate you correcting the issues and submitting a new
version of the patchset if applicable. Please ensure you add/increment the
version number when sending the new version (i.e. [PATCH] -> [PATCH v2] ->
[PATCH v3] -> ...).

---
Guidelines:     https://www.openembedded.org/wiki/Commit_Patch_Message_Guidelines
Test framework: http://git.yoctoproject.org/cgit/cgit.cgi/patchtest
Test suite:     http://git.yoctoproject.org/cgit/cgit.cgi/patchtest-oe

Re: [PATCH 1/5] RFC image_types.bbclass: add image size limit for tar image type

[Richard Purdie]

On Thu, 2018-11-29 at 14:21 +0200, Mikko Rapeli wrote:
> If IMAGE_ROOTFS_SIZE_LIMIT is defined in image configuration, then
> build will fail if for tar image type the uncompressed tar ball size
> exceeds the value, as reported by 'du -ks'.
> 
> This check could be extended to other image types as well.
> 
> There already exists a check with IMAGE_ROOTFS_SIZE variable
> but I could not figure out how to actually use it. It does
> some quite complex overhead calculations which did not seem
> to work for me on sumo.
> 
> When the image size is exceeded, build fails like this:
> 
> ERROR: image-1.0-r0 do_image_tar: Image size 170712 of
> /home/builder/src/base/build/tmp/work/linux/image/1.0-r0/deploy-
> image-complete/image.rootfs.tar reported by 'du -ks' is larger than
> limit IMAGE_ROOTFS_SIZE_LIMIT 170000

What about IMAGE_ROOTFS_MAXSIZE?

I think IMAGE_ROOTFS_SIZE is about adding extra space to the image, not
limiting its size...

Cheers,

Richard

Re: [PATCH 1/5] RFC image_types.bbclass: add image size limit for tar image type

[Mikko.Rapeli]

On Thu, Nov 29, 2018 at 02:04:14PM +0000, Richard Purdie wrote:
> On Thu, 2018-11-29 at 14:21 +0200, Mikko Rapeli wrote:
> > If IMAGE_ROOTFS_SIZE_LIMIT is defined in image configuration, then
> > build will fail if for tar image type the uncompressed tar ball size
> > exceeds the value, as reported by 'du -ks'.
> > 
> > This check could be extended to other image types as well.
> > 
> > There already exists a check with IMAGE_ROOTFS_SIZE variable
> > but I could not figure out how to actually use it. It does
> > some quite complex overhead calculations which did not seem
> > to work for me on sumo.
> > 
> > When the image size is exceeded, build fails like this:
> > 
> > ERROR: image-1.0-r0 do_image_tar: Image size 170712 of
> > /home/builder/src/base/build/tmp/work/linux/image/1.0-r0/deploy-
> > image-complete/image.rootfs.tar reported by 'du -ks' is larger than
> > limit IMAGE_ROOTFS_SIZE_LIMIT 170000
> 
> What about IMAGE_ROOTFS_MAXSIZE?
>
> I think IMAGE_ROOTFS_SIZE is about adding extra space to the image, not
> limiting its size...

Yea, forgot to mention that I tried that too but got inconsisten results.
I know it's bad but it was easier for me to add this new test than to figure
out what's wrong with current yocto image size checks. Hence RFC.

-Mikko

> Cheers,
> 
> Richard

Re: [PATCH 1/5] RFC image_types.bbclass: add image size limit for tar image type

[Christopher Larson]

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

This seems like it’d make a good general image qa check instead, rather
than being part of do_image_tar.

On Thu, Nov 29, 2018 at 7:29 AM <Mikko.Rapeli@bmw.de> wrote:

> On Thu, Nov 29, 2018 at 02:04:14PM +0000, Richard Purdie wrote:
> > On Thu, 2018-11-29 at 14:21 +0200, Mikko Rapeli wrote:
> > > If IMAGE_ROOTFS_SIZE_LIMIT is defined in image configuration, then
> > > build will fail if for tar image type the uncompressed tar ball size
> > > exceeds the value, as reported by 'du -ks'.
> > >
> > > This check could be extended to other image types as well.
> > >
> > > There already exists a check with IMAGE_ROOTFS_SIZE variable
> > > but I could not figure out how to actually use it. It does
> > > some quite complex overhead calculations which did not seem
> > > to work for me on sumo.
> > >
> > > When the image size is exceeded, build fails like this:
> > >
> > > ERROR: image-1.0-r0 do_image_tar: Image size 170712 of
> > > /home/builder/src/base/build/tmp/work/linux/image/1.0-r0/deploy-
> > > image-complete/image.rootfs.tar reported by 'du -ks' is larger than
> > > limit IMAGE_ROOTFS_SIZE_LIMIT 170000
> >
> > What about IMAGE_ROOTFS_MAXSIZE?
> >
> > I think IMAGE_ROOTFS_SIZE is about adding extra space to the image, not
> > limiting its size...
>
> Yea, forgot to mention that I tried that too but got inconsisten results.
> I know it's bad but it was easier for me to add this new test than to
> figure
> out what's wrong with current yocto image size checks. Hence RFC.
>
> -Mikko
>
> > Cheers,
> >
> > Richard
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core
>


-- 
Christopher Larson
kergoth at gmail dot com
Founder - BitBake, OpenEmbedded, OpenZaurus
Senior Software Engineer, Mentor Graphics

[-- Attachment #2: Type: text/html, Size: 2749 bytes --]

Re: [PATCH 1/5] RFC image_types.bbclass: add image size limit for tar image type

[richard.purdie]

On Thu, 2018-11-29 at 14:17 +0000, Mikko.Rapeli@bmw.de wrote:
> On Thu, Nov 29, 2018 at 02:04:14PM +0000, Richard Purdie wrote:
> > On Thu, 2018-11-29 at 14:21 +0200, Mikko Rapeli wrote:
> > > If IMAGE_ROOTFS_SIZE_LIMIT is defined in image configuration,
> > > then
> > > build will fail if for tar image type the uncompressed tar ball
> > > size
> > > exceeds the value, as reported by 'du -ks'.
> > > 
> > > This check could be extended to other image types as well.
> > > 
> > > There already exists a check with IMAGE_ROOTFS_SIZE variable
> > > but I could not figure out how to actually use it. It does
> > > some quite complex overhead calculations which did not seem
> > > to work for me on sumo.
> > > 
> > > When the image size is exceeded, build fails like this:
> > > 
> > > ERROR: image-1.0-r0 do_image_tar: Image size 170712 of
> > > /home/builder/src/base/build/tmp/work/linux/image/1.0-r0/deploy-
> > > image-complete/image.rootfs.tar reported by 'du -ks' is larger
> > > than
> > > limit IMAGE_ROOTFS_SIZE_LIMIT 170000
> > 
> > What about IMAGE_ROOTFS_MAXSIZE?
> > 
> > I think IMAGE_ROOTFS_SIZE is about adding extra space to the image,
> > not
> > limiting its size...
> 
> Yea, forgot to mention that I tried that too but got inconsisten
> results.
> I know it's bad but it was easier for me to add this new test than to
> figure out what's wrong with current yocto image size checks. Hence
> RFC.

How would we document this new variable? Recommend users to set both
and hope for the best? :)

Seriously, we need one good way of doing this which works. That means
either you debug the IMAGE_ROOTFS_MAXSIZE option or at least file a bug
with as much information as you can about the problems you're seeing...

Cheers,

Richard

Re: [PATCH 3/5] cmake.bbclass: append includedir to implicit include dirs

[Douglas Royds]

This commit is having an unintended side-effect in the -native (and 
probably nativesdk) case.

In the target build, $includedir is normally /usr/include, 
fully-qualified. This path is already in CMake's list of implicit 
include directories, and we don't include any header files from the 
build PC's /usr/include in a target build in any case. This addition to 
CMAKE_C/CXX_IMPLICIT_INCLUDE_DIRECTORIES has no effect.

In the -native case, the $includedir is set to STAGING_INCDIR_NATIVE, 
but this path has already been added to the BUILD_CPPFLAGS as a system 
include directory, so there will already be no warnings for any header 
file in the STAGING_INCDIR_NATIVE.

There is a nasty side-effect in the -native case: CMake excludes headers 
in the CMAKE_C/CXX_IMPLICIT_INCLUDE_DIRECTORIES from its generated 
dependency files, meaning that a change in any library header file will 
not trigger a recompile of affected source files in the dependent CMake 
component. In out-of-tree builds this isn't a problem, as cmake.bbclass 
deletes the *entire* ${B} directory at configure time, but where this is 
not the case, the build breaks with any change in library headers.

I haven't examined the nativesdk case. The $includedir is set off to 
$SDKPATHNATIVE/usr/include, but this path is not added as a system 
include dir. The same problem will apply as in the -native case, that 
CMake will not generate dependencies for headers staged in the 
SDKPATHNATIVE.

Was nativesdk perhaps the intended case for this commit? Is there a 
better way to silence warnings in this case? Do we need to silence 
library header warnings at all? Should we instead add 
$SDKPATHNATIVE/usr/include as a system include dir via the 
BUILDSDK_CPPFLAGS?

I examined this problem using the Unix Makefiles generator. CMake 
appears to equally exclude these headers from the ninja *.o.d dependency 
files, though I haven't examined it closely.


On 30/11/18 1:21 AM, Mikko Rapeli wrote:

> From: Michael Ho <Michael.Ho@bmw.de>
>
> This resolves issues with paths being marked as system includes that
> differ from /usr/include but are considered implicit by the toolchain.
> This enables developers to add directories to system includes
> to supress compiler compiler warnings from them.
>
> Signed-off-by: Michael Ho <Michael.Ho@bmw.de>
> Cc: Pascal Bach <pascal.bach@siemens.com>
> ---
>   meta/classes/cmake.bbclass | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/meta/classes/cmake.bbclass b/meta/classes/cmake.bbclass
> index fd40a98..485aea6 100644
> --- a/meta/classes/cmake.bbclass
> +++ b/meta/classes/cmake.bbclass
> @@ -108,6 +108,10 @@ list(APPEND CMAKE_MODULE_PATH "${STAGING_DATADIR}/cmake/Modules/")
>   # add for non /usr/lib libdir, e.g. /usr/lib64
>   set( CMAKE_LIBRARY_PATH ${libdir} ${base_libdir})
>   
> +# add include dir to implicit includes in case it differs from /usr/include
> +list(APPEND CMAKE_C_IMPLICIT_INCLUDE_DIRECTORIES ${includedir})
> +list(APPEND CMAKE_CXX_IMPLICIT_INCLUDE_DIRECTORIES ${includedir})
> +
>   EOF
>   }
>   

Re: [PATCH 3/5] cmake.bbclass: append includedir to implicit include dirs

[Douglas Royds]

An error in my analysis below: We *do* delete the dependency files in 
the in-tree build case as well. The side-effect is masked in both 
in-tree and out-of-tree build cases except here at Tait, where we don't 
delete the entire build at configure time, due to a 20min+ rebuild time, 
even with the benefit of icecc.

The rest of my concern still applies: This setting has no effect in the 
target case, and only an unfortunate side effect in the -native case. 
I'm not clear what the benefit is.


On 11/07/19 2:51 PM, Douglas Royds wrote:

> This commit is having an unintended side-effect in the -native (and 
> probably nativesdk) case.
>
> In the target build, $includedir is normally /usr/include, 
> fully-qualified. This path is already in CMake's list of implicit 
> include directories, and we don't include any header files from the 
> build PC's /usr/include in a target build in any case. This addition 
> to CMAKE_C/CXX_IMPLICIT_INCLUDE_DIRECTORIES has no effect.
>
> In the -native case, the $includedir is set to STAGING_INCDIR_NATIVE, 
> but this path has already been added to the BUILD_CPPFLAGS as a system 
> include directory, so there will already be no warnings for any header 
> file in the STAGING_INCDIR_NATIVE.
>
> There is a nasty side-effect in the -native case: CMake excludes 
> headers in the CMAKE_C/CXX_IMPLICIT_INCLUDE_DIRECTORIES from its 
> generated dependency files, meaning that a change in any library 
> header file will not trigger a recompile of affected source files in 
> the dependent CMake component. In out-of-tree builds this isn't a 
> problem, as cmake.bbclass deletes the *entire* ${B} directory at 
> configure time, but where this is not the case, the build breaks with 
> any change in library headers.
>
> I haven't examined the nativesdk case. The $includedir is set off to 
> $SDKPATHNATIVE/usr/include, but this path is not added as a system 
> include dir. The same problem will apply as in the -native case, that 
> CMake will not generate dependencies for headers staged in the 
> SDKPATHNATIVE.
>
> Was nativesdk perhaps the intended case for this commit? Is there a 
> better way to silence warnings in this case? Do we need to silence 
> library header warnings at all? Should we instead add 
> $SDKPATHNATIVE/usr/include as a system include dir via the 
> BUILDSDK_CPPFLAGS?
>
> I examined this problem using the Unix Makefiles generator. CMake 
> appears to equally exclude these headers from the ninja *.o.d 
> dependency files, though I haven't examined it closely.
>
>
> On 30/11/18 1:21 AM, Mikko Rapeli wrote:
>
>> From: Michael Ho <Michael.Ho@bmw.de>
>>
>> This resolves issues with paths being marked as system includes that
>> differ from /usr/include but are considered implicit by the toolchain.
>> This enables developers to add directories to system includes
>> to supress compiler compiler warnings from them.
>>
>> Signed-off-by: Michael Ho <Michael.Ho@bmw.de>
>> Cc: Pascal Bach <pascal.bach@siemens.com>
>> ---
>>   meta/classes/cmake.bbclass | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/meta/classes/cmake.bbclass b/meta/classes/cmake.bbclass
>> index fd40a98..485aea6 100644
>> --- a/meta/classes/cmake.bbclass
>> +++ b/meta/classes/cmake.bbclass
>> @@ -108,6 +108,10 @@ list(APPEND CMAKE_MODULE_PATH 
>> "${STAGING_DATADIR}/cmake/Modules/")
>>   # add for non /usr/lib libdir, e.g. /usr/lib64
>>   set( CMAKE_LIBRARY_PATH ${libdir} ${base_libdir})
>>   +# add include dir to implicit includes in case it differs from 
>> /usr/include
>> +list(APPEND CMAKE_C_IMPLICIT_INCLUDE_DIRECTORIES ${includedir})
>> +list(APPEND CMAKE_CXX_IMPLICIT_INCLUDE_DIRECTORIES ${includedir})
>> +
>>   EOF
>>   }
>
>