Olaf Hering
2013-Sep-18 18:11 UTC
[PATCH] qemu-traditional: do not strip binaries during make install
Signed-off-by: Olaf Hering <olaf@aepfle.de> Acked-by: Matt Wilson <msw@amazon.com> --- Makefile | 2 +- Makefile.target | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 37c7066..594f0ef 100644 --- a/Makefile +++ b/Makefile @@ -243,7 +243,7 @@ endif install: all $(if $(BUILD_DOCS),install-doc) mkdir -p "$(DESTDIR)$(bindir)" ifneq ($(TOOLS),) - $(INSTALL) -m 755 -s $(TOOLS) "$(DESTDIR)$(bindir)" + $(INSTALL) -m 755 $(TOOLS) "$(DESTDIR)$(bindir)" endif ifneq ($(BLOBS),) mkdir -p "$(DESTDIR)$(datadir)" diff --git a/Makefile.target b/Makefile.target index 19bb0fd..1cf7f34 100644 --- a/Makefile.target +++ b/Makefile.target @@ -755,7 +755,7 @@ clean: install: all install-hook ifneq ($(PROGS),) - $(INSTALL) -m 755 -s $(PROGS) "$(DESTDIR)$(bindir)" + $(INSTALL) -m 755 $(PROGS) "$(DESTDIR)$(bindir)" endif # Include automatically generated dependency files
Matthew Daley
2013-Sep-19 00:21 UTC
Re: [PATCH] qemu-traditional: do not strip binaries during make install
On Thu, Sep 19, 2013 at 6:11 AM, Olaf Hering <olaf@aepfle.de> wrote:> @@ -243,7 +243,7 @@ endif > install: all $(if $(BUILD_DOCS),install-doc) > mkdir -p "$(DESTDIR)$(bindir)" > ifneq ($(TOOLS),) > - $(INSTALL) -m 755 -s $(TOOLS) "$(DESTDIR)$(bindir)" > + $(INSTALL) -m 755 $(TOOLS) "$(DESTDIR)$(bindir)" > endif > ifneq ($(BLOBS),) > mkdir -p "$(DESTDIR)$(datadir)" > diff --git a/Makefile.target b/Makefile.target > index 19bb0fd..1cf7f34 100644 > --- a/Makefile.target > +++ b/Makefile.target > @@ -755,7 +755,7 @@ clean: > > install: all install-hook > ifneq ($(PROGS),) > - $(INSTALL) -m 755 -s $(PROGS) "$(DESTDIR)$(bindir)" > + $(INSTALL) -m 755 $(PROGS) "$(DESTDIR)$(bindir)"Perhaps it would be worthwhile to only do this if debug=y is set, like in commit 8e4610e (which is the equivalent of this patch for qemu-xen)? - Matthew
Olaf Hering
2013-Sep-19 07:41 UTC
Re: [PATCH] qemu-traditional: do not strip binaries during make install
On Thu, Sep 19, Matthew Daley wrote:> On Thu, Sep 19, 2013 at 6:11 AM, Olaf Hering <olaf@aepfle.de> wrote: > > @@ -243,7 +243,7 @@ endif > > install: all $(if $(BUILD_DOCS),install-doc) > > mkdir -p "$(DESTDIR)$(bindir)" > > ifneq ($(TOOLS),) > > - $(INSTALL) -m 755 -s $(TOOLS) "$(DESTDIR)$(bindir)" > > + $(INSTALL) -m 755 $(TOOLS) "$(DESTDIR)$(bindir)" > > endif > > ifneq ($(BLOBS),) > > mkdir -p "$(DESTDIR)$(datadir)" > > diff --git a/Makefile.target b/Makefile.target > > index 19bb0fd..1cf7f34 100644 > > --- a/Makefile.target > > +++ b/Makefile.target > > @@ -755,7 +755,7 @@ clean: > > > > install: all install-hook > > ifneq ($(PROGS),) > > - $(INSTALL) -m 755 -s $(PROGS) "$(DESTDIR)$(bindir)" > > + $(INSTALL) -m 755 $(PROGS) "$(DESTDIR)$(bindir)" > > Perhaps it would be worthwhile to only do this if debug=y is set, like > in commit 8e4610e (which is the equivalent of this patch for > qemu-xen)?I dont think any "make install" has to modify the binaries. If debuginfo was requested by passing "-g" via CFLAGS then the generated debuginfo has to be preserved during make install. Olaf
Ian Jackson
2013-Oct-10 17:19 UTC
Re: [PATCH] qemu-traditional: do not strip binaries during make install
Olaf Hering writes ("[PATCH] qemu-traditional: do not strip binaries during
make install"):> diff --git a/Makefile b/Makefile
> index 37c7066..594f0ef 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -243,7 +243,7 @@ endif
> install: all $(if $(BUILD_DOCS),install-doc)
> mkdir -p "$(DESTDIR)$(bindir)"
> ifneq ($(TOOLS),)
> - $(INSTALL) -m 755 -s $(TOOLS) "$(DESTDIR)$(bindir)"
> + $(INSTALL) -m 755 $(TOOLS) "$(DESTDIR)$(bindir)"
...
I agree that the existing code is wrong but I worry that we are
replacing "cannot get it not to strip" with "cannot get it to
strip".
Can you also replace the relevant $(INSTALL) with $(INSTALL_PROGRAM)
(defined appropriately if necessary) ?
Thanks,
Ian.
Olaf Hering
2013-Oct-14 15:29 UTC
Re: [PATCH] qemu-traditional: do not strip binaries during make install
On Thu, Oct 10, Ian Jackson wrote:> Olaf Hering writes ("[PATCH] qemu-traditional: do not strip binaries during make install"): > > diff --git a/Makefile b/Makefile > > index 37c7066..594f0ef 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -243,7 +243,7 @@ endif > > install: all $(if $(BUILD_DOCS),install-doc) > > mkdir -p "$(DESTDIR)$(bindir)" > > ifneq ($(TOOLS),) > > - $(INSTALL) -m 755 -s $(TOOLS) "$(DESTDIR)$(bindir)" > > + $(INSTALL) -m 755 $(TOOLS) "$(DESTDIR)$(bindir)" > ... > > I agree that the existing code is wrong but I worry that we are > replacing "cannot get it not to strip" with "cannot get it to strip". > > Can you also replace the relevant $(INSTALL) with $(INSTALL_PROGRAM) > (defined appropriately if necessary) ?I will test this patch: It is wrong to strip code during make install, unless explicit requested. Introduce a new variable INSTALL_PROG and use it along with an optional STRIP_OPT where currently install -s -m 755 is used. This is what upstream qemu offers in version 1.6. Signed-off-by: Olaf Hering <olaf@aepfle.de> --- Makefile | 2 +- Makefile.target | 2 +- configure | 1 + 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 37c7066..ed9b28a 100644 --- a/Makefile +++ b/Makefile @@ -243,7 +243,7 @@ endif install: all $(if $(BUILD_DOCS),install-doc) mkdir -p "$(DESTDIR)$(bindir)" ifneq ($(TOOLS),) - $(INSTALL) -m 755 -s $(TOOLS) "$(DESTDIR)$(bindir)" + $(INSTALL_PROG) $(STRIP_OPT) $(TOOLS) "$(DESTDIR)$(bindir)" endif ifneq ($(BLOBS),) mkdir -p "$(DESTDIR)$(datadir)" diff --git a/Makefile.target b/Makefile.target index 19bb0fd..3c3db2b 100644 --- a/Makefile.target +++ b/Makefile.target @@ -755,7 +755,7 @@ clean: install: all install-hook ifneq ($(PROGS),) - $(INSTALL) -m 755 -s $(PROGS) "$(DESTDIR)$(bindir)" + $(INSTALL_PROG) $(STRIP_OPT) $(PROGS) "$(DESTDIR)$(bindir)" endif # Include automatically generated dependency files diff --git a/configure b/configure index ace3c3e..4547359 100755 --- a/configure +++ b/configure @@ -1215,6 +1215,7 @@ echo "docdir=\${prefix}$docsuffix" >> $config_mak echo "#define CONFIG_QEMU_SHAREDIR \"$prefix$datasuffix\"" >> $config_h echo "MAKE=$make" >> $config_mak echo "INSTALL=$install" >> $config_mak +echo "INSTALL_PROG=$install -m 0755" >> $config_mak echo "CC=$cc" >> $config_mak echo "HOST_CC=$host_cc" >> $config_mak echo "AR=$ar" >> $config_mak
Olaf Hering
2013-Oct-15 09:42 UTC
[PATCH] qemu-traditional: do not strip binaries during make install
It is wrong to strip code during make install, unless explicit
requested. Introduce a new variable INSTALL_PROG and use it along with
an optional STRIP_OPT where currently install -s -m 755 is used.
This is what upstream qemu offers in version 1.6.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
Makefile | 2 +-
Makefile.target | 2 +-
configure | 1 +
3 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index 37c7066..ed9b28a 100644
--- a/Makefile
+++ b/Makefile
@@ -243,7 +243,7 @@ endif
install: all $(if $(BUILD_DOCS),install-doc)
mkdir -p "$(DESTDIR)$(bindir)"
ifneq ($(TOOLS),)
- $(INSTALL) -m 755 -s $(TOOLS) "$(DESTDIR)$(bindir)"
+ $(INSTALL_PROG) $(STRIP_OPT) $(TOOLS) "$(DESTDIR)$(bindir)"
endif
ifneq ($(BLOBS),)
mkdir -p "$(DESTDIR)$(datadir)"
diff --git a/Makefile.target b/Makefile.target
index 19bb0fd..3c3db2b 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -755,7 +755,7 @@ clean:
install: all install-hook
ifneq ($(PROGS),)
- $(INSTALL) -m 755 -s $(PROGS) "$(DESTDIR)$(bindir)"
+ $(INSTALL_PROG) $(STRIP_OPT) $(PROGS) "$(DESTDIR)$(bindir)"
endif
# Include automatically generated dependency files
diff --git a/configure b/configure
index ace3c3e..4547359 100755
--- a/configure
+++ b/configure
@@ -1215,6 +1215,7 @@ echo "docdir=\${prefix}$docsuffix" >>
$config_mak
echo "#define CONFIG_QEMU_SHAREDIR \"$prefix$datasuffix\""
>> $config_h
echo "MAKE=$make" >> $config_mak
echo "INSTALL=$install" >> $config_mak
+echo "INSTALL_PROG=$install -m 0755" >> $config_mak
echo "CC=$cc" >> $config_mak
echo "HOST_CC=$host_cc" >> $config_mak
echo "AR=$ar" >> $config_mak