Skip to content
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

getExecutableForCommand requires reading pubspec.yaml #4222

Open
sigurdm opened this issue Apr 15, 2024 · 0 comments
Open

getExecutableForCommand requires reading pubspec.yaml #4222

sigurdm opened this issue Apr 15, 2024 · 0 comments
Labels
type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@sigurdm
Copy link
Contributor

sigurdm commented Apr 15, 2024

Discussion

Currently dart run package:command will only run commands if package is a direct (dev or non-dev) dependency of the current root package.

This convention makes sense. Only the packages you directly depend on are the ones you should rely on running tools from.

But deciding if a package is in dependencies requires parsing pubspec.yaml of the root package. As this is in the hot path of dart run we would like to only rely on information from .dart_tool/package_config.json. (json is faster to parse than yaml, it will be hot on the drive when passed to the VM that also needs the file).

One solution would be to add the names of dependencies of each package to the package listing in .dart_tool/package_config.json.

One concern with this, is that the size of .dart_tool/package_config.json potentially will be O(n^2) in the number of packages in the resolution.

This is information is also strictly more than we need. We really only need to know if a dependency is direct dependency of the root package or not. We could instead add a bit (is-direct-dependency-of-root) to each package listed in the file.

This is however not enough when we introduce workspaces to pub. Here a number of root packages share a .dart_tool/package_config.json, and when we do dart run inside a package a in a workspace, we only want access to the packages that are direct dependencies of a, not of every root in the workspace.

That leads to:

Proposal

Add the list of direct dependencies only to the root packages. dart run in a directory x would then need to find out which package is associated with x and could thus see if the package is one of its dependencies.

This would not have the O(n^2) behavior (unless o(n) of the packages are root packages...)

Example json:

{
  "configVersion": 2,
  "packages": [
    {
      "name": "dep",
      "rootUri": "file:///usr/local/google/home/sigurdm/.pub-cache/hosted/pub.dev/dep-1.0.0",
      "packageUri": "lib/",
      "languageVersion": "3.0" // Not a root package, no dependencies listing
    },
    {
      "name": "dev_dep",
      "rootUri": "file:///usr/local/google/home/sigurdm/.pub-cache/hosted/pub.dev/dev_dep-1.0.0",
      "packageUri": "lib/",
      "languageVersion": "3.0"  // Not a root package, no dependencies listing
    },
    {
      "name": "myapp",
      "rootUri": "../",
      "packageUri": "lib/",
      "languageVersion": "3.7"
      "dependencies": ["dep", "b"] // Depends on external dep, and workspace member b.
    },
    {
      "name": "b",
      "rootUri": "../pkgs/b",
      "packageUri": "lib/",
      "languageVersion": "3.7"
      "dependencies": ["dev_dep"] // Depends on external dev_dep
    },
    {
      "name": "a",
      "rootUri": "../pkgs/a",
      "packageUri": "lib/",
      "languageVersion": "3.7"
      "dependencies": [] // No dependencies
    }
  ],
  "generated": "2024-04-15T13:37:03.000003Z",
  "generator": "pub",
  "generatorVersion": "3.7.0",
  "pubCache": "file:///usr/local/google/home/sigurdm/.pub-cache"
}

If there are no dependencies associated with x we could either

  • fail (allow no packages)
  • allow all packages

Allowing all packages would be backwards compatible, but I'm not sure that is needed, as we rewrite package_config.json when the dart sdk is updated.

Alternatives

We could also just let dart run package:command run commands from any package. I think that would rarely give any problems, but might be slightly less disciplined.

@sigurdm sigurdm added the type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) label May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

1 participant