std: Expand the area of std::fs #24711

alexcrichton · 2015-04-23T01:22:44Z

This commit is an implementation of RFC 1044 which adds additional
surface area to the std::fs module. All new APIs are #[unstable] behind
assorted feature names for each one.

The new APIs added are:

fs::canonicalize - bindings to realpath on unix and
GetFinalPathNameByHandle on windows.
fs::symlink_metadata - similar to lstat on unix
fs::FileType and accessor methods as is_{file,dir,symlink}
fs::Metadata::file_type - accessor for the raw file type
fs::DirEntry::metadata - acquisition of metadata which is free on Windows
but requires a syscall on unix.
fs::DirEntry::file_type - access the file type which may not require a
syscall on most platforms.
fs::DirEntry::file_name - access just the file name without leading
components.
fs::PathExt::symlink_metadata - convenience method for the top-level
function.
fs::PathExt::canonicalize - convenience method for the top-level
function.
fs::PathExt::read_link - convenience method for the top-level
function.
fs::PathExt::read_dir - convenience method for the top-level
function.
std::os::raw - type definitions for raw OS/C types available on all
platforms.
std::os::$platform - new modules have been added for all currently supported
platforms (e.g. those more specific than just unix).
std::os::$platform::raw - platform-specific type definitions. These modules
are populated with the bare essentials necessary for lowing I/O types into
their raw representations, and currently largely consist of the stat
definition for unix platforms.

This commit also deprecates Metadata::{modified, accessed} in favor of
inspecting the raw representations via the lowering methods of Metadata.

Closes #24796

rust-highfive · 2015-04-23T01:22:50Z

r? @pcwalton

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

alexcrichton · 2015-04-23T01:22:57Z

r? @aturon

Note that this should not be merged until the RFC is approved.

lambda · 2015-04-23T04:02:03Z

src/libstd/sys/windows/ext.rs

-                                                        -> io::Result<()>
-    {
-        sys::fs2::symlink_inner(src.as_ref(), dst.as_ref(), true)
-    }


Looks like you missed moving the new symlink APIs when breaking these modules out into separate files.

Oops! I had a feeling I shouldn't have ignored that merge conflict.

alexcrichton · 2015-04-23T16:49:50Z

@lambda

Thanks for the info! I'm moving discussion out here as your comment is now collapsed. This actually poses some difficulties and the current solution makes me feel uncomfortable. There are currently two methods of acquiring a FileType, one through DirEntry and the other through FileAttr (speaking in fs2 terms). The DirEntry one knows whether it's a symlink or not (by reading the dwReserved0 field you mentioned), but the FileAttr does not).

There are a few places which we can get a FileAttr:

DirEntry::metadata - this is fine as it already knows whether it's a symlink or not
File::metadata - this is problematic as to learn about whether the file is a symlink it requires a syscall
lstat - this is less problematic as we can switch to using FindFirstFile as you mentioned, but it is somewhat unnerving that accidental * or ? characters in the path could produce some odd results.
stat - this suffers the same problems as File::metadata and lstat

Overall this basically means that we need to have an extra syscall or some different semantics (which now makes me worry about read_dir...). This is an unfortunate deviation for what may not be that important all the time, so I suppose it suffices to say that I'm glad that these functions are #[unstable] for now!

Regardless, though, I think the current implementation is at least "more correct", although I'm not convinced that we actually want it!

lambda · 2015-04-24T23:58:57Z

I think that the latest approach that you've added looks fine, and I think that the abstraction is reasonable; having to spend an extra syscall to find out if a file is a symlink or not doesn't seem unreasonable, especially since you only pay that extra cost for reparse points, which should be fairly rare. And I think your current approach is fine, you're right, there are some concerns using FindFirstFile so this way of doing it should be fine.

On the other hand, I'm not a Windows expert; I just noticed the problem since I'd been reading the documentation closely while reviewing rust-lang/rfcs#1044 and writing rust-lang/rfcs#1048. @retep998 do you have any opinion on this, you seem to have the most Windows API experience?

retep998 · 2015-04-25T02:06:27Z

As long as we're only doing that extra syscall for reparse points, the cost should be negligible since most files are not reparse points. That said, the implementation of invoking readlink just to find out whether a reparse point is a symlink seems a bit too heavy. It would be nice to eventually expose all the possible reparse tag variants, not just symbolic links.

bors · 2015-04-25T07:45:58Z

