Add short option fix for #3798, fix bug in test/SCONSFLAGS.py, fix space issue from #3436

CHANGES.txt

@@ -43,6 +43,11 @@ RELEASE  VERSION/DATE TO BE FILLED IN LATER
     - Suppress missing SConscript deprecation warning if `must_exist=False`
       is used.
 
+  From Dillan Mills:
+    - Fix support for short options (`-x`). 
+    - Add support for long and short options with nargs != '?' to contain spaces:
+      - `--extra A B C` and `-x A B C` will both work as expected
+
 
 RELEASE 4.0.1 - Mon, 16 Jul 2020 16:06:40 -0700
 

SCons/Script/SConsOptions.py

@@ -261,6 +261,76 @@ def error(self, msg):
         sys.stderr.write("SCons Error: %s\n" % msg)
         sys.exit(2)
 
+    def _process_short_opts(self, rargs, values):
+        """
+        SCons-specific processing of short options.
+
+        This is copied directly from the normal
+        optparse._process_short_opts() method, except that, if configured
+        to do so, we catch the exception thrown when an unknown option
+        is encountered and just stick it back on the "leftover" arguments
+        for later (re-)processing.
+        """
+        arg = rargs.pop(0)
+        stop = False
+        i = 1
+        for ch in arg[1:]:
+            opt = "-" + ch
+            option = self._short_opt.get(opt)
+            i += 1                      # we have consumed a character
+
+            try:
+                if not option:
+                    raise optparse.BadOptionError(opt)
+            except optparse.BadOptionError:
+                if self.preserve_unknown_options:
+                    # SCons-specific:  if requested, add unknown options to
+                    # the "leftover arguments" list for later processing.
+                    self.largs.append(arg)
+                    return
+                raise
+
+            if option.takes_value():
+                had_explicit_value = False
+
+                # Any characters left in arg?  Pretend they're the
+                # next arg, and stop consuming characters of arg.
+                if i < len(arg):
+                    rargs.insert(0, arg[i:])
+                    stop = True
+                    had_explicit_value = True
+
+                nargs = option.nargs
+                if len(rargs) < nargs:
+                    if nargs == 1:
+                        self.error(_("%s option requires an argument") % opt)
+                    else:
+                        self.error(_("%s option requires %d arguments")
+                                   % (opt, nargs))
+                elif nargs == 1:
+                    value = rargs.pop(0)
+                    if not had_explicit_value:
+                        SCons.Script._Remove_Target(value)
+                        if '=' in value:
+                            SCons.Script._Remove_Argument(value)
+                else:
+                    value = tuple(rargs[0:nargs])
+                    del rargs[0:nargs]
+                    for i in range(len(value)):
+                        if not had_explicit_value or i > 0:
+                            SCons.Script._Remove_Target(value[i])
+                            if '=' in value[i]:
+                                SCons.Script._Remove_Argument(value[i])
+
+
+            else:                       # option doesn't take a value
+                value = None
+
+            option.process(opt, value, values, self)
+
+            if stop:
+                break
+
     def _process_long_opt(self, rargs, values):
         """
         SCons-specific processing of long options.
@@ -322,9 +392,18 @@ def _process_long_opt(self, rargs, values):
                                % (opt, nargs))
             elif nargs == 1:
                 value = rargs.pop(0)
+                if not had_explicit_value:
+                    SCons.Script._Remove_Target(value)
+                    if '=' in value:
+                        SCons.Script._Remove_Argument(value)
             else:
                 value = tuple(rargs[0:nargs])
                 del rargs[0:nargs]
+                for i in range(len(value)):
+                    if not had_explicit_value or i > 0:
+                        SCons.Script._Remove_Target(value[i])
+                        if '=' in value[i]:
+                            SCons.Script._Remove_Argument(value[i])
 
         elif had_explicit_value:
             self.error(_("%s option does not take a value") % opt)
@@ -334,69 +413,6 @@ def _process_long_opt(self, rargs, values):
 
         option.process(opt, value, values, self)
 
-    def reparse_local_options(self):
-        """ Re-parse the leftover command-line options.
-
-        Parse options stored in `self.largs`, so that any value
-        overridden on the command line is immediately available
-        if the user turns around and does a :func:`GetOption` right away.
-
-        We mimic the processing of the single args
-        in the original OptionParser :func:`_process_args`, but here we
-        allow exact matches for long-opts only (no partial argument names!).
-        Otherwise there could be  problems in :func:`add_local_option`
-        below. When called from there, we try to reparse the
-        command-line arguments that
-
-        1. haven't been processed so far (`self.largs`), but
-        2. are possibly not added to the list of options yet.
-
-        So, when we only have a value for "--myargument" so far,
-        a command-line argument of "--myarg=test" would set it,
-        per the behaviour of :func:`_match_long_opt`,
-        which allows for partial matches of the option name,
-        as long as the common prefix appears to be unique.
-        This would lead to further confusion, because we might want
-        to add another option "--myarg" later on (see issue #2929).
-
-        """
-        rargs = []
-        largs_restore = []
-        # Loop over all remaining arguments
-        skip = False
-        for l in self.largs:
-            if skip:
-                # Accept all remaining arguments as they are
-                largs_restore.append(l)
-            else:
-                if len(l) > 2 and l[0:2] == "--":
-                    # Check long option
-                    lopt = (l,)
-                    if "=" in l:
-                        # Split into option and value
-                        lopt = l.split("=", 1)
-
-                    if lopt[0] in self._long_opt:
-                        # Argument is already known
-                        rargs.append('='.join(lopt))
-                    else:
-                        # Not known yet, so reject for now
-                        largs_restore.append('='.join(lopt))
-                else:
-                    if l == "--" or l == "-":
-                        # Stop normal processing and don't
-                        # process the rest of the command-line opts
-                        largs_restore.append(l)
-                        skip = True
-                    else:
-                        rargs.append(l)
-
-        # Parse the filtered list
-        self.parse_args(rargs, self.values)
-        # Restore the list of remaining arguments for the
-        # next call of AddOption/add_local_option...
-        self.largs = self.largs + largs_restore
-
     def add_local_option(self, *args, **kw):
         """
         Adds a local option to the parser.
@@ -413,7 +429,7 @@ def add_local_option(self, *args, **kw):
             self.local_option_group = group
 
         result = group.add_option(*args, **kw)
-
+        
         if result:
             # The option was added successfully.  We now have to add the
             # default value to our object that holds the default values
@@ -424,8 +440,8 @@ def add_local_option(self, *args, **kw):
             # available if the user turns around and does a GetOption()
             # right away.
             setattr(self.values.__defaults__, result.dest, result.default)
-            self.reparse_local_options()
-
+            self.parse_args(self.largs, self.values)
+            
         return result
 
 class SConsIndentedHelpFormatter(optparse.IndentedHelpFormatter):
@@ -508,7 +524,7 @@ def Parser(version):
     """
 
     formatter = SConsIndentedHelpFormatter(max_help_position=30)
-
+    
     op = SConsOptionParser(option_class=SConsOption,
                            add_help_option=False,
                            formatter=formatter,

SCons/Script/__init__.py

@@ -215,6 +215,21 @@ def _Add_Arguments(alist):
         ARGUMENTS[a] = b
         ARGLIST.append((a, b))
 
+def _Remove_Argument(aarg):
+    if aarg:
+        a, b = aarg.split('=', 1)
+
+        # remove from ARGLIST first which would contain duplicates if -x A=B A=B was specified on the CL
+        if (a, b) in ARGLIST:
+            ARGLIST.remove((a, b))
+
+            # Remove first in case no matching values left in ARGLIST
+            ARGUMENTS.pop(a, None)
+            # Set ARGUMENTS[A] back to latest value in ARGLIST (assuming order matches CL order)
+            for item in ARGLIST:
+                if item[0] == a:
+                    ARGUMENTS[a] = item[1]
+
 def _Add_Targets(tlist):
     if tlist:
         COMMAND_LINE_TARGETS.extend(tlist)
@@ -225,6 +240,15 @@ def _Add_Targets(tlist):
         _build_plus_default._add_Default = _build_plus_default._do_nothing
         _build_plus_default._clear = _build_plus_default._do_nothing
 
+def _Remove_Target(targ):
+    if targ:
+        if targ in COMMAND_LINE_TARGETS:
+            COMMAND_LINE_TARGETS.remove(targ)
+        if targ in BUILD_TARGETS:
+            BUILD_TARGETS.remove(targ)
+        if targ in _build_plus_default:
+            _build_plus_default.remove(targ)
+
 def _Set_Default_Targets_Has_Been_Called(d, fs):
     return DEFAULT_TARGETS