Skip to content

More newrt file io work #9235

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

Closed
wants to merge 21 commits into from

Conversation

olsonjeffery
Copy link
Contributor

A quick rundown:

  • added file::{readdir, stat, mkdir, rmdir}
  • Added access-constrained versions of FileStream; FileReader and FileWriter respectively
  • big rework in uv::file .. most actions are by-val-self methods on FsRequest; FileDescriptor has gone the way of the dinosaurs
  • playing nice w/ homing IO (I just copied ecr's work, hehe), etc
  • added FileInfo trait, with an impl for Path
    • wrapper for file-specific actions, with the file path always implied by self's value
    • has the means to create FileReader & FileWriter (this isn't exposed in the top-level free function API)
    • has "safe" wrappers for stat() that won't throw in the event of non-existence/error (in this case, I mean is_file and exists)
    • actions should fail if done on non-regular-files, as appropriate
  • added DirectoryInfo trait, with an impl for Path
    • pretty much ditto above, but for directories
    • added readdir (!!) to iterate over entries in a dir as a ~[Path] (this was brutal to get working)

...and lots of other stuffnot really. Do your worst!

@steveklabnik steveklabnik mentioned this pull request Sep 16, 2013
}

/// Internal representation of a FileStream, used to consolidate functionality
/// exposed in the public API
pub struct FileStream {
Copy link
Member

Choose a reason for hiding this comment

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

If this is an internal representation, does this still need to be pub?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see this is returned in at least one place. nevermind.

std: remove unneeded field from RequestData struct

std: rt::uv::file - map us_fs_stat & start refactoring calls into FsRequest

std: stubbing out stat calls from the top-down into uvio

std: us_fs_* operations are now by-val self methods on FsRequest

std: post-rebase cleanup

std: add uv_fs_mkdir|rmdir + tests & minor test cleanup in rt::uv::file

WORKING: fleshing out FileStat and FileInfo + tests

std: reverting test files..

refactoring back and cleanup...
add ignores for win32 tests on previous file io stuff...
@olsonjeffery
Copy link
Contributor Author

heh, bummer. blew away the comments by rebasing. apologies.

anyways, pretty much anything that I didn't respond to inilne, above, I handled in the updated commits to this PR.

/// Will raise an `io_error` condition if the provided `path` doesn't exist,
/// the process lacks permissions to view the contents or if the `path` points
/// at a non-directory file
pub fn readdir<P: PathLike>(path: &P) -> Option<~[Path]> {
Copy link
Member

Choose a reason for hiding this comment

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

Are we "fixed" on following the C names? Would it be better to follow Rust convention and call this read_dir or even read_directory?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm hewing to the libuv conventions, rather than C (although I don't think that's horrible, either). It's familiar to some; not so to others. Yes, I know we want to expose other mappings and we don't want to couple, idealogically, to libuv too much. I just reached for what was convenient, and the naming is in keeping with existing, rust API that hews towards low-level API naming conventions and all-around brevity.

Can we punt this decision to a future PR? Thank you.

@olsonjeffery
Copy link
Contributor Author

@alexcrichton I only got this via email. re: the behavior of uv_fs_readdir():

Ben Noordhuis: It's option 1. Adding a streaming readdir() is a low-prio TODO. If you want, you can open an issue for it.

Let's not let this block the PR. I'm fine with looking at doing the work in libuv or asking someone else to do it. Can we get this landed?

EDIT: libuv issue: #931


/// Open a file for reading/writing, as indicated by `path`.
///
/// # Example
Copy link
Member

Choose a reason for hiding this comment

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

nice!

bors added a commit that referenced this pull request Sep 17, 2013
Closes #9045

Built on top of #9235, which isn't strictly needed for now, but I imagine I will use part of. Unsure.

I mostly wanted to start this off to get some feedback from @catamorphism and others. These are the directories that actually need made, but I was thinking about adding a few other things:

1. an `examples` directory, since it seems like that's a common pattern
2. a `.gitignore` file that ignores `build`. And anything else that makes sense
3. a sample module that'd actually compile


Feedback?
bors added a commit that referenced this pull request Sep 17, 2013
A quick rundown:

- added `file::{readdir, stat, mkdir, rmdir}`
- Added access-constrained versions of `FileStream`; `FileReader` and `FileWriter` respectively
- big rework in `uv::file` .. most actions are by-val-self methods on `FsRequest`; `FileDescriptor` has gone the way of the dinosaurs
- playing nice w/ homing IO (I just copied ecr's work, hehe), etc
- added `FileInfo` trait, with an impl for `Path`
  - wrapper for file-specific actions, with the file path always implied by self's value
  - has the means to create `FileReader` & `FileWriter` (this isn't exposed in the top-level free function API)
  - has "safe" wrappers for `stat()` that won't throw in the event of non-existence/error (in this case, I mean `is_file` and `exists`)
  - actions should fail if done on non-regular-files, as appropriate
- added `DirectoryInfo` trait, with an impl for `Path`
  - pretty much ditto above, but for directories
  - added `readdir` (!!) to iterate over entries in a dir as a `~[Path]` (this was *brutal* to get working)

...<del>and lots of other stuff</del>not really. Do your worst!
@bors bors closed this Sep 17, 2013
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jul 28, 2022
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.

7 participants