☔ The latest upstream changes (presumably #24724) made this pull request unmergeable. Please resolve the merge conflicts.

GuillaumeGomez · 2015-04-27T14:17:13Z

src/libstd/sys/windows/fs2.rs

+        self.root.join(&self.file_name())
+    }
+
+    pub fn file_name(&self) -> OsString {


Like I pointed out in the RFC (a little lately, sorry for that). If the entry is a directory or something else, is file_name is still a good method name ?

This matches the file_name methods of Path, which was the source for this name. The directory/file distinction here is known to not apply much.

I'm not really fine with that. I find it not clear. It's really a shame that I was aware of the RFC so lately...

aturon · 2015-04-27T21:10:32Z

src/libstd/fs.rs

+
+    /// Return the metadata for the file that this entry points at.
+    ///
+    /// This function may not require an extra system call on some platforms


How about: "This function may require an extra system call on some platforms" (or, better yet: we could go ahead and add the # Platform behavior section and give a fuller breakdown).

Good idea! I've started out with the platform behavior sections to see where it takes us.

This commit is an implementation of [RFC 1044][rfc] which adds additional surface area to the `std::fs` module. All new APIs are `#[unstable]` behind assorted feature names for each one. [rfc]: rust-lang/rfcs#1044 The new APIs added are: * `fs::canonicalize` - bindings to `realpath` on unix and `GetFinalPathNameByHandle` on windows. * `fs::symlink_metadata` - similar to `lstat` on unix * `fs::FileType` and accessor methods as `is_{file,dir,symlink}` * `fs::Metadata::file_type` - accessor for the raw file type * `fs::DirEntry::metadata` - acquisition of metadata which is free on Windows but requires a syscall on unix. * `fs::DirEntry::file_type` - access the file type which may not require a syscall on most platforms. * `fs::DirEntry::file_name` - access just the file name without leading components. * `fs::PathExt::symlink_metadata` - convenience method for the top-level function. * `fs::PathExt::canonicalize` - convenience method for the top-level function. * `fs::PathExt::read_link` - convenience method for the top-level function. * `fs::PathExt::read_dir` - convenience method for the top-level function. * `std::os::raw` - type definitions for raw OS/C types available on all platforms. * `std::os::$platform` - new modules have been added for all currently supported platforms (e.g. those more specific than just `unix`). * `std::os::$platform::raw` - platform-specific type definitions. These modules are populated with the bare essentials necessary for lowing I/O types into their raw representations, and currently largely consist of the `stat` definition for unix platforms. This commit also deprecates `Metadata::{modified, accessed}` in favor of inspecting the raw representations via the lowering methods of `Metadata`.

alexcrichton · 2015-04-28T00:18:01Z

@bors: r=aturon 9348700

alexcrichton · 2015-04-28T00:31:54Z

@aturon I actually forgot that I didn't implement DirBuilder. I've just added a small commit which does so, re-r?

aturon · 2015-04-28T17:11:02Z

@alexcrichton Lovely!

@bors: r+

bors · 2015-04-28T17:11:03Z

📌 Commit ebc2440 has been approved by aturon

bors · 2015-04-28T19:13:12Z

⌛ Testing commit ebc2440 with merge a23c862...

bors · 2015-04-28T20:12:58Z

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

This is the last remaining portion of rust-lang#24796

alexcrichton · 2015-04-29T00:39:49Z

@bors: r+ 0368abb

bors · 2015-04-29T06:03:44Z

⌛ Testing commit 0368abb with merge e3c5027...

bors · 2015-04-29T07:22:02Z

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

alexcrichton · 2015-04-29T15:03:10Z

@bors: retry

On Wed, Apr 29, 2015 at 12:22 AM, bors notifications@github.com wrote:

[image: 💔] Test failed - auto-linux-32-nopt-t
http://buildbot.rust-lang.org/builders/auto-linux-32-nopt-t/builds/4726

—
Reply to this email directly or view it on GitHub
#24711 (comment).

This commit is an implementation of [RFC 1044][rfc] which adds additional surface area to the `std::fs` module. All new APIs are `#[unstable]` behind assorted feature names for each one. [rfc]: rust-lang/rfcs#1044 The new APIs added are: * `fs::canonicalize` - bindings to `realpath` on unix and `GetFinalPathNameByHandle` on windows. * `fs::symlink_metadata` - similar to `lstat` on unix * `fs::FileType` and accessor methods as `is_{file,dir,symlink}` * `fs::Metadata::file_type` - accessor for the raw file type * `fs::DirEntry::metadata` - acquisition of metadata which is free on Windows but requires a syscall on unix. * `fs::DirEntry::file_type` - access the file type which may not require a syscall on most platforms. * `fs::DirEntry::file_name` - access just the file name without leading components. * `fs::PathExt::symlink_metadata` - convenience method for the top-level function. * `fs::PathExt::canonicalize` - convenience method for the top-level function. * `fs::PathExt::read_link` - convenience method for the top-level function. * `fs::PathExt::read_dir` - convenience method for the top-level function. * `std::os::raw` - type definitions for raw OS/C types available on all platforms. * `std::os::$platform` - new modules have been added for all currently supported platforms (e.g. those more specific than just `unix`). * `std::os::$platform::raw` - platform-specific type definitions. These modules are populated with the bare essentials necessary for lowing I/O types into their raw representations, and currently largely consist of the `stat` definition for unix platforms. This commit also deprecates `Metadata::{modified, accessed}` in favor of inspecting the raw representations via the lowering methods of `Metadata`. Closes #24796

bors · 2015-04-29T16:31:28Z

⌛ Testing commit 0368abb with merge 1696dcf...

bors · 2015-04-29T17:48:51Z

💔 Test failed - auto-linux-32-opt

alexcrichton · 2015-04-29T17:57:55Z

@bors: retry

On Wed, Apr 29, 2015 at 10:48 AM, bors notifications@github.com wrote:

[image: 💔] Test failed - auto-linux-32-opt
http://buildbot.rust-lang.org/builders/auto-linux-32-opt/builds/4742

—
Reply to this email directly or view it on GitHub
#24711 (comment).

bors · 2015-04-29T19:03:39Z

⚡ Previous build results for auto-linux-64-nopt-t, auto-mac-64-opt, auto-win-64-nopt-t are reusable. Rebuilding only auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-win-32-nopt-t, auto-win-32-opt, auto-win-64-opt...

bors · 2015-04-29T20:16:55Z

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

alexcrichton · 2015-04-29T22:43:52Z

@bors: retry

On Wed, Apr 29, 2015 at 1:16 PM, bors notifications@github.com wrote:

[image: 💔] Test failed - auto-linux-32-nopt-t
http://buildbot.rust-lang.org/builders/auto-linux-32-nopt-t/builds/4737

—
Reply to this email directly or view it on GitHub
#24711 (comment).

bors · 2015-04-29T23:30:52Z

⚡ Previous build results for auto-linux-64-nopt-t, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-64-nopt-t are reusable. Rebuilding only auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-win-32-nopt-t, auto-win-32-opt, auto-win-64-opt...

This commit is an implementation of [RFC 1044][rfc] which adds additional surface area to the `std::fs` module. All new APIs are `#[unstable]` behind assorted feature names for each one. [rfc]: rust-lang/rfcs#1044 The new APIs added are: * `fs::canonicalize` - bindings to `realpath` on unix and `GetFinalPathNameByHandle` on windows. * `fs::symlink_metadata` - similar to `lstat` on unix * `fs::FileType` and accessor methods as `is_{file,dir,symlink}` * `fs::Metadata::file_type` - accessor for the raw file type * `fs::DirEntry::metadata` - acquisition of metadata which is free on Windows but requires a syscall on unix. * `fs::DirEntry::file_type` - access the file type which may not require a syscall on most platforms. * `fs::DirEntry::file_name` - access just the file name without leading components. * `fs::PathExt::symlink_metadata` - convenience method for the top-level function. * `fs::PathExt::canonicalize` - convenience method for the top-level function. * `fs::PathExt::read_link` - convenience method for the top-level function. * `fs::PathExt::read_dir` - convenience method for the top-level function. * `std::os::raw` - type definitions for raw OS/C types available on all platforms. * `std::os::$platform` - new modules have been added for all currently supported platforms (e.g. those more specific than just `unix`). * `std::os::$platform::raw` - platform-specific type definitions. These modules are populated with the bare essentials necessary for lowing I/O types into their raw representations, and currently largely consist of the `stat` definition for unix platforms. This commit also deprecates `Metadata::{modified, accessed}` in favor of inspecting the raw representations via the lowering methods of `Metadata`. Closes rust-lang#24796

bors · 2015-04-29T23:33:33Z

⛄ The build was interrupted to prioritize another pull request.

bors · 2015-04-29T23:59:37Z

⚡ Previous build results for auto-linux-64-nopt-t, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-64-nopt-t are reusable. Rebuilding only auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-win-32-nopt-t, auto-win-32-opt, auto-win-64-opt...

bors · 2015-04-30T00:07:09Z

⛄ The build was interrupted to prioritize another pull request.

rust-highfive assigned pcwalton Apr 23, 2015

rust-highfive assigned aturon and unassigned pcwalton Apr 23, 2015

alexcrichton mentioned this pull request Apr 23, 2015

RFC: Expand the scope of std::fs rust-lang/rfcs#1044

Merged

lambda reviewed Apr 23, 2015
View reviewed changes

alexcrichton force-pushed the fs2.1 branch 5 times, most recently from 4327621 to 791156a Compare April 23, 2015 16:38

ogham mentioned this pull request Apr 27, 2015

Compiling on OS X Yosemite ogham/exa#60

Closed

GuillaumeGomez reviewed Apr 27, 2015
View reviewed changes

alexcrichton force-pushed the fs2.1 branch from 791156a to 00ef896 Compare April 27, 2015 16:24

aturon reviewed Apr 27, 2015
View reviewed changes

alexcrichton force-pushed the fs2.1 branch 2 times, most recently from c657c08 to 92eae9b Compare April 27, 2015 22:39

alexcrichton force-pushed the fs2.1 branch from 92eae9b to 9348700 Compare April 28, 2015 00:17

alexcrichton force-pushed the fs2.1 branch from ba4e4fc to ebc2440 Compare April 28, 2015 00:36

std: Implement fs::DirBuilder

0368abb

This is the last remaining portion of rust-lang#24796

alexcrichton force-pushed the fs2.1 branch from ebc2440 to 0368abb Compare April 29, 2015 00:39

bors merged commit 0368abb into rust-lang:master Apr 30, 2015

alexcrichton deleted the fs2.1 branch April 30, 2015 02:04

omasanori mentioned this pull request Jun 24, 2015

RELEASES.md lacks [fs-expand] #26537

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

std: Expand the area of std::fs #24711

std: Expand the area of std::fs #24711

alexcrichton commented Apr 23, 2015

rust-highfive commented Apr 23, 2015

alexcrichton commented Apr 23, 2015

lambda Apr 23, 2015

alexcrichton Apr 23, 2015

alexcrichton commented Apr 23, 2015

lambda commented Apr 24, 2015

retep998 commented Apr 25, 2015

bors commented Apr 25, 2015

GuillaumeGomez Apr 27, 2015

alexcrichton Apr 27, 2015

GuillaumeGomez Apr 27, 2015

aturon Apr 27, 2015

alexcrichton Apr 27, 2015

alexcrichton commented Apr 28, 2015

alexcrichton commented Apr 28, 2015

aturon commented Apr 28, 2015

bors commented Apr 28, 2015

bors commented Apr 28, 2015

bors commented Apr 28, 2015

alexcrichton commented Apr 29, 2015

bors commented Apr 29, 2015

bors commented Apr 29, 2015

alexcrichton commented Apr 29, 2015

bors commented Apr 29, 2015

bors commented Apr 29, 2015

alexcrichton commented Apr 29, 2015

bors commented Apr 29, 2015

bors commented Apr 29, 2015

alexcrichton commented Apr 29, 2015

bors commented Apr 29, 2015

bors commented Apr 29, 2015

bors commented Apr 29, 2015

bors commented Apr 30, 2015

std: Expand the area of std::fs #24711

std: Expand the area of std::fs #24711

Conversation

alexcrichton commented Apr 23, 2015

rust-highfive commented Apr 23, 2015

alexcrichton commented Apr 23, 2015

lambda Apr 23, 2015

Choose a reason for hiding this comment

alexcrichton Apr 23, 2015

Choose a reason for hiding this comment

alexcrichton commented Apr 23, 2015

lambda commented Apr 24, 2015

retep998 commented Apr 25, 2015

bors commented Apr 25, 2015

GuillaumeGomez Apr 27, 2015

Choose a reason for hiding this comment

alexcrichton Apr 27, 2015

Choose a reason for hiding this comment

GuillaumeGomez Apr 27, 2015

Choose a reason for hiding this comment

aturon Apr 27, 2015

Choose a reason for hiding this comment

alexcrichton Apr 27, 2015

Choose a reason for hiding this comment

alexcrichton commented Apr 28, 2015

alexcrichton commented Apr 28, 2015

aturon commented Apr 28, 2015

bors commented Apr 28, 2015

bors commented Apr 28, 2015

bors commented Apr 28, 2015

alexcrichton commented Apr 29, 2015

bors commented Apr 29, 2015

bors commented Apr 29, 2015

alexcrichton commented Apr 29, 2015

bors commented Apr 29, 2015

bors commented Apr 29, 2015

alexcrichton commented Apr 29, 2015

bors commented Apr 29, 2015

bors commented Apr 29, 2015

alexcrichton commented Apr 29, 2015

bors commented Apr 29, 2015

bors commented Apr 29, 2015

bors commented Apr 29, 2015

bors commented Apr 30, 2015