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