cmake: don't hard-code absolute path name /etc

CESNET / netopeer2

NETCONF toolset

BSD 3-Clause "New" or "Revised" License

300 stars 188 forks source link

cmake: don't hard-code absolute path name /etc #1558

Closed jktjkt closed 5 months ago

jktjkt commented 5 months ago

cmake: don't hard-code absolute path name /etc

this breaks packaging and installation into a prefix in general
there's a GNUInstallDirs variable for /etc, too

Also, don't make this installation conditional on the existence of /etc. Whether or not that directory exists on a build system has no meaning for installation to a target system. This had a potential to break cross-compiling.

michalvasko commented 5 months ago

Okay, but like always, please target the devel branch. Also, in case PAM is not installed on the system, should the config file be copied anyway?

jktjkt commented 5 months ago

Okay, but like always, please target the devel branch.

oops, sorry, fixed

Also, in case PAM is not installed on the system, should the config file be copied anyway?

There's no harm in doing that; it's a single text file and it does not bring any extra dependencies. If there's a downstream package which does not like that file, they are free to not include it in a package.

michalvasko commented 5 months ago

Okay but why remove the directory existence check? Because of cross-compilation?

jktjkt commented 5 months ago

Okay but why remove the directory existence check? Because of cross-compilation?

Yes, this is explained in the PR description and in the commit message. In fact, this isn't just about cross-compiling. If you have a build-time check like this, you're introducing a non-explicit dependency into your build process. This might be fine when building on a developer's laptop, but in any real, automated packaging solution, this implies a need for a "useless" build-time dependency on libpam, otherwise you have a risk of producing a non-complete package.

michalvasko commented 5 months ago

Fine, I suppose.