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

Add support for reference assemblies to ProjInfo #200

Merged
merged 3 commits into from
Feb 6, 2024

Conversation

baronfel
Copy link
Collaborator

@baronfel baronfel commented Feb 6, 2024

Adds support for reading and deferring to TargetRefPath when it exists. This property points to the reference assembly that matches the implementation assembly that exists in TargetPath. There's a new member to help navigate the proper usage. When ref assemblies are present, the FSharpReferencedProject items returned as part of a generated FSharpProjectOptions object will use the ref assembly as the source of truth, not the implementation assembly.

@baronfel baronfel force-pushed the support-reference-assemblies branch 2 times, most recently from b94bb28 to d459c98 Compare February 6, 2024 04:04
@baronfel baronfel requested a review from TheAngryByrd February 6, 2024 04:09
@@ -804,13 +806,14 @@ module ProjectLoader =
"BaseIntermediateOutputPath"
"IntermediateOutputPath"
"TargetPath"
"TargetRefPath"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to grab the property in the first place

Comment on lines +771 to +773
TargetRefPath =
props
|> Seq.tryPick (fun n -> if n.Name = "TargetRefPath" then Some n.Value else None)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

once we have the potential property, we can flow it through

Comment on lines +72 to +76
/// ResolvedTargetPath is the path to the primary reference assembly for this project.
/// For projects that produce ReferenceAssemblies, this is the path to the reference assembly.
/// For other projects, this is the same as TargetPath.
member x.ResolvedTargetPath =
defaultArg x.TargetRefPath x.TargetPath
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and we can make it easier for callers to get the correct field.

@baronfel
Copy link
Collaborator Author

baronfel commented Feb 6, 2024

One question I had @TheAngryByrd - does the Adaptive LSP server use this library? or does it have a parallel mechanism? if so, I can replicate this in the FSAC codebase if you can point me in the right direction.

@safesparrow
Copy link
Contributor

Thanks for looking into this @baronfel .

I wonder if we there is an existing MSBuild target that could be reused here to perform the standard logic - after all, MSBuild has to do the same logic to come up with compiler args.

Unless TargetPath/TargetRefPath is as high level as we can get.

@TheAngryByrd
Copy link
Member

One question I had @TheAngryByrd - does the Adaptive LSP server use this library? or does it have a parallel mechanism? if so, I can replicate this in the FSAC codebase if you can point me in the right direction.

Yeah it uses ProjInfo, ProjInfo.FCS, and ProjInfo.Sln. So this should flow through. I did turn off RefAssembly stuff that we'll probably need to turn on.

It doesn't use ProjectSystem because it needed to use Adaptive's Filewatcher mechanisms.

@baronfel baronfel force-pushed the support-reference-assemblies branch from d459c98 to f43f28a Compare February 6, 2024 14:32
@baronfel
Copy link
Collaborator Author

baronfel commented Feb 6, 2024

Thanks for looking into this @baronfel .

I wonder if we there is an existing MSBuild target that could be reused here to perform the standard logic - after all, MSBuild has to do the same logic to come up with compiler args.

Unless TargetPath/TargetRefPath is as high level as we can get.

Technically we could call the GetTargetPath target and read the Return items from that target, but as you can see from this image a reference assembly doesn't change the TargetPath, it just adds a metadata with the ReferenceAssembly if there is one. This is functionally equivalent to what we're doing here, so I think it's ok for us to stick with what I've got.

image

@baronfel baronfel merged commit 2a44d6b into main Feb 6, 2024
9 checks passed
@baronfel baronfel deleted the support-reference-assemblies branch February 6, 2024 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants