[PATCH] Add DESTDIR to make install

Commit 04d29530 was incomplete by not using DESTDIR on installation. This can cause trouble when creating the rpm. Also, make install now is installing correctly service and firewall configuration files. --- Makefile.am | 54 +++++++++++++++++++++++++++++++------------ contrib/kimchi.spec.fedora.in | 3 --- 2 files changed, 39 insertions(+), 18 deletions(-) diff --git a/Makefile.am b/Makefile.am index 91a0fa2..46e0e0e 100644 --- a/Makefile.am +++ b/Makefile.am @@ -116,26 +116,50 @@ ChangeLog: install-data-local: @if test -d /usr/lib/systemd/system/ ; then \ - $(INSTALL_DATA) contrib/kimchid.service.fedora /usr/lib/systemd/system/kimchid.service; \ - systemctl daemon-reload; \ + mkdir -p $(DESTDIR)/usr/lib/systemd/system/; \ + $(INSTALL_DATA) contrib/kimchid.service.fedora $(DESTDIR)/usr/lib/systemd/system/kimchid.service; \ elif test -d /etc/systemd/system; then \ - $(INSTALL_DATA) contrib/kimchid.service.fedora /etc/systemd/system/kimchid.service; \ - systemctl daemon-reload; \ - else \ - $(INSTALL_DATA) contrib/kimchid.sysvinit /etc/init.d/kimchid; \ - chmod +x /etc/init.d/kimchid; \ - fi + mkdir -p $(DESTDIR)/etc/systemd/system/; \ + $(INSTALL_DATA) contrib/kimchid.service.fedora $(DESTDIR)/etc/systemd/system/kimchid.service; \ + else \ + mkdir -p $(DESTDIR)/etc/init.d/ \ + $(INSTALL_DATA) contrib/kimchid.sysvinit $(DESTDIR)/etc/init.d/kimchid; \ + chmod +x $(DESTDIR)/etc/init.d/kimchid; \ + if test -f /etc/fedora-release || test -f /etc/redhat-release; then \ + $(INSTALL_DATA) contrib/kimchid-upstart.conf.fedora $(DESTDIR)/etc/init/kimchi.conf; \ + elif test -f /etc/debian_version; then \ + $(INSTALL_DATA) contrib/kimchid-upstart.conf.debian $(DESTDIR)/etc/init/kimchi.conf; \ + fi; \ + fi; \ + if test -d /usr/lib/firewalld/services/; then \ + mkdir -p $(DESTDIR)/usr/lib/firewalld/services/; \ + $(INSTALL_DATA) src/firewalld.xml $(DESTDIR)/usr/lib/firewalld/services/kimchid.xml; \ + fi; \ + mkdir -p $(DESTDIR)/var/lib/kimchi/{debugreports,screenshots,vnc-tokens,isos} + touch $(DESTDIR)/var/lib/kimchi/objectstore + mkdir -p $(DESTDIR)/var/log/kimchi/ + touch $(DESTDIR)/var/log/kimchi/kimchi-access.log + touch $(DESTDIR)/var/log/kimchi/kimchi-error.log + mkdir -p $(DESTDIR)/etc/kimchi/ + touch $(DESTDIR)/etc/nginx/conf.d/kimchi.conf uninstall-local: @if test -f /usr/lib/systemd/system/kimchid.service; then \ - $(RM) /usr/lib/systemd/system/kimchid.service; \ - systemctl daemon-reload; \ - elif test -f /etc/systemd/system/kimchid.service; then \ - $(RM) /etc/systemd/system/kimchid.service; \ + $(RM) $(DESTDIR)/usr/lib/systemd/system/kimchid.service; \ + elif test -f $(DESTDIR)/etc/systemd/system/kimchid.service; then \ + $(RM) $(DESTDIR)/etc/systemd/system/kimchid.service; \ elif test -f /etc/init.d/kimchid; then \ - $(RM) /etc/init.d/kimchid; \ - fi - + $(RM) $(DESTDIR)/etc/init.d/kimchid; \ + $(RM) $(DESTDIR)/etc/init/kimchi.conf; \ + fi; \ + if test -d /usr/lib/firewalld/services/; then \ + $(RM) $(DESTDIR)/usr/lib/firewalld/services/kimchid.xml; \ + fi; \ + $(RM) -rf $(DESTDIR)/var/lib/kimchi + $(RM) -rf $(DESTDIR)/var/log/kimchi + $(RM) -rf $(DESTDIR)/etc/kimchi + $(RM) $(DESTDIR)/etc/nginx/conf.d/kimchi.conf + VERSION: @if test -d .git; then \ git describe --abbrev=0 > $@; \ diff --git a/contrib/kimchi.spec.fedora.in b/contrib/kimchi.spec.fedora.in index 30f8417..4e12327 100644 --- a/contrib/kimchi.spec.fedora.in +++ b/contrib/kimchi.spec.fedora.in @@ -98,9 +98,6 @@ touch %{buildroot}/%{_localstatedir}/log/kimchi/kimchi-error.log mkdir -p %{buildroot}/%{_sysconfdir}/kimchi/ touch %{buildroot}/%{_sysconfdir}/nginx/conf.d/kimchi.conf -# Install the systemd scripts -install -Dm 0644 contrib/kimchid.service.fedora %{buildroot}%{_unitdir}/kimchid.service -install -Dm 0640 src/firewalld.xml %{buildroot}%{_prefix}/lib/firewalld/services/kimchid.xml %endif %if 0%{?rhel} == 6 -- 2.1.0

