Install icon & .desktop file to XDG data #15068

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.

Sign up for GitHub

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

Jump to bottom

Closed

luke-jr wants to merge 9 commits into bitcoin:master from luke-jr:xdg

Member

luke-jr commented Dec 31, 2018

With #13809 (packaging split off), we lost the .desktop and .protocol files.

This PR restores them, and puts them in their proper places as part of make install. (Systems without XDG can be configured --without-xdg to avoid this, and the macOS/Windows gitian descriptors are updated to use it.)

Note that:

bitcoin128.svg needs to be named such because (for some reason) bitcoin128 has become the de facto standard XDG icon name for the generic Bitcoin icon.
org.bitcoin.bitcoin-qt.desktop is required to match QAPP_ORG_DOMAIN, which we have set to "bitcoin.org" for some reason. (Changing that is out of scope for this PR)

fanquake added the Build system label

Member

maflcko commented Dec 31, 2018

Build fails with

make[1]: *** No rule to make target 'src/qt/res/src/bitcoin128.svg', needed by 'all-am'.  Stop.

Member

hebasto commented Dec 31, 2018

Could the Comment field in the .desktop file be updated with more languages?

sharidz313 approved these changes

View reviewed changes

Member Author

luke-jr commented Dec 31, 2018

@MarcoFalke Why? It's part of the git branch...

sharidz313 approved these changes

View reviewed changes

sharidz313 approved these changes

View reviewed changes

sharidz313 approved these changes

View reviewed changes

sharidz313 approved these changes

View reviewed changes

bitcoin deleted a comment from sharidz313

Member

laanwj commented Jan 2, 2019

Concept ACK, this the better way to do it IMO than installing it with the packaging.

Member Author

luke-jr commented Jan 3, 2019

@hebasto After this PR, I don't see why not.

Member

hebasto commented Jan 3, 2019

Concept ACK.

Member

laanwj commented Jan 8, 2019

Regarding the build system issue, it looks like the icon is not part of make dist? Likely it needs to be added to another makefile variable.

Contributor

jonasschnelli commented Jan 8, 2019

Concept ACK

bitcoin128.svg needs to be named such because (for some reason) bitcoin128 has become the de facto standard XDG icon name for the generic Bitcoin icon

Do we want to set the standard Bitcoin icon? IMO Bitcoin Core is a specific application and not sure if that icon should be the default "Bitcoin Icon".

Member Author

luke-jr commented Jan 8, 2019

There we go, Travis is happy now. Shall I squash?

Member Author

luke-jr commented Jan 8, 2019

@jonasschnelli Bitcoin Core doesn't have its own icon at this time - we've always used the generic Bitcoin icon. When/if it does, then the PR adding it can change to use that instead...

Member

hebasto commented Jan 9, 2019

The file name org.bitcoin.bitcoin-qt.desktop does not follow Desktop Entry Specification:

The name of the desktop entry should follow the "reverse DNS" convention: it should start with a reversed DNS domain name controlled by the author of the application, in lower case. The domain name should be followed by the name of the application, which is conventionally written with words run together and initial capital letters (CamelCase). For example, if the owner of example.org writes "Foo Viewer", they might choose the name org.example.FooViewer, resulting in a file named org.example.FooViewer.desktop.

Well-known names containing the dash are allowed but not recommended, because the dash is not allowed in some related uses of reversed DNS names, such as D-Bus object paths and interface names, and Flatpak app IDs. If the author's domain name contains a dash, replacing it with an underscore is recommended: this cannot cause ambiguity, because underscores are not allowed in DNS domain names.

hebasto requested changes

View reviewed changes

share/qt/org.bitcoin.bitcoin-qt.desktop.in

+              Encoding=UTF-8
+              Version=1.0
+              Name=@PACKAGE_NAME@
+              GenericName=Bitcoin full node@AND_WALLET@

Member

hebasto Jan 9, 2019

Localized [Comment] strings imply localized [GenericName] strings as well :)

share/qt/org.bitcoin.bitcoin-qt.desktop.in

+              Type=Application
+              Icon=bitcoin128
+              MimeType=x-scheme-handler/bitcoin;
+              Categories=Office;Finance;P2P;Network;Qt;

Member

hebasto Jan 9, 2019

Is Office a relevant category?

share/qt/org.bitcoin.bitcoin-qt.desktop.in

+              Comment[fr]=Bitcoin, monnaie virtuelle cryptographique pair à pair
+              Comment[tr]=Bitcoin, eşten eşe kriptografik sanal para birimi
+              TryExec=bitcoin-qt
+              Exec=bitcoin-qt %u

Member

hebasto Jan 9, 2019

%u will not be recognized by bitcoin-qt:

%u - A single URL. Local files may either be passed as file: URLs or as file path.

Member Author

luke-jr Jan 10, 2019

Why not? We expect URIs.

share/qt/org.bitcoin.bitcoin-qt.desktop.in

		@@ -0,0 +1,17 @@
		[Desktop Entry]

Member

hebasto Jan 9, 2019

Why not use the same key order as described in Desktop Entry Specification?

Member Author

luke-jr Jan 10, 2019

This file predates the PR. It's just being restored at this time.

share/qt/org.bitcoin.bitcoin-qt.desktop.in

		@@ -0,0 +1,17 @@
		[Desktop Entry]
		Encoding=UTF-8

Member

hebasto Jan 9, 2019

Desktop Entry Specification:

The Encoding key is deprecated.

Member Author

luke-jr commented Jan 10, 2019

org.bitcoin.bitcoin-qt.desktop is required to match QAPP_ORG_DOMAIN, which we have set to "bitcoin.org" for some reason. (Changing that is out of scope for this PR)

Member

laanwj commented Jan 16, 2019

org.bitcoin.bitcoin-qt.desktop is required to match QAPP_ORG_DOMAIN, which we have set to "bitcoin.org" for some reason.

I think what he means is the second part, suggesting replacing - with _

Member Author

luke-jr commented Feb 11, 2019

Ok, renamed it

Contributor

DrahtBot commented Mar 4, 2019 •

edited

Loading

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

build: set minimum supported macOS to 10.14 #20419 (build: set minimum supported macOS to 10.14 by fanquake)
Disable and fix tests for when BDB is not compiled #20267 (Disable and fix tests for when BDB is not compiled by achow101)
wallet: Make BDB support optional #20202 (wallet: Make BDB support optional by achow101)
depends: Properly pass $PATH to configure and pin #20019 (depends: Properly pass $PATH to configure and pin by dongcarl)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

DrahtBot added the Needs rebase label

A6GibKm commented Aug 15, 2019 •

edited

Loading

Consider adding a appdata as in
https://github.com/MarcoFalke/flathub/blob/f3d313f7991f6fec14f7ddc2c0fec19973f69814/org.bitcoincore.bitcoin-qt.appdata.xml
This should be installed to {~/.local,/usr/local}/share/metainfo/bitcoin.appdata.xml (or less desirably to /usr/share...)

EDIT: Correct link https://github.com/flathub/org.bitcoincore.bitcoin-qt/blob/master/org.bitcoincore.bitcoin-qt.metainfo.xml

hebasto mentioned this pull request

Add ability to integrate the client with desktop environment on Linux bitcoin-core/gui#38

Closed

Member

hebasto commented Aug 4, 2020

The alternative approach is bitcoin-core/gui#38 that does not require make install, and works with downloaded official binaries.

luke-jr added 3 commits

August 20, 2020 15:56


          configure: Option --without-xdg to disable future XDG data installation

b947626


          MOVEONLY: Rename bitcoin.svg to bitcoin128.svg

e07a87a

The latter is the common icon name used everywhere


          Install bitcoin128.svg as an icon

7b2d16c

luke-jr added 6 commits

August 20, 2020 15:59


          Install .desktop file for GUI

dd0290f


          Make .desktop file more complete

05149b6


          fixup! Install bitcoin128.svg as an icon

29d3599


          fixup! Install bitcoin128.svg as an icon

1d6d880


          fixup! Install .desktop file for GUI

97310f5


          fixup! Install .desktop file for GUI

aec2109

luke-jr force-pushed the xdg branch from d3792c4 to aec2109 Compare

August 20, 2020 16:00

Member Author

luke-jr commented Aug 20, 2020

Consider adding a appdata as in
https://github.com/MarcoFalke/flathub/blob/f3d313f7991f6fec14f7ddc2c0fec19973f69814/org.bitcoincore.bitcoin-qt.appdata.xml

Dead link.

Member Author

luke-jr commented Aug 20, 2020

Rebased

A6GibKm commented Aug 20, 2020

Sorry about that, here is the working link. Which I just realized needs the 0.20.1 release.

DrahtBot removed the Needs rebase label

Member Author

luke-jr commented Aug 20, 2020

Which XDG standard defines this?

A6GibKm commented Aug 20, 2020

hebasto mentioned this pull request

build, qt: Add SVG support, and replace bitcoin PNG image with SVG one #19780

Closed

A6GibKm commented Sep 13, 2020

The Desktop file fails desktop-file-validate

$ desktop-file-validate org.bitcoin.bitcoin_qt.desktop.in
org.bitcoin.bitcoin_qt.desktop: warning: key "Encoding" in group "Desktop Entry" is deprecated
org.bitcoin.bitcoin_qt.desktop: hint: value "Office;Finance;P2P;Network;Qt;" for key "Categories" in group "Desktop Entry" contains more than one main category; application might appear more than once in the application menu

Consider renaming to org.bitcoincore.bitcoin-qt to match the already existing flatpak at https://github.com/flathub/org.bitcoincore.bitcoin-qt.

DrahtBot mentioned this pull request

depends: Properly pass $PATH to configure and pin #20019

Closed

DrahtBot mentioned this pull request

wallet: Make BDB support optional #20202

Merged

DrahtBot mentioned this pull request

Disable and fix tests for when BDB is not compiled #20267

Merged

DrahtBot mentioned this pull request

build: set minimum supported macOS to 10.14 #20419

Merged

Contributor

DrahtBot commented Nov 23, 2020

🐙 This pull request conflicts with the target branch and needs rebase.

_{Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft".}

DrahtBot added the Needs rebase label

Contributor

DrahtBot commented Dec 15, 2021

There hasn't been much activity lately and the patch still needs rebase. What is the status here?

Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
Is it no longer relevant? ➡️ Please close.
Did the author lose interest or time to work on this? ➡️ Please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.

1 similar comment

Contributor

DrahtBot commented Mar 21, 2022

There hasn't been much activity lately and the patch still needs rebase. What is the status here?

Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
Is it no longer relevant? ➡️ Please close.
Did the author lose interest or time to work on this? ➡️ Please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.

Member

maflcko commented Mar 22, 2022

This has been needing rebase for about 2 years, so I am closing for now. Can be reopened any time, just let us know when.

maflcko closed this

bitcoin locked and limited conversation to collaborators

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

Labels

Build system Needs rebase