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

[Feature]: Support overriding centrally managed version on a package reference #9465

Closed
cristinamanum opened this issue Apr 22, 2020 · 6 comments
Assignees
Labels
Area:RestoreCPM Central package management Functionality:Restore Priority:1 High priority issues that must be resolved in the current sprint. Type:Feature

Comments

@cristinamanum
Copy link
Contributor

cristinamanum commented Apr 22, 2020

Details about Problem

Feature request

CPVM support version overrides.

Version overrides are supported as a product of MsBuild flexibility.

Option 1: use the Import MsBuild element but there is not a build in experience for the version overrides.

<Project>
	  <ItemGroup>
	    <PackageVersion Include="Xunit.StaFact.Mac" Version="1.0.24-beta-g88c3555268"/>
	    <PackageVersion Include="xunit.stafact" Version="1.0.24-beta-g88c3555268"/>
	  </ItemGroup>
	
	  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory).., Directory.Packages.props))\Directory.Packages.props" Condition="'$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory).., Directory.Packages.props))' != ''" />
	</Project>

Option 2 : use the MsBuild Update as :
<PackageVersion Update="Foo" Version="1.2.3"/>

Option 3: use the MsBuild Conditions as :

Directory.Packages.Props

<Project>
  <ItemGroup Condition="'$(Group)'=='Test'">
    <PackageVersion Include="Newtonsoft.Json" Version="12.0.2" />
  </ItemGroup>
  <ItemGroup>
    <PackageVersion Include="Newtonsoft.Json" Version="12.0.3" />
    <PackageVersion Include="EntityFramework" Version="6.2.0" />
    <PackageVersion Include="ParentLibTestCM1" Version="3.0.0" />
    <PackageVersion Include="LibTestCM1" Version="3.0.0" />
    <PackageVersion Include="NUnit" Version="3.12.0" />
    <PackageVersion Include="Microsoft.Azure.Storage.Common" Version="11.1.2" /> 
  </ItemGroup>
</Project>

Directory.Build.props (for the project that needs Newtonsoft 12.0.2)

<Project>
  <PropertyGroup>
    <Group>Test</Group>
  </PropertyGroup>
</Project>
@cristinamanum
Copy link
Contributor Author

Three options were discussed. Added them below.

  1. Reuse the Version attribute on the PackageReference item to override the version defined centrally

Behavior changes
In the current implementation having a project with a defined Version on a PackageReference is an error case that leads to a failed restore.
With the proposed solution the behavior will change and the Version value from the PackageReference will override the centrally defined version.
Pros:
Reduced cognitive overload. The users do not need to learn about a new concept and we(NuGet) do not need to create/maintain new documentation.
Reduced engineering cost. The data is already pipped from the clients (MsBuild, ProjectSystem) and integrated through the NuGet code paths. Also there will be no need to support a new error code.
The onboarding is easier/friendlier. As the projects with defined PackageReference Versions will not fail restore, the users can opt-in at their own peace to CPVM.
Cons:
There could be cases when the users will forget to remove the Version attribute and override unintentionally.

  1. Introduce a new metadata attribute “VersionOverride”

Behavior changes
None. The restore will continue to fail on the projects with PackageReference items that have Version defined.
To override the centrally defined versions the PackageReference items will need to define VersionOverride =”1.2.3”
Pros:
Reduce the chance to override unintentionally.
Closer model with the current MsBuild SDK solution.
Cons:
Increased cognitive overload. The users will need to learn a new concept and we(NuGet) will need to create/maintain new documentation.
Increased engineering cost.
• NuGet will need to read and process new data.
• ProjectSystem will need to pass new information to NuGet.
Usability drawback. There will be two actions needed to have a Version overridden:
• Remove the Version attribute.
• Add the VersionOverride attribute.

  1. Use a Grouping method but with friendlier interface.
    Directory.Packages.props
<Project>
  <ItemGroup name="Test">
    <PackageVersion Include="Newtonsoft.Json" Version="12.0.2" />
  </ItemGroup>
 
  <ItemGroup>
    <PackageVersion Include="Newtonsoft.Json" Version="12.0.3" />
    <PackageVersion Include="EntityFramework" Version="6.2.0" />
    <PackageVersion Include="ParentLibTestCM1" Version="3.0.0" />
    <PackageVersion Include="LibTestCM1" Version="3.0.0" />
    <PackageVersion Include="NUnit" Version="3.12.0" />
    <PackageVersion Include="Microsoft.Azure.Storage.Common" Version="11.1.2" /> 
  </ItemGroup>
</Project>

.csproj

<Project>
  <ItemGroup>
    <PackageReference Include="Newtonsoft.Json" Group=”Test” />
  </ItemGroup>
</Project>

@cristinamanum
Copy link
Contributor Author

Cleaned the milestone as more customer insights are needed to decide the right workflow.

@cristinamanum cristinamanum removed this from the Sprint 173 - 2020.07.06 milestone Jul 13, 2020
@nkolev92 nkolev92 added the Priority:2 Issues for the current backlog. label Jul 16, 2020
@cristinamanum cristinamanum added this to the Sprint 174 - 2020.07.27 milestone Jul 27, 2020
@kzu
Copy link

kzu commented Aug 6, 2020

Hi there,
I've been exploring this and I think there is a third option that is a mix of what's proposed above.

  1. Reuse the Version attribute on the PackageReference item to override a DefaultVersion defined centrally

Behavior changes
In the current implementation having a project with a defined Version on a PackageReference is an error case that leads to a failed restore. With the proposed solution the behavior will change and the Version value from the PackageReference will override the centrally defined version but only if it's declared as DefaultVersion. If it's Version, behavior will not change.

Pros:
Same as for option 1 plus option 2!
Overridability is explicity by the choice of DefaultVersion instead of Version in the centrally defined PackageVersion.
If someone overrides by mistake, this causes a failure (since Version will continue to behave like it currently does).

Cons:
Increased cognitive overhead for the new feature (learn difference between Version and DefaultVersion)

Thanks

@cristinamanum
Copy link
Contributor Author

Hello @kzu, thank you very much for the feedback! Would the proposed solution be similar with #9465 (comment) ?
Also we are currently engaging with developers to collect feedback for this feature and CPVM overall. Please let us know if you would you like to help in this study? @rconard

@cristinamanum cristinamanum changed the title [CPVM-OnBoard] Support version overrides [CPVM-OnBoard] Support version overrides - prototyping with the current spec version Sep 8, 2020
@kzu
Copy link

kzu commented Dec 21, 2020

Heya, yes, count me in if it's not too late.

It was confusing that the options were not all laid out it in the main description, and rather just a comment, but yes, the comment includes what I mentioned.

@jeffkl jeffkl self-assigned this Jan 3, 2022
@jeffkl jeffkl added this to the Sprint 2022-01 milestone Jan 3, 2022
@jeffkl jeffkl changed the title [CPVM-OnBoard] Support version overrides - prototyping with the current spec version [CPM] Support overriding centrally managed version on a package reference Jan 12, 2022
@jeffkl jeffkl changed the title [CPM] Support overriding centrally managed version on a package reference [Feature]: Support overriding centrally managed version on a package reference Jan 12, 2022
@jeffkl jeffkl added Priority:1 High priority issues that must be resolved in the current sprint. and removed Priority:2 Issues for the current backlog. labels Jan 12, 2022
@jeffkl
Copy link
Contributor

jeffkl commented Jan 20, 2022

Closing in favor of #11516 which has final design

@jeffkl jeffkl closed this as completed Jan 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area:RestoreCPM Central package management Functionality:Restore Priority:1 High priority issues that must be resolved in the current sprint. Type:Feature
Projects
None yet
Development

No branches or pull requests

7 participants