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

Mark dependencies as private assets for GitVersion.MsBuild #3921

Merged
merged 1 commit into from
Feb 20, 2024
Merged

Mark dependencies as private assets for GitVersion.MsBuild #3921

merged 1 commit into from
Feb 20, 2024

Conversation

Louis9902
Copy link

Description

Change that all references of the msbuild package are marked as private assets, so they don't get added as dependency to the nuget package.

The dependecies field of the nuspec before the change:

<dependencies>
  <group targetFramework="net6.0">
    <dependency id="System.Text.Json" version="8.0.1" exclude="Build,Analyzers" />
  </group>
  <group targetFramework="net7.0">
    <dependency id="System.Text.Json" version="8.0.1" exclude="Build,Analyzers" />
  </group>
  <group targetFramework="net8.0">
    <dependency id="System.Text.Json" version="8.0.1" exclude="Build,Analyzers" />
  </group>
</dependencies>

and afer the change the dependencies of the nuspec look like this:

<dependencies>
  <group targetFramework="net6.0" />
  <group targetFramework="net7.0" />
  <group targetFramework="net8.0" />
</dependencies>

Related Issue

No, so far, happy to create one if the pr is not enough

Motivation and Context

The dependencies cause extra dll's to be published, for build only dependencies (PrivateAssets="All"). This problem seems to be cause by the change to the ceontral package management, where System.Text.Json was promoted to be included in the Directory.Build.props

How Has This Been Tested?

Build locally and checked the generated nuspec file in the outpur directory.

Screenshots (if appropriate):

grafik

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Chnage that all references of the msbuild package are marked as private
assets, so they don't get added as dependency to the nuget package.

The dependecies field of the nuspec before the change:
```xml
<dependencies>
  <group targetFramework="net6.0">
    <dependency id="System.Text.Json" version="8.0.1" exclude="Build,Analyzers" />
  </group>
  <group targetFramework="net7.0">
    <dependency id="System.Text.Json" version="8.0.1" exclude="Build,Analyzers" />
  </group>
  <group targetFramework="net8.0">
    <dependency id="System.Text.Json" version="8.0.1" exclude="Build,Analyzers" />
  </group>
</dependencies>
```

and afer the change the dependencies of the nuspec look like this:
```xml
<dependencies>
  <group targetFramework="net6.0" />
  <group targetFramework="net7.0" />
  <group targetFramework="net8.0" />
</dependencies>
```
@arturcic
Copy link
Member

@Louis9902 it seems the PR passes all the tests we have, including testing the actual nuget package.

I will check the nuget package the CI generated locally and see if it works as expected.

Thanks btw for the changes, it makes the nuspec much cleaner.

@arturcic
Copy link
Member

It works. Looks great

@arturcic arturcic changed the title mark dependencies as private assets for msbuild Mark dependencies as private assets for GitVersion.MsBuild Feb 20, 2024
@arturcic arturcic added this to the 6.x milestone Feb 20, 2024
@arturcic arturcic merged commit f91c014 into GitTools:main Feb 20, 2024
134 checks passed
Copy link
Contributor

mergify bot commented Feb 20, 2024

Thank you @Louis9902 for your contribution!

@arturcic arturcic modified the milestones: 6.x, 6.0.0-beta.6 Mar 9, 2024
@arturcic
Copy link
Member

arturcic commented Mar 9, 2024

🎉 This issue has been resolved in version 6.0.0-beta.6 🎉
The release is available on:

Your GitReleaseManager bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants