language icon indicating copy to clipboard operation
language copied to clipboard

Macros file access

Open davidmorgan opened this issue 1 year ago • 18 comments

The spec currently includes file access

https://github.com/dart-lang/language/blob/main/working/macros/feature-specification.md#resources

it either needs splitting out of the feature, or implementing :)

davidmorgan avatar Jul 02 '24 11:07 davidmorgan

This sounds very much like "assets". Could something be shared? Or is it just an accident of both wanting to access bytes or strings?

I can see a macro having its own binary/string assets available. Then it doesn't need to read from files, they assets are created when the macro is built. If the applying package needs to provide data at compile-time, then it probably cannot use assets, and it needs to provide some reference to its own files. Probably only those inside lib/, since that's all someone else compiling your code has access to.

The API could be made a little more consistent. (I'd personally drop either the async or the sync version. If there is a sync version, I'll use it every time. I don't think a macro is going to be massively parallel, so loading something, more slowly, in the background isn't going to improve performance. I can be wrong. If so, have only the async version.)

/// A read-only resource API for use in macro implementation code.
class Resource {
  /// Either a `package:` URI, or an absolute URI which is under the root of
  /// one or more packages in the current package config.
  final Uri uri;

Don't allow absolute URIs under other packages' roots. The only files of another package that is guaranteed to be available when compiling are those under lib/ (under the package root), and those should only be accessed using package: URIs. That's the only stable access.

For files inside the current package (which is the package that applies the macro), it could be possible to access outside of lib, at least from macro applications that are themselves outside of lib/, but it should probably still be relative to an implicit root, not an absolute URI.

Generally, don't allow absolute references.

  /// Creates a resource reference.
  ///
  /// The [uri] must be valid for this compilation, which means that it exists
  /// under the root URI of one or more packages in the package config file.

package root URI.

  ///
  /// Throws an [InvalidResourceException] if [uri] is not valid.
  Resource(this.uri);

  /// Whether or not a resource actually exists at [uri].
  bool get exists;

As both sync and async?

  /// Read this resource as a stream of bytes.
  Stream<Uint8List> openRead([int? start, int? end]);

openRead([int start = 0, int? end])

  /// Asynchronously reads this resource as bytes.
  Future<Uint8List> readAsBytes();

Allow [int start = 0, int? end] here too, and for the sync version. Maybe also for the strings. Maybe especially for those. I could see myself putting all string resources into a single file, with an index at the start, or hard-coded offsets, and then read a string when I need it.

  /// Synchronously reads this resource as bytes.
  Uint8List readAsBytesSync();

  /// Asynchronously reads this resource as text using [encoding].
  Future<String> readAsString({Encoding encoding = utf8});

  /// Synchronously reads this resource as text using [encoding].
  String readAsStringSync({Encoding encoding = utf8});

  /// Asynchronously reads the resource as lines of text using [encoding].
  Future<List<String>> readAsLines({Encoding encoding = utf8});

  /// Synchronously reads the resource as lines of text using [encoding].
  List<String> readAsLinesSync({Encoding encoding = utf8});
}

lrhn avatar Jul 02 '24 14:07 lrhn

This sounds very much like "assets". Could something be shared?

"Assets" (assuming you mean flutter assets) are afaik a runtime only concept, things to be bundled in the final APK of your app and accessed at runtime. "Resources" are a compile time only concept for macros. They generally should not be available in the final application. So, I don't think they overlap.

Or is it just an accident of both wanting to access bytes or strings?

This is just a property of anything wanting to read files?

I can see a macro having its own binary/string assets available. Then it doesn't need to read from files, they assets are created when the macro is built. If the applying package needs to provide data at compile-time, then it probably cannot use assets, and it needs to provide some reference to its own files. Probably only those inside lib/, since that's all someone else compiling your code has access to.

The primary intended use case is people supplying files as input to macros (for example, a template or schema). We need to provide access to things outside of lib/ at least in the case that the macro is applied to a library not under lib/.

