With the recent PR proposing adding arguments to user-defined script/batch files (MSVC_USE_SCRIPT can pass in args #4102), perhaps it is worthwhile to revive the discussion had earlier in PR #4063.
There are three potential enhancements to msvc usage that would enhance the user experience:
- Passing user-defined arguments to the user-defined script specified via MSVC_USE_SCRIPT,
- Passing user-defined arguments when using the normal SCons msvc detection,
- Passing a user-defined environment (i.e., ENV-like) dictionary to be used in lieu of SCons msvc detection or user-specified script.
In the interest of full disclosure: the author also has a code branch ready implementing Items 1 and 3 above but has not updated the requisite documentation yet. Implementation of Item 2 in the current SCons code base is not as straightforward as Items 1 and 3 and has been deferred to a later date.
The first two items make available to an SCons end-user all of the msvc batch file options available to command-line users. The first two items effectively provide a "universal escape hatch" which also reduces the burden on SCons development to "catch-up" with currently unsupported features and/or any new shiny features that may be added to msvc batch files in the future.
The third item is a middle ground between the all (SCons msvc detection via restricted system execution environment) or nothing (user-populated environment) currently offered. While the utility may not be readily apparent, the reader is encouraged to keep an open mind before rushing to a snap judgment. Additional support material will be provided in future posts.
The user-provided content for Item 1 (script arguments) and Item 2 (pass-through script arguments) are not likely to be the same as "native" features are added to SCons. In the current SCons implementation, the store/UWP argument is available via the MSVC_UWP_APP variable while a user-defined script would need to use the "store" argument. If toolset support were added to SCons, the script argument(s) would still need the toolset specification while the pass-through argument(s) would not.
It would likely be best if there are two distinct environment variables for user-defined script arguments and user-defined pass-through arguments:
- MSVC_USE_SCRIPT_ARGS for user-defined script arguments
- MSVC_ADD_ARGS for pass-through arguments (or possibly MSVC_PASS_ARGS)
Item 1 - MSVC_USE_SCRIPT_ARGS rationale:
- MSVC_USE_SCRIPT_ARGS is only used when MSVC_USE_SCRIPT is defined.
- MSVC_USE_SCRIPT_ARGS would follow MSVC_USE_SCRIPT in the documentation.
- the usage restriction is fairly apparent by the name.
- allowing a (scalar) string or list of strings is a useful user-facing enhancement.
Item 2 - MSVC_ADD_ARGS or MSVC_PASS_ARGS notes:
- This is a little trickier to find a decent variable name. The trade-off is finding a variable name that is both descriptive and cannot be easily confused with the name of the user-defined script arguments variable.
Item 3 - ENV-like dictionary notes:
- There is a wide gap between using SCons msvc detection and bypassing detection altogether: it is tantamount to an all or nothing proposition.
- Passing in a user-defined ENV-like dictionary allows for:
- alternate user-side msvc detection which could overcome any limitations imposed by the current restricted SCons system execution environment,
- alternate user-side msvc detection which can provide features not available in the current SCons implementation (e.g., selection of msvc preview versions, selection by msvc toolset, selection by msvc buildtools, selection by msvc runtime, alternate msvc selection algorithms, etc.),
- user-side msvc environment caching outside of SCons, and
- provides a "testbed" hook for rapid prototyping new features outside the SCons code base.
- Additional rationale and/or examples demonstrating the utility will have to be provided in future posts.
The relevant posts from closed PR #4063 are quoted here for convenience (note that the proposed implementation details have changed since originally posted):
Originally posted by @jcbrill in #4063 (comment)
A bit off topic: an enhancement that I was planning on proposing once this was resolved is the addition of a user specified script argument: MSVC_USE_SCRIPT_ARG to be used in conjunction with MSVC_USE_SCRIPT. This would allow any flags recognized by the msvc batch files to be passed along with the script name.
For example, this would allow the following which currently can't be achieved directly in a SConstruct/SConscript file without resorting to a user-defined batch file (VS2022 Preview, amd64 target, toolset 14.2, sdk 10.0.19041.0):
use_version = '14.3'
use_script = 'c:\\software\\msvs143bt-pre\\VC\\Auxiliary\\Build\\vcvarsall.bat'
use_script_arg = 'amd64 --vcvars_ver=14.2 10.0.19041.0'
env = Environment(MSVC_VERSION = use_version, MSVC_USE_SCRIPT = use_script, MSVC_USE_SCRIPT_ARG = use_script_arg)
If something like this were added, there would be no good way to validate the user provided script argument other than call subprocess. In this case it could be harder to distinguish between a user error and a system error.
Unfortunately, current msvc logic will not pick up batch file errors correctly (unless cl.exe is not added to the path). There is room for improvement in this area.
Originally posted by @jcbrill in #4063 (comment)
Further off-topic: If a user script argument were allowed and an overloaded version when MSVC_USE_SCRIPT=None allowing a dictionary to be passed, it is possible in user space to unify/resolve most of the outstanding msvc issues/requests (preview, toolsets, components, etc).
Working SConstruct fragments via script argument enhancements:
from vsdetect import scons_msvc_byinstance as scons_msvc
# SCons runs batch file: MSVC_USE_SCRIPT (string) and MSVC_USE_SCRIPT_ARG (string)
env = Environment(**scons_msvc(MSVC_VERSION='14.3', MSVC_PREVIEW=True, MSVC_COMPONENT='BuildTools', MSVC_TOOLSET='14.2', TARGET_ARCH='amd64', MSVC_ADD_ARG='10.0.19041.0', run_script=False))
# user runs batch file: MSVC_USE_SCRIPT (None) and MSVC_USE_SCRIPT_ARG (dictionary)
env = Environment(**scons_msvc(MSVC_VERSION='14.3', MSVC_PREVIEW=True, MSVC_COMPONENT='BuildTools', MSVC_TOOLSET='14.2', TARGET_ARCH='amd64', MSVC_ADD_ARG='10.0.19041.0', run_script=True))
# lookup by c runtime
env = Environment(**scons_msvc(MSVC_RUNTIME='140', MSVC_PREVIEW=True, MSVC_COMPONENT='BuildTools', MSVC_TOOLSET='14.2', TARGET_ARCH='amd64', MSVC_ADD_ARG='10.0.19041.0', run_script=True))
# lookup by toolset
env = Environment(**scons_msvc(MSVC_TOOLSET='14.2', MSVC_PREVIEW=True, MSVC_COMPONENT='BuildTools', TARGET_ARCH='amd64', MSVC_ADD_ARG='10.0.19041.0', run_script=True))
This would also allow user-side caching if the overloaded dictionary support were added.
Originally posted by @bdbaddog in #4063 (comment)
@jcbrill - can you file an issue about additional arguments to vcvarsall.bat, I have some additional thoughts which would be good to capture in such an issue rather than here.
For discussion purposes, the author's proposed implementation for Items 1 and 3 is:
class MSVCScriptArgsError(VisualCException):
pass
...
def msvc_setup_env(env):
...
use_script = env.get('MSVC_USE_SCRIPT', True)
if SCons.Util.is_String(use_script):
use_script_orig = use_script
use_script = use_script.strip()
if not os.path.exists(use_script):
raise MSVCScriptNotFound('Script specified by MSVC_USE_SCRIPT not found: "{}"'.format(use_script_orig))
use_script_args = env.get('MSVC_USE_SCRIPT_ARGS', None)
if use_script_args is not None:
use_script_args_orig = use_script_args
use_script_args_list = SCons.Util.flatten(use_script_args)
try:
use_script_args = ' '.join(use_script_args_list)
except TypeError as e:
error_msg = 'Script arguments specified by MSVC_USE_SCRIPT_ARGS must be a string or a sequence of strings:\nvalue: {}\n{}'.format(use_script_args_orig, str(e))
raise MSVCScriptArgsError(error_msg)
use_script_args = use_script_args.strip()
if not use_script_args:
use_script_args = None
debug('MSVC_USE_SCRIPT String %s %s', repr(use_script), repr(use_script_args))
d = script_env(use_script, use_script_args)
elif SCons.Util.is_Dict(use_script):
d = use_script
debug('MSVC_USE_SCRIPT Dict %s', d)
elif use_script:
d = msvc_find_valid_batch_script(env,version)
debug('MSVC_USE_SCRIPT True %s', d)
if not d:
return d
else:
debug('MSVC_USE_SCRIPT False')
warn_msg = "MSVC_USE_SCRIPT set to False, assuming environment " \
"set correctly."
SCons.Warnings.warn(SCons.Warnings.VisualCMissingWarning, warn_msg)
return None
...
Comments:
- There is a minor change to the existing implementation of the MSVCScriptNotFound error message: the original user-specified string is saved and then used in the error message rather than using the "stripped" version. This probably should have been in the author's original submission/implementation.
- The debug messages have been slightly changed to more-readily identify the code path in the debug output (e.g., "MSVC_USE_SCRIPT 1" has been changed to "MSVC_USE_SCRIPT String").
- MSVC_USE_SCRIPT_ARGS can be a scalar string or a list of strings. The utility function flatten is used to construct a list and type error exceptions due to the string join are reported to the user with the original user-provided content. The user-provided argument is type checked immediately with a user-friendly error message. The author finds that in some instances, a list is more useful from an end-user standpoint (e.g., comments after each argument in a list formatted across several lines).
- MSVC_USE_SCRIPT is "overloaded" to allow a dictionary. This dictionary is intended to contain similar content as the SCons msvc detection (e.g., INCLUDE, LIB, LIBPATH, PATH, etc.). This dictionary is processed in approximately the same manner as the normal SCons msvc detection. An empty dictionary is intentionally passed through to the validation check that cl.exe is found on the derived system path.
With the recent PR proposing adding arguments to user-defined script/batch files (MSVC_USE_SCRIPT can pass in args #4102), perhaps it is worthwhile to revive the discussion had earlier in PR #4063.
There are three potential enhancements to msvc usage that would enhance the user experience:
In the interest of full disclosure: the author also has a code branch ready implementing Items 1 and 3 above but has not updated the requisite documentation yet. Implementation of Item 2 in the current SCons code base is not as straightforward as Items 1 and 3 and has been deferred to a later date.
The first two items make available to an SCons end-user all of the msvc batch file options available to command-line users. The first two items effectively provide a "universal escape hatch" which also reduces the burden on SCons development to "catch-up" with currently unsupported features and/or any new shiny features that may be added to msvc batch files in the future.
The third item is a middle ground between the all (SCons msvc detection via restricted system execution environment) or nothing (user-populated environment) currently offered. While the utility may not be readily apparent, the reader is encouraged to keep an open mind before rushing to a snap judgment. Additional support material will be provided in future posts.
The user-provided content for Item 1 (script arguments) and Item 2 (pass-through script arguments) are not likely to be the same as "native" features are added to SCons. In the current SCons implementation, the store/UWP argument is available via the MSVC_UWP_APP variable while a user-defined script would need to use the "store" argument. If toolset support were added to SCons, the script argument(s) would still need the toolset specification while the pass-through argument(s) would not.
It would likely be best if there are two distinct environment variables for user-defined script arguments and user-defined pass-through arguments:
Item 1 - MSVC_USE_SCRIPT_ARGS rationale:
Item 2 - MSVC_ADD_ARGS or MSVC_PASS_ARGS notes:
Item 3 - ENV-like dictionary notes:
The relevant posts from closed PR #4063 are quoted here for convenience (note that the proposed implementation details have changed since originally posted):
For discussion purposes, the author's proposed implementation for Items 1 and 3 is:
Comments: