Build: Allow reading non-lib/ assets outside of the root package

[simolus3]

Currently, attempting to read a non-lib/ asset outside of the root package throws an InvalidInputException.
In fact, those assets aren't included in the build at all, since the AssetTracker doesn't pick them up:

https://github.com/dart-lang/build/blob/7a3d15df9b04ec18507455bacb1d23e81677569e/build_runner_core/lib/src/generate/build_definition.dart#L171-L172

I couldn't find any documentation describing _why_ this restriction exists. So I think that, either

• InvalidInputException should have a more descriptive error message explaining why this isn't possible
• or, if possible, the current restrictions should be loosened

As an example of a use-case that can't be written today, consider a builder writing all LICENSE files across dependencies into a Dart string literal. Another common scenarios are multi-package builds: Let's say I had a web app and server in a monorepo. Building the server should also build the app, and copy the generated js into a directory for static files. If the app applies a builder like build_version, the build fails with an InvalidInputException. I'd expect the build to behave exactly like if I only built the app.

[jakemac53]

This is an intentional limitation. Only the files under lib are considered to be the public files in a package, and so those are the only ones we make available.

This is both a performance optimization (far fewer assets in the build) as well as a philosophical argument around not accessing private files.

[jakemac53]

Note that any builder which outputs files to a non-lib dir (like build_version) should build to source, and only be applied as a dev dependency, to avoid this problem.

There are concretely some use cases that are made harder/impossible by this (you might want to read the pubspecs of deps for instance), and I think the LICENSE example could also be considered legitimate (those are conceptually "public" things).

We could possibly consider allowing some specific files or something from deps but that also doesn't feel like a great solution.

[simolus3]

Thanks for the explanation! I agree that this limitation makes sense in general, but maybe it can be loosened.

For instance, do you think it's reasonable to allow builders to read non-lib files from the package enclosing their primary input? This keeps the concept of package-private files intact, while enabling the use-cases I described. It would still be a bit awkward to use (there would have to be a $lib$ (?) -> license.txt builder to leak the file into lib/, followed by another one to collect those files into the main package).

Alternatively, maybe the set of conceptually public files could be defined on a per-package basis in build.yaml. The default would probably be something like ['lib/**', 'LICENSE', 'pubspec.yaml']. Packages wanting to expose other files, like assets, could add additional globs as needed. This also has a smaller performance impact since users would carefully choose what to add.

[jakemac53]

For instance, do you think it's reasonable to allow builders to read non-lib files from the package enclosing their primary input?

If I recall, we did consider this. It also could still be on the table, although it would still involve some performance impacts on its own. For instance it means we would end up parsing (but not compiling) and creating the module structure for all the tests, examples, etc in all your transitive deps, none of which would actually be used by you (or even could be).

Alternatively, maybe the set of conceptually public files could be defined on a per-package basis in build.yaml

Something along these lines would avoid the performance implications. Possibly a combination of the two features could be reasonable.

[simolus3]

I started to play around with the second approach at https://github.com/simolus3/build/tree/configure-public-files. I added a public_assets key to build.yaml, which is used to check whether an asset from a dependency can be accessed or not.

The implementation needs to be cleaned up a bit and I couldn't test this yet, but if you think something like this is worth having, I can open a PR eventually.

[jakemac53]

cc @natebosch wdyt? I can't remember if there were any additional reasons we did not allow this originally.

It is worth mentioning that this wouldn't be something which we could support internally, but that might not matter so much.

Edit: actually on second thought we could support this to some degree internally maybe....we wouldn't ever make dart files outside of lib available from public bazel targets, but could possibly make other files available.

[natebosch]

As an example of a use-case that can't be written today, consider a builder writing all LICENSE files across dependencies into a Dart string literal.

I don't think this use case is addressed by public_assets specified on the package side. Am I understanding correctly that this would require _all_ packages to specify that LICENSE is in public_assets?

I'd like to explore the implications of letting a Builder specify what non-lib assets it wants to read. We would need to decide if it is allowed to do this for _all_ packages, or just for the packages that it will be applied on. The latter sounds safer. @jakemac53 this would be somewhat analogous to our custom aspects to grab extra inputs. WDYT?

[jakemac53]

I don't think this use case is addressed by public_assets specified on the package side. Am I understanding correctly that this would require _all_ packages to specify that LICENSE is in public_assets?

I think the intention of this would be to globally make additional assets available - effectively it would change the default sources we allow for deps. So it would work but all users using builders like this would have to add a build.yaml and add this configuration, etc.

I'd like to explore the implications of letting a Builder specify what non-lib assets it wants to read. We would need to decide if it is allowed to do this for _all_ packages, or just for the packages that it will be applied on.

Flutter does have some use cases that might be made easier if builders could read non-lib files from other packages - specifically for instance the assets dir. For instance you could create an asset manifest builder, or build multiple versions of assets at different sizes, etc.

We wouldn't want builders to be able to add available sources to another package - they would need to already exist in some pre-existing target (otherwise we don't know what target to add the sources to). That also kind of matches with the bazel setup.

The latter sounds safer. @jakemac53 this would be somewhat analogous to our custom aspects to grab extra inputs. WDYT?

It is somewhat similar, and could be implemented using that feature assuming the sources were available in the dependency targets.

[natebosch]

otherwise we don't know what target to add the sources to

Good point, that's probably a blocker.

[simolus3]

Am I understanding correctly that this would require _all_ packages to specify that LICENSE is in public_assets

With the approach I had in mind, yes. But I would expand the default to contain reasonable values based on established package layout conventions (so the default might include bin/**, lib/**, LICENSE, README* and pubspec.yaml to reflect conceptually public files without adding too many assets). So in those obvious use-cases it wouldn't be necessary to change dependent packages.

would be to globally make additional assets available

That's definitely an interesting approach too. In my opinion it kind of defeats the philosophical argument about "package-private" files though, since the control over visibility would be moved away from the package.
If one _really_ needs a specific file across some packages to be visible regardless, the package_name.build.yaml override mechanism could enable that. It's impractical to apply this to all packages of course, but I'll claim that something so common should be the default anyway.

[jakemac53]

If we are adding these files explicitly in each packages build.yaml, that definitely does match up better with the internal (bazel) style approach and is something we could support there which is nice, and alleviates one potential issue.

I think it does make it a bit difficult to write some of these builders, particularly flutter based ones - I think we might get some pressure in that case to add assets/** as a default public sources glob which could have some negative implications giving the potential size of assets under there. But we could take that on as a separate discussion.

[jakemac53]

One additional thought here - in terms of the build outputs there is another restriction here. We currently do not have any place under there in which to put these files.

For all dependencies we output a path like packages/<package-name>/<path-under-lib>.

This matches the layout that is expected on the web for assets and things, and we want to retain that structure. I don't think it is really something we can easily (or maybe not even feasibly) change.

So, where should these files go?

[natebosch]

So, where should these files go?

We could potentially allow _reading_ source files, but not _writing_ even to-cache assets outside of lib, and all of them would be considered removed for output directories.

[jakemac53]

We could potentially allow _reading_ source files, but not _writing_ even to-cache assets outside of lib, and all of them would be considered removed for output directories.

There is I think a larger problem here of there being additional "source" files which are available in the build but are not available to the built output, I think that gets pretty confusing.

[jakemac53]

One possible proposal to fix the build output issue is to make packages/<package-name> be just symlinks to some new top level directory. That new directory would have a full package structure, so something like dependencies/<package-name>/<entire-package-under-here>.

Technically it would be breaking but unlikely to break anybody in practice.

[natebosch]

There is I think a larger problem here of there being additional "source" files which are available in the build but are not available to the built output, I think that gets pretty confusing.

Why is that? We have many builders which remove source files from appearing in the output.

[natebosch]

symlinks to some new top level directory

Adding a new top level directory in outputs may be breaking. We got away with using packages since there was a history of using that name so we could reasonably expect projects wouldn't be using it for a different reason.

[jakemac53]

Why is that? We have many builders which remove source files from appearing in the output.

Those are explicitly removed, these would be implicitly "deleted" in essence.

[jakemac53]

Adding a new top level directory in outputs may be breaking. We got away with using packages since there was a history of using that name so we could reasonably expect projects wouldn't be using it for a different reason.

Ya, I did mention that. I don't think it actually matters as long as we choose a pretty unique name. We could do a breaking release of build_runner if we want to also.

[natebosch]

I see reading and outputting non-lib files as slightly separate problems. The direct request is for reading, and it's less breaking to allow reading than to also change the output directories. IMO the safer approach is to be more incremental - allow reading first and see how far that gets us. If we get requests for having these source files carried through to the outputs we can revisit the discussion then. WDYT?

[jakemac53]

My concern with only allowing reading is just that I think it will be confusing, it adds another weird edge case to the behavior. I could probably be talked into it though.

[simolus3]

I have a use-case for a private project where I need to be able to write non-lib files too. I don't need them in any output directory though, I have another builder copying those from the cache into the lib/ directory of the root project (as source).

It sounds like having cached non-lib assets in the graph is easier to implement than creating another subdirectory in the output, so maybe we can allow writes (which would have to be build_to: cache) without including them in the output? But I'm happy with an incremental approach too, since we mostly agree on the semantics of reading non-lib files already. So I could work on that first and open another issue to clarify if and how writing could work.

[natebosch]

It sounds like having cached non-lib assets in the graph is easier to implement than creating another subdirectory in the output, so maybe we can allow writes (which would have to be build_to: cache) without including them in the output?

Yes we could easily allow the writes to cache which can be read later in the build but disallow them in the output directory. That would exacerbate the confusion that @jakemac53 is concerned about I think, but I'd still be in favor of that approach over adding an extra directory in the output.

[jakemac53]

Today write to cache outputs do appear in the final output unless explicitly removed, so I would like to avoid adding another edge case here. Limiting it to just reading extra assets seems at least a bit less likely to cause confusion.

[awagege]

Now the files in the bin directory are ready to be scanned, but I put multiple packages into the bin completely and found that build_runner watch would only scan these packages/lib/src/xxx.dart for the first time after launch. When these files changed, the terminal did not react at all. I use mobx_codegen.