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 package version pinning for ref/ assemblies in servicing #25851

Merged
merged 2 commits into from
Sep 13, 2020

Conversation

dougbu
Copy link
Member

@dougbu dougbu commented Sep 13, 2020

  • dotnet/runtime now keeps $(AssemblyVersion) values consistent
  • remove all use of the System.Security.AccessControl package

- dotnet/runtime now keeps `$(AssemblyVersion)` values consistent
- remove all use of the System.Security.AccessControl package
@dougbu dougbu added area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework tell-mode Indicates a PR which is being merged during tell-mode labels Sep 13, 2020
@dougbu dougbu requested a review from a team September 13, 2020 01:42
@dougbu
Copy link
Member Author

dougbu commented Sep 13, 2020

@Pilchie here's the third of three more tell mode changes that I've found will be required for servicing. All that's left is the $(AssemblyVersion) change and that's validating now…

runtime packages for everything else. This is not an issue for assemblies available in Microsoft.NETCore.App.Ref or
Microsoft.Extensions.Internal.Transport because it is next to impossible we would service those packages.

System.Security.AccessControl should only be referenced in Dependencies.props and RepoTasks.csproj. Because
Copy link
Member

Choose a reason for hiding this comment

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

What changed to make this no longer necessary?

Copy link
Member Author

Choose a reason for hiding this comment

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

As I mentioned in the description, "dotnet/runtime now keeps $(AssemblyVersion) values consistent", specifically for the ref/ assemblies packaged outside Microsoft.NETCore.App.Ref and Microsoft.Extensions.Internal.Transport (where versions were already pinned). Their change is checked in for RC2 and so we no longer need any of these overrides.

Did I miss anything @ericstj @Anipik

Copy link
Member

Choose a reason for hiding this comment

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

But from the comment we used to include it in our ref pack, even though the Core ref didn't. Don't we still need to do that?

Copy link
Member Author

Choose a reason for hiding this comment

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

We'll grab the ref/ assemblies from the current (latest) packages rather than the .0 packages. System.Security.AccessControl in particular comes in due to transitive references; we don't need to reference it directly because we don't need to pin the ref/ assembly.

Copy link
Member

Choose a reason for hiding this comment

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

Got it, thanks.

@dougbu dougbu merged commit bfc1ec6 into release/5.0-rc2 Sep 13, 2020
@dougbu dougbu deleted the dougbu/unpin.package.versions branch September 13, 2020 21:24
dougbu added a commit that referenced this pull request Sep 25, 2020
- doing now ensures we don't forget if we need to service targeting packs
- not needed in release/3.1 because that branch still pins packages at `X.Y.0` in targeting packs
  - did not do bfc1ec6 / #25851 work in 3.1 because dotnet/runtime ref/ assemblies change there
- revert no-longer-necessary parts of 6418c8f / #25986
  - `$(MicrosoftNETCoreAppRuntimeVersion)` is now correct whenever `GeneratePackageOverrides` runs
dougbu added a commit that referenced this pull request Sep 25, 2020
* Support override of PackageOverrides.txt content in servicing
- doing now ensures we don't forget if we need to service targeting packs
- not needed in release/3.1 because that branch still pins packages at `X.Y.0` in targeting packs
  - did not do bfc1ec6 / #25851 work in 3.1 because dotnet/runtime ref/ assemblies change there
- revert no-longer-necessary parts of 6418c8f / #25986
  - `$(MicrosoftNETCoreAppRuntimeVersion)` is now correct whenever `GeneratePackageOverrides` runs

nits:
- rename `GeneratePackageConflictManifest` target to `GeneratePackageOverrides`
- rename `$(PackageConflictManifestFileName)` to `$(PackageOverridesFileName)`
dougbu added a commit that referenced this pull request Nov 29, 2020
…ng (#25851)"

- dotnet/runtime assembly versions are unexpectedly changing in servicing

This reverts commit bfc1ec6.
dotnet-maestro bot added a commit that referenced this pull request Nov 30, 2020
…28087)

[release/5.0] Update dependencies from dotnet/efcore dotnet/runtime


 - Merge branch 'release/5.0' into darc-release/5.0-d4478e43-6d04-47a1-8a7c-c6c2dcd90d64

 - Tweak tests

 - Remove Extensions.Internal.Transport from Runtime

- Microsoft.AspNetCore.App.Runtime project does not expect compilation-only references

 - !fixup! Rearrange a `Condition`
- slight change to 49cc13c workaround

 - Do not compile against assemblies with newer assembly versions
- avoid problems with e.g. System.Extensions.DependencyInjection in 5.0.1

 - Revert "Remove package version pinning for ref/ assemblies in servicing (#25851)"
- dotnet/runtime assembly versions are unexpectedly changing in servicing

This reverts commit bfc1ec6.

 - Update `SharedFxTests` to handle dotnet/runtime version changes
- assemblies with non-0.0 versions end up in Microsoft.AspNetCore.App
- future-proofs these tests because more dotnet/runtime versions may change

 - !fixup! Revert of bfc1ec6 messed up `RepoTasks`
- need the RTM-versioned packages on all platforms
  - we only target `net472` on Windows
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework tell-mode Indicates a PR which is being merged during tell-mode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants