fix(cli): Windows: set exe file icon and PE metadata #6693

Spoonbender · 2020-07-10T02:10:15Z

Windows: set executable icon and PE metadata

Result:

…'t set

caspervonb · 2020-07-10T05:35:21Z

cli/Cargo.toml

+[package.metadata.winres]
+# On Windows, this section defines the metadata that appears in the Deno.exe Portable Executable header
+OriginalFilename = "Deno.exe"
+LegalCopyright = "Copyright © 2018-2020 the Deno authors. All rights reserved. MIT license."


Not sure what's correct here so this is just a side note.

Was poking around on my Windows VM and for Microsoft products (notepad, explorer) etc this doesn't contain a year, it's just "© Microsoft Corporation. All Rights Reserved."

"Copyright: Copyright © 2018-2020 The Deno Authors" has a bit of an echo to it.

Depends on what the deno project prefers really.

PuTTY has: Copyright © 1997 - 2017 Simon Tatham
RealVNC has: Copyright © 2002-2020 RealVNC Ltd.
nodejs has: Copyright Node.js contributors. MIT license.
Wireshark has: Copyright © 2000 Gerald Combs [email protected]
Firefox has: ©Firefox and Mozilla Developers: available under the MPL 2 license.
Chrome has: Copyright 2019 Google LLC. All rights reserved.

I guess I'll remove the "Copyright" and the years and the ©

cli/Cargo.toml

Co-authored-by: Mark Tiedemann <[email protected]>

MarkTiedemann · 2020-07-10T11:48:52Z

When using the Windows dark mode, the dino and the rain aren't white so the icon looks a little bit off:

Spoonbender · 2020-07-10T12:14:28Z

When using the Windows dark mode, the dino and the rain aren't white so the icon looks a little bit off:

Fixed - kept it transparent on the outside and white on the inside

MarkTiedemann · 2020-07-10T13:24:49Z

Fixed - kept it transparent on the outside and white on the inside

Nice!

Minor nitpick: Now, the border has a few white pixels on my machine:

Note that the white pixels only appear when zooming out, e.g. "Large Icon" view. In the smaller "Tiles" view, for example, you can't see them.

Spoonbender · 2020-07-10T14:22:12Z

Fixed - kept it transparent on the outside and white on the inside

Nice!

Minor nitpick: Now, the border has a few white pixels on my machine:

Note that the white pixels only appear when zooming out, e.g. "Large Icon" view. In the smaller "Tiles" view, for example, you can't see them.

Yeah, that is as far as my graphic skills go... maybe we can get one of the deno artists to provide an icon file that works well in light and dark themes?

MarkTiedemann · 2020-07-10T14:41:15Z

@Spoonbender The default language seems to be "Language Neutral". Since Deno is only available in English so far, do you think it makes sense to set the language as well?

Spoonbender · 2020-07-10T16:25:40Z

@Spoonbender The default language seems to be "Language Neutral". Since Deno is only available in English so far, do you think it makes sense to set the language as well?

Yes, because I can't resist the detailitis!

MarkTiedemann · 2020-07-10T21:37:02Z

Yeah, that is as far as my graphic skills go... maybe we can get one of the deno artists to provide an icon file that works well in light and dark themes?

According to https://deno.land/artwork, @kevinkassimo created this logo. Perhaps he can help?

lucacasonato · 2020-07-10T22:00:15Z

Now, the border has a few white pixels on my machine

@MarkTiedemann Image with infill, without the white border:

MarkTiedemann · 2020-07-10T22:07:23Z

@lucacasonato Can you export the image as an .ico file for @Spoonbender?

lucacasonato · 2020-07-10T22:10:09Z

ICO file https://drive.google.com/file/d/1AGuc2yUvkiWR0UhVhDyTRv5vMqOdRJ0M/view?usp=sharing

… by Luca Casonato

Spoonbender · 2020-07-11T01:44:47Z

Thanks @lucacasonato
This ICO file indeed solves the white lining.
I think it lacks the bigger sizes (shows small size even when displaying "Large" or "Extra Large" icons), though it could be just some issue on my machine.
Anyhow, committing this version for now.

MarkTiedemann · 2020-07-11T11:18:57Z

I think it lacks the bigger sizes (shows small size even when displaying "Large" or "Extra Large" icons), though it could be just some issue on my machine.

Can confirm that this is an issue on my machine as well.

lucacasonato · 2020-07-11T11:29:07Z

Huh that is weird. I exported it at the largest possible .ico size of 255x255. I'll take a look

ry · 2020-07-14T18:27:50Z

I would lean towards just landing this. We can improve it in the future. Is there more work to be done here?

piscisaureus · 2020-07-14T18:36:07Z

The white border is a conversion artefact I suspect - some tool was used to make the ICO and it dropped the alpha (transparency) channel.

But let's land it as-is. We can always improve it further, PR numbers are cheap :)

lucacasonato · 2020-07-14T18:39:11Z

Can you try this .ico as a final attempt before landing? https://drive.google.com/file/d/1-10fAAErCQWQ-yVTCrcIXZEbeGMjDM-w/view?usp=sharing

MarkTiedemann · 2020-07-14T19:14:23Z

Can you try this .ico as a final attempt before landing? https://drive.google.com/file/d/1-10fAAErCQWQ-yVTCrcIXZEbeGMjDM-w/view?usp=sharing

That one looks perfect to me, even with "Extra large icons". :)

Spoonbender · 2020-07-15T06:51:09Z

Can you try this .ico as a final attempt before landing? https://drive.google.com/file/d/1-10fAAErCQWQ-yVTCrcIXZEbeGMjDM-w/view?usp=sharing

That one looks perfect to me, even with "Extra large icons". :)

Looks great! I committed and pushed this new icon, I guess we can move forward now @piscisaureus

piscisaureus · 2020-07-15T19:46:52Z

cli/build.rs

