<div dir="ltr"><div><div>(There is a summary of the biggest issues at the bottom)</div><div><br></div><div>Note, some of my comments may be addressed by content further in the document (I'm reading mostly sequentially). Such comments should not be brushed off as "oh, he just didn't get there yet"; they should be interpreted as meaning that the content should be reorganized so that a reader does not encounter such questions in the first place. Also if I make a comment that seems to be due to me misunderstanding some part of what is written, that should be interpreted as meaning that that part of the document should be revised to be clearer.<br>
</div><div><br></div><div>+Building LLVM on Windows using the MinGW32, MinGW64, and Visual Studio toolchains</div><div>+is complicated. [...]</div></div><div><br></div><div>This seems completely generic about building on windows. Where do buildbots come into this? This should be clarified in the introduction.</div>
<div><br></div><div>+[...]This document aims to make it easy for you to configure<br></div><div>+Windows to build LLVM with the major windows toolchains, bourne shells, and</div><div>+natively with Microsoft's ``cmd.exe``.</div>
<div><br></div><div>Here you list 3 different things as the major parts of this document. They should each be a top-level section (i.e. accessible through the table of contents) if they are so prominent.</div><div><br></div>
<div><div>+In the text that follows, your LLVM working directory will be referred to as</div><div>+``%LLVMDIR%``.</div></div><div><br></div><div>This is ambiguous/confusing. A lot of people have a directory with different llvm checkouts/builddir's that could be considered an "LLVM working directory". Please explicitly say the root directory of the source checkout.</div>
<div><br></div><div><div>+MinGW currently exists in two versions: MinGW32 (32-bit tools) and MinGW64</div><div>+(64-bit tools). The 32-bit tools are readily available from the `main MinGW</div><div>+website <<a href="http://www.mingw.org">http://www.mingw.org</a>>`_. The 64-bit tools are available from the</div>
<div>+`MinGW W64 website <<a href="http://mingw-w64.sourceforge.net">http://mingw-w64.sourceforge.net</a>>`_.</div></div><div><br></div><div>Why would a user prefer one or the other? Can we mention only one to make things simpler?</div>
<div><br></div><div><div>+Install GnuWin32 by following the *Looking for the latest version?* link at</div><div>+`GnuWin32 on SourceForge.net <<a href="http://gnuwin32.sourceforge.net">gnuwin32.sourceforge.net</a>>`_. Download and run</div>
</div><div><br></div><div>I don't see a "Looking for the latest version?" link there. Also, it seems like these are 32-bit. Does that interact in any way with a user's choice for 32/64 bit in e.g. the mingw choice?</div>
<div><br></div><div><div>+The primary purpose of GnuWin32 is to supply utilities missing in MinGW-32</div><div>+MSYS. GnuWin32 will be after MSYS in the path and handle utilities that are not</div><div>+available in MSYS.</div>
</div><div><br></div><div>Which utilities? Why are they needed?</div><div><br></div><div><div>+Installing Subversion</div><div>+---------------------</div></div><div><br></div><div>Many people are much more comfortable with git. Please include instructions for them.</div>
<div><br></div><div><div>+Ninja is a *very* fast build tool that is one of the builders in CMake.</div><div>+You can download a binary from</div><div>+`CMakeScript <<a href="http://sourceforge.net/projects/cmakescript/files/">http://sourceforge.net/projects/cmakescript/files/</a>>`_. You</div>
<div>+can save it to your standard tools directory or to ``%LLVMDIR%`` and invoke it</div><div>+from there.</div><div><br></div><div>That link seems really sketchy (and I found it hard to find the ninja binary). If it is so vital to provide a binary download (see the comment just below), I would prefer to build a known-good version of ninja and host it on <a href="http://llvm.org">llvm.org</a>.</div>
<div><br></div><div>+</div><div>+**Notice:** This version of Ninja requires you to have the Microsoft Visual</div><div>+C++ 2010 Redistributable installed. You can get it from `Microsoft's Download</div><div>+Center <<a href="http://www.microsoft.com/en-us/download/details.aspx?id=5555">http://www.microsoft.com/en-us/download/details.aspx?id=5555</a>>`_.</div>
</div><div><br></div><div>This almost seems more complicated than just building ninja from source? It's literally just git clone and ./bootstrap.py.</div><div><br></div><div><div>+ | Devel/DejaGnu.</div><div>+ | Interpreter/tcl.</div>
<div>+ | Tcl/expect.</div></div><div><br></div><div>Do we still need these?</div><div><br></div><div><div>+**Notice:** If you do not plan to run the test suite, or sshd server, you don't</div><div>+need Cygwin. You can build LLVM + Clang with only Subversion, MingwNN, and CMake.</div>
</div><div><br></div><div>I feel like the way you are handling these notices is backwards. Your current approach seems to be to spew all things that could possibly need to be installed and then have little "Notice" sections telling you whether that section is actually of interest to you; this is extremely inefficient and confusing for the reader. I recommend turning that upside down: create a nested list whose structure shows which components are necessary for what you want to do. E.g.</div>
<div><br></div><div>- Required:</div><div> + Thing1. <<a href="http://downloadlink1">http://downloadlink1</a>>. Succinct description of any nonobvious specific steps.</div><div> + Thing2. <<a href="http://downloadlink2">http://downloadlink2</a>>. ...</div>
<div> + ...</div><div>- Only need these if you want to do X:</div><div> + OptionalThing1 ...</div><div> + ...</div><div>- Only need these if you want to do Y:</div><div> + OtherOptionalThing2 ...</div><div> + ...</div>
<div><br></div><div><div>+If you want to setup your Windows build machine as Buildslave in the LLVM </div><div>+Buildbot Infrastructure (Zorg project), </div></div><div><br></div><div>In view of the title of this document, it seems like this "if" is pointless since it will always be true.</div>
<div><br></div><div><div>+[...]For unix style builds, [...]</div></div><div><br></div><div>I just had to read through setup instructions for setting up a bunch of unix utilities and now you're telling me that unix-style builds are only one of the options, and so I was wasting my time if I wasn't doing a unix-style build? Please clarify. Try to achieve the following goal: The document is structured/organized in such a way that a reader can know exactly which parts of the document are relevant for their situation and will spend (i.e. waste) no time looking at instructions that are not relevant for their desired configuration.</div>
<div><br></div><div>+[...]MSVC and Intel compilers require setup</div><div>+scripts that are invoked from existing environment variables. An example for MSVC10</div><div>+is included. [...]</div><div><br></div><div>Ok now this is worrying. This is a HowTo, there shouldn't be a big piece of functionality like a crucial script missing. This guide cannot purport to be a HowTo for configurations for which it doesn't provide this component, so either clarify that this HowTo is only for MSVC10 (and also unix-like), or (preferred) provide instructions for the other scenarios. Also, AFAIK almost all LLVM MSVC users are running 12.</div>
<div><br></div><div>+ svn co <a href="http://llvm.org/svn/llvm-project/llvm/trunk">http://llvm.org/svn/llvm-project/llvm/trunk</a> llvm-trunk<br></div><div><br></div><div>Please provide a git option as well, especially since you are assuming git is present below.</div>
<div><br></div><div><div>+.. code-block:: bat</div><div><br></div></div><div>bat sucks, can you just write these scripts in Python (with the subprocess module)? It should be much clearer than goto-ridden bat.</div><div><br>
</div><div><div>+The whole thing takes about five minutes so you can safely go get a cup of</div><div>+coffee or what you normally do when you have to wait for your machine.</div></div><div><br></div><div>I doubt this is consistently five minutes. Checkout time is highly dependent on network speed and may take much longer. You don't need to give a numerical estimate ("safely go get a cup of coffee" is fine).</div>
<div><br></div><div><div>+If everything went okay and there were no error messages, you've succesfully</div><div>+got all of the relevant sources from the LLVM Subversion repository.</div></div><div><br></div><div>And if there were error messages? Then what do I do? (suggestion: direct the reader to the mailing lists)</div>
<div><br></div><div><div>+There's a complete list of CMake variables in the `Building LLVM with CMake</div><div>+<<a href="http://llvm.org/docs/CMake.html">http://llvm.org/docs/CMake.html</a>>`_ document.</div></div>
<div><br></div><div>Use :ref:`CMake` instead of an HTTP link.</div><div><br></div><div>+**Notice:** ``cmake-gui`` is not part of the special Ninja version of CMake.<br></div><div><br></div><div>What "special Ninja version of CMake"?</div>
<div><br></div><div><div>+[...] Preferredly, you can do a ``ninja install`` to make</div><div>+the binaries, libraries, and headers, and have them installed in the location</div><div>+you specified as part of the ``CMAKE_INSTALL_PREFIX`` variable above.</div>
</div><div><br></div><div>Is this really preferred? Especially on a bot (consider the title), this doesn't seem like it is "preferred" at all.</div><div><br></div><div><br></div><div><div>The biggest issues I see are</div>
<div>1) Extremely poor organization. Try to achieve the following goal: The document is structured/organized in such a way that a reader can know exactly which parts of the document are relevant for their situation and will spend (i.e. waste) no time looking at instructions that are not relevant for their desired configuration.<br>
</div><div> + For example, I still don't know which "toolchains" this is supposed to cover. Intel and MSVC were mentioned in passing?</div><div> + There's a *ton* of software that this is guiding the reader to install. I don't use Windows much, but last I did (this Summer), all it took to build LLVM was git, MSVC, CMake, and Ninja, and it was like 1 step, and I didn't need to significantly fiddle with any paths or anything. Where is that "easy" option?</div>
<div>2) This seems to have almost nothing to do with builbots, despite having the phrase "for the LLVM Buildbot infrastructure" in the title. I suggest just removing that part of the title (and renaming the file).</div>
<div>3) Please just obliterate the sentence: "This document aims to make it easy for you to configure Windows to build LLVM with the major windows toolchains, bourne shells, and natively with Microsoft’s cmd.exe.". I literally just wrote like 10 lines of text describing things that are wrong with this sentence (I decided it's just not worth getting into). Please just nuke it and write a replacement in the context of the (freshly organized) document.</div>
</div><div><br></div><div>Bottom line: This document has lots of good content (clearly gleaned from "battle scars" doing this kind of setup in the field), but it doesn't seem to have a very good idea of what it is trying to accomplish, and doesn't provide any way for the reader to be able to efficiently navigate the content or know what is relevant. It needs some reorganization, but I would like to see something like this added to our docs.</div>
<div><br></div><div>-- Sean Silva</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Nov 11, 2013 at 6:50 PM, Mikael Lyngvig <span dir="ltr"><<a href="mailto:mikael@lyngvig.org" target="_blank">mikael@lyngvig.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">This document describes how to set up a Windows builder and is a document that quite a few have asked about during the past few years. <div>
<br></div><div>Please find attached diff.</div><div><br></div><div>
Please direct all criticism, error reports, and enhancement requests to me and I'll see what I can do.</div><div><br></div><div><br></div><div>Sincerely,</div><div>Mikael Lyngvig</div><div><br></div><div><br></div></div>
<br>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div>