On 27/05/2015 17:25, Ramon Medeiros wrote:
Commit 04d29530 was incomplete by not using DESTDIR on installation. This can cause trouble when creating the rpm. Also, make install now is installing correctly service and firewall configuration files. --- Makefile.am | 54 +++++++++++++++++++++++++++++++------------ contrib/kimchi.spec.fedora.in | 3 --- 2 files changed, 39 insertions(+), 18 deletions(-)
diff --git a/Makefile.am b/Makefile.am index 91a0fa2..46e0e0e 100644 --- a/Makefile.am +++ b/Makefile.am @@ -116,26 +116,50 @@ ChangeLog:
install-data-local: @if test -d /usr/lib/systemd/system/ ; then \ - $(INSTALL_DATA) contrib/kimchid.service.fedora /usr/lib/systemd/system/kimchid.service; \ - systemctl daemon-reload; \ + mkdir -p $(DESTDIR)/usr/lib/systemd/system/; \ + $(INSTALL_DATA) contrib/kimchid.service.fedora $(DESTDIR)/usr/lib/systemd/system/kimchid.service; \ elif test -d /etc/systemd/system; then \ - $(INSTALL_DATA) contrib/kimchid.service.fedora /etc/systemd/system/kimchid.service; \ - systemctl daemon-reload; \ - else \ - $(INSTALL_DATA) contrib/kimchid.sysvinit /etc/init.d/kimchid; \ - chmod +x /etc/init.d/kimchid; \ - fi + mkdir -p $(DESTDIR)/etc/systemd/system/; \ + $(INSTALL_DATA) contrib/kimchid.service.fedora $(DESTDIR)/etc/systemd/system/kimchid.service; \
+ else \ + mkdir -p $(DESTDIR)/etc/init.d/ \ + $(INSTALL_DATA) contrib/kimchid.sysvinit $(DESTDIR)/etc/init.d/kimchid; \ + chmod +x $(DESTDIR)/etc/init.d/kimchid; \ + if test -f /etc/fedora-release || test -f /etc/redhat-release; then \ + $(INSTALL_DATA) contrib/kimchid-upstart.conf.fedora $(DESTDIR)/etc/init/kimchi.conf; \ + elif test -f /etc/debian_version; then \ + $(INSTALL_DATA) contrib/kimchid-upstart.conf.debian $(DESTDIR)/etc/init/kimchi.conf; \ + fi; \
Why will we install the sysvinit script in a machine which uses upstart script?
+ fi; \ + if test -d /usr/lib/firewalld/services/; then \ + mkdir -p $(DESTDIR)/usr/lib/firewalld/services/; \ + $(INSTALL_DATA) src/firewalld.xml $(DESTDIR)/usr/lib/firewalld/services/kimchid.xml; \ + fi; \
Seems there is a problem in the indentation above.
+ mkdir -p $(DESTDIR)/var/lib/kimchi/{debugreports,screenshots,vnc-tokens,isos} + touch $(DESTDIR)/var/lib/kimchi/objectstore + mkdir -p $(DESTDIR)/var/log/kimchi/ + touch $(DESTDIR)/var/log/kimchi/kimchi-access.log + touch $(DESTDIR)/var/log/kimchi/kimchi-error.log + mkdir -p $(DESTDIR)/etc/kimchi/ + touch $(DESTDIR)/etc/nginx/conf.d/kimchi.conf
I don't think we need it as those files will be created on demand on Kimchi startup
uninstall-local: @if test -f /usr/lib/systemd/system/kimchid.service; then \ - $(RM) /usr/lib/systemd/system/kimchid.service; \ - systemctl daemon-reload; \ - elif test -f /etc/systemd/system/kimchid.service; then \ - $(RM) /etc/systemd/system/kimchid.service; \ + $(RM) $(DESTDIR)/usr/lib/systemd/system/kimchid.service; \ + elif test -f $(DESTDIR)/etc/systemd/system/kimchid.service; then \ + $(RM) $(DESTDIR)/etc/systemd/system/kimchid.service; \ elif test -f /etc/init.d/kimchid; then \ - $(RM) /etc/init.d/kimchid; \ - fi - + $(RM) $(DESTDIR)/etc/init.d/kimchid; \ + $(RM) $(DESTDIR)/etc/init/kimchi.conf; \ + fi; \ + if test -d /usr/lib/firewalld/services/; then \ + $(RM) $(DESTDIR)/usr/lib/firewalld/services/kimchid.xml; \ + fi; \ + $(RM) -rf $(DESTDIR)/var/lib/kimchi + $(RM) -rf $(DESTDIR)/var/log/kimchi + $(RM) -rf $(DESTDIR)/etc/kimchi + $(RM) $(DESTDIR)/etc/nginx/conf.d/kimchi.conf + VERSION: @if test -d .git; then \ git describe --abbrev=0 > $@; \ diff --git a/contrib/kimchi.spec.fedora.in b/contrib/kimchi.spec.fedora.in index 30f8417..4e12327 100644 --- a/contrib/kimchi.spec.fedora.in +++ b/contrib/kimchi.spec.fedora.in @@ -98,9 +98,6 @@ touch %{buildroot}/%{_localstatedir}/log/kimchi/kimchi-error.log mkdir -p %{buildroot}/%{_sysconfdir}/kimchi/ touch %{buildroot}/%{_sysconfdir}/nginx/conf.d/kimchi.conf
-# Install the systemd scripts -install -Dm 0644 contrib/kimchid.service.fedora %{buildroot}%{_unitdir}/kimchid.service -install -Dm 0640 src/firewalld.xml %{buildroot}%{_prefix}/lib/firewalld/services/kimchid.xml %endif
You need to do the same for the SUSE spec, right?
%if 0%{?rhel} == 6

Hi Ramon, DESTDIR is not the only issue I think : - your assuming that systemd services go into /usr/lib/systemd/system (for kimchid and firewalld) but on debian/ubuntu system level services from packages go into /lib/systemd/system. You can try the following : adding this to configure.ac (requires a build dep on pkg-config) : --- # check for systemd PKG_PROG_PKG_CONFIG AC_ARG_WITH([systemdsystemunitdir], AS_HELP_STRING([--with-systemdsystemunitdir=DIR], [Directory for systemd service files]), [], [with_systemdsystemunitdir=$($PKG_CONFIG --variable=systemdsystemunitdir systemd)]) if test "x$with_systemdsystemunitdir" != xno; then AC_SUBST([systemdsystemunitdir], [$with_systemdsystemunitdir]) fi --- then you can use $(DESTDIR)$(systemdsystemunitdir) for kimchid.service and firewalld.xml - on the same topic, I'm not sure one kimchi should install any service in /etc/systemd/system/ : man systemd.unit says in "Table 1" that this is for local configuration and that installed packages should go in $(systemdsystemunitdir)/lib/systemd/system So no need of /etc/systemd/system part I'd say. - you use /etc/debian_version to determine if one should copy an upstart config script but debian and ubuntu have these file but debian does not use upstart init system. Maybe a better place to identify the distro would be /etc/os-release (ID field) which seems to be used on debian/ubuntu/fedora/pkvm Fred On Wed, 27 May 2015 17:25:24 -0300, Ramon Medeiros <ramonn@linux.vnet.ibm.com> wrote:
Commit 04d29530 was incomplete by not using DESTDIR on installation. This can cause trouble when creating the rpm. Also, make install now is installing correctly service and firewall configuration files. --- Makefile.am | 54 +++++++++++++++++++++++++++++++------------ contrib/kimchi.spec.fedora.in | 3 --- 2 files changed, 39 insertions(+), 18 deletions(-)
diff --git a/Makefile.am b/Makefile.am index 91a0fa2..46e0e0e 100644 --- a/Makefile.am +++ b/Makefile.am @@ -116,26 +116,50 @@ ChangeLog:
install-data-local: @if test -d /usr/lib/systemd/system/ ; then \ - $(INSTALL_DATA) contrib/kimchid.service.fedora /usr/lib/systemd/system/kimchid.service; \ - systemctl daemon-reload; \ + mkdir -p $(DESTDIR)/usr/lib/systemd/system/; \ + $(INSTALL_DATA) contrib/kimchid.service.fedora $(DESTDIR)/usr/lib/systemd/system/kimchid.service; \ elif test -d /etc/systemd/system; then \ - $(INSTALL_DATA) contrib/kimchid.service.fedora /etc/systemd/system/kimchid.service; \ - systemctl daemon-reload; \ - else \ - $(INSTALL_DATA) contrib/kimchid.sysvinit /etc/init.d/kimchid; \ - chmod +x /etc/init.d/kimchid; \ - fi + mkdir -p $(DESTDIR)/etc/systemd/system/; \ + $(INSTALL_DATA) contrib/kimchid.service.fedora $(DESTDIR)/etc/systemd/system/kimchid.service; \ + else \ + mkdir -p $(DESTDIR)/etc/init.d/ \ + $(INSTALL_DATA) contrib/kimchid.sysvinit $(DESTDIR)/etc/init.d/kimchid; \ + chmod +x $(DESTDIR)/etc/init.d/kimchid; \ + if test -f /etc/fedora-release || test -f /etc/redhat-release; then \ + $(INSTALL_DATA) contrib/kimchid-upstart.conf.fedora $(DESTDIR)/etc/init/kimchi.conf; \ + elif test -f /etc/debian_version; then \ + $(INSTALL_DATA) contrib/kimchid-upstart.conf.debian $(DESTDIR)/etc/init/kimchi.conf; \ + fi; \ + fi; \ + if test -d /usr/lib/firewalld/services/; then \ + mkdir -p $(DESTDIR)/usr/lib/firewalld/services/; \ + $(INSTALL_DATA) src/firewalld.xml $(DESTDIR)/usr/lib/firewalld/services/kimchid.xml; \ + fi; \ + mkdir -p $(DESTDIR)/var/lib/kimchi/{debugreports,screenshots,vnc-tokens,isos} + touch $(DESTDIR)/var/lib/kimchi/objectstore + mkdir -p $(DESTDIR)/var/log/kimchi/ + touch $(DESTDIR)/var/log/kimchi/kimchi-access.log + touch $(DESTDIR)/var/log/kimchi/kimchi-error.log + mkdir -p $(DESTDIR)/etc/kimchi/ + touch $(DESTDIR)/etc/nginx/conf.d/kimchi.conf
uninstall-local: @if test -f /usr/lib/systemd/system/kimchid.service; then \ - $(RM) /usr/lib/systemd/system/kimchid.service; \ - systemctl daemon-reload; \ - elif test -f /etc/systemd/system/kimchid.service; then \ - $(RM) /etc/systemd/system/kimchid.service; \ + $(RM) $(DESTDIR)/usr/lib/systemd/system/kimchid.service; \ + elif test -f $(DESTDIR)/etc/systemd/system/kimchid.service; then \ + $(RM) $(DESTDIR)/etc/systemd/system/kimchid.service; \ elif test -f /etc/init.d/kimchid; then \ - $(RM) /etc/init.d/kimchid; \ - fi - + $(RM) $(DESTDIR)/etc/init.d/kimchid; \ + $(RM) $(DESTDIR)/etc/init/kimchi.conf; \ + fi; \ + if test -d /usr/lib/firewalld/services/; then \ + $(RM) $(DESTDIR)/usr/lib/firewalld/services/kimchid.xml; \ + fi; \ + $(RM) -rf $(DESTDIR)/var/lib/kimchi + $(RM) -rf $(DESTDIR)/var/log/kimchi + $(RM) -rf $(DESTDIR)/etc/kimchi + $(RM) $(DESTDIR)/etc/nginx/conf.d/kimchi.conf + VERSION: @if test -d .git; then \ git describe --abbrev=0 > $@; \ diff --git a/contrib/kimchi.spec.fedora.in b/contrib/kimchi.spec.fedora.in index 30f8417..4e12327 100644 --- a/contrib/kimchi.spec.fedora.in +++ b/contrib/kimchi.spec.fedora.in @@ -98,9 +98,6 @@ touch %{buildroot}/%{_localstatedir}/log/kimchi/kimchi-error.log mkdir -p %{buildroot}/%{_sysconfdir}/kimchi/ touch %{buildroot}/%{_sysconfdir}/nginx/conf.d/kimchi.conf
-# Install the systemd scripts -install -Dm 0644 contrib/kimchid.service.fedora %{buildroot}%{_unitdir}/kimchid.service -install -Dm 0640 src/firewalld.xml %{buildroot}%{_prefix}/lib/firewalld/services/kimchid.xml %endif
%if 0%{?rhel} == 6 -- 2.1.0
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel

On 05/28/2015 11:24 AM, Frederic Bonnard wrote:
man systemd.unit says in "Table 1" that this is for local configuration and that installed packages should go in $(systemdsystemunitdir)/lib/systemd/system So no need of /etc/systemd/system part I'd say. HI Frederic,
i put on /etc/systemd/system because debian8 does not have a /usr/lib/systemd/system directory. Should i created it? -- Ramon Nunes Medeiros Kimchi Developer Linux Technology Center Brazil IBM Systems & Technology Group Phone : +55 19 2132 7878 ramonn@br.ibm.com

man systemd.unit says in "Table 1" that this is for local configuration and that installed packages should go in $(systemdsystemunitdir)/lib/systemd/system So no need of /etc/systemd/system part I'd say. HI Frederic,
i put on /etc/systemd/system because debian8 does not have a /usr/lib/systemd/system directory. Should i created it?
The equivalent of /usr/lib/systemd/system on debian/ubuntu is /lib/systemd/system. The latter should already exist. If you use $(systemdsystemunitdir)/lib/systemd/system it should be all good. You should have $(systemdsystemunitdir)/lib/systemd/system resolving to - /usr/lib/systemd/system on fedora-like systems - /lib/systemd/system on debian-like systems That means that your initial section should be enough on all systems. And the section with /etc can be removed. If I have time I can rework your patch and send it to you so that you can check/review it. F.
--
Ramon Nunes Medeiros Kimchi Developer Linux Technology Center Brazil IBM Systems & Technology Group Phone : +55 19 2132 7878 ramonn@br.ibm.com

