chu11 / freeipmi-mirror

Mirror of GNU FreeIPMI Git Repo - http://savannah.gnu.org/projects/freeipmi/. I maintain the upstream of FreeIPMI and can accept Github pull requests.
GNU General Public License v3.0
12 stars 16 forks source link

Hardcoded config directory #44

Closed grawlinson closed 3 years ago

grawlinson commented 3 years ago

I forgot to check this particular case before #41 closed.

https://github.com/chu11/freeipmi-mirror/blob/fc0f91d5ebc11b096d06b22f402d51a6bd4d19fd/etc/bmc-watchdog.service.in#L8

I still had sed -i "s/sysconfig/conf.d/" in my build scripts. Sorry! That's on me.

Arch Linux has a specific directory for sysconfig /etc/conf.d/ where other Linux distributions have /etc/sysconfig or /etc/default or whatever else there is. Sure would be nice if distributions could agree on a few things.

This isn't a major concern because I've already worked around it in the AUR package.

chu11 commented 3 years ago

Doh! Is there is preferred way to make this configurable/an option? I haven't been able to find anything semi-universal at the moment.

grawlinson commented 3 years ago

Could just set a distroconfigdir variable to what it is at the moment (sysconfig) and let distro packagers override the variable at build time.

chu11 commented 3 years ago

My random sampling of other RPMs eventually found pacemaker uses:

CONFIGDIR=""
AC_ARG_WITH([configdir],
    [AS_HELP_STRING([--with-configdir=DIR],
        [directory for Pacemaker configuration file @<:@SYSCONFDIR/sysconfig@:>@])],
    [ CONFIGDIR="$withval" ]
)

which sounds as good as anything else :P Redhat appears to just hard code /etc/sysconfig in alot of places (based on just random sampling of RPMs).

chu11 commented 3 years ago

@grawlinson could you give this tree a shot

https://github.com/chu11/freeipmi-mirror/tree/systemconfigdir_option

I ended up naming with --with-systemconfigdir ¯\(ツ)

Edit: oh yeah, you might need a define for the spec file, created one there too and force-pushed to the branch Edit2: oh wait, the spec file change I made doesn't work. Working on it :P

chu11 commented 3 years ago

@grawlinson fixed up a few things, can you give the branch a shot, thanks. To build an rpm I added an option so you can do rpmbuild -D "_systemconfigdir /foo/bar"

grawlinson commented 3 years ago

Works great! I look forward to seeing this in the next release.

I ended up cherry picking & squashing the relevant commits into a single patch that I could apply on top of the 1.6.8 release, as shown in this commit here.

I really appreciate you taking the time to do this, thank you!