Fix option parsing conflict with build request.

Because of our use of the relaxed option parsing of Lyra, to account for the B2 adhoc options, there was a conflict where invalid options would feed into the build request. That caused all kinds of confusion for the user seeing messages about unknown targets or invalid requests instead of proper option errors.

The fix is to only feed the request parsing unrecognized arguments. And fix the option parsing so that it checks and errors when it encounters bad known option usage.

This change also fixes an inverted interpretation of the `---keep-going` option.

fixes #562

Author: grafikrobot

doc/src/history.adoc

@@ -39,6 +39,10 @@
   -- _Paolo Pastori_
 * Fix GLOB_ARCHIVE builtin rule to work with BSD archive (ar).
   -- _Paolo Pastori_
+* Fix parsing of options to not conflict with command line build request.
+  -- _René Ferdinand Rivera Morell_
+* Fix inversion of logic for `--keep-going` option.
+  -- _Paolo Pastori_
 
 == Version 5.4.2
 

src/build-system.jam

@@ -1,5 +1,5 @@
 # Copyright 2003, 2005, 2007 Dave Abrahams
-# Copyright 2006, 2007 Rene Rivera
+# Copyright 2006-2026 René Ferdinand Rivera Morell
 # Copyright 2003, 2004, 2005, 2006 Vladimir Prus
 # Distributed under the Boost Software License, Version 1.0.
 # (See accompanying file LICENSE.txt or copy at
@@ -39,9 +39,6 @@ import virtual-target ;
 #
 ################################################################################
 
-# Shortcut used in this module for accessing used command-line parameters.
-.argv = [ modules.peek : ARGV ] ;
-
 # Flag indicating we should display additional debugging information related to
 # locating and loading Boost Build configuration files.
 .debug-config = [ args.get-arg debug-configuration ] ;
@@ -523,7 +520,7 @@ local rule process-explicit-toolset-requests
     local extra-properties ;
 
     local option-toolsets  = [ regex.split-list [ args.get-arg toolsets ] : "," ] ;
-    local feature-toolsets = [ regex.split-list [ MATCH ^toolset=(.*)$ : $(.argv) ] : "," ] ;
+    local feature-toolsets = [ regex.split-list [ MATCH ^toolset=(.*)$ : [ args.extra-args ] ] : "," ] ;
 
     for local t in $(option-toolsets) $(feature-toolsets)
     {
@@ -564,7 +561,7 @@ local rule process-explicit-toolset-requests
         # Make sure we get an appropriate property into the build request in
         # case toolset has been specified using the "--toolset=..." command-line
         # option form.
-        if ! $(t) in $(.argv) $(feature-toolsets)
+        if ! $(t) in [ args.extra-args ] $(feature-toolsets)
         {
             if $(.debug-config)
             {
@@ -683,7 +680,7 @@ local rule should-clean-project ( project )
     # target ids containing explicit project references. See what to do about
     # those as such 'lazy loading' may cause problems that are then extremely
     # difficult to debug.
-    local build-request = [ build-request.from-command-line $(.argv)
+    local build-request = [ build-request.from-extra-args [ args.extra-args ]
         $(extra-properties) ] ;
     local target-ids = [ $(build-request).get-at 1 ] ;
     local properties = [ $(build-request).get-at 2 ] ;
@@ -895,7 +892,7 @@ local rule should-clean-project ( project )
             local os        = [ modules.peek : OS OSPLAT JAMUNAME ] "" ;
             local timestamp = [ modules.peek : JAMDATE ] ;
             local cwd       = [ PWD ] ;
-            local command   = $(.argv) ;
+            local command   = [ modules.peek : ARGV ] ;
             local bb-version = [ version.boost-build ] ;
             .header on $(xml-file) =
                 "<?xml version=\"1.0\" encoding=\"utf-8\"?>"

src/build/build-request.jam

@@ -1,4 +1,5 @@
 # Copyright 2002 Dave Abrahams
+# Copyright 2026 René Ferdinand Rivera Morell
 # Distributed under the Boost Software License, Version 1.0.
 # (See accompanying file LICENSE.txt or https://www.bfgroup.xyz/b2/LICENSE.txt)
 
@@ -172,37 +173,24 @@ local rule looks-like-implicit-value ( v )
 # specified in the command line, and second is the set of requested build
 # properties. Expects all the project files to already be loaded.
 #
-rule from-command-line ( command-line * )
+rule from-extra-args ( extra-args * )
 {
     local targets ;
     local properties ;
 
-    command-line = $(command-line[2-]) ;
-    local skip-next = ;
-    for local e in $(command-line)
+    for local e in $(extra-args)
     {
-        if $(skip-next)
+        # Build request spec either has "=" in it or completely consists of
+        # implicit feature values.
+        local fs = feature-space ;
+        if [ MATCH "(.+=.*)" : $(e) ]
+            || [ looks-like-implicit-value $(e:D=) : $(feature-space) ]
         {
-            skip-next = ;
+            properties += $(e) ;
         }
-        else if ! [ MATCH ^(-) : $(e) ]
+        else if $(e)
         {
-            # Build request spec either has "=" in it or completely consists of
-            # implicit feature values.
-            local fs = feature-space ;
-            if [ MATCH "(.+=.*)" : $(e) ]
-                || [ looks-like-implicit-value $(e:D=) : $(feature-space) ]
-            {
-                properties += $(e) ;
-            }
-            else if $(e)
-            {
-                targets += $(e) ;
-            }
-        }
-        else if [ MATCH "^(-[-ldjfsto])$" : $(e) ]
-        {
-            skip-next = true ;
+            targets += $(e) ;
         }
     }
     return [ new vector
@@ -361,52 +349,52 @@ rule __test__ ( )
 
     try ;
     {
-        r = [ build-request.from-command-line bjam gcc/debug runtime-link=dynamic/static ] ;
+        r = [ build-request.from-extra-args gcc/debug runtime-link=dynamic/static ] ;
         build-request.convert-command-line-elements [ $(r).get-at 2 ] ;
     }
     catch \"static\" is not an implicit feature value ;
 
-    r = [ build-request.from-command-line bjam debug runtime-link=dynamic ] ;
+    r = [ build-request.from-extra-args debug runtime-link=dynamic ] ;
     assert.equal [ $(r).get-at 1 ] : ;
     assert.equal [ $(r).get-at 2 ] : debug runtime-link=dynamic ;
 
     assert.equal
         [ build-request.convert-command-line-elements debug runtime-link=dynamic ]
         : debug <runtime-link>dynamic ;
 
-    r = [ build-request.from-command-line bjam -d2 --debug debug target runtime-link=dynamic ] ;
+    r = [ build-request.from-extra-args debug target runtime-link=dynamic ] ;
     assert.equal [ $(r).get-at 1 ] : target ;
     assert.equal [ $(r).get-at 2 ] : debug runtime-link=dynamic ;
 
     assert.equal
         [ build-request.convert-command-line-elements debug runtime-link=dynamic ]
         : debug <runtime-link>dynamic ;
 
-    r = [ build-request.from-command-line bjam debug runtime-link=dynamic,static ] ;
+    r = [ build-request.from-extra-args debug runtime-link=dynamic,static ] ;
     assert.equal [ $(r).get-at 1 ] : ;
     assert.equal [ $(r).get-at 2 ] : debug runtime-link=dynamic,static ;
 
     assert.equal
         [ build-request.convert-command-line-elements debug runtime-link=dynamic,static ]
         : debug <runtime-link>dynamic <runtime-link>static ;
 
-    r = [ build-request.from-command-line bjam debug gcc/runtime-link=dynamic,static ] ;
+    r = [ build-request.from-extra-args debug gcc/runtime-link=dynamic,static ] ;
     assert.equal [ $(r).get-at 1 ] : ;
     assert.equal [ $(r).get-at 2 ] : debug gcc/runtime-link=dynamic,static ;
 
     assert.equal
         [ build-request.convert-command-line-elements debug gcc/runtime-link=dynamic,static ]
         : debug gcc/<runtime-link>dynamic gcc/<runtime-link>static ;
 
-    r = [ build-request.from-command-line bjam msvc gcc,borland/runtime-link=static ] ;
+    r = [ build-request.from-extra-args msvc gcc,borland/runtime-link=static ] ;
     assert.equal [ $(r).get-at 1 ] : ;
     assert.equal [ $(r).get-at 2 ] : msvc gcc,borland/runtime-link=static ;
 
     assert.equal
         [ build-request.convert-command-line-elements msvc gcc,borland/runtime-link=static ]
         : msvc gcc/<runtime-link>static borland/<runtime-link>static ;
 
-    r = [ build-request.from-command-line bjam gcc-3.0 ] ;
+    r = [ build-request.from-extra-args gcc-3.0 ] ;
     assert.equal [ $(r).get-at 1 ] : ;
     assert.equal [ $(r).get-at 2 ] : gcc-3.0 ;
 

src/engine/jam.cpp

@@ -14,7 +14,7 @@
 
 /* This file is ALSO:
  * Copyright 2001-2004 David Abrahams.
- * Copyright 2018-2025 Rene Rivera
+ * Copyright 2018-2026 René Ferdinand Rivera Morell
  * Distributed under the Boost Software License, Version 1.0.
  * (See accompanying file LICENSE.txt or copy at
  * https://www.bfgroup.xyz/b2/LICENSE.txt)
@@ -222,6 +222,18 @@ struct args_data
 	std::string out_filename;
 } args_data;
 
+namespace b2 { namespace args {
+list_ref extra_args()
+{
+	list_ref result;
+	for (auto const & a : args_data.extra_args)
+	{
+		result.push_back(a);
+	}
+	return result;
+}
+}} // namespace b2::args
+
 int guarded_main(int argc, char * argv[])
 {
 	int status = 0;
@@ -230,8 +242,18 @@ int guarded_main(int argc, char * argv[])
 	module_t * environ_module;
 	b2::system_info sys_info;
 
+	// On each reparse of arguments we need to reset the data we collect of
+	// arguments.
+	b2::args::set_reparse_callback([]() { args_data = { }; });
+
 	auto & cli = b2::args::lyra_cli().relaxed();
 
+	// In specifying options we do validation at the time of parsing the option
+	// value. We need to do it this way as we use the relaxed option parser
+	// which will ignore options it fails on, for example by not being able to
+	// convert the value or otherwise. Hence we do our own parse checks and
+	// failures.
+
 	cli |= lyra::arg(args_data.extra_args, "request")
 			   .help("Targets, requirements, etc.")
 			   .choices([](const std::string & v) {
@@ -266,40 +288,54 @@ int guarded_main(int argc, char * argv[])
 	/* Undocumented -p3 (acts like both -p1 -p2) means separate pipe action
 	 * stdout and stderr.
 	 */
-	cli |= lyra::opt(globs.pipe_action, "x")
+	cli |= lyra::opt(
+		[](const std::string & v) {
+			if (!lyra::detail::from_string(v, globs.pipe_action)
+				|| globs.pipe_action < 0 || globs.pipe_action > 3)
+			{
+				out_printf("error: Invalid value for the '-p' option.\n");
+				b2::clean_exit(EXITBAD);
+			}
+		},
+		"x")
 			   .name("-p")
 			   .help(
 				   "x=0, pipes action stdout and stderr "
-				   "merged into action output.")
-			   .choices([](int val) { return 0 <= val && val <= 3; });
+				   "merged into action output.");
 
 	cli |= lyra::opt(globs.quitquick)
 			   .name("-q")
 			   .help("Quit quickly as soon as a target fails.");
 
 	cli |= lyra::opt(
 		[](const std::string & v) {
-			globs.quitquick = (v == "on" || v == "yes" || v == "true");
+			if (v != "on" && v != "yes" && v != "true" && v != "off"
+				&& v != "no" && v != "false")
+			{
+				out_printf(
+					"error: Invalid value for the '--keep-going' option.\n");
+				b2::clean_exit(EXITBAD);
+			}
+			globs.quitquick = (v == "off" || v == "no" || v == "false");
 		},
 		"x")
 			   .name("--keep-going")
 			   .help(
 				   "Specify if we continue to build after failures or not "
-				   "(--keep-going=no is equivalent to -q)")
-			   .choices("on", "yes", "true", "off", "no", "false");
+				   "(--keep-going=no is equivalent to -q)");
 
-	cli |= lyra::opt(globs.jobs, "x")
+	cli |= lyra::opt(
+		[](const std::string & v) {
+			if (!lyra::detail::from_string(v, globs.jobs) || globs.jobs < 1)
+			{
+				out_printf("error: Invalid value for the '-j' option.\n");
+				b2::clean_exit(EXITBAD);
+			}
+		},
+		"x")
 			   .name("-j")
 			   .name("--jobs")
-			   .help("Run up to x shell commands concurrently.")
-			   .choices([](int val) {
-				   if (val < 1)
-				   {
-					   err_printf("Invalid value for the '-j' option.\n");
-					   b2::clean_exit(EXITBAD);
-				   }
-				   return true;
-			   });
+			   .help("Run up to x shell commands concurrently.");
 
 	cli |= lyra::opt(globs.newestfirst)
 			   .name("-g")
@@ -410,7 +446,6 @@ int guarded_main(int argc, char * argv[])
 		argv[2] = argv[0];
 		arg_v = argv = (argv + 2);
 		globs.debug_interface = global_config::debug_interface_child;
-		args_data = {};
 		b2::args::set_args(arg_c, arg_v);
 		b2::args::process_args(true);
 	}
@@ -429,7 +464,6 @@ int guarded_main(int argc, char * argv[])
 			arg_c = argc = debug_child_data.argc;
 			arg_v = argv = (char **)debug_child_data.argv;
 			globs.debug_interface = global_config::debug_interface_child;
-			args_data = {};
 			b2::args::set_args(arg_c, arg_v);
 			b2::args::process_args(true);
 		}
@@ -605,7 +639,6 @@ int guarded_main(int argc, char * argv[])
 		}
 
 		// Process options.
-		args_data = {};
 		b2::args::set_args(arg_c, arg_v);
 		b2::args::process_args();
 
@@ -675,7 +708,7 @@ struct SetConsoleCodepage
 	UINT orig_console_output_cp = 0;
 };
 
-static const SetConsoleCodepage g_console_codepage_setter {};
+static const SetConsoleCodepage g_console_codepage_setter { };
 
 } // namespace
 #endif

src/engine/mod_args.cpp

@@ -1,5 +1,5 @@
 /*
-Copyright 2024 René Ferdinand Rivera Morell
+Copyright 2024-2026 René Ferdinand Rivera Morell
 Distributed under the Boost Software License, Version 1.0.
 (See accompanying file LICENSE.txt or https://www.bfgroup.xyz/b2/LICENSE.txt)
 */
@@ -12,6 +12,7 @@ Distributed under the Boost Software License, Version 1.0.
 #include "startup.h"
 #include "value.h"
 
+#include <functional>
 #include <memory>
 #include <sstream>
 #include <string>
@@ -34,6 +35,7 @@ struct args_reg
 	std::unordered_map<std::string, std::shared_ptr<list_ref>> options;
 	std::vector<std::string> args;
 	bool need_reparse = true;
+	std::function<void()> reparse_callback;
 
 	args_reg() { cli.style_print_short_first(); }
 
@@ -48,6 +50,7 @@ struct args_reg
 
 	void reparse()
 	{
+		if (reparse_callback) reparse_callback();
 		need_reparse = false;
 		// We can call this multiple times. Hence we go back to a clean state
 		// on the collected values to get reproducible parsing.
@@ -100,7 +103,7 @@ struct args_reg
 			return globs.display_help ? list_ref("true") : list_ref();
 		if (name == "debug-configuration")
 			return globs.debug_configuration ? list_ref("true") : list_ref();
-		return {};
+		return { };
 	}
 
 	bool has_opt(const value_ref & name) { return (options.count(name) > 0); }
@@ -127,8 +130,7 @@ lyra::cli & lyra_cli() { return args_reg::ref().cli; }
 void process_args(bool silent)
 {
 	args_reg::ref().reparse();
-	if (silent)
-		return;
+	if (silent) return;
 
 	std::ostringstream out;
 	int return_code = EXITOK;
@@ -149,6 +151,11 @@ void process_args(bool silent)
 	}
 }
 
+void set_reparse_callback(std::function<void()> const & f)
+{
+	args_reg::ref().reparse_callback = f;
+}
+
 const char * args_module::init_code = R"jam(
 rule __test__ ( )
 {