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

Package Icon should be able to come from inside the package #352

Open
johnataylor opened this Issue Apr 3, 2015 · 51 comments

Comments

@johnataylor
Member

johnataylor commented Apr 3, 2015

Package Icon should be able to come form inside the package.
It should be possible to put a path in the iconUrl that is relative to the package.

@ChrisSfanos ChrisSfanos added this to the 3.1-Beta milestone Apr 7, 2015

@veleek

This comment has been minimized.

veleek commented Jun 2, 2015

Note: Transferred from issue on CodePlex - Packages containing Icon and License

Packages containing Icon and License

Problem Statement

Currently, the NuGet schema only allows the package author to specify the LicenseUrl and IconUrl as external URLs for the license and icon respectively. This puts the burden on the developer to find a place to host those files and update the NuSpec with their locations.

Solution

The NuGet Gallery should support unpacking a license and icon within the package folder both explicitly specified and by convention.

Details

Convention Based Approach

The convention based approach is very simple.

File Description
Icon.png An Icon.png at the root of the package will be unpacked and used as the icon for the package
License.txt A license.txt file at the root of the package will be used as the license

Explicit Reference

Alternatively, we can support explicit references using the file:// prefixed URLs relative to the package root. For example:

<iconUrl>file://content/images/nugetIcon.png</iconUrl>

This grabs the file from the content/images folder of the package and hosts it on the server. This is useful in cases where the icon is also hosted.

Device Specific Icon Images

There a multiple places where icons are displayed.

  • NuGet Dialog (32x32)
  • NuGet.org packages list (50x50)
  • NuGet.org package details page (100x100)

We should consider a convention that allows developers to include an icon specific to each of these devices.

Proposal

Use a file extension for each.

  • Icon_dialog.png (32x32)
  • Icon_list.png (50x50)
  • Icon_details.png (100x100)
@csharpfritz

This comment has been minimized.

Member

csharpfritz commented Sep 28, 2015

We see some customers unable to view icons as the web location of the icon image is blocked by their corporate firewall. Possible solution could be to download and serve the images from NuGet.org as a resource at the same URI as the package when an image media type is requested

@maartenba

This comment has been minimized.

Contributor

maartenba commented Sep 28, 2015

@xavierdecoster xavierdecoster changed the title from Package Icon should be able to come form inside the package to Package Icon should be able to come from inside the package Oct 9, 2015

@xavierdecoster

This comment has been minimized.

Member

xavierdecoster commented Oct 9, 2015

To support an embedded package icon in the nupkg, it seems we only need the 32x32 icon. All other images are only served on the gallery and don't need to be in the package.

As this issue is about supporting an embedded icon in the package, it sounds to me that a convention-based icon.png in the package root could be sufficient?
IconUrl is an optional nuspec element, so not sure if we should make it required to support an embedded icon.
Perhaps we should simply fallback from IconUrl to icon.png (if present) to default nuget icon?

@maartenba

This comment has been minimized.

Contributor

maartenba commented Oct 9, 2015

That's an entirely different discussion. All we need is package icon proxying over www.nuget.org.

@xavierdecoster

This comment has been minimized.

Member

xavierdecoster commented Oct 9, 2015

Nope, your discussion is here: NuGet/NuGetGallery#2613
Related,
but not the topic of this client issue 😊
So, on topic, how about this client issue?

@yishaigalatzer

This comment has been minimized.

yishaigalatzer commented Nov 25, 2015

Proposal

Use a file extension for each.

Icon_dialog.png (32x32)
Icon_list.png (50x50)
Icon_details.png (100x100)

We should specify the size in the icon name instead

icon32x32.png
icon50x50.png
icon100x100.png

We need to discuss if we require all of them, can they be updated later etc.

@Allann

This comment has been minimized.

Allann commented Feb 23, 2016

I'd prefer to see the scale convention used by win8 manifests used here too. this would mean that brand images can be re-used and we don't need to create even more files named differently.

@hesantan

This comment has been minimized.

hesantan commented Jun 1, 2016

We should specify the size in the icon name instead

icon32x32.png
icon50x50.png
icon100x100.png

We need to discuss if we require all of them, can they be updated later etc.

I think that just requiring the biggest required size is enough. That image can be downsized to all the other necessary resolutions.

@ghuntley

This comment has been minimized.

ghuntley commented Aug 19, 2016

  • Yes please, this would be fantastic.
  • Please only require single size (biggest) and do your conversion using NuGet.exe at the pack stage which then creates the needed sizes for display in the NuGet.org gallery and Visual Studio/Xamarin Studio which uses this by default.
  • By using the embedded image by default in Visual Studio/Xamarin Studio it prevents network profiling/leakage of which packages a company/developer uses. Fallback to ImageUrl if the embedded image is not found (provides legacy transition path)
  • Potentially use the most excellent library ImageResizer.NET within NuGet.exe?
@ohadschn

This comment has been minimized.

ohadschn commented Oct 16, 2016

Downsizing is suboptimal. There is no escape from separate files if you want decent quality. Even the official purple ".NET" icon doesn't look that great in its downsized 32x32 form.

Having the option to only provide one size and have the rest downsized is great, as long as there's a way to provide files for the different sizes separately.

@adamralph

This comment has been minimized.

adamralph commented Nov 16, 2016

The image in package details on nuget.org is 128x128, not 100x100.

image

Although, in order to have that showing with full fidelity on a high DPI display, you need a larger image. E.g. I run my display scaled at 200%. Notice the difference between a 128x128 image and a 256x256 image, even though the dimensions within the page are still 128x128:

image

For this reason, I am gradually changing all my NuGet packages to have 256x256 logos instead of 128x128. Of course, that only solves this precisely for high DPI scaling at 200%, and any other level of scaling, e.g. 125%, 150% or greater than 200% will still be suboptimal. One thing that would solve all these issues, forever, is SVG support, since that eliminates the need for image scaling entirely.

BTW - the dimensions in the package list are 48x48, not 50x50:

image

@SimonCropp

This comment has been minimized.

SimonCropp commented Nov 16, 2016

+1 for svg as the default

@angamuthu-g

This comment has been minimized.

angamuthu-g commented Jan 2, 2017

hi,
I'm facing issue while creating windows installer. this is the issue i'm getting

electron-windows-installer:spawn Spawning mono /var/www/html/Extraction/node_modules/electron-winstaller/vendor/nuget.exe pack /tmp/si-11702-14710-a00hji.hot6ueg66r/app.nuspec -BasePath dist/app-win32-ia32 -OutputDirectory /tmp/si-11702-14710-a00hji.hot6ueg66r -NoDefaultExcludes +3ms
No dice: Failed with exit code: 1
Output:

Unhandled Exception: System.TypeLoadException: Could not load type 'NuGet.Program' from assembly 'NuGet, Version=2.8.50926.602, Culture=neutral, PublicKeyToken=null'.
[ERROR] FATAL UNHANDLED EXCEPTION: System.TypeLoadException: Could not load type 'NuGet.Program' from assembly 'NuGet, Version=2.8.50926.602, Culture=neutral, PublicKeyToken=null'.
WARNING: The runtime version supported by this application is unavailable.
Using default runtime: v2.0.50727

@markusschaber

This comment has been minimized.

markusschaber commented Aug 1, 2018

Just a question from one jumping in here late: Why is SVG not in the current spec, only PNG and JPG?

@PathogenDavid

This comment has been minimized.

PathogenDavid commented Aug 1, 2018

@karann-msft

Spec looks great, thanks for pushing this issue through!


@StingyJack

that spec is missing "Browse from a private repository". We have a private repo from running Nuget.Server.

I would assume that private NuGet servers (MyGet, NuGet.Server, Nexus, Chocolatey, etc) would all work the same as NuGet.org


@markusschaber

Why is SVG not in the current spec, only PNG and JPG?

I would suspect because SVG is non-trivial for non-web clients like Visual Studio to render.

@StingyJack

This comment has been minimized.

Contributor

StingyJack commented Aug 1, 2018

I would assume that private NuGet servers (MyGet, NuGet.Server, Nexus, Chocolatey, etc) would all work the same as NuGet.org

So many features that I need from nuget are removed with every release (ps1, tt, mutable content, 4 part version numbering, consistent restore, etc). With this omission in the spec I could reasonably assume non-nuget.org repos will go the same way, and the omission being the only notice about it.

@adamralph

This comment has been minimized.

adamralph commented Aug 1, 2018

Would it be possible to default to using icon.{known extension} if present, to avoid having to boilerplate <PackageIcon source="icon.png", target=""/> across the ecosystem?

@caioproiete

This comment has been minimized.

caioproiete commented Aug 1, 2018

@karann-msft - spec looks great!

Would be nice if SVG was supported - with a fallback to PNG or JPG. i.e. Would be nice if developers could include multiple image types in the same package, for example, SVG and/or PNG and/or JPG and the client can decide which one to render, based on what it supports... If client can't render SVG, then fallback to PNG or JPG, etc.

e.g. in the package root:
/icon.svg
/icon.png
/icon.jpg

@karann-msft

This comment has been minimized.

Contributor

karann-msft commented Aug 2, 2018

@adamralph Great feedback and we considered doing that. Since multiple extensions are supported, we didn't want to cycle through to check for the presence of one and then for the next. This may have perf implications for clients.

@caioproiete @markusschaber Supporting SVG would be great and is definitely on our backlog. At present, supporting SVG will break many NuGet clients that do not support SVG. We could change the protocol that allows the client to express the supported types for icons to the server and server then provides the right format for package icons to the requesting client. But then this felt like something that's beyond the MVP and can be taken up later.

@caioproiete

This comment has been minimized.

caioproiete commented Aug 2, 2018

@karann-msft Great! All makes sense to me... Supporting PNG and/or JPG would be a fantastic start. Just wanted to make sure you guys leave the door open for other formats in future versions!

@markusschaber

This comment has been minimized.

markusschaber commented Aug 2, 2018

@karann-msft I see, thanks.

My first thought was that if we allow a list of ;-separated filenames in the source tag, this cannot be added later in a backwards compatible way if clients don't know how to parse that kind of list.

But then I realized that we could add a separate attribute, e. G. highsource="icon.svg", to the PackageIcon tag (in analogy to <img lowsrc="..." in HTML), or nested tags, so capable clients can display the high resolution image, while non-capable clients will use the pixel version by default...

Btw, if the protocol is HTTP based, there's already the http Accept header which could serve the purpose of letting the client decide which image types it supports.

@adamralph

This comment has been minimized.

adamralph commented Aug 2, 2018

@karann-msft why not bake in the default icon at packaging time? Effectively, when creating the package, the presence of icon.{known extension} in the project root would result in the same packaging action as would the presence of <PackageIcon source="icon.png", target=""/>.

@markusschaber

This comment has been minimized.

markusschaber commented Aug 2, 2018

