-
-
Notifications
You must be signed in to change notification settings - Fork 320
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
Wrong detection of Visual Studio Express 2010 and 2012 #2885
Comments
Again - very old issue, but have no clue if we correctly detect Express 2010/2012 - I'm assuming so? @jcbrill is there still anything to this? |
@mwichmann SCons correctly detects MSVS Express 2010 and MSVS 2012 Express. The registry queries are correct for both. However, as stated above, SCons also detects that MSVS 2010 and MSVS 2012 are installed. The registry entries are also correct for the non-express versions and point to the same installed folders. Because of the way amd64 targets are evaluated in the current SCons, using the msvc version without the "Exp" suffix now appears to work both with and without the "Exp" suffix. This was tested below. Basically the same msvc batch files are used (i.e., there are only 4 cache entries for the 8 builds). I can't be sure if this was always the case. The SDK 7.0A as shown above is present in sdk.py (I claim no expertise with sdk.py). As it happens, MSVS 2008 Professional was the last full version that I purchased. MSVS 2010 Express and MSVS 2012 Express are both installed on my development box. See the note below for targeting amd64 with MSVS 2010 Express. The following SConstruct successfully compiles a simple "Hello, World" program for MSVS 2010 Express (
Output fragments with MSCommon debugging enabled:
Note: MSVS 2010 Express does not include the 64-bit tools. The 64-bit tools must be installed via the Windows SDK 7.1. Once the SDK is installed there will not be a vcvars64.bat batch file in the amd folder. I use a vcvars64.bat that forwards the call to a batch that calls the SDK 7.1 SetEnv.cmd batch file. The indirection is necessary because (1) SetEnv.cmd expects delayed expansion to be enabled and (2) SetEnv.cmd expects environment variables that are not provided by scons. |
Second explanation attempt... The original post is not really whether MSVS 2012 Express and MSVS 2010 Express are detected correctly but rather when MSVS 2012 Express and MSVS 2010 Express are installed, MSVS 2012 and MSVS 2010 are detected as well. The behavior identified in the original post still exists: when MSVS 2012 Express and MSVS 2010 Express are installed, the installed versions list from vc.py detection reports that The original post rectifies this behavior by removing the "full" version from the installed list when the express version exists. Ultimately, the decision is whether this step is necessary or not. This behavior is the result of the registry keys for the installers. The express versions and full versions generally cannot exist at the same time for 2012 and 2010. The express version and the full version are installed to the same default folder. The express registry keys and the "normal" registry keys both exist and point to the same folder which is why detection reports that both the express version and the full version are installed. The vc.py detection treats the same visual studio root folder as two different versions (express and full) as there is no check for the development environment binary. From the perspective of vc.py (effectively the same as building from the command-line using the msvc tools) and the manner in which the target architectures are handled, there is not really a difference between the express and full versions as the development environment binary is not being invoked. As posted above, either seems to work fine. The behavior still exists. It is probably easier to document the behavior than change the code. A better fix in the future would be to unify the vs and vc detection and only process visual studio roots once and classify the edition based on the installed binaries that are present. PS: I have not checked the behavior in vs.py detection yet. |
Okay, good to know.
Personally, and especially given the age of the products in question, I'd prefer the "document and move on" approach. Certainly open to suggestions on what to say and where to say it... |
Thinking from a users perspective: What is the harm in this mis-detection? (I have a client who's stuck at MSVS 6, 2010 and a few other old versions, though they don't use SCons yet..) |
As near as I can tell, there is no harm. I'm not completely convinced it is a mis-detection from the command-line build tools perspective as the current vc.py implementation is somewhat flexible to the expected layout of the tools. However, an explicit user-defined
I have legacy c-based dll's from past consulting projects that have msvc runtime dependencies for pretty much every version of MSVS since 6 (with the exception of 2002/2003). That is why I have all of the old versions installed. |
IMHO, the exhibited behavior is a feature and not a bug. Perhaps this issue can be closed. For msvc versions Eliminating the so-called "mis-detection" may break existing builds and could make sconstruct files less portable. However, there are drawbacks based on feature availability. Also, not specifying a desired target architecture could lead to build failures (e.g., target architecture mis-match if linking external libraries). These drawbacks have existed since VS2008. Modern express versions tend to support the same target architectures as the full editions with certain restrictions, if any, around advanced usage (e.g., store/UWP builds and/or sdk version specification). The proposed branch (#4409) makes the non-express version (e.g., The table below shows the existing and proposed detected versions in a VM with only express editions installed.
Legend:
Note:
Installed VCS (white-space added for visual comparison):
|
So "won't fix"? More than happy to close it out, it's ancient versions anyway. |
Won't fix. Happy with the behavior. I'm proposing that we make a newer version (e.g., 2017) work like the legacy behavior which can be discussed in the other PR. |
ok, closing. |
This issue was originally created at: 2012-12-05 14:55:15.
This issue was reported by:
buildsmith
.buildsmith said at 2012-12-05 14:55:15
buildsmith said at 2012-12-05 15:04:38
buildsmith said at 2012-12-05 15:17:44
buildsmith said at 2012-12-06 01:32:48
The text was updated successfully, but these errors were encountered: