8:50 a.m.
It's been a while since 'make uninstall' cleaned up everything
that 'make install' created. I've noticed this while trying to
figure out why some build test on *BSD is failing (fix for that
is in the first patch btw).
Michal Privoznik (4):
nss: Try harder to uninstall
examples: Try harder to uninstall nwfilter
docs: Uninstall libvirt logo too
docs: Don't leave any documentation behind
docs/Makefile.am | 7 +++++++
examples/Makefile.am | 2 +-
tools/Makefile.am | 2 +-
3 files changed, 9 insertions(+), 2 deletions(-)
--
2.7.3
8:50 a.m.
New subject: [libvirt] [PATCH 1/4] nss: Try harder to uninstall
On BSD we are creating this symlink to libnss_libvirt.so called
nss_libvirt.so. That's just the way it is on BSD. However, when
uninstalling, we try to remove libnss_libvirt.so instead of the
symlink. Moreover, if file we are trying to remove does not exist
we error out instead of ignoring the error.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tools/Makefile.am | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/Makefile.am b/tools/Makefile.am
index 560a9a5..c474d75 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -427,7 +427,7 @@ install-exec-hook:
$(LN_S) libnss_libvirt.so.$(NSS_SO_VER) nss_libvirt.so.$(NSS_SO_VER)
uninstall-local:
- rm $(DESTDIR)$(libdir)/libnss_libvirt.so.$(NSS_SO_VER)
+ -rm -f $(DESTDIR)$(libdir)/nss_libvirt.so.$(NSS_SO_VER)
else ! WITH_BSD_NSS
LIBVIRT_NSS_SYMBOL_FILE = \
$(srcdir)/nss/libvirt_nss.syms
--
2.7.3
9:10 a.m.
New subject: [libvirt] [PATCH 1/4] nss: Try harder to uninstall
On 04/19/2016 09:50 AM, Michal Privoznik wrote:
On BSD we are creating this symlink to libnss_libvirt.so called
nss_libvirt.so. That's just the way it is on BSD. However, when
uninstalling, we try to remove libnss_libvirt.so instead of the
symlink. Moreover, if file we are trying to remove does not exist
we error out instead of ignoring the error.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tools/Makefile.am | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/Makefile.am b/tools/Makefile.am
index 560a9a5..c474d75 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -427,7 +427,7 @@ install-exec-hook:
$(LN_S) libnss_libvirt.so.$(NSS_SO_VER) nss_libvirt.so.$(NSS_SO_VER)
uninstall-local:
- rm $(DESTDIR)$(libdir)/libnss_libvirt.so.$(NSS_SO_VER)
+ -rm -f $(DESTDIR)$(libdir)/nss_libvirt.so.$(NSS_SO_VER)
^
Looks like a classic cut-n-paste error
ACK w/ the adjustment
John
else ! WITH_BSD_NSS
LIBVIRT_NSS_SYMBOL_FILE = \
$(srcdir)/nss/libvirt_nss.syms
8:50 a.m.
New subject: [libvirt] [PATCH 2/4] examples: Try harder to uninstall nwfilter
We have this code in our Makefile that tries to remove
/etc/libvirt/nwfilter if directory is left empty after all our
example nwfilters were uninstalled. However, the check for that
is missing quotation marks thus rendering the test useless:
test -z allow-arp.xml allow-dhcp-server.xml .. qemu-announce-self.xml || \
rmdir "/some/path/libvirt.git/_install/etc/libvirt/nwfilter"
/bin/sh: line 0: test: too many arguments
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
examples/Makefile.am | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/examples/Makefile.am b/examples/Makefile.am
index 46465f9..e1c37f0 100644
--- a/examples/Makefile.am
+++ b/examples/Makefile.am
@@ -90,5 +90,5 @@ uninstall-local::
for f in $(FILTERS); do \
rm -f "$(NWFILTER_DIR)/`basename $$f`"; \
done
- -test -z $(shell ls $(NWFILTER_DIR)) || rmdir $(NWFILTER_DIR)
+ -test -z "$(shell ls $(NWFILTER_DIR))" || rmdir $(NWFILTER_DIR)
endif WITH_NWFILTER
--
2.7.3
9:20 a.m.
New subject: [libvirt] [PATCH 2/4] examples: Try harder to uninstall nwfilter
On 04/19/2016 09:50 AM, Michal Privoznik wrote:
We have this code in our Makefile that tries to remove
/etc/libvirt/nwfilter if directory is left empty after all our
example nwfilters were uninstalled. However, the check for that
is missing quotation marks thus rendering the test useless:
test -z allow-arp.xml allow-dhcp-server.xml .. qemu-announce-self.xml || \
rmdir "/some/path/libvirt.git/_install/etc/libvirt/nwfilter"
/bin/sh: line 0: test: too many arguments
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
examples/Makefile.am | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
ACK
John
8:50 a.m.
New subject: [libvirt] [PATCH 3/4] docs: Uninstall libvirt logo too
While we could leave it behind as an indelible sign that libvirt
has been running on host, other users might not be that fond of
it.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
docs/Makefile.am | 1 +
1 file changed, 1 insertion(+)
diff --git a/docs/Makefile.am b/docs/Makefile.am
index f4ce9ce..c80c37b 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -368,3 +368,4 @@ uninstall-local:
for f in $(devhelphtml) $(devhelppng) $(devhelpcss); do \
rm -f $(DESTDIR)$(DEVHELP_DIR)/$$(basename $$f); \
done
+ rm -f $(DESTDIR)$(pkgdatadir)/libvirtLogo.png
--
2.7.3
9:20 a.m.
New subject: [libvirt] [PATCH 3/4] docs: Uninstall libvirt logo too
On 04/19/2016 09:50 AM, Michal Privoznik wrote:
While we could leave it behind as an indelible sign that libvirt
has been running on host, other users might not be that fond of
it.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
docs/Makefile.am | 1 +
1 file changed, 1 insertion(+)
ACK
John
8:50 a.m.
New subject: [libvirt] [PATCH 4/4] docs: Don't leave any documentation behind
Our uninstall script is not exact counterpart of install one.
Therefore we are leaving couple of files behind. This should not
happen.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
docs/Makefile.am | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/docs/Makefile.am b/docs/Makefile.am
index c80c37b..9524c94 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -363,8 +363,14 @@ install-data-local:
$(INSTALL_DATA) $(srcdir)/libvirtLogo.png $(DESTDIR)$(pkgdatadir)
uninstall-local:
+ for f in $(css) $(dot_html) $(gif) $(png); do \
+ rm -f $(DESTDIR)$(HTML_DIR)/$$f; \
+ done
for h in $(apihtml); do rm -f $(DESTDIR)$(HTML_DIR)/$$h; done
for p in $(apipng); do rm -f $(DESTDIR)$(HTML_DIR)/$$p; done
+ for f in $(internals_html); do \
+ rm -f $(DESTDIR)$(HTML_DIR)/$$f; \
+ done
for f in $(devhelphtml) $(devhelppng) $(devhelpcss); do \
rm -f $(DESTDIR)$(DEVHELP_DIR)/$$(basename $$f); \
done
--
2.7.3
9:38 a.m.
New subject: [libvirt] [PATCH 4/4] docs: Don't leave any documentation behind
On 04/19/2016 09:50 AM, Michal Privoznik wrote:
Our uninstall script is not exact counterpart of install one.
Therefore we are leaving couple of files behind. This should not
happen.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
docs/Makefile.am | 6 ++++++
1 file changed, 6 insertions(+)
At 'install-data-local:', there's a :
$(mkinstalldirs) $(DESTDIR)$(HTML_DIR)
why not just the far more all encompassing:
rm -rf $(DESTDIR)$(HTML_DIR)
and
rm -rf $(DESTDIR)$(DEVHELP_DIR)
Rather than picking each part we install to uninstall? and missing
something in the future or even now. Do the 'html' or 'internals'
directories gets removed? And then of course the toplevel directory
which we created.
IOW: There's no corollary for the:
$(mkinstalldirs) $(DESTDIR)$(HTML_DIR)
$(mkinstalldirs) $(DESTDIR)$(HTML_DIR)/html
$(mkinstalldirs) $(DESTDIR)$(HTML_DIR)/internals
$(mkinstalldirs) $(DESTDIR)$(DEVHELP_DIR)
John
diff --git a/docs/Makefile.am b/docs/Makefile.am
index c80c37b..9524c94 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -363,8 +363,14 @@ install-data-local:
$(INSTALL_DATA) $(srcdir)/libvirtLogo.png $(DESTDIR)$(pkgdatadir)
uninstall-local:
+ for f in $(css) $(dot_html) $(gif) $(png); do \
+ rm -f $(DESTDIR)$(HTML_DIR)/$$f; \
+ done
for h in $(apihtml); do rm -f $(DESTDIR)$(HTML_DIR)/$$h; done
for p in $(apipng); do rm -f $(DESTDIR)$(HTML_DIR)/$$p; done
+ for f in $(internals_html); do \
+ rm -f $(DESTDIR)$(HTML_DIR)/$$f; \
+ done
for f in $(devhelphtml) $(devhelppng) $(devhelpcss); do \
rm -f $(DESTDIR)$(DEVHELP_DIR)/$$(basename $$f); \
done
9:48 a.m.
New subject: [libvirt] [PATCH 4/4] docs: Don't leave any documentation behind
On 19.04.2016 16:38, John Ferlan wrote:
On 04/19/2016 09:50 AM, Michal Privoznik wrote:
> Our uninstall script is not exact counterpart of install one.
> Therefore we are leaving couple of files behind. This should not
> happen.
>
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
> docs/Makefile.am | 6 ++++++
> 1 file changed, 6 insertions(+)
>
At 'install-data-local:', there's a :
$(mkinstalldirs) $(DESTDIR)$(HTML_DIR)
why not just the far more all encompassing:
rm -rf $(DESTDIR)$(HTML_DIR)
and
rm -rf $(DESTDIR)$(DEVHELP_DIR)
Rather than picking each part we install to uninstall? and missing
something in the future or even now. Do the 'html' or 'internals'
directories gets removed? And then of course the toplevel directory
which we created.
IOW: There's no corollary for the:
$(mkinstalldirs) $(DESTDIR)$(HTML_DIR)
$(mkinstalldirs) $(DESTDIR)$(HTML_DIR)/html
$(mkinstalldirs) $(DESTDIR)$(HTML_DIR)/internals
$(mkinstalldirs) $(DESTDIR)$(DEVHELP_DIR)
Yeah. That's the other way of doing that. It's just that if users put
anything in $(DESTDIR)$(HTML_DIR) it will be removed by uninstall. But I
can propose v2 if you want.
Michal
9:57 a.m.
New subject: [libvirt] [PATCH 4/4] docs: Don't leave any documentation behind
On 04/19/2016 10:48 AM, Michal Privoznik wrote:
On 19.04.2016 16:38, John Ferlan wrote:
>
>
> On 04/19/2016 09:50 AM, Michal Privoznik wrote:
>> Our uninstall script is not exact counterpart of install one.
>> Therefore we are leaving couple of files behind. This should not
>> happen.
>>
>> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
>> ---
>> docs/Makefile.am | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>
> At 'install-data-local:', there's a :
>
> $(mkinstalldirs) $(DESTDIR)$(HTML_DIR)
>
> why not just the far more all encompassing:
>
> rm -rf $(DESTDIR)$(HTML_DIR)
>
> and
>
> rm -rf $(DESTDIR)$(DEVHELP_DIR)
>
> Rather than picking each part we install to uninstall? and missing
> something in the future or even now. Do the 'html' or 'internals'
> directories gets removed? And then of course the toplevel directory
> which we created.
>
> IOW: There's no corollary for the:
>
> $(mkinstalldirs) $(DESTDIR)$(HTML_DIR)
> $(mkinstalldirs) $(DESTDIR)$(HTML_DIR)/html
> $(mkinstalldirs) $(DESTDIR)$(HTML_DIR)/internals
> $(mkinstalldirs) $(DESTDIR)$(DEVHELP_DIR)
>
>
Yeah. That's the other way of doing that. It's just that if users put
anything in $(DESTDIR)$(HTML_DIR) it will be removed by uninstall. But I
can propose v2 if you want.
I see there are other 'rf -rm' usages in other "clean" labels...
I don't have a strong feeling either way - perhaps there's other
opinionated folks that would like to chime in. If no one chimes in,
then I'm OK with what's here...
I also now see there's "-rm " usages - so looks like it makes my comment
in 1/4 unnecessary. It just looked strange to me...
John
9:37 a.m.
New subject: [libvirt] [PATCH 4/4] docs: Don't leave any documentation behind
On Tue, Apr 19, 2016 at 10:57:25AM -0400, John Ferlan wrote:
On 04/19/2016 10:48 AM, Michal Privoznik wrote:
> On 19.04.2016 16:38, John Ferlan wrote:
>>
>>
>> On 04/19/2016 09:50 AM, Michal Privoznik wrote:
>>> Our uninstall script is not exact counterpart of install one.
>>> Therefore we are leaving couple of files behind. This should not
>>> happen.
>>>
>>> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
>>> ---
>>> docs/Makefile.am | 6 ++++++
>>> 1 file changed, 6 insertions(+)
>>>
>>
>> At 'install-data-local:', there's a :
>>
>> $(mkinstalldirs) $(DESTDIR)$(HTML_DIR)
>>
>> why not just the far more all encompassing:
>>
>> rm -rf $(DESTDIR)$(HTML_DIR)
>>
>> and
>>
>> rm -rf $(DESTDIR)$(DEVHELP_DIR)
>>
>> Rather than picking each part we install to uninstall? and missing
>> something in the future or even now. Do the 'html' or
'internals'
>> directories gets removed? And then of course the toplevel directory
>> which we created.
>>
>> IOW: There's no corollary for the:
>>
>> $(mkinstalldirs) $(DESTDIR)$(HTML_DIR)
>> $(mkinstalldirs) $(DESTDIR)$(HTML_DIR)/html
>> $(mkinstalldirs) $(DESTDIR)$(HTML_DIR)/internals
>> $(mkinstalldirs) $(DESTDIR)$(DEVHELP_DIR)
>>
>>
>
> Yeah. That's the other way of doing that. It's just that if users put
> anything in $(DESTDIR)$(HTML_DIR) it will be removed by uninstall. But I
> can propose v2 if you want.
>
I see there are other 'rf -rm' usages in other "clean" labels...
I don't have a strong feeling either way - perhaps there's other
opinionated folks that would like to chime in. If no one chimes in,
then I'm OK with what's here...
rm -rf is fine with me and I believe with others as well, so ACK from me.
I also now see there's "-rm " usages - so looks like it
makes my comment
in 1/4 unnecessary. It just looked strange to me...
I think '-' means "don't error out if this fails", similarly to
'@'
meaning "Don't print the command being ran" IIRC.
John
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
10:59 a.m.
New subject: [libvirt] [PATCH 4/4] docs: Don't leave any documentation behind
On 20.04.2016 16:37, Martin Kletzander wrote:
On Tue, Apr 19, 2016 at 10:57:25AM -0400, John Ferlan wrote:
>
>
> On 04/19/2016 10:48 AM, Michal Privoznik wrote:
>> On 19.04.2016 16:38, John Ferlan wrote:
>>>
>>>
>>> On 04/19/2016 09:50 AM, Michal Privoznik wrote:
>>>> Our uninstall script is not exact counterpart of install one.
>>>> Therefore we are leaving couple of files behind. This should not
>>>> happen.
>>>>
>>>> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
>>>> ---
>>>> docs/Makefile.am | 6 ++++++
>>>> 1 file changed, 6 insertions(+)
>>>>
>>>
>>> At 'install-data-local:', there's a :
>>>
>>> $(mkinstalldirs) $(DESTDIR)$(HTML_DIR)
>>>
>>> why not just the far more all encompassing:
>>>
>>> rm -rf $(DESTDIR)$(HTML_DIR)
>>>
>>> and
>>>
>>> rm -rf $(DESTDIR)$(DEVHELP_DIR)
>>>
>>> Rather than picking each part we install to uninstall? and missing
>>> something in the future or even now. Do the 'html' or
'internals'
>>> directories gets removed? And then of course the toplevel directory
>>> which we created.
>>>
>>> IOW: There's no corollary for the:
>>>
>>> $(mkinstalldirs) $(DESTDIR)$(HTML_DIR)
>>> $(mkinstalldirs) $(DESTDIR)$(HTML_DIR)/html
>>> $(mkinstalldirs) $(DESTDIR)$(HTML_DIR)/internals
>>> $(mkinstalldirs) $(DESTDIR)$(DEVHELP_DIR)
>>>
>>>
>>
>> Yeah. That's the other way of doing that. It's just that if users put
>> anything in $(DESTDIR)$(HTML_DIR) it will be removed by uninstall. But I
>> can propose v2 if you want.
>>
>
> I see there are other 'rf -rm' usages in other "clean" labels...
>
> I don't have a strong feeling either way - perhaps there's other
> opinionated folks that would like to chime in. If no one chimes in,
> then I'm OK with what's here...
>
rm -rf is fine with me and I believe with others as well, so ACK from me.
Thank you both guys. I've pushed these.
Michal
3140
days inactive
3141
days old
12 comments
3 participants
participants (3)
-
John Ferlan -
Martin Kletzander -
Michal Privoznik