@adamralph As far as I can see, this is not backwards compatible (there might be projects which happen to have an icon.png in the root directory for other reasons - I'm not sure whether it's a good idea to start exposing this image silently.

Citing "The Zen of Python":

Explicit is better than implicit.

@adamralph

This comment has been minimized.

adamralph commented Aug 2, 2018

@markusschaber

As far as I can see, this is not backwards compatible (there might be projects which happen to have an icon.png in the root directory for other reasons - I'm not sure whether it's a good idea to start exposing this image silently.

I was waiting for someone to play this card. 😉

I agree that it is not backwards compatible, but I expect the number of affected packages to be vanishingly small, and it would only cause a minor, cosmetic, problem. The cost of fixing those packages will be much less than the cost of having to boilerplate <PackageIcon source="icon.png", target=""/> across almost very package in the entire ecosystem.

Explicit is better than implicit.

In some contexts I would strongly disagree. e.g. compare the new "SDK-style" csproj format (sensible, implicit defaults) to the old one (everything explicit).

@karann-msft

This comment has been minimized.

Contributor

karann-msft commented Aug 2, 2018

@adamralph the potential perf impact I cited was for clients to have to cycle through to look for multiple files with different extensions when for example browsing packages from a folder based

@ohadschn

This comment has been minimized.

ohadschn commented Aug 2, 2018

@karann-msft just to nit pick, "cycle through" implies sequentiality, where I don't see any reason for the client not to try all 3 (or whatever number of possible fallbacks) files in parallel...

@adamralph

This comment has been minimized.

adamralph commented Aug 2, 2018

@karann-msft the location of the icon inside the package can still be baked into the package metadata. My suggestion doesn't change that. What I am saying is that when creating the package, the presence of icon.{known extension} in the project folder has the same effect as <PackageIcon source="icon.{known extension}", target=""/> in the csproj file.

@karann-msft

This comment has been minimized.

Contributor

karann-msft commented Aug 7, 2018

@adamralph I now see what you mean. What's your typical package creation workflow? Do you run pack on a folder, nuspec, or a csproj?

@markusschaber

This comment has been minimized.

markusschaber commented Aug 7, 2018

@karann-msft about the perf impact with "cycle through": As the client knows which file types it supports, it can filter by known extensions and choose the first hit, just while parsing the list of icon pathes, long before hitting the network. Also, it's usually only a handful of icons (I don't see any need to add more than 2 - a bitmap and a vector one). So the performance impact is not that serious.

@adamralph

This comment has been minimized.

adamralph commented Aug 7, 2018

@karann-msft I use both nuspec and csproj for creating packages.

@StingyJack

This comment has been minimized.

Contributor

StingyJack commented Aug 7, 2018

These all sound great, but can we get <iconUrl>file://content/images/nugetIcon.png</iconUrl> to work first? That I think is the minimum for me.

@adamralph

This comment has been minimized.

adamralph commented Aug 7, 2018

@StingyJack you'd have less to do if icon.png in the root of your project were used automatically.

@StingyJack

This comment has been minimized.

Contributor

StingyJack commented Aug 7, 2018

True, but someone would have to program that extra bit of automagic instead of just using the existing setting and uri. Either would be less than figuring out all the scaling and image format support. None of these are as important as the other open issues I have.

@karann-msft

This comment has been minimized.

Contributor

karann-msft commented Aug 10, 2018

@markusshaber we'd rather not make package consumers take any perf hit if possible. On the authoring side, you used to specify a url before (and had to find a place to host the image), now it's a file path to an image within the package.

@adamralph would you expect only a specific file name and extension to get picked up from that known location? So only if the image is named icon.png and is placed right next to the csproj or is at the root of the folder you run pack on, it'll get picked up at pack.

@adamralph

This comment has been minimized.

adamralph commented Aug 11, 2018

@karann-msft yes, I would expect only icon.png at the root of the project/folder to be used by default. I guess you could also support icon.jpg or other extensions, but PNG will probaby take care of 99% of cases.

@karann-msft

This comment has been minimized.

Contributor

karann-msft commented Aug 15, 2018

Thanks for the feedback @adamralph, updated the spec.

@adamralph

This comment has been minimized.

adamralph commented Aug 15, 2018

Thanks @karann-msft. The spec looks good.

@StingyJack

This comment has been minimized.

Contributor

StingyJack commented Aug 16, 2018

@karann-msft - spec still only talks about Nuget.org and folder based repos, and even has provisions for nuget.org webmasters to block some images.

Am i reading that support for icons in private nuget repositories based on NuGet.Server is not happening and that this could break them or the packages therein (like the unwanted version normalization)? Or is it not included because private repo's themselves are not going to be supported? Or is it just an oversight?

@josesimoes

This comment has been minimized.

josesimoes commented Oct 12, 2018

Any update on this, please?

@karann-msft

This comment has been minimized.

Contributor

karann-msft commented Oct 16, 2018

@josesimoes we have started working on this. I don't have a hard ETA on this yet.

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