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

[BUG] 6.0.1 requires dotnetsdk 8 but doesn't say that #1625

Closed
drdamour opened this issue Feb 21, 2024 · 24 comments · Fixed by #1637
Closed

[BUG] 6.0.1 requires dotnetsdk 8 but doesn't say that #1625

drdamour opened this issue Feb 21, 2024 · 24 comments · Fixed by #1637
Labels
blocking-users Issue is blocking some users bug Something isn't working Priority:1 Very important to release, not a ship blocker Solved The issue is solved and can be closed

Comments

@drdamour
Copy link

Describe the bug
Had a build tied to 6.* for a version using sdk 6 and it failed with

Data collector 'XPlat code coverage' message: [coverlet]System.TypeLoadException: Could not load type 'Microsoft.Extensions.DependencyInjection.ServiceCollection' from assembly 'Microsoft.Extensions.DependencyInjection.Abstractions, Version=2.2.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60'.
   at Coverlet.Collector.DataCollection.CoverletCoverageCollector.GetDefaultServiceCollection(TestPlatformEqtTrace eqtTrace, TestPlatformLogger logger, String testModule)
   at Coverlet.Collector.DataCollection.CoverletCoverageCollector.OnSessionStart(Object sender, SessionStartEventArgs sessionStartEventArgs) in /_/src/coverlet.collector/DataCollection/CoverletCoverageCollector.cs:line 135.

To Reproduce
build with dotnet6

Expected behavior
Expect coverlet to run and report successfully

Actual behavior
fails to generate report

Configuration (please complete the following information):
Please provide more information on your .NET configuration:
* Which coverlet package and version was used?
* reproduceable with 6.0.1
* Which version of .NET is the code running on?
* 6.0.x (tried a few versions...all reproduced)
* What OS and version, and what distro if applicable?
* window 10 and linux ubuntu (whatever azdo is using now a days)
* What is the architecture (x64, x86, ARM, ARM64)?
* x64
* Do you know whether it is specific to that configuration?
* seems not to be

Additional context
the changes in this PR #1601 seems to force having dotnet 8 installed so that thinks like System.Text.Json will load the 8.0 version from the shared framework.

❗ Please also read Known Issues

doesn't appear to be any

overall, seems like mistake to break backwards compatability between 6.0.0 and 6.0.1 like this...doesn't seem to match semver...but maybe that was your intent? understand 6 is EOL at end of year so we should be moving to 8 sdk soon...

@github-actions github-actions bot added the untriaged To be investigated label Feb 21, 2024
@Bertk
Copy link
Collaborator

Bertk commented Feb 21, 2024

Thank you for reporting.

I used the coverlet HelloWord example and the test was executed successfully.

image

The .NET SDK 8.0 was installed.

@SeWieland
Copy link

Same problem here.. All our CI pipelines are failing because we are adding coverlet.collector dynamically to our test solutions via dotnet add package.

@Bertk the hello world example still uses PackageReference Include="coverlet.collector" Version="6.0.0" > => Version 6.0.0

dotnet add package will install version 6.0.1 (a patch update that must not contain breaking changes according to SemVer) and this package does not include any dependency to system.text.json at all.

@Bertk Bertk added bug Something isn't working and removed untriaged To be investigated labels Feb 21, 2024
@andreycha
Copy link

Have same issue. We're using .NET 6 SDK and have coverlet.collector referenced as 6.0.*. Having such a breaking change in a patch version is quite a surprise. Changes described in #1626 require at least new minor version.

@Bertk
Copy link
Collaborator

Bertk commented Feb 21, 2024

Sorry, I missed the to update the version.

@MarcoRossignoli @daveMueller We should update the version to 6.1.0.

@SeWieland
Copy link

SeWieland commented Feb 21, 2024

Another idea: "SuppressDependenciesWhenPacking" is set on the collector - effectively hiding all dependencies. Would it be possible to pack these two required package references as dependencies instead or something similar?
The breaking behaviour could thus probably be reverted for .NET < 8

@andreycha
Copy link

@Bertk would it be also possible to release 6.0.2 with reverted changes?

@Bertk
Copy link
Collaborator

Bertk commented Feb 21, 2024

@SeWieland Thank you for the hint but coverlet.collector package has no dependencies. The dilemma is the support for framework net6.0, net7.0 and net8.0 and this works with the latest packages for

    <PackageReference Include="System.Reflection.Metadata" VersionOverride="8.0.0" />
    <PackageReference Include="System.Collections.Immutable" VersionOverride="8.0.0" />
image

@MarcoRossignoli
Copy link
Collaborator

@MarcoRossignoli @daveMueller We should update the version to 6.1.0.

Why? we bump the patch we will bump the minor if we ship a new feature and a major if we do breaking change.

@Bertk
Copy link
Collaborator

Bertk commented Feb 21, 2024

@MarcoRossignoli coverlet 6.0.1 is a bugfix version but requires a .NET 8.0 SDK.

semver 2.0 has this paragraph

What should I do if I update my own dependencies without changing the public API?

That would be considered compatible since it does not affect the public API. Software that explicitly depends on the same dependencies as your package should have their own dependency specifications and the author will notice any conflicts. Determining whether the change is a patch level or minor level modification depends on whether you updated your dependencies in order to fix a bug or introduce new functionality. We would usually expect additional code for the latter instance, in which case it’s obviously a minor level increment.

@MarcoRossignoli
Copy link
Collaborator

but requires a .NET 8.0 SDK.

Why? we ship for
image

so we're good for .NET Framework and every project with tfm >= 6.0

@SeWieland
Copy link

@SeWieland Thank you for the hint but coverlet.collector package has no dependencies. The dilemma is the support for framework net6.0, net7.0 and net8.0 and this works with the latest packages for

    <PackageReference Include="System.Reflection.Metadata" VersionOverride="8.0.0" />
    <PackageReference Include="System.Collections.Immutable" VersionOverride="8.0.0" />
image

IMHO if there is support for .NET 6.0 it now has dependencies. If I have to pin other packages to a specific version for coverlet to be compatible, that package is a depdendency... Therefore, right now my workaround is to pin coverlet to v6.0.0 in all .NET 6 projects.

@drdamour
Copy link
Author

drdamour commented Feb 21, 2024

but requires a .NET 8.0 SDK.

Why? we ship for image

so we're good for .NET Framework and every project with tfm >= 6.0

Doesnt matter what u target or if u declare a dependency or not...u now require system.text.json 8 in 6.0.1…when u used to NOT..just cause u do not claim a dependendency doesnt mean u dont have one.

in fact by not claiming a dependency u seem to have..u’ve made yourself backwards incompatible.

So let me be clearer..i think its a mistake to require json 8 and think you should revert that change back to (i think it was 3.1) unless u require some functionality in json 8. Thats wouldnbe the better solution IMO.

If u MUST use json 8 you should bump a major version since users bow need it to implicitly be present or declare that dependency.

i too must pin to 6.0.0 when using sdk 6

@MarcoRossignoli
Copy link
Collaborator

MarcoRossignoli commented Feb 21, 2024

u now require system.text.json 8 in 6.0.1

I saw now what you mean https://github.com/coverlet-coverage/coverlet/pull/1601/files#diff-1d12ae6a5243ca16cfc9b56abc8423c15a917dd1adcdc3d7dd3482ccff2208da this was wrong...
tfm is 6.0 https://github.com/coverlet-coverage/coverlet/pull/1601/files#diff-1d12ae6a5243ca16cfc9b56abc8423c15a917dd1adcdc3d7dd3482ccff2208daL5

@Bertk @daveMueller we cannot declare >= 6.0 and insert non 6 lib...this needs to be fixed.

@MarcoRossignoli MarcoRossignoli added blocking-users Issue is blocking some users Priority:1 Very important to release, not a ship blocker and removed documentation labels Feb 21, 2024
@MarcoRossignoli MarcoRossignoli pinned this issue Feb 21, 2024
@Bertk Bertk added the Solved The issue is solved and can be closed label Feb 24, 2024
@tvbishan
Copy link

tvbishan commented Feb 26, 2024

I have the same issue. Works well with the coverlet.collector Version="6.0.0". But when upgraded to the coverlet.collector Version="6.0.1", it failed.

Data collector 'XPlat code coverage' message: [coverlet]System.IO.FileLoadException: Could not load file or assembly 'System.Text.Json, Version=8.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51'. Could not find or load a specific file. (0x80131621)
File name: 'System.Text.Json, Version=8.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51'

@cremor
Copy link

cremor commented Feb 26, 2024

I suggest to release 6.0.2 with that fix ASAP.

@kev24uk
Copy link

kev24uk commented Feb 26, 2024

Just experienced the same issue in one of our builds where it is unable to find System.Text.Json, Version=8.0.0.0 - downgrading to 6.0.0 fixed it for now.

@akutuev
Copy link

akutuev commented Feb 27, 2024

Hi @Bertk, can you please confirm, did you decide to keep .NET 8 for this and further versions yes?

@MarcoRossignoli
Copy link
Collaborator

Hi @Bertk, can you please confirm, did you decide to keep .NET 8 for this and further versions yes?

It's fixed here #1628 cc: @daveMueller

@akutuev
Copy link

akutuev commented Feb 27, 2024

Hi @Bertk, can you please confirm, did you decide to keep .NET 8 for this and further versions yes?

It's fixed here #1628 cc: @daveMueller

Ah I see, thanks for heads up! Hope NuGet repo will be updated soon

@drdamour
Copy link
Author

hope we see a release soon

@JeroenDeHaan1980
Copy link

When do you expect to have a new version released with the issue solved?

tspascoal added a commit to tspascoal/GitHubActions.Gates.Samples that referenced this issue Mar 1, 2024
tspascoal added a commit to tspascoal/GitHubActions.Gates.Samples that referenced this issue Mar 1, 2024
* Split CI into 2 jobs.
One for build and test and another for codescanning

Downgrade coverlet to workaround issue coverlet-coverage/coverlet#1625
blowdart added a commit to blowdart/idunno.PasswordGenerator that referenced this issue Mar 5, 2024
@daveMueller
Copy link
Collaborator

Hi guys, we finally merged everything we broke with the last release. We would really appreciate if someone could give it a try.
Here is how to consume the nightly: https://github.com/coverlet-coverage/coverlet/blob/master/Documentation/ConsumeNightlyBuild.md

@tvbishan
Copy link

tvbishan commented Mar 9, 2024

@daveMueller

It is working now.
Tested with image: mcr.microsoft.com/dotnet/sdk:6.0 and coverlet.collector --version 6.0.2-preview.8.g892d86e16b

@daveMueller
Copy link
Collaborator

We now have a new official release 6.0.2 that can be consumed from nuget.org.

@daveMueller daveMueller unpinned this issue Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocking-users Issue is blocking some users bug Something isn't working Priority:1 Very important to release, not a ship blocker Solved The issue is solved and can be closed
Projects
None yet
Development

Successfully merging a pull request may close this issue.