I just checked and saw that is /lib/systemd/system is also available on fedora. Can we use only this dir so? On 05/29/2015 01:43 PM, Frederic Bonnard wrote: >>> man systemd.unit says in "Table 1" that this is for local >>> configuration and that installed packages should go in $(systemdsystemunitdir)/lib/systemd/system >>> So no need of /etc/systemd/system part I'd say. >> HI Frederic, >> >> i put on /etc/systemd/system because debian8 does not have a >> /usr/lib/systemd/system directory. Should i created it? > The equivalent of /usr/lib/systemd/system on debian/ubuntu is > /lib/systemd/system. The latter should already exist. > If you use $(systemdsystemunitdir)/lib/systemd/system it should be all good. > You should have $(systemdsystemunitdir)/lib/systemd/system resolving to > - /usr/lib/systemd/system on fedora-like systems > - /lib/systemd/system on debian-like systems > > That means that your initial section should be enough on all systems. And > the section with /etc can be removed. > > If I have time I can rework your patch and send it to you so that you can > check/review it. > > F. > >> -- >> >> Ramon Nunes Medeiros >> Kimchi Developer >> Linux Technology Center Brazil >> IBM Systems & Technology Group >> Phone : +55 19 2132 7878 >> ramonn@br.ibm.com >> -- Ramon Nunes Medeiros Kimchi Developer Linux Technology Center Brazil IBM Systems & Technology Group Phone : +55 19 2132 7878 ramonn@br.ibm.com

On 01/06/2015 09:37, Ramon Medeiros wrote: > I just checked and saw that is /lib/systemd/system is also available > on fedora. > > Can we use only this dir so? If you use the macro $(systemdsystemunitdir) it will work to every distro. You don't need to hard coded any directory. > > On 05/29/2015 01:43 PM, Frederic Bonnard wrote: >>>> man systemd.unit says in "Table 1" that this is for local >>>> configuration and that installed packages should go in >>>> $(systemdsystemunitdir)/lib/systemd/system >>>> So no need of /etc/systemd/system part I'd say. >>> HI Frederic, >>> >>> i put on /etc/systemd/system because debian8 does not have a >>> /usr/lib/systemd/system directory. Should i created it? >> The equivalent of /usr/lib/systemd/system on debian/ubuntu is >> /lib/systemd/system. The latter should already exist. >> If you use $(systemdsystemunitdir)/lib/systemd/system it should be >> all good. >> You should have $(systemdsystemunitdir)/lib/systemd/system resolving to >> - /usr/lib/systemd/system on fedora-like systems >> - /lib/systemd/system on debian-like systems >> >> That means that your initial section should be enough on all systems. >> And >> the section with /etc can be removed. >> >> If I have time I can rework your patch and send it to you so that you >> can >> check/review it. >> >> F. >> >>> -- >>> >>> Ramon Nunes Medeiros >>> Kimchi Developer >>> Linux Technology Center Brazil >>> IBM Systems & Technology Group >>> Phone : +55 19 2132 7878 >>> ramonn@br.ibm.com >>> >
participants (3)
-
Aline Manera
-
Frederic Bonnard
-
Ramon Medeiros