[PATCH 17/22] tests/functional: generalize archive_extract

Daniel P. Berrangé posted 22 patches 3 weeks, 5 days ago
There is a newer version of this series
[PATCH 17/22] tests/functional: generalize archive_extract
Posted by Daniel P. Berrangé 3 weeks, 5 days ago
There are many types of archives that the tests deal with, and
'archive_extract' suggests it can cope with any, rather than only
tar files. Rename the existing method to 'tar_extract' and add a
new method that can dynamically extract any zip, tar or cpio file
based on file extension.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/functional/qemu_test/utils.py | 31 +++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/tests/functional/qemu_test/utils.py b/tests/functional/qemu_test/utils.py
index bafe7fb80e..8c1df8f8c2 100644
--- a/tests/functional/qemu_test/utils.py
+++ b/tests/functional/qemu_test/utils.py
@@ -14,6 +14,7 @@
 import shutil
 import subprocess
 import tarfile
+import zipfile
 
 from .cmd import run_cmd
 
@@ -38,7 +39,33 @@ def image_pow2ceil_expand(path):
             with open(path, 'ab+') as fd:
                 fd.truncate(size_aligned)
 
-def archive_extract(archive, dest_dir, member=None):
+def archive_extract(archive, dest_dir, format=None, member=None):
+    if format == "tar":
+        tar_extract(archive, dest_dir, member)
+    elif format == "zip":
+        zip_extract(archive, dest_dir, member)
+    elif format == "cpio":
+        if member is not None:
+            raise Exception("Unable to filter cpio extraction")
+        cpio_extract(archive, dest_dir)
+    elif format == "deb":
+        deb_extract(archive, dest_dir, "./" + member)
+    else:
+        raise Exception(f"Unknown archive format {format}")
+
+def guess_archive_format(path):
+    if ".tar." in path or path.endswith("tgz"):
+        return "tar"
+    elif path.endswith(".zip"):
+        return "zip"
+    elif path.endswith(".cpio"):
+        return "cpio"
+    elif path.endswith(".deb") or path.endswith(".udeb"):
+        return "deb"
+    else:
+        raise Exception(f"Unknown archive format for {path}")
+
+def tar_extract(archive, dest_dir, member=None):
     with tarfile.open(archive) as tf:
         if hasattr(tarfile, 'data_filter'):
             tf.extraction_filter = getattr(tarfile, 'data_filter',
@@ -62,7 +89,7 @@ def deb_extract(archive, dest_dir, member=None):
         (stdout, stderr, ret) = run_cmd(['ar', 't', archive])
         file_path = stdout.split()[2]
         run_cmd(['ar', 'x', archive, file_path])
-        archive_extract(file_path, dest_dir, member)
+        archive_extract(file_path, dest_dir, format="tar", member=member)
     finally:
         os.chdir(cwd)
 
-- 
2.46.0


Re: [PATCH 17/22] tests/functional: generalize archive_extract
Posted by Thomas Huth 3 weeks, 3 days ago
On 29/11/2024 18.31, Daniel P. Berrangé wrote:
> There are many types of archives that the tests deal with, and
> 'archive_extract' suggests it can cope with any, rather than only
> tar files. Rename the existing method to 'tar_extract' and add a
> new method that can dynamically extract any zip, tar or cpio file
> based on file extension.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   tests/functional/qemu_test/utils.py | 31 +++++++++++++++++++++++++++--
>   1 file changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/functional/qemu_test/utils.py b/tests/functional/qemu_test/utils.py
> index bafe7fb80e..8c1df8f8c2 100644
> --- a/tests/functional/qemu_test/utils.py
> +++ b/tests/functional/qemu_test/utils.py
> @@ -14,6 +14,7 @@
>   import shutil
>   import subprocess
>   import tarfile
> +import zipfile
>   
>   from .cmd import run_cmd
>   
> @@ -38,7 +39,33 @@ def image_pow2ceil_expand(path):
>               with open(path, 'ab+') as fd:
>                   fd.truncate(size_aligned)
>   
> -def archive_extract(archive, dest_dir, member=None):
> +def archive_extract(archive, dest_dir, format=None, member=None):

Why not doing "if not format: format = guess_archive_format(archive)" here?
Otherwise this helper function is rather useless - if you have to know the 
format, you could directly call the appropriate function anyway.

  Thomas


> +    if format == "tar":
> +        tar_extract(archive, dest_dir, member)
> +    elif format == "zip":
> +        zip_extract(archive, dest_dir, member)
> +    elif format == "cpio":
> +        if member is not None:
> +            raise Exception("Unable to filter cpio extraction")
> +        cpio_extract(archive, dest_dir)
> +    elif format == "deb":
> +        deb_extract(archive, dest_dir, "./" + member)
> +    else:
> +        raise Exception(f"Unknown archive format {format}")
> +
> +def guess_archive_format(path):
> +    if ".tar." in path or path.endswith("tgz"):
> +        return "tar"
> +    elif path.endswith(".zip"):
> +        return "zip"
> +    elif path.endswith(".cpio"):
> +        return "cpio"
> +    elif path.endswith(".deb") or path.endswith(".udeb"):
> +        return "deb"
> +    else:
> +        raise Exception(f"Unknown archive format for {path}")
> +
> +def tar_extract(archive, dest_dir, member=None):
>       with tarfile.open(archive) as tf:
>           if hasattr(tarfile, 'data_filter'):
>               tf.extraction_filter = getattr(tarfile, 'data_filter',
> @@ -62,7 +89,7 @@ def deb_extract(archive, dest_dir, member=None):
>           (stdout, stderr, ret) = run_cmd(['ar', 't', archive])
>           file_path = stdout.split()[2]
>           run_cmd(['ar', 'x', archive, file_path])
> -        archive_extract(file_path, dest_dir, member)
> +        archive_extract(file_path, dest_dir, format="tar", member=member)
>       finally:
>           os.chdir(cwd)
>   


Re: [PATCH 17/22] tests/functional: generalize archive_extract
Posted by Daniel P. Berrangé 3 weeks, 2 days ago
On Mon, Dec 02, 2024 at 11:20:19AM +0100, Thomas Huth wrote:
> On 29/11/2024 18.31, Daniel P. Berrangé wrote:
> > There are many types of archives that the tests deal with, and
> > 'archive_extract' suggests it can cope with any, rather than only
> > tar files. Rename the existing method to 'tar_extract' and add a
> > new method that can dynamically extract any zip, tar or cpio file
> > based on file extension.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >   tests/functional/qemu_test/utils.py | 31 +++++++++++++++++++++++++++--
> >   1 file changed, 29 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tests/functional/qemu_test/utils.py b/tests/functional/qemu_test/utils.py
> > index bafe7fb80e..8c1df8f8c2 100644
> > --- a/tests/functional/qemu_test/utils.py
> > +++ b/tests/functional/qemu_test/utils.py
> > @@ -14,6 +14,7 @@
> >   import shutil
> >   import subprocess
> >   import tarfile
> > +import zipfile
> >   from .cmd import run_cmd
> > @@ -38,7 +39,33 @@ def image_pow2ceil_expand(path):
> >               with open(path, 'ab+') as fd:
> >                   fd.truncate(size_aligned)
> > -def archive_extract(archive, dest_dir, member=None):
> > +def archive_extract(archive, dest_dir, format=None, member=None):
> 
> Why not doing "if not format: format = guess_archive_format(archive)" here?
> Otherwise this helper function is rather useless - if you have to know the
> format, you could directly call the appropriate function anyway.

That's how I had things at first, but then I hit the problem that
about 80% of the files we're extracting are cached assets, and the
asset filenames are just a hash digest, so there's no file extension
to query.

I guess I could make this work by pushing the Asset object instance
right down into this method, rather than converting from Asset ->
filename in the caller.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|