Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

cap-lints when building rustdoc JSON #357

Merged
merged 5 commits into from
Feb 11, 2023

Conversation

Emilgardis
Copy link
Contributor

@Emilgardis Emilgardis commented Feb 9, 2023

Due to using --document-private-items --document-hidden-items, some new warnings not previously found by the creator(s) of a crate can be triggered as most do not check that rustdoc builds when documenting private/hidden items.

This also happens when rustdoc behaviour changes between versions, see obi1kenobi/cargo-semver-checks-action#17

This fixes that

@Emilgardis
Copy link
Contributor Author

Was bit by this when using

#![deny(rustdoc::broken_intra_doc_links)]

@obi1kenobi
Copy link
Owner

Wow, thanks -- this is awesome, I didn't know it existed and we've needed something like it before. A couple of questions:

  • Could you share the crate and git hash/version where this came up? I'd love it if we could add it to our CI test suite as a regression test.
  • Do you know if using --cap-lints forces a full rebuild or not? We were considering using the RUSTFLAGS env var to allow all lints, but decided against using that always & by default because it would always cause a full rebuild which would be unnecessarily expensive.

@Emilgardis
Copy link
Contributor Author

It doesn't trigger a rebuild for me atleast

got this specifically on twitch_api v0.7.0-rc.4 as baseline, however I wouldn't use that as a test case as I'm most likely going to be janking all versions (including release-candidates) of that crate before 0.7.0 (and all versions <0.7.0 have already been janked)

@obi1kenobi
Copy link
Owner

It doesn't trigger a rebuild for me atleast

Awesome!

got this specifically on twitch_api v0.7.0-rc.4 as baseline

If you don't mind specifying a repo and gitsha for that, we can add it as a test like that too. It doesn't have to be published, we'll just check out that commit specifically and use the --baseline-rev flag to set a baseline.

@Emilgardis
Copy link
Contributor Author

here's the commit da7c6cc60963cf47a9f8e9394744eaf14643b488 on https://github.com/twitch-rs/twitch_api

@jyn514
Copy link

jyn514 commented Feb 9, 2023

Do you know if using --cap-lints forces a full rebuild or not? We were considering using the RUSTFLAGS env var to allow all lints, but decided against using that always & by default because it would always cause a full rebuild which would be unnecessarily expensive.

RUSTFLAGS affects all dependencies, including dependencies not in the workspace. RUSTDOCFLAGS is only passed to rustdoc, so it only affects crates in the workspace, since you're passing --no-deps to rustdoc.

@obi1kenobi
Copy link
Owner

Thanks @jyn514, that makes sense!

I'll add the test case to this PR shortly and then merge.

@Emilgardis
Copy link
Contributor Author

Emilgardis commented Feb 9, 2023

just fyi, maybe not the best test case to check for only this, since it seems on the current main to take a loong time to lint the crate :3

@obi1kenobi
Copy link
Owner

Thanks for the heads-up! I'll take a look at why it's taking so long, hopefully something I might be able to fix while I'm at it.

@obi1kenobi obi1kenobi enabled auto-merge (squash) February 11, 2023 03:55
@obi1kenobi obi1kenobi disabled auto-merge February 11, 2023 03:55
@obi1kenobi
Copy link
Owner

Good news and bad news on the slow check time.

The good news is that I know what the problem is and I have a solution in mind that's guaranteed to fix it: https://predr.ag/blog/speeding-up-rust-semver-checking-by-over-2000x/

It's because twitch_api is a large crate with ~69k items, around 30% of the size of the crate I used in the example in that blog post.

Bad news: it's not something I'll be able to fix today or this week. It needs a bit more polishing and, while I'm doing the best I can, I'm stretched quite thin.

I'll try to tweak some of the queries to see if I can get some quicker wins in the meantime.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants