Use workspaces

[jakemac53]

Testing out the new pub workspaces feature, it seems to work!

General stuff I ran into:

• We had test fixture packages with general names that I had to rename, because they were listed as real dev deps in some packages and there can only be one resolution per name. Fixed by just renaming them.
• Initially I got confusing errors about dependency overrides that I didn't understand how to resolve (we previously had dependency overrides to path dependencies in each pubspec).
  • The issue was you aren't allowed to override any package that is a part of the mono repo.
    • My first try to fix it was to move the overrides into the workspace pubspec, but I got the same error.
    • It wasn't obvious to me that I just needed to remove them entirely, they are automatically all path dependencies.
• It looks like mono_repo will need some work to support workspaces Actually I think it is OK to run dart pub get from any dir, so it might be OK

In general, it really wasn't difficult 👍

cc @jonasfj @kevmoo @natebosch @sigurdm

[jakemac53]

I am working on several more fixes related to how workspaces are handled in build_runner as well here.

[natebosch]

What local workflows do we expect to be impacted? Anything outside of changes to the behavior of pub get?

Can we split this up a bit into separate PRs so that the code changes aren't mixed in with so much other noise?

[sigurdm]

It wasn't obvious to me that I just needed to remove them entirely, they are automatically all path dependencies.

Maybe we can improve the help message... It currently just says:

Cannot override workspace packages.
Package `$override` at `${overriddenWorkspacePackage.presentationDir}` is overridden in `${package.pubspecPath}`.

Maybe just suggesting to remove the override would be enough?

[jakemac53]

Maybe just suggesting to remove the override would be enough?

Yeah, I think the key thing I was missing is that there is effectively an implicit (path) override already there for all workspace packages. It made sense once I understood that, it just wasn't clear to me that was there at first. My thought when seeing this message was "but I need them to be path dependencies!".

[jakemac53]

@natebosch note that I had to remove watching of the .dart_tool/package_config.json file... in the workspace case it is not readable, because it lives outside of any package.

For now I just logged a warning that you would have to manually restart if you run a pub get/upgrade, because the package graph might have changed (generally this matters when new deps are added, or old ones removed).

[jonasfj]

I had to remove watching of the .dart_tool/package_config.json file... in the workspace case it is not readable, because it lives outside of any package.

Inside .dart_tool/ there is a file referencing the package_config.json, so finding it should be easy.

nvm, that reference file is in .dart_tool/pub/, consider it private, don't use it 🤣

Just walk up the parent directories until you find .dart_tool/package_config.json -- that's what all other tools do.

[jakemac53]

The problem isn't finding it, the problem is we have a file system abstraction which does not allow file access outside of the current package, or one of its transitive deps. Everything goes through an AssetId which is a package + path.

When running on a sub-package, the workspace packages is not a transitive dep, and so file cannot be referenced.

We also don't allow access to anything outside of lib/ from dependencies which further complicates the issue.

[sigurdm]

Everything goes through an AssetId which is a package + path.

Could you reach the package_config through this abstraction before? That is also outside lib/ of any package.

Maybe we need to make a special AssetId for the package-config? Or provide access to it in some other way.

[jakemac53]

Could you reach the package_config through this abstraction before? That is also outside lib/ of any package.

In the root package, you are allowed to read anything, so yes it was readable. It is only for dependencies that we block anything outside of lib by default, because we only want to expose the public files.

Maybe we need to make a special AssetId for the package-config? Or provide access to it in some other way.

It breaks the model quite a lot, and I don't have the time to try and come up with a fix. It would have to a one-off for sure.

Ultimately, this just means for people using workspaces, they will have to manually shut down and restart build_runner after running pub get. I think it is probably acceptable.

[sigurdm]

Ultimately, this just means for people using workspaces, they will have to manually shut down and restart build_runner after running pub get. I think it is probably acceptable.

I think we can launch like that. Opening an issue for fixing this later though.