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

P2P reference treated as metadata reference when ref assemblies are enabled #2478

Open
jcouv opened this issue Jun 21, 2017 · 13 comments
Open
Assignees
Labels
Feature-Language-Service Populating the Roslyn workspace with references, source files, analyzers, etc Feature-Reference-Assembly Triage-Investigate Reviewed and investigation needed by dev team
Milestone

Comments

@jcouv
Copy link
Member

jcouv commented Jun 21, 2017

Repro:

  1. Set up 2 .NET Core projects: a library and a client.
  2. Turn on reference assemblies in both (<CompileUsingReferenceAssemblies>true</CompileUsingReferenceAssemblies>).
  3. Instantiate a type from the library in the client code and add the project reference.
  4. In the client, use "Go To Definition" on a type from the library. This opens a "[from metadata]" window. But I expect it would open the library source.
  5. In the library, add a public API. Then try to use it from the client, I expect intellisense to pick up this new API. But the current behavior is that a Build operation is required for the new API to become visible to the client.

From discussion with Jason, this is likely a misbehavior of the project system exposing information about project references to the IDE.

Relates to:

FYI @panopticoncentral @davkean @srivatsn for triage. Can we take a look for 15.3?

@jcouv
Copy link
Member Author

jcouv commented Jun 21, 2017

From discussion with @srivatsn, the new project system simply passes to Roslyn the same path that was given to csc.exe, then Roslyn compares that with known outputs from other project. If any of them match, the reference is rewired to a compilation reference.

What is likely happening here is that csc.exe is getting a path to the ref assembly, passing that to Roslyn, but it doesn't match any known outputs.
Sri suggested there may be multiple design solutions, which likely involve project-system, Roslyn or both.

I'll let @davkean @jasonmalinowski comment on possible designs. I can set up a quick meeting if needed.

@jcouv
Copy link
Member Author

jcouv commented Jun 21, 2017

From discussion with Dave and Jason, I think there are three options so far:

  1. have the project-system pass the output path for the ref assembly (bin/ref/a.dll path) instead of current path (bin/a.dll)
  2. have the project-system pass the ref assembly as an additional value to the existing one and have Roslyn handle that extra value
  3. use some generalized mechanism for passing values (bag of properties or other format) across from project-system to Roslyn

Note that there is an msbuild flag that lets a client project ignore the ref assembly produced by a library project. We didn't discuss how that might fit in either of those options (it probably can't fit with option (1)).
I'll follow-up with Sri and Jared to get a sense of what is realistic trade-off, if any, for 15.3. My sense is that options (2) and (3) seem too late in the cycle.

@jcouv
Copy link
Member Author

jcouv commented Jun 21, 2017

@srivatsn @jaredpar What would you think if we put option (1) in place for 15.3, just to unblock our dogfooding of the ref assemblies feature, and then followed-up with a more proper and refined solution for 15.5?

@jcouv
Copy link
Member Author

jcouv commented Jun 22, 2017

From discussion with Sri and Jason, we're leaning towards option (2). That means leave the existing API as-is (to avoid breaking other consumers) and adding a new call to pass a list of output paths.
Sri will get back to us on that option, and we'll also get @Pilchie's thoughts on that.

For the record, we discussed an option (4) where Rolsyn would do some string manipulation (remove "ref/" in the path) before converting path to project reference.

Also, Rainer confirmed that the output path for reference assemblies can be customized in MSBuild (with @(IntermediateRefAssembly)).

@jcouv
Copy link
Member Author

jcouv commented Jun 22, 2017

From discussion with Jason and Kevin about 15.3 scope, I think we're landing on option (4) after all, which is a Roslyn-side workaround. Filed dotnet/roslyn#20412 to track that.
We should continue the discussion about full fix for 15.5. Thanks

@davkean
Copy link
Member

davkean commented Jun 22, 2017

Agreed, I'd like to introduce a relationship where we just pass a property bag over to the language service, and we just have a CPS XAML rule that decides what gets in that property bag. It's the same relationship we have with NuGet and it works really well and is cheap to add additional data.

@jcouv
Copy link
Member Author

jcouv commented Sep 11, 2017

@panopticoncentral @davkean Just a kindly ping to make sure this is still on the radar for 15.5. The implemented workaround for patching P2P references seems to work, but it still has hardcoded logic for the "ref" folder.
Let me know. Thanks

@jcouv
Copy link
Member Author

jcouv commented Sep 18, 2017

@panopticoncentral @davkean I didn't hear back from you.

@jcouv
Copy link
Member Author

jcouv commented Sep 25, 2017

@panopticoncentral @davkean I didn't hear back from you for two weeks.

@Pilchie Pilchie modified the milestones: 15.5, 15.6 Oct 17, 2017
@Pilchie Pilchie modified the milestones: 15.6, 15.7 Dec 20, 2017
@Pilchie Pilchie added Feature-Language-Service Populating the Roslyn workspace with references, source files, analyzers, etc Area-New-Project-System labels Dec 20, 2017
@Pilchie Pilchie assigned davkean and unassigned panopticoncentral Mar 30, 2018
@Pilchie Pilchie modified the milestones: 15.7, 15.8 Mar 30, 2018
@davkean davkean modified the milestones: 15.8, 16.0 May 23, 2018
@davkean
Copy link
Member

davkean commented Jan 22, 2019

@jcouv We've introduced this property bag and I'm ready to start passing reference assembly data across to Roslyn. From above, it's not clear what data you actually want passed. Can you clarify?

@jcouv
Copy link
Member Author

jcouv commented Jan 22, 2019

@davkean You should probably sync up with @jasonmalinowski.
Jason introduced a workaround (there is some hardcoded assumption that ref assemblies live in a folder called ref). He should confirm whether the property bag you introduced meets his needs and the workaround could be removed now.

@jasonmalinowski
Copy link
Member

So all we just need is the path to the reference assembly in the bin path. I'm not sure if that's exposed via a property that would be pulled off of the MSBuild evaluation or something else, but just toss it into the property bag and we'll do it from there.

@jjmew jjmew modified the milestones: 16.0, 16.X Feb 13, 2019
@NinoFloris
Copy link

I would be stoked to have this work soonish ^_^

@jjmew jjmew added this to the Backlog milestone Apr 7, 2020
@jjmew jjmew added Feature-Reference-Assembly Triage-Investigate Reviewed and investigation needed by dev team labels Apr 7, 2020
@drewnoakes drewnoakes removed the Bug label Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature-Language-Service Populating the Roslyn workspace with references, source files, analyzers, etc Feature-Reference-Assembly Triage-Investigate Reviewed and investigation needed by dev team
Projects
None yet
Development

No branches or pull requests

9 participants