Skip to content

Prevent downstream dependencies to pickup MinVer as a depencency #1838

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

romerod
Copy link

@romerod romerod commented May 23, 2025

Proposed Changes

MinVer is beeing picked up by downstream dependencies which it shouldn't, see https://github.com/adamralph/minver/blob/main/README.md?plain=1#L55 .

This even breaks compatibility of the package with NonSDK projects.

https://www.nuget.org/packages/RabbitMQ.Client.OAuth2 was already published with this dependency.

As I didn't change any code I didn't run the tests.

Types of Changes

What types of changes does your code introduce to this project?
Put an x in the boxes that apply

  • Bug fix (non-breaking change which fixes issue #NNNN)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause an observable behavior change in existing systems)
  • Documentation improvements (corrections, new content, etc)
  • Cosmetic change (whitespace, formatting, etc)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating
the PR. If you're unsure about any of them, don't hesitate to ask on the
mailing list. We're here to help! This is simply a reminder of what we are
going to look for before merging your code.

  • I have read the CONTRIBUTING.md document
  • I have signed the CA (see https://cla.pivotal.io/sign/rabbitmq)
  • All tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • Any dependent changes have been merged and published in related repositories

@lukebakken lukebakken self-assigned this May 23, 2025
@lukebakken lukebakken added this to the 7.2.0 milestone May 23, 2025
Copy link
Collaborator

@lukebakken lukebakken left a comment

Choose a reason for hiding this comment

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

Thank you @romerod

Does this apply to any of the other package references in this project as well?

@paulomorgado
Copy link
Contributor

Thank you @romerod

Does this apply to any of the other package references in this project as well?

Any package that is development time only and not a run time dependency, should have this.

This should be automatic when adding the reference through NuGet tooling.

@romerod
Copy link
Author

romerod commented May 23, 2025

Thank you @romerod
Does this apply to any of the other package references in this project as well?

Any package that is development time only and not a run time dependency, should have this.

This should be automatic when adding the reference through NuGet tooling.

exactly, I didn't focus on the other global references.

Did take a quick look now and at first sight they also seem to be development time dependencies.

@lukebakken
Copy link
Collaborator

@paulomorgado @romerod please make the necessary changes for all dev-time deps in this PR. Thank you.

@paulomorgado
Copy link
Contributor

To start, I've created a PowerShell script to list all packages in referenced by the project:

$rr = dir *.csproj -File -Recurse | ? { ($_.DirectoryName -notlike '*Test*') -and ($_.DirectoryName -notlike '*Benchmarks*') -and ($_.DirectoryName -notlike '*Build*') -and ($_.DirectoryName -notlike '*toxiproxy-netcore*') } | % { $p = $_.Name; $r = (dotnet msbuild $_.FullName  -getItem:PackageReference | ConvertFrom-Json); $r.Items | % { $_.PackageReference | Add-Member -NotePropertyName 'Project' -NotePropertyValue $p }; $r.Items.PackageReference }

One can query it as:

$rr | select Project, Identity, Version, IncludeAssets, PrivateAssets | ft -a
Project Identity Version IncludeAssets PrivateAssets
CreateChannel.csproj Microsoft.CodeAnalysis.PublicApiAnalyzers Runtime;Build;Native;contentFiles;Analyzers All
CreateChannel.csproj Microsoft.SourceLink.GitHub Runtime;Build;Native;contentFiles;Analyzers All
CreateChannel.csproj MinVer Runtime;Build;Native;contentFiles;Analyzers All
GH-1647.csproj Microsoft.CodeAnalysis.PublicApiAnalyzers Runtime;Build;Native;contentFiles;Analyzers All
GH-1647.csproj Microsoft.SourceLink.GitHub Runtime;Build;Native;contentFiles;Analyzers All
GH-1647.csproj MinVer Runtime;Build;Native;contentFiles;Analyzers All
GH-1749.csproj EasyNetQ.Management.Client
GH-1749.csproj Microsoft.CodeAnalysis.PublicApiAnalyzers Runtime;Build;Native;contentFiles;Analyzers All
GH-1749.csproj Microsoft.SourceLink.GitHub Runtime;Build;Native;contentFiles;Analyzers All
GH-1749.csproj MinVer Runtime;Build;Native;contentFiles;Analyzers All
MassPublish.csproj Microsoft.CodeAnalysis.PublicApiAnalyzers Runtime;Build;Native;contentFiles;Analyzers All
MassPublish.csproj Microsoft.SourceLink.GitHub Runtime;Build;Native;contentFiles;Analyzers All
MassPublish.csproj MinVer Runtime;Build;Native;contentFiles;Analyzers All
PublisherConfirms.csproj Microsoft.CodeAnalysis.PublicApiAnalyzers Runtime;Build;Native;contentFiles;Analyzers All
PublisherConfirms.csproj Microsoft.SourceLink.GitHub Runtime;Build;Native;contentFiles;Analyzers All
PublisherConfirms.csproj MinVer Runtime;Build;Native;contentFiles;Analyzers All
RabbitMQ.Client.csproj System.IO.Pipelines
RabbitMQ.Client.csproj System.Threading.RateLimiting
RabbitMQ.Client.csproj Nullable all
RabbitMQ.Client.csproj Microsoft.CodeAnalysis.PublicApiAnalyzers Runtime;Build;Native;contentFiles;Analyzers All
RabbitMQ.Client.csproj Microsoft.SourceLink.GitHub Runtime;Build;Native;contentFiles;Analyzers All
RabbitMQ.Client.csproj MinVer Runtime;Build;Native;contentFiles;Analyzers All
RabbitMQ.Client.OAuth2.csproj MinVer [6.0.0,)
RabbitMQ.Client.OAuth2.csproj RabbitMQ.Client [7.0.0,)
RabbitMQ.Client.OpenTelemetry.csproj OpenTelemetry.Api
RabbitMQ.Client.OpenTelemetry.csproj Microsoft.CodeAnalysis.PublicApiAnalyzers Runtime;Build;Native;contentFiles;Analyzers All
RabbitMQ.Client.OpenTelemetry.csproj Microsoft.SourceLink.GitHub Runtime;Build;Native;contentFiles;Analyzers All
RabbitMQ.Client.OpenTelemetry.csproj MinVer Runtime;Build;Native;contentFiles;Analyzers All

Since GlobalPackageReference is being used and, according to the docs, IncludeAssets="Runtime;Build;Native;contentFiles;Analyzers" and PrivateAssets="All" is included. I would remove the direct references in the projects and let GlobalPackageReference do its work.

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