Skip to content

PackageUris information from kernel file #36023

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

Closed
zichangg opened this issue Feb 25, 2019 · 8 comments
Closed

PackageUris information from kernel file #36023

zichangg opened this issue Feb 25, 2019 · 8 comments
Assignees
Labels
legacy-area-front-end Legacy: Use area-dart-model instead. P1 A high priority bug; for example, a single project is unusable or has many test failures

Comments

@zichangg
Copy link
Contributor

The URI VM reads from kernel (KernelReaderHelper::SourceTableUriFor) is
file:///usr/local/google/home/zichangguo/dart-sdk/sdk/Junk/breakpoint_test/lib/all_scripts.dart
file:///usr/local/google/home/zichangguo/dart-sdk/sdk/Junk/breakpoint_test/lib/src/func.dart.

However, the URI requested from VS code debugger is:
package:breakpoint_test/src/func.dart
package:breakpoint_test/all_scripts.dart

This causes the mismatch for breakpoint setting. I think it would be helpful for frontend team to provide both package:breakpoint_test/src/func.dart and file:///usr/local/google/home/zichangguo/dart-sdk/sdk/Junk/breakpoint_test/lib/src/func.dart for all the scripts.

This is blocking issue #35859

@zichangg zichangg added the legacy-area-front-end Legacy: Use area-dart-model instead. label Feb 25, 2019
@bkonyi bkonyi added the P1 A high priority bug; for example, a single project is unusable or has many test failures label Feb 27, 2019
@bkonyi
Copy link
Contributor

bkonyi commented Feb 28, 2019

Bumping priority to P1 as not having access to the fully resolved URI is causing quite a few headaches for finding sources for dart:* libraries. For example, sources in dart:* libraries can be found in all sorts of places including:

  • sdk/lib/$LIBRARY_NAME/
  • runtime/bin
  • runtime/lib

This would be fine if all the sources for a library 'dart:x' were in directory sdk/lib/x or one of the runtime directories, since then it would be possible to simply chop the dart:x part of the script path off and then use a substring comparison to find the correct script. However, there's special cases where this approach won't work which will require even more hacks in the VM to deal with.

@bkonyi
Copy link
Contributor

bkonyi commented Mar 1, 2019

@kmillikin, can this work be made a priority?

cc @a-siva

@bkonyi
Copy link
Contributor

bkonyi commented Mar 6, 2019

Friendly ping @kmillikin. This is blocking fixes for multiple issues. Can we prioritize this change?

@jensjoha
Copy link
Contributor

Bumping priority to P1 as not having access to the fully resolved URI is causing quite a few headaches for finding sources for dart:* libraries. For example, sources in dart:* libraries can be found in all sorts of places including:

  • sdk/lib/$LIBRARY_NAME/
  • runtime/bin
  • runtime/lib

This would be fine if all the sources for a library 'dart:x' were in directory sdk/lib/x or one of the runtime directories, since then it would be possible to simply chop the dart:x part of the script path off and then use a substring comparison to find the correct script. However, there's special cases where this approach won't work which will require even more hacks in the VM to deal with.

I don't understand how this is relevant to issue about PackageUris?
Also, you have the resolved URI already (although it is something like org-dartlang-sdk:///sdk/lib/async/future.dart (but that's basically relative to a dir as specified in runtime/vm/BUILD.gn)) - the problem here as I understand it is that for some things (parts) you don't have the the import uri.
Please enlighten me if I'm wrong.

Also the kernel format is currently "semi-locked-down" so I'm not sure we can actually make a change to include the import uri for every source at the moment. @a-siva @alexmarkov may know more about how locked down the format is though.
That being said, I'll try looking at actually gathering the information so the frontend can provide it.

@jensjoha
Copy link
Contributor

Also the kernel format is currently "semi-locked-down" so I'm not sure we can actually make a change to include the import uri for every source at the moment. @a-siva @alexmarkov may know more about how locked down the format is though.
That being said, I'll try looking at actually gathering the information so the frontend can provide it.

Actually I think I can come around that. As I (now) understand a new VM should just be able to read both old and new dill files. I can work with that.

@jensjoha
Copy link
Contributor

Can this issue be closed (#35859, e9e0dae, 58882ff, 9f00d1b)?

@DanTup
Copy link
Collaborator

DanTup commented Mar 21, 2019

I've confirmed #35859 is now fixed, though I'm not sure if the changes made satisfy what @bkonyi needed above, so probably this is a question for him now.

Thanks!

@bkonyi
Copy link
Contributor

bkonyi commented Mar 21, 2019

Let's close this issue. If something else comes up I'll reopen and let you know. Thanks!

@bkonyi bkonyi closed this as completed Mar 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
legacy-area-front-end Legacy: Use area-dart-model instead. P1 A high priority bug; for example, a single project is unusable or has many test failures
Projects
None yet
Development

No branches or pull requests

4 participants