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 31 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
20 changes: 17 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,23 @@
// 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).
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');
}
49 changes: 49 additions & 0 deletions pkgs/io_file/lib/src/file_system.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Member

Choose a reason for hiding this comment

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

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'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.
Copy link
Member

Choose a reason for hiding this comment

The 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 WindowsMetadata below on Windows?
(That is: What is the use-cases for this class. Is it a default least-common-denominator of the platform specific metadata subclasses? Can users use it as input to any function of the file system, where they can fx. choose between passing in a Metadata or a WindowsMetadata to a WindowsFilesystem? If so, they can use this class on Windows. If not, does it need a public constructor?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You don't have to use WindowsMetadata on Windows. The usual pattern of calling fileSystem.metadata(path) will return a Metadata, which should contain the metadata available on all platforms. WindowsMetadata is designed to capture the Windows platform-specific attributes.

The constructor is public because I want other people to be able to implement their own file system that return Metadata or a subclass.

Maybe I should make this an abstract interface? But then I won't be able to add to it in the future.

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(
'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 {
Copy link
Member

Choose a reason for hiding this comment

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

(Make this abstract? Don't expect anyone creating an instance without extending is intended. In that case, these methods could be abstract instead, forcing subclasses to implement them.
Unless you want to use instances of this class as a null-object for file systems. Then you should give it a const constructor. Probably should in any case, so one can create constant singleton instances of subclasses.)

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.

/// Renames, and possibly moves a file system object from one path to another.
Expand All @@ -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');
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.

}
}
211 changes: 209 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 @@ -10,6 +10,11 @@ import 'package:win32/win32.dart' as win32;

import 'file_system.dart';

DateTime _fileTimeToDateTime(int t) {
final microseconds = t ~/ 10;
Copy link
Member

Choose a reason for hiding this comment

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

(Might want a constant hundredsOfNanosecondsPerMicrosecond 😁)

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.

return DateTime.utc(1601, 1, 1).add(Duration(microseconds: microseconds));
Copy link
Member

Choose a reason for hiding this comment

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

Can be just DateTime.utc(1601, 1, 1, 0, 0, 0, 0, microseconds).

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 _formatMessage(int errorCode) {
final buffer = win32.wsalloc(1024);
try {
Expand Down Expand Up @@ -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;
Copy link
Member

Choose a reason for hiding this comment

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

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.


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,
Copy link
Member

Choose a reason for hiding this comment

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

Sounds like answering whether it's a zip file :)

Maybe hasArchiveFlag, isArchiveReady, isArchiveFlagSet (too long and indirect).

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 just using the Microsoft names (with an is prefix and dartCasing). I don't mind changing it but I'd like to be able to refer to Microsoft's documentation and it be clear which of our fields correspends to their attributes.

I'm file with hasReadOnlyFlag, etc. but we should do that everywhere for consistency

Copy link
Member

Choose a reason for hiding this comment

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

Adding is works for most things, but not all. One size doesn't fit all, as usual :)

I can see why "Flag" isn't great in the name. needsArchive, isArchiveReady or isArchiveable might work (if the last one is even a word). So stepping back ...

The read only flag means that the file "is read-only". ✓
The hidden flag means that the file "is hidden". ✓
The system flag means that the file "is (a) system (file)". ✓

The "archive" bit is really a "dirty" bit, ... and isDirty would be fine, it's just not the traditional name.
Phrased in terms of "archiving", it means "needs to be archived", "do archive" (matching the flag name) and "is not archived", which is the opposite of "is archived", and isArchive just doesn't work in that context.
We could use isArchived if we flip the value. It's my best suggestion so far.

  if (!file.isArchived) copy(file, archivePathOf(file)); // ✓

this.isTemporary = false,
this.isOffline = false,
this.isContentNotIndexed = false,
Copy link
Member

Choose a reason for hiding this comment

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

Consider inverting flag, with a default of true. ...NotIndex = false is a double-negation.
Alternative, isContentUnindexed. Slightly better, "Un..." is not as strong and separate a negation as "Not" when read, it's more part of the thing it binds 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.

I was trying to use the same names as Microsoft (but with dartCasing). What do you think is more valuable:

  1. keep consistent with Microsoft
  2. using names that are actually clear ;-)

I don't have strong feelings about this.

Copy link
Member

Choose a reason for hiding this comment

The 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 isEmpty and isNotEmpty.)


this.creationTime100Nanos = 0,
this.lastAccessTime100Nanos = 0,
this.lastWriteTime100Nanos = 0,
});

@override
bool operator ==(Object other) =>
other is WindowsMetadata &&
super == other &&
Copy link
Member

Choose a reason for hiding this comment

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

This == is not symmetric, MetaData() == WindowsMetadata() is true, the opposite direction is false.

Just because of that, I'd make the superclass abstract, and not have == on any non-leaf class.
(If you want a general metadata, make it a separate subclass which uses is MinimalMetadata in its == too.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch! I made Metadata an interface. Maybe I'll want a MinimalMetadata later (for creating Fakes) but not for now.

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 {
@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 @@ -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(
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? 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;
}

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

final isDirectory = attributes & win32.FILE_ATTRIBUTE_DIRECTORY > 0;
final isLink = attributes & win32.FILE_ATTRIBUTE_REPARSE_POINT > 0;
final isFile = !(isDirectory || isLink);
Copy link
Member

Choose a reason for hiding this comment

The 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 isDirectory and isLink to be true. (Can't check right now, no Windows machine.)

In that case isFile should probably just be !isDirectory (And you don't need to store it, you can just have bool get isFile => !isDiretory;.)

Or you could make isDirectory be (attributes & ...) & !isLink, and then have WindowsMetadata specific getters for isFileLink and/or isDirectoryLink, separate from isFile and isDirectory, so isFile, isDirectory and isLink are mutually exclusive again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In Windows, the type of link is not indicated in GetFileAttributes

And yeah, there is some tension here. Right now I was going to make one of the is* true and the values would be consistent on POSIX and Windows.

Copy link
Member

Choose a reason for hiding this comment

The 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.
(It doesn't say whether it's a junction, symbolic link, or something more esoteric, I think you need a second query to get the reparse-data.)

Every entity in the Windows FS is either a file or a directory, including links, controlled by the DIRECTORY bit.
If it's a reparse-point, then it's also a link, so it can be link-and-file or link-and-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.

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,
Copy link
Member

Choose a reason for hiding this comment

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

(I'd use != 0 instead of > 0. It should be the same unless the bit is the top bit.
But I'd rather not have to worry about the exception, or have a reader wonder why < 0 should be accepted.

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.

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,
);
});
}
9 changes: 7 additions & 2 deletions pkgs/io_file/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,15 @@ 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:
dart_flutter_team_lints: ^3.4.0
path: ^1.9.1
test: ^1.24.0
Loading