This is an archive of the discontinued LLVM Phabricator instance.

[docs] Update Getting Started with Visual Studio guide
ClosedPublic

Authored by yaron.keren on Aug 21 2021, 11:00 AM.

Diff Detail

Event Timeline

yaron.keren created this revision.Aug 21 2021, 11:00 AM
yaron.keren requested review of this revision.Aug 21 2021, 11:00 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 21 2021, 11:00 AM

add all commits

yaron.keren retitled this revision from changes to [docs] Update Getting Started with Visual Studio guide.Aug 21 2021, 11:05 AM
yaron.keren edited the summary of this revision. (Show Details)

Fix typo

Trying to fix the diff

Trying yet again to fix the diff

kuhnel accepted this revision.Aug 23 2021, 1:10 AM

Thx for cleaning up the Windows instructions!

I only have a view minor remarks (see inline comments).

As a side note:
I'm also working on a tutorial for Visual Studio Code on Windows, however I didn't really have time to finish that yet.
What's missing so far: Debugging with LLDB, installing the Visual Studio compiler (without UI) and use these.

llvm/docs/GettingStartedVS.rst
48–50

Nit: Maybe add a comment that the "Community Edition" is enough.

113

Nit: Maybe call out, that you're only building clang here and users need to configure the other projects as needed, e.g. as explained in https://llvm.org/docs/CMake.html#frequently-used-llvm-related-variables

118–123

I'm not familiar with using CMake with the Visual Studio build system, but don't you need to set the output build system with cmake -G "Visual Studio 17 2022? Or is that set automatically?

145–146

I guess we should also update these instructions, based on your new tool recommendations.

150

Why not Release with Debug info? This gives you much more information when debugging problems?

This revision is now accepted and ready to land.Aug 23 2021, 1:10 AM
aaron.ballman added inline comments.Aug 23 2021, 5:04 AM
llvm/docs/GettingStartedVS.rst
42

If we're going to recommend hardware, should we also recommend a significant amount of RAM?

55

I'm a bit worried about losing this information; I didn't have luck skipping GnuWin32 the last time I tried (about a year ago), so I don't think just Python is sufficient. WDYT?

59
113

Before getting to "finally", should we explain what needs installation for tests to work (like GnuWin32)?

182–183

Not certain why we need to churn these examples (FWIW, capital drive letters are still quite common).

I also thank you for working to improve this documentation. I mostly agree the other reviewer. I've only added a couple inline comments to raise questions. I wouldn't hold this back for either of those.

llvm/docs/GettingStartedVS.rst
55

The git-provided versions of common unix tools have been sufficient for me (except there's no make in git). The git ones are also newer than the GnuWin32 ones, and a couple of them behave slightly differently.

The only reason I still I install GnuWin32 is because lldb tests require make. That said, I make sure that GnuWin32 comes _after_ git in my PATH. Otherwise, other things go wrong. GnuWin32 is old and unmaintained. I don't think we can expect Windows developers to continue to rely on it.

I'm not against mentioning it as a possibility, but I don't think it belongs in the mainline "Getting Started" recommendations.

118–123

That's an interesting question. I've never actually built from the Visual Studio IDE. I use CMake to generate solution and project files for browsing, editing, and debugging. But I also have CMake generate a ninja solution and actually build from the command line. In both cases, I explicitly specify the generator. If we're not recommending ninja for builds, then maybe we don't need to mention how to install it.

mstorsjo added inline comments.Aug 23 2021, 11:53 PM
llvm/docs/GettingStartedVS.rst
55

Also FYI, regarding GnuWin32 vs the git unix tools - the llvm lit tests also have a provision where they try to locate the git unix tools and add them to the path, but this isn't hooked up when running other testsuites (like for libcxx). See https://github.com/llvm/llvm-project/blob/main/llvm/utils/lit/lit/llvm/config.py#L29-L35.

aaron.ballman added inline comments.Aug 24 2021, 4:38 AM
llvm/docs/GettingStartedVS.rst
55

If the git tools are sufficient, that's fantastic news (I've been hoping we can get rid of this insecure and long-unsupported requirement for years)! When I last tried, they weren't sufficient; I recall some Clang lit tests were failing because of missing utilities like sed. But if you think they're now sufficient, perhaps we should document that more clearly as a need for the testing system? (Also, perhaps the issue is that Git for Windows that comes with VS 2019 may be different from what you download manually, and if so, that would be useful to tell people if it actually matters to the testing system.)

mstorsjo added inline comments.Aug 24 2021, 4:44 AM
llvm/docs/GettingStartedVS.rst
55

At least the pre-merge test bot runs without GnuWin32 now, but it manually adds the c:\Program Files\Git\usr\bin directory to the path instead. The llvm lit config that I linked does look for a path to something similar via the registry, I guess that doesn't find git bundled with Visual Studio (and I'm not sure if that bundled the same unix utilities in the same way).

yaron.keren marked 15 inline comments as done.Aug 24 2021, 11:25 AM
yaron.keren added inline comments.
llvm/docs/GettingStartedVS.rst
42

OK

48–50

ok

55

The Visual Studio installer installs a slightly older git (2.31.1) than the standalone installer from the web site (2.33). Otherwise identical and (unlike cmake) installed into the same install location (c:\Program Files\Git\).

The bash utilities installed in c:\Program Files\Git\usr\bin\ (from either version of Git for Windows) pass regression tests for llvm & clang pass. Nothing besides the documented here is required.

llvm-lit config.py indeed can find Git for Windows utilities even if not on the PATH by looking in the registry. This is a very nice "just works" functionality to share with other llvm projects.

I had not tested projects of LLVM other than llvm and clang - this is the scope of a "Getting Started" page ?

59

Thanks for catching this!

113

GnuWin32 functionality is replaced by the bash tools in Git for Windows (in c:\Program Files\Git\usr\bin\)

113

Clarified at the page that the scope is llvm&clang only, to keep this simple. Refered to the CMake page.

118–123

On Windows cmake defaults to the latest Visual Studio (and on Linux to Unix Makefiles).

145–146

OK

150

On my system Release build is 1.8GB whereas RelWithDebInfo build is 16GB. Will explain this.

182–183

Yes indeed this page should be Visual Studio specific - removed!

yaron.keren marked 10 inline comments as done.

Address comments

aaron.ballman accepted this revision.Aug 24 2021, 11:36 AM

LGTM, thank you for the improved documentation! (Please wait a bit before landing in case other active reviewers to chime in with further feedback.)

llvm/docs/GettingStartedVS.rst
55

I had not tested projects of LLVM other than llvm and clang - this is the scope of a "Getting Started" page ?

I'd say it's not, but if others want to build a case for why it is, I could be persuaded. Personally, I think the docs are good as-is now.

sylvestre.ledru resigned from this revision.Aug 24 2021, 12:40 PM

Sorry, I don't know enough about Visual Studio to be a proper reviewer :)

amccarth accepted this revision.Aug 24 2021, 4:05 PM

LGTM.

llvm/docs/GettingStartedVS.rst
55

The Visual Studio installer installs a slightly older git (2.31.1) than the standalone installer from the web site (2.33).

True. But the good news is that git will prompt you to update within a day or so.

I recall some Clang lit tests were failing because of missing utilities like sed.

$ where sed
C:\Program Files\Git\usr\bin\sed.exe

The only missing utility that I'm aware of is make, which is necessary for lldb's tests.

I generally agree that projects other than llvm and clang are out of scope for this page. It would be nice, however, if additional projects were strictly additive. That is, if you've gone through these steps and now you also want to build llvm-xyzzy, having to install a couple more dependents is fine. But if you'd have to undo part of your setup and do something else instead, that would be (and has been) annoying. Right now, I think we're in a good state.

Upload lal local commits to include all local changes

This revision was landed with ongoing or failed builds.Aug 26 2021, 11:21 AM
This revision was automatically updated to reflect the committed changes.

Thanks for all the comments!

continuing from D119074, there are still a bunch of references to gnuwin around the codebase, those should be cleaned up

git bash is great for development, but it's kind of hard to automatically download and install on bots. our LLVM bots have git available via Chrome tools (specifically depot_tools)

git bash is great for development, but it's kind of hard to automatically download and install on bots. our LLVM bots have git available via Chrome tools (specifically depot_tools)

The premerge buildbots install it in their Docker image with Chocolatey, with a single command choco install -y git, see https://github.com/google/llvm-premerge-checks/blob/main/containers/agent-windows-vs2019/Dockerfile#L41-L43.

kuhnel added a comment.EditedFeb 17 2022, 12:35 AM

We're actually using chocolatey for all tools that it offers in pre-merge and that works very well, much better than downloading installers and executing them...

Visual Studio was quite tricky to automate and required lots of trial-and-error. But bot owners can copy and past from the docker image if they want to. We could also share a Visual Studio base image if anyone cares...

We're actually using chocolatey for all tools that it offers in pre-merge and that works very well, much better than downloading installers and executing them...

Which package are you using to get gnu tools?

Visual Studio was quite tricky to automate and required lots of trial-and-error. But bot owners can copy and past from the docker image if they want to. We could also share a Visual Studio base image if anyone cares...

I mostly work with the Chrome LLVM bots, and Chrome has its own installation of Visual Studio on its Windows bots so that's not a problem for us.

We're actually using chocolatey for all tools that it offers in pre-merge and that works very well, much better than downloading installers and executing them...

Which package are you using to get gnu tools?

Git for Windows comes with its own installation of a bunch of them, and llvm's lit does pick them up automatically if they aren't available in PATH: https://github.com/llvm/llvm-project/blob/main/llvm/utils/lit/lit/llvm/config.py#L31-L35

Git for Windows comes with its own installation of a bunch of them, and llvm's lit does pick them up automatically if they aren't available in PATH: https://github.com/llvm/llvm-project/blob/main/llvm/utils/lit/lit/llvm/config.py#L31-L35

It's easy to have Git for Windows installed by the Visual Studio installer itself : in Workloads tab, select the Desktop development with C++ workload. Under Individual components tab, select Git for Windows.

We're actually using chocolatey for all tools that it offers in pre-merge and that works very well, much better than downloading installers and executing them...

Which package are you using to get gnu tools?

Oh, looks like @goncharov has changed from GnuWin to git almost a year ago. In the past we were using this package.

You can find the details on the tool installation in the Windows Dockerfile.