#241 Reworked src/arch/gtk3/data/Makefile.am

[James Le Cuirot]

When making my earlier fixes, I didn't notice how messed up this Makefile.am is. When you added the cp vice.gresource line (it breaks because it doesn't respect DESTDIR), I wondered why gtk3data_DATA was being ignored and then I started to realise what was going on.

I've changed it so much that a patch would just be confusing so here's the whole thing. In short, it deals with the confusing rats nest of conditionals and fixes the broken install target.

I could go into more detail but I need to get some sleep. You can just take my word for it or you can wait for me to go into more detail if you like. I'm just getting it out there now in case you start doing more work on it.

Note I'm not going to test Windows so please try that for me.

[James Le Cuirot]

Oh, I forgot to mention that you need to remove the generated vice-org.pokefinder.vice.directory from the repository because it's screwing with the Makefile VPATH search logic.

[James Le Cuirot]

Okay so:

gtk3data_DATA was being ignored because you'd overridden the install target. I think you kinda knew that though? The install-data-hook target can be used to add a custom recipe to install.

We don't need to define our own clean target as we can just define the CLEANFILES variable instead.

We don't need to define the all target either. Adding files to gtk3data_DATA means that they will be built anyway.

We install the .directory and .desktop files with our custom recipe rather than the usual mechanism so they wouldn't be built as part of all. I have worked around this by adding them to noinst_DATA.

The .desktop files were being rebuilt every time because of the quotes on these lines:

X64_DESKTOP = "$(XDG_VENDOR)-$(X64_BINARY).desktop"
X64SC_DESKTOP = "$(XDG_VENDOR)-$(X64SC_BINARY).desktop"
...

You generally should not use quotes when defining Makefile variables because they actually become part of the string value itself.

I guess you defined VICE_DIR_FILE prefixed with $(top_builddir) because you'd accidentally committed vice-org.pokefinder.vice.directory to the repository. With that file removed, it figures the path out for you.

We can avoid repeating the SUPPORT_X64 logic by defining the list of .desktop files in one place with the VICE_DESKTOP_FILES variable and then passing this to xdg-desktop-menu to install all the .desktop files in one go.

I wonder whether the various .desktop recipes could be refactored using variables like the following but I'm not sure and I'm tired of this now!

X64_DESC = Crappy C64 emulator
X64SC_DESC = Cycle-exact C64 emulator
...

[James Le Cuirot]

With all that figured out, you could potentially install the .directory and .desktop files in the usual manner instead of via xdg-desktop-menu. I've seen both approaches and I don't know whether you picked this one for a reason. As I've mentioned, xdg-desktop-menu is a little awkward for packagers but it can be worked around. Perhaps you picked it because it ensures the files are written to the correct location, regardless of the prefix given to configure, and it also works for non-root users.

[compyx]

Thanks for the patch, or rather, rewrite :)

The old Makefile had indeed become a mess. First it was just generating the gresource file, then Windows icons got added and then I asked a friend to create new icons for VICE and I felt I had to actually use them, hence the .desktop files.

I knew I broke the install target, I was planning on figuring out how to not overwrite the implicit install target, but other things got in the way. Luckily you came along with the install-data-hook target.

As for further refactoring the desktop file recipes, I'll look into that, I'm not a fan of using shell scripts with a long of arguments, that stuff breaks easily.

And indeed I used xdg-desktop-menu to get the correct path and allow non-root installs. And also because I could find a way to 'install' the .directory file separately but still make sure the .desktop items ended up in the menu the .directory file generates. Perhaps you can point me to an ebuild file or some Gentoo packaging docs that show how to avoid xdg-desktop-menu?

For Windows I was already working on generating .ico files from the .png icons (using icotools) and .res files for the Windows binaries. That's going to add yet more complexity, so I'm considering splitting the OS-specific stuff into separate Makefile.am's (ie data/Makefile.am, data/win32/Makefile.am, data/macos/Makefile.am and data/unix/Makefile.am). Not sure that's a good idea though, I'll have to experiment a bit with that.

[compyx]

I split the Makefile.am into OS-specific dirs. Works on Linux, but not sure about Windows or MacOS (though MacOS is an empty file).

So here's an svn diff to test.

[James Le Cuirot]

Looks good, at a glance. I can only test Linux and you said that works.

[compyx]

Works on Windows as well. So I commited the patch.

[Greg King]

GTK3VICE cannot be built on Windows because "gtk3/data/win32/Makefile.am" doesn't create "icon.res".

[compyx]

Odd, worked for me on msys2. I suspect icon.res was left over in my build tree then. Will check. As I mentioned in bug #1292, I'll be working on icon/res stuff for Windows anway to provide proper icon sets for the emulator on Windows. That's also the reason for the splitting of Makefile.am into OS-specific Makefile.am's, hopefully making the icon/launcher stuff a bit more maintainable.

[compyx]

Alright, building on Windows does break on icon.res. I did indeed have that file in my build tree, so for me building still 'worked'. After a fresh checkout, configure and make, building fails on icon.res, as noted.

[compyx]

Linking error should have been fixed with commit r38655.

[gpz]

can we close this? :)

[compyx]

Yes please, the buildsystem has evolved way beyond this.