-
Notifications
You must be signed in to change notification settings - Fork 798
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
Fix two problems with TypeProviderConfig.ReferencedAssemblies #13721
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…s to ReferencedAssemblies
…s to ReferencedAssemblies
Different paths in printing tests: 2022-08-19T15:56:39.4580448Z - --> Referenced 'C:/Users/dsyme/.nuget/packages/newtonsoft.json/13.0.1/lib/net45/Newtonsoft.Json.dll' (file may be locked by F# Interactive process)
2022-08-19T15:56:39.4582311Z + --> Referenced 'C:/Users/cloudtest/.nuget/packages/newtonsoft.json/13.0.1/lib/net45/Newtonsoft.Json.dll' (file may be locked by F# Interactive process) |
@vzarytovskii This is ready, I've added a test |
Merged
T-Gro
approved these changes
Oct 7, 2022
vzarytovskii
approved these changes
Oct 7, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #13710 - two problems with the F# tooling when instantiating type providers from packages that have nuget dependencies. In particular
TypeProviderConfig.ReferencedAssemblies
was reporting incorrect results after splitting FSharp.Data into multiple components.This is not time-critical because I've worked around this in FSharp.TypeProviders.SDK and FSharp.Data by re-activating an ugly hack that we had in place some time ago when
TypeProviderConfig.ReferencedAssemblies
was also producing incorrect results. However I've prepared the essence of the fixes now while the details are fresh in my mind.The two fixes:
A group of sequential
#r
in a script should be processed together, meaning each should be resolved, and then all added into TcImports at once. This is important when processing the set of assemblies implied by a nuget package reference, because the process of adding the assemblies into TcImports will create any type provider instances, and these in turn will want to see the complete set of available referenced assemblies via TypeProviderConfig.ReferencedAssemblies.Further, the referenced assemblies can grow incrementally as more referenced assemblies are added by further
#r
. These should be reliably reported to the type provider instance, since the type provider may later be used with types from these assemblies.In order to make the first fix accurately I've cleaned up a fair bit of
fsi.fs
(renaming, some indentation, some removal of duplicate code).I've tested manually. For example this one, which was failing and now passes: