libemf2svg icon indicating copy to clipboard operation
libemf2svg copied to clipboard

Switch to cmake-suggested install path variables

Open merkys opened this issue 3 years ago • 2 comments

I noticed the project uses *_INSTALL_DIR variables to control installation destinations. CMake recommends using CMAKE_INSTALL_*DIR variables, as "[T]his allows package maintainers to control the install destination by setting the appropriate cache variables." I am packaging libemf2svg for Debian, and having CMAKE_INSTALL_*DIR variables would be more convenient to do that.

merkys avatar Apr 20 '21 05:04 merkys

Hello,

Thanks for the PR.

However, I have two concerns regarding this change:

  1. the cmake debian helper seems to cope ok with the existing install variables, I've in fact already packaged libemf2svg (though my packaging is probably not compliant with Debian policies), and the rule file didn't need any tweak, https://github.com/kakwa/amkecpak/blob/master/libemf2svg/debian/rules, the resulting packages are available here (https://mirror.kakwalab.ovh/deb.sid/raw/). However, there are indeed some warnings in the build logs regarding these variables.
  2. libemf2svg is also packaged by other distributions (Suse and Alpine Linux), and I don't want to risk breaking there packaging (https://build.opensuse.org/package/view_file/graphics/libemf2svg/libemf2svg.spec?expand=1). Also, it doesn't seem the cmake macro used by RPM supports the CMAKE_INSTALL* variables: https://src.fedoraproject.org/rpms/cmake/blob/rawhide/f/macros.cmake.

If this change really do make things easier/more compliant, would it be possible for you to rework it so that both *_INSTALL_DIR and CMAKE_INSTALL_*DIR variables work?

This issue also has been discussed in the past in https://github.com/kakwa/libemf2svg/pull/22

kakwa avatar Apr 20 '21 17:04 kakwa

However, I have two concerns regarding this change:

  1. the cmake debian helper seems to cope ok with the existing install variables, I've in fact already packaged libemf2svg (though my packaging is probably not compliant with Debian policies), and the rule file didn't need any tweak, https://github.com/kakwa/amkecpak/blob/master/libemf2svg/debian/rules, the resulting packages are available here (https://mirror.kakwalab.ovh/deb.sid/raw/). However, there are indeed some warnings in the build logs regarding these variables.

Interesting. Maybe things have changed in cmake debian helper since then, as I had to do some minor tweaking. As for warnings, CMake reports unused variables from the command line, so I don't think this is of much importance.

  1. libemf2svg is also packaged by other distributions (Suse and Alpine Linux), and I don't want to risk breaking there packaging (https://build.opensuse.org/package/view_file/graphics/libemf2svg/libemf2svg.spec?expand=1). Also, it doesn't seem the cmake macro used by RPM supports the CMAKE_INSTALL* variables: https://src.fedoraproject.org/rpms/cmake/blob/rawhide/f/macros.cmake.

Indeed, my PR as-is would break packaging for these distributions. I wasn't aware that these distributions did not honor CMAKE_INSTALL*. Thus it makes sense to adapt packaging for new distribution to already existing install procedure.

If this change really do make things easier/more compliant, would it be possible for you to rework it so that both *_INSTALL_DIR and CMAKE_INSTALL_*DIR variables work?

This could indeed be done. I will give it a look.

This issue also has been discussed in the past in #22

Thanks for the link.

merkys avatar Apr 22 '21 04:04 merkys