Skip to content

add ICON from crates.io to the cargo executable on windows#16056

Closed
ShadowDara wants to merge 1 commit into
rust-lang:masterfrom
ShadowDara:master
Closed

add ICON from crates.io to the cargo executable on windows#16056
ShadowDara wants to merge 1 commit into
rust-lang:masterfrom
ShadowDara:master

Conversation

@ShadowDara

Copy link
Copy Markdown

Thanks for the pull request 🎉!
Please read the contribution guide: https://doc.crates.io/contrib/.

What does this PR try to resolve?

The Change add a software icon to the cargo executable on windows

How to test and review this PR?

The Code changes the standard Icon from windows to the Icon from crates.io

@rustbot rustbot added A-infrastructure Area: infrastructure around the cargo repo, ci, releases, etc. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 6, 2025
@rustbot

rustbot commented Oct 6, 2025

Copy link
Copy Markdown
Collaborator

r? @weihanglo

rustbot has assigned @weihanglo.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@weihanglo weihanglo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the code contribution.

Per our contributing guide, we encourage to discuss first before jumping straight to a pull request.

If you're willing to, please open a new issue for discussions. I'll mark this as draft for now.

@weihanglo weihanglo marked this pull request as draft October 6, 2025 18:23
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 6, 2025
@ShadowDara

Copy link
Copy Markdown
Author

Okay i am sorry, i though this just a little change so ... I'm sorry

@ShadowDara

Copy link
Copy Markdown
Author

Issue as Feature Request or Inspiring Idea? I'll Inspiring Idea for now, because its not really a Feature

@ShadowDara

Copy link
Copy Markdown
Author

@joshtriplett

Copy link
Copy Markdown
Member

Some preliminary feedback (leaving aside for the moment the question of whether we want to do this): this should be conditional on the target being windows, not the build architecture being windows, so that you get the icon even when cross-compiling to windows. Look at windows_manifest() for the way the conditionals are structured.

@mathstuf

mathstuf commented Oct 7, 2025

Copy link
Copy Markdown
Contributor

Additionally, what is the license of the icon? (I know I can follow the link in the code, but I feel like it needs to be more explicit here.)

@CodesInChaos

Copy link
Copy Markdown

winres appears to be unmaintained and have known bugs. It also adds a second version of the toml crate. Perhaps the winresource fork would be a better choice?

Or perhaps a pre-built res-file to avoid this dependency (and the dependency on rc.exe)?

@ShadowDara

Copy link
Copy Markdown
Author

Oh i'm sorry thats a lot of stuff i didn't knew about before

@Aloso

Aloso commented Oct 11, 2025

Copy link
Copy Markdown

Additionally, what is the license of the icon?

CC-BY. See the trademark policy.

It's best to ask the Rust Foundation for permission, but I don't see a reason why they would be against it.

@rustbot

rustbot commented Oct 22, 2025

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (possibly #16137) made this pull request unmergeable. Please resolve the merge conflicts.

@rustbot rustbot added the S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. label Oct 22, 2025
@ShadowDara ShadowDara closed this by deleting the head repository Oct 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-infrastructure Area: infrastructure around the cargo repo, ci, releases, etc. S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants