Revised proposal for Adding dotnet CLI support for projects onboarded to CPM #11873
Conversation
There was a problem hiding this comment.
Thanks for updating the spec @kartheekp-ms.
A few comments.
| | 1 | ❌ | ✔️ | ✔️ (in Directory.Packages.Props or .csproj file) | ❌ | Restore failed with NU1008 error and NO edits were made to the csproj file (same in VS and dotnet CLI) | `PackageReference` should be added to .(cs/vb)proj file and `PackageVersion` should be added to the closest `Directory.Packages.Props` file | ✔️ | | ||
| | 2 | ❌ | ❌ | ✔️ in (cs/vb)proj file | ❌ | Restore failed with NU1008 error and NO edits were made to the csproj file (same in VS and dotnet CLI) | `PackageReference` and `Version` should be added to .(cs/vb)proj file.| ✔️ | | ||
| | 3 | ❌ | ✔️ | ❌ | ❌ | `PackageReference` and `Version` added to .(cs/vb)proj file. | In addition to the current behavior `Directory.packages.props` file should be deleted if it exists in the project folder | ❌ | | ||
| | 4 | ✔️ | ✔️ | ✔️ | ❌ | **dotnet CLI** - Restore failed with NU1008 error and NO edits were made to the csproj file. **VS** - Clicked on Update package in PM UI. New `PackageVersion` added to csproj file and `Directory.packages.props` file was not updated | No changes should be made to the `PackageReference` item in .(cs/vb)proj file but `PackageVersion` item should be updated with the new version in the appropriate `Directory.Packages.Props` file.|✔️ [More Info](https://github.com/NuGet/Home/pull/11849#discussion_r890639808) | |
There was a problem hiding this comment.
I don't think add package should update versions.
This is something that's not readily apparent to the user.
Imo, the behavior should be either version override or an explciit command to update in CPM depending on whether --version has been passed it or not.
There was a problem hiding this comment.
We went down on this path because dotnet add package currently updates the package version if the reference already exists. In the case of CPM, updating version in the Directory.Packages.Props file is an option.
Your suggestion to add VersionOverride if the package reference already exists is also a good one. @jeffkl what do you think?
There was a problem hiding this comment.
In my opinion, its just more work for a user to use a different gesture for add vs update. They might be running it on a bunch of projects via automation and they'd have to determine whether not that package is already referenced. So I think add package should just update the version for them.
I'd rather not make VersionOverride easier to use personally. Its an escape hatch/last restore and I'd much rather people try to unify their graph. If we wanted to use VersionOverride, I would see that as a command-line argument for you opt into that functionality.
There was a problem hiding this comment.
In my opinion, its just more work for a user to use a different gesture for add vs update. They might be running it on a bunch of projects via automation and they'd have to determine whether not that package is already referenced. So I think add package should just update the version for them.
The reason why I bring it up is because we have issues and customer interviews that have suggested otherwise.
There's also an ask for a dotnet update package command, which is something we'd want to do eventually as well.
I think dotnet add package should match the VS experience for install package.
When you go to an individual project and run add package, you don't expect the package to be updated for other projects as well.
You could introduce potential compatibility issues but you wouldn't know about that until you get to your build, because there's no prevalidation for that.
There was a problem hiding this comment.
I think dotnet add package should match the VS experience for install package.
Current behavior in VS - "Clicked on Update package in PM UI. New PackageVersion added to csproj file and Directory.packages.props file was not updated". I think it is a bug because the tooling adds PackageVersion XML element not the VersionOverride.
EDIT - When I launched @jeffkl's PR branch NuGet/NuGet.Client#4642 in an experimental instance, I noticed that updating package version in Visual Studio for CPM project didn't update .csproj file also. But the updated version is present in the assets.json file and solution explorer dependencies node except the props file.
There was a problem hiding this comment.
I filed an issue for the <PackageVersion /> being added to the project: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1544117
There was a problem hiding this comment.
@jeffkl - Hope you are okay with changing the logic to add VersionOverride when add package is trying to update the package version. I will update the proposal shortly.
cc @pragnya17
There was a problem hiding this comment.
I updated the spec to capture the summary of this conversation. No changes should be made to the Directory.Packages.Props file. Add VersionOverride attribute to the existing PackageReference item in .(cs/vb)proj file.
There was a problem hiding this comment.
Yes if VersionOverride is already specified, we should update that value.
There was a problem hiding this comment.
Yes if VersionOverride is already specified, we should update that value.
I agree. Updated the spec.
Can I get approval on the updated spec?
…dated for update scenario
|
Can recommend this dotnet tool for automatic migration to CPM: |
Tracking issue #11858
Rendered Design Spec
Proposal for 11807
Here is what changed when compared to the initial proposal:
VersionOverridescenario