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

Begin finalizing the io::file module #10179

Merged
merged 5 commits into from
Nov 4, 2013

Conversation

alexcrichton
Copy link
Member

This fleshes out the io::file module a fair bit more, adding all of the functionality that I can think of that we would want. Some questions about the representation which I'm curious about:

  • I modified FileStat to be a little less platform-agnostic, but it's still fairly platform-specific. I don't want to hide information that we have, but I don't want to depend on this information being available. One possible route is to have an extra field which has all this os-dependent stuff which is clearly documented as it should be avoided.
  • Does it make sense for directory functions to be top-level functions instead of static methods? It seems silly to import std::rt::io::file and std::rt::io::File at the top of files that need to deal with directories and files.

@alexcrichton
Copy link
Member Author

Description updated.

use std::path::Path;
use std::rt::io;
use std::rt::io::file;
use std::rt::io::File;
Copy link
Member

Choose a reason for hiding this comment

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

yuck, capitalisation is the only distinction?

Copy link
Member Author

Choose a reason for hiding this comment

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

The underlying distinction is that io::file is a module and io::File is a re-export of io::file::File for more ergonomic access.

I'm a little in favor of moving all of these functions to File instead of having some as top-level functions of io::file, but I was hesitant because they don't really have anything to do with files...

@huonw
Copy link
Member

huonw commented Oct 30, 2013

I think it's weird that the only reason one would import std::rt::io::file is to handle directories... maybe we could have struct Dir { ... } and so it becomes Dir::read, Dir::mk, Dir::walk, (or even as non-static methods), similar to the File version.

@alexcrichton
Copy link
Member Author

I'd be a little hesitant to crate a magical struct Dir; which doesn't actually have anything inside of it just to namespace some methods. This probably just means that I need to put the inside of File.

@emberian
Copy link
Member

std::rt::io::fs?

@alexcrichton
Copy link
Member Author

Hm, I actually quite like the name fs instead of file for this module. It's short, concise, and allows us to easily move file/directory operations under it.

Additionally, I'm not sure that we need to keep the static methods any more. If we called the module fs, I would rather have things like fs::copy, File::open, fs::rmdir_recursive, etc. I think that the constructors of a File should be static methods, but nothing else really needs to be.

@alexcrichton
Copy link
Member Author

New fs module introduced, I very much like how it turned out.

///
/// use std::rt::io::File;
///
/// let contents = File::open("foo.txt").read_to_end();
Copy link
Member

Choose a reason for hiding this comment

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

This should be using a Path, right?

These traits belong here, and were simply waiting for the std::io traits to get
removed. It's time they take their rightful positions!
The invocation for making a directory should be able to specify a mode to make
the directory with (instead of defaulting to one particular mode). Additionally,
libuv and various OSes implement efficient versions of renaming files, so this
operation is exposed as an IoFactory call.
This commit moves all thread-blocking I/O functions from the std::os module.
Their replacements can be found in either std::rt::io::file or in a hidden
"old_os" module inside of native::file. I didn't want to outright delete these
functions because they have a lot of special casing learned over time for each
OS/platform, and I imagine that these will someday get integrated into a
blocking implementation of IoFactory. For now, they're moved to a private module
to prevent bitrot and still have tests to ensure that they work.

I've also expanded the extensions to a few more methods defined on Path, most of
which were previously defined in std::os but now have non-thread-blocking
implementations as part of using the current IoFactory.

The api of io::file is in flux, but I plan on changing it in the next commit as
well.

Closes rust-lang#10057
This adds bindings to the remaining functions provided by libuv, all of which
are useful operations on files which need to get exposed somehow.

Some highlights:

* Dropped `FileReader` and `FileWriter` and `FileStream` for one `File` type
* Moved all file-related methods to be static methods under `File`
* All directory related methods are still top-level functions
* Created `io::FilePermission` types (backed by u32) that are what you'd expect
* Created `io::FileType` and refactored `FileStat` to use FileType and
  FilePermission
* Removed the expanding matrix of `FileMode` operations. The mode of reading a
  file will not have the O_CREAT flag, but a write mode will always have the
  O_CREAT flag.

Closes rust-lang#10130
Closes rust-lang#10131
Closes rust-lang#10121
This renames the `file` module to `fs` because that more accurately describes
its current purpose (manipulating the filesystem, not just files).

Additionally, this adds an UnstableFileStat structure as a nested structure of
FileStat to signify that the fields should not be depended on. The structure is
currently flagged with #[unstable], but it's unlikely that it has much meaning.

Closes rust-lang#10241
bors added a commit that referenced this pull request Nov 4, 2013
This fleshes out the io::file module a fair bit more, adding all of the functionality that I can think of that we would want. Some questions about the representation which I'm curious about:

* I modified `FileStat` to be a little less platform-agnostic, but it's still fairly platform-specific. I don't want to hide information that we have, but I don't want to depend on this information being available. One possible route is to have an `extra` field which has all this os-dependent stuff which is clearly documented as it should be avoided.

* Does it make sense for directory functions to be top-level functions instead of static methods? It seems silly to import `std::rt::io::file` and `std::rt::io::File` at the top of files that need to deal with directories and files.
@bors bors closed this Nov 4, 2013
@bors bors merged commit 3c3ed14 into rust-lang:master Nov 4, 2013
@alexcrichton alexcrichton deleted the rt-improvements branch November 4, 2013 22:05
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jan 12, 2023
…, r=xFrednet

trim paths in `suspicious_to_owned`

This continues my path trimming spree. I'm not going to add yet another changelog entry, we should have one "trim paths in some applicable lints" entry instead.

---

changelog: none
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.

4 participants