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

Remove netstandard1.x dependencies in source-build #5885

Conversation

ViktorHofer
Copy link
Contributor

@ViktorHofer ViktorHofer commented Jun 28, 2024

Bug

Fixes: dotnet/source-build#4482

Regression? Last working version:

Description

  • Remove unnecessary netstandard1.x dependencies
  • Make build task projects target net and netfx
    (NuGet.Build.Tasks.Pack and build/docs.proj)

Unrelated:

  • Mark build.sh as executable (chmod +x)

PR Checklist

  • PR has a meaningful title

  • PR has a linked issue.

  • Described changes

  • Tests

    • Automated tests added
    • OR
    • Test exception
    • OR
    • N/A
  • Documentation

    • Documentation PR or issue filled
    • OR
    • N/A

- Remove unnecessary netstandard1.x dependencies
- Make NuGet.Build.Tasks.Pack target net and netfx
- Upgrade Microsoft.CSharp version to latest 4.7.0
  (to avoid bringing in netstandard1.x)

Unrelated:
- Mark build.sh as executable (chmod +x)
@ViktorHofer ViktorHofer requested a review from a team as a code owner June 28, 2024 09:06
@dotnet-policy-service dotnet-policy-service bot added the Community PRs created by someone not in the NuGet team label Jun 28, 2024
@@ -41,7 +41,7 @@
<None Include="$(BuildCommonDirectory)NOTICES.txt" Pack="true" PackagePath="\" />
</ItemGroup>

<ItemGroup Condition=" '$(TargetFramework)' != '$(NETFXTargetFramework)' ">
<ItemGroup Condition=" '$(TargetFramework)' == '$(NETCoreTargetFramework)' ">
Copy link
Contributor

Choose a reason for hiding this comment

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

use TFI instead?

@ViktorHofer ViktorHofer marked this pull request as draft June 28, 2024 13:11
@dotnet-policy-service dotnet-policy-service bot added the Status:No recent activity PRs that have not had any recent activity and will be closed if the label is not removed label Jul 5, 2024
@dotnet-policy-service dotnet-policy-service bot removed the Status:No recent activity PRs that have not had any recent activity and will be closed if the label is not removed label Jul 15, 2024
@@ -41,7 +41,7 @@
<None Include="$(BuildCommonDirectory)NOTICES.txt" Pack="true" PackagePath="\" />
</ItemGroup>

<ItemGroup Condition=" '$(TargetFramework)' != '$(NETFXTargetFramework)' ">
<ItemGroup Condition=" '$(TargetFramework)' == '$(NETCoreTargetFramework)' ">
<PackageReference Include="Microsoft.Build.Framework" ExcludeAssets="runtime" GeneratePathProperty="true" />
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rainersigwald is it recommended to reference the MSBuild packages on all frameworks or just on the ones where references aren't available? Asking as in L35-L36, msbuild references are used from the .NET Framework installation when targeting netfx.

Copy link
Contributor

Choose a reason for hiding this comment

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

The .NET Framework references are the "most compatible" option, but . . . do we really support modern Pack-in-MSBuild in Visual Studio 2010? That doesn't seem required.

I would lean toward "reference the packages at the explicit lowest-supported-version for the netfx output".

@dotnet-policy-service dotnet-policy-service bot added the Status:No recent activity PRs that have not had any recent activity and will be closed if the label is not removed label Jul 23, 2024
@ViktorHofer
Copy link
Contributor Author

ViktorHofer commented Jul 26, 2024

I will get back to this when I return from my leave but I would be happy if someone else wants to pick this up. cc @MichaelSimons

@dotnet-policy-service dotnet-policy-service bot removed the Status:No recent activity PRs that have not had any recent activity and will be closed if the label is not removed label Jul 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Community PRs created by someone not in the NuGet team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove netstandard1.x assets from SBRP
3 participants