Re: [Qemu-devel] [PATCH v1] Makefile: remove DESTDIR from firmware file content

[Laszlo Ersek <lersek@redhat.com>]

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"; \
>>