google / file.dart Goto Github PK

Web is still in beta, but I'm trying to make my packages compatible with web.
As a first quick fix for flutter_cache_manager I made use of the MemoryFileSystem to save the files and don't need any changes. This works perfectly as a temporary memory cache, but I can't get a 'web' badge on pub.

The problem is that 'File' of this package implements 'io.File'. Really the only reference to 'dart:io' is that interface. At least File has a counterpart in dart:html, so maybe we can create a conditional import for that, but Directory doesn't have a counterpart. The ideal solution would be to make a generic interface (abstract class) in the dart engine which is implemented by the abstract io.File. However, that would need everybody to use the generic File interface instead of the io interface. It wouldn't really break anything though when people still use the io.File as long as they don't use flutter_cache_manager for example which would use the generic one.

This is a pretty big change for relatively little effect (mainly the badge, because it already works on the web). So I mainly want your opinion whether it is worth it to create a proposal and really make such a change. If you have any other ideas how to fix this it that would also be appreciated.

The next flutter/engine -> flutter/flutter roll will receive a new 2.1.0-dev Dart SDK, which requires us to upgrade all 3rd party packages. Most of them have already newer versions published which are compatible with Dart 2, but the latest commits to this package:file repository haven't been published yet.

@tvolkert @yjbanov @matanlurey Would you mind publishing a new version? (This is blocking flutter/flutter#20427)

Provides the ability to mount arbitrary file systems as "directories"

@cbracken, coupled with ChrootFileSystem, this would provide the functionality you were talking about a few weeks ago.

Is there a way to use the testing package (https://github.com/google/file.dart/tree/master/packages/file_testing) in our projects which use file.dart. I'd like to use the matchers in my own unit testers.

It might be nice to have a ChrootFileSystem implementation that automatically encrypted all writes and decrypted all reads.

file tests fail due to type errors of the familiar

type '_InternalLinkedHashMap<String, dynamic>' is not a subtype of type 'Map<String, String>' of 'input'

variety.

See: https://travis-ci.org/google/file.dart/jobs/414381120

A few changes get us most of the way there:

PathContextCodec =>

static path.Context _decode(Map<String, dynamic> input) {

and

static const Converter<Map<String, dynamic>, path.Context> deserialize =
      const _ForwardingConverter<Map<String, dynamic>, path.Context>(_decode);

but someone who understands the source should look a little closer.

/cc @tvolkert ?

MemoryFile.readAsLines/readAsLinesSync behave differently than normal File when there is a trailing newline in the file:

import 'package:file/file.dart';
import 'package:file/memory.dart';

void main() {
  final fs = MemoryFileSystem();
  final f = fs.file('test.txt');
  f.writeAsStringSync('\n');
  final lines = f.readAsLinesSync();
  print('${lines.length}'); // Prints 2.
}

If using LocalFileSystem or dart:io's File instead, readAsLinesSync() will return a List<String> with only one element.

readAsBytes indicates that the file was written correctly, so the problem is in readAsLinesSync.

Steps to reproduce

Create foo.dart, containing the following:

import 'dart:async';

import 'package:file/file.dart';
import 'package:file/local.dart';

Future<void> main() async {
  FileSystem fs = const LocalFileSystem();
  Directory dir = fs.directory('foo')..createSync();
  await dir.rename('baz');
}

Run the file in Dart 2 mode:

$ dart --preview-dart-2 foo.dart

Expected result

A directory named foo should be created, then renamed to baz (as happens when you run that code in Dart 1 mode).

Actual result

Unhandled exception:
type 'LocalDirectory' is not a subtype of type 'FutureOr<LocalDirectory>' of 'value'
#0      _AsyncAwaitCompleter.complete (dart:async/runtime/libasync_patch.dart)
#1      ForwardingFileSystemEntity.rename (package:file/src/forwarding/forwarding_file_system_entity.dart)
<asynchronous suspension>
#2      main (file:///Users/tvolkert/project/file.dart/packages/file/foo.dart:9:13)
<asynchronous suspension>
#3      _startIsolate.<anonymous closure> (dart:isolate/runtime/libisolate_patch.dart:279:19)
#4      _RawReceivePortImpl._handleMessage (dart:isolate/runtime/libisolate_patch.dart:165:12)

https://pub.dartlang.org/packages/file points to an outdated github under "Homepage" on the right hand side.

... rather than expecting raw strings, which is fragile (it won't work across locales or operating systems)

This will fix a bunch of tests that currently fail when run on Windows.

FYI @goderbauer

A ForwardingFileSystem that throws for all write operations.

This prevents deletes of files that contain forward slashes.

Example:

import 'package:file/memory.dart';

void main() {
  final fs = MemoryFileSystem(style: FileSystemStyle.windows);
  final file = fs.file(r'C:\a\b/c.dart');
  file.createSync(recursive: true);
  file.deleteSync();
  print(file.existsSync()); // true, expected false
}

I did some debugging locally and found that basename in the MemoryFileSystemEntity is listed as c.dart here, instead of b/c.dart.

Using a recent dev version of Dart, like 2.0.0-dev.55.0 or so:

dart --preview-dart-2 test/replay_test.dart

See many failures like:

00:00 +3 -6: Replay common FileSystem file rerun allowsDirectoryArgument [E]
  type '_InternalLinkedHashMap<String, dynamic>' is not a subtype of type 'Map<String, String>' of 'input'
  package:file/src/backends/record_replay/codecs.dart                     _ForwardingConverter.convert
  package:file/src/backends/record_replay/replay_proxy_mixin.dart 134:30  _ReplayFileSystemImpl&FileSystem&ReplayProxyMixin.noSuchMethod
  package:file/src/interface/file_system.dart                             _ReplayFileSystemImpl&FileSystem&ReplayProxyMixin.path
  test/common_tests.dart 134:17                                           runCommonTests.<fn>.ns

RecordingFileSystem will delegate to an underlying FileSystem, recording all activity that passes through it.

ReplayFileSystem will take a serialized recording and respond to matching calls with recorded info (and throw for calls not in the recording).

This will enable record/replay tests of code that accesses the file system.

I added a MemoryFileSystem-based implementation of RandomAccessFile in #136. I enabled File > Open tests for test/memory_test.dart, but I did not notice that various other tests depend on MemoryFileSystem and also explicitly disable File > open tests.

Attempting to enable File > open tests in test/replay_test.dart generates lots of errors that I don't yet understand:

test/replay_test.dart: Replay common File open READ RandomAccessFile read rerun read [E]
  type 'List<dynamic>' is not a subtype of type 'Uint8List' of 'input'
  package:file/src/backends/record_replay/codecs.dart                          Passthrough.convert
  package:file/src/backends/record_replay/replay_proxy_mixin.dart 133:30       ReplayProxyMixin.noSuchMethod
  package:file/src/backends/record_replay/replay_random_access_file.dart 17:7  ReplayRandomAccessFile.readSync
  test/common_tests.dart 1825:41                                               runCommonTests.<fn>.<fn>.<fn>.testRandomAccessFileOperations.<fn>.<fn>.<fn>

test/replay_test.dart: Replay common File open READ RandomAccessFile read rerun readIntoWithBufferLargerThanContent [E]
  No matching invocation found: readIntoSync(Instance(length:1024) of '_GrowableList'0, null, )
  package:file/src/backends/record_replay/replay_proxy_mixin.dart 125:7        ReplayProxyMixin.noSuchMethod
  package:file/src/backends/record_replay/replay_random_access_file.dart 17:7  ReplayRandomAccessFile.readIntoSync
  test/common_tests.dart 1832:37                                               runCommonTests.<fn>.<fn>.<fn>.testRandomAccessFileOperations.<fn>.<fn>.<fn>

I'm adding a MemoryRandomAccessFile class (PR #136). The current implementation does not do anything special to honor file system permissions. I have not looked closely at MemoryFile, but I suspect that it doesn't either.

Admittedly, file permissions are not very meaningful for an ephemeral MemoryFileSystem that is local to the running application; anything in it is something that the application put there, so it implicitly has permission to read or write or do whatever it wants.

However, honoring file permissions would improve testing fidelity (i.e., verifying that an application would behave correctly if permission errors are encountered). Note that dart:io currently provides no mechanism to modify permissions (see dart-lang/sdk#15078), so MemoryFileSystem would need to provide its own.

how change RecordingFileSystem in the new version?

ChrootFileSystem attempts to use the filesystem path style of the source filesystem. This fails because rootPrefix on Windows is "C:", so the current directory gets set to that. Later directory queries result in paths like "c:\chroot\path\herec:\subpath". Any attempts to set the current directory to "/" or "\" fails because it can't find a file.

ChrootFileSystem should enforce a specific path style for consistent behavior. "/" should always be the root of the ChrootFileSystem, both types of slashes should be accepted (and converted to the source FileSystem's slashes).

Coverage run has identified that we need to add test coverage to the following:

• Verify that the individual fields in FileStat are what we expect them to be in FileSystem.stat() and FileSystemEntity.stat()
• Test the concrete methods in FileSystem (isFile(), isDirectory(), and isLink())

It's currently not implemented out of convenience. It shouldn't be too much work.

The Dart SDK constraint needs to be updated to declare compatibility with Dart 2 stable:

1. https://github.com/google/file.dart/blob/master/packages/file/pubspec.yaml#L20
2. https://github.com/google/file.dart/blob/master/packages/file_testing/pubspec.yaml#L13

• Dart SDK : 2.0.0-dev.69.2 (Mon Jul 23 17:10:56 2018 +0200) on "windows_x64"
• file_testing : 2.0.0

I'm using the file_testing package with the test package. The issue is that file_testing depends on test: ^0.12.33, but I'm using the version 1.3.0. So I can't install the dependencies of my project :

$ pub get
Resolving dependencies...
Because no versions of file_testing match >2.0.0 <3.0.0 and file_testing 2.0.0 depends on test ^0.12.33, file_testing ^2.0.0 requires test ^0.12.33.
So, because my_project depends on both test ^1.3.0 and file_testing ^2.0.0, version solving failed.
Process finished with exit code 1

It would be cool if the test constraint could be upgraded.

In #157, I made MemoryFile/MemoryRandomAccessFile try to handle the file being renamed/moved/removed from under them. Currently they always follow POSIX behavior (the equivalent of the open file handle tracks the equivalent of the inode), which is consistent with how dart:io File behaves on POSIX systems.

I have not verified what happens on Windows systems, but I expect that rename/move/remove operations on dart:io's File will fail with an exception if they're attempted on files with open handles. If so, MemoryFile should follow the behavior and
would need to keep track of openRead/openWrite/open/openAsync calls (and determine when the corresponding Stream/IOSink/MemoryRandomAccessFile is closed) and disallow the file from being renamed/moved/removed. (For completeness I think we'd also need to prevent such operations on all parent directories too.)

Since one might want to test behavior for a platform other what a test is running on, @dnfield suggested that we could make the behavior configurable based on the FileSystemStyle.

A ForwardingFileSystem that throws for all sync operations.

While adding tests for #142, I discovered that ReplayFile has the opposite problem: it discards trailing blank lines.

          f.writeAsStringSync('\n');
          expect(f.readAsLinesSync(), <String>['']); // Fails; actual value is [].

          f.writeAsStringSync('\n\n');
          expect(f.readAsLinesSync(), <String>['', '']); // Fails; actual value is ['']

I think that this is happening when the blob is written (when ReplayFile.readAsLinesSync is called for the first case, the blobToBytes converter returns an empty Uint8List.)

Sometimes you want to test everything in your project with DDC. MemoryFileSystem imports too much to be compiled with DDC.

Currently, MemoryFileSystem emulates POSIX semantics. Sometimes you actually need to test that code works correctly when Windows file semantics are used. It would be nice if you could switch MemoryFileSystem to Windows mode.

See flutter/flutter#14246 for an example where this would be desirable.

After version 5.0.8, packages using package:file in conjunction with a StreamConsumer<List<int>> will break in recent versions, because you can no longer pipe Stream<Uint8List> to a StreamConsumer<List<int>>. (dart-lang/sdk#37179)

I'm more than willing to send a PR for this, but I think the File interface should include overrides that indicate that calls to openRead, etc. return Uint8List.

It was pretty hard to track this down, but it turned out to be a pretty breaking change in my experience.

Again, willing to send in a PR.

The test package version in the dependency for file_testing should be updated to atleast version 1.2.0.
Currently when this package is used in flutter tools with an updated Dart SDK and mockito package updated to 3.0.0 it results in errors

Building flutter tool...
Because file_testing >=2.0.1 <2.0.2 depends on test ^0.12.33 and flutter_tools
depends on test 1.3.0, file_testing >=2.0.1 <2.0.2 is forbidden.
So, because flutter_tools depends on file_testing 2.0.1, version solving failed.

See flutter/flutter#26017 for more.

See skipOnPlatform ('windows') in https://github.com/google/file.dart/blob/master/test/local_test.dart

ForwardingFileSystem already exists and is the basis of ChrootFileSystem and LocalFileSystem. We probably should expose it publicly.

As is tradition!

Spawned from a thread on Gitter, it would be nice to be able to create a file system that uses a maximum amount of resources when reading from a file system.

It would use package:pool.

Suggestion:

abstract class PooledFileSystem implements FileSystem {
    factory PooledFileSystem({
      // Default to LocalFileSystem?
      FileSystem fileSystem,

      // Default to a Pool where count = Platform.numberOfProcessors * 2?
      Pool pool,
    }) = ...
}

Originally filed by @matanlurey

Something along the lines of this:

https://github.com/flutter/flutter/blob/ee97e5639ec704df4b5179cc88ebc19f0de9d8bd/packages/flutter_tools/lib/src/base/file_system.dart#L35

(originally filed by @xster)

(and then enable the corresponding tests in memory_test.dart)

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

abstract class FileSystem {
  path.Context get path;
}

This will allow users of any file system to reliably be able to use path APIs against the correct path Style.

Side note: this will make the pathSeparator field in FileSystem redundant.

It was witnessed performing abysmally with a recording manifest that contained 140,000 entries. We should create a test that synthesizes a large recording in a MemoryFileSystem and tries to replay from that recording, then set a timeout on the test to make sure it performs within acceptable guidelines.

/cc @goderbauer

MemoryFileSystem File.readAsBytesSync() produces different results than the default File.readAsBytesSync(). Adding Uint8List.fromList to the resultant bytes corrects the problem.

I should not have used _Node. I didn't check with the analyzer, but it now says:

Analyzing lib...
  error • The argument type '(_DirectoryNode) → Null' can't be assigned to the parameter type '(_Node) → void' at lib/src/backends/memory/memory_directory.dart:76:9 • argument_type_not_assignable

I added a basic MemoryRandomAccessFile class in PR #136, but I didn't bother implementing lock/lockSync and unlock/unlockSync. Mostly that's because it's not obvious to me how they should behave. (Possibly they should do nothing; on a MemoryFileSystem, whom would we be locking access from?)

Arguably implementations that do something might be desirable for testing fidelity (similar to #138), in which case some mechanism for simulating locks from another process probably would need to be added to MemoryFileSystem.

Running the tests in dart 2 mode, see failures

type 'Future' is not a subtype of type 'Future' where
Future is from dart:async
Future is from dart:async
String is from dart:core

../../file/test/recording_test.dart _RecordingClass.futureProperty
../../file/test/recording_test.dart 78:12 main...

type '_ControllerStream' is not a subtype of type 'Stream' where
_ControllerStream is from dart:async
Stream is from dart:async
String is from dart:core

../../file/test/recording_test.dart _RecordingClass.streamMethod
../..file/test/recording_test.dart 96:36 main...

type 'LocalDirectory' is not a subtype of type 'FutureOr' of ':return_value' where
LocalDirectory is from package:file/src/backends/local/local_directory.dart
LocalDirectory is from package:file/src/backends/local/local_directory.dart
Directory is from dart:io
FutureOr is from dart:async
LocalDirectory is from package:file/src/backends/local/local_directory.dart

dart:async _Completer.completeError
package:file/src/forwarding/forwarding_file_system_entity.dart ForwardingFileSystemEntity.delete
===== asynchronous gap ===========================
dart:async _asyncThenWrapperHelper
package:file/src/forwarding/forwarding_file_system_entity.dart ForwardingFileSystemEntity.delete
../../file/test/common_tests.dart 732:28 runCommonTests....

and more

It would be nice to have something like MemoryFileSystem that can be initialized from an actual directory on disk. Write operations would always write to memory. Read operations would check for an in-memory version of the file first, and if it didn't find it check for an on-disk version.

Today, there are tests in flutter_tool that have to initialize an in memory FS, copy a bunch of stuff from disk, and then go on their way. The proposal here is to let us lazily do that copy so we only end up with more memory usage (and data copying) if we attempt to modify a file (rather than just read it).

/cc @jonahwilliams

See https://github.com/dart-lang/test_descriptor

The LocalFileSystem common File copy succeedsIfDestinationExistsAsLinkToFile and LocalFileSystem common File copy succeedsIfDestinationIsLinkToFileInDifferentDirectory tests seem to fail consistently on Windows. Examples:

• https://ci.appveyor.com/project/tvolkert/file-dart/builds/33475434?fullLog=true
• https://ci.appveyor.com/project/tvolkert/file-dart/builds/33475233?fullLog=true

Based on #56, should these be added to the skip list? They are mentioned in dart-lang/sdk#29096.

I don't have a Windows machine handy to test with.

As we want to add more and more file systems, does it make sense to break into sub-packages?

(Proof of concept: https://github.com/dart-lang/build)

Rationale:

• Avoid adding dependencies just for some implementation (i.e. we require Intl now for Record)
• Allow contributing more specific types of file systems into a mono-repo but separate versions

This came up because I had an idea to work on an ArchiveFileSystem (with a dependency on https://pub.dartlang.org/packages/archive), that acts as a view into an archive file format.

Thoughts?

Every so often, they fail with something like:

FileSystemException: Cannot open file, path = '/tmp/file_test_Wh77a0/foo' (OS Error: No such file or directory, errno = 2)

I'd like to copy a file from the MemoryFileSystem to the LocalFileSystem (basically, write it to disk). When I try to run the following, I get the below error (localFileSystemPath is '/Users/mikem/example.mp4')

  File copyFile(File memoryFileSystemFile, String localFileSystemPath) {
    file.copy(path);
  }

FileSystemException (FileSystemException: No such file or directory, path = 'Users' (OS Error: No such file or directory, errno = 2))

I assume this happens because file.copy tries to copy into a new file on its own filesystem (MemoryFileSystem) but the local file system's /Users directory isn't there. What is the recommended way to copy a file from one FileSystem to another? And can this be done without making an extra copy of the data in memory?

The following does work, but requires the entire file to be read into memory before writing can even begin. Maybe this isn't an issue because it's already in memory (it's on a MemoryFileSystem), and the readAsBytesSync call shortcuts and returns a reference to the existing list of bytes? Just trying to figure out the best way to write a potentially very large file from memory to disk, and this doesn't seem like it. Can't this be done in chunks with Streams to avoid having to read the whole file at once?

  File copyFile(File memoryFileSystemFile, String localFileSystemPath) {
    List bytes = memoryFileSystemFile.readAsBytesSync();
    File newFile = LocalFileSystem().file(localFileSystemPath);
    newFile.writeAsBytesSync(bytes);
    return newFile;
  }

I was moving some tests in Flutter over to the memory file system and hit some issues because of differences in behaviour. In the real implementation, if you use a forward slash in a path, it still resolves correctly. However in the memory file system it treats it as a different path.

The below (when run on Windows) prints:

true
false

import 'package:file/local.dart';
import 'package:file/memory.dart';

final localFs = new LocalFileSystem();
final memoryFs = new MemoryFileSystem(style: FileSystemStyle.windows);

main() {
  [
    new LocalFileSystem(),
    new MemoryFileSystem(style: FileSystemStyle.windows),
  ].forEach((fs) {
    // Create a file at test\test.txt
    fs.currentDirectory = fs.systemTempDirectory;
    fs.directory('test').createSync(recursive: true);
    fs.file('test\\test.txt').createSync();

    // Check if it exists using a forward slash
    print(fs.file('test/test.txt').existsSync());
  });
}

I'll fix this in flutter by using the correct slashes, but I guess the intention is for these to behave the same where possible. It's possibly that #56 would cover this, I'm not sure.