Skip to content
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

Build ico file from scrcpy icon.xpm, package ico file in release #6

Merged
merged 4 commits into from
Sep 18, 2021

Conversation

amoshydra
Copy link
Contributor

@amoshydra amoshydra commented Aug 2, 2021

Summary

Generate ico file from https://github.com/Genymobile/scrcpy/blob/v1.18/app/src/icon.xpm during build time.

The generated ico file will be stored in this repository

png2ico dependency has been removed from the list as the ico file will be packaged in the .nupkg

An additional script scripts/generate-icon.ps1 is created to do the icon generation. This script depends on:

Related issue

This PR aims to resolve checksum mismatch issue with the current android icon fetched from iconfinder

Downloading android
  from 'https://www.iconfinder.com/icons/317758/download/png/128'

Download of android.png (-1 B) completed.
Error - hashes do not match. Actual value was 'D0B659EB903B08B9A67D97257FC10B7C3269581A913E9F72B20D6E890C01D988'.
ERROR: Checksum for 'C:\Users\username\AppData\Local\Temp\chocolatey\android.png' did not meet 'B2FE913ED202C908D10ACD1D602CCDB24E2E79F3009570E2CADAF22694FFA4DA' for checksum type 'sha256'. Consider passing the actual checksums through with --checksum --checksum64 once you validate the checksums are appropriate. A less secure option is to pass --ignore-checksums if necessary.
The install of scrcpy was NOT successful.
Error while running 'C:\ProgramData\chocolatey\lib\scrcpy\tools\chocolateyinstall.ps1'.
 See log for details.

Related conversation / issues:

How is this tested

Tested installation

choco pack
choco install scrcpy -s . -y

Output:
image

Icons in start menu and file explorer

image

@n3rd4i
Copy link
Owner

n3rd4i commented Aug 4, 2021

This looks like good solution, however, the problem you have when incorporating files is that you either:

  1. have to be the author of those files
  2. have to ask permission to use the file from original sources

This is what Chocolatey admins told me, hence downloading it from internet its bypassing this rule.

Aside from that [legal issue] I see no problem (we could already merge).

@amoshydra
Copy link
Contributor Author

This looks like good solution, however, the problem you have when incorporating files is that you either:

have to be the author of those files
have to ask permission to use the file from original sources

I see. Thanks for the explanation. I've overlooked on this matter.

genymobile/[email protected] is licensed under Apache License 2.0. If we are going to package the icon.ico (a "Derivative Works"), we will also need to include a copy of the license to fulfill Apache License 2.0's "License and copyright notice" requirement.

However I am not sure if this is acceptable for Chocolatey admins.


Can't you download this source file on-the-fly and use it in installing ?

Technically possible, however, in my opinion it doesn't feel right to install additional dependencies into user's system.


But this conversation also gave me a new idea to address the legal issue.

I propose we:

  1. During build step:
    1. include Genymobile/scrcpy as a submodule into this repository. This way we preserve Genymobile/scrcpy's license file and source file.
    2. check in the generated ico file into this repository. This allow us to host the ico file on GitHub. The .nupkg will not contain this ico file.
  2. During install step:
    1. download the generated ico file from this repository

A proof of concept is available at #7

@n3rd4i n3rd4i merged commit f02e668 into n3rd4i:master Sep 18, 2021
@amoshydra amoshydra deleted the package-app-icon-locally branch October 2, 2021 04:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants