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

Fix System.Data.SqlClient dependency version on compat pack #55956

Merged
merged 1 commit into from
Jul 20, 2021

Conversation

safern
Copy link
Member

@safern safern commented Jul 19, 2021

Fixes: #55952

We should not treat System.Data.SqlClient as an indexed dependency on the compat pack as it is coming from an external repository, so every time a new version is released if we update the dependency version and don't update the packageIndex our infrastructure would treat it as a prerelease version rather than stable.

@ghost
Copy link

ghost commented Jul 19, 2021

Tagging subscribers to this area: @Anipik, @safern, @ViktorHofer
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes: #55952

We should not treat System.Data.SqlClient as an indexed dependency on the compat pack as it is coming from an external repository, so every time a new version is released if we update the dependency version and don't update the packageIndex our infrastructure would treat it as a prerelease version rather than stable.

Author: safern
Assignees: -
Labels:

area-Infrastructure-libraries

Milestone: -

@safern
Copy link
Member Author

safern commented Jul 20, 2021

Failures are unrelated.

@safern safern merged commit e3cf002 into main Jul 20, 2021
@safern safern deleted the safern-patch-1 branch July 20, 2021 19:13
@safern
Copy link
Member Author

safern commented Jul 20, 2021

/backport to release/6.0-preview7

@github-actions
Copy link
Contributor

Started backporting to release/6.0-preview7: https://github.com/dotnet/runtime/actions/runs/1050047702

@ViktorHofer
Copy link
Member

Thanks for the fix Santi 👍 One question so that I better understand the fallout:

We should not treat System.Data.SqlClient as an indexed dependency on the compat pack as it is coming from an external repository, so every time a new version is released if we update the dependency version and don't update the packageIndex our infrastructure would treat it as a prerelease version rather than stable.

I'm not sure if that's correct. System.Data.SqlClient lives in corefx which we don't treat as an external repository (do we?). Microsoft.Data.SqlClient is the new package which lives in https://github.com/dotnet/SqlClient but which we don't depend on in the Windows.Compatibility package.

Aside from servicing fixes, AFAIK the System.Data.SqlClient package should never change again. Does that impact the fix applied here?

@safern
Copy link
Member Author

safern commented Jul 22, 2021

I'm not sure if that's correct. System.Data.SqlClient lives in corefx which we don't treat as an external repository (do we?). Microsoft.Data.SqlClient is the new package which lives in https://github.com/dotnet/SqlClient but which we don't depend on in the Windows.Compatibility package.

IMHO it shouldn't be a decision based on what repo the package is built, I think we should only index dependencies for packages that are still live built at the same branch as the compat pack. The reason for that is because an indexed dependency means that is a stable package is not listed we will produce a version with the prerelease label that we are producing on that current build based on date and time of the build being produce. So I think that is what leads to this kind of bugs, now we have testing of this package going forward so it doesn't matter much as we would catch it as part of the build, but since packages that are no longer built on the current branch should not be indexed and should just be pinned as those packages will never have a package with version with the prerelease label produced on that branch.

@ghost ghost locked as resolved and limited conversation to collaborators Aug 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing dependency of Microsoft.Windows.Compatibility 6.0.0-preview.6.21352.12
4 participants