[PATCH] D109833: [Dexter] Mutually exclusive argument group for --builder and --binary

[Stephen Tozer via Phabricator via llvm-commits]

StephenTozer created this revision.
StephenTozer added reviewers: jmorse, Orlando, TWeaver, chrisjackson.
StephenTozer added a project: debug-info.
StephenTozer requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Dexter currently accepts two possible arguments to determine the binary used for testing; either --builder <builder> (and optionally compiler/linker flags) to build the binary, or --binary <binary> to use the provided binary directly. If both are passed, then --binary overrides --builder; if neither are passed, then an error is raised. This patch instead puts these arguments into a required mutually exclusive argument group, so that an error is automatically raised by argparse if both or neither are given.

As an additional change, the --cflags and --ldflags will now raise a warning if they are passed without the --builder flag, as they are meaningless if Dexter is using a pre-built binary.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D109833

Files:
  cross-project-tests/debuginfo-tests/dexter/dex/builder/ParserOptions.py
  cross-project-tests/debuginfo-tests/dexter/dex/tools/TestToolBase.py


Index: cross-project-tests/debuginfo-tests/dexter/dex/tools/TestToolBase.py
===================================================================
--- cross-project-tests/debuginfo-tests/dexter/dex/tools/TestToolBase.py
+++ cross-project-tests/debuginfo-tests/dexter/dex/tools/TestToolBase.py
@@ -54,15 +54,11 @@
     def handle_options(self, defaults):
         options = self.context.options
 
-        # We accept either or both of --binary and --builder.
-        if not options.binary and not options.builder:
-            raise Error('expected --builder or --binary')
+        if not options.builder and (options.cflags or options.ldflags):
+            warn(self.context, '--cflags and --ldflags will be ignored when not'
+                               ' using --builder')
 
-        # --binary overrides --builder
         if options.binary:
-            if options.builder:
-                warn(self.context, "overriding --builder with --binary\n")
-
             options.binary = os.path.abspath(options.binary)
             if not os.path.isfile(options.binary):
                 raise Error('<d>could not find binary file</> <r>"{}"</>'
Index: cross-project-tests/debuginfo-tests/dexter/dex/builder/ParserOptions.py
===================================================================
--- cross-project-tests/debuginfo-tests/dexter/dex/builder/ParserOptions.py
+++ cross-project-tests/debuginfo-tests/dexter/dex/builder/ParserOptions.py
@@ -38,11 +38,12 @@
 
 
 def add_builder_tool_arguments(parser):
-    parser.add_argument('--binary',
-                        metavar="<file>",
-                        help='provide binary file to override --builder')
+    build_group = parser.add_mutually_exclusive_group(required=True)
+    build_group.add_argument('--binary',
+                             metavar="<file>",
+                             help='provide binary file instead of --builder')
 
-    parser.add_argument(
+    build_group.add_argument(
         '--builder',
         type=str,
         choices=sorted(_find_build_scripts().keys()),


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D109833.372716.patch
Type: text/x-patch
Size: 2095 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210915/07cba52f/attachment.bin>