search

Jacalz / rymdport

Cross-platform application for easy encrypted file, folder, and text sharing between devices.
https://rymdport.github.io/
GNU General Public License v3.0
1.08k stars 53 forks source link

split up install target for packaging #119

Closed erdii closed 1 year ago

erdii commented 1 year ago

Description:

The AUR package [1] for rymdport uses the install target [2] but there's no need to reload the icon cache when packaging.

This patch splits the install target so that the packaging process can invoke the install-static target without tying to invoke sudo gtk-update-icon-cache.

Existing usage of make install should be unaffected.

I've also added .PHONY targets to the makefile.

[1] https://aur.archlinux.org/packages/rymdport [2] https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=rymdport#n24

Checklist:

Jacalz commented 1 year ago

Thanks for the PR. I saw that Solus actually patched out the icon cache change from the Makefile so I was planning on doing something similar either way. Much appreciated :)

Just one question, do I understand correctly that you would have to call make static-install if you don't want to update the icon cache? I think Solus runs make install so it might be better to turn the logic around and actually change the behaviour of make install. What do you think?

erdii commented 1 year ago

Just one question, do I understand correctly that you would have to call make static-install if you don't want to update the icon cache?

Exactly! And make install if you want to install and update the icon cache in one go.

I think Solus runs make install so it might be better to turn the logic around and actually change the behaviour of make install. What do you think?

From solus' POV: They'd probably be happy to have a target without the icon cache update and drop the patch code. On the other hand: Splitting the target in the makefile could break the patching you mentioned in their build process.

From my AUR package's POV I'd definitely prefer dropping the icon cache update from make install because then I wouldn't need to change the PKGBUILD. On the other hand: changing one line from make install ... to make static-install ... is not much work for me either.

From the POV of a user that builds and installs rymdport from source and maybe has automation scripts in place for updates: I'd not be happy about a behavior change to make install if I rely on the icon cache to be updated afterwards.

Jacalz commented 1 year ago

I think Solus would be very happy to get rid of the patch and have make install behave like it did before. The thing is that their build system has a %make_install macro that sets up all the flags such. As such, I think the breaking change is the best path forward.

A user might be confused yes, but I think the impact will be minimal. Most users will either download a package (AUR or whatever), use Flatpak or just download the binaries on the release page. We can add a comment (as they are printed to stdout) to the Makefile, at the bottom of the install command, that they might want to run make update-icon-cache afterwards. I think that's the optimal solution in my opinion.

erdii commented 1 year ago

Sounds very reasonable to me! :)

erdii commented 1 year ago

I have updated the PR according to your feedback.

PS: Tell me If you want me to rewrite the PR into a single commit for cleanness.

Jacalz commented 1 year ago

Thanks @erdii. I appreciate that you took the time to contribute this change. I can always squash on merge if I feel a need for it but I merged it as it was :)

I'll try to roll a release in a few weeks here. There is a lot on my plate at the moment so it might drag out unfortunately.