Skip to content
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

Allow Check targets needed for optional doc-scraping to fail without killing the build #11450

Merged
merged 2 commits into from
Dec 11, 2022

Conversation

willcrichton
Copy link
Contributor

@willcrichton willcrichton commented Dec 3, 2022

What does this PR try to resolve?

In doing a Crater run of -Zrustdoc-scrape-examples, I found that the only remaining regressions are cases where:

  • A library does not type-check
  • The library has examples
  • Cargo tries to scrape the examples, which requires checking the library
  • The Check unit for the library fails, crashing the build

The core issue is that the Check unit should be able to fail without killing the build. This PR fixes this issue by checking for this condition, and then allowing the unit to fail.

Specifically, I added a new method BuildContext::unit_can_fail_for_docscraping that determines the conditions for whether a unit is allowed to fail. This method is used both in JobQueue to interpret process failure, and in the rustc/rustdoc functions to emit a warning upon failure. I modified rustc to handle the case of failure similar to rustdoc, but with a slightly different diagnostic.

How should we test and review this PR?

The unit test no_fail_bad_lib has been extended with example files to test this case.

r? @weihanglo

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 3, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 8, 2022
…provements, r=notriddle

Improve Rustdoc scrape-examples UI

This PR combines a few different improvements to the scrape-examples UI. See a live demo here: https://willcrichton.net/misc/scrape-examples/small-first-example/clap/struct.Arg.html

### 1. The first scraped example now takes up significantly less screen height.
Inserting the first scraped example takes up a lot of vertical screen space. I don't want this addition to overwhelm users, so I decided to reduce the height of the initial example in two ways: (A) the default un-expanded height is reduced from 240px (10 LOC) to 120px (5 LOC), and (B) the link to the example is now positioned *over* the example instead of *atop* the example (only on desktop though, not mobile). The changes to `scrape-examples.js` and `rustdoc.css` implement this fix.

Here is what an example docblock now looks like:

![Screen Shot 2022-12-06 at 10 02 21 AM](https://user-images.githubusercontent.com/663326/205987450-3940063c-5973-4a34-8579-baff6a43aa9b.png)

### 2. Expanding all docblocks will not expand "More examples".
The "More examples blocks" are huge, so fully expanding everything on the page would take up too much vertical space. The changes to `main.js` implement this fix. This is tested in `scrape-examples-toggle.goml`.

### 3. Examples from binary crates are sorted higher than examples from library crates.
Code that is written as an example of an API is probably better for learning than code that happens to use an API, but isn't intended for pedagogic purposes. Unfortunately Rustc doesn't know whether a particular crate comes from an example target (only Cargo knows this). But we can at least create a proxy that prefers examples from binary crates over library crates, which we know from `--crate-type`.

This change is implemented by adding a new field `bin_crate` in `Options` (see `config.rs`). An `is_bin` field has been added to the scraped examples metadata (see `scrape_examples.rs`). Then the example sorting metric uses `is_bin` as the first entry of a lexicographic sort on `(is_bin, example_size, display_name)` (see `render/mod.rs`).

Note that in the future we can consider adding another flag like `--scrape-examples-cargo-target` that would pass target information from Cargo into the example metadata. But I'm proposing a less intrusive change for now.

### 4. The scrape-examples help page has been updated to reflect the latest Cargo interface.

See `scrape-examples-help.md`.

r? `@notriddle`

P.S. once this PR and rust-lang/cargo#11450 are merged, then I think the scrape-examples feature is officially ready for deployment on docs.rs!
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 8, 2022
…provements, r=notriddle

Improve Rustdoc scrape-examples UI

This PR combines a few different improvements to the scrape-examples UI. See a live demo here: https://willcrichton.net/misc/scrape-examples/small-first-example/clap/struct.Arg.html

### 1. The first scraped example now takes up significantly less screen height.
Inserting the first scraped example takes up a lot of vertical screen space. I don't want this addition to overwhelm users, so I decided to reduce the height of the initial example in two ways: (A) the default un-expanded height is reduced from 240px (10 LOC) to 120px (5 LOC), and (B) the link to the example is now positioned *over* the example instead of *atop* the example (only on desktop though, not mobile). The changes to `scrape-examples.js` and `rustdoc.css` implement this fix.

Here is what an example docblock now looks like:

![Screen Shot 2022-12-06 at 10 02 21 AM](https://user-images.githubusercontent.com/663326/205987450-3940063c-5973-4a34-8579-baff6a43aa9b.png)

### 2. Expanding all docblocks will not expand "More examples".
The "More examples blocks" are huge, so fully expanding everything on the page would take up too much vertical space. The changes to `main.js` implement this fix. This is tested in `scrape-examples-toggle.goml`.

### 3. Examples from binary crates are sorted higher than examples from library crates.
Code that is written as an example of an API is probably better for learning than code that happens to use an API, but isn't intended for pedagogic purposes. Unfortunately Rustc doesn't know whether a particular crate comes from an example target (only Cargo knows this). But we can at least create a proxy that prefers examples from binary crates over library crates, which we know from `--crate-type`.

This change is implemented by adding a new field `bin_crate` in `Options` (see `config.rs`). An `is_bin` field has been added to the scraped examples metadata (see `scrape_examples.rs`). Then the example sorting metric uses `is_bin` as the first entry of a lexicographic sort on `(is_bin, example_size, display_name)` (see `render/mod.rs`).

Note that in the future we can consider adding another flag like `--scrape-examples-cargo-target` that would pass target information from Cargo into the example metadata. But I'm proposing a less intrusive change for now.

### 4. The scrape-examples help page has been updated to reflect the latest Cargo interface.

See `scrape-examples-help.md`.

r? ``@notriddle``

P.S. once this PR and rust-lang/cargo#11450 are merged, then I think the scrape-examples feature is officially ready for deployment on docs.rs!
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 9, 2022
…provements, r=notriddle

Improve Rustdoc scrape-examples UI

This PR combines a few different improvements to the scrape-examples UI. See a live demo here: https://willcrichton.net/misc/scrape-examples/small-first-example/clap/struct.Arg.html

### 1. The first scraped example now takes up significantly less screen height.
Inserting the first scraped example takes up a lot of vertical screen space. I don't want this addition to overwhelm users, so I decided to reduce the height of the initial example in two ways: (A) the default un-expanded height is reduced from 240px (10 LOC) to 120px (5 LOC), and (B) the link to the example is now positioned *over* the example instead of *atop* the example (only on desktop though, not mobile). The changes to `scrape-examples.js` and `rustdoc.css` implement this fix.

Here is what an example docblock now looks like:

![Screen Shot 2022-12-06 at 10 02 21 AM](https://user-images.githubusercontent.com/663326/205987450-3940063c-5973-4a34-8579-baff6a43aa9b.png)

### 2. Expanding all docblocks will not expand "More examples".
The "More examples blocks" are huge, so fully expanding everything on the page would take up too much vertical space. The changes to `main.js` implement this fix. This is tested in `scrape-examples-toggle.goml`.

### 3. Examples from binary crates are sorted higher than examples from library crates.
Code that is written as an example of an API is probably better for learning than code that happens to use an API, but isn't intended for pedagogic purposes. Unfortunately Rustc doesn't know whether a particular crate comes from an example target (only Cargo knows this). But we can at least create a proxy that prefers examples from binary crates over library crates, which we know from `--crate-type`.

This change is implemented by adding a new field `bin_crate` in `Options` (see `config.rs`). An `is_bin` field has been added to the scraped examples metadata (see `scrape_examples.rs`). Then the example sorting metric uses `is_bin` as the first entry of a lexicographic sort on `(is_bin, example_size, display_name)` (see `render/mod.rs`).

Note that in the future we can consider adding another flag like `--scrape-examples-cargo-target` that would pass target information from Cargo into the example metadata. But I'm proposing a less intrusive change for now.

### 4. The scrape-examples help page has been updated to reflect the latest Cargo interface.

See `scrape-examples-help.md`.

r? `@notriddle`

P.S. once this PR and rust-lang/cargo#11450 are merged, then I think the scrape-examples feature is officially ready for deployment on docs.rs!
Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable. Thank you!

src/cargo/core/compiler/rustdoc.rs Show resolved Hide resolved
src/cargo/core/compiler/mod.rs Outdated Show resolved Hide resolved
src/cargo/core/compiler/mod.rs Outdated Show resolved Hide resolved
src/cargo/core/compiler/mod.rs Outdated Show resolved Hide resolved
src/cargo/core/compiler/mod.rs Outdated Show resolved Hide resolved
RalfJung pushed a commit to RalfJung/miri that referenced this pull request Dec 11, 2022
…s, r=notriddle

Improve Rustdoc scrape-examples UI

This PR combines a few different improvements to the scrape-examples UI. See a live demo here: https://willcrichton.net/misc/scrape-examples/small-first-example/clap/struct.Arg.html

### 1. The first scraped example now takes up significantly less screen height.
Inserting the first scraped example takes up a lot of vertical screen space. I don't want this addition to overwhelm users, so I decided to reduce the height of the initial example in two ways: (A) the default un-expanded height is reduced from 240px (10 LOC) to 120px (5 LOC), and (B) the link to the example is now positioned *over* the example instead of *atop* the example (only on desktop though, not mobile). The changes to `scrape-examples.js` and `rustdoc.css` implement this fix.

Here is what an example docblock now looks like:

![Screen Shot 2022-12-06 at 10 02 21 AM](https://user-images.githubusercontent.com/663326/205987450-3940063c-5973-4a34-8579-baff6a43aa9b.png)

### 2. Expanding all docblocks will not expand "More examples".
The "More examples blocks" are huge, so fully expanding everything on the page would take up too much vertical space. The changes to `main.js` implement this fix. This is tested in `scrape-examples-toggle.goml`.

### 3. Examples from binary crates are sorted higher than examples from library crates.
Code that is written as an example of an API is probably better for learning than code that happens to use an API, but isn't intended for pedagogic purposes. Unfortunately Rustc doesn't know whether a particular crate comes from an example target (only Cargo knows this). But we can at least create a proxy that prefers examples from binary crates over library crates, which we know from `--crate-type`.

This change is implemented by adding a new field `bin_crate` in `Options` (see `config.rs`). An `is_bin` field has been added to the scraped examples metadata (see `scrape_examples.rs`). Then the example sorting metric uses `is_bin` as the first entry of a lexicographic sort on `(is_bin, example_size, display_name)` (see `render/mod.rs`).

Note that in the future we can consider adding another flag like `--scrape-examples-cargo-target` that would pass target information from Cargo into the example metadata. But I'm proposing a less intrusive change for now.

### 4. The scrape-examples help page has been updated to reflect the latest Cargo interface.

See `scrape-examples-help.md`.

r? `@notriddle`

P.S. once this PR and rust-lang/cargo#11450 are merged, then I think the scrape-examples feature is officially ready for deployment on docs.rs!
Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! It's good to merge :)

@weihanglo
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Dec 11, 2022

📌 Commit 29407d0 has been approved by weihanglo

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 11, 2022
@bors
Copy link
Collaborator

bors commented Dec 11, 2022

⌛ Testing commit 29407d0 with merge 4a8d17e...

@bors
Copy link
Collaborator

bors commented Dec 11, 2022

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing 4a8d17e to master...

@bors bors merged commit 4a8d17e into rust-lang:master Dec 11, 2022
weihanglo added a commit to weihanglo/rust that referenced this pull request Dec 14, 2022
8 commits in 70898e522116f6c23971e2a554b2dc85fd4c84cd..cc0a320879c17207bbfb96b5d778e28a2c62030d
2022-12-05 19:43:44 +0000 to 2022-12-14 14:46:57 +0000
- artifact deps should works when target field specified coexists with `optional = true` (rust-lang/cargo#11434)
- Add `home` crate to cargo crates (rust-lang/cargo#11359)
- Use proper git URL for GitHub repos (rust-lang/cargo#11475)
- Fixes flock(fd, LOCK_UN) emulation on Solaris. (rust-lang/cargo#11474)
- Allow Check targets needed for optional doc-scraping to fail without killing the build (rust-lang/cargo#11450)
- fix: gleaning rustdocflags from `target.cfg(…)` is not supported (rust-lang/cargo#11323)
- Fix typo (rust-lang/cargo#11470)
- resolver.md: Fix naming in example (rust-lang/cargo#11469)
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 14, 2022
Update cargo

8 commits in 70898e522116f6c23971e2a554b2dc85fd4c84cd..cc0a320879c17207bbfb96b5d778e28a2c62030d 2022-12-05 19:43:44 +0000 to 2022-12-14 14:46:57 +0000
- artifact deps should works when target field specified coexists with `optional = true` (rust-lang/cargo#11434)
- Add `home` crate to cargo crates (rust-lang/cargo#11359)
- Use proper git URL for GitHub repos (rust-lang/cargo#11475)
- Fixes flock(fd, LOCK_UN) emulation on Solaris. (rust-lang/cargo#11474)
- Allow Check targets needed for optional doc-scraping to fail without killing the build (rust-lang/cargo#11450)
- fix: gleaning rustdocflags from `target.cfg(…)` is not supported (rust-lang/cargo#11323)
- Fix typo (rust-lang/cargo#11470)
- resolver.md: Fix naming in example (rust-lang/cargo#11469)

r? `@ghost`
@ehuss ehuss added this to the 1.68.0 milestone Dec 30, 2022
@weihanglo weihanglo added the Z-scrape-examples Nightly: rustdoc scrape examples label Sep 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. Z-scrape-examples Nightly: rustdoc scrape examples
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants