[PATCH 1/2] oeqa/selftest/case: Add recipeinc method

[PATCH 1/2] oeqa/selftest/case: Add recipeinc method

[Ola x Nilsson]

The recipeinc method returns the absolute path of the test_recipe.inc
file of a specified recipe.  It replaces four instances of identical
code, and make it possible to access the filename from a testcase for
cleanup.

The write_recipeinc and append_recipeinc methods are changed to return
the path to the file in case that is useful.

The test_recipe.inc file is usually cleaned up in a finally block,
but that block executes before any teardown operations.  This blocks
any teardown that requires the presence of the test_recipe.inc file.

Signed-off-by: Ola x Nilsson <olani@axis.com>
---
 meta/lib/oeqa/selftest/case.py | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/meta/lib/oeqa/selftest/case.py b/meta/lib/oeqa/selftest/case.py
index 871009c568..e09915b495 100644
--- a/meta/lib/oeqa/selftest/case.py
+++ b/meta/lib/oeqa/selftest/case.py
@@ -212,27 +212,33 @@ to ensure accurate results.")
         self.logger.debug("Removing from: %s\n%s\n" % (self.testinc_path, data))
         ftools.remove_from_file(self.testinc_path, data)
 
+    def recipeinc(self, recipe):
+        """Return absolute path of meta-sefltest/recipes-test/<recipe>/test_recipe.inc"""
+        return os.path.join(self.testlayer_path, 'recipes-test', recipe, 'test_recipe.inc')
+
     def write_recipeinc(self, recipe, data):
         """Write to meta-sefltest/recipes-test/<recipe>/test_recipe.inc"""
-        inc_file = os.path.join(self.testlayer_path, 'recipes-test', recipe, 'test_recipe.inc')
+        inc_file = self.recipeinc(recipe)
         self.logger.debug("Writing to: %s\n%s\n" % (inc_file, data))
         ftools.write_file(inc_file, data)
+        return inc_file
 
     def append_recipeinc(self, recipe, data):
         """Append data to meta-sefltest/recipes-test/<recipe>/test_recipe.inc"""
-        inc_file = os.path.join(self.testlayer_path, 'recipes-test', recipe, 'test_recipe.inc')
+        inc_file = self.recipeinc(recipe)
         self.logger.debug("Appending to: %s\n%s\n" % (inc_file, data))
         ftools.append_file(inc_file, data)
+        return inc_file
 
     def remove_recipeinc(self, recipe, data):
         """Remove data from meta-sefltest/recipes-test/<recipe>/test_recipe.inc"""
-        inc_file = os.path.join(self.testlayer_path, 'recipes-test', recipe, 'test_recipe.inc')
+        inc_file = self.recipeinc(recipe)
         self.logger.debug("Removing from: %s\n%s\n" % (inc_file, data))
         ftools.remove_from_file(inc_file, data)
 
     def delete_recipeinc(self, recipe):
         """Delete meta-sefltest/recipes-test/<recipe>/test_recipe.inc file"""
-        inc_file = os.path.join(self.testlayer_path, 'recipes-test', recipe, 'test_recipe.inc')
+        inc_file = self.recipeinc(recipe)
         self.logger.debug("Deleting file: %s" % inc_file)
         try:
             os.remove(inc_file)
@@ -259,13 +265,13 @@ to ensure accurate results.")
         self.logger.debug("Writing to: %s\n%s\n" % (self.machineinc_path, data))
         ftools.write_file(self.machineinc_path, data)
 
-    # check does path exist    
+    # check does path exist
     def assertExists(self, expr, msg=None):
         if not os.path.exists(expr):
             msg = self._formatMessage(msg, "%s does not exist" % safe_repr(expr))
             raise self.failureException(msg)
-    
-    # check does path not exist 
+
+    # check does path not exist
     def assertNotExists(self, expr, msg=None):
         if os.path.exists(expr):
             msg = self._formatMessage(msg, "%s exists when it should not" % safe_repr(expr))
-- 
2.11.0

[PATCH 2/2] externalsrc.bbclass: Avoid symlink clashes for virtclasses

[Ola x Nilsson]

There was a race condifion in externalsrc_configure_prefuncs when the
same source folder is used for several variants of the same recipe,
like this:

EXTERNALSRC_pn-foo = "..."
EXTERNALSRC_pn-foo-native = "..."

The symlinks were created once for each variant of the recipe, and
where they led in the end depended on which do_configure task executed
last.  Create one set of symlinks for each variant by adding an EXTSRC_SUFFIX
variable to the end of the link names.
Tries to handle all known virtclasses and multilib variants.

Use a lockfile for externalsrc_configure_prefuncs to protect the
.git/info/exclude file.

Signed-off-by: Ola x Nilsson <olani@axis.com>
---
 meta/classes/externalsrc.bbclass        | 16 ++++++++++-
 meta/lib/oeqa/selftest/cases/devtool.py | 48 +++++++++++++++++++++++++++++++++
 2 files changed, 63 insertions(+), 1 deletion(-)

diff --git a/meta/classes/externalsrc.bbclass b/meta/classes/externalsrc.bbclass
index 8141f25e04..980e339cdc 100644
--- a/meta/classes/externalsrc.bbclass
+++ b/meta/classes/externalsrc.bbclass
@@ -24,8 +24,21 @@
 # EXTERNALSRC_BUILD_pn-myrecipe = "/path/to/my/source/tree"
 #
 
+def get_symlink_suffix(var, suffixes, d):
+    # Like oe.utils.prune_suffix but return the suffix instead
+    for suffix in suffixes:
+        if var.endswith(suffix):
+            return suffix
+    if var.startswith('nativesdk-'):
+        return '-nativesdk'
+    prefix = d.getVar("MLPREFIX")
+    if prefix and var.startswith(prefix):
+        return '-' + prefix[:-1]
+    return ''
+
 SRCTREECOVEREDTASKS ?= "do_patch do_unpack do_fetch"
-EXTERNALSRC_SYMLINKS ?= "oe-workdir:${WORKDIR} oe-logs:${T}"
+EXTSRC_SUFFIX = "${@get_symlink_suffix(d.getVar('PN'), d.getVar('SPECIAL_PKGSUFFIX').split(), d)}"
+EXTERNALSRC_SYMLINKS ?= "oe-workdir${EXTSRC_SUFFIX}:${WORKDIR} oe-logs${EXTSRC_SUFFIX}:${T}"
 
 python () {
     externalsrc = d.getVar('EXTERNALSRC')
@@ -128,6 +141,7 @@ python () {
             d.setVar('STAMPCLEAN', '${STAMPS_DIR}/work-shared/${PN}/*-*')
 }
 
+externalsrc_configure_prefunc[lockfiles] += " ${S}/conf_prefunc.lock"
 python externalsrc_configure_prefunc() {
     s_dir = d.getVar('S')
     # Create desired symlinks
diff --git a/meta/lib/oeqa/selftest/cases/devtool.py b/meta/lib/oeqa/selftest/cases/devtool.py
index 88d69724f9..4119280b48 100644
--- a/meta/lib/oeqa/selftest/cases/devtool.py
+++ b/meta/lib/oeqa/selftest/cases/devtool.py
@@ -491,6 +491,54 @@ class DevtoolTests(DevtoolBase):
         result = runCmd('devtool status')
         self.assertNotIn('mdadm', result.output)
 
+    def test_devtool_modify_configure_prefunc(self):
+        self.write_config("""
+MACHINE = "qemux86-64"
+require conf/multilib.conf
+MULTILIBS = "multilib:lib32"
+DEFAULTTUNE_virtclass-multilib-lib32 = "x86"
+""")
+        self.add_command_to_tearDown('bitbake-layers remove-layer */workspace')
+        self.write_recipeinc('emptytest', '''
+BBCLASSEXTEND = "native nativesdk"
+do_patch[noexec] = "1"
+INHIBIT_DEFAULT_DEPS = "1"
+''')
+        self.track_for_cleanup(self.recipeinc('emptytest'))
+        targets = 'emptytest emptytest-native nativesdk-emptytest lib32-emptytest'
+        self.add_command_to_tearDown('bitbake -c clean ' + targets)
+        tempdir = tempfile.mkdtemp(prefix='devtoolqa')
+        self.track_for_cleanup(tempdir)
+        runCmd('devtool modify emptytest -x %s' % tempdir)
+        self.track_for_cleanup(self.workspacedir)
+        self.append_recipeinc('emptytest', 'EXTERNALSRC_pn-emptytest-native = "%s"\n' % tempdir)
+        self.append_recipeinc('emptytest', 'EXTERNALSRC_pn-nativesdk-emptytest = "%s"\n' % tempdir)
+        self.append_recipeinc('emptytest', 'EXTERNALSRC_pn-lib32-emptytest = "%s"\n' % tempdir)
+        bitbake('-c cleansstate ' + targets)
+        bitbake('-c configure ' + targets)
+
+        def assert_link(linkname, destdir):
+            linkname = os.path.join(tempdir, linkname)
+            self.assertExists(linkname)
+            self.assertTrue(os.path.islink(linkname))
+            dst = os.readlink(linkname)
+            self.assertEqual(dst, destdir)
+
+        bbvars = {'': get_bb_vars(['WORKDIR', 'T'], 'emptytest'),
+                  '-native': get_bb_vars(['WORKDIR', 'T'], 'emptytest-native'),
+                  '-lib32': get_bb_vars(['WORKDIR', 'T'], 'lib32-emptytest'),
+                  '-nativesdk': get_bb_vars(['WORKDIR', 'T'], 'nativesdk-emptytest')}
+        for variant in bbvars:
+            assert_link('oe-logs' + variant, bbvars[variant]['T'])
+            assert_link('oe-workdir' + variant, bbvars[variant]['WORKDIR'])
+
+        with open(os.path.join(tempdir, '.git/info/exclude')) as efile:
+            lines = set(efile.readlines())
+            expected = {'/%s%s\n' % (base, sfx)
+                        for base in ['oe-logs', 'oe-workdir']
+                        for sfx in bbvars}
+            self.assertTrue(expected <= lines)
+
     @OETestID(1620)
     def test_devtool_buildclean(self):
         def assertFile(path, *paths):
-- 
2.11.0

Re: [PATCH 2/2] externalsrc.bbclass: Avoid symlink clashes for virtclasses

[Ola x Nilsson]

I noticed that the preceding commit has been merged. 
Is there some problem with this one?

--
Ola Nilsson
________________________________________
From: openembedded-core-bounces@lists.openembedded.org <openembedded-core-bounces@lists.openembedded.org> on behalf of Ola x Nilsson <olani@axis.com>
Sent: Monday, August 28, 2017 16:58
To: openembedded-core@lists.openembedded.org
Subject: [OE-core] [PATCH 2/2] externalsrc.bbclass: Avoid symlink clashes       for virtclasses

There was a race condifion in externalsrc_configure_prefuncs when the
same source folder is used for several variants of the same recipe,
like this:

EXTERNALSRC_pn-foo = "..."
EXTERNALSRC_pn-foo-native = "..."

The symlinks were created once for each variant of the recipe, and
where they led in the end depended on which do_configure task executed
last.  Create one set of symlinks for each variant by adding an EXTSRC_SUFFIX
variable to the end of the link names.
Tries to handle all known virtclasses and multilib variants.

Use a lockfile for externalsrc_configure_prefuncs to protect the
.git/info/exclude file.

Signed-off-by: Ola x Nilsson <olani@axis.com>
---
 meta/classes/externalsrc.bbclass        | 16 ++++++++++-
 meta/lib/oeqa/selftest/cases/devtool.py | 48 +++++++++++++++++++++++++++++++++
 2 files changed, 63 insertions(+), 1 deletion(-)

diff --git a/meta/classes/externalsrc.bbclass b/meta/classes/externalsrc.bbclass
index 8141f25e04..980e339cdc 100644
--- a/meta/classes/externalsrc.bbclass
+++ b/meta/classes/externalsrc.bbclass
@@ -24,8 +24,21 @@
 # EXTERNALSRC_BUILD_pn-myrecipe = "/path/to/my/source/tree"
 #

+def get_symlink_suffix(var, suffixes, d):
+    # Like oe.utils.prune_suffix but return the suffix instead
+    for suffix in suffixes:
+        if var.endswith(suffix):
+            return suffix
+    if var.startswith('nativesdk-'):
+        return '-nativesdk'
+    prefix = d.getVar("MLPREFIX")
+    if prefix and var.startswith(prefix):
+        return '-' + prefix[:-1]
+    return ''
+
 SRCTREECOVEREDTASKS ?= "do_patch do_unpack do_fetch"
-EXTERNALSRC_SYMLINKS ?= "oe-workdir:${WORKDIR} oe-logs:${T}"
+EXTSRC_SUFFIX = "${@get_symlink_suffix(d.getVar('PN'), d.getVar('SPECIAL_PKGSUFFIX').split(), d)}"
+EXTERNALSRC_SYMLINKS ?= "oe-workdir${EXTSRC_SUFFIX}:${WORKDIR} oe-logs${EXTSRC_SUFFIX}:${T}"

 python () {
     externalsrc = d.getVar('EXTERNALSRC')
@@ -128,6 +141,7 @@ python () {
             d.setVar('STAMPCLEAN', '${STAMPS_DIR}/work-shared/${PN}/*-*')
 }

+externalsrc_configure_prefunc[lockfiles] += " ${S}/conf_prefunc.lock"
 python externalsrc_configure_prefunc() {
     s_dir = d.getVar('S')
     # Create desired symlinks
diff --git a/meta/lib/oeqa/selftest/cases/devtool.py b/meta/lib/oeqa/selftest/cases/devtool.py
index 88d69724f9..4119280b48 100644
--- a/meta/lib/oeqa/selftest/cases/devtool.py
+++ b/meta/lib/oeqa/selftest/cases/devtool.py
@@ -491,6 +491,54 @@ class DevtoolTests(DevtoolBase):
         result = runCmd('devtool status')
         self.assertNotIn('mdadm', result.output)

+    def test_devtool_modify_configure_prefunc(self):
+        self.write_config("""
+MACHINE = "qemux86-64"
+require conf/multilib.conf
+MULTILIBS = "multilib:lib32"
+DEFAULTTUNE_virtclass-multilib-lib32 = "x86"
+""")
+        self.add_command_to_tearDown('bitbake-layers remove-layer */workspace')
+        self.write_recipeinc('emptytest', '''
+BBCLASSEXTEND = "native nativesdk"
+do_patch[noexec] = "1"
+INHIBIT_DEFAULT_DEPS = "1"
+''')
+        self.track_for_cleanup(self.recipeinc('emptytest'))
+        targets = 'emptytest emptytest-native nativesdk-emptytest lib32-emptytest'
+        self.add_command_to_tearDown('bitbake -c clean ' + targets)
+        tempdir = tempfile.mkdtemp(prefix='devtoolqa')
+        self.track_for_cleanup(tempdir)
+        runCmd('devtool modify emptytest -x %s' % tempdir)
+        self.track_for_cleanup(self.workspacedir)
+        self.append_recipeinc('emptytest', 'EXTERNALSRC_pn-emptytest-native = "%s"\n' % tempdir)
+        self.append_recipeinc('emptytest', 'EXTERNALSRC_pn-nativesdk-emptytest = "%s"\n' % tempdir)
+        self.append_recipeinc('emptytest', 'EXTERNALSRC_pn-lib32-emptytest = "%s"\n' % tempdir)
+        bitbake('-c cleansstate ' + targets)
+        bitbake('-c configure ' + targets)
+
+        def assert_link(linkname, destdir):
+            linkname = os.path.join(tempdir, linkname)
+            self.assertExists(linkname)
+            self.assertTrue(os.path.islink(linkname))
+            dst = os.readlink(linkname)
+            self.assertEqual(dst, destdir)
+
+        bbvars = {'': get_bb_vars(['WORKDIR', 'T'], 'emptytest'),
+                  '-native': get_bb_vars(['WORKDIR', 'T'], 'emptytest-native'),
+                  '-lib32': get_bb_vars(['WORKDIR', 'T'], 'lib32-emptytest'),
+                  '-nativesdk': get_bb_vars(['WORKDIR', 'T'], 'nativesdk-emptytest')}
+        for variant in bbvars:
+            assert_link('oe-logs' + variant, bbvars[variant]['T'])
+            assert_link('oe-workdir' + variant, bbvars[variant]['WORKDIR'])
+
+        with open(os.path.join(tempdir, '.git/info/exclude')) as efile:
+            lines = set(efile.readlines())
+            expected = {'/%s%s\n' % (base, sfx)
+                        for base in ['oe-logs', 'oe-workdir']
+                        for sfx in bbvars}
+            self.assertTrue(expected <= lines)
+
     @OETestID(1620)
     def test_devtool_buildclean(self):
         def assertFile(path, *paths):
--
2.11.0

--
_______________________________________________
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core

Re: [PATCH 2/2] externalsrc.bbclass: Avoid symlink clashes for virtclasses

[Richard Purdie]

On Tue, 2017-09-12 at 11:23 +0000, Ola x Nilsson wrote:
> I noticed that the preceding commit has been merged. 
> Is there some problem with this one?

Its not something I can describe easily, more a gut feeling that is
shared by some others I've talked to.

Basically externalsrc is turning into an ever more complicated pile of
corner cases and it worries me a lot. I can't help thinking we really
want to simplify and streamline this, not add more complexity to it.

How exactly we do that I'm not sure, I haven't had the time to spend
looking at the code. I do appreciate you added a test case though.

I guess what might help would be creating standard API in lib/oe to get
things like the remapped name.

I'm actually tempted to throw out he externalsrc symlinks stuff. If our
directory layout is so bad we really should fix that "properly" and
that's something I'm thinking about for the next development cycle...

Cheers,

Richard

Re: [PATCH 2/2] externalsrc.bbclass: Avoid symlink clashes for virtclasses

[Ola x Nilsson]

________________________________________
> From: Richard Purdie <richard.purdie@linuxfoundation.org>
> Sent: Wednesday, September 13, 2017 12:41
>
>On Tue, 2017-09-12 at 11:23 +0000, Ola x Nilsson wrote:
>> I noticed that the preceding commit has been merged.
>>  Is there some problem with this one?
>
> Its not something I can describe easily, more a gut feeling that is
> shared by some others I've talked to.
>
> Basically externalsrc is turning into an ever more complicated pile of
> corner cases and it worries me a lot. I can't help thinking we really
> want to simplify and streamline this, not add more complexity to it.

Can't argue with that.

/Ola