-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Include ref projects in SLN if the library is packable #56494
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
Conversation
… part of NETCoreApp and ship a package.
Ref projects are needed if for packages since they will have TargetFrameworks that aren't covered by an up-front build of libs subset.
|
Tagging subscribers to this area: @Anipik, @safern, @ViktorHofer Issue DetailsWithout this any SLN that contained one of these libraries would fail to build, as it's src project would be missing the contract assembly for non-NetCoreAppCurrent builds, or the ref would be missing from some project up stack.
|
| <!-- Include CompilerServices.Unsafe as it's a transitive test dependency. --> | ||
| <IncludeInSolutionFile Condition="'$(IsNETCoreAppRef)' == 'true' and '$(MSBuildProjectName)' == 'System.Runtime.CompilerServices.Unsafe'">true</IncludeInSolutionFile> | ||
| <!-- Don't include shared framework projects except the leaf's project and OOBs. --> | ||
| <IncludeInSolutionFile Condition="'$(IsNETCoreAppRef)' == 'true' and '$(MSBuildProjectName)' != '$(SlnGenMainProject)' and '$(IsNETCoreAppPackage)' != 'true'">false</IncludeInSolutionFile> |
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.
After doing this I realize that another condition that might work is '$(TargetFrameworks)' != '$(NetCoreAppCurrent)' since this will also catch references that didn't build in the up-front build pass. Let me know what makes more sense to use.
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.
Yes, I would prefer that over the static list (which we likely need anyway for another change but probably not here) as the existence of another TargetFramework than $(NetCoreAppCurrent)* should indicate that the reference project should be present in the solution file.
|
I reverted every sln that only had guid changes, so this should only be the ones with actual changes. @jeffkl would it be possible to get stable guids here? Are we running it wrong that it churns guids every time? |
Answering my own question. Looks like today the answer is no: https://github.com/microsoft/slngen/blob/04b9e981a2651f3b4aa7cbea9adeb855db927732/src/Microsoft.VisualStudio.SlnGen/SlnFolder.cs#L29 But there is precedent for preserving Guids elsewhere: Seems like the same could be done for folders. Opened microsoft/slngen#238 |
|
I noticed that many solution file format updates were due to two unnecessary P2Ps in TestUtilities.csproj. I submitted #57483 to replace this PR and used the newer version of slngen. Hope that's ok. |
Without this any SLN that contained one of these libraries would fail to build, as it's src project would be missing the contract assembly for non-NetCoreAppCurrent builds, or the ref would be missing from some project up stack.