+
+#[cfg(target_os = "windows")]
+fn set_binary_metadata() {
+ let mut res = winres::WindowsResource::new();


I really dig that we can pull this in as a cargo dep, rather than figuring out how to get rc.exe installed on every user's machine. I wish we could do that with our ninja/gn/python build dependencies too.

Read through the implementation a couple of days ago, it doesn't bring it's rc.exe. It has to be in PATH which is the case if you have the Windows SDK.

Definitively agree that it is is clean and convenient how this works out tho!

This is why my original implementation did not treat winres compilation errors as fatal - so users without rc.exe can keep building Deno on their machines. I guess we can always improve this in the future, perhaps by fetching rc.exe on-demand, if the need arises.

piscisaureus

LGTM

Spoonbender added 3 commits July 10, 2020 05:05

Windows: set executable icon and PE metadata

e710861

do not fail build if the toolchain for creating Windows resources isn…

54d0962

…'t set

Fix non-Windows builds

1a22597

caspervonb reviewed Jul 10, 2020

View reviewed changes

MarkTiedemann reviewed Jul 10, 2020

View reviewed changes

cli/Cargo.toml Outdated Show resolved Hide resolved

MarkTiedemann reviewed Jul 10, 2020

View reviewed changes

cli/Cargo.toml Outdated Show resolved Hide resolved

Spoonbender and others added 2 commits July 10, 2020 13:44

Update cli/Cargo.toml - deno.exe in lowercase

634582a

Co-authored-by: Mark Tiedemann <[email protected]>

Update cli/Cargo.toml - deno.exe in lowercase

720db7c

Co-authored-by: Mark Tiedemann <[email protected]>

Spoonbender added 2 commits July 10, 2020 15:01

Update copyright text

0838b08

Fix icon colors - transparent outside the circle, white inside of it.

71d8de3

Set the language in deno.exe PE metadata as English (United States)

fc34e08

ry requested a review from piscisaureus July 10, 2020 16:37

Fix deno icon surrounding white line: replace with ICO file generated…

8463783

… by Luca Casonato

bartlomieju added build build system or continuous integration related cli related to cli/ dir windows Related to Windows platform labels Jul 14, 2020

Fixing deno icon size issues

ebd8e4c

piscisaureus added 2 commits July 15, 2020 18:26

Merge remote-tracking branch 'origin/master' into fix-6593

65cc730

Edit for brevity

dffd625

piscisaureus reviewed Jul 15, 2020

View reviewed changes

piscisaureus approved these changes Jul 15, 2020

View reviewed changes

piscisaureus merged commit b0f2bd4 into denoland:master Jul 15, 2020

MarkTiedemann mentioned this pull request Nov 2, 2020

New .ico files #8196

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(cli): Windows: set exe file icon and PE metadata #6693

fix(cli): Windows: set exe file icon and PE metadata #6693

Spoonbender commented Jul 10, 2020

caspervonb Jul 10, 2020 •

edited

Loading

Spoonbender Jul 10, 2020 •

edited

Loading

MarkTiedemann commented Jul 10, 2020

Spoonbender commented Jul 10, 2020

MarkTiedemann commented Jul 10, 2020 •

edited

Loading

Spoonbender commented Jul 10, 2020

MarkTiedemann commented Jul 10, 2020

Spoonbender commented Jul 10, 2020

MarkTiedemann commented Jul 10, 2020

lucacasonato commented Jul 10, 2020

MarkTiedemann commented Jul 10, 2020

lucacasonato commented Jul 10, 2020

Spoonbender commented Jul 11, 2020

MarkTiedemann commented Jul 11, 2020

lucacasonato commented Jul 11, 2020

ry commented Jul 14, 2020

piscisaureus commented Jul 14, 2020

lucacasonato commented Jul 14, 2020

MarkTiedemann commented Jul 14, 2020

Spoonbender commented Jul 15, 2020

piscisaureus Jul 15, 2020

caspervonb Jul 16, 2020

Spoonbender Jul 16, 2020

piscisaureus left a comment

fix(cli): Windows: set exe file icon and PE metadata #6693

fix(cli): Windows: set exe file icon and PE metadata #6693

Conversation

Spoonbender commented Jul 10, 2020

caspervonb Jul 10, 2020 • edited Loading

Choose a reason for hiding this comment

Spoonbender Jul 10, 2020 • edited Loading

Choose a reason for hiding this comment

MarkTiedemann commented Jul 10, 2020

Spoonbender commented Jul 10, 2020

MarkTiedemann commented Jul 10, 2020 • edited Loading

Spoonbender commented Jul 10, 2020

MarkTiedemann commented Jul 10, 2020

Spoonbender commented Jul 10, 2020

MarkTiedemann commented Jul 10, 2020

lucacasonato commented Jul 10, 2020

MarkTiedemann commented Jul 10, 2020

lucacasonato commented Jul 10, 2020

Spoonbender commented Jul 11, 2020

MarkTiedemann commented Jul 11, 2020

lucacasonato commented Jul 11, 2020

ry commented Jul 14, 2020

piscisaureus commented Jul 14, 2020

lucacasonato commented Jul 14, 2020

MarkTiedemann commented Jul 14, 2020

Spoonbender commented Jul 15, 2020

piscisaureus Jul 15, 2020

Choose a reason for hiding this comment

caspervonb Jul 16, 2020

Choose a reason for hiding this comment

Spoonbender Jul 16, 2020

Choose a reason for hiding this comment

piscisaureus left a comment

Choose a reason for hiding this comment

caspervonb Jul 10, 2020 •

edited

Loading

Spoonbender Jul 10, 2020 •

edited

Loading

MarkTiedemann commented Jul 10, 2020 •

edited

Loading