Allow doctest to refer to external crates (in context of a Cargo project) #19
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…t` from running rudundantly and producing bogus errors.
1. LockedDep iterator: track change in metadata for dependency ID. it's no longer whitespace based. 2. (same): also handle package in cargo workspace inheriting version from workspace (ID doesn't have `@`) 3. KeeperConfigParser: change default for target_dir to `target/debug` if manifest_dir is specified, leave it as `target` if not. (arguable)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Addresses issue #9 (maybe?)
Certainly allows me to use external crates naturally in my book samples, when embedded in a Cargo project.
The symptom I had was compiler "use of undefined external crate", even though the crate was correctly configured as a dependency in my
Cargo.toml.With this fix:
manifest_dirin yourbook.toml(this tellsmdbook-keeperto read the Cargo.toml, which it does not do by default).target_dir. It now defaults totarget/debug(but only ifmanifest_dirwas configured above).mdbook serveas usual. Refrain from runningmdbook test! This runsmdbook-keeper' (which works) but then a second test pass viarustdocwithinmdbook, which will generate all the oldexternal crate` errors, making you think the plug in is not working (I was doing this for a while and was horribly confused).Summary of changes
The big issue is that
mdbook-keeperwasn't invokingrustcwith--externs, because it wasn't finding the extern dependencies that were declared inCargo.toml. The format of the package ID in theFingerprintseems to have changed sincerust-skepticanalyzed it: it no longer uses whitespace delimiters, rather it seems to be using<registryOrFilePath>#<libname>@<version>(or<registryOrFilePath>#<version>when package inherits version from the Cargo workspace definition, see comments inLockedDepiterator). This is all reverse engineering, I make no promises I got it right, or that it will stay right.Future directions
I'd like to move this logic into
mdbookitself and avoid the need formdbook-keeperwhen all you want is external crates in book code samples (and who doesn't?) As the redundantmdbook testbehavior shows, the preprocessor plugin interface is not the best place for the doctest logic.But the current dependency parsing logic depends on unstable internal interfaces from Cargo, I can well understand
mdbookteam will be reluctant, we need a stable interface from Cargo.I'm currently looking at the 'Compiler' interface in the Cargo api to see if it can be used to invoke
mdBook; that would get Cargo to provide the list of-Land --externargs that we need. But the moment, it looks like it's forrustdocandrustc` only. Any advice would be appreciated!