NEW 126407
Set VSINSTALLDIR in win ews start script
https://bugs.webkit.org/show_bug.cgi?id=126407
Summary Set VSINSTALLDIR in win ews start script
Roger Fong
Reported 2014-01-02 14:45:14 PST
Set VSINSTALLDIR to where we install VS2013. Alternatively we can run VCVARSALL.bat in the script but the change seems to be equivalent and that'd mean we'd be running a batch script from a shell script which is being run from a batch script... maybe that's not that weird...
Attachments
patch (1.05 KB, patch)
2014-01-02 14:49 PST, Roger Fong
bfulgham: review+
vcvarsall version (1.06 KB, patch)
2014-01-02 14:52 PST, Roger Fong
bfulgham: review-
vcvarsall version fixed (1.06 KB, patch)
2014-01-02 16:24 PST, Roger Fong
no flags
Roger Fong
Comment 1 2014-01-02 14:49:03 PST
Roger Fong
Comment 2 2014-01-02 14:52:31 PST
Created attachment 220257 [details] vcvarsall version
Brent Fulgham
Comment 3 2014-01-02 15:29:05 PST
Comment on attachment 220257 [details] vcvarsall version View in context: https://bugs.webkit.org/attachment.cgi?id=220257&action=review > Tools/EWSTools/start-queue-win.sh:25 > + cmd /c "$PROGRAMFILES\Microsoft Visual Studio 12.0\VC\vcvarsall.bat" This should be "CALL "$PROGRAMFILES...." We want the queue to run with the contents of the batch file added to the shell environment. The 'cmd' version will spawn a new shell, which will cease to exist once the command finishes running.
Brent Fulgham
Comment 4 2014-01-02 15:29:43 PST
Comment on attachment 220256 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=220256&action=review > Tools/EWSTools/start-queue-win.sh:25 > + export VSINSTALLDIR="$PROGRAMFILES\Microsoft Visual Studio 12.0" I don't care for this version as much, because it doesn't set the other environment variables (such as LIBPATH, etc.) which might be useful for building.
Roger Fong
Comment 5 2014-01-02 16:24:41 PST
Created attachment 220265 [details] vcvarsall version fixed
Roger Fong
Comment 6 2014-01-02 16:26:31 PST
uh hold up, call command not found
Brent Fulgham
Comment 7 2014-01-02 16:34:20 PST
Duh! Sorry -- I was thinking in DOS BATCH, not BASH. We want to introduce these variables to the Cygwin environment, so maybe we do need to go with your original version that just adds the environment variable.
Roger Fong
Comment 8 2014-01-03 10:58:59 PST
(In reply to comment #7) > Duh! Sorry -- I was thinking in DOS BATCH, not BASH. > > We want to introduce these variables to the Cygwin environment, so maybe we do need to go with your original version that just adds the environment variable. Hmm, after some amount of research I don't think cygwin has anything that can save the environment settings set by a batch script. I could always convert the batch script to a bash script, check it in, and we can modify it as need be instead.
Roger Fong
Comment 9 2014-01-03 14:49:42 PST
What do you think Brent? Think we should just go with setting VSINSTALLDIR ourselves?
Brent Fulgham
Comment 10 2014-01-06 11:08:12 PST
Comment on attachment 220256 [details] patch Watch out for LIBPATH or INCLUDE_PATH problems in scripts since we aren't setting these, but this seems to work.
Roger Fong
Comment 11 2014-01-06 11:14:44 PST
(In reply to comment #10) > (From update of attachment 220256 [details]) > Watch out for LIBPATH or INCLUDE_PATH problems in scripts since we aren't setting these, but this seems to work. Ok, I'll keep an eye out.
Roger Fong
Comment 12 2014-01-06 12:15:39 PST
http://trac.webkit.org/changeset/161355 Keeping open until EWS bots are actually reporting working builds
Note You need to log in before you can comment on or make changes to this bug.