Re: [PATCH 1/5] iotests: explicitly pass source/build dir to 'check' command

["Alex Bennée" <alex.bennee@linaro.org>]

Daniel P. Berrangé <berrange@redhat.com> writes:

> The 'check' script has some rather dubious logic whereby it assumes
> that if invoked as a symlink, then it is running from a separate
> source tree and build tree, otherwise it assumes the current working
> directory is a combined source and build tree.
>
> This doesn't work if you want to invoke the 'check' script using
> its full source tree path while still using a split source and build
> tree layout. This would be a typical situation with meson if you ask
> it to find the 'check' script path using files('check').
>
> Rather than trying to make the logic more magical, add support for
> explicitly passing the dirs using --source-dir and --build-dir. If
> either is omitted the current logic is maintained.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  tests/qemu-iotests/check      |  8 +++++++-
>  tests/qemu-iotests/testenv.py | 17 +++++++++++++----
>  2 files changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
> index 9bdda1394e..806abc21d6 100755
> --- a/tests/qemu-iotests/check
> +++ b/tests/qemu-iotests/check
> @@ -113,6 +113,10 @@ def make_argparser() -> argparse.ArgumentParser:
>                         'middle of the process.')
>      g_sel.add_argument('tests', metavar='TEST_FILES', nargs='*',
>                         help='tests to run, or "--" followed by a command')
> +    g_sel.add_argument('--build-dir', default=None,
> +                       help='Path to iotests build directory')
> +    g_sel.add_argument('--source-dir', default=None,
> +                       help='Path to iotests build directory')
>

I'd be tempted to push all the automagic stuff into the options so you
have something like:

--8<---------------cut here---------------start------------->8---
modified   tests/qemu-iotests/check
@@ -27,8 +27,23 @@ from testenv import TestEnv
 from testrunner import TestRunner
 
 
+def get_default_path(follow_link=False):
+    """
+    Try to automagically figure out the path we are running from.
+    """
+    # called from the build tree?
+    if os.path.islink(sys.argv[0]):
+        if follow_link:
+            return os.path.dirname(os.readlink(sys.argv[0]))
+        else:
+            return os.path.dirname(os.path.abspath(sys.argv[0]))
+    else:  # or source tree?
+        return os.getcwd()
+
+
 def make_argparser() -> argparse.ArgumentParser:
-    p = argparse.ArgumentParser(description="Test run options")
+    p = argparse.ArgumentParser(description="Test run options",
+                                formatter_class=argparse.ArgumentDefaultsHelpFormatter)
 
     p.add_argument('-n', '--dry-run', action='store_true',
                    help='show me, do not run tests')
@@ -113,9 +128,9 @@ def make_argparser() -> argparse.ArgumentParser:
                        'middle of the process.')
     g_sel.add_argument('tests', metavar='TEST_FILES', nargs='*',
                        help='tests to run, or "--" followed by a command')
-    g_sel.add_argument('--build-dir', default=None,
+    g_sel.add_argument('--build-dir', default=get_default_path(),
                        help='Path to iotests build directory')
-    g_sel.add_argument('--source-dir', default=None,
+    g_sel.add_argument('--source-dir', default=get_default_path(follow_link=True),
                        help='Path to iotests build directory')
 
     return p
modified   tests/qemu-iotests/testenv.py
@@ -213,23 +213,8 @@ def __init__(self, imgfmt: str, imgproto: str, aiomode: str,
         # Initialize generic paths: build_root, build_iotests, source_iotests,
         # which are needed to initialize some environment variables. They are
         # used by init_*() functions as well.
-
-        if os.path.islink(sys.argv[0]):
-            # called from the build tree
-            self.source_iotests = os.path.dirname(
-                os.readlink(sys.argv[0]))
-            self.build_iotests = os.path.dirname(
-                os.path.abspath(sys.argv[0]))
-        else:
-            # called from the source tree
-            self.source_iotests = os.getcwd()
-            self.build_iotests = os.getcwd()
-
-        if source_dir is not None:
-            self.source_iotests = source_dir
-        if build_dir is not None:
-            self.build_iotests = build_dir
-
+        self.source_iotests = source_dir
+        self.build_iotests = build_dir
         self.build_root = os.path.join(self.build_iotests, '..', '..')
 
         self.init_directories()
--8<---------------cut here---------------end--------------->8---


>      return p
>  
> @@ -124,7 +128,9 @@ if __name__ == '__main__':
>                    aiomode=args.aiomode, cachemode=args.cachemode,
>                    imgopts=args.imgopts, misalign=args.misalign,
>                    debug=args.debug, valgrind=args.valgrind,
> -                  gdb=args.gdb, qprint=args.print)
> +                  gdb=args.gdb, qprint=args.print,
> +                  source_dir=args.source_dir,
> +                  build_dir=args.build_dir)
>  
>      if len(sys.argv) > 1 and sys.argv[-len(args.tests)-1] == '--':
>          if not args.tests:
> diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
> index a864c74b12..9bf37cd381 100644
> --- a/tests/qemu-iotests/testenv.py
> +++ b/tests/qemu-iotests/testenv.py
> @@ -177,7 +177,9 @@ def __init__(self, imgfmt: str, imgproto: str, aiomode: str,
>                   debug: bool = False,
>                   valgrind: bool = False,
>                   gdb: bool = False,
> -                 qprint: bool = False) -> None:
> +                 qprint: bool = False,
> +                 source_dir: Optional[str] = None,
> +                 build_dir: Optional[str] = None) -> None:
>          self.imgfmt = imgfmt
>          self.imgproto = imgproto
>          self.aiomode = aiomode
> @@ -213,12 +215,19 @@ def __init__(self, imgfmt: str, imgproto: str, aiomode: str,
>  
>          if os.path.islink(sys.argv[0]):
>              # called from the build tree
> -            self.source_iotests = os.path.dirname(os.readlink(sys.argv[0]))
> -            self.build_iotests = os.path.dirname(os.path.abspath(sys.argv[0]))
> +            self.source_iotests = os.path.dirname(
> +                os.readlink(sys.argv[0]))
> +            self.build_iotests = os.path.dirname(
> +                os.path.abspath(sys.argv[0]))
>          else:
>              # called from the source tree
>              self.source_iotests = os.getcwd()
> -            self.build_iotests = self.source_iotests
> +            self.build_iotests = os.getcwd()
> +
> +        if source_dir is not None:
> +            self.source_iotests = source_dir
> +        if build_dir is not None:
> +            self.build_iotests = build_dir
>  
>          self.build_root = os.path.join(self.build_iotests, '..', '..')


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro