Page MenuHomePhabricator

Update GettingStarted guide to recommend that people use the new official Git repository.
ClosedPublic

Authored by jyknight on Jan 13 2019, 5:08 PM.

Details

Summary

Remove the directions for using git-svn, and demote the prominence of
the svn instructions.

Also, fix a few other issues while I'm in there:

  • Mention LLVM_ENABLE_PROJECTS more.
  • Getting started doesn't need to mention test-suite, but should mention clang and the other projects.
  • Remove mentions of "configure", since that's long gone.

I've also adjusted a few other mentions of svn to point to github, but
have not done so comprehensively.

Diff Detail

Repository
rL LLVM

Event Timeline

jyknight created this revision.Jan 13 2019, 5:08 PM
smeenai added inline comments.
llvm/docs/DeveloperPolicy.rst
85 ↗(On Diff #181493)

checkout from git? Or maybe clone.

llvm/docs/GettingStarted.rst
65 ↗(On Diff #181493)

Do you think LLVM_ENABLE_RUNTIMES is worth mentioning as well, or would that be information overload for a getting started guide?

Also, typo: semicolor -> semicolon

434 ↗(On Diff #181493)

so you'll

Well this is heroic.

llvm/docs/GettingStarted.rst
24 ↗(On Diff #181493)

Nit: "There's" stands for "There is", which doesn't fit. :)

26 ↗(On Diff #181493)

Comma after libc++?

41 ↗(On Diff #181493)

Maybe we could be explicit that this checks out "greater LLVM" rather than "lesser LLVM", especially since this is different from how it used to be.

96 ↗(On Diff #181493)

or just make -j?

438 ↗(On Diff #181493)

"points upstream" (or I suppose "points to the upstream repository")

438 ↗(On Diff #181493)

points to your

439 ↗(On Diff #181493)

s#is rebased onto#is based on origin/master without merges#? Like, rebasing is an action, and the result of the action is...that one branch is based on another without merges (or something).

444 ↗(On Diff #181493)

I wonder if now would be a good point to mention clang-format (and the git-clang-format script) instead? Like, overall that's probably more useful to introduce to beginners than git diff --check.

582 ↗(On Diff #181493)

Instead of or in addition to listing them, would it be worthwhile to explain where the canonical list of allowed strings lives (or how to print it)? That way the list never goes out of date.

llvm/docs/Phabricator.rst
200 ↗(On Diff #181493)

I would think that you don't want to merge the change into your local "master" branch, because you probably (?) want that to be a copy of origin/master, and upstream is going to rewrite this change. IOW git checkout master; git pull --ff-only origin/master will fail.

Would it make more sense to checkout arcpatch-D123, merge origin/master into it, and then git llvm push from that branch?

(I'm also confused why we're talking about patching in a change from phab -- isn't the most likely scenario that you already have the change in your git repository?)

mehdi_amini added inline comments.Jan 14 2019, 7:08 AM
llvm/docs/GettingStarted.rst
96 ↗(On Diff #181493)

make -j is "unbounded" parallelism, it use to hang my machine for a while last time I tried it on clang.

450 ↗(On Diff #181493)

These instructions assume that "my branch" is perfectly rebased, I think this should use three dots to do what we want: git diff origin/master...mybranch

llvm/docs/Phabricator.rst
200 ↗(On Diff #181493)

Would it make more sense to checkout arcpatch-D123, merge origin/master into it, and then git llvm push from that branch?

My usual flow would be to rebase: git checkout arcpatch-D123 && git fetch && git rebase origin/master

jyknight updated this revision to Diff 181641.Jan 14 2019, 1:35 PM
jyknight marked 20 inline comments as done.

Address various comments.

jyknight added inline comments.Jan 14 2019, 1:36 PM
llvm/docs/GettingStarted.rst
65 ↗(On Diff #181493)

I don't actually know what that does and when one would want to use it. So either I've been missing out terribly, or else it's not needed for a getting started guide. :)

96 ↗(On Diff #181493)

"make -j" is bad, it doesn't run number-of-CPUs jobs, instead it puts NO limit on the number of jobs. That usually, IME, eats up all your RAM and destroys your computer.

444 ↗(On Diff #181493)

Seems reasonable.

450 ↗(On Diff #181493)

Now that I look over this again, I don't think most of this belongs in here, at all. I rewrote this part to just point people to the Phab instructions, and just left a note that email is also OK.

582 ↗(On Diff #181493)

I have no idea how to print a list from cmake. Is that even possible? I don't think pointing to the source code would be very helpful, I'd prefer to just have an outdated list vs that.

llvm/docs/Phabricator.rst
200 ↗(On Diff #181493)

I've adjusted these instructions.

smeenai added inline comments.Jan 14 2019, 1:43 PM
llvm/docs/GettingStarted.rst
65 ↗(On Diff #181493)

It automatically sets up a two-stage build where the selected runtimes (it supports all of libc++, libunwind, compiler-rt, etc.) are built using the just-built clang instead of the host compiler. That's supposedly The Right Thing™ to do for building the runtimes, which is why I was thinking about promoting it, but it's probably not needed here.

jlebar accepted this revision.Jan 14 2019, 1:43 PM
jlebar added a subscriber: tra.
jlebar added inline comments.
llvm/docs/GettingStarted.rst
582 ↗(On Diff #181493)

I have no idea how to print a list from cmake. Is that even possible?

Heh, no idea either. /me summons @tra, he has suffered through more than his fair share of cmake.

This revision is now accepted and ready to land.Jan 14 2019, 1:43 PM
smeenai added inline comments.Jan 14 2019, 1:45 PM
llvm/docs/GettingStarted.rst
65 ↗(On Diff #181493)

The typo is still there, I think ... semicolor -> semicolon

jyknight marked an inline comment as done.Jan 14 2019, 2:29 PM

Thanks all! Going ahead and committing this now; if there's further changes requested, I'll do followup commits.

llvm/docs/GettingStarted.rst
65 ↗(On Diff #181493)

Oops, missed that half of the comment, now fixed.

This revision was automatically updated to reflect the committed changes.
tra added inline comments.Jan 14 2019, 3:05 PM
llvm/docs/GettingStarted.rst
582 ↗(On Diff #181493)

AFAICT, there's no explicitly defined canonical list of projects. Judging by the llvm/CMakeLists.txt it appears that a project must be a top-level repo directory :
https://github.com/llvm-mirror/llvm/blob/master/CMakeLists.txt#L113

I'm not sure if all top level directories are projects, though.