| Summary: | webkitdirs.pm does not use environment variable VSINSTALLDIR | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | David Delaune <david.delaune> |
| Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> |
| Status: | RESOLVED FIXED | ||
| Severity: | Normal | CC: | ap, bfulgham, dimich, ian, jonlee, jshin, mbaker, roger_fong |
| Priority: | P2 | ||
| Version: | 528+ (Nightly build) | ||
| Hardware: | PC | ||
| OS: | Windows 7 | ||
|
Description
David Delaune
2013-12-30 17:43:43 PST
Visual Studio no longer sets VSINSTALLDIR, so checking for this incorrectly causes the build system to use VS2005 or earlier. We do not want that. This fix was needed to support our build farm. I'm surprised it caused any issues for you; do you not have VS2013 installed? What error messages did you get when attempting to build? Hi, What makes you think Visual Studio has been changed to no longer set the %VSINSTALLDIR% variable? Visual Studio 2013 (Version 12) does set the %VSINSTALLDIR% environment variable. Best Wishes, -David Delaune (In reply to comment #1) > This fix was needed to support our build farm. I'm surprised it caused any issues for you; do you not have VS2013 installed? So it is exactly as I suspected. This quick hack was added to fix a single-user scenario while potentially breaking compilations for the rest of the world. Someone should invest 5 minutes looking into this. It only takes a few minutes to peruse vcvarsall.bat where you will find that depending on the architecture it will call \VC\bin\vcvars[*].bat which calls "%VS120COMNTOOLS%VCVarsQueryRegistry.bat" to set %VSINSTALLDIR%. or perhaps if you only have 10 seconds to invest you could open a Visual Studio command prompt and type 'echo %VSINSTALLDIR%' If you really want to check the version then you should consider taking advantage of the %VISUALSTUDIOVERSION% environment variable. Best Wishes, -David Delaune It sounds like we have to execute the VSVARSALL.BAT script to (temporarily) set the VSINSTALLDIR environment variable. I don't see how executing this script as part of our build process is any better than coding the path to the required Visual Studio installation. We could certainly manually create a VSINSTALLDIR environment variable, but I'm not sure what problem we are solving by doing so. Can you be specific about the issue you are actually having? * Are you trying to build with something besides VS3013? * Did you install VS2013 in an unusual location and are having a problem? If so, what errors are you seeing? I work with several other Windows developers, who had no problem with this change. Please provide specific error messages and build logs so we can deal with the actual problem you are encountering. Hey Brent, Surely you remember me? I am the guy who worked and created the WinCairoRequirements Visual Studio project that you graciously uploaded to github. (In reply to comment #4) > It sounds like we have to execute the VSVARSALL.BAT script > to (temporarily) set the VSINSTALLDIR environment variable. > I don't see how executing this script as part of our build > process is any better than coding the path to the required > Visual Studio installation. How is it even possible that you are not understanding this? The patch hard codes the path to Visual Studio to either the OS drive or the C drive. If Visual Studio is installed anywhere else they will be unable to build. > > We could certainly manually create a VSINSTALLDIR environment > variable, but I'm not sure what problem we are solving by doing so. The VSVARSALL.BAT script batch file sets several environment variables, PATH, INCLUDE, LIB and LIBPATH. It also adds the path to Windows SDK and corresponding toolsets. > > Can you be specific about the issue you are actually having? > Unable to compile without fixing webkitdirs.pm library. > * Are you trying to build with something besides VS3013? I am obviously compiling with Visual Studio 2013 (Version 12) > * Did you install VS2013 in an unusual location and are having > a problem? If so, what errors are you seeing? Define unusual. My workstation has Visual Studio 2013 installed on the D: drive. The operating system is on the C: drive. I mention this in the first comment. I find it disturbing that you do not know what errors occur when the build script cannot find the compiler. I am beginning to wonder if you are being pretensive. > > I work with several other Windows developers, who had no problem with this change. Please provide specific error messages and build logs so we can deal with the actual problem you are encountering. They would certainly be able to compile if Visual Studio installed on either the C: drive or OS drive. Best Wishes, -David Delaune Rolled out r160846 here: https://bugs.webkit.org/show_bug.cgi?id=126395 |