-
Notifications
You must be signed in to change notification settings - Fork 12.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Start shipping the Cargo book #45692
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Just one minor nit but otherwise r=me
src/bootstrap/doc.rs
Outdated
run.builder.ensure(CargoBook { | ||
target: run.target, | ||
name: INTERNER.intern_str("cargo"), | ||
src: INTERNER.intern_path(PathBuf::from("src/tools/cargo/src/doc/book")), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this and the name
field above are best left as local variables in run
perhaps because I don't think they're inputs so much as calculated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah makes sense, I did a bit TOO much cargo culting.
I'll update this after we decide what the name should be, and then
@@ -28,6 +28,7 @@ Rust provides a number of book-length sets of documentation, collectively | |||
nicknamed 'The Rust Bookshelf.' | |||
|
|||
* [The Rust Programming Language][book] teaches you how to program in Rust. | |||
* [The Cargo Book][cargo-book] is a guide to Cargo, Rust's build tool and dependency manager. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're calling it "The Cargo Manual" in the cargo repo. Should we update that to say "The Cargo Book"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm okay with either, but the rest are almost all called "Book". I'll defer to @rust-lang/cargo here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with "The Cargo Book", it jives better with me with "bookshelf"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool! Let's go with what you have here and I'll submit a PR for cargo to update the title there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR to update cargo: rust-lang/cargo#4695
4a7f3e6
to
ac90301
Compare
@bors: r=alexcrichton |
📌 Commit ac90301 has been approved by |
⌛ Testing commit ac90301c48fb4e1d1b05ac5b4be62be768b52ae5 with merge af64ce8e3be13f3bc1bed4021c25f1a4df74fc7d... |
💔 Test failed - status-travis |
|
Triage ping @steveklabnik — we haven't heard from you in a week and this PR has some test failures that need to be addressed! |
Whoops, thanks. Will look into this soon. |
Fix broken links in cargo book Needed to address failures in rust-lang/rust#45692
I checked these files ( |
Oh, I just saw that you fixed some of the links, @steveklabnik! Thanks! |
@alexcrichton are we free to just update the cargo repo whenever? We'd need to do that to land this PR, I thought it was generally updated once a cycle. I could be wrong. |
We're free to update the Cargo submodule whenever, yes. We try to do it at least once a cycle, but generally it happens about 3-5 times based on bugfixes landing. |
3ce57d2
to
8f0f3b1
Compare
⌛ Testing commit 8f0f3b131105b0723d4428b36118f4f9d32f91bf with merge 4ef9cb0494c4267c6f202d2b1a3832c00bdff5b5... |
💔 Test failed - status-travis |
FreeBSD, could not compile cargo because libc is updated. (Note that this will also affect at least NetBSD)
|
I... have no idea what to do here, then. |
@steveklabnik Cargo needs to modify |
I've opened rust-lang/cargo#4713 to hopefully fix this. |
rust-lang/cargo#4713 has been merged, the cargo submodule can be updated again. @steveklabnik |
☔ The latest upstream changes (presumably #45949) made this pull request unmergeable. Please resolve the merge conflicts. |
8f0f3b1
to
184077d
Compare
Should be resolved! |
I feel like this shouldn't need to update Cargo -- perhaps that's accidental / needs a rebase? I think current Cargo master is in |
Fixes rust-lang#44910 Fixes rust-lang#39588 See both of those bugs for more details.
184077d
to
3b32a3a
Compare
I had updated it because I needed to update before; I guess given the conflict, it's probably not needed. I've been travelling and so I haven't been paying as close attention to the queue as usual. I've removed that bit and rebased, let's see what Travis says. |
@bors: r=alexcrichton looks like this is passing! |
📌 Commit 3b32a3a has been approved by |
☀️ Test successful - status-appveyor, status-travis |
For some reason the book is not correctly built on auto-deploy. I filed a new issue for it: #46195 |
Fixes #44910
Fixes #39588
See both of those bugs for more details.