(I'd personally drop either the async or the sync version. If there is a sync version, I'll use it every time. I don't think a macro is going to be massively parallel, so loading something, more slowly, in the background isn't going to improve performance. I can be wrong. If so, have only the async version.)

The sync API might not be implementable in all situations, so I would probably go for the async one (this is what build_runner does).

Don't allow absolute URIs under other packages' roots. The only files of another package that is guaranteed to be available when compiling are those under lib/ (under the package root), and those should only be accessed using package: URIs. That's the only stable access.

For files inside the current package (which is the package that applies the macro), it could be possible to access outside of lib, at least from macro applications that are themselves outside of lib/, but it should probably still be relative to an implicit root, not an absolute URI.

We could possibly say something like "a macro applied under lib can only read package: URIs". I am not sure what that would look like to enforce.

But yes generally the intention here is to allow macro applications outside of lib to access files outside of lib.

Note that for assets, flutter does allow access to them outside of lib, even for other packages. One could definitely argue that is a bad thing though.

In general, these absolute URIs can only be reliably constructed from relative URIs anyways. They are just converted to an absolute URI under the covers. Attempting to do anything else will fail for a published package. And I don't particularly care if a user hard codes a weird absolute path for their own local development.

We could say that the actual resource API only accepts relative paths or package scheme URIs?

Stream<Uint8List> openRead([int? start, int? end]);

What if this was the only API we exposed directly, and the rest was all extension methods? Would that be problematic?

jakemac53 avatar Jul 02 '24 15:07 jakemac53

"Assets" (assuming you mean flutter assets)

I mean Dart data assets. They are also runtime only, but a macro that is running could use its own assets, or assets of its dependencies.

lrhn avatar Jul 02 '24 20:07 lrhn

They are also runtime only, but a macro that is running could use its own assets, or assets of its dependencies.

Ah, so still seems like a different feature then, although maybe we could share some APIs, I can take a look.

jakemac53 avatar Jul 02 '24 21:07 jakemac53

One use-case to keep in mind is: Project-wide configuration files, such as build_runner's build.yaml It is placed next to the pubspec.yaml, and needs to be read by macro applications inside /lib.

I'd expect macros to be able to read anything that's at the pubspec level or lower.

rrousselGit avatar Jul 03 '24 02:07 rrousselGit

FWIW, this looks like something we could safely move to the "Later" milestone and out of the first feature spec :)

davidmorgan avatar Jul 03 '24 06:07 davidmorgan

As in, not have this in the initial stable release?

I do think it's quite important to be able to define a global configuration file. Lots of generators have global configurations. Removing such a file would make migrating to macros a bit painful.

rrousselGit avatar Jul 03 '24 10:07 rrousselGit

Yes. As mentioned elsewhere, we are looking for ways to reduce the feature scope so we can launch sooner, and this looks like an easy one: the functionality is orthogonal and straightforward, and the feature works fine for many use cases without it.

davidmorgan avatar Jul 04 '24 06:07 davidmorgan

I'd expect macros to be able to read anything that's at the pubspec level or lower.

Those are a part of the package, and so they would typically be covered by the package_config.json. However, certain configurations only expose the lib dir in the package_config.json, for dependencies. Because of that, reaching outside the lib dir from a macro applied in lib, might break in some situations. It is also for example considered by many a mistake that pub downloads files outside of lib at all, as you end up getting many files that are not useful and it is just a waste of network bandwidth.

I would certainly expect for the root package, macro applications outside of lib will be able to read any file in the root package though.

And, we might also allow access from lib/ to outside of lib/ within the same package, but that is a little less clear.

jakemac53 avatar Jul 08 '24 16:07 jakemac53

And, we might also allow access from lib/ to outside of lib/ within the same package, but that is a little less clear.

That has all the same issues. If another package refers to "this" package, its code can access anything inside lib/, and nothing outside, so of this package's lib/ code accesses anything outside of lib/, it won't work when used as a dependency.

Always assume that code inside lib/ can be accessed from anywhere, only using package: URIs, and code outside can only be accessed from the same package, and never from inside lib/.

(Don't know if hooks broke that rule.)

lrhn avatar Jul 08 '24 19:07 lrhn

That has all the same issues. If another package refers to "this" package, its code can access anything inside lib/, and nothing outside, so of this package's lib/ code accesses anything outside of lib/, it won't work when used as a dependency.

I agree it has potential issues, but we might allow it anyways. Afaik flutter already allows access to assets outside of lib/ so the ship has somewhat sailed on only allowing access to lib/ for dependencies.

There is a concrete difference as well, which is you do have a reliable way to reference the files (relative path). There is no way at all to reference files outside of lib/ from another package.

I certainly understand the desire to put as an example your schema files in a top level schema/ dir, and then have macros applied to some file under lib/ which generate the classes based on that schema. Yes, we could just require those files to be under lib/, and we might, but the question is whether that is purely an arbitrary decision or whether it provides concrete value to impose.

jakemac53 avatar Jul 08 '24 19:07 jakemac53

flutter already allows access to assets

Those assets are probably included in the built program by the build system, so they are known to be accessible to running code through a designated API. It's not just any file outside of lib/.

We can allow access to things outside of lib/ only if we designate them as a kind of "runtime accessible assets", in some way, and make the compilers provide access to their content in the compiled program.

That, again, sounds like the Dart "assets" feature.

lrhn avatar Jul 09 '24 07:07 lrhn

It is also for example considered by many a mistake that pub downloads files outside of lib at all, as you end up getting many files that are not useful and it is just a waste of network bandwidth.

Fwiw, package authors can specify .pubignore and not publish anything that's useless for users. Someone who only cares about the lib folder could write something among the lines of:

*
!lib/
!pubspec.yaml
# TODO Include more, like LICENSE and stuff

So I don't think the bandwidth concern is a real issue here. If something shouldn't be published, authors should ignore those.

rrousselGit avatar Jul 09 '24 07:07 rrousselGit

We can allow access to things outside of lib/ only if we designate them as a kind of "runtime accessible assets", in some way, and make the compilers provide access to their content in the compiled program.

These are compile time only assets, we do not want them compiled as a part of the program.

jakemac53 avatar Jul 09 '24 19:07 jakemac53

We do want the assets used by the macro implementation to be runtime assets for the compiled macro program.

We don't have a distinction between code from another package that's used by a macro implementation, and code from another package that's used by "final program". For all we know the same library can be used for both.

That is, we don't have a definition of "the program". Each macro implementation is a program.

lrhn avatar Jul 10 '24 06:07 lrhn

We do want the assets used by the macro implementation to be runtime assets for the compiled macro program.

This is out of scope, it is not describing the same problem the macro Resource API is trying to solve. Maybe this could be solved by the Dart Assets proposal, but I don't know of any use cases for it amongst all the potential macros we have evaluated.

We don't have a distinction between code from another package that's used by a macro implementation, and code from another package that's used by "final program". For all we know the same library can be used for both.

The specific problem we are trying to solve, is passing an external resource from the users program (more specifically, the program which is currently being compiled, which could be a macro program too) to a macro (separate program) via a URI. For example a template file or schema file. The resource in this case only needs to exist in the users compile time assets. It also only needs to be accessible via the package config of the users program (not the macro program, which might be different).

Both build_runner and bazel do have a distinction between compile time and runtime assets. These "Resource"s fit squarely into the compile time ones in those contexts.

Flutter apps also have that distinction I think (the assets in that context are runtime assets, anything else is compile time only). Dart apps afaik today don't really support either, but it sounds like we are getting a notion of runtime assets. The macro Resources are compile time assets.

jakemac53 avatar Jul 10 '24 15:07 jakemac53

A library that converts assets/images/avatar.png to dart Assets.images.avatar if you want to experiment and try macros for it, you can fork it or I can add you as a collaborator https://github.com/amorenew/assets_scanner_plus It use a yaml file for configurations assets_scanner_plus_options.yaml

amrgetment avatar Oct 04 '24 14:10 amrgetment

We don't need any contributions related to file handling at this time. Thanks.

davidmorgan avatar Oct 04 '24 14:10 davidmorgan