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

Support multiple versions of MdBook for docs #57495

Open
wants to merge 8 commits into
base: master
from

Conversation

Projects
None yet
6 participants
@jamesmunns
Copy link
Member

jamesmunns commented Jan 10, 2019

Only the compatibility items from the embedded book PR. PR with embedded book components: #56291

CC @steveklabnik, @ehuss, and rust-lang-nursery/edition-guide#134

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Jan 10, 2019

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@jamesmunns jamesmunns changed the title Only the compatibility items from the embedded book PR Support multiple versions of MdBook for docs Jan 10, 2019

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Jan 10, 2019

The job x86_64-gnu-llvm-6.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
travis_time:end:13daa8e1:start=1547149245340135286,finish=1547149248225762036,duration=2885626750
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#pull-requests-and-security-restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
Setting environment variables from .travis.yml
$ export IMAGE=x86_64-gnu-llvm-6.0
---
[00:01:38] downloading https://static.rust-lang.org/dist/2019-01-04/cargo-beta-x86_64-unknown-linux-gnu.tar.gz
[00:01:39] 
######################################################################## 100.0%
[00:01:39] extracting /checkout/obj/build/cache/2019-01-04/cargo-beta-x86_64-unknown-linux-gnu.tar.gz
[00:01:39] warning: /checkout/src/tools/rustbook/Cargo.toml: the cargo feature `rename-dependency` is now stable and is no longer necessary to be listed in the manifest
[00:01:39]     Updating crates.io index
[00:01:48] error: the lock file /checkout/Cargo.lock needs to be updated but --locked was passed to prevent this
[00:01:48] Build completed unsuccessfully in 0:00:22
[00:01:48] Makefile:71: recipe for target 'prepare' failed
[00:01:48] make: *** [prepare] Error 1
[00:01:49] Command failed. Attempt 2/5:
[00:01:49] Command failed. Attempt 2/5:
[00:01:49] warning: /checkout/src/tools/rustbook/Cargo.toml: the cargo feature `rename-dependency` is now stable and is no longer necessary to be listed in the manifest
[00:01:49] error: the lock file /checkout/Cargo.lock needs to be updated but --locked was passed to prevent this
[00:01:49] Build completed unsuccessfully in 0:00:00
[00:01:49] Makefile:71: recipe for target 'prepare' failed
[00:01:49] make: *** [prepare] Error 1
[00:01:51] Command failed. Attempt 3/5:
[00:01:51] Command failed. Attempt 3/5:
[00:01:51] warning: /checkout/src/tools/rustbook/Cargo.toml: the cargo feature `rename-dependency` is now stable and is no longer necessary to be listed in the manifest
[00:01:52] error: the lock file /checkout/Cargo.lock needs to be updated but --locked was passed to prevent this
[00:01:52] Build completed unsuccessfully in 0:00:00
[00:01:52] make: *** [prepare] Error 1
[00:01:52] Makefile:71: recipe for target 'prepare' failed
[00:01:55] Command failed. Attempt 4/5:
[00:01:55] Command failed. Attempt 4/5:
[00:01:55] warning: /checkout/src/tools/rustbook/Cargo.toml: the cargo feature `rename-dependency` is now stable and is no longer necessary to be listed in the manifest
[00:01:55] error: the lock file /checkout/Cargo.lock needs to be updated but --locked was passed to prevent this
[00:01:55] Build completed unsuccessfully in 0:00:00
[00:01:55] make: *** [prepare] Error 1
[00:01:55] Makefile:71: recipe for target 'prepare' failed
[00:01:59] Command failed. Attempt 5/5:
[00:01:59] Command failed. Attempt 5/5:
[00:01:59] warning: /checkout/src/tools/rustbook/Cargo.toml: the cargo feature `rename-dependency` is now stable and is no longer necessary to be listed in the manifest
[00:01:59] error: the lock file /checkout/Cargo.lock needs to be updated but --locked was passed to prevent this
[00:01:59] Build completed unsuccessfully in 0:00:00
[00:01:59] make: *** [prepare] Error 1
[00:01:59] Makefile:71: recipe for target 'prepare' failed
[00:01:59] The command has failed after 5 attempts.
---
travis_time:end:091de74f:start=1547149382103166182,finish=1547149382108608360,duration=5442178
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:2912f45c
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:20e4d594
travis_time:start:20e4d594
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:100d9a48
$ dmesg | grep -i kill

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@jamesmunns

This comment has been minimized.

Copy link
Member

jamesmunns commented Jan 10, 2019

Fixing some things locally, will push updates soon

jamesmunns added some commits Jan 10, 2019

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Jan 10, 2019

@ehuss

This comment has been minimized.

Copy link
Contributor

ehuss commented Jan 10, 2019

Just a heads up, this will likely fail the license check. I think arrayref will need to be added to the exception list. I'm pretty sure the other dependencies will be ok, but I'm not certain.

Am I correct that this can't really be used until the print.html issue is solved?

@jamesmunns

This comment has been minimized.

Copy link
Member

jamesmunns commented Jan 10, 2019

@ehuss I was not aware of the license check

@jamesmunns

This comment has been minimized.

Copy link
Member

jamesmunns commented Jan 10, 2019

This may work for you, depending on how you are doing links. It does not work for the style the embedded-book uses, but I am not sure if your changes will have an issue.

You could rebase your changes on this PR, and try running python x.py test src/tools/linkchecker?

@steveklabnik

This comment has been minimized.

Copy link
Member

steveklabnik commented Jan 11, 2019

Hey @jamesmunns , thanks for this! I think this is a great step forward.

I:

  • built your PR locally
  • wrote a new book in tree with two pages, linking to each other
  • built with mdbook1 and watched the linkcheck fail
  • built with mdbook2 and watched only print.html fail

I'm not sure about the license check mention, given that travis passed here. @ehuss could you say more about the problem you see here?

Given that both of us are still running into rust-lang-nursery/mdBook#828, here's what I'd like to propose:

  1. I'm going to look into the print.html issue now
  2. we let this PR sit over the weekend, I highly doubt that I'll have a bugfix ready in the remaining time I have today
  3. Hopefully on Monday i'll have a patch and we can put out a new mdbook release, update this PR to include it, and then merge.

Sound like a plan?

Thanks again for working on this; I didn't realize that we could depend on both versions so easily!

@ehuss

This comment has been minimized.

Copy link
Contributor

ehuss commented Jan 11, 2019

I'm not sure about the license check mention, given that travis passed here. @ehuss could you say more about the problem you see here?

Yea, the license checks are only performed in the distcheck builder, which is not run on PRs. I've wondered in the past if it might be possible to run a reduced job on PRs that only did license checks, but I'm not sure if distcheck is too different to get an accurate result quickly (distcheck requires generating a release tarball, extracting it, yadda yadda). It's a pain because nobody knows about the problem until bors tries to merge.

AFAIK, it can be run locally using the following steps:

  1. Run cargo vendor
  2. Edit config.toml and set vendor to true.
  3. ./x.py test tidy

After it's done, don't forget to delete .cargo/config and unset vendor and maybe delete the vendor directory.

License rules and exceptions are stored in https://github.com/rust-lang/rust/blob/master/src/tools/tidy/src/deps.rs.

@steveklabnik

This comment has been minimized.

Copy link
Member

steveklabnik commented Jan 11, 2019

Ah ha, thanks. I wonder if that's something I forgot or something that's changed since last time I delt with this...

@jamesmunns

This comment has been minimized.

Copy link
Member

jamesmunns commented Jan 14, 2019

Hey @steveklabnik, I didn't see this (my inbox is a mess right now), but your plan sounds good! I'll be available during Berlin business hours this week as well if I can help with anything.

If you are able to fix the print.html issues, I can also get the embedded book PR fixed up ASAP!

@steveklabnik

This comment has been minimized.

Copy link
Member

steveklabnik commented Jan 14, 2019

It's all good!

So, between Friday and this morning, I think I've found the culprit. Bad news is, I'm not sure the fix will be easy. I'll fill in more details on the mdbook issue.

@steveklabnik

This comment has been minimized.

Copy link
Member

steveklabnik commented Jan 15, 2019

Update: I have a patch in (rust-lang-nursery/mdBook#866) testing it locally now.

@steveklabnik

This comment has been minimized.

Copy link
Member

steveklabnik commented Jan 15, 2019

Good news! Not only has my local test been successful, but I applied #56291 on top of it as well, and the tests seem to pass!

Next steps are to get rust-lang-nursery/mdBook#866 reviewed, a release cut, and update this PR to use it.

Thank you again for sticking through this, it's been a journey but I'm really excited about it. This "two versions of mdbook" strategy is going to work out much more nicely than my initial "oh god do the work of updating all of the books at the same time" thing I thought I had to do.

@steveklabnik

This comment has been minimized.

Copy link
Member

steveklabnik commented Jan 18, 2019

@jamesmunns I have released mdbook 0.2.3 with the fix; can you update this PR to use that version, and then we can merge 🎊

@jamesmunns

This comment has been minimized.

Copy link
Member

jamesmunns commented Jan 18, 2019

Sure! I'll try to get to this in the next 2-3 hours!

@jamesmunns

This comment has been minimized.

Copy link
Member

jamesmunns commented Jan 19, 2019

@steveklabnik updated to use the new 0.2.3 release. @ehuss I have also added a license exception for arrayref.

@jamesmunns

This comment has been minimized.

Copy link
Member

jamesmunns commented Jan 21, 2019

Hey @steveklabnik, reping on this for the r+ :)

@steveklabnik

This comment has been minimized.

Copy link
Member

steveklabnik commented Jan 21, 2019

@bors: r+

Thanks so much again!

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Jan 21, 2019

📌 Commit f3ac6b3 has been approved by steveklabnik

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jan 21, 2019

Rollup merge of rust-lang#57495 - jamesmunns:mdbook-compat, r=stevekl…
…abnik

Support multiple versions of MdBook for docs

Only the compatibility items from the embedded book PR. PR with embedded book components: rust-lang#56291

CC @steveklabnik, @ehuss, and rust-lang-nursery/edition-guide#134

bors added a commit that referenced this pull request Jan 21, 2019

Auto merge of #57816 - GuillaumeGomez:rollup, r=GuillaumeGomez
Rollup of 5 pull requests

Successful merges:

 - #57495 (Support multiple versions of MdBook for docs)
 - #57552 (Default images)
 - #57791 (Add regression test for #54582)
 - #57795 (Use structured suggestion in stead of notes)
 - #57798 (Corrected spelling inconsistency)

Failed merges:

r? @ghost
@bors

This comment has been minimized.

Copy link
Contributor

bors commented Jan 22, 2019

🔒 Merge conflict

This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again.

How do I rebase?

Assuming self is your fork and upstream is this repository, you can resolve the conflict following these steps:

  1. git checkout mdbook-compat (switch to your branch)
  2. git fetch upstream master (retrieve the latest master)
  3. git rebase upstream/master -p (rebase on top of it)
  4. Follow the on-screen instruction to resolve conflicts (check git status if you got lost).
  5. git push self mdbook-compat --force-with-lease (update this PR)

You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial.

Please avoid the "Resolve conflicts" button on GitHub. It uses git merge instead of git rebase which makes the PR commit history more difficult to read.

Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Cargo.lock conflict is handled during merge and rebase. This is normal, and you should still perform step 5 to update this PR.

Error message
warning: Cannot merge binary files: Cargo.lock (HEAD vs. heads/homu-tmp)
Auto-merging src/tools/tidy/src/deps.rs
Auto-merging Cargo.lock
CONFLICT (content): Merge conflict in Cargo.lock
Automatic merge failed; fix conflicts and then commit the result.

@jamesmunns

This comment has been minimized.

Copy link
Member

jamesmunns commented Jan 22, 2019

I'll do my best to address this tonight. @steveklabnik I've sent you an invite to my fork if you'd like to commit directly there (before I have a chance)

@jamesmunns

This comment has been minimized.

Copy link
Member

jamesmunns commented Jan 22, 2019

@steveklabnik this needs a re-up please!

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Jan 22, 2019

☔️ The latest upstream changes (presumably #57805) made this pull request unmergeable. Please resolve the merge conflicts.

@jamesmunns

This comment has been minimized.

Copy link
Member

jamesmunns commented Jan 22, 2019

Looks like Cargo.lock is getting rowdy today. Fixed another merge conflict, I guess we wait another couple hours for CI.

@jamesmunns

This comment has been minimized.

Copy link
Member

jamesmunns commented Jan 23, 2019

@steveklabnik okay, CI is green again! (sorry for the excess of pings, I want to get this merged before I get pulled away again)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment