Re: [PATCH v2 02/12] tests/docker: better handle symlinked libs

Re: [PATCH v2 02/12] tests/docker: better handle symlinked libs

[Robert Foley]

Hi,
I was looking at this patch and have a comment about the number of
groups that are expected to be found by this regex.
It seems like the old code expected two groups to be found otherwise
it will not append the library to the found libs.
def _get_so_libs(executable):
    libs = []
    ldd_re = re.compile(r"(/.*/)(\S*)")
    try:
        ldd_output = subprocess.check_output(["ldd",
executable]).decode('utf-8')
        for line in ldd_output.split("\n"):
            search = ldd_re.search(line)
            if search and len(search.groups()) == 2:  <<<<<<<<<<<<<<<
                so_path = search.groups()[0]
                so_lib = search.groups()[1]
                libs.append("%s/%s" % (so_path, so_lib))

I did a bit of experimenting with output from ldd and found a few
strings where the new regex seems
to generate only one group for the entire path+lib rather than one group
for the path and another group for the lib.

$ ldd build/aarch64-softmmu/qemu-system-aarch64
__snip__
      /lib/ld-linux-aarch64.so.1 (0x0000ffff9c41f000)
      libgmodule-2.0.so.0 =>
/usr/lib/aarch64-linux-gnu/libgmodule-2.0.so.0 (0x0000ffff9a96e000)
__snip
$ python3
Python 3.6.8 (default, Oct  7 2019, 12:59:55)
[GCC 8.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import re
>>> ldd_re = re.compile(r"(?:\S+ => )?(\S*) \(:?0x[0-9a-f]+\)")
>>> a = "/lib/ld-linux-aarch64.so.1 (0x0000ffff9c41f000)"
>>> b = "libgmodule-2.0.so.0 => /usr/lib/aarch64-linux-gnu/libgmodule-2.0.so.0 (0x0000ffff9a96e000)"
>>> ldd_re.search(a).groups()
('/lib/ld-linux-aarch64.so.1',)
>>> ldd_re.search(b).groups()
('/usr/lib/aarch64-linux-gnu/libgmodule-2.0.so.0',)
>>> len(ldd_re.search(a).groups())
1
>>> len(ldd_re.search(b).groups())
1
>>> ldd_re_old = re.compile('(/.*/)(\S*)')
>>> ldd_re_old.search(a).groups()
('/lib/', 'ld-linux-aarch64.so.1')
>>> ldd_re_old.search(b).groups()
('/usr/lib/aarch64-linux-gnu/', 'libgmodule-2.0.so.0')
>>> len(ldd_re_old.search(a).groups())
2
>>> len(ldd_re_old.search(b).groups())
2
>>>

Thanks & Regards,
-Rob

On Thu, 30 Jan 2020 at 06:40, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> When we are copying we want to ensure we grab the first
> resolution (the found in path section). However even that binary might
> be a symlink so lets make sure we chase the symlinks to copy the right
> binary to where it can be found.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  tests/docker/docker.py | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> diff --git a/tests/docker/docker.py b/tests/docker/docker.py
> index 31d8adf836..7dfca63fe4 100755
> --- a/tests/docker/docker.py
> +++ b/tests/docker/docker.py
> @@ -109,7 +109,7 @@ def _get_so_libs(executable):
>      ensure theright data is copied."""
>
>      libs = []
> -    ldd_re = re.compile(r"(/.*/)(\S*)")
> +    ldd_re = re.compile(r"(?:\S+ => )?(\S*) \(:?0x[0-9a-f]+\)")
>      try:
>          ldd_output = subprocess.check_output(["ldd", executable]).decode('utf-8')
>          for line in ldd_output.split("\n"):
> @@ -145,7 +145,8 @@ def _copy_binary_with_libs(src, bin_dest, dest_dir):
>      if libs:
>          for l in libs:
>              so_path = os.path.dirname(l)
> -            _copy_with_mkdir(l, dest_dir, so_path)
> +            real_l = os.path.realpath(l)
> +            _copy_with_mkdir(real_l, dest_dir, so_path)
>  def _check_binfmt_misc(executable):
> --
> 2.20.1
>

Re: [PATCH v2 02/12] tests/docker: better handle symlinked libs

[Philippe Mathieu-Daudé]

On 1/30/20 3:37 PM, Robert Foley wrote:
> Hi,
> I was looking at this patch and have a comment about the number of
> groups that are expected to be found by this regex.
> It seems like the old code expected two groups to be found otherwise
> it will not append the library to the found libs.
> def _get_so_libs(executable):
>      libs = []
>      ldd_re = re.compile(r"(/.*/)(\S*)")
>      try:
>          ldd_output = subprocess.check_output(["ldd",
> executable]).decode('utf-8')
>          for line in ldd_output.split("\n"):
>              search = ldd_re.search(line)
>              if search and len(search.groups()) == 2:  <<<<<<<<<<<<<<<
>                  so_path = search.groups()[0]
>                  so_lib = search.groups()[1]
>                  libs.append("%s/%s" % (so_path, so_lib))

Yes you are right, this part need change to handle a single group now.

> 
> I did a bit of experimenting with output from ldd and found a few
> strings where the new regex seems
> to generate only one group for the entire path+lib rather than one group
> for the path and another group for the lib.
> 
> $ ldd build/aarch64-softmmu/qemu-system-aarch64
> __snip__
>        /lib/ld-linux-aarch64.so.1 (0x0000ffff9c41f000)
>        libgmodule-2.0.so.0 =>
> /usr/lib/aarch64-linux-gnu/libgmodule-2.0.so.0 (0x0000ffff9a96e000)
> __snip
> $ python3
> Python 3.6.8 (default, Oct  7 2019, 12:59:55)
> [GCC 8.3.0] on linux
> Type "help", "copyright", "credits" or "license" for more information.
>>>> import re
>>>> ldd_re = re.compile(r"(?:\S+ => )?(\S*) \(:?0x[0-9a-f]+\)")
>>>> a = "/lib/ld-linux-aarch64.so.1 (0x0000ffff9c41f000)"
>>>> b = "libgmodule-2.0.so.0 => /usr/lib/aarch64-linux-gnu/libgmodule-2.0.so.0 (0x0000ffff9a96e000)"
>>>> ldd_re.search(a).groups()
> ('/lib/ld-linux-aarch64.so.1',)
>>>> ldd_re.search(b).groups()
> ('/usr/lib/aarch64-linux-gnu/libgmodule-2.0.so.0',)
>>>> len(ldd_re.search(a).groups())
> 1
>>>> len(ldd_re.search(b).groups())
> 1
>>>> ldd_re_old = re.compile('(/.*/)(\S*)')
>>>> ldd_re_old.search(a).groups()
> ('/lib/', 'ld-linux-aarch64.so.1')
>>>> ldd_re_old.search(b).groups()
> ('/usr/lib/aarch64-linux-gnu/', 'libgmodule-2.0.so.0')
>>>> len(ldd_re_old.search(a).groups())
> 2
>>>> len(ldd_re_old.search(b).groups())
> 2
>>>>
> 
> Thanks & Regards,
> -Rob
> 
> On Thu, 30 Jan 2020 at 06:40, Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>> When we are copying we want to ensure we grab the first
>> resolution (the found in path section). However even that binary might
>> be a symlink so lets make sure we chase the symlinks to copy the right
>> binary to where it can be found.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>   tests/docker/docker.py | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>> diff --git a/tests/docker/docker.py b/tests/docker/docker.py
>> index 31d8adf836..7dfca63fe4 100755
>> --- a/tests/docker/docker.py
>> +++ b/tests/docker/docker.py
>> @@ -109,7 +109,7 @@ def _get_so_libs(executable):
>>       ensure theright data is copied."""
>>
>>       libs = []
>> -    ldd_re = re.compile(r"(/.*/)(\S*)")
>> +    ldd_re = re.compile(r"(?:\S+ => )?(\S*) \(:?0x[0-9a-f]+\)")
>>       try:
>>           ldd_output = subprocess.check_output(["ldd", executable]).decode('utf-8')
>>           for line in ldd_output.split("\n"):
>> @@ -145,7 +145,8 @@ def _copy_binary_with_libs(src, bin_dest, dest_dir):
>>       if libs:
>>           for l in libs:
>>               so_path = os.path.dirname(l)
>> -            _copy_with_mkdir(l, dest_dir, so_path)
>> +            real_l = os.path.realpath(l)
>> +            _copy_with_mkdir(real_l, dest_dir, so_path)
>>   def _check_binfmt_misc(executable):
>> --
>> 2.20.1
>>
> 

[PATCH v2 00/12] testing/next (with build fixes!)

[Alex Bennée]

Hi,

It seemed the CI degraded while I wasn't looking so I've rolled up a
bunch of fixes to try and get things working again.

There is some messing around for documentation builds (disabling on
shippable, moving to bionic on Travis), a build fix for hppa +
no-default-configs and some attempts to squash timeouts on our new
multiple architecture builds.

I shall build the forthcoming PR directly from this posted series.

The following patches need review:
  01 - tests docker move most cross compilers to buster 
  02 - tests docker better handle symlinked libs
  03 - gitlab ci Refresh the list of iotests
  04 - travis.yml Install genisoimage package
  05 - .shippable disable docs for cross compile tests
  07 - .travis.yml Drop superfluous use of python python
  08 - .travis.yml Add description to each job
  09 - .travis.yml build documents under bionic
  10 - .travis.yml move cache flushing to early common p
  11 - .travis.yml probe for number of available process
  12 - .travis.yml limit the arm64 target list

Alex Bennée (7):
  tests/docker: move most cross compilers to buster base
  tests/docker: better handle symlinked libs
  .shippable: --disable-docs for cross-compile tests
  .travis.yml: build documents under bionic
  .travis.yml: move cache flushing to early common phase
  .travis.yml: probe for number of available processors
  .travis.yml: limit the arm64 target list

Philippe Mathieu-Daudé (3):
  hw/hppa/Kconfig: LASI chipset requires PARALLEL port
  .travis.yml: Drop superfluous use of --python=python3 parameter
  .travis.yml: Add description to each job

Thomas Huth (1):
  gitlab-ci: Refresh the list of iotests

Wainer dos Santos Moschetta (1):
  travis.yml: Install genisoimage package

 .gitlab-ci.yml                                |  12 +-
 .shippable.yml                                |   2 +-
 .travis.yml                                   | 145 ++++++++++++------
 hw/hppa/Kconfig                               |   1 +
 tests/docker/Makefile.include                 |  16 +-
 tests/docker/docker.py                        |   5 +-
 tests/docker/dockerfiles/debian-amd64.docker  |   2 +-
 .../dockerfiles/debian-armel-cross.docker     |   2 +-
 .../dockerfiles/debian-armhf-cross.docker     |   2 +-
 .../dockerfiles/debian-mips64el-cross.docker  |   2 +-
 .../dockerfiles/debian-mipsel-cross.docker    |   2 +-
 .../dockerfiles/debian-ppc64el-cross.docker   |   2 +-
 .../dockerfiles/debian-s390x-cross.docker     |   2 +-
 13 files changed, 121 insertions(+), 74 deletions(-)

-- 
2.20.1

[PATCH v2 02/12] tests/docker: better handle symlinked libs

[Alex Bennée]

When we are copying we want to ensure we grab the first
resolution (the found in path section). However even that binary might
be a symlink so lets make sure we chase the symlinks to copy the right
binary to where it can be found.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/docker/docker.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tests/docker/docker.py b/tests/docker/docker.py
index 31d8adf836..7dfca63fe4 100755
--- a/tests/docker/docker.py
+++ b/tests/docker/docker.py
@@ -109,7 +109,7 @@ def _get_so_libs(executable):
     ensure theright data is copied."""
 
     libs = []
-    ldd_re = re.compile(r"(/.*/)(\S*)")
+    ldd_re = re.compile(r"(?:\S+ => )?(\S*) \(:?0x[0-9a-f]+\)")
     try:
         ldd_output = subprocess.check_output(["ldd", executable]).decode('utf-8')
         for line in ldd_output.split("\n"):
@@ -145,7 +145,8 @@ def _copy_binary_with_libs(src, bin_dest, dest_dir):
     if libs:
         for l in libs:
             so_path = os.path.dirname(l)
-            _copy_with_mkdir(l, dest_dir, so_path)
+            real_l = os.path.realpath(l)
+            _copy_with_mkdir(real_l, dest_dir, so_path)
 
 
 def _check_binfmt_misc(executable):
-- 
2.20.1

Re: [PATCH v2 02/12] tests/docker: better handle symlinked libs

[Philippe Mathieu-Daudé]

On 1/30/20 12:32 PM, Alex Bennée wrote:
> When we are copying we want to ensure we grab the first
> resolution (the found in path section). However even that binary might
> be a symlink so lets make sure we chase the symlinks to copy the right
> binary to where it can be found.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   tests/docker/docker.py | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/docker/docker.py b/tests/docker/docker.py
> index 31d8adf836..7dfca63fe4 100755
> --- a/tests/docker/docker.py
> +++ b/tests/docker/docker.py
> @@ -109,7 +109,7 @@ def _get_so_libs(executable):
>       ensure theright data is copied."""
>   
>       libs = []
> -    ldd_re = re.compile(r"(/.*/)(\S*)")
> +    ldd_re = re.compile(r"(?:\S+ => )?(\S*) \(:?0x[0-9a-f]+\)")
>       try:
>           ldd_output = subprocess.check_output(["ldd", executable]).decode('utf-8')
>           for line in ldd_output.split("\n"):
> @@ -145,7 +145,8 @@ def _copy_binary_with_libs(src, bin_dest, dest_dir):
>       if libs:
>           for l in libs:
>               so_path = os.path.dirname(l)
> -            _copy_with_mkdir(l, dest_dir, so_path)
> +            real_l = os.path.realpath(l)
> +            _copy_with_mkdir(real_l, dest_dir, so_path)
>   
>   
>   def _check_binfmt_misc(executable):
> 

Rob raised an issue in this patch, it appears in a separated thread:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg675307.html

Re: [PATCH v2 02/12] tests/docker: better handle symlinked libs

[Alex Bennée]

Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> On 1/30/20 12:32 PM, Alex Bennée wrote:
>> When we are copying we want to ensure we grab the first
>> resolution (the found in path section). However even that binary might
>> be a symlink so lets make sure we chase the symlinks to copy the right
>> binary to where it can be found.
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>   tests/docker/docker.py | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>> diff --git a/tests/docker/docker.py b/tests/docker/docker.py
>> index 31d8adf836..7dfca63fe4 100755
>> --- a/tests/docker/docker.py
>> +++ b/tests/docker/docker.py
>> @@ -109,7 +109,7 @@ def _get_so_libs(executable):
>>       ensure theright data is copied."""
>>         libs = []
>> -    ldd_re = re.compile(r"(/.*/)(\S*)")
>> +    ldd_re = re.compile(r"(?:\S+ => )?(\S*) \(:?0x[0-9a-f]+\)")
>>       try:
>>           ldd_output = subprocess.check_output(["ldd", executable]).decode('utf-8')
>>           for line in ldd_output.split("\n"):
>> @@ -145,7 +145,8 @@ def _copy_binary_with_libs(src, bin_dest, dest_dir):
>>       if libs:
>>           for l in libs:
>>               so_path = os.path.dirname(l)
>> -            _copy_with_mkdir(l, dest_dir, so_path)
>> +            real_l = os.path.realpath(l)
>> +            _copy_with_mkdir(real_l, dest_dir, so_path)
>>     
>>   def _check_binfmt_misc(executable):
>> 
>
> Rob raised an issue in this patch, it appears in a separated thread:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg675307.html

If fixed it up thusly:

def _get_so_libs(executable):
    """Return a list of libraries associated with an executable.

    The paths may be symbolic links which would need to be resolved to
    ensure the right data is copied."""

    libs = []
    ldd_re = re.compile(r"(?:\S+ => )?(\S*) \(:?0x[0-9a-f]+\)")
    try:
        ldd_output = subprocess.check_output(["ldd", executable]).decode('utf-8')
        for line in ldd_output.split("\n"):
            search = ldd_re.search(line)
            if search:
                try:
                    libs.append(s.group(1))
                except IndexError:
                    pass
    except subprocess.CalledProcessError:
        print("%s had no associated libraries (static build?)" % (executable))

    return libs


-- 
Alex Bennée