Skip to content

Set PackageValidationBaselineName to assemblyName when packageID is not set - #18658

Merged
Anipik merged 5 commits into
dotnet:mainfrom
Anipik:package2
Jul 1, 2021
Merged

Anipik merged 5 commits into
dotnet:mainfrom
Anipik:package2

Conversation

@Anipik

@Anipik Anipik commented Jul 1, 2021 •

Copy link
Copy Markdown
Contributor

Fixes issue in dotnet/runtime#54250

@Anipik
Anipik requested review from ViktorHofer and safern July 1, 2021 06:14
@ghost

ghost commented Jul 1, 2021

Copy link
Copy Markdown

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@ViktorHofer

Copy link
Copy Markdown
Member

In which cases is PackageId not set?

Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com>
@Anipik

Anipik commented Jul 1, 2021

Copy link
Copy Markdown
Contributor Author

In which cases is PackageId not set?

Its being set late during the sdk evaluation.

@ViktorHofer

Copy link
Copy Markdown
Member

Its being set late during the sdk evaluation.

Doesn't that mean that PackageId is never used then?

@Anipik

Anipik commented Jul 1, 2021

Copy link
Copy Markdown
Contributor Author

Doesn't that mean that PackageId is never used then?

unless its a global property

Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com>
@ViktorHofer

Copy link
Copy Markdown
Member

unless its a global property

which is never the case usually.

@Anipik

Anipik commented Jul 1, 2021

Copy link
Copy Markdown
Contributor Author

which is never the case usually.

setting the packageID in the csproj would be another scenario.

@ViktorHofer

ViktorHofer commented Jul 1, 2021 •

Copy link
Copy Markdown
Member

setting the packageID in the csproj would be another scenario.

That's also very rarely the case for projects unless they would want to override it.

You could easily avoid this sequencing issue by inlining the PackageValidationBaselineName value into the two locations where it's used:

  • The PackageDownload item
  • In the task as a property later

By inlining it, you will only access the PackageId value after it is set by NuGet. And with that you can then remove the AssemblyName fallback scenario as well.

@ViktorHofer

Copy link
Copy Markdown
Member

LGTM module the one remaining feedback item.

Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com>
@Anipik
Anipik merged commit 471e20e into dotnet:main Jul 1, 2021
v-wuzhai pushed a commit that referenced this pull request Apr 22, 2024
ViktorHofer pushed a commit that referenced this pull request May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants