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

qt: Make bitcoin.ico non-executable #18650

Merged
merged 1 commit into from
Apr 16, 2020

Conversation

practicalswift
Copy link
Contributor

@practicalswift practicalswift commented Apr 15, 2020

Make bitcoin.ico non-executable.

No need to execute icons and having +x bits laying around breaks find … -executable :)

Before this patch:

$ find $(git ls-files -- ":(exclude)*.sh" ":(exclude)*.py") -executable
ci/retry/retry
contrib/macdeploy/macdeployqtplus
depends/config.guess
depends/config.sub
src/qt/res/icons/bitcoin.ico
src/secp256k1/src/modules/recovery/main_impl.h

After this patch:

$ find $(git ls-files -- ":(exclude)*.sh" ":(exclude)*.py") -executable
ci/retry/retry
contrib/macdeploy/macdeployqtplus
depends/config.guess
depends/config.sub
src/secp256k1/src/modules/recovery/main_impl.h

FWIW:

$ file $(find $(git ls-files -- ":(exclude)*.sh" ":(exclude)*.py") -executable)
ci/retry/retry:                                 Bourne-Again shell script, UTF-8 Unicode text executable
contrib/macdeploy/macdeployqtplus:              Python script, ASCII text executable
depends/config.guess:                           POSIX shell script, ASCII text executable
depends/config.sub:                             POSIX shell script, ASCII text executable
src/qt/res/icons/bitcoin.ico:                   MS Windows icon resource - 10 icons, 48x48, 16 colors, 4 bits/pixel, 32x32, 16 colors, 4 bits/pixel
src/secp256k1/src/modules/recovery/main_impl.h: C source, ASCII text

@fanquake fanquake added the GUI label Apr 15, 2020
real-or-random added a commit to bitcoin-core/secp256k1 that referenced this pull request Apr 15, 2020
ffef45c Make recovery/main_impl.h non-executable (Elichai Turkel)

Pull request description:

  Opened it because of bitcoin/bitcoin#18650

  I assume it doesn't matter?
  But because I'm not sure I preferred to open this then let the info go away in case someone thinks it does matter.

ACKs for top commit:
  real-or-random:
    ACK ffef45c

Tree-SHA512: 381aed7f99fd739f4059b2e526ba9cd75b55b4fa86c9cc040fbf6b93055ce8558cc69c4ccf5d8a422b17022ca376cc9a608cf5af8d5841d62c5953f40825f5ff
@DrahtBot
Copy link
Contributor

Gitian builds

File commit 20c0e2e
(master)
commit 68be658
(master and this pull)
bitcoin-0.20.99-aarch64-linux-gnu-debug.tar.gz 3eb305a935b01faa... 5db3eae0ce8424e2...
bitcoin-0.20.99-aarch64-linux-gnu.tar.gz 482baf6ab21e47c1... 782965b57d00af74...
bitcoin-0.20.99-arm-linux-gnueabihf-debug.tar.gz 65759a93baafaa64... 4047c89d87ba716c...
bitcoin-0.20.99-arm-linux-gnueabihf.tar.gz bcce5bf08fe59a89... 469409f58256c174...
bitcoin-0.20.99-osx-unsigned.dmg 65ed8a9604386f5c... 608a719b6653e223...
bitcoin-0.20.99-osx64.tar.gz afb22c784e21dac1... 781d5379dc30f963...
bitcoin-0.20.99-riscv64-linux-gnu-debug.tar.gz 25e307d8a86e31a6... 73d69bb64b92bd91...
bitcoin-0.20.99-riscv64-linux-gnu.tar.gz fece52653d568ecf... 80957e5cdfe2b590...
bitcoin-0.20.99-win64-debug.zip 78e378d8c951f154... 734ea2ec8192fa65...
bitcoin-0.20.99-win64-setup-unsigned.exe dd561c62d1f48529... c6a1e3b706552f07...
bitcoin-0.20.99-win64.zip ebbb3789652ab494... b1e5756c711c7f88...
bitcoin-0.20.99-x86_64-linux-gnu-debug.tar.gz 676ef190159684e0... 50cd2d668c34046e...
bitcoin-0.20.99-x86_64-linux-gnu.tar.gz 9a46151166b42b4c... 7d4448f92dc988f9...
bitcoin-0.20.99.tar.gz 9c25e35a255eb09b... 56641c5377c73730...
bitcoin-core-linux-0.21-res.yml 95df04566296a268... febba66b2fb94506...
bitcoin-core-osx-0.21-res.yml 807dd89331a9a8b0... 1303d7e72e1073d6...
bitcoin-core-win-0.21-res.yml 37e9dd0ba8ec184b... caef7355aaf73b63...
linux-build.log dc381fa997aba9d1... 85e8f10139129f63...
osx-build.log 3cd16623bae4d730... 12887d2d8c94dae7...
win-build.log ed50b3f1b5cc921e... 0f7b9b34c45233a8...
bitcoin-core-linux-0.21-res.yml.diff b98d9503b5bbb0f2...
bitcoin-core-osx-0.21-res.yml.diff b11137cd55b4dffb...
bitcoin-core-win-0.21-res.yml.diff 76a8d6b44c9f4a89...
linux-build.log.diff cae8164936a9a322...
osx-build.log.diff d56bfde523e6eb1c...
win-build.log.diff 438b91dea8169553...

@practicalswift
Copy link
Contributor Author

The src/secp256k1/src/modules/recovery/main_impl.h case has now been fixed upstreams thanks to @elichai -- see bitcoin-core/secp256k1#740 :)

@maflcko
Copy link
Member

maflcko commented Apr 16, 2020

ACK a95af77 gitian build finished, so it doesn't look like the icon used in Windows resource files needs to be executable. Though, I didn't read the documentation.

Copy link
Member

@jonatack jonatack left a comment

Choose a reason for hiding this comment

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

Nice find.

ACK a95af77

@maflcko
Copy link
Member

maflcko commented Apr 16, 2020

The testnet icon doesn't have +x either, so I think this is good to merge.

$ ll ./src/qt/res/icons/bitcoin*.ico
-rwxrwxr-x. 1 marco marco 57964 Apr 14 15:18 ./src/qt/res/icons/bitcoin.ico
-rw-rw-r--. 1 marco marco 57251 Apr 14 15:18 ./src/qt/res/icons/bitcoin_testnet.ico

@maflcko maflcko merged commit 79b0459 into bitcoin:master Apr 16, 2020
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Apr 16, 2020
a95af77 qt: Make bitcoin.ico non-executable (practicalswift)

Pull request description:

  Make `bitcoin.ico` non-executable.

  No need to execute icons and having +x bits laying around breaks `find … -executable` :)

  Before this patch:

  ```sh
  $ find $(git ls-files -- ":(exclude)*.sh" ":(exclude)*.py") -executable
  ci/retry/retry
  contrib/macdeploy/macdeployqtplus
  depends/config.guess
  depends/config.sub
  src/qt/res/icons/bitcoin.ico
  src/secp256k1/src/modules/recovery/main_impl.h
  ```

  After this patch:

  ```sh
  $ find $(git ls-files -- ":(exclude)*.sh" ":(exclude)*.py") -executable
  ci/retry/retry
  contrib/macdeploy/macdeployqtplus
  depends/config.guess
  depends/config.sub
  src/secp256k1/src/modules/recovery/main_impl.h
  ```

  FWIW:

  ```
  $ file $(find $(git ls-files -- ":(exclude)*.sh" ":(exclude)*.py") -executable)
  ci/retry/retry:                                 Bourne-Again shell script, UTF-8 Unicode text executable
  contrib/macdeploy/macdeployqtplus:              Python script, ASCII text executable
  depends/config.guess:                           POSIX shell script, ASCII text executable
  depends/config.sub:                             POSIX shell script, ASCII text executable
  src/qt/res/icons/bitcoin.ico:                   MS Windows icon resource - 10 icons, 48x48, 16 colors, 4 bits/pixel, 32x32, 16 colors, 4 bits/pixel
  src/secp256k1/src/modules/recovery/main_impl.h: C source, ASCII text
  ```

ACKs for top commit:
  MarcoFalke:
    ACK a95af77 gitian build finished, so it doesn't look like the icon used in Windows resource files needs to be executable. Though, I didn't read the documentation.
  jonatack:
    ACK a95af77

Tree-SHA512: ecf9154077824ae4c274b4341e985797f3648c0cb0c31cb25ce382163b923a3acbc7048683720be4ae3663501801129cd0f48c441a36f049cc304ebe9f30994e
@practicalswift practicalswift deleted the bitcoin.ico-executable-flag branch April 10, 2021 19:40
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants