Correctly handle <ProjectReferences> that don't produce references #1956
Conversation
|
|
||
| var originalItemSpec = referencePathItem.GetMetadataValue(MetadataNames.OriginalItemSpec); | ||
| if (originalItemSpec.EndsWith(".csproj", StringComparison.OrdinalIgnoreCase)) | ||
| var sourceProjectFile = referencePathItem.GetMetadataValue(MetadataNames.MSBuildSourceProjectFile); |
There was a problem hiding this comment.
Still confirming with the MSBuild team precise which property should be used here.
There was a problem hiding this comment.
-
OriginalItemSpec: available on all RAR outputs. TheIncludevalue of the RAR input that caused that output. (This is actually automatically set in the engine whenever populating an item from another item) -
ProjectReferenceOriginalItemSpec: available on RAR outputs that were resolved from@(ProjectReference)items. TheOriginalItemSpecvalue of theReferencethatResolveProjectReferencesproduced from theProjectReferenceitem--so theProjectReference'sInclude. -
`OriginalProjectReferenceItemSpec: ¯\(°_o)/¯
Copy OriginalItemSpec to OriginalProjectReferenceItemSpec, so that when ResolveAssemblyReferences
takes these items and resolves them to ReferencePath, we can still recover the real OriginalItemSpec
for the unresolved reference items.Doesn't appear to be used anywhere. My advice: avoid.
-
MSBuildSourceProjectFile: available on all items that are outputs of the<MSBuildtask. Points to the project that created the item. For RAR outputs, inherited from the Reference generated by<MSBuildinResolveProjectReferences
So the thing that most matches your use case is ProjectReferenceOriginalItemSpec. It has a weakness though: it's a relative path, not fully resolved. As long as you deal with that I'd go with it. MSBuildSourceProjectFile should always be an absolute path which is nice but it could get confused with custom reference-resolution schemes.
There was a problem hiding this comment.
@rainersigwald What we noticed is that the fully qualified MSBuildSourceProjectFile prepends the /private/ folder name on MacOS for the shadowcopied unit tests. This causes them to be different that the paths we qualify with Path.GetFullPath. I'll take your advice and update to use ProjectReferenceOriginalItemSpec.
| <OutputItemType>Analyzer</OutputItemType> | ||
| <ReferenceOutputAssembly>false</ReferenceOutputAssembly> |
There was a problem hiding this comment.
This is a pattern we're seeing is more common with people writing source generators, since you often want to have the generator in your same solution as the project itself. It's not the best supported because reloading the analyzer is often difficult if it changes on disk.
jasonmalinowski
force-pushed
the
fix-handling-of-projectreferences
branch
from
c2da705
to
198ace8
Compare
|
@jasonmalinowski If for any reason you need your test project to be built prior to your new unit test running then you can add the folder name to this list |
|
@JoeRobich Shouldn't be necessary, and honestly if it was that'd be a bug in it's own right. |
src/OmniSharp.MSBuild/ProjectFile/ProjectFileInfo.ProjectData.cs
Outdated
Show resolved
Hide resolved
jasonmalinowski
force-pushed
the
fix-handling-of-projectreferences
branch
2 times, most recently
from
e8ee7fd
to
c5caf6d
Compare
|
sorry! it needs another rebase/merge due to a merge of #1964 |
<ProjectReference> elements can have metadata that specify what type of item to create, or that they shouldn't create items at all. The existing code that was processing references wasn't checking for this metadata, so you could end up with project references between projects that shouldn't be there. The approach to take here is to never look at the <ProjectReference> items directly, rather only look at the ReferencePath items that are produced, and if we see metadata that indicates it came from a project, convert it to a project reference at that point.
jasonmalinowski
force-pushed
the
fix-handling-of-projectreferences
branch
from
c5caf6d
to
1c487ea
Compare
|
@filipw Rebased. I swear you're the only repo I know that actually has that 'must be up to date' thing checked. 😄 |
thank you!
|
also, I learnt something here and I can now drop |
|
@filipw And note that generally, in Visual Studio for Windows we don't look at that property, and instead just wire up things based on output paths. So if project A is producing a DLL to an output path and project B is consuming that same (exact, full) path, we wire them up at that point. That may be something that needs to happen here longer term; this fix at least will help in unblocking us investigating other issues. |
|
Thanks! |
elements can have metadata that specify what type of item to create, or that they shouldn't create items at all. The existing code that was processing references wasn't checking for this metadata, so you could end up with project references between projects that shouldn't be there.
The approach to take here is to never look at the items directly, rather only look at the ReferencePath items that are produced, and if we see metadata that indicates it came from a project, convert it to a project reference at that point.