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

fs: use cfgs on fns instead of OS ext traits #3264

Merged
merged 1 commit into from Dec 14, 2020
Merged

Conversation

carllerche
Copy link
Member

Instead of using OS specific extension traits, OS specific methods are
moved onto the structs themselves and guarded with cfg. The API
documentation should highlight the function is platform specific.

Closes #2925

Instead of using OS specific extension traits, OS specific methods are
moved onto the structs themselves and guarded with `cfg`. The API
documentation should highlight the function is platform specific.

Closes #2925
@carllerche carllerche added this to the v1.0 milestone Dec 12, 2020
@Darksonn Darksonn added A-tokio Area: The main tokio crate M-fs Module: tokio/fs labels Dec 13, 2020
Comment on lines +94 to +102
feature! {
#![windows]

mod symlink_dir;
pub use self::symlink_dir::symlink_dir;

mod symlink_file;
pub use self::symlink_file::symlink_file;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This will not show up on docs.rs at all. We should do something about that.

Copy link
Member Author

Choose a reason for hiding this comment

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

It does if you pick windows? What happened before?

Copy link
Contributor

Choose a reason for hiding this comment

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

It did not show up previously on the default view either, but I'm pretty sure rustdoc wont complain if we compile them on windows or docsrs, since it doesn't compile the body of the functions, only their signature.

Copy link
Member Author

Choose a reason for hiding this comment

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

Interesting, I can give it a shot later.

Copy link
Member Author

Choose a reason for hiding this comment

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

I gave it a shot, rustdoc did not like it due to broken intra doc links:

error: unresolved link to `std::os::windows::fs::symlink_dir`
  --> tokio/src/fs/symlink_dir.rs:13:12
   |
13 | /// [std]: std::os::windows::fs::symlink_dir
   |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no item named `windows` in module `os`
   |
note: the lint level is defined here
  --> tokio/src/lib.rs:13:26
   |
13 | #![cfg_attr(docsrs, deny(broken_intra_doc_links))]
   |                          ^^^^^^^^^^^^^^^^^^^^^^

error: unresolved link to `std::os::windows::fs::symlink_file`
  --> tokio/src/fs/symlink_file.rs:13:12
   |
13 | /// [std]: std::os::windows::fs::symlink_file
   |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no item named `windows` in module `os`

error: unresolved link to `std::os::windows::fs::symlink_dir`
  --> tokio/src/fs/symlink_dir.rs:13:12
   |
13 | /// [std]: std::os::windows::fs::symlink_dir
   |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no item named `windows` in module `os`

The references fns do not exist in std under unix. This is non-standard, so I vote we skip unless you have another strategy to try?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hm. Annoying. But yes, let's not do it for now.

@carllerche carllerche merged commit 3f29212 into master Dec 14, 2020
@carllerche carllerche deleted the no-os-ext-trait branch December 14, 2020 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate M-fs Module: tokio/fs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

meta: reconsider OS specific Ext traits
2 participants