dart-lang / pub

The pub command line tool
https://dart.dev/tools/pub/cmd
BSD 3-Clause "New" or "Revised" License
1.04k stars 229 forks source link

Add a way to find the `package_config.json` file for a given `pubspec.yaml` file #4218

Open kenzieschmoll opened 7 months ago

kenzieschmoll commented 7 months ago

With upcoming monorepo support, we can no longer assume that the package_config.json file lives in the same package root as the pubspec.yaml file:

foo/
  .dart_tool/
    package_config.json
  pubspec.yaml

In a monorepo, the package_config.json file may live further up in the directory structure.

For some development tools (DevTools extensions, other features powered by the Dart Tooling Daemon) we need to know both where the pubspec.yaml files are in an IDE workspace and where the pub solve for each pubspec.yaml file lives. Due to some constraints with file access, we can't walk up the tree until we find the first package_config.json file, as this file may not even be in the user's IDE workspace.

Ideally, we should have an API or some shared package that we can use to find the expected location of the package_config.json for a given pubspec.yaml file.

CC @jonasfj @sigurdm @bkonyi @CoderDake

sigurdm commented 7 months ago

Good point.

The rule is:

Not sure where we best place to share this logic. Perhaps https://pub.dev/packages/pubspec_parse... or https://pub.dev/packages/package_config.

Alternatively we need a way to ensure resolution of current location is resolved and has an up-to-date package_config.json. That is exposed by pub into the sdk, but we could potentially expose it as a command.

jonasfj commented 7 months ago

I might be missing something. I'm not sure why other tools need to read pubspec.yaml, and much less why they would need to read about workspace: section.

If you are a Dart tool

An outline of what a Dart tool can do, at the risk of being slightly off topic :see_no_evil:

If you are a Dart tool. You can:

As a Dart tool you cannot:

This is just not available in package_config.json, but I think we're increasingly leaning in the direction of convincing people that it should be.

Ability to inspect the dependency-graph might be useful for something like extension_discovery. Though it kind of depends how we'd want it to behave.

Dart tooling in workspace-world

If we imagine a directory layout as follows:

/home/foo/projects/my-mono-repo/
  pubspec.yaml
  pubspec.lock
  .dart_tool/
    package_config.json
  pkg_a/
    pubspec.yaml
    lib/...
  pkg_b/
    pubspec.yaml
    lib/...
/home/foo/projects/normal_project/
  pubspec.yaml
  pubspec.lock
  .dart_tool/
    package_config.json
  lib/
    foo.dart
    src/
      bar.dart

Due to some constraints with file access, we can't walk up the tree until we find the first package_config.json file, as this file may not even be in the user's IDE workspace.

Do I understand it correctly that you concern is that:

Is that different from what happens today, if the user opens /home/foo/projects/normal_project/src/ in the IDE?

It's possible that I don't understand the problem here.

sigurdm commented 7 months ago

I think I agree with Jonas' comment. But I'd like you to expand on your use case.

We might not really understand what you are trying to do.

kenzieschmoll commented 7 months ago

I might be missing something. I'm not sure why other tools need to read pubspec.yaml, and much less why they would need to read about workspace: section.

Some additional pieces of context:

As a Dart tool you cannot:

  • Distinguish between transitive, dev- or direct-dependencies.
  • Inspect the dependency-graph.

This is just not available in package_config.json, but I think we're increasingly leaning in the direction of convincing people that it should be.

Unrelated to this issue, but I am very much in favor of this too 😄 this would allow us to distinguish between DevTools extensions provided by direct and transitive dependencies. We likely want to ignore extensions from transitive deps by default.

Due to some constraints with file access, we can't walk up the tree until we find the first package_config.json file, as this file may not even be in the user's IDE workspace.

Do I understand it correctly that you concern is that:

  • The IDE may not able to find package_config.json if the user only opens:

    • /home/foo/projects/my-mono-repo/pkg_a/
    • /home/foo/projects/my-mono-repo/pkg_b/
    • /home/foo/projects/my-mono-repo/pkg_a/lib/
  • But that IDE is only able to find package_config.json if the user opens:

    • /home/foo/projects/
    • /home/foo/projects/my-mono-repo/

Yes this is correct. Imagine a user opens /home/foo/projects/my-mono-repo/pkg_a/ in their IDE. One of pkg_a's dependencies provides a DevTools extension. To find available DevTools extensions for a project, we need to pass the path to the package_config.json file to the findExtensions method from package:extension_discovery.

To do this, we look for the package root of the connected application. This logic does the following for a running app: 1) First, we try to use the Dart Tooling Daemon to detect the directory that contains the .dart_tool directory. We start from the root URI of the main isolate, and start walking up the tree until we find the .dart_tool directory.

Once we have this "root", we build the URI to the package_config.json file by assuming that it lives at <package_root>/.dart_tool/package_config.json. This assumption is incorrect for pkg_a.

sigurdm commented 6 months ago

It seems to me that "Dart Tooling Daemon" is not supposed to read files outside the IDE workspace root. But with pub workspaces .dart_tool/package_config.json is going to be placed at the root of the pub-workspace. So if the IDE workspace does not include the root of the pub workspace we need to add special handling to Dart Tooling Daemon for finding and returning the package config.

Should we transfer this issue to the sdk - as this is a "Dart Tooling Daemon" feature request?

kenzieschmoll commented 6 months ago

.dart_tool/package_config.json is going to be placed at the root of the pub-workspace

Can you elaborate on what you mean by "pub-workspace"? I don't think DTD should have special logic to "find the package config", as this would likely require DTD to walk the directory structure beyond its restrictions. These are not technical restrictions of DTD, but intentionally placed security restrictions on DTD.

What does the "workspace" section in a pubspec.yaml file look like? Does this contain the path to the .dart_tool/package_config.json file we are looking for?

sigurdm commented 6 months ago

Can you elaborate on what you mean by "pub-workspace"?

I'm working on flutter.dev/go/pub-workspace

With this proposal the .dart_tool/package_config.json can be placed above the root level of a package if it is a member of a workspace. If the IDE is opened with that package only, then we need some kind of escape hatch to reach the package config (or DTD will allow access to the entire workspace, not only the "focused" package.

kenzieschmoll commented 6 months ago

How would pub find the .dart_tool/package_config.json location if it is not in the IDE workspace scope (if it is above any directory opened in the IDE)? What sounds like the best solution to me would be to share the logic that pub uses to do this, like you proposed in your original comment:

The rule is:

  • Walk up the tree
  • Until you find a pubspec.yaml with no workspace: section.
  • Adjacent to that is the expected location of package_config.json.

Not sure where we best place to share this logic. Perhaps https://pub.dev/packages/pubspec_parse... or https://pub.dev/packages/package_config.

Could pub expose a command dart pub find-resolution --path=path/to/my/package/root that would return the location of the package resolution for the specified file path?

sigurdm commented 6 months ago

How would pub find the .dart_tool/package_config.json location if it is not in the IDE workspace scope

Pub (and other dart processes) has access to the whole file system, not only from the IDE root.

kenzieschmoll commented 6 months ago

Pub (and other dart processes) has access to the whole file system, not only from the IDE root.

Great! What do you think then about exposing a command that the Dart Tooling Daemon, which is limited to the scope of the IDE workspace, could use?

kenzieschmoll commented 6 months ago

Or another idea would be to modify or add a new API to package:extension_discovery to findExtensions when given a path to a pubspec.yaml file instead of a package_config.json file, and then package:extension_discovery could re-use the pub logic to find a package_config.json file for a given pubspec.

sigurdm commented 6 months ago

Great! What do you think then about exposing a command that the Dart Tooling Daemon, which is limited to the scope of the IDE workspace, could use?

I don't think the tooling daemon is limited in what it can access, I rather think it itself imposes limits on what it allows its client to access. This restriction is imposed here: https://github.com/dart-lang/sdk/blob/ba8306735d7efa8aabfbf4d8f9a618bd00bfbcbd/pkg/dtd_impl/lib/src/service/file_system_service.dart#L204

And I believe we could special-case that limit somehow to allow accessing the .dart_tool/package_config.json (or perhaps allow accessing the entire workspace instead of only the IDE part).

kenzieschmoll commented 6 months ago

I don't think the tooling daemon is limited in what it can access, I rather think it itself imposes limits on what it allows its client to access.

This is correct. Because third party tools can connect to DTD, we need to limit what parts of the file system can be read from / written to.

And I believe we could special-case that limit somehow to allow accessing the .dart_tool/package_config.json

This may be acceptable. @bkonyi and @CoderDake what are your thoughts?

I do still think extending extension_discovery to handle this case (comment) may be the best option, since detecting extensions for a pub workspace will apply to all types of extensions, not just devtools (for example, we find extensions for vs_code to support promoting VS Code extensions for 3P packages).

(or perhaps allow accessing the entire workspace instead of only the IDE part).

This would concern me that we could end up walking all the way up the tree if the .dart_tool/package_config.json file for the workspace cannot be found, although this may be an unlikely edge case. But it also would be surprising if a 3P tool can read and write to files that the user has not opened in their IDE. When a user opens a project in an IDE, they are granting access to that part of their file system to the IDE and all associated tooling. It may be a violation of expectations to grant access to other parts of the file system that the user has not explicitly opened.