| Message ID | 20190530192812.17637-1-olaf@aepfle.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1] Makefile: remove DESTDIR from firmware file content | expand |
On Thu, May 30, 2019 at 09:28:11PM +0200, Olaf Hering wrote: > The resulting firmware files should only contain the runtime path. > Fixes commit 26ce90fde5c ("Makefile: install the edk2 firmware images > and their descriptors") > > Signed-off-by: Olaf Hering <olaf@aepfle.de> > --- > Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Makefile b/Makefile > index f0be624f47..61267bf1a4 100644 > --- a/Makefile > +++ b/Makefile > @@ -844,7 +844,7 @@ ifneq ($(DESCS),) > $(INSTALL_DIR) "$(DESTDIR)$(qemu_datadir)/firmware" > set -e; tmpf=$$(mktemp); trap 'rm -f -- "$$tmpf"' EXIT; \ > for x in $(DESCS); do \ > - sed -e 's,@DATADIR@,$(DESTDIR)$(qemu_datadir),' \ > + sed -e 's,@DATADIR@,$(qemu_datadir),' \ > "$(SRC_PATH)/pc-bios/descriptors/$$x" > "$$tmpf"; \ > $(INSTALL_DATA) "$$tmpf" \ > "$(DESTDIR)$(qemu_datadir)/firmware/$$x"; \ Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel
[Odd, I'm pretty sure I replied to this last week but can't find it, so again] Cc'ing Laszlo since he wrote this. On 5/30/19 9:28 PM, Olaf Hering wrote: > The resulting firmware files should only contain the runtime path. > Fixes commit 26ce90fde5c ("Makefile: install the edk2 firmware images > and their descriptors") > > Signed-off-by: Olaf Hering <olaf@aepfle.de> > --- > Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Makefile b/Makefile > index f0be624f47..61267bf1a4 100644 > --- a/Makefile > +++ b/Makefile > @@ -844,7 +844,7 @@ ifneq ($(DESCS),) > $(INSTALL_DIR) "$(DESTDIR)$(qemu_datadir)/firmware" > set -e; tmpf=$$(mktemp); trap 'rm -f -- "$$tmpf"' EXIT; \ > for x in $(DESCS); do \ > - sed -e 's,@DATADIR@,$(DESTDIR)$(qemu_datadir),' \ > + sed -e 's,@DATADIR@,$(qemu_datadir),' \ I'm fine with this patch. Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Laszlo, did you use the full path for a particular reason? > "$(SRC_PATH)/pc-bios/descriptors/$$x" > "$$tmpf"; \ > $(INSTALL_DATA) "$$tmpf" \ > "$(DESTDIR)$(qemu_datadir)/firmware/$$x"; \ >
On 06/05/19 16:46, Philippe Mathieu-Daudé wrote: > [Odd, I'm pretty sure I replied to this last week but can't find it, so > again] > Cc'ing Laszlo since he wrote this. > > On 5/30/19 9:28 PM, Olaf Hering wrote: >> The resulting firmware files should only contain the runtime path. >> Fixes commit 26ce90fde5c ("Makefile: install the edk2 firmware images >> and their descriptors") >> >> Signed-off-by: Olaf Hering <olaf@aepfle.de> >> --- >> Makefile | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/Makefile b/Makefile >> index f0be624f47..61267bf1a4 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -844,7 +844,7 @@ ifneq ($(DESCS),) >> $(INSTALL_DIR) "$(DESTDIR)$(qemu_datadir)/firmware" >> set -e; tmpf=$$(mktemp); trap 'rm -f -- "$$tmpf"' EXIT; \ >> for x in $(DESCS); do \ >> - sed -e 's,@DATADIR@,$(DESTDIR)$(qemu_datadir),' \ >> + sed -e 's,@DATADIR@,$(qemu_datadir),' \ > > I'm fine with this patch. > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > > Laszlo, did you use the full path for a particular reason? Yes. The reason was that I had no idea what DESTDIR stood for. After grepping the QEMU source tree for DESTDIR, I *still* have no idea what it stands for. DESTDIR seems to be an installation prefix that is not controlled through ./configre --prefix=... Based on memories from other projects -- with better documentation -- though, DESTDIR looks like an install-time-only pre-prefix. In that sense, the patch looks correct (and the vague expression "runtime path" in the commit message starts to make some sense). Presumably DESTDIR is used in package build scripts... Yup, the RHEL spec files use it too, for "make install". So, the patch is correct IMO: Reviewed-by: Laszlo Ersek <lersek@redhat.com> Too bad QEMU is chronically under-documented (and the commit message on the patch doesn't help much in that regard). Thanks Laszlo >> "$(SRC_PATH)/pc-bios/descriptors/$$x" > "$$tmpf"; \ >> $(INSTALL_DATA) "$$tmpf" \ >> "$(DESTDIR)$(qemu_datadir)/firmware/$$x"; \ >>
On 6/5/19 6:31 PM, Laszlo Ersek wrote: > On 06/05/19 16:46, Philippe Mathieu-Daudé wrote: >> [Odd, I'm pretty sure I replied to this last week but can't find it, so >> again] >> Cc'ing Laszlo since he wrote this. >> >> On 5/30/19 9:28 PM, Olaf Hering wrote: >>> The resulting firmware files should only contain the runtime path. >>> Fixes commit 26ce90fde5c ("Makefile: install the edk2 firmware images >>> and their descriptors") >>> >>> Signed-off-by: Olaf Hering <olaf@aepfle.de> >>> --- >>> Makefile | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/Makefile b/Makefile >>> index f0be624f47..61267bf1a4 100644 >>> --- a/Makefile >>> +++ b/Makefile >>> @@ -844,7 +844,7 @@ ifneq ($(DESCS),) >>> $(INSTALL_DIR) "$(DESTDIR)$(qemu_datadir)/firmware" >>> set -e; tmpf=$$(mktemp); trap 'rm -f -- "$$tmpf"' EXIT; \ >>> for x in $(DESCS); do \ >>> - sed -e 's,@DATADIR@,$(DESTDIR)$(qemu_datadir),' \ >>> + sed -e 's,@DATADIR@,$(qemu_datadir),' \ >> >> I'm fine with this patch. >> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> >> Laszlo, did you use the full path for a particular reason? > > Yes. The reason was that I had no idea what DESTDIR stood for. After > grepping the QEMU source tree for DESTDIR, I *still* have no idea what > it stands for. > > DESTDIR seems to be an installation prefix that is not controlled through > > ./configre --prefix=... > > Based on memories from other projects -- with better documentation -- > though, DESTDIR looks like an install-time-only pre-prefix. In that > sense, the patch looks correct (and the vague expression "runtime path" > in the commit message starts to make some sense). Presumably DESTDIR is > used in package build scripts... Yup, the RHEL spec files use it too, > for "make install". > > So, the patch is correct IMO: > > Reviewed-by: Laszlo Ersek <lersek@redhat.com> > > Too bad QEMU is chronically under-documented (and the commit message on > the patch doesn't help much in that regard). At least the git history is public :)
diff --git a/Makefile b/Makefile index f0be624f47..61267bf1a4 100644 --- a/Makefile +++ b/Makefile @@ -844,7 +844,7 @@ ifneq ($(DESCS),) $(INSTALL_DIR) "$(DESTDIR)$(qemu_datadir)/firmware" set -e; tmpf=$$(mktemp); trap 'rm -f -- "$$tmpf"' EXIT; \ for x in $(DESCS); do \ - sed -e 's,@DATADIR@,$(DESTDIR)$(qemu_datadir),' \ + sed -e 's,@DATADIR@,$(qemu_datadir),' \ "$(SRC_PATH)/pc-bios/descriptors/$$x" > "$$tmpf"; \ $(INSTALL_DATA) "$$tmpf" \ "$(DESTDIR)$(qemu_datadir)/firmware/$$x"; \
The resulting firmware files should only contain the runtime path. Fixes commit 26ce90fde5c ("Makefile: install the edk2 firmware images and their descriptors") Signed-off-by: Olaf Hering <olaf@aepfle.de> --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)