Skip to content

Workaround for issue 40015: nuget restore uses 'resource' instead of 'resources'#44612

Merged
marcpopMSFT merged 1 commit intodotnet:mainfrom
YehezkelShB:ResourceAssetType
Nov 12, 2024
Merged

Workaround for issue 40015: nuget restore uses 'resource' instead of 'resources'#44612
marcpopMSFT merged 1 commit intodotnet:mainfrom
YehezkelShB:ResourceAssetType

Conversation

@YehezkelShB
Copy link
Copy Markdown
Contributor

Fixes #40015

Tested locally with a project manifesting the issue (throwing System.InvalidOperationException: Unrecognized AssetType 'resource'), and with the new Microsoft.NET.Build.Tasks.dll it doesn't happen

Copy link
Copy Markdown
Member

@dsplaisted dsplaisted left a comment

Choose a reason for hiding this comment

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

I'm not sure about the details of how this issue happens. It sounds like it may be a malformed package that causes the error, but the package apparently agrees with NuGet about what the asset type should be called.

Anyway, this seems like it will probably fix the issue or at least not make things worse, so I think it's a good idea.

@yitzhaks
Copy link
Copy Markdown

yitzhaks commented Nov 5, 2024

/ba-g Build failure is wholly unrelated to this change

@YehezkelShB
Copy link
Copy Markdown
Contributor Author

Seems like all the checks have passed 🥳
Is it possible to merge it, please, or there is anything else I have to do?
Thanks!

@dsplaisted
Copy link
Copy Markdown
Member

FYI @baronfel @marcpopMSFT

@marcpopMSFT marcpopMSFT merged commit 1254e8c into dotnet:main Nov 12, 2024
@YehezkelShB YehezkelShB deleted the ResourceAssetType branch November 13, 2024 14:38
@YehezkelShB
Copy link
Copy Markdown
Contributor Author

Any chance to backport this to release/8.0.4xx and/or release/9.0.1xx? 🙏
@dsplaisted @marcpopMSFT @baronfel

@marcpopMSFT
Copy link
Copy Markdown
Member

9.0.2xx yes. I don't know if we have enough feedback to justify taking it for 1xx or 4xx which are fairly locked down. The risk looks low but generally to get approval we need a strong customer justification.

How long has this been broken (I think the answer is always)? Why is this coming up more recently as impacting rather than in previous releases?

@YehezkelShB
Copy link
Copy Markdown
Contributor Author

It isn't new, as you said, but only lately we started to care as we now need to build a project for new .NET and then it fails (for .Net Fx it works somehow). Adding this fix to 8.0.4xx will unblock us in a reasonable timeline. Waiting to 9.0.2xx, or even waiting for the monorepo to start using 9.0.1xx, means we have to find a workaround with today SDK.

To be clear: if there is any workaround suggestion, we'd be very happy to try it so we can be unblocked without burdening you with backporting considerations. Thanks in advance!

@YehezkelShB
Copy link
Copy Markdown
Contributor Author

Well, seems like 8.0.4xx series isn't interesting for us anymore. Not sure if I am still pursuing a backport for 9.0.1xx either as it seems that switching from RuntimeIdentifiers definition in the project to using separated RuntimeIdentifier definitions, avoids this issue. Does it make sense to you?

@dsplaisted
Copy link
Copy Markdown
Member

/backport to release/9.0.2xx

@github-actions
Copy link
Copy Markdown
Contributor

Started backporting to release/9.0.2xx: https://github.com/dotnet/sdk/actions/runs/12013646895

@dsplaisted
Copy link
Copy Markdown
Member

To consider backporting to 9.0.1xx, what we would need is a better description / understanding of what scenario is broken. My understanding is that the issue is with NuGet packages that have an asset type of resource. That seems to be a rare occurrence, otherwise we'd be seeing a lot more people hit this bug. How are such packages created? Where do they come from?

Thanks.

@YehezkelShB
Copy link
Copy Markdown
Contributor Author

There is no assetType: resource added anywhere in the NuGet package creation. As we saw in the discussion in the original issue, the problem comes from NuGet restore code directly. The point is that NuGet adds assetType only if the asset doesn't have RID assigned in the NuGet creation, which makes sense for language resource DLLs, but it isn't necessarily what everyone does. Also, as mentioned above, it seems like using RuntimeIdentifier (singular) in the consumer project instead of RuntimeIdentifiers (plural) also avoids this issue (I guess the SDK doesn't look at assetType at all in this case).

Anyway, it seems like we are fine with the current state so no additional backport is needed as much as we care. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-NetSDK untriaged Request triage from a team member

Projects

None yet

4 participants