Skip to content

Various SDK build fixes within VS #49748

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

Merged
merged 7 commits into from
Jul 18, 2025
Merged

Conversation

MiYanni
Copy link
Member

@MiYanni MiYanni commented Jul 12, 2025

Builds on: #49746

Summary

After trying to do "Rebuild All" within VS for the repo, I ran into a bunch of problems. This should address some of them.

Changes

  • Changed manifest-packages.proj to manifest-packages.csproj for VS compatibility
  • Added NU5039 to NoWarn since we get this warning for all the packages we make
  • Added -ExecutionPolicy Bypass since you cannot build the redist project if your PowerShell is set to Restricted ExecutionPolicy (default on new Windows installs)
  • Added GetTargetPath target to sdk-tasks.csproj as it was complaining about it when trying to rebuild
    • I was trying to find the correct fix from here but it didn't work
  • Added script to copy the downloaded runtime into Program Files

… NoWarn for NU5039. Added -ExecutionPolicy Bypass for calls to PowerShell. Added blank GetTargetPath to sdk-tasks.csproj. Other minor cleanup.
@MiYanni MiYanni marked this pull request as ready for review July 16, 2025 00:10
@Copilot Copilot AI review requested due to automatic review settings July 16, 2025 00:10
@MiYanni MiYanni requested a review from a team as a code owner July 16, 2025 00:10
@MiYanni MiYanni requested a review from a team July 16, 2025 00:10
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Addresses several build and tooling issues when performing a full rebuild in Visual Studio by renaming project files for VS compatibility, suppressing new NuGet warnings, ensuring PowerShell scripts run under restrictive execution policies, and adding a helper script for runtime copying.

  • Rename manifest-packages.projmanifest-packages.csproj and update all references.
  • Suppress additional NuGet warnings (NU5039) and add -ExecutionPolicy Bypass to MSI-generation scripts.
  • Introduce an empty GetTargetPath MSBuild target and add a new copy-runtime.ps1 script to copy built runtimes into Program Files.

Reviewed Changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
test/Microsoft.DotNet.PackageValidation.Tests/*.csproj Added Microsoft.CodeAnalysis.CSharp package reference
test/Directory.Build.props Combined and clarified NoWarn entries for NU5125/NU5123
src/Workloads/VSInsertion/workloads.csproj Updated <ProjectReference> to point at .csproj file
src/Workloads/Manifests/Directory.Build.props Added NU5039 to NoWarn
src/Tasks/sdk-tasks/sdk-tasks.csproj Added stub <Target Name="GetTargetPath" />
src/Layout/redist/targets/GenerateMSIs.targets Added -ExecutionPolicy Bypass to PowerShell <Exec> commands
src/Layout/redist/targets/BundledManifests.targets Updated MSBuild project path to .csproj
src/Layout/redist/redist.csproj Updated <ProjectReference> to .csproj
sdk.slnx Updated solution entry for manifest-packages.csproj
eng/copy-runtime.ps1 New admin-run PowerShell script to copy built runtime into Program Files
Directory.Build.props Added NU5039 to root NoWarn
.vsconfig New Visual Studio configuration file

</ItemGroup>

<ItemGroup>
<ProjectReference Include="$(RepoRoot)src\Compatibility\ApiCompat\Microsoft.DotNet.PackageValidation\Microsoft.DotNet.PackageValidation.csproj" />
<ProjectReference Include="..\Microsoft.NET.TestFramework\Microsoft.NET.TestFramework.csproj" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp" />
Copy link
Member Author

Choose a reason for hiding this comment

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

I just moved this into the ItemGroup that is for PackageReferences. Not sure why it was here to begin with.

@ViktorHofer
Copy link
Member

manifest-packages shouldn't be a csproj as it doesn't invoke the compiler. We have many proj files in our repos so that should work. Otherwise this is a VS / project-system bug.

@MiYanni
Copy link
Member Author

MiYanni commented Jul 17, 2025

manifest-packages shouldn't be a csproj as it doesn't invoke the compiler. We have many proj files in our repos so that should work. Otherwise this is a VS / project-system bug.

That's not how it works with solutions in Visual Studio. You can't have .proj files as part of the solution. All the .proj files in this repo are not part of the solution; they are invoked via other targets. The project itself is using the Microsoft.Build.Traversal SDK, which (I believe) doesn't use the C# compiler. It should be functionally similar to a NoTargets project in that way. And not using .proj in a solution in VS is not a bug; .proj has no language identified with it in the project system, so VS doesn't know what to do with it by design. Just like if you made up something like .blahproj. It'll know it is a project, but it doesn't have a clear path on how to build it.

I know don't of the specifics beyond that, but it has always been that way. My general rule-of-thumb has always been; if the project needs to be in a solution, I use a .csproj with the NoTargets SDK. If not, I can use a .proj if something else is building it, outside a solution.

@MiYanni MiYanni enabled auto-merge July 18, 2025 00:12
@MiYanni MiYanni merged commit e834542 into dotnet:main Jul 18, 2025
27 checks passed
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.

4 participants