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

std: Add a new fs module #21936

Merged
merged 1 commit into from
Feb 10, 2015
Merged

std: Add a new fs module #21936

merged 1 commit into from
Feb 10, 2015

Conversation

alexcrichton
Copy link
Member

This commit is an implementation of RFC 739 which adds a new std::fs
module to the standard library. This module provides much of the same
functionality as std::old_io::fs but it has many tweaked APIs as well as uses
the new std::path module.

@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

@alexcrichton
Copy link
Member Author

Note that this should not be accepted before rust-lang/rfcs#739 is accepted as well.

r? @aturon

cc @retep998

let path = path.as_path();
let inner = try!(fs_imp::File::open(path, &opts.0));

// On *BSD systems, we can open a directory as a file and read from
Copy link
Member

Choose a reason for hiding this comment

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

You should mention that directories can be opened as files on Windows as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

We actually explicitly enable that today but I made sure to not carry it over to the current implementation

@aturon
Copy link
Member

aturon commented Feb 4, 2015

Ok, I've given this an initial pass and it looks pretty good to me! I left several suggestions, some of which are already comments on the RFC thread.

I didn't worry too much about the os::platform APIs since those are not being stabilized for right now; I suspect we'll need to revisit the overall design principles for these submodules (e.g., should we work with raw int representations or newtypes?) but we can do that later.

/// learned.
pub struct ReadDir(fs_imp::ReadDir);

/// Entries returned by the `ReadDir` entry.
Copy link
Member

Choose a reason for hiding this comment

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

/// Entries returned by the ReadDir entry.

s/entry/iterator/

@l0kod
Copy link
Contributor

l0kod commented Feb 7, 2015

Here are some thoughts about the path/descriptor dilemma…

Rename File::open(&Path) to File::new(&Path), add a root: Option<AsIo> field to OpenOptions and implement common Path and File functions:

impl AsIo for Path {
    fn read_dir(&self)fn walk_dir(&self)fn metadata(&self)fn exists(&self)fn is_file(&self)fn is_dir(&self) …
    …
    fn copy(&self, &AsIo)fn open(&self, &Path)// openat(2)}
impl AsIo for File {}

// open("/foo/bar")
let f1 = File::new(Path::new("/foo/bar"));
// openat(foo, "bar")
let f2 = File::new(Path::new("/foo")).open(Path::new("bar"));

// stat("/foo")
let s1 = Path::new("/foo").metadata();
// fstat(foo)
let s2 = File::new(Path::new("/foo")).metadata());

@aturon
Copy link
Member

aturon commented Feb 9, 2015

@l0kod

Thanks for contiuing to push on the descriptor-based IO API!

Maybe a new trait (e.g. AsIo) implemented for Path and File could be used instead of Path arguments? This could solve the path-based or descriptor-based dilemma.

I think this may be a good avenue to explore in the long run -- certainly, it's a backwards compatible way to introduce descriptor-based programming as you're suggesting.

However, as @alexcrichton and I have mentioned a few times, for this stage of IO reform we plan to expose only limited hooks for extracting descriptors (like the ones that are present today). This does not represent bias against descriptors per se, just that the current reform is focused on stabilizing existing high-level APIs. Lower-level, descriptor-based APIs are also important, but we'd like to have more time to experiment and work through separate RFCs on the topic. There are a lot of details that need to be sorted out.

For now, we need to land these API revisions as per the RFC, so that we can offer some basic IO features, to be supplemented later on.

@aturon
Copy link
Member

aturon commented Feb 9, 2015

@bors r+ c481e9b

@bors
Copy link
Contributor

bors commented Feb 9, 2015

⌛ Testing commit c481e9b with merge 8264fff...

@bors
Copy link
Contributor

bors commented Feb 9, 2015

💔 Test failed - auto-win-64-nopt-t

@alexcrichton
Copy link
Member Author

@bors: r=aturon 9e42db55

@bors
Copy link
Contributor

bors commented Feb 9, 2015

🙀 You have the wrong number! Please try again with 942db55.

@alexcrichton
Copy link
Member Author

@bors: r=aturon 942db55

@bors
Copy link
Contributor

bors commented Feb 10, 2015

⌛ Testing commit 942db55 with merge e2b62b0...

@bors
Copy link
Contributor

bors commented Feb 10, 2015

💔 Test failed - auto-win-32-nopt-t

@bors
Copy link
Contributor

bors commented Feb 10, 2015

💔 Test failed - auto-mac-64-opt

@alexcrichton
Copy link
Member Author

@bors: r=aturon 6bfbad9

This commit is an implementation of [RFC 739][rfc] which adds a new `std::fs`
module to the standard library. This module provides much of the same
functionality as `std::old_io::fs` but it has many tweaked APIs as well as uses
the new `std::path` module.

[rfc]: rust-lang/rfcs#739
@bors
Copy link
Contributor

bors commented Feb 10, 2015

⌛ Testing commit 6bfbad9 with merge 0bfe358...

bors added a commit that referenced this pull request Feb 10, 2015
This commit is an implementation of [RFC 739][rfc] which adds a new `std::fs`
module to the standard library. This module provides much of the same
functionality as `std::old_io::fs` but it has many tweaked APIs as well as uses
the new `std::path` module.

[rfc]: rust-lang/rfcs#739
@bors bors merged commit 6bfbad9 into rust-lang:master Feb 10, 2015
@alexcrichton alexcrichton deleted the fsv2 branch February 10, 2015 06:43
@alexcrichton alexcrichton mentioned this pull request Feb 10, 2015
@aturon aturon mentioned this pull request Feb 18, 2015
91 tasks
@l0kod
Copy link
Contributor

l0kod commented Feb 24, 2015

FYI, I'm especially thinking about Capsicum (file descriptor-based sandboxing) who is integrated in FreeBSD, has work in progress for DragonFly BSD and should hopefully soon land for Linux.

To be able to create "capsicumable" programs, Rust must be able to only rely on file descriptor-based syscalls (e.g. openat).

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

Successfully merging this pull request may close these issues.

8 participants