----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49926/#review141965 -----------------------------------------------------------
support/windows-build.bat (line 1) <https://reviews.apache.org/r/49926/#comment207411> Although we use this commend syntax in other batch scripts, we should avoid the LABEL syntax `::` and instead use `REM`. The latter is a standards-compliant, well-documented way of adding comments, while the former is not a commenting scheme at all. And because it is not a commenting scheme, there are many rules about where you can and can't place them (for example, you can't put any whitespace before the `::` symbols, and they can't appear in a `for` block), and if you disregard these things, you will get weird and hard-to-diagnose errors (for example, it might cause you to accidentally ignore parts of lexical blocks or output errors like `The system cannot find the drive specified` for seemingly no reason). For more information, see the bulleted list at the end of this post: http://www.robvanderwoude.com/comments.php support/windows-build.bat (lines 32 - 33) <https://reviews.apache.org/r/49926/#comment207413> I'm not super familiar with Jenkins, but I wonder why we create `tmp` in the Jenkins script, but create `build` here? It seems like we'd want to create the `tmp` file in this script, and leave the Jenkins script to do only the configuration stuff, like getting patch.exe? What do you think? support/windows-build.bat (line 34) <https://reviews.apache.org/r/49926/#comment207414> Do you need to check the error code for `MKDIR` too? I'm not super up on my batch semantics. support/windows-build.bat (line 40) <https://reviews.apache.org/r/49926/#comment207412> Interesting, why does Jenkins pass in `PATCHEXE_PATH` but not `ENABLE_LIBEVENT`? support/windows-build.bat (line 44) <https://reviews.apache.org/r/49926/#comment207415> Heh. Do we want to actually explain that this is a bug? This comment seems more like an inside joke right now. :) - Alex Clemmer On July 12, 2016, 1:11 a.m., Joseph Wu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49926/ > ----------------------------------------------------------- > > (Updated July 12, 2016, 1:11 a.m.) > > > Review request for mesos, Daniel Pravat, Artem Harutyunyan, Alex Clemmer, and > Vinod Kone. > > > Bugs: MESOS-5822 > https://issues.apache.org/jira/browse/MESOS-5822 > > > Repository: mesos > > > Description > ------- > > This adds a script that will be run on the ASF CI, similar to how the > `docker_build.sh` script is used for the existing Linux-based CI. > > This Jenkins job for Mesos on Windows can be found here: > https://builds.apache.org/job/Mesos-Windows/ > > > Diffs > ----- > > support/windows-build.bat PRE-CREATION > > Diff: https://reviews.apache.org/r/49926/diff/ > > > Testing > ------- > > Pointed the Jenkins job at my local branch and triggered the build. > > This is what Jenkins is running: > ``` > :: Run a script provided by Visual Studio which sets up a > :: couple environment variables needed by the build. > CALL "F:/Microsoft/Visual Studio CE 2015/VC/vcvarsall.bat" > > @echo on > > :: Recreate the `/tmp` directory needed by tests. > RMDIR /q /s %CD:~0,3%tmp > MKDIR %CD:~0,3%tmp > if %errorlevel% neq 0 exit /b %errorlevel% > > :: We need to specify the path to GNU Patch, > :: since it is installed in a non-default location. > SET OTHER_CMAKE_OPTIONS=-DPATCHEXE_PATH="F:/Program Files (x86)/GnuWin32/bin" > > :: Call the Windows build script. > "support/windows-build.bat" > ``` > > > Thanks, > > Joseph Wu > >
