-
Notifications
You must be signed in to change notification settings - Fork 5
[io_file] Add the ability to get file metadata on Windows. #202
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
base: main
Are you sure you want to change the base?
Changes from 31 commits
5db08c8
a6f5c1c
f89d063
0f81c95
ed98ec4
2dfc83b
8bf09fb
1dcd074
2877b5e
7e4717d
278de74
69490d5
9704766
36647c2
a363b49
305655c
88dc69e
647a6a5
f7c6d76
dd67782
9185690
a687b84
e3fc41a
309c186
2c51b7d
fa3e0a2
b287897
b88bfca
d14f1a9
4d8ccfd
1aeb152
4fd4da1
b6ffeff
8c95248
18e6f8b
186c228
7d11d36
90d4070
c878448
0188da6
ea0946f
e28568d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,47 @@ | |
// for details. All rights reserved. Use of this source code is governed by a | ||
// BSD-style license that can be found in the LICENSE file. | ||
|
||
/// Information about a directory, link, etc. stored in the [FileSystem]. | ||
base class Metadata { | ||
// TODO(brianquinlan): Document all public fields. | ||
|
||
final bool isFile; | ||
final bool isDirectory; | ||
final bool isLink; | ||
final int size; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (Totally over-optimizing, ... but you can store three bools in one int and save two storage slots. I don't trust our compilers to do that for you.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll consider it ;-) |
||
|
||
Metadata({ | ||
this.isDirectory = false, | ||
this.isFile = false, | ||
this.isLink = false, | ||
this.size = 0, | ||
}) { | ||
final count = (isDirectory ? 1 : 0) + (isFile ? 1 : 0) + (isLink ? 1 : 0); | ||
if (count > 1) { | ||
// TODO(brianquinlan): Decide whether a path must be a a file, directory | ||
// or link and whether it can be more than one of these at once. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Windows links are either file links or directory links. It makes sense that they can represent the difference. Do you have to use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You don't have to use The constructor is public because I want other people to be able to implement their own file system that return Maybe I should make this an I'm not confident in my API design choices here :-( |
||
// Rust requires that at most one of these is true. Python has no such | ||
// restriction. | ||
|
||
// TODO(bquinlan): if we keep this logic, use `ArgumentError.value`. | ||
throw ArgumentError( | ||
brianquinlan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
'only one of isDirectory, isFile, or isLink must be true', | ||
); | ||
} | ||
} | ||
|
||
@override | ||
bool operator ==(Object other) => | ||
other is Metadata && | ||
isDirectory == other.isDirectory && | ||
isFile == other.isFile && | ||
isLink == other.isLink && | ||
size == other.size; | ||
|
||
@override | ||
int get hashCode => Object.hash(isDirectory, isFile, isLink, size).hashCode; | ||
} | ||
|
||
/// An abstract representation of a file system. | ||
base class FileSystem { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (Make this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
/// Renames, and possibly moves a file system object from one path to another. | ||
|
@@ -23,4 +64,12 @@ base class FileSystem { | |
void rename(String oldPath, String newPath) { | ||
throw UnsupportedError('rename'); | ||
} | ||
|
||
/// Metadata for the file system object at [path]. | ||
/// | ||
/// If `path` represents a symbolic link then metadata for the link is | ||
/// returned. | ||
Metadata metadata(String path) { | ||
throw UnsupportedError('metadata'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (If every subclass of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My idea is that I want to be able to add methods to the abstract Instead, I was coping to make Maybe that is a bad idea :-) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's a valid design for a growing API, but also a potentially troublesome one. It can become a problem if people can't trust that methods are implemented. If anyone ever feels they need to write Maybe some annotations can help ... And then one might as well just add MAYBE, just for complete pedantic adherence to semver:
Not sure the complexity is worth the result. It'll still end up with failed resolution. The only benefit is that an incremental addition to the API isn't breaking (and does not require a major version increment, which is hard to push through if multiple packages use the library) for code that only uses the default implementation. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't want people to check for |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,11 @@ import 'package:win32/win32.dart' as win32; | |
|
||
import 'file_system.dart'; | ||
|
||
DateTime _fileTimeToDateTime(int t) { | ||
final microseconds = t ~/ 10; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (Might want a constant There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
return DateTime.utc(1601, 1, 1).add(Duration(microseconds: microseconds)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can be just There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
} | ||
|
||
String _formatMessage(int errorCode) { | ||
final buffer = win32.wsalloc(1024); | ||
try { | ||
|
@@ -62,14 +67,93 @@ Exception _getError(int errorCode, String message, String path) { | |
} | ||
} | ||
|
||
/// File system entity data available on Windows. | ||
final class WindowsMetadata extends Metadata { | ||
// TODO(brianquinlan): Reoganize fields when the POSIX `metadata` is | ||
// available. | ||
// TODO(brianquinlan): Document the public fields. | ||
|
||
final bool isReadOnly; | ||
final bool isHidden; | ||
final bool isSystem; | ||
final bool isArchive; | ||
final bool isTemporary; | ||
final bool isOffline; | ||
final bool isContentNotIndexed; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (Ob-optimizing: Can definitely save space by storing these as a bit-vector.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
|
||
final int creationTime100Nanos; | ||
final int lastAccessTime100Nanos; | ||
final int lastWriteTime100Nanos; | ||
|
||
DateTime get creation => _fileTimeToDateTime(creationTime100Nanos); | ||
DateTime get access => _fileTimeToDateTime(lastAccessTime100Nanos); | ||
DateTime get modification => _fileTimeToDateTime(lastWriteTime100Nanos); | ||
|
||
/// TODO(bquinlan): Document this constructor. | ||
WindowsMetadata({ | ||
super.isDirectory = false, | ||
super.isFile = false, | ||
super.isLink = false, | ||
|
||
super.size = 0, | ||
|
||
this.isReadOnly = false, | ||
this.isHidden = false, | ||
this.isSystem = false, | ||
this.isArchive = false, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds like answering whether it's a zip file :) Maybe There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm just using the Microsoft names (with an I'm file with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Adding I can see why "Flag" isn't great in the name. The read only flag means that the file "is read-only". ✓ The "archive" bit is really a "dirty" bit, ... and if (!file.isArchived) copy(file, archivePathOf(file)); // ✓ |
||
this.isTemporary = false, | ||
this.isOffline = false, | ||
this.isContentNotIndexed = false, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider inverting flag, with a default of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was trying to use the same names as Microsoft (but with dartCasing). What do you think is more valuable:
I don't have strong feelings about this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd go with a good Dart API, with names that are relatable to the windows API, with documentation about the specific relation. If we were just doing a Windows API, it would be an integer and bit flags. So something like: /// Whether the file is considered archived, and not in need of being archived.
///
/// The Windows file system ARCHIVE flag (traditionally displayed as an "A" attribute)
/// is set when the file needs archiving.
/// This value expresses whether the ARCHIVE flag is unset.
abstract final bool isArchived; or /// Whether the file needs to be archived.
///
/// The Windows file system ARCHIVE flag (traditionally displayed as an "A" attribute)
/// is set when the file needs archiving.
/// This value expresses whether the ARCHIVE flag is set.
abstract final bool isNotArchived; (Heck, have both! It's not unheard of to have both |
||
|
||
this.creationTime100Nanos = 0, | ||
this.lastAccessTime100Nanos = 0, | ||
this.lastWriteTime100Nanos = 0, | ||
}); | ||
|
||
@override | ||
bool operator ==(Object other) => | ||
other is WindowsMetadata && | ||
super == other && | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This Just because of that, I'd make the superclass abstract, and not have There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice catch! I made |
||
isReadOnly == other.isReadOnly && | ||
isHidden == other.isHidden && | ||
isSystem == other.isSystem && | ||
isArchive == other.isArchive && | ||
isTemporary == other.isTemporary && | ||
isOffline == other.isOffline && | ||
isContentNotIndexed == other.isContentNotIndexed && | ||
creationTime100Nanos == other.creationTime100Nanos && | ||
lastAccessTime100Nanos == other.lastAccessTime100Nanos && | ||
lastWriteTime100Nanos == other.lastWriteTime100Nanos; | ||
|
||
@override | ||
int get hashCode => Object.hash( | ||
super.hashCode, | ||
isReadOnly, | ||
isHidden, | ||
isSystem, | ||
isArchive, | ||
isTemporary, | ||
isOffline, | ||
isContentNotIndexed, | ||
creationTime100Nanos, | ||
lastAccessTime100Nanos, | ||
lastWriteTime100Nanos, | ||
); | ||
} | ||
|
||
/// A [FileSystem] implementation for Windows systems. | ||
base class WindowsFileSystem extends FileSystem { | ||
brianquinlan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
@override | ||
void rename(String oldPath, String newPath) => using((arena) { | ||
WindowsFileSystem() { | ||
// Calling `GetLastError` for the first time causes the `GetLastError` | ||
// symbol to be loaded, which resets `GetLastError`. So make a harmless | ||
// call before the value is needed. | ||
// | ||
// TODO(brianquinlan): Remove this after it is fixed in the Dart SDK. | ||
win32.GetLastError(); | ||
} | ||
|
||
@override | ||
void rename(String oldPath, String newPath) => using((arena) { | ||
if (win32.MoveFileEx( | ||
oldPath.toNativeUtf16(allocator: arena), | ||
newPath.toNativeUtf16(allocator: arena), | ||
|
@@ -80,4 +164,127 @@ base class WindowsFileSystem extends FileSystem { | |
throw _getError(errorCode, 'rename failed', oldPath); | ||
} | ||
}); | ||
|
||
/// Sets metadata for the file system entity. | ||
/// | ||
/// TODO(brianquinlan): Document the arguments. | ||
void setMetadata( | ||
lrhn marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it make any sense to take a It's not an atomic operation to It also means that if I first call If I could pass the If the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
String path, { | ||
bool? isReadOnly, | ||
bool? isHidden, | ||
bool? isSystem, | ||
bool? isArchive, | ||
bool? isTemporary, | ||
bool? isContentNotIndexed, | ||
bool? isOffline, | ||
}) => using((arena) { | ||
if ((isReadOnly ?? | ||
isHidden ?? | ||
isSystem ?? | ||
isArchive ?? | ||
isTemporary ?? | ||
isContentNotIndexed ?? | ||
isOffline) == | ||
null) { | ||
return; | ||
} | ||
final fileInfo = arena<win32.WIN32_FILE_ATTRIBUTE_DATA>(); | ||
final nativePath = path.toNativeUtf16(allocator: arena); | ||
if (win32.GetFileAttributesEx( | ||
nativePath, | ||
win32.GetFileExInfoStandard, | ||
fileInfo, | ||
) == | ||
win32.FALSE) { | ||
final errorCode = win32.GetLastError(); | ||
throw _getError(errorCode, 'set metadata failed', path); | ||
} | ||
|
||
var attributes = fileInfo.ref.dwFileAttributes; | ||
if (attributes == win32.FILE_ATTRIBUTE_NORMAL) { | ||
// `FILE_ATTRIBUTE_NORMAL` indicates that no other attributes are set and | ||
// is valid only when used alone. | ||
attributes = 0; | ||
brianquinlan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
int updateBit(int base, int value, bool? bit) => switch (bit) { | ||
null => base, | ||
true => base | value, | ||
false => base & ~value, | ||
}; | ||
|
||
attributes = updateBit( | ||
attributes, | ||
win32.FILE_ATTRIBUTE_READONLY, | ||
isReadOnly, | ||
); | ||
attributes = updateBit(attributes, win32.FILE_ATTRIBUTE_HIDDEN, isHidden); | ||
attributes = updateBit(attributes, win32.FILE_ATTRIBUTE_SYSTEM, isSystem); | ||
attributes = updateBit(attributes, win32.FILE_ATTRIBUTE_ARCHIVE, isArchive); | ||
attributes = updateBit( | ||
attributes, | ||
win32.FILE_ATTRIBUTE_TEMPORARY, | ||
isTemporary, | ||
); | ||
attributes = updateBit( | ||
attributes, | ||
win32.FILE_ATTRIBUTE_NOT_CONTENT_INDEXED, | ||
isContentNotIndexed, | ||
); | ||
attributes = updateBit(attributes, win32.FILE_ATTRIBUTE_OFFLINE, isOffline); | ||
if (attributes == 0) { | ||
// `FILE_ATTRIBUTE_NORMAL` indicates that no other attributes are set and | ||
// is valid only when used alone. | ||
attributes = win32.FILE_ATTRIBUTE_NORMAL; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (Could we check here whether the value didn't change, and then not write anything? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm supporting |
||
if (win32.SetFileAttributes(nativePath, attributes) == win32.FALSE) { | ||
brianquinlan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
final errorCode = win32.GetLastError(); | ||
throw _getError(errorCode, 'set metadata failed', path); | ||
} | ||
}); | ||
|
||
@override | ||
WindowsMetadata metadata(String path) => using((arena) { | ||
final fileInfo = arena<win32.WIN32_FILE_ATTRIBUTE_DATA>(); | ||
if (win32.GetFileAttributesEx( | ||
path.toNativeUtf16(allocator: arena), | ||
win32.GetFileExInfoStandard, | ||
fileInfo, | ||
) == | ||
win32.FALSE) { | ||
final errorCode = win32.GetLastError(); | ||
throw _getError(errorCode, 'metadata failed', path); | ||
} | ||
final info = fileInfo.ref; | ||
final attributes = info.dwFileAttributes; | ||
|
||
final isDirectory = attributes & win32.FILE_ATTRIBUTE_DIRECTORY > 0; | ||
final isLink = attributes & win32.FILE_ATTRIBUTE_REPARSE_POINT > 0; | ||
final isFile = !(isDirectory || isLink); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If a link is a link to a directory (or to a link to a directory) I expect both of In that case Or you could make There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In Windows, the type of link is not indicated in And yeah, there is some tension here. Right now I was going to make one of the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I checked, and I think a reparse point having the DIRECTORY bit set controls whether it's a file or directory reparse point. Every entity in the Windows FS is either a file or a directory, including links, controlled by the DIRECTORY bit. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What makes sense from an API POV? I think that no more than one of these should be set. And maybe it makes sense for zero of them to be set e.g. for a unix domain socket, a named pipe, etc. |
||
|
||
return WindowsMetadata( | ||
isReadOnly: attributes & win32.FILE_ATTRIBUTE_READONLY > 0, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (I'd use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
isHidden: attributes & win32.FILE_ATTRIBUTE_HIDDEN > 0, | ||
isSystem: attributes & win32.FILE_ATTRIBUTE_SYSTEM > 0, | ||
isDirectory: isDirectory, | ||
isArchive: attributes & win32.FILE_ATTRIBUTE_ARCHIVE > 0, | ||
isTemporary: attributes & win32.FILE_ATTRIBUTE_TEMPORARY > 0, | ||
isLink: isLink, | ||
isFile: isFile, | ||
isOffline: attributes & win32.FILE_ATTRIBUTE_OFFLINE > 0, | ||
isContentNotIndexed: | ||
attributes & win32.FILE_ATTRIBUTE_NOT_CONTENT_INDEXED > 0, | ||
|
||
size: info.nFileSizeHigh << 32 | info.nFileSizeLow, | ||
creationTime100Nanos: | ||
info.ftCreationTime.dwHighDateTime << 32 | | ||
info.ftCreationTime.dwLowDateTime, | ||
lastAccessTime100Nanos: | ||
info.ftLastAccessTime.dwHighDateTime << 32 | | ||
info.ftLastAccessTime.dwLowDateTime, | ||
lastWriteTime100Nanos: | ||
info.ftLastWriteTime.dwHighDateTime << 32 | | ||
info.ftLastWriteTime.dwLowDateTime, | ||
); | ||
}); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(For future extension: The OSX file system has a hidden flag too, you can set it with
chflags hidden /path/to/folder/
. Not all tools recognize it - Finder does,ls
doesn't. It's from BSD, so other Unixes might have it too. We should probably support it if possible/reasonable.)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, linux has a hidden flag too - it's just not used in many file systems. I plan on moving some of these attributes around when I do the POSIX stat implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that the semantics probably are quite different it makes a lot of sense to not share the hidden flag between platforms though they are named the same...