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

Add Freedesktop files #358

Merged
merged 1 commit into from
May 24, 2021
Merged

Conversation

hfiguiere
Copy link
Contributor

Add and install the freedesktop files.

I use them for the flatpak flathub/flathub#1626

@dvzrv
Copy link
Contributor

dvzrv commented Sep 19, 2020

Good stuff! @monocasual please include this

@monocasual
Copy link
Collaborator

monocasual commented Sep 24, 2020

@hfiguiere thanks! Sorry for the delay, we were super busy with the CMake port. Two issues:

  1. could you please apply the changes in the CMakeLists.txt file? The Makefile.am/Configure.ac stuff is dead now (luckily 😎 )
  2. We are using the following Desktop file for the AppImage package:
[Desktop Entry]
Name=Giada
Name[es]=Giada
GenericName=Drum machine and loop sequencer
GenericName[es]=Caja de ritmos y secuenciador de loops
Icon=giada
Type=Application
Exec=giada
Terminal=false
Categories=AudioVideo;Audio;X-Digital_Processing;X-Jack;X-MIDI;Midi;
Keywords=Giada;

Do you think it can be safely merged into the one you proposed?

@hfiguiere
Copy link
Contributor Author

@hfiguiere thanks! Sorry for the delay, we were super busy with the CMake port. Two issues:

1. could you please apply the changes in the `CMakeLists.txt` file? The `Makefile.am`/`Configure.ac` stuff is dead now (luckily sunglasses )

I have no clue on how to do that in CMake.

2. We are using the following Desktop file for the AppImage package:

I couldn't find the .desktop file but I have merged most of the one below into the one I am submitting.

@monocasual
Copy link
Collaborator

I couldn't find the .desktop file but I have merged most of the one below into the one I am submitting.

Perfect.

I have no clue on how to do that in CMake.

The problem is that we are about to get rid of Autotools very soon (#396). What do variables such as desktopdir, desktop_DATA, appdatadir, appdata_DATA, ... do? They are defined in Makefile.am but (apparently) not used anywhere. With a bit of more context I could try to translate them into CMake commands.

@hfiguiere
Copy link
Contributor Author

hfiguiere commented Oct 6, 2020

The problem is that we are about to get rid of Autotools very soon (#396). What do variables such as desktopdir, desktop_DATA, appdatadir, appdata_DATA, ... do? They are defined in Makefile.am but (apparently) not used anywhere. With a bit of more context I could try to translate them into CMake commands.

This is standard automake syntax. "something_DATA" list data files to be installed into "somethingdir". So in that case:

destkop_DATA -> desktopdir, appdata_DATA -> appdatadir

They are used by automake that will generate the proper install targets.

And this is in my patch, protected by an AM_CONDITIONAL that is set if the platform is Linux or BSD because it's only relevant on not (Windows or macOS).

@hfiguiere
Copy link
Contributor Author

And the files are listed in EXTRA_DIST so that a make dist always add them to the tarball whichever the platform make dist is run on.

@monocasual
Copy link
Collaborator

monocasual commented Oct 7, 2020

I see, thanks @hfiguiere for the explanation. So I guess this should be covered by CMake installation rules and CPack for packaging (the make dist part).

@dvzrv
Copy link
Contributor

dvzrv commented Nov 20, 2020

@monocasual As the cmake setup is not yet complete, it would have been nice to include this into the currently working setup.
As the ticket for the cmake installation target is still open, integrating it there should not be much harder either.

@monocasual
Copy link
Collaborator

@dvzrv the problem here is that we still have to figure out how to translate that automake syntax into cmake commands. Any hint would be appreciated.

@musicinmybrain
Copy link
Contributor

I am not a CMake expert, so I can’t help with that part unless I have some time to dive through the documentation. I just wanted to note that I am updating the giada package in Fedora Linux, and I am using the files from this PR in the package.

@hfiguiere
Copy link
Contributor Author

hfiguiere commented Feb 22, 2021

Having them in the upstream repository even without install would help a lot.

Maybe I'll just update the PR with these files alone.

(I use them for the Flatpak)

@musicinmybrain
Copy link
Contributor

Using these in a GNOME/Wayland session, a generic icon and name (“FLTK”) are shown. This is a common problem, and it is not caused by any problem with the appdata and desktop file; instead, the program needs to find a way to set the application ID.

See spyder-ide/spyder#13786 for a detailed writeup of the same issue in a different application. Unfortunately the fix there was Qt-specific. Similarly for LyX and frescobaldi. See also https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/950, which is related.

This is not a deal-breaker for using these files, as having the correct launcher is much better than nothing. However, I’m hoping someone can figure out the right source code patch to allow gnome-shell to match the window with the desktop file.

@gvnnz gvnnz added this to the 1.0 milestone Apr 17, 2021
@hfiguiere hfiguiere force-pushed the freedesktop branch 2 times, most recently from c7b5f9c to cf4845d Compare May 22, 2021 16:02
@hfiguiere
Copy link
Contributor Author

Now with this update of the patch, the files get installed with CMake, including the icons

@gvnnz
Copy link
Contributor

gvnnz commented May 23, 2021

@hfiguiere thanks a lot for the update, I've just added a couple of comments.

CMakeLists.txt Show resolved Hide resolved
extras/com.giadamusic.Giada.metainfo.xml Show resolved Hide resolved
@gvnnz gvnnz changed the base branch from master to freedesktop-files May 24, 2021 19:05
@gvnnz gvnnz merged commit d8e5323 into monocasual:freedesktop-files May 24, 2021
@gvnnz
Copy link
Contributor

gvnnz commented May 24, 2021

Merged to freedesktop-files branch. I have fixed few minor things here and there, @hfiguiere please let me know if it looks good to you.

@hfiguiere hfiguiere deleted the freedesktop branch May 25, 2021 00:23
@hfiguiere
Copy link
Contributor Author

hfiguiere commented May 25, 2021

The change of app-id is unfortunate, and even I don't see it being necassary since you have the domain as well. This will translate to have to keep patches for the flathub package.

@gvnnz
Copy link
Contributor

gvnnz commented May 30, 2021

@hfiguiere changes reverted. Green light?

@hfiguiere
Copy link
Contributor Author

looks ok to me

@gvnnz
Copy link
Contributor

gvnnz commented Jun 1, 2021

Merged manually. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants