Skip to content

[GenerateEmbeddedResourcesManifest] handle Items with empty LogicalName #29306 #47887

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

Merged

Conversation

rafd123
Copy link
Contributor

@rafd123 rafd123 commented Apr 25, 2023

[GenerateEmbeddedResourcesManifest] handle Items with empty LogicalName #29306

  • You've read the Contributor Guide and Code of Conduct.
  • You've included unit or integration tests for your change, where applicable.
  • You've included inline docs for your change, where applicable.
  • There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.

Ensure that the GenerateEmbeddedResourcesManifest task handles the possibility of Items with no LogicalName

Description

A LogicalName isn't assigned to Resx resources. See https://github.com/dotnet/msbuild/blob/6300d22b25cc1bcb634663bd7087e31d8312bb15/src/Tasks/CreateManifestResourceName.cs#L231-L238

As a result, GenerateEmbeddedResourcesManifest needs to handle this possibility...otherwise it'll generate an invalid Microsoft.Extensions.FileProviders.Embedded.Manifest.xml with empty name attributes for File elements and empty ResourcePath elements.

Fixes #29306

@ghost ghost added needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically community-contribution Indicates that the PR has been added by a community member labels Apr 25, 2023
@ghost
Copy link

ghost commented Apr 25, 2023

Thanks for your PR, @rafd123. Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@rafd123
Copy link
Contributor Author

rafd123 commented Apr 25, 2023

@dotnet-policy-service agree

@mkArtakMSFT mkArtakMSFT added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates and removed needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically labels Apr 26, 2023
Copy link
Member

@MackinnonBuck MackinnonBuck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution, @rafd123!

Just a small suggestion and then this should be good to go.

dotnet#29306

A LogicalName isn't assigned to Resx resources.

See https://github.com/dotnet/msbuild/blob/6300d22b25cc1bcb634663bd7087e31d8312bb15/src/Tasks/CreateManifestResourceName.cs#L231-L238

As a result, GenerateEmbeddedResourcesManifest needs to handle this
possibility...otherwise it'll generate an invalid
Microsoft.Extensions.FileProviders.Embedded.Manifest.xml with empty
name attribute for File elements and empty ResourcePath elements.
@rafd123 rafd123 force-pushed the issue-29306-handle-missing-logicalname branch from 8ca5dee to 12383a6 Compare April 26, 2023 17:49
Copy link
Member

@MackinnonBuck MackinnonBuck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

@MackinnonBuck MackinnonBuck merged commit 2dbc828 into dotnet:main Apr 26, 2023
@ghost ghost added this to the 8.0-preview5 milestone Apr 26, 2023
@rafd123 rafd123 deleted the issue-29306-handle-missing-logicalname branch April 27, 2023 00:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ManifestEmbeddedFileProvider corrupt manifest file
3 participants