-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Set RoslynAssembliesPath #50433
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
base: release/10.0.1xx
Are you sure you want to change the base?
Set RoslynAssembliesPath #50433
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -336,9 +336,15 @@ Copyright (c) .NET Foundation. All rights reserved. | |
</Otherwise> | ||
</Choose> | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we set RoslynAssembliesPath when There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Definitely, that was my intention, thanks. |
||
<!-- NOTE: Compiler toolset packages should set the following properties on their own, hence we set them only for RoslynCompilerType being Core or Framework. --> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📝 I opened a follow-up in roslyn to set RoslynAssembliesPath by the toolset packages as well: dotnet/roslyn#80025 |
||
<PropertyGroup Condition="'$(RoslynCompilerType)' == 'Core' or '$(RoslynCompilerType)' == 'Framework'"> | ||
<!-- In Full Framework MSBuild, RoslynTargetsPath originally points to the directory with Full Framework Roslyn assemblies. --> | ||
<RoslynAssembliesPath Condition="'$(RoslynAssembliesPath)' == '' and '$(MSBuildRuntimeType)' != 'Core'">$(RoslynTargetsPath)</RoslynAssembliesPath> | ||
</PropertyGroup> | ||
<PropertyGroup Condition="'$(RoslynCompilerType)' == 'Core'"> | ||
<RoslynTargetsPath Condition="'$(MSBuildRuntimeType)' == 'Core'">$(MSBuildThisFileDirectory)..\..\..\Roslyn</RoslynTargetsPath> | ||
<RoslynTasksAssembly Condition="'$(MSBuildRuntimeType)' == 'Core'">$(MSBuildThisFileDirectory)..\..\..\Roslyn\Microsoft.Build.Tasks.CodeAnalysis.dll</RoslynTasksAssembly> | ||
<RoslynAssembliesPath Condition="'$(RoslynAssembliesPath)' == '' and '$(MSBuildRuntimeType)' == 'Core'">$(MSBuildThisFileDirectory)..\..\..\Roslyn\bincore</RoslynAssembliesPath> | ||
<RoslynTargetsPath Condition="'$(MSBuildRuntimeType)' != 'Core'">$(MSBuildThisFileDirectory)..\..\..\Roslyn\binfx</RoslynTargetsPath> | ||
<RoslynTasksAssembly Condition="'$(MSBuildRuntimeType)' != 'Core'">$(MSBuildThisFileDirectory)..\..\..\Roslyn\binfx\Microsoft.Build.Tasks.CodeAnalysis.Sdk.dll</RoslynTasksAssembly> | ||
<CSharpCoreTargetsPath>$(MSBuildThisFileDirectory)..\..\..\Roslyn\Microsoft.CSharp.Core.targets</CSharpCoreTargetsPath> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ericstj this value can point to assemblies that target
net472
or .NET core. Will the API compat tool be okay with both of these?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would expect it to point to the correct framework for the MSBuild host process. So if MSBuild is running as netfx, point to netfx assemblies. If MSBuild is running as core, point to core assemblies.
If you think there is a scenario for cross-framework execution (IE: if MSBuild might allow a core-taskhost in netfx build process the future) then we should also have specific properties that always point at core and netfx.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently the property should point to the matching TFM assemblies. I can clarify the docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting. Why did you pick this? My intuition is that
RoslynAssembliesPath
would point to the assemblies that would be used by the build task.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I picked it so that ApiCompat (currently the only consumer of RoslynAssembliesPath) works. And I would expect other consumers behave similarly in this regard - it's not possible to load netcore MS.CA.dll from custom netfx build task; our bridge compiler build task can work only because it starts an executable; if it loaded roslyn as a library it wouldn't work either. And I assume people will use RoslynAssembliesPath for the library DLLs, not the executables.