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

dotnet restore fails, but reports success #6281

Closed
ghost opened this issue Mar 19, 2021 · 7 comments · Fixed by #6312
Closed

dotnet restore fails, but reports success #6281

ghost opened this issue Mar 19, 2021 · 7 comments · Fixed by #6312
Assignees

Comments

@ghost
Copy link

ghost commented Mar 19, 2021

Copied from issue NuGet/Home#10606 per jeffkl's guidance

Details about Problem

Cross posting from dotnet sdk, as it isn't clear if this issue is in the sdk or the nuget client used in that sdk.
Repro posted on dotnet/sdk#16031 (comment)

Product version:

dotnetsdk 3.1.406

Repro steps and/or sample project

Repro is simple. Try to restore a package that doesnt exist. E.g. Microsoft.Build.Traversal2

aaronla@aaronla-rabbit:MyRepo$ dotnet restore
X:\MyRepo\dirs.proj : error : Unable to find package Microsoft.Build.Traversal with version (= 2.999.1)
X:\MyRepo\dirs.proj : error :   - Found 24 version(s) in MyCustomFeedOne [ Nearest version: 3.0.0 ]
X:\MyRepo\dirs.proj : error :   - Found 0 version(s) in MyCustomFeedTwo

aaronla@aaronla-rabbit:MyRepo$ echo %ERRORLEVEL%
0

Verbose Logs

@ghost ghost added bug needs-triage Have yet to determine what bucket this goes in. labels Mar 19, 2021
@jeffkl
Copy link
Contributor

jeffkl commented Mar 19, 2021

This is currently happening because restore is trying to do a "best effort" of loading the project and executing restore. Sometimes restore itself will pull in build logic from a package but the project can't be evaluated until restore is done. So we set some special flags during restore to accomplish this:

https://github.com/dotnet/msbuild/blob/main/src/MSBuild/XMake.cs#L1424-L1426

The logic to fail gracefully if an SDK isn't found was added in #2991 so that users in Visual Studio would get a better experience.

From the command-line, this logic culminates in a project loaded with no errors and potentially no targets, then nothing is executed and since there are no real errors the build succeeds. The intention of ignoring invalid imports is more about trying to successfully load a project even if stuff is missing. However, later on when we build, it should still fail if the project is "invalid".

We'll probably need to add a new flag that we specify at the command-line that says an SDK resolution failure is considered fatal. This will allow Visual Studio to still load the project as intended. Other missing imports can still be ignored and hopefully they'll show up during build since they could be restored from a package.

@ghost
Copy link
Author

ghost commented Mar 19, 2021

Makes sense. Just make sure that flag is set to true when either invoking the msbuild /t:Restore or restore dotnetcli commands, as it should be fatal for those commands.

If I ask dotnet to restore, and it doesn't, that should clearly be considered a failure.

@benvillalobos
Copy link
Member

Team Triage: @jeffkl is this something you wanted to tackle? it sounds like we'll need to catch restore as a command line arg and pass it along to account for this.

@jeffkl
Copy link
Contributor

jeffkl commented Mar 24, 2021

Yes sorry I couldn’t make it to today’s sync, I’ll attend next week to talk through the options.

@jeffkl
Copy link
Contributor

jeffkl commented Mar 31, 2021

@benvillalobos please assign this issue to me

@benvillalobos benvillalobos removed the needs-triage Have yet to determine what bucket this goes in. label Mar 31, 2021
jeffkl added a commit to jeffkl/msbuild that referenced this issue Mar 31, 2021
Forgind pushed a commit that referenced this issue Apr 8, 2021
…#6312)

Fixes #6281

Context
BuildRequestDataFlags and ProjectLoadSettings are set during /t:restore in a best effort to run the Restore target in hopes that it will correct the potentially bad state that a project is in. Visual Studio also sets ProjectLoadSettings.IgnoreMissingImports so that an unresolved MSBuild project SDK doesn't prevent loading of a bad project so it can give the user an error and let them edit the file.

However, this means that from the command-line an unresolved SDK doesn't fail /t:restore. This is because the missing "import" is ignored and non-existent targets are ignored so the build succeeds.

Changes Made
Introduced two new BuildRequestDataFlags:

SkipNonexistentNonTopLevelTargets - A new flag to be used in this context to tell the build to ignore non-existent targets but not top level ones. In this case we're specifying /t:restore so if the Restore target doesn't exist, that should be an error. Only other targets that are trying to run are ignored (like InitialTargets, Before/AfterTargets, etc).
FailOnUnresolvedSdk - We still need to ignore missing imports and I can't introduce a new flag to split the implementation now since Visual Studio sets ProjectLoadSettings.IgnoreMissingImports as a way to ignore unresolved SDKs. So this flag tells the evaluator to fail on an unresolved SDK but to continue ignoring other missing imports.
@ghost
Copy link
Author

ghost commented Apr 14, 2021

@jeffkl will this be available on either the 3.1 or 5.0 sdks? We're currently on 2.1, but in process of moving to the current LTS (3.1).

@ghost
Copy link
Author

ghost commented May 21, 2021

Confirmed fixed in 6.0.100-preview.6.21271.8

@AR-May AR-May added the triaged label Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants