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

Tracking issue for Rust 2024: Fix doctest include paths #132230

Closed
5 tasks done
traviscross opened this issue Oct 27, 2024 · 12 comments
Closed
5 tasks done

Tracking issue for Rust 2024: Fix doctest include paths #132230

traviscross opened this issue Oct 27, 2024 · 12 comments
Labels
A-edition-2024 Area: The 2024 edition C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. S-tracking-ready-for-edition Status: This issue is ready for inclusion in the edition. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@traviscross
Copy link
Contributor

traviscross commented Oct 27, 2024

This is a tracking issue for fixing the path used for include in rustdoc doctests.

About tracking issues

Tracking issues are used to record the overall progress of implementation. They are also used as hubs connecting to other relevant issues, e.g., bugs or open design questions. A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature. Instead, open a dedicated issue for the specific matter and add the relevant feature gate label.

Steps

Unresolved Questions

  • Should this be an edition item for Rust 2024?

Related

cc @rust-lang/rustdoc @ehuss @notriddle @GuillaumeGomez

@traviscross traviscross added A-edition-2024 Area: The 2024 edition C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Oct 27, 2024
@notriddle
Copy link
Contributor

I ran a grep over the Reference, and only https://doc.rust-lang.org/1.82.0/reference/input-format.html and https://doc.rust-lang.org/1.82.0/reference/attributes.html mention include_str!. I checked it off.

@notriddle
Copy link
Contributor

@rfcbot fcp merge

Since the compiler can already auto-correct include! paths, I don't think this needs a migration lint. But if you disagree, and can come up with a way to make it work, please let me know!

@rfcbot
Copy link

rfcbot commented Oct 28, 2024

Team member @notriddle has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Oct 28, 2024
@ehuss
Copy link
Contributor

ehuss commented Oct 29, 2024

I'd like to stress that we need to move quickly if we want to make this change. The crater run should have started a few weeks ago. We have the last edition PR going in as I write this, and we were planning to start the crater run today. Delaying for this is stressing our timeline.

I would encourage considering the forward-compatible warning approach suggested in #132210 (comment) as a way to simplify the problem and the code, since you won't need to retain the arguably old broken behavior. (Or, just accept the breakage in #132203 and help those packages fix their tests.)

Can the rustdoc team focus on getting this resolved and merged within a few days? I would prefer to not delay for something last minute like this.

@traviscross
Copy link
Contributor Author

Given the timeline, and building on what @ehuss said, here are the questions and items probably most in need of focus:

  1. Might the @rust-lang/rustdoc team just accept the breakage in Rust 1.83?
    • This is arguably a bug-fix, which is normally allowed breakage.
    • The number of crates affected is not large.
    • Unlike other forms of breakage we worry a lot about, this does not affect the dependency graph or whether the program works.
  2. If the team does want this to be an edition item, then please:
    • Post the chapter to the edition guide tomorrow (cc @notriddle).
    • While the FCP can continue, in the interest of time we need a presumptive decision by the team lead about the direction here. I.e., @GuillaumeGomez, if this is an ask for an edition item, please confirm that's the direction you want to go.
    • With the edition guide done and that presumptive decision, we'll merge rustdoc: make doctest span tweak a 2024 edition change #132210, which will allow us to go ahead with our edition crater runs.

@notriddle
Copy link
Contributor

I would encourage considering the forward-compatible warning approach suggested

The problem is that the only way to get rid of the warning is to bump the edition, or to stop using include_bytes! in your README examples, or to stop using #![doc=include_str!("../README.md")] to test your README examples. There's no localized fix to make the code work both before and after the compiler change at the same time.

This is also why I'd rather not just accept the breakage. There's no way for the affected crates to fix nightly without breaking stable.

Post the chapter to the edition guide tomorrow

rust-lang/edition-guide#329

@GuillaumeGomez
Copy link
Member

I confirm this is the direction I want to go. I'm surprised an FCP is needed here though but I trust @notriddle's judgement. Let's try to get it approved by tomorrow.

traviscross pushed a commit to rust-lang/edition-guide that referenced this issue Oct 30, 2024
@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Oct 30, 2024
@rfcbot
Copy link

rfcbot commented Oct 30, 2024

🔔 This is now entering its final comment period, as per the review above. 🔔

@GuillaumeGomez
Copy link
Member

It got approved. I think we can exceptionally ignore the FCP if needed. If no one is against it, I'll r+ this evening.

@ehuss
Copy link
Contributor

ehuss commented Oct 30, 2024

Thank you, I appreciate it!

@traviscross
Copy link
Contributor Author

@rustbot labels +S-tracking-ready-for-edition

This item is now ready for Rust 2024. Thanks to @notriddle, @GuillaumeGomez, and the rustdoc team for putting everything in place quickly here.

@rustbot rustbot added the S-tracking-ready-for-edition Status: This issue is ready for inclusion in the edition. label Oct 31, 2024
@camelid
Copy link
Member

camelid commented Nov 1, 2024

@rfcbot cancel

Given the unusual conditions, the FCP was foreshortened but rfcbot doesn't realize that. Let's cancel it to avoid confusion later.

@rfcbot rfcbot removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Nov 1, 2024
@camelid camelid added disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. labels Nov 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-edition-2024 Area: The 2024 edition C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. S-tracking-ready-for-edition Status: This issue is ready for inclusion in the edition. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

7 participants