Handle case of null packageStream #977
Conversation
There was a problem hiding this comment.
Given the NuGet lInk you shared earlier, LGTM. cc: @sharwell
| downloadResult.PackageSource, | ||
| #pragma warning restore SA1114 // Parameter list should follow declaration | ||
| #endif | ||
| downloadResult.PackageStream, |
There was a problem hiding this comment.
In the reference link, this path passes both the PackageReader and the PackageStream. Should we be doing that?
There was a problem hiding this comment.
That's the 3rd overload, and it would require stream not be null too:
https://github.com/nuget/nuget.client/blob/0ff5f67097522299620f34bd13f92d3d58e41f93/src/NuGet.Core/NuGet.Packaging/PackageExtractor.cs#L152
Again though I really am just pattern matching here. We could commit this on the assumption that clearly it would only affect a path that's failing already. Or, we could consult someone on the Nuget team, what do you prefer?
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
|
Rerunning now tests fixed |
|
@sharwell now failing thus error : Failed to verify the root directory of local source 'https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-tools/nuget/v3/index.json'. |
|
@sharwell can you please review? |
Fix #974
TBH, I do not know whether this is the right fix (or even a fix).
I am following this pattern:
https://github.com/nuget/nuget.client/blob/0ff5f67097522299620f34bd13f92d3d58e41f93/src/NuGet.Core/NuGet.PackageManagement/Projects/FolderNuGetProject.cs#L166-L188
review without whitespace changes
https://github.com/dotnet/roslyn-sdk/pull/977/files?diff=unified&w=1