> On March 13, 2018, 5:21 p.m., Jeff Coffler wrote: > > 3rdparty/boost-1.65.0.patch > > Lines 12 (patched) > > <https://reviews.apache.org/r/66046/diff/1/?file=1974924#file1974924line12> > > > > I'm thinking the other way around. That is, if the compiler version < > > 1910, issue the warning, otherwise be quiet. > > > > That way: > > > > 1. We only update this when we update minimum compiler version, > > 2. Greater versions are legal, as long as it's >= the minimum compiler > > version. > > > > The way you have it now, we'd need to update this every single time a > > new compiler came out. Yuck. I think we only care if new compiler is > > REQUIRED for some reason. > > Andrew Schwartzmeyer wrote: > That's not quite what's going on here. The upstream code is checking if > `_MSC_VER > 1910`, that is, "is the compiler newer than what we've tested > (1910)? If so, emit this warning." But we compile with, for instance, MSVC > 1912 (latest dot-update to VS). > > But look closer, this is a new patch file _removing_ this code because > the check is causing _way_ too many warnings. No, Boost, you did not break > between 1910 and 1912; stop warning us. > > Jeff Coffler wrote: > I understand that. I agree the current code is broken. My question: If > we're using a "less than supported" version of the compiler, isn't this a > GOOD error to get? Or is this something that cmake checks for? > > I guess, what I'm asking here: What happens if my version of Visual > Studio is too low? Do we support ALL versions of Visual Studio 2017? Or do we > need patch levels? If we do need a patch level, I think we should make it > obvious by checking for that here (or by aborting during the cmake step).
We're not using a less than supported version. We're using a "more" than supported version. This bit of code in Boost does not test less than, it tests more than. If you're VS studio is too low, you'll fail a lot earlier than this. Moreover, the rest of this file does minimum version checks. This hunk does "max" version checks. It's confusing and dumb. - Andrew ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66046/#review199133 ----------------------------------------------------------- On March 13, 2018, 3:30 p.m., Andrew Schwartzmeyer wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66046/ > ----------------------------------------------------------- > > (Updated March 13, 2018, 3:30 p.m.) > > > Review request for mesos, Akash Gupta, Benjamin Bannier, Jeff Coffler, John > Kordich, Joseph Wu, and Michael Park. > > > Bugs: MESOS-8556 > https://issues.apache.org/jira/browse/MESOS-8556 > > > Repository: mesos > > > Description > ------- > > On Windows, Boost explicitly checks if it's being compiled with a > compiler version they've tested. As we tend to update Visual Studio > whenever a stable update is available, this leads to a Boost warning, > "Unknown compiler version..." being emitted repeatedly (for every file > which includes a Boost header). > > If it were emitted once, we would leave it be, but because it's > repeated hundreds of times, and is mostly harmless, we patch the build > to remove the warning itself. Unfortunately, there is no compile-time > option to disable the warning instead of a patch. > > Note that it is not straight-forward to generate a patch file for > Boost. The steps were: clone the Boost repo recursively, go the > `libs/config` submodule, edit the `visualc.hpp` file, go to the > `include` subdirectory, and use `git diff --relative` to generate the > patch with the corrects paths expected for the extracted tarball. > > > Diffs > ----- > > 3rdparty/CMakeLists.txt 61dc788edf96ca6e8e3ee736eb232c0c118e6191 > 3rdparty/boost-1.65.0.patch PRE-CREATION > > > Diff: https://reviews.apache.org/r/66046/diff/1/ > > > Testing > ------- > > Built on Windows; no more warning. > > > Thanks, > > Andrew Schwartzmeyer > >
