search

CommunityToolkit / Tooling-Windows-Submodule

Community Toolkit infrastructure for use as a submodule 'tooling' directory in other repositories.
Other
30 stars 9 forks source link

Fix NuGet pack #7

Closed michael-hawker closed 1 year ago

michael-hawker commented 1 year ago

Describe the bug

Related to CommunityToolkit/Tooling-Windows-Submodule#77

If you right-click on a library and try and Pack to build a NuGet package you get an error about Icon.png missing.

In face these resources for the icon don't exist in the repro:

https://github.com/CommunityToolkit/Labs-Windows/blob/c322845be3b1692fae6665480e9ed18ce6e4787a/Windows.Toolkit.Common.props#L12-L13

https://github.com/CommunityToolkit/Labs-Windows/blob/ed6bd23523770f34ebf6b41304dd257374fe1f04/Directory.Build.targets#L14-L19

We should be able to pack locally as well as have the proper files and icons included in the NuGet package.

@niels9001 do you know which icon asset should we use?

Steps to reproduce

1) Open an experiment or the all solution
2) Find a Library project
3) right-click on it and select 'Pack'
4) see error

Expected behavior

Able to pack NuGet package

Screenshots

No response

Code Platform

Windows Build Number

Other Windows Build number

No response

App minimum and target SDK version

Other SDK version

No response

Visual Studio Version

No response

Visual Studio Build Number

No response

Device form factor

No response

Additional context

I guess this is ignored on the commandline?

Help us help you

Yes, but only if others can assist.

michael-hawker commented 1 year ago

Note: If you comment out the Icon.png line 12 of the Windows.Toolkit.Common.props, then you can pack locally.

Nirmal4G commented 1 year ago

Just stumbled across this when going through repo...

  1. Remove <PackageIconUrl> as it's no longer needed. It is replaced by <PackageIcon>.
  2. Place the Icon.png in the RepositoryDirectory or $(BuildToolsDirectory).
  3. Then update the line... 
    <None Include="$(RepositoryDirectory)nuget.png" Pack="true" PackagePath="\Icon.png" />

    TO

    <None Include="$(RepositoryDirectory)Icon.png" Pack="true" PackagePath="\" />

    OR

    <None Include="$(BuildToolsDirectory)Icon.png" Pack="true" PackagePath="\" />

That should fix the issue. Ping me if any build code that was updated by me causing any problems. You can check git blame to see who wrote what line.

michael-hawker commented 1 year ago

Thanks for the tips @Nirmal4G. I remember we were trying to use PackageIconUrl still for some reason, but I don't remember what that reason was now... Maybe it was some prior VS issue that maybe's now resolved.

@niels9001 where else were we talking about the icon? I see we have CommunityToolkit/Tooling-Windows-Submodule#77 as well, but I don't see our prior chat there.

Let's get the new icon setup next week during a 1:1 and then we can get a PR opened to close this and that issue together.

niels9001 commented 1 year ago

For reference, NuGet icon for Labs can be found here: https://github.com/CommunityToolkit/design-assets/blob/main/Logos/Windows-Labs/Windows-Labs.nuget-256.png

michael-hawker commented 1 year ago

https://dev.azure.com/dotnet/CommunityToolkit/_artifacts/feed/CommunityToolkit-MainLatest/NuGet/CommunityToolkit.Uwp.Extensions/overview/8.0.230801-preview

Huh, I updated the PackageIcon property and fixed this in 162, but then I don't see it showing up properly in NuGet Explorer...

image

image

Same in VS:

image

Looks like it's missing even though it's clearly defined here: https://github.com/CommunityToolkit/Windows/blob/710c30ff564fe596b03db7ab247d267c7fd8a171/Windows.Toolkit.Common.props#L12

I'm not sure why this didn't work...

Nirmal4G commented 1 year ago

@michael-hawker I don't see an <icon/> element in the package manifest xml file you mentioned above. There's a possibility that <PackageIcon/> value either clears out before it enters the NuGet pack logic or does not pass through at all. Check it with MSBuild Log Viewer.

michael-hawker commented 1 year ago

Thanks for pointing that out @Nirmal4G, I'll have to look. We'll also have to be aware of this issue as well: https://github.com/NuGet/Home/issues/9250 - but I'd imagine the Package Explorer app wouldn't have that same issue.

michael-hawker commented 1 year ago

Going to try updating the NuGet pack lib we were using 5.2.0, but there's a 6.7.0 available - iconUrl is working fine... just not PackageIcon...

michael-hawker commented 1 year ago

Confirmed this is fixed by https://github.com/CommunityToolkit/Windows/pull/174 now! 🎉🎉🎉

image

image

According to this doc still it's recommended to specify both so that older clients can get the icon from the url. So, I'll leave that in.

Was definitely due to us using version 5.2.0... 🤦‍♂️ Filed an issue as that is the version called out in the docs as well: https://github.com/NuGet/docs.microsoft.com-nuget/issues/3124