[PATCH v4 0/4] Feature to use symbolic links to binaries for ptest

[PATCH v4 0/4] Feature to use symbolic links to binaries for ptest

[Mariano Lopez]

Some binaries are renamed by update-alternatives class at build time
and some ptest run against a binary belonging to another package. Take
for example busybox or util-linux.

This series adds a directory within the ptest with symlinks to the
binaries produced by the package, so just adding this directory to the
PATH will test the correct binaries.

To use this feature just add PTEST_BINDIR = "1" to the recipe.

v2 changes:

Add busybox recipe to use this method of symlinks generation.

Refactor the way the binary ptest directory is generated. Now the
directory will contain only symlinks to binaries that were renamed by
update-alternatives and not symlinks to all the package's binaries. This
have some advantages:
- Less lines and the code is cleaner.
- There is no need to have symlinks for every binary.
- Reuse existing update-alternatives functionality, don't need to add
  new features to the class.

v3 changes:

Fix white space mismatch

v4 changes:

Check for PTEST_ENABLED to add the functionality, this will solve the
issue when building nativesdk packages

The following changes since commit ffa5a1bda6741f5dc9f1b8db1bb37b0c6f103c99:

  asciidoc: specify XML catalogue to use (2019-04-03 14:56:27 +0100)

are available in the Git repository at:

  git://github.com/justanotherboy/poky bug13238v4
  https://github.com/justanotherboy/poky/tree/bug13238v4

Mariano Lopez (4):
  update-alternatives.bbclass: Add function to get metadata
  ptest.bbclass: Add feature to populate a binary directory
  util-linux: Use PTEST binary directory
  busybox: Use PTEST binary directory

 meta/classes/ptest.bbclass                    | 32 +++++++++
 meta/classes/update-alternatives.bbclass      | 70 ++++++++++++-------
 meta/recipes-core/busybox/busybox.inc         | 16 +----
 meta/recipes-core/util-linux/util-linux.inc   |  5 +-
 .../util-linux/util-linux/run-ptest           |  4 ++
 5 files changed, 86 insertions(+), 41 deletions(-)

-- 
2.19.2

[PATCH v4 1/4] update-alternatives.bbclass: Add function to get metadata

[Mariano Lopez]

This adds update_alternatives_alt_targets function to get the metadata
for a package. This is for code reuse because the metadata would help
other classes that needs to be aware of how update-alternatives modify
the final package.

[YOCTO #12597]
[YOCTO #13238]

Signed-off-by: Mariano Lopez <just.another.mariano@gmail.com>
---
 meta/classes/update-alternatives.bbclass | 70 +++++++++++++++---------
 1 file changed, 45 insertions(+), 25 deletions(-)

diff --git a/meta/classes/update-alternatives.bbclass b/meta/classes/update-alternatives.bbclass
index 537e85d9a3..b702e77ee5 100644
--- a/meta/classes/update-alternatives.bbclass
+++ b/meta/classes/update-alternatives.bbclass
@@ -216,42 +216,62 @@ python apply_update_alternative_renames () {
             update_files(alt_target, alt_target_rename, pkg, d)
 }
 
+def update_alternatives_alt_targets(d, pkg):
+    """
+    Returns the update-alternatives metadata for a package.
+
+    The returned format is a list of tuples where the tuple contains:
+    alt_name:     The binary name
+    alt_link:     The path for the binary (Shared by different packages)
+    alt_target:   The path for the renamed binary (Unique per package)
+    alt_priority: The priority of the alt_target
+
+    All the alt_targets will be installed into the sysroot. The alt_link is
+    a symlink pointing to the alt_target with the highest priority.
+    """
+
+    pn = d.getVar('BPN')
+    pkgdest = d.getVar('PKGD')
+    updates = list()
+    for alt_name in (d.getVar('ALTERNATIVE_%s' % pkg) or "").split():
+        alt_link     = d.getVarFlag('ALTERNATIVE_LINK_NAME', alt_name)
+        alt_target   = d.getVarFlag('ALTERNATIVE_TARGET_%s' % pkg, alt_name) or \
+                       d.getVarFlag('ALTERNATIVE_TARGET', alt_name) or \
+                       d.getVar('ALTERNATIVE_TARGET_%s' % pkg) or \
+                       d.getVar('ALTERNATIVE_TARGET') or \
+                       alt_link
+        alt_priority = d.getVarFlag('ALTERNATIVE_PRIORITY_%s' % pkg,  alt_name) or \
+                       d.getVarFlag('ALTERNATIVE_PRIORITY',  alt_name) or \
+                       d.getVar('ALTERNATIVE_PRIORITY_%s' % pkg) or  \
+                       d.getVar('ALTERNATIVE_PRIORITY')
+
+        # This shouldn't trigger, as it should have been resolved earlier!
+        if alt_link == alt_target:
+            bb.note('alt_link == alt_target: %s == %s -- correcting, this should not happen!' % (alt_link, alt_target))
+            alt_target = '%s.%s' % (alt_target, pn)
+
+        if not os.path.lexists('%s/%s' % (pkgdest, alt_target)):
+            bb.warn('%s: NOT adding alternative provide %s: %s does not exist' % (pn, alt_link, alt_target))
+            continue
+
+        alt_target = os.path.normpath(alt_target)
+        updates.append( (alt_name, alt_link, alt_target, alt_priority) )
+
+    return updates
+
 PACKAGESPLITFUNCS_prepend = "populate_packages_updatealternatives "
 
 python populate_packages_updatealternatives () {
     if not update_alternatives_enabled(d):
         return
 
-    pn = d.getVar('BPN')
-
     # Do actual update alternatives processing
-    pkgdest = d.getVar('PKGD')
     for pkg in (d.getVar('PACKAGES') or "").split():
         # Create post install/removal scripts
         alt_setup_links = ""
         alt_remove_links = ""
-        for alt_name in (d.getVar('ALTERNATIVE_%s' % pkg) or "").split():
-            alt_link     = d.getVarFlag('ALTERNATIVE_LINK_NAME', alt_name)
-            alt_target   = d.getVarFlag('ALTERNATIVE_TARGET_%s' % pkg, alt_name) or d.getVarFlag('ALTERNATIVE_TARGET', alt_name)
-            alt_target   = alt_target or d.getVar('ALTERNATIVE_TARGET_%s' % pkg) or d.getVar('ALTERNATIVE_TARGET') or alt_link
-            # Sometimes alt_target is specified as relative to the link name.
-            alt_target   = os.path.join(os.path.dirname(alt_link), alt_target)
-
-            alt_priority = d.getVarFlag('ALTERNATIVE_PRIORITY_%s' % pkg,  alt_name) or d.getVarFlag('ALTERNATIVE_PRIORITY',  alt_name)
-            alt_priority = alt_priority or d.getVar('ALTERNATIVE_PRIORITY_%s' % pkg) or d.getVar('ALTERNATIVE_PRIORITY')
-
-            # This shouldn't trigger, as it should have been resolved earlier!
-            if alt_link == alt_target:
-                bb.note('alt_link == alt_target: %s == %s -- correcting, this should not happen!' % (alt_link, alt_target))
-                alt_target = '%s.%s' % (alt_target, pn)
-
-            if not os.path.lexists('%s/%s' % (pkgdest, alt_target)):
-                bb.warn('%s: NOT adding alternative provide %s: %s does not exist' % (pn, alt_link, alt_target))
-                continue
-
-            # Default to generate shell script.. eventually we may want to change this...
-            alt_target = os.path.normpath(alt_target)
-
+        updates = update_alternatives_alt_targets(d, pkg)
+        for alt_name, alt_link, alt_target, alt_priority in updates:
             alt_setup_links  += '\tupdate-alternatives --install %s %s %s %s\n' % (alt_link, alt_name, alt_target, alt_priority)
             alt_remove_links += '\tupdate-alternatives --remove  %s %s\n' % (alt_name, alt_target)
 
-- 
2.19.2

[PATCH v4 2/4] ptest.bbclass: Add feature to populate a binary directory

[Mariano Lopez]

This adds the functionality to create a binary directory within
PTEST_PATH directory. This directory will be populated with
symlinks pointing to the binaries installed by the package and
then renamed by update-alternatives. This way the ptest only needs
to source this binary directory in order to use the expected
binaries.

To enable this feature just add PTEST_BINDIR = "1" to the recipe.

[YOCTO #12597]
[YOCTO #13238]

Signed-off-by: Mariano Lopez <just.another.mariano@gmail.com>
---
 meta/classes/ptest.bbclass | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/meta/classes/ptest.bbclass b/meta/classes/ptest.bbclass
index 97865c9338..e87a9659cb 100644
--- a/meta/classes/ptest.bbclass
+++ b/meta/classes/ptest.bbclass
@@ -65,6 +65,38 @@ do_install_ptest_base() {
     done
 }
 
+PTEST_BINDIR_PKGD_PATH = "${PKGD}${PTEST_PATH}/bin"
+
+# This function needs to run after apply_update_alternative_renames because the
+# aforementioned function will update the ALTERNATIVE_LINK_NAME flag. Append is
+# used here to make this function to run as late as possible.
+PACKAGE_PREPROCESS_FUNCS_append = "${@bb.utils.contains('PTEST_BINDIR', '1', \
+                                    bb.utils.contains('PTEST_ENABLED', '1', ' ptest_update_alternatives', '', d), '', d)}"
+
+python ptest_update_alternatives() {
+    """
+    This function will generate the symlinks in the PTEST_BINDIR_PKGD_PATH
+    to match the renamed binaries by update-alternatives.
+    """
+
+    if not bb.data.inherits_class('update-alternatives', d) \
+           or not update_alternatives_enabled(d):
+        return
+
+    bb.note("Generating symlinks for ptest")
+    bin_paths = { os.environ["bindir"], os.environ["base_bindir"],
+                  os.environ["sbindir"], os.environ["base_sbindir"] }
+    ptest_bindir = d.getVar("PTEST_BINDIR_PKGD_PATH")
+    os.mkdir(ptest_bindir)
+    for pkg in (d.getVar('PACKAGES') or "").split():
+        alternatives = update_alternatives_alt_targets(d, pkg)
+        for alt_name, alt_link, alt_target, _ in alternatives:
+            # Some alternatives are for man pages,
+            # check if the alternative is in PATH
+            if os.path.dirname(alt_link) in bin_paths:
+                os.symlink(alt_target, os.path.join(ptest_bindir, alt_name))
+}
+
 do_configure_ptest_base[dirs] = "${B}"
 do_compile_ptest_base[dirs] = "${B}"
 do_install_ptest_base[dirs] = "${B}"
-- 
2.19.2

Re: [PATCH v4 2/4] ptest.bbclass: Add feature to populate a binary directory

[Richard Purdie]

On Tue, 2019-04-09 at 00:44 -0500, Mariano Lopez wrote:
> This adds the functionality to create a binary directory within
> PTEST_PATH directory. This directory will be populated with
> symlinks pointing to the binaries installed by the package and
> then renamed by update-alternatives. This way the ptest only needs
> to source this binary directory in order to use the expected
> binaries.
> 
> To enable this feature just add PTEST_BINDIR = "1" to the recipe.
> 
> [YOCTO #12597]
> [YOCTO #13238]
> 
> Signed-off-by: Mariano Lopez <just.another.mariano@gmail.com>
> ---
>  meta/classes/ptest.bbclass | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/meta/classes/ptest.bbclass b/meta/classes/ptest.bbclass
> index 97865c9338..e87a9659cb 100644
> --- a/meta/classes/ptest.bbclass
> +++ b/meta/classes/ptest.bbclass
> @@ -65,6 +65,38 @@ do_install_ptest_base() {
>      done
>  }
>  
> +PTEST_BINDIR_PKGD_PATH = "${PKGD}${PTEST_PATH}/bin"
> +
> +# This function needs to run after apply_update_alternative_renames
> because the
> +# aforementioned function will update the ALTERNATIVE_LINK_NAME
> flag. Append is
> +# used here to make this function to run as late as possible.
> +PACKAGE_PREPROCESS_FUNCS_append = "${@bb.utils.contains('PTEST_BINDI
> R', '1', \
> +                                    bb.utils.contains('PTEST_ENABLED
> ', '1', ' ptest_update_alternatives', '', d), '', d)}"
> +
> +python ptest_update_alternatives() {
> +    """
> +    This function will generate the symlinks in the
> PTEST_BINDIR_PKGD_PATH
> +    to match the renamed binaries by update-alternatives.
> +    """
> +
> +    if not bb.data.inherits_class('update-alternatives', d) \
> +           or not update_alternatives_enabled(d):
> +        return
> +
> +    bb.note("Generating symlinks for ptest")
> +    bin_paths = { os.environ["bindir"], os.environ["base_bindir"],

I'm going to merge this but is there a good reason you're using
os.environ[] here? If not we should merge a small fixup to use
d.getVar() since ultimately we don't really want these things in the
environment if we can help it.

Cheers,

Richard

Re: [PATCH v4 2/4] ptest.bbclass: Add feature to populate a binary directory

[Mariano Lopez]

On 4/11/19 3:11 PM, Richard Purdie wrote:
> On Tue, 2019-04-09 at 00:44 -0500, Mariano Lopez wrote:
>> This adds the functionality to create a binary directory within
>> PTEST_PATH directory. This directory will be populated with
>> symlinks pointing to the binaries installed by the package and
>> then renamed by update-alternatives. This way the ptest only needs
>> to source this binary directory in order to use the expected
>> binaries.
>>
>> To enable this feature just add PTEST_BINDIR = "1" to the recipe.
>>
>> [YOCTO #12597]
>> [YOCTO #13238]
>>
>> Signed-off-by: Mariano Lopez <just.another.mariano@gmail.com>
>> ---
>>   meta/classes/ptest.bbclass | 32 ++++++++++++++++++++++++++++++++
>>   1 file changed, 32 insertions(+)
>>
>> diff --git a/meta/classes/ptest.bbclass b/meta/classes/ptest.bbclass
>> index 97865c9338..e87a9659cb 100644
>> --- a/meta/classes/ptest.bbclass
>> +++ b/meta/classes/ptest.bbclass
>> @@ -65,6 +65,38 @@ do_install_ptest_base() {
>>       done
>>   }
>>   
>> +PTEST_BINDIR_PKGD_PATH = "${PKGD}${PTEST_PATH}/bin"
>> +
>> +# This function needs to run after apply_update_alternative_renames
>> because the
>> +# aforementioned function will update the ALTERNATIVE_LINK_NAME
>> flag. Append is
>> +# used here to make this function to run as late as possible.
>> +PACKAGE_PREPROCESS_FUNCS_append = "${@bb.utils.contains('PTEST_BINDI
>> R', '1', \
>> +                                    bb.utils.contains('PTEST_ENABLED
>> ', '1', ' ptest_update_alternatives', '', d), '', d)}"
>> +
>> +python ptest_update_alternatives() {
>> +    """
>> +    This function will generate the symlinks in the
>> PTEST_BINDIR_PKGD_PATH
>> +    to match the renamed binaries by update-alternatives.
>> +    """
>> +
>> +    if not bb.data.inherits_class('update-alternatives', d) \
>> +           or not update_alternatives_enabled(d):
>> +        return
>> +
>> +    bb.note("Generating symlinks for ptest")
>> +    bin_paths = { os.environ["bindir"], os.environ["base_bindir"],
> I'm going to merge this but is there a good reason you're using
> os.environ[] here? If not we should merge a small fixup to use
> d.getVar() since ultimately we don't really want these things in the
> environment if we can help it.
>
> Cheers,
>
> Richard
>
Sorry about that, I just sent the fix for this

Mariano

[PATCH v4 3/4] util-linux: Use PTEST binary directory

[Mariano Lopez]

Some binaries generated by util-linux will be replaced by core-utils
in the final image by update-alternatives, so use a dedicated directory
with symlinks to avoid using a binary generated by another package.

This will solve the issue with the ptest runner timing out when
running the kill ptests for util-linux.

[YOCTO #13238]

Signed-off-by: Mariano Lopez <just.another.mariano@gmail.com>
---
 meta/recipes-core/util-linux/util-linux.inc       | 5 +++--
 meta/recipes-core/util-linux/util-linux/run-ptest | 4 ++++
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/meta/recipes-core/util-linux/util-linux.inc b/meta/recipes-core/util-linux/util-linux.inc
index 18c3af240e..a67318e84e 100644
--- a/meta/recipes-core/util-linux/util-linux.inc
+++ b/meta/recipes-core/util-linux/util-linux.inc
@@ -284,6 +284,7 @@ ALTERNATIVE_LINK_NAME[wall.1] = "${mandir}/man1/wall.1"
 
 BBCLASSEXTEND = "native nativesdk"
 
+PTEST_BINDIR = "1"
 do_compile_ptest() {
     oe_runmake buildtest-TESTS
 }
@@ -312,8 +313,8 @@ do_install_ptest() {
         '/^\tif[[:space:]]\[[[:space:]]![[:space:]]-x[[:space:]]"$1"/s|$1|`which $1 2>/dev/null`|g' \
          ${D}${PTEST_PATH}/tests/functions.sh
 
-    # "kill -L" behaves differently than "/bin/kill -L" so we need an additional fix
+    # Running "kill" without the the complete path would use the shell's built-in kill
     sed -i -e \
-         '/^TS_CMD_KILL/ s|kill|/bin/kill|g' \
+         '/^TS_CMD_KILL/ s|kill|${PTEST_PATH}/bin/kill|g' \
          ${D}${PTEST_PATH}/tests/commands.sh
 }
diff --git a/meta/recipes-core/util-linux/util-linux/run-ptest b/meta/recipes-core/util-linux/util-linux/run-ptest
index fbc2f9b56a..2178ab8fef 100644
--- a/meta/recipes-core/util-linux/util-linux/run-ptest
+++ b/meta/recipes-core/util-linux/util-linux/run-ptest
@@ -1,5 +1,9 @@
 #!/bin/sh
 
+current_path=$(readlink -f $0)
+export bindir=$(dirname $current_path)
+export PATH=$bindir/bin:$PATH
+
 cd tests || exit 1                                                          
 
 comps=$(find ts/ -type f -perm -111 -regex ".*/[^\.~]*" |  sort)
-- 
2.19.2

[PATCH v4 4/4] busybox: Use PTEST binary directory

[Mariano Lopez]

This will generate the symlinks in the ptest binary directory using the
ptest class functionality instead of generating them manually. Because
the ptest class uses update-alternatives to get the metadata for the
symlinks it will respect the use of BUSYBOX_SPLIT_SUID automatically.

[YOCTO #12597]

Signed-off-by: Mariano Lopez <just.another.mariano@gmail.com>
---
 meta/recipes-core/busybox/busybox.inc | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/meta/recipes-core/busybox/busybox.inc b/meta/recipes-core/busybox/busybox.inc
index c9d25ff1ca..174ce5a8c0 100644
--- a/meta/recipes-core/busybox/busybox.inc
+++ b/meta/recipes-core/busybox/busybox.inc
@@ -347,24 +347,12 @@ do_install () {
 	fi
 }
 
+PTEST_BINDIR = "1"
+
 do_install_ptest () {
 	cp -r ${B}/testsuite ${D}${PTEST_PATH}/
 	cp ${B}/.config      ${D}${PTEST_PATH}/
 	ln -s /bin/busybox   ${D}${PTEST_PATH}/busybox
-
-	mkdir ${D}${PTEST_PATH}/bin
-	if [ "${BUSYBOX_SPLIT_SUID}" = "1" ]; then
-		while read link; do
-			ln -s ${base_bindir}/busybox.suid ${D}${PTEST_PATH}/bin/$(basename $link)
-		done <${D}${sysconfdir}/busybox.links.suid
-		while read link; do
-			ln -s ${base_bindir}/busybox.nosuid ${D}${PTEST_PATH}/bin/$(basename $link)
-		done <${D}${sysconfdir}/busybox.links.nosuid
-	else
-		while read link; do
-			ln -s ${base_bindir}/busybox ${D}${PTEST_PATH}/bin/$(basename $link)
-		done <${D}${sysconfdir}/busybox.links
-	fi
 }
 
 inherit update-alternatives
-- 
2.19.2