<div dir="ltr"><div dir="ltr"><div dir="ltr"><div>Thanks for raising this.</div><div dir="ltr"><br></div><div dir="ltr">This is a topic I've been interested in for a while too, as I've had to do a few of those lite.site.cfg fix-ups that you mention (in fact I have one sitting unreviewed at <a href="https://reviews.llvm.org/D40522">https://reviews.llvm.org/D40522</a> although I've not pinged it in a long time so I'll need to double check that it's still an issue). There are also other issues. For example LLVM_ENABLE_ABI_BREAKING_CHECKS is implemented in such a way that by default the value is defined at CMake time based on the value of LLVM_ENABLE_ASSERTIONS which gets confusing with the Visual Studio generator where the value of LLVM_ENABLE_ASSERTIONS does not necessarily correspond to whether assertions are enabled or not.</div><div dir="ltr"><br><div>As I understand it, what you're proposing is to not support building for any configs that return true for GENERATOR_IS_MULTI_CONFIG. This includes all of the Visual Studio generators, but also the Xcode generator. I'm not an Xcode user. Does anyone make use of that generator or is it entirely replaced in practice by single-config generators, i.e. Ninja?</div><div><br></div><div>We're still using the Visual Studio generators in production at Sony at the moment. This is largely because until recently they were actually faster than Ninja for us due to the availability of distributed builds on our network. We've recently patched in support for our system into our private branch of Ninja now so in theory it should be faster/on-par again but we've not yet pulled the trigger on making them the default. If there's consensus that this is the way forward, then we'll definitely need some time to make the change internally. I'm only speaking personally in this reply as I'll need to discuss with the rest of the team before we can reach a position, but basically I wouldn't want the conclusion of this thread to be "No dissenting voices, so here's an immediate patch to remove support!"</div><div><br></div><div>I've not tried the workflow you describe. I'll try it out in the coming days to see how it works for me. My main concerns are:</div><div><br></div><div>* How far will it raise the barrier of entry to new developers? My impression is that a lot of students coming to LLVM for the first time, first build out of the box with Visual Studio before later discovering this magical thing called Ninja that will speed things up. Potentially this could be mitigated with good enough documentation in the getting started guide I expect.</div><div><br></div><div>* LLVM's CMake is super-slow on Windows, and we'd need to run it twice whenever there are project changes. This could be a significant drawback in the proposed workflow but I'll need to try it before I can say that for sure.</div><div><br></div><div>* My muscle memory causing repeated Ctrl+Shift+B :-). I wonder if we could add a PRE_BUILD custom target conditional on GENERATOR_IS_MULTI_CONFIG to automatically fail any builds with a useful help message.<br></div><div><br></div><div><br></div><div>If the decision is that we continue supporting these generators, then at the very least we should look into adding a buildbot configured to use one of the Visual Studio Generators rather than ninja so that issues get spotted on commit.<br></div><div><br></div><div>-Greg</div><div><br></div><div><br></div><div><br></div><div><br></div></div></div></div></div><br><div class="gmail_quote"><div dir="ltr">On Sun, 7 Oct 2018 at 21:51, Zachary Turner via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">This has been on my mind for quite some time, but recently it's been popping up more and more seeing some of the issues people have run into.<div><br></div><div>Before people get the wrong idea, let me make one thing clear. **I am not proposing we stop supporting the CMake Visual Studio generator. I am only proposing we stop supporting actually compiling with the generated project**. Yes the distinction is important, and I'll elaborate more on why later. First though, here are some of the issues with the VS generator:</div><div><br></div><div>1) Using MSBuild is slower than Ninja.</div><div>2) Unless you remember to pass -Thost=x64 on the command line, you won't be able to successfully build. We can (and have) updated the documentation to indicate this, but it's not intuitive and still bites people because for some reason this is not the default.</div><div><div><font color="#212121">3) Even if you do pass -Thost=x64 to CMake, it will apparently still fail sometimes. See this thread for details: <a href="http://lists.llvm.org/pipermail/cfe-dev/2018-October/059609.html" target="_blank">http://lists.llvm.org/pipermail/cfe-dev/2018-October/059609.html</a>. It seems the parallel build scheduler does not do a good job and can bring a machine down. This is not the first time though, every couple of months there's a thread about how building or running tests from within VS doesn't work.</font></div></div><div>4) Supporting it is a continuous source of errors and mistakes when writing tests. The VS generator outputs a project which can build Debug / Release with a single project. This means that `CMAKE_BUILD_TYPE=Debug` is a no-op on this generator. The reason this matters for the test suite is because `${CMAKE_CURRENT_BINARY_DIR}` isn't sufficient to identify the location of the binaries. You need `<span style="font-size:13px;color:rgb(33,33,33)">${CMAKE_CURRENT_BINARY_DIR}/${</span><span style="font-size:13px;color:rgb(33,33,33)">CMAKE_CFG_INTDIR}` instead.</span></div><div><span style="font-size:13px;color:rgb(33,33,33)"><br></span></div><div><span style="color:rgb(33,33,33)">There is a continuous source of problems in our CMake [1, 2, 3, 4, 5]. It also affects tests, and every time someone adds a new lit site configuration, they have to remember to add this magic block of code:</span></div><div><span style="color:rgb(33,33,33)"><br></span></div><div><div><font color="#212121"># Support substitution of the tools_dir with user parameters. This is</font></div><div><font color="#212121"># used when we can't determine the tool dir at configuration time.</font></div><div><font color="#212121">try:</font></div><div><font color="#212121"> config.llvm_tools_dir = config.llvm_tools_dir % lit_config.params</font></div><div><font color="#212121"> config.llvm_shlib_dir = config.llvm_shlib_dir % lit_config.params</font></div><div><font color="#212121">except KeyError:</font></div><div><font color="#212121"> e = sys.exc_info()[1]</font></div><div><font color="#212121"> key, = e.args</font></div><div><font color="#212121"> lit_config.fatal("unable to find %r parameter, use '--param=%s=VALUE'" % (key,key))</font></div></div><div><font color="#212121"><br></font></div><div><font color="#212121">to the file (even though only about 2 people actually understand what this does), which has caused problems several times.</font></div><div><font color="#212121"><br></font></div><div><font color="#212121">5) VSCode and Visual Studio both support opening CMake projects directly now, which bypasses MSBuild. I don't know how well Visual Studio supports LLVM's CMake, but the last time I tried it with VSCode on Linux it worked fine.</font></div><div><br></div><div>----</div><div><br></div><div><font color="#212121">I mentioned earlier that the distinction between not *building* with a VS-generated project and not supporting the VS generator is important.</font></div><div><font color="#212121"><br></font></div><div><font color="#212121">I don't want to speak for everyone, but I believe that *most* people use the VS generator because they want IDE support for their projects. They want to be able to browse code, hit F5 to debug, F9 to set breakpoints, etc. They don't necessarily care that Ctrl+Shift+B is how the code is generated versus some other incantation. I'm asserting that it's possible to still have all the things people actually want from the VS generator without actually building from inside of VS. In fact, I've been doing this for several years. The workflow is:</font></div><div><font color="#212121"><br></font></div><div><font color="#212121">1) Run CMake twice, generating to separate output directories. Once using -G "Visual Studio 15 2017" and once using -G Ninja, each to different directories.</font></div><div><font color="#212121"><br></font></div><div><font color="#212121">2) Open the VS one. You have full IDE support.</font></div><div><font color="#212121"><br></font></div><div><font color="#212121">3) Instead of hitting Ctrl+Shift+B to build, have a command prompt window open and type ninja. Wait for it to complete. If you want to you can make a custom tool command in Visual Studio so that you can access this from a keyboard shortcut.</font></div><div><font color="#212121"><br></font></div><div><font color="#212121">4) When you want to debug, set your startup project (as you normally would), right click and hit properties, go to Debugging, change Command from $(TargetPath) to <type the full path to bin/foo.exe of the program you want to debug>.</font></div><div><font color="#212121"><br></font></div><div><font color="#212121">5) Hit F5.</font></div><div><font color="#212121"><br></font></div><div><font color="#212121">In short, with only 2 simple additional steps (run CMake an extra time, and type a path into a window), people can have the exact workflow they are used to, plus faster builds, minus all of the problems and complexities associated with building from within VS.</font></div><div><font color="#212121"><br></font></div><div><font color="#212121">And we can simplify our CMake logic and lit configuration files as well.</font></div><div><font color="#212121"><br></font></div><div><font color="#212121">----</font></div><div><font color="#212121"><br></font></div><div><div><span style="color:rgb(33,33,33)"><br class="m_5362396185258994246m_4328515970606342552inbox-inbox-Apple-interchange-newline">[1] - </span><font color="#212121"><a href="https://reviews.llvm.org/D43096" target="_blank">https://reviews.llvm.org/D43096</a></font><br></div><div><font color="#212121">[2] - <a href="https://reviews.llvm.org/D46642" target="_blank">https://reviews.llvm.org/D46642</a></font></div><div><font color="#212121">[3] - <a href="https://reviews.llvm.org/D45918" target="_blank">https://reviews.llvm.org/D45918</a></font></div><div><font color="#212121">[4] - <a href="https://reviews.llvm.org/D45333" target="_blank">https://reviews.llvm.org/D45333</a></font></div><div><font color="#212121">[5] - <a href="https://reviews.llvm.org/D46334" target="_blank">https://reviews.llvm.org/D46334</a></font></div><div><font color="#212121"><br></font></div><br class="m_5362396185258994246m_4328515970606342552inbox-inbox-Apple-interchange-newline"></div></div>
_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
</blockquote></div>