Skip to content

[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

Open
wants to merge 42 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
5db08c8
Add `rename` support for Windows
brianquinlan Mar 13, 2025
a6f5c1c
Update rename_test.dart
brianquinlan Mar 13, 2025
f89d063
Some fixes
brianquinlan Mar 14, 2025
0f81c95
Update vm_windows_file_system.dart
brianquinlan Mar 14, 2025
ed98ec4
Update vm_windows_file_system.dart
brianquinlan Mar 14, 2025
2dfc83b
Test utils
brianquinlan Mar 14, 2025
8bf09fb
Metadata
brianquinlan Mar 16, 2025
1dcd074
Set metadata
brianquinlan Mar 17, 2025
2877b5e
Update vm_windows_file_system.dart
brianquinlan Mar 17, 2025
7e4717d
Tests
brianquinlan Mar 17, 2025
278de74
Merge branch 'main' into metadata
brianquinlan Mar 17, 2025
69490d5
Fixes
brianquinlan Mar 17, 2025
9704766
Windows metadata tests
brianquinlan Mar 17, 2025
36647c2
More windows properties
brianquinlan Mar 17, 2025
a363b49
Update vm_windows_file_system.dart
brianquinlan Mar 17, 2025
305655c
Test fixes
brianquinlan Mar 17, 2025
88dc69e
Update metadata_windows_test.dart
brianquinlan Mar 17, 2025
647a6a5
size
brianquinlan Mar 17, 2025
f7c6d76
creation time
brianquinlan Mar 17, 2025
dd67782
not such path
brianquinlan Mar 17, 2025
9185690
GetLastError
brianquinlan Mar 17, 2025
a687b84
Update metadata_test.dart
brianquinlan Mar 17, 2025
e3fc41a
Times
brianquinlan Mar 17, 2025
309c186
More fixes
brianquinlan Mar 17, 2025
2c51b7d
More tests
brianquinlan Mar 17, 2025
fa3e0a2
Update metadata_windows_test.dart
brianquinlan Mar 17, 2025
b287897
Docs
brianquinlan Mar 17, 2025
b88bfca
Update vm_windows_file_system.dart
brianquinlan Mar 18, 2025
d14f1a9
Update vm_windows_file_system.dart
brianquinlan Mar 18, 2025
4d8ccfd
Code review updates
brianquinlan Mar 18, 2025
1aeb152
Review feedback.
brianquinlan Mar 19, 2025
4fd4da1
Merge branch 'main' into metadata
brianquinlan Mar 27, 2025
b6ffeff
Add library
brianquinlan Mar 27, 2025
8c95248
Update file_system.dart
brianquinlan Mar 27, 2025
18e6f8b
Review comments
brianquinlan Mar 27, 2025
186c228
Update file_system.dart
brianquinlan Mar 27, 2025
7d11d36
Update file_system_test.dart
brianquinlan Mar 27, 2025
90d4070
Update vm_windows_file_system.dart
brianquinlan Mar 27, 2025
c878448
Merge remote-tracking branch 'upstream/main' into pr/brianquinlan/202
brianquinlan Apr 3, 2025
0188da6
Allow if original metadata
brianquinlan Apr 4, 2025
ea0946f
Update vm_windows_file_system.dart
brianquinlan Apr 16, 2025
e28568d
Fixes
brianquinlan Apr 17, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 19 additions & 3 deletions pkgs/io_file/example/io_file_example.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,25 @@
// 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.

import 'package:io_file/posix_file_system.dart';
import 'package:io_file/io_file.dart';
import 'package:io_file/windows_file_system.dart';

import 'package:path/path.dart' as p;

bool isHidden(String path) {
if (fileSystem case final WindowsFileSystem fs) {
Copy link
Member

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.)

Copy link
Contributor Author

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.

Copy link
Contributor

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...

return fs.metadata(path).isHidden;
} else {
// On POSIX, convention is that files and directories starting with a period
// are hidden (except for the special files representing the current working
// directory and parent directory).
//
// In addition, macOS has the UF_HIDDEN flag.
final name = p.basename(path);
return name.startsWith('.') && name != '.' && name != '..';
}
}

void main() {
// TODO(brianquinlan): Create a better example.
PosixFileSystem().rename('foo.txt', 'bar.txt');
isHidden('somefile');
}
20 changes: 19 additions & 1 deletion pkgs/io_file/lib/src/file_system.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,16 @@

import 'dart:typed_data';

/// Information about a directory, link, etc. stored in the [FileSystem].
abstract interface class Metadata {
// TODO(brianquinlan): Document all public fields.

bool get isFile;
bool get isDirectory;
bool get isLink;
int get size;
}

/// The modes in which a File can be written.
class WriteMode {
/// Open the file for writing such that data can only be appended to the end
Expand All @@ -29,7 +39,7 @@ class WriteMode {
}

/// An abstract representation of a file system.
base class FileSystem {
abstract base class FileSystem {
/// Renames, and possibly moves a file system object from one path to another.
///
/// If `newPath` is a relative path, it is resolved against the current
Expand All @@ -50,6 +60,14 @@ base class FileSystem {
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');
Copy link
Member

Choose a reason for hiding this comment

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

(If every subclass of FileSystem is intended to implement the method, then it can be abstract, or it can throw UnimplementedError, which signals that someone forgot to implement it, rather than that they chose not to.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 FileSystem without breaking people. For example, if I added createLink and that were abstract then every implementation of FileSystem not in the package would break.

Instead, I was coping to make FileSystem base and only have concrete methods so that wouldn't happen.

Maybe that is a bad idea :-)

Copy link
Member

Choose a reason for hiding this comment

The 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 try { fs.something(); } on UnsupportedError { ... }, I'd say the design has failed.

Maybe some annotations can help ...
Maybe start with @subclassMustOverride ... something() => throw UnsupportedError("something not implemented"); and then, in a later version, change it to ... something();. But that's still a breaking change, so it'd deserve a major version increment.

And then one might as well just add something(); directly, in a new major version. If most users will just use the file systems supprted by default, and only a few rarely-used packages will try to implement FileSystem, then .... well, versioning is still hard.

MAYBE, just for complete pedantic adherence to semver:

  • Make FileSystem a final class.
  • Export FileSystemBase which implements FileSystem and is base from a different package intended for file system implementors (with a strict dependence on the FileSystem package it comes from).
  • Then FileSystem can add a new member in a minor version increment, because to users it's a minor version increment, but the FileSystemBase class package gets a major version increment.
  • So FS 3.5.0 adds a new member. FSB (base package) releases 35.0.0 supporting (and requiring) that member. If you want to use FS 3.5.0 for that new member, and your custom file system implementation doesn't have a version that works with FSB 35.0.0, it won't solve. You can use FS 3.4.0/FSB 34.0.0, but not the new member, or you can make CustomeFS release a version that implements FSB 35.0.0's FileSystem.

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.

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 don't want people to check for UnsupportedError. What I was imagining is that the user can safely upgrade io_files and they will not get UnsupportedError unless they call some of the newly added methods. If they do start calling new methods and get UnsupportedError then that is a signal that a dependency (e.g. 3p FileSystem implementation) must be upgraded.

}

/// Reads the entire file contents as a list of bytes.
Uint8List readAsBytes(String path) {
throw UnsupportedError('readAsBytes');
Expand Down
257 changes: 255 additions & 2 deletions pkgs/io_file/lib/src/vm_windows_file_system.dart
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,13 @@ import 'package:win32/win32.dart' as win32;
import 'file_system.dart';
import 'internal_constants.dart';

const _hundredsOfNanosecondsPerMicrosecond = 10;

DateTime _fileTimeToDateTime(int t) {
final microseconds = t ~/ _hundredsOfNanosecondsPerMicrosecond;
return DateTime.utc(1601, 1, 1, 0, 0, 0, 0, microseconds);
}

String _formatMessage(int errorCode) {
final buffer = win32.wsalloc(1024);
try {
Expand Down Expand Up @@ -66,14 +73,140 @@ Exception _getError(int errorCode, String message, String path) {
}
}

/// File system entity data available on Windows.
final class WindowsMetadata implements Metadata {
// TODO(brianquinlan): Reoganize fields when the POSIX `metadata` is
// available.
// TODO(brianquinlan): Document the public fields.

/// Will never have the `FILE_ATTRIBUTE_NORMAL` bit set.
int _attributes;

@override
bool get isDirectory => _attributes & win32.FILE_ATTRIBUTE_DIRECTORY != 0;

@override
bool get isFile => !isDirectory && !isLink;

@override
bool get isLink => _attributes & win32.FILE_ATTRIBUTE_REPARSE_POINT != 0;

@override
final int size;

bool get isReadOnly => _attributes & win32.FILE_ATTRIBUTE_READONLY != 0;
bool get isHidden => _attributes & win32.FILE_ATTRIBUTE_HIDDEN != 0;
bool get isSystem => _attributes & win32.FILE_ATTRIBUTE_SYSTEM != 0;

// TODO(brianquinlan): Refer to
// https://learn.microsoft.com/en-us/previous-versions/windows/internet-explorer/ie-developer/windows-scripting/5tx15443(v=vs.84)?redirectedfrom=MSDN
bool get needsArchive => _attributes & win32.FILE_ATTRIBUTE_ARCHIVE != 0;
bool get isTemporary => _attributes & win32.FILE_ATTRIBUTE_TEMPORARY != 0;
bool get isOffline => _attributes & win32.FILE_ATTRIBUTE_OFFLINE != 0;
bool get isContentIndexed =>
_attributes & win32.FILE_ATTRIBUTE_NOT_CONTENT_INDEXED == 0;

final int creationTime100Nanos;
final int lastAccessTime100Nanos;
final int lastWriteTime100Nanos;

DateTime get creation => _fileTimeToDateTime(creationTime100Nanos);
DateTime get access => _fileTimeToDateTime(lastAccessTime100Nanos);
DateTime get modification => _fileTimeToDateTime(lastWriteTime100Nanos);

WindowsMetadata._(
this._attributes,
this.size,
this.creationTime100Nanos,
this.lastAccessTime100Nanos,
this.lastWriteTime100Nanos,
);

/// TODO(bquinlan): Document this constructor.
///
/// Make sure to reference:
/// [File Attribute Constants](https://learn.microsoft.com/en-us/windows/win32/fileio/file-attribute-constants)
factory WindowsMetadata.fromFileAttributes({
int attributes = 0,
int size = 0,
int creationTime100Nanos = 0,
int lastAccessTime100Nanos = 0,
int lastWriteTime100Nanos = 0,
}) => WindowsMetadata._(
attributes == win32.FILE_ATTRIBUTE_NORMAL ? 0 : attributes,
size,
creationTime100Nanos,
lastAccessTime100Nanos,
lastWriteTime100Nanos,
);

/// TODO(bquinlan): Document this constructor.
factory WindowsMetadata.fromLogicalProperties({
bool isDirectory = false,
bool isLink = false,

int size = 0,

bool isReadOnly = false,
bool isHidden = false,
bool isSystem = false,
bool needsArchive = false,
bool isTemporary = false,
bool isOffline = false,
bool isContentIndexed = false,

int creationTime100Nanos = 0,
int lastAccessTime100Nanos = 0,
int lastWriteTime100Nanos = 0,
}) => WindowsMetadata._(
(isDirectory ? win32.FILE_ATTRIBUTE_DIRECTORY : 0) |
(isLink ? win32.FILE_ATTRIBUTE_REPARSE_POINT : 0) |
(isReadOnly ? win32.FILE_ATTRIBUTE_READONLY : 0) |
(isHidden ? win32.FILE_ATTRIBUTE_HIDDEN : 0) |
(isSystem ? win32.FILE_ATTRIBUTE_SYSTEM : 0) |
(needsArchive ? win32.FILE_ATTRIBUTE_ARCHIVE : 0) |
(isTemporary ? win32.FILE_ATTRIBUTE_TEMPORARY : 0) |
(isOffline ? win32.FILE_ATTRIBUTE_OFFLINE : 0) |
(!isContentIndexed ? win32.FILE_ATTRIBUTE_NOT_CONTENT_INDEXED : 0),
size,
creationTime100Nanos,
lastAccessTime100Nanos,
lastWriteTime100Nanos,
);

@override
bool operator ==(Object other) =>
other is WindowsMetadata &&
_attributes == other._attributes &&
size == other.size &&
creationTime100Nanos == other.creationTime100Nanos &&
lastAccessTime100Nanos == other.lastAccessTime100Nanos &&
lastWriteTime100Nanos == other.lastWriteTime100Nanos;

@override
int get hashCode => Object.hash(
_attributes,
size,
isContentIndexed,
creationTime100Nanos,
lastAccessTime100Nanos,
lastWriteTime100Nanos,
);
}

/// A [FileSystem] implementation for Windows systems.
base class WindowsFileSystem extends FileSystem {
@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),
Expand All @@ -85,6 +218,126 @@ base class WindowsFileSystem extends FileSystem {
}
});

/// Sets metadata for the file system entity.
///
/// TODO(brianquinlan): Document the arguments.
/// Make sure to document that [original] should come from a call to
/// `metadata`. Creating your own `WindowsMetadata` will result in unsupported
/// fields being cleared.
void setMetadata(
Copy link
Member

Choose a reason for hiding this comment

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

Would it make any sense to take a Metadata argument that can be used as base, instead of fetching it from the file system.

It's not an atomic operation to setMetadata if it starts by reading metadata, does updating, and the writes it back. If it's not an atomic operation, it means the file system can change between fetching the existing attributes and writing them back.
Not being atomic is generally not a good thing for low-level functions.

It also means that if I first call metadata to look at the data, then decide to change the values using setMetadata, then I'll actually read the attributes twice.

If I could pass the WindowsMetadata I got from metadata as a base argument to setMetadata, so it uses that instead of fetching new, then the set operation becomes atomic. (The entire operation doesn't, but that's clear when I call two different methods.)

If the base argument is omitted, it can still fetch the attributes from the file system.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

String path, {
bool? isReadOnly,
bool? isHidden,
bool? isSystem,
bool? needsArchive,
bool? isTemporary,
bool? isContentIndexed,
bool? isOffline,
WindowsMetadata? original,
}) => using((arena) {
if ((isReadOnly ??
isHidden ??
isSystem ??
needsArchive ??
isTemporary ??
isContentIndexed ??
isOffline) ==
null) {
return;
}
final fileInfo = arena<win32.WIN32_FILE_ATTRIBUTE_DATA>();
final nativePath = path.toNativeUtf16(allocator: arena);
int attributes;
if (original == null) {
if (win32.GetFileAttributesEx(
nativePath,
win32.GetFileExInfoStandard,
fileInfo,
) ==
win32.FALSE) {
final errorCode = win32.GetLastError();
throw _getError(errorCode, 'set metadata failed', path);
}
attributes = fileInfo.ref.dwFileAttributes;
} else {
attributes = original._attributes;
}

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;
}

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,
needsArchive,
);
attributes = updateBit(
attributes,
win32.FILE_ATTRIBUTE_TEMPORARY,
isTemporary,
);
attributes = updateBit(
attributes,
win32.FILE_ATTRIBUTE_NOT_CONTENT_INDEXED,
isContentIndexed != null ? !isContentIndexed : null,
);
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;
}
Copy link
Member

Choose a reason for hiding this comment

The 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?
So keep a copy of the original, and if (original == attributes) return;.
It's not the same - if the underlying file system has changed in the meantime, it matters whether we write or not. Especially if using a base metadata argument, then you should probably always write.)

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 supporting base (called original) and always write.

if (win32.SetFileAttributes(nativePath, attributes) == win32.FALSE) {
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;
return WindowsMetadata.fromFileAttributes(
attributes: attributes,
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,
);
});

@override
Uint8List readAsBytes(String path) => using((arena) {
// Calling `GetLastError` for the first time causes the `GetLastError`
Expand Down
9 changes: 7 additions & 2 deletions pkgs/io_file/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,18 @@ dependencies:
ffi: ^2.1.4
stdlibc:
git:
# Change this to a released version.
# TODO(brianquinlan): Change this to a released version.
url: https://github.com/canonical/stdlibc.dart.git
win32: ^5.11.0
win32:
git:
# TODO(brianquinlan): Change this to a released version.
url: https://github.com/halildurmus/win32.git
path: packages/win32

dev_dependencies:
args: ^2.7.0
benchmark_harness: ^2.3.1
dart_flutter_team_lints: ^3.4.0
path: ^1.9.1
test: ^1.24.0
uuid: ^4.5.1
5 changes: 2 additions & 3 deletions pkgs/io_file/test/file_system_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,12 @@
// 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.

import 'package:io_file/io_file.dart';
import 'package:test/test.dart';

void main() {
group('FileSystem', () {
test('rename', () {
expect(() => FileSystem().rename('a', 'b'), throwsUnsupportedError);
test('TODO(brianquinlan)', () {
// Add tests.
});
});
}
Loading
Loading