-
Notifications
You must be signed in to change notification settings - Fork 45
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
Improve docs.rs builds #195
Conversation
@paholg I'm looking to get |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay. Just want to make sure we're not breaking anything for anyone with removal of force_unix_path_separator
and I'll be happy to merge/publish.
Can you be more specific? Typenum's CI runs builds on Windows, so there are at least some cases where it doesn't. |
Yes, seems it only occurs in a situation where mod gen1 {
include!(r"D:\Projects\typenum_test/include.rs");
}
mod gen2 {
include!(r"\\?\D:\Projects\typenum_test\include.rs");
}
mod gen3 {
include!(r"\\?\D:\Projects\typenum_test/include.rs");
} first two work, but the last causes an error Windows canonical path |
It's this specific issue: rust-lang/rust#75075 (comment) Error happens if https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation
|
I do actually kind of like the way that hydroflow PR handled the dynamic include path. That likely wouldn’t interfere with the docs.rs stuff like the previous system, which was the point of this PR. Still, the issue as you described this is a pervasive issue with Windows/Rust in general and to patch it in a few specific crates is a bandaid at best. |
I will merge a mitigation fix in hydroflow to remove the |
@MingweiSamuel I would welcome a PR. My concern is if it falls to the same issues that my prior attempts had. Ideally, we would reproduce the various problematic setups in CI to ensure things work for everyone, but I'm not sure how realistic that is. |
While working on
generic-array 1.0
, I noticed docs.rs does not contain theconst-generics
portions oftypenum
, so the links were broken. Enabling support for that with automatic "Available on crate featureconst-generics
only." text would have been easy, except for theforce_unix_path_separator
feature interfering with it.After looking through the commit history, your original commit and message 694130d implied enabling the feature made it more compatible. If I'm wrong, please correct me, but I don't see any reason why this should not be the default behavior. Making
force_unix_path_separator
the default behavior has no visible downsides and allowsdoc_auto_cfg
to function correctly, more or less.I added a couple extra hints to rustdoc for i128 implementations as well. Unfortunately, these rustdoc hints don't work for the
scale-info
derives, so they are still excluded from docs.rs.As a bonus, I added a Rust Playground metadata section to enable i128 and const-generics support there.