Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

msvc initialization can blow up if nonexistant or not installed TARGET_ARCH requested #4312

Closed
mwichmann opened this issue Mar 2, 2023 · 35 comments
Labels
MSVC Microsoft Visual C++ Support

Comments

@mwichmann
Copy link
Collaborator

mwichmann commented Mar 2, 2023

The scenario is: on a lightly provisioned (but working) Windows instance with VS2022, test/MSVC/TARGET_ARCH.py blew up scons with an IndexError indexing into an empty list. In fact, the third and fourth tests in that file both do this, the specific lines are these:

env = Environment(tools=['default', 'msvc'], TARGET_ARCH = 'arm', MSVC_VERSION='11.0')

env = Environment(tools=['default', 'msvc'], TARGET_ARCH = 'arm64', MSVC_VERSION='11.0')

It takes a little work to generate a backtrace (I ended up pulling it into a standalone SConstruct and running outside the test framework), but the output including trace looks like this, trimmed down. I'll attach the full trace also, but I don't think the rest of it is interesting:

scons: warning: unsupported host, target combination ('amd64', 'arm64') for MSVC version 11.0
File "C:\Users\mats\github\scons\TRY.THIS.4", line 1, in <module>

scons: warning: MSVC version '11.0' was not found.
  No versions of the MSVC compiler were found.

  Visual Studio C/C++ compilers may not be set correctly
File "C:\Users\mats\github\scons\TRY.THIS.4", line 1, in <module>
scons: *** [.] IndexError : list index out of range
scons: internal stack trace:
  File "C:\Users\mats\github\scons\scripts\..\SCons\Taskmaster\Job.py", line 219, in start
    task.prepare()
....
  File "C:\Users\mats\github\scons\scripts\..\SCons\Tool\mslink.py", line 312, in generate
    msvc_setup_env_once(env, tool=tool_name)
  File "C:\Users\mats\github\scons\scripts\..\SCons\Tool\MSCommon\vc.py", line 1113, in msvc_setup_env_once
    req_tools = MSVC.SetupEnvDefault.register_iserror(env, tool, msvc_exists)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\mats\github\scons\scripts\..\SCons\Tool\MSCommon\MSVC\SetupEnvDefault.py", line 191, in register_iserror
    re_nchar_min, re_tools_min = _Data.default_tools_re_list[-1]
                                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^

Adding a little check shows in this circumstance _Data.default_tools_re_list is empty, seems like a guard against this case is needed.

msvciniterr.txt

@mwichmann mwichmann added the MSVC Microsoft Visual C++ Support label Mar 2, 2023
@mwichmann
Copy link
Collaborator Author

@jcbrill I can make the test pass by just wrapping the block of code in lines 191-203 in a if _Data.default_tools_re_list: but wanted to know if there was "more to it than this" in your opinion....

@jcbrill
Copy link
Contributor

jcbrill commented Mar 2, 2023

@mwichmann I'll look into it.

At first glance, I'm a little concerned about why it reports that no versions of the MSVC compiler were found:

scons: warning: unsupported host, target combination ('amd64', 'arm64') for MSVC version 11.0
File "C:\Users\mats\github\scons\TRY.THIS.4", line 1, in <module>

scons: warning: MSVC version '11.0' was not found.
  No versions of the MSVC compiler were found.
  Visual Studio C/C++ compilers may not be set correctly

@jcbrill
Copy link
Contributor

jcbrill commented Mar 2, 2023

@mwichmann If you have it running "stand-alone", is there any chance you could enable the MSCommon debugging (i.e., set SCONS_MSCOMMON_DEBUG=msvciniterr-debug.txt) and post the log?

@mwichmann
Copy link
Collaborator Author

Was just typing that I hadn't captured the debug.... will do that

@bdbaddog bdbaddog changed the title msvc initialization can blow up if nonexistent target requested msvc initialization can blow up if nonexistant or not installed TARGET_ARCH requested Mar 2, 2023
@mwichmann
Copy link
Collaborator Author

debug log added

debug.txt

@mwichmann
Copy link
Collaborator Author

Meanwhile, it's certainly finding a compiler - 14.3 (I can add the cache file too if useful)... there was no arm or arm64 support in 11.0, but I assume we're not smart enough to know that much?

@jcbrill
Copy link
Contributor

jcbrill commented Mar 2, 2023

Does the toolset 14.35.32215 folder exist and is it populated?
C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.35.32215\bin\Hostx64\arm64\cl.exe

@mwichmann
Copy link
Collaborator Author

No... in the directory a bit up, Hostx64 and Hostx86 only have x64 and x86 subdirectories, not arm or amd64. Nor does my fully built-up system, although it does have arm64 dirs in the 2019 install...

@jcbrill
Copy link
Contributor

jcbrill commented Mar 2, 2023

Still not sure what is happening. The log for 14.3 reports:

00397ms:MSCommon/vc.py:get_installed_vcs#945: trying to find VC 14.3
00397ms:MSCommon/vc.py:find_vc_pdir_vswhere#677: VSWHERE: C:\Program Files (x86)\Microsoft Visual Studio\Installer\vswhere.exe
00397ms:MSCommon/vc.py:find_vc_pdir_vswhere#682: running: ['C:\\Program Files (x86)\\Microsoft Visual Studio\\Installer\\vswhere.exe', '-version', '[17.0, 18.0)', '-property', 'installationPath']
00537ms:MSCommon/vc.py:find_vc_pdir#739: VC found: '14.3'
00537ms:MSCommon/vc.py:get_installed_vcs#949: found VC 14.3
00537ms:MSCommon/vc.py:get_host_target#473: HOST_ARCH:x86_64
00537ms:MSCommon/vc.py:get_host_target#481: TARGET_ARCH:arm64
00537ms:MSCommon/vc.py:_check_cl_exists_in_vc_dir#851: host_platform amd64, target_platform arm64 host_target_list [('amd64', 'arm64'), ('x86', 'arm64')]
00537ms:MSCommon/vc.py:_check_cl_exists_in_vc_dir#876: host platform amd64, target platform arm64 for version 14.3
00537ms:MSCommon/vc.py:_check_cl_exists_in_vc_dir#885: checking for cl.exe at C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.35.32215\bin\Hostx64\arm64\cl.exe
00537ms:MSCommon/vc.py:_check_cl_exists_in_vc_dir#876: host platform x86, target platform arm64 for version 14.3
00537ms:MSCommon/vc.py:_check_cl_exists_in_vc_dir#885: checking for cl.exe at C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.35.32215\bin\Hostx86\arm64\cl.exe
00537ms:MSCommon/vc.py:get_installed_vcs#953: no compiler found 14.3

It appears like it is only checking 14.3 with a given TARGET_ARCH='arm64'.

I'm a little rusty evidently...

EDIT: changed 14.2 to 14.3.

@jcbrill
Copy link
Contributor

jcbrill commented Mar 3, 2023

@mwichmann The root problem is with the construction of the installed VCS cache list (i.e., __INSTALLED_VCS_RUN) in vc.py.

The installed VCS list is cached (i.e., computed once). However, there are indirect dependencies on values from the environment. Most notably, the TARGET_ARCH.

In some of the tests, when the installed VCS cache list is constructed, the TARGET_ARCH (e.g., arm and arm64) does not exist in VS2022 and therefore there are no detected VCS.

The quick fix below uses an undefined TARGET_ARCH during the initial construction and then restores the original value prior to exit. This should be enough to accurately detect all installations with at least one usable HOST/TARGET combination.

I'm not convinced that a user specified VSWHERE via the environment is guaranteed to be used in light of the installed VCS cache. The VSWHERE specification might only work during the initial construction. I could be really wrong though.

This is a quick fix for the immediate problem:
https://github.com/jcbrill/scons/tree/jbrill-msvc-vcscache

The _UNDEFINED object was moved to the top of the file as it was now being used in code "above" the original declaration.

Still not exactly sure how/why the original symptom is manifested yet. It seems to have disappeared with the quick fix.

@jcbrill
Copy link
Contributor

jcbrill commented Mar 3, 2023

When testing on my development box which happens to have MSVC 11.0 Express installed, the arm batch file appears to succeed.

However, the debug log for the batch file invocation indicates usage errors but no exception is raised because cl.exe is found on the path. I believe it to be the correct cl.exe as well.

Unfortunately, when reviewing the relevant code, there is a bug involving the host platform and likely just express versions due to an error of omission of an additional check for existence. I need to test that an additional check does not break anything that is currently working as expected.

@jcbrill
Copy link
Contributor

jcbrill commented Mar 4, 2023

@mwichmann updated branch available at:
https://github.com/jcbrill/scons/tree/jbrill-msvc-vcscache

The implementation now passes the msvc/msvs tests (79 tests, 5 skipped) on my development box and in a VMWare virtual machine with a minimal VS2022 (x86 and amd64 for latest 14.3 toolset) as described above (79 tests, 15 skipped).

Changes:

  • The issue with the installed VCS cache list has been resolved and tweaked from the code listing above to handle the test suite when env=None is passed.
  • A test for the existence of cl.exe for the host/target combination was added to the batch file existence check for VS6 to VS2015 in find_batch_file. There may be issues with VS2017+. I know there is a looming problem with VS2022 on ARM64 when trying to configure the ARM64 native batch files.
  • The sdk_batch_file query was moved into its own function and the sdk script invocation(s) are no longer interleaved with the vc script invocations. The sdk script invocations are only evaluated if all vc script host/target combinations have been exhausted without finding a valid vc script. The existence of cl.exe and the batch file combined is a "stronger" guarantee than the batch file existence alone.

@mwichmann
Copy link
Collaborator Author

I don't know if it's relevant to the issue under investigation here (and sorry haven't caught up with the above few comments), but came across these two errors in a pylint report:

SCons/Tool/MSCommon/MSVC/SetupEnvDefault.py:87:19: E1135: Value '_Data.msvc_tools' doesn't support membership test (unsupported-membership-test)
SCons/Tool/MSCommon/MSVC/SetupEnvDefault.py:100:58: E1135: Value '_Data.msvc_tools' doesn't support membership test (unsupported-membership-test)

@jcbrill
Copy link
Contributor

jcbrill commented Mar 4, 2023

(and sorry haven't caught up with the above few comments),

No worries.

_Data.msvc_tools is initialized to None. It is later assigned a set(). In between there is code in functions that tests for membership.

I'm guessing that might be why pylint is complaining. I'll take at look at the call sequence.

I believe that the issue under investigation is the result of an internal state being constructed improperly (i.e., the installed vcs cache) which causes other issues.

This morning I added the requisite configuration data structures for ARM64 hosts.
Good news: sconstruct builds are working as expected.
Bad news: due to some code movement and renamed configuration data structures 4 tests in the test suite now fail.

@mwichmann
Copy link
Collaborator Author

_Data.msvc_tools is initialized to None... I'm guessing that might be why pylint is complaining

Yes, it has a LOT of complaints on the SCons codebase where it can't be 100% sure that something that starts out with a sentinel None actually arrives at a particular point with no chance of still being None. Most of those are classes, where SCons tries to defer creating an instance until it's needed, so there's a bunch of instance of Foo has no Bar member because it thinks it could arrive there as None. Sigh.

@mwichmann
Copy link
Collaborator Author

(and sorry haven't caught up with the above few comments),

No worries.

_Data.msvc_tools is initialized to None. It is later assigned a set(). In between there is code in functions that tests for membership.

I'm guessing that might be why pylint is complaining. I'll take at look at the call sequence.

I believe that the issue under investigation is the result of an internal state being constructed improperly (i.e., the installed vcs cache) which causes other issues.

This morning I added the requisite configuration data structures for ARM64 hosts. Good news: sconstruct builds are working as expected. Bad news: due to some code movement and renamed configuration data structures 4 tests in the test suite now fail.

Ah. yeah, that's a pain when tests start failing, even when you made things better. I did try running that test with no caching, FWIW, which didn't seem to have an impact.

@jcbrill
Copy link
Contributor

jcbrill commented Mar 4, 2023

According to git blame, the installed vcs cache was added 13 years ago. Back then, there weren't too many options for targets.

The code used to construct the installed vcs list indirectly evaluates TARGET_ARCH. This is BAD for a cached value as the value of TARGET_ARCH can certainly change.

For the tests that fail, the installed vcs list is using arm and arm64. Since they are not installed, it believes that VS2022 is not installed. This may have downstream consequences.

I'm not sure why it is failing now unless there was some change that might have affected the order in which environments are initialized (i.e., default vs user).

For the initial construction TARGET_ARCH=None or not defined at all is best. The current "fix" is to forcibly delete the TARGET_ARCH, construct the list, and then restore the original value.

@jcbrill
Copy link
Contributor

jcbrill commented Mar 4, 2023

Unrelated: the final check to see if cl.exe is in the environment path appears to consider the os.environ['PATH'] if cl.exe is not found in the environment.

This could not possibly be a good idea. I think in vc.py is should be env.WhereIs('cl'). Any thoughts?

SCons\Tool\MSCommon.py (function msvc_setup_env, lines 1280-1288):

   ...
    # final check to issue a warning if the compiler is not present
    if not find_program_path(env, 'cl'):
        debug("did not find %s", _CL_EXE_NAME)
        if CONFIG_CACHE:
            propose = "SCONS_CACHE_MSVC_CONFIG caching enabled, remove cache file {} if out of date.".format(CONFIG_CACHE)
        else:
            propose = "It may need to be installed separately with Visual Studio."
        warn_msg = "Could not find MSVC compiler 'cl'. {}".format(propose)
        SCons.Warnings.warn(SCons.Warnings.VisualCMissingWarning, warn_msg)
...

find_program_path fragment:

def find_program_path(env, key_program, default_paths=None, add_path=False) -> Optional[str]:
...
    # First search in the SCons path
    path = env.WhereIs(key_program)
    if path:
        return path

    # Then in the OS path
    path = SCons.Util.WhereIs(key_program)
    if path:
        if add_path:
            env.AppendENVPath('PATH', os.path.dirname(path))
        return path
...

SCons.Util.WhereIs fragment:

    def WhereIs(file, path=None, pathext=None, reject=None) -> Optional[str]:
        if path is None:
            try:
                path = os.environ['PATH']
            except KeyError:
                return None
...

@jcbrill
Copy link
Contributor

jcbrill commented Mar 5, 2023

I can make the test pass by just wrapping the block of code in lines 191-203 in a if _Data.default_tools_re_list: but wanted to know if there was "more to it than this" in your opinion....

@mwichmann It is certainly a GOOD THING to make sure the list has at least one entry. I added the test on my end along with a debug message if the list is empty.

It was an error of omission and hubris to assume the list would be populated upon first use. I've given myself a reprimand and a time-out.

Given that the code incorrectly thought there were no installed VCs, I still don't understand why this issue did not arise in the previous testing concerning no VCs installed. With the detection corrected, the tests pass without the additional check. However, better with the additional test than without it.

Still stress testing the detection fix and the ARM64 host support...

@jcbrill
Copy link
Contributor

jcbrill commented Mar 14, 2023

Currently testing candidate: https://github.com/jcbrill/scons/tree/jbrill-gh4312-fixplus

Previous branch deleted.

@jcbrill
Copy link
Contributor

jcbrill commented Jun 24, 2023

@mwichmann When you have nothing better to do, could you take a peek at #4312 (comment) above?

Another set of eyes would be useful as I have convinced myself that the warning message could be suppressed based on finding (any) cl.exe on the system path (i.e., os.environ) when no cl.exe was found on the detected environment path (i.e., env).

Probably unlikely, but without a valid cl.exe in the environment, possibly destined for failure downstream during the build.

As always, I could be wrong.

@mwichmann
Copy link
Collaborator Author

Not sure quite what the question is... there are a few tools that directly use both WhereIs functions, which does indeed seem odd, and then, as you note, there's find_program_path. Someone thought it was a good idea to fall back to looking in os.environ's path, and it's been that way for a long time, though that seems kind of pointless. @bdbaddog ?

@jcbrill
Copy link
Contributor

jcbrill commented Jun 24, 2023

Why does it matter if cl.exe was found on the system path when it is missing from the constructed environment path?

My guess is that should a user have a cl.exe on the system path and there is a problem with the constructed environment, the build could fail without the warning which in that case seems to be useful/important diagnostic information.

@mwichmann
Copy link
Collaborator Author

Why does it matter if cl.exe was found on the system path when it is missing from the constructed environment path?

My guess is that should a user have a cl.exe on the system path and there is a problem with the constructed environment, the build could fail without the warning which in that case seems to be useful/important diagnostic information.

assuming someone actually does something with that information - which I think, in general, we don't. find_program_path doesn't give us any way to say "hey, I found your tool, but not in a place that's going to be useful".

@jcbrill
Copy link
Contributor

jcbrill commented Jun 24, 2023

assuming someone actually does something with that information - which I think, in general, we don't. find_program_path doesn't give us any way to say "hey, I found your tool, but not in a place that's going to be useful".

There is no guarantee that the tool found in the system environment is the same tool that was requested in scons.

The environment may be configured for one version of msvc (either via command-line batch file or permanently registered environment variables) while the scons msvc version request is different.

@mwichmann
Copy link
Collaborator Author

There is no guarantee that the tool found in the system environment is the same tool that was requested in scons.

The environment may be configured for one version of msvc (either via command-line batch file or permanently registered environment variables) while the scons msvc version request is different.

Indeed, that's a good point. So to me it seems like the behavior of find_program_path is not what we really want.

@bdbaddog
Copy link
Contributor

Not sure quite what the question is... there are a few tools that directly use both WhereIs functions, which does indeed seem odd, and then, as you note, there's find_program_path. Someone thought it was a good idea to fall back to looking in os.environ's path, and it's been that way for a long time, though that seems kind of pointless. @bdbaddog ?

Do you mean in find_program_path()?

@jcbrill
Copy link
Contributor

jcbrill commented Jun 25, 2023

Do you mean in find_program_path()?

See #4312 (comment) above for original question and code fragments.

The final check to issue a warning if the compiler is not present in msvc_setup_env uses the function find_program_path to search for cl.exe. Function find_program_path will check if the program is found in the scons environment path, and if not found, whether the program is found on the os.environ path.

In this context, env.WhereIs seems more appropriate than find_program_path.

Why should the warning message should be suppressed if a cl.exe is not in the constructed environment but happens to found on the system path?

The cl.exe on the system path may have no relation to the msvc version requested.

Am I missing something?

@bdbaddog
Copy link
Contributor

Do you mean in find_program_path()?

See #4312 (comment) above for original question and code fragments.

The final check to issue a warning if the compiler is not present in msvc_setup_env uses the function find_program_path to search for cl.exe. Function find_program_path will check if the program is found in the scons environment path, and if not found, whether the program is found on the os.environ path.

In this context, env.WhereIs seems more appropriate than find_program_path.

Why should the warning message should be suppressed if a cl.exe is not in the constructed environment but happens to found on the system path?

The cl.exe on the system path may have no relation to the msvc version requested.

Am I missing something?

Gotcha.
It seems like (minimally) the WhereIs() call should not be done by default.. or at least a way to disable.
Also the error message doesn't let the user know where it did actually find cl.exe..

@mwichmann
Copy link
Collaborator Author

I assume there was a reason why this system evolved the way it did, but I don't know it, as a "newcomer". Even env.WhereIs calls SCons.Util.WhereIs evenutally, which has a go at os.environ['PATH'].

@jcbrill
Copy link
Contributor

jcbrill commented Jun 25, 2023

Even env.WhereIs calls SCons.Util.WhereIs evenutally, which has a go at os.environ['PATH']

SCons.Util.WhereIs will only use os.environ['PATH'] if the path argument is None.

env.WhereIs will pass path = self['ENV']['PATH'] to SCons.Util.WhereIs as the path argument as long as ['ENV']['PATH'] does not result in a KeyError. This path will suppress the use of os.environ['PATH'] in SCons.Util.WhereIs.

It seems like (minimally) the WhereIs() call should not be done by default.. or at least a way to disable.

I don't understand.

The scons environment path should be checked for cl.exe. The system path (os.environ) should not be checked for cl.exe.

Three code paths lead to the check: MSVC_USE_SCRIPT (user-defined script) , MSVC_USE_SETTINGS (user-defined dictionary), and MSVC_USE_SCRIPT (MSVC_VERSION request). The only case that does an early exit is when MSVC_USE_SCRIPT is set to False to bypass detection.

All three cases require a cl.exe instance on the environment path or a downstream compile/link command likely will likely fail. In particular, MSVC_USE_SETTINGS doesn't go through any other of the detection code so the check is relatively important.

Also the error message doesn't let the user know where it did actually find cl.exe..

In this context, scons did not find the tool requested.

I'm not sure finding a cl.exe on the system path is useful information as the check is only performed for the three cases listed above and not when a user is actually bypassing detection via MSVC_USE_SCRIPT.

For example, the system environment could be configured for the 2008 x86 cl.exe. The scons build request could be for 2022 arm64. I'm not sure reporting tool found on the system path is necessarily reliable and/or useful.

@bdbaddog
Copy link
Contributor

Even env.WhereIs calls SCons.Util.WhereIs evenutally, which has a go at os.environ['PATH']

SCons.Util.WhereIs will only use os.environ['PATH'] if the path argument is None.

env.WhereIs will pass path = self['ENV']['PATH'] to SCons.Util.WhereIs as the path argument as long as ['ENV']['PATH'] does not result in a KeyError. This path will suppress the use of os.environ['PATH'] in SCons.Util.WhereIs.

It seems like (minimally) the WhereIs() call should not be done by default.. or at least a way to disable.

I don't understand.

The scons environment path should be checked for cl.exe. The system path (os.environ) should not be checked for cl.exe.

Three code paths lead to the check: MSVC_USE_SCRIPT (user-defined script) , MSVC_USE_SETTINGS (user-defined dictionary), and MSVC_USE_SCRIPT (MSVC_VERSION request). The only case that does an early exit is when MSVC_USE_SCRIPT is set to False to bypass detection.

All three cases require a cl.exe instance on the environment path or a downstream compile/link command likely will likely fail. In particular, MSVC_USE_SETTINGS doesn't go through any other of the detection code so the check is relatively important.

Also the error message doesn't let the user know where it did actually find cl.exe..

In this context, scons did not find the tool requested.

I'm not sure finding a cl.exe on the system path is useful information as the check is only performed for the three cases listed above and not when a user is actually bypassing detection via MSVC_USE_SCRIPT.

For example, the system environment could be configured for the 2008 x86 cl.exe. The scons build request could be for 2022 arm64. I'm not sure reporting tool found on the system path is necessarily reliable and/or useful.

Other tools use these functions, and for those, configuration is pretty much as simple as finding the tool.
I'm sure that's what lead to much of that logic.

@mwichmann
Copy link
Collaborator Author

Well, one could argue either way: that a message noting that a desired executable was found, but only in a path that won't be used, has some benefit - or that it's completely useless. What's presumably not useful is to proceed as if all were well if it is found, but not in the execution environment, because then it won't work.

@jcbrill
Copy link
Contributor

jcbrill commented Jun 26, 2023

What's presumably not useful is to proceed as if all were well if it is found, but not in the execution environment, because then it won't work.

+1

@jcbrill
Copy link
Contributor

jcbrill commented Jun 26, 2023

Sigh. Apologies for hijacking.

To wade even deeper into the weeds, there is yet another possible state to consider: the requested tool was not found but there is a tool already in the users env['ENV']['PATH'].

IMHO, there needs to be a warning if the requested tool was not found regardless of whether a tool is found in the env['ENV']['PATH'] (or os.environ['PATH']).

An additional warning should probably be issued if any other tool is found in the environment as there is no easy way to check if the found tool is "the same" as the requested tool.

Bug-riddled pseudo-code to illustrate the possible states:

# BEGIN NEW

def _find_cl_dict(d):
    rval = None
    path = d.get('PATH')
    if path:
        # note: no substitution performed on path
        rval = SCons.Util.WhereIs('cl', path=path, pathext='.exe')
    return rval

def _find_cl_env(env):
    rval = None
    path = env['ENV'].get('PATH')
    if path:
        # note: substitution performed on path
        rval = env.WhereIs('cl', path=path, pathext='.exe')
    return rval

def _cl_warning(d, env):

    cl_request = _find_cl_dict(d)
    if cl_request:
        debug("%s found in d['PATH']", _CL_EXE_NAME)
        return

    # TODO: cache propose needs to be restored

    debug("%s not found in d['PATH']", _CL_EXE_NAME)
    warn_msg = "Could not find requested MSVC compiler 'cl'."
    SCons.Warnings.warn(SCons.Warnings.VisualCMissingWarning, warn_msg)
   
    cl_env = _find_cl_env(env)
    if cl_env:
        debug("%s found in env['ENV']['PATH']", _CL_EXE_NAME)
        # cl.exe not found in d['PATH']
        # cl.exe found in env['ENV']['PATH']
        warn_msg = "MSVC compiler 'cl' found may not be the same as the requested 'cl'."
        SCons.Warnings.warn(SCons.Warnings.VisualCMissingWarning, warn_msg)
	return 
       
    debug("%s not found in env['ENV']['PATH']", _CL_EXE_NAME)
    cl_environ = _find_cl_dict(os.environ)
    if cl_environ:
        debug("%s found in os.environ['PATH']", _CL_EXE_NAME)
        # cl.exe not found in d['PATH']      
        # cl.exe not found in env['ENV']['PATH']
        # cl.exe found in os.environ['PATH']
        warn_msg = "MSVC compiler 'cl' found on the system path but will not be used."
        SCons.Warnings.warn(SCons.Warnings.VisualCMissingWarning, warn_msg)
        return

    debug("%s not found in os.environ['PATH']", _CL_EXE_NAME)

# END NEW

def msvc_setup_env(env):
    debug('called')
    version = get_default_version(env)
    if version is None:
        if not msvc_setup_env_user(env):
            MSVC.SetupEnvDefault.set_nodefault()
        return None

    # XXX: we set-up both MSVS version for backward
    # compatibility with the msvs tool
    env['MSVC_VERSION'] = version
    env['MSVS_VERSION'] = version
    env['MSVS'] = {}

    use_script, use_settings = get_use_script_use_settings(env)
    if SCons.Util.is_String(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))
        args = env.subst('$MSVC_USE_SCRIPT_ARGS')
        debug('use_script 1 %s %s', repr(use_script), repr(args))
        d = script_env(env, use_script, args)
    elif use_script:
        d = msvc_find_valid_batch_script(env,version)
        debug('use_script 2 %s', d)
        if not d:
            return d
    elif use_settings is not None:
        if not SCons.Util.is_Dict(use_settings):
            error_msg = 'MSVC_USE_SETTINGS type error: expected a dictionary, found {}'.format(type(use_settings).__name__)
            raise MSVCUseSettingsError(error_msg)
        d = use_settings
        debug('use_settings %s', d)
    else:
        debug('MSVC_USE_SCRIPT set to False')
        warn_msg = "MSVC_USE_SCRIPT set to False, assuming environment " \
                   "set correctly."
        SCons.Warnings.warn(SCons.Warnings.VisualCMissingWarning, warn_msg)
        return None

    # !!! ISSUE WARNING HERE BEFORE PREPENDING !!!
    #     inspect d['PATH'], env['ENV']['PATH'], and os.environ['PATH'] if necessary

    _cl_warning(d, env)

    for k, v in d.items():
        env.PrependENVPath(k, v, delete_existing=True)
        debug("env['ENV']['%s'] = %s", k, env['ENV'][k])

    # !!! EXISTING WARNING REMOVED FROM HERE !!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
MSVC Microsoft Visual C++ Support
Projects
None yet
Development

No branches or pull requests

3 participants