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

RestoreLockedMode=true & RestoreForceEvaluate=true should error #8222

Open
nkolev92 opened this issue Jun 13, 2019 · 6 comments
Open

RestoreLockedMode=true & RestoreForceEvaluate=true should error #8222

nkolev92 opened this issue Jun 13, 2019 · 6 comments
Assignees
Labels
Area:ErrorHandling warnings and errors/log messages & related error codes. Area:RestoreRepeatableBuild The lock file features Category:Quality Week Issues that should be considered for quality week Functionality:Restore help wanted Considered good issues for community contributions. Priority:2 Issues for the current backlog. Type:Bug

Comments

@nkolev92
Copy link
Member

Currently that's not the case.

Repro steps

  1. dotnet new classlib
  2. Enable lock file
  3. dotnet restore (generate lock file).
  4. dotnet restore /p:LockedMode="true"
  5. dotnet restore /p:LockedMode="true" /p:RestoreForceEvaluate="true"

The last one should fail, as the instructions do not make sense.
From skimming the code, RestoreForceEvaluate takes precedence.

https://github.com/NuGet/NuGet.Client/blob/bdcec93681afdd1f5cd553af07414a3b3d9a866c/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs#L462

//cc @anangaur
It's a trivial fix, so prioritize if you see a need for it.

@nkolev92 nkolev92 added Area:ErrorHandling warnings and errors/log messages & related error codes. Area:RestoreRepeatableBuild The lock file features Functionality:Restore labels Jun 13, 2019
@nkolev92 nkolev92 added this to the Backlog milestone Jun 13, 2019
@anangaur
Copy link
Member

anangaur commented Jun 13, 2019

Agree. But based on the options semantic meaning, shouldn’t it fail only if there needs to lock file update/modify?

@nkolev92
Copy link
Member Author

They feel like different commands :) Personally think error is better. Do we see a scenario where doing this makes sense?

Regardless though, currently the behavior is respecting ForceEvaluate and not considering RestoreLockedMode, which is counter to both our expectations.

@anangaur
Copy link
Member

They feel like different commands :) Personally think error is better. Do we see a scenario where doing this makes sense?

There is a corner case but I think for now, an error should be good enough.

@anangaur
Copy link
Member

Just to add /p:LockedMode="true" is the repeatable build promise on CI/CD machines and should not be broken. Hence I feel we should just fix this asap.

@nkolev92 nkolev92 added Priority:2 Issues for the current backlog. Sprint 154 and removed Triage:NeedsTriageDiscussion labels Jun 13, 2019
@nkolev92
Copy link
Member Author

fyi @rrelyea

//cc @NuGet/nuget-client If anyone is interested in taking this in week 3.
It's a simple fix, but a nice entry into the lock file/PR restore space.

@nkolev92
Copy link
Member Author

@cristinamanum

As I know you've been familiarizing yourself with lock file stuff, here's a lock file bug, that you can look into.

@nkolev92 nkolev92 self-assigned this Aug 27, 2019
@aortiz-msft aortiz-msft added the Category:Quality Week Issues that should be considered for quality week label Apr 2, 2020
@nkolev92 nkolev92 added the help wanted Considered good issues for community contributions. label Nov 6, 2020
@martinrrm martinrrm self-assigned this Jan 24, 2022
@martinrrm martinrrm modified the milestones: Backlog, Sprint 2022-01 Jan 24, 2022
@aortiz-msft aortiz-msft removed this from the Sprint 2022-01 milestone Jan 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area:ErrorHandling warnings and errors/log messages & related error codes. Area:RestoreRepeatableBuild The lock file features Category:Quality Week Issues that should be considered for quality week Functionality:Restore help wanted Considered good issues for community contributions. Priority:2 Issues for the current backlog. Type:Bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants