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

[dtd] It is difficult to develop features that use the getIDEWorkspaceRoots API #55002

Open
kenzieschmoll opened this issue Feb 23, 2024 · 8 comments
Labels
area-dart-cli Use area-dart-cli for issues related to the 'dart' command like tool. dart-cli-tooling-daemon Issues related to the 'dart tooling-daemon' tool pkg-dtd For issues related to the Dart Tooling Daemon (package:dtd or pkg/dtd_impl within the Dart SDK)

Comments

@kenzieschmoll
Copy link
Contributor

kenzieschmoll commented Feb 23, 2024

When developing tools that use DTD, it is useful to start a local instance of DTD to interact with. To do this, you can run dart tooling-daemon --unrestricted from the CLI. This command starts DTD and removes restrictions that make local development difficult:

  • checks for the file system APIs that ensure file read / writes are within a user's IDE workspace.
  • secret requirements to call the setIDEWorkspaceRoots

One API that is broken in unrestricted mode is getIDEWorkspaceRoots. The IDE that starts DTD usually sets these via setIDEWorkspaceRoots, but since in unrestricted mode, DTD is started from the CLI, these workspace roots will not be set.

This makes it difficult to develop features that use the getIDEWorkspaceRoots DTD API.

Proposal
Add a way to set workspace roots in unrestricted mode.

  1. One way to do this would be to use an environment variable dtd_workspace_roots that takes a comma separated list of workspace roots. DTD could then read this environment variable when in unrestricted mode to see if workspace roots have been set.
    dart tooling-daemon --unrestricted --dart-define=dtd_workspace_roots=/path/to/foo,/path/to/bar

  2. Another option would be to add another flag to the tooling-daemon command. This flag should be a no-op when not in unrestricted mode.
    dart tooling-daemon --unrestricted --workspaceRoots=/path/to/foo,/path/to/bar

  3. A less than ideal solution would be that every DTD client can implement their own affordance to call setIDEWorkspaceRoots for local development. This isn't preferred because it requires implementing the same thing multiple times for several different clients (DevTools, DevTools Extensions, maybe Analysis Server, Flutter tools, etc. in the future)

CC @bkonyi @CoderDake for discussion.

@CoderDake
Copy link

the --workspaceRoots one feels the cleanest to me.

Unfortunately it is awkward that the dev even needs to be responsible for this.

Would it be possible for us to make some sort of seperate repo where we could host some "simulated env tools"

We could have a dart file that does execs for:

  • DTD then gets the uri
  • The simulated env startup, we could pass the url on the cmd line using dart-define

We could either pass the directories there or have the user set them in the UI.

I'm not sure if there is a better way to pass them that binary though, so I'm open to some smarter ideas :)

@kenzieschmoll
Copy link
Contributor Author

the --workspaceRoots one feels the cleanest to me.

Agreed this might be better.

Unfortunately it is awkward that the dev even needs to be responsible for this.

I'm not sure that we have a way around this. There is no way for DTD to know what the project directories are unless they are set somehow, and if the IDE isn't setting them, the user will have to manually specify these since there isn't a way to infer them.

We could either pass the directories there or have the user set them in the UI.

Even with a solution like this, the user is still having to set the directories manually. At least with a command line flag, the user could set an alias or use terminal history to run the right command. With requiring the user to set the directories manually in a GUI, we'd have to implement some sort of caching to make this a nice developer experience.

@lrhn lrhn added area-pkg Used for miscellaneous pkg/ packages not associated with specific area- teams. pkg-dartdev labels Feb 25, 2024
@kenzieschmoll
Copy link
Contributor Author

Another idea: if we make the DTD uri accessible from the IDE, a workflow could look like this:

I am a developer building a DevTools feature (either a first party devtools feature or a feature in a DevTools extension).

  1. I open an IDE with my test app. I may or may not run my app depending on whether my feature requires a running app.
  2. The IDE starts DTD and provides a way for me to access this URI. Example: a VS code action similar to what we provide for copying the VM service uri:
    Screenshot 2024-02-26 at 9 37 19 AM
  3. I copy this DTD uri and connect my debug client to it.

Pros:

  • The developer doesn't have to manually start DTD with special flags.
  • The developer is testing against a DTD instance that is started exactly how one will be started for an end-user using the tool under development

Cons:

  • Developers will be forced to open an IDE with a test project and app in order to develop a feature that uses DTD.
    • This would no longer be an issue once [dds] Start DTD from the DevTools server #54937 is resolved. If we move to a model that we always have a DTD instance when we have a running Dart or Flutter app (started from DevTools server in the CLI case and by the IDE in the IDE start case), then developers can use the production instance of DTD without having to start one for local development.

@DanTup
Copy link
Collaborator

DanTup commented Feb 26, 2024

--workspaceRoots=/path/to/foo,/path/to/bar

Commas are valid in paths, so although unlikely, it's probably better to allow the flag multiple times? --workspaceRoots /path/to/foo --workspaceRoots /path/to/bar

The IDE starts DTD and provides a way for me to access this URI. Example: a VS code action similar to what we provide for copying the VM service uri:

This sounds handy to me. I considered this when adding DTD to the language status area, as we have a "Copy URI" option next to the number of debug sessions there too.

FWIW in the meantime for your own use, you could enable the DTD log in your VS Code settings:

"dart.toolingDaemonLogFile": "C:\\Dev\\Logs\\tooling_daemon.txt",

This will record both the traffic over stdin/stdout and over Dart-Code's connection to DTD:

Mon Feb 26 2024 [15:16:47 GMT+0000 (Greenwich Mean Time)] Log file started
[3:16:47 PM] [DartToolingDaemon] [Info] Spawning C:\Dev\Tools\Dart\Nightly\bin\dart.exe with args ["tooling-daemon","--machine"]
[3:16:47 PM] [DartToolingDaemon] [Info] ..  with {"toolEnv":{"FLUTTER_HOST":"VSCode","PUB_ENVIRONMENT":"vscode.dart-code"}}
[3:16:47 PM] [DartToolingDaemon] [Info]     PID: 4420
[3:16:47 PM] [DartToolingDaemon] [Info] <== {"tooling_daemon_details":{"uri":"ws://127.0.0.1:60401/","trusted_client_secret":"nnA0XC4MopzWyQEz"}}
[3:16:47 PM] [DartToolingDaemon] [Info] Connecting to DTD at ws://127.0.0.1:60401/...
[3:16:47 PM] [DartToolingDaemon] [Info] Connected to DTD
[3:16:47 PM] [DartToolingDaemon] [Info] ==> {"id":"1","jsonrpc":"2.0","method":"FileSystem.setIDEWorkspaceRoots","params":{"secret":"nnA0XC4MopzWyQEz","roots":["file:///c%3A/Dev/Google/dart-sdk/sdk/pkg/analysis_server","file:///c%3A/Dev/Google/dart-sdk/sdk/pkg/analyzer","file:///c%3A/Dev/Google/dart-sdk/sdk/pkg/analyzer_plugin","file:///c%3A/Dev/Google/dart-sdk/sdk/pkg/analysis_server_client","file:///c%3A/Dev/Google/dart-sdk/sdk/pkg/analyzer_utilities","file:///c%3A/Dev/Google/dart-sdk/sdk/pkg/dartdev","file:///c%3A/Dev/Test%20Projects/completion_counts","file:///c%3A/Dev/Google/dart-sdk/sdk/third_party/pkg/language_server_protocol","file:///c%3A/Dev/Google/dart-sdk/sdk/pkg/linter"]}}
[3:16:47 PM] [DartToolingDaemon] [Info] <== {"jsonrpc":"2.0","result":{"type":"Success"},"id":"1"}
Mon Feb 26 2024 [17:53:25 GMT+0000 (Greenwich Mean Time)] Log file ended

@CoderDake
Copy link

I really do like the idea of DTD just always being present when debugging an app. It feels a lot more intuitive to just have it available just the the vm_service uri is available.

@kenzieschmoll is there anything stopping us from quickly adding the code for DTD to spin up in the Devtools Server if one isn't provided?

@bkonyi bkonyi added area-dart-cli Use area-dart-cli for issues related to the 'dart' command like tool. dart-cli-tooling-daemon Issues related to the 'dart tooling-daemon' tool and removed area-pkg Used for miscellaneous pkg/ packages not associated with specific area- teams. pkg-dartdev labels Feb 26, 2024
@bkonyi
Copy link
Contributor

bkonyi commented Feb 26, 2024

A multi-option named --workspace-root sounds like a reasonable solution.

Another solution could be just to disable the workspace root checks when --unrestricted is provided so the developer would have full access to their machine's file system. Obviously this isn't a secure configuration and developers would need to eventually test with a DTD instance with the workspace roots set to ensure their code works properly, but it might be acceptable for a development workflow.

@DanTup
Copy link
Collaborator

DanTup commented Feb 26, 2024

Another solution could be just to disable the workspace root checks when --unrestricted is provided so the developer would have full access to their machine's file system.

My understanding from the above was that this already happens, but the problem is that getIDEWorkspaceRoots() return nothing?

--workspace-root could also possibly default to . (the current directory) when run with --unrestricted? (this gives getIDEWorkspaceRoots a reasonable value to return, and might even be a way to keep the file system check rather than code a bypass?)

@bkonyi
Copy link
Contributor

bkonyi commented Feb 26, 2024

My understanding from the above was that this already happens, but the problem is that getIDEWorkspaceRoots() return nothing?

--workspace-root could also possibly default to . (the current directory) when run with --unrestricted? (this gives getIDEWorkspaceRoots a reasonable value to return, and might even be a way to keep the file system check rather than code a bypass?)

Ah, right, that makes sense since all these tools will be relying on paths relative to the workspace roots provided by that API... Defaulting to . would work, assuming we resolve the path before we actually give it to DTD as a root.

@kenzieschmoll kenzieschmoll added the pkg-dtd For issues related to the Dart Tooling Daemon (package:dtd or pkg/dtd_impl within the Dart SDK) label Jul 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-dart-cli Use area-dart-cli for issues related to the 'dart' command like tool. dart-cli-tooling-daemon Issues related to the 'dart tooling-daemon' tool pkg-dtd For issues related to the Dart Tooling Daemon (package:dtd or pkg/dtd_impl within the Dart SDK)
Projects
None yet
Development

No branches or pull requests

5 participants