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: add support for DirBuilder and DirBuilderExt #2524

Merged
merged 6 commits into from
May 21, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
137 changes: 137 additions & 0 deletions tokio/src/fs/dir_builder.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
use crate::fs::asyncify;

use std::io;
use std::path::Path;

/// A builder for creating directories in various manners.
///
/// Additional Unix-specific options are available via importing the
/// [os::unix::fs::DirBuilderExt] trait.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we change the link text to [`DirBuilderExt`]? Note that the link is broken.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

///
/// This is a specialized version of [`std::fs::DirBuilder`] for usage from
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// This is a specialized version of [`std::fs::DirBuilder`] for usage from
/// This is a specialized version of [`std::fs::DirBuilder`] for usage on

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! Fixed

/// the Tokio runtime.
///
/// [os::unix::fs::DirBuilderExt]: ../os/unix/fs/trait.DirBuilderExt.html
/// [std::fs::DirBuilder]: https://doc.rust-lang.org/std/fs/struct.DirBuilder.html
Copy link
Contributor

Choose a reason for hiding this comment

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

This should use intra-rustdoc links, so that broken links are automatically detected.

Suggested change
/// [os::unix::fs::DirBuilderExt]: ../os/unix/fs/trait.DirBuilderExt.html
/// [std::fs::DirBuilder]: https://doc.rust-lang.org/std/fs/struct.DirBuilder.html
/// [os::unix::fs::DirBuilderExt]: crate::os::unix::fs::DirBuilderExt
/// [std::fs::DirBuilder]: std::fs::DirBuilder

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#[derive(Debug, Default)]
pub struct DirBuilder {
/// Indicates whether to create parent directories if they are missing.
recursive: bool,

/// Set the Unix mode for newly created directories.
#[cfg(unix)]
mode: Option<u32>,
}

impl DirBuilder {
/// Creates a new set of options with default mode/security settings for all
/// platforms and also non-recursive.
///
/// This is an async version of [`std::fs::DirBuilder::new`][std]
///
/// [std]: std::fs::DirBuilder::new
///
/// # Examples
///
/// ```no_run
/// use tokio::fs::DirBuilder;
///
/// let builder = DirBuilder::new();
/// ```
pub fn new() -> DirBuilder {
#[cfg(not(unix))]
let builder = DirBuilder { recursive: false };

#[cfg(unix)]
let builder = DirBuilder {
Copy link
Contributor

Choose a reason for hiding this comment

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

We can just use the Default impl here.

pub fn new() -> Self {
    Default::default()
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you, switched to the Default impl.

recursive: false,
mode: None,
};

builder
}

/// Indicates whether to create directories recursively (including all parent directories).
/// Parents that do not exist are created with the same security and permissions settings.
///
/// This option defaults to `false`.
///
/// This is an async version of [`std::fs::DirBuilder::recursive`][std]
///
/// [std]: std::fs::DirBuilder::recursive
///
/// # Examples
///
/// ```no_run
/// use tokio::fs::DirBuilder;
///
/// let mut builder = DirBuilder::new();
/// builder.recursive(true);
/// ```
pub fn recursive(&mut self, recursive: bool) -> &mut Self {
self.recursive = recursive;
self
}

/// Creates the specified directory with the configured options.
///
/// It is considered an error if the directory already exists unless
/// recursive mode is enabled.
///
/// This is an async version of [`std::fs::DirBuilder::create`][std]
///
/// [std]: std::fs::DirBuilder::create
///
/// # Errors
///
/// An error will be returned under the following circumstances:
///
/// * Path already points to an existing file.
/// * Path already points to an existing directory and the mode is
/// non-recursive.
/// * The calling process doesn't have permissions to create the directory
/// or its missing parents.
/// * Other I/O error occurred.
///
/// # Examples
///
/// ```no_run
/// use tokio::fs::DirBuilder;
/// use std::io;
///
/// #[tokio::main]
/// async fn main() -> io::Result<()> {
/// DirBuilder::new()
/// .recursive(true)
/// .create("/tmp/foo/bar/baz")
/// .await?;
///
/// Ok(())
/// }
/// ```
pub async fn create<P: AsRef<Path>>(&self, path: P) -> io::Result<()> {
let path = path.as_ref().to_owned();
let mut builder = std::fs::DirBuilder::new();
builder.recursive(self.recursive);

#[cfg(unix)]
{
if let Some(mode) = self.mode {
Darksonn marked this conversation as resolved.
Show resolved Hide resolved
std::os::unix::fs::DirBuilderExt::mode(&mut builder, mode);
}
}

asyncify(move || builder.create(path)).await
}
}

cfg_unix! {
use std::os::unix::fs::DirBuilderExt;

impl DirBuilderExt for DirBuilder {
fn mode(&mut self, mode: u32) -> &mut Self {
self.mode = Some(mode);
self
}
}
}
3 changes: 3 additions & 0 deletions tokio/src/fs/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ pub use self::create_dir::create_dir;
mod create_dir_all;
pub use self::create_dir_all::create_dir_all;

mod dir_builder;
pub use self::dir_builder::DirBuilder;

mod file;
pub use self::file::File;

Expand Down
9 changes: 9 additions & 0 deletions tokio/src/macros/cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,15 @@ macro_rules! cfg_uds {
}
}

macro_rules! cfg_unix {
($($item:item)*) => {
$(
#[cfg(unix)]
$item
)*
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

We use #[cfg(unix)] everywhere else in the codebase. Maybe adding this cfg_* makes sense, but I would rather want it in its own PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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


macro_rules! cfg_unstable {
($($item:item)*) => {
$(
Expand Down
19 changes: 18 additions & 1 deletion tokio/tests/fs_dir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
#![cfg(feature = "full")]

use tokio::fs;
use tokio_test::assert_ok;
use tokio_test::{assert_err, assert_ok};

use std::sync::{Arc, Mutex};
use tempfile::tempdir;
Expand All @@ -28,6 +28,23 @@ async fn create_all() {
assert!(new_dir_2.is_dir());
}

#[tokio::test]
async fn build_dir() {
let base_dir = tempdir().unwrap();
let new_dir = base_dir.path().join("foo").join("bar");
let new_dir_2 = new_dir.clone();

assert_ok!(fs::DirBuilder::new().recursive(true).create(new_dir).await);

assert!(new_dir_2.is_dir());
assert_err!(
fs::DirBuilder::new()
.recursive(false)
.create(new_dir_2)
.await
);
}

#[tokio::test]
async fn remove() {
let base_dir = tempdir().unwrap();
Expand Down