[for-4.1] contrib/elf2dmp: Build download.o with CURL_CFLAGS
diff mbox series

Message ID 20190719100955.17180-1-peter.maydell@linaro.org
State New
Headers show
Series
  • [for-4.1] contrib/elf2dmp: Build download.o with CURL_CFLAGS
Related show

Commit Message

Peter Maydell July 19, 2019, 10:09 a.m. UTC
contrib/elf2dmp has a source file which uses curl/curl.h;
although we link the final executable with CURL_LIBS, we
forgot to build this source file with CURL_CFLAGS, so if
the curl header is in a place that's not already on the
system include path then it will fail to build.

Add a line specifying the cflags needed for download.o;
while we are here, bring the specification of the libs
into line with this, since using a per-object variable
setting is preferred over adding them to the final
executable link line.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
I'm pretty sure this is what's causing the compile
failure described at:
https://stackoverflow.com/questions/57102476/qemu-recipe-for-target-contrib-elf2dmp-download-o-failed
I haven't actually got a setup that reproduces the error,
though, so this is tested by looking at the command lines
run on an Ubuntu setup that compiles even without the fix.

There's an argument for splitting this into two patches,
I suppose, one which just fixes the CURL_CFLAGS bug and
one which tidies the CURL_LIBS handling. But it didn't
seem worth it to me. Let me know if you'd prefer it split.
---
 Makefile                      | 1 -
 contrib/elf2dmp/Makefile.objs | 3 +++
 2 files changed, 3 insertions(+), 1 deletion(-)

Comments

Marc-André Lureau July 19, 2019, 10:18 a.m. UTC | #1
Hi

On Fri, Jul 19, 2019 at 2:10 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> contrib/elf2dmp has a source file which uses curl/curl.h;
> although we link the final executable with CURL_LIBS, we
> forgot to build this source file with CURL_CFLAGS, so if
> the curl header is in a place that's not already on the
> system include path then it will fail to build.
>
> Add a line specifying the cflags needed for download.o;
> while we are here, bring the specification of the libs
> into line with this, since using a per-object variable
> setting is preferred over adding them to the final
> executable link line.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
> I'm pretty sure this is what's causing the compile
> failure described at:
> https://stackoverflow.com/questions/57102476/qemu-recipe-for-target-contrib-elf2dmp-download-o-failed
> I haven't actually got a setup that reproduces the error,
> though, so this is tested by looking at the command lines
> run on an Ubuntu setup that compiles even without the fix.
>
> There's an argument for splitting this into two patches,
> I suppose, one which just fixes the CURL_CFLAGS bug and
> one which tidies the CURL_LIBS handling. But it didn't
> seem worth it to me. Let me know if you'd prefer it split.
> ---
>  Makefile                      | 1 -
>  contrib/elf2dmp/Makefile.objs | 3 +++
>  2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index f9791dcb827..27dabb9b1a0 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -626,7 +626,6 @@ ifneq ($(EXESUF),)
>  qemu-ga: qemu-ga$(EXESUF) $(QGA_VSS_PROVIDER) $(QEMU_GA_MSI)
>  endif
>
> -elf2dmp$(EXESUF): LIBS += $(CURL_LIBS)
>  elf2dmp$(EXESUF): $(elf2dmp-obj-y)
>         $(call LINK, $^)
>
> diff --git a/contrib/elf2dmp/Makefile.objs b/contrib/elf2dmp/Makefile.objs
> index e3140f58cf7..15057169160 100644
> --- a/contrib/elf2dmp/Makefile.objs
> +++ b/contrib/elf2dmp/Makefile.objs
> @@ -1 +1,4 @@
>  elf2dmp-obj-y = main.o addrspace.o download.o pdb.o qemu_elf.o
> +
> +download.o-cflags := $(CURL_CFLAGS)
> +download.o-libs   := $(CURL_LIBS)
> --
> 2.20.1
>
>
Peter Maydell July 22, 2019, 12:06 p.m. UTC | #2
On Fri, 19 Jul 2019 at 11:18, Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
>
> Hi
>
> On Fri, Jul 19, 2019 at 2:10 PM Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > contrib/elf2dmp has a source file which uses curl/curl.h;
> > although we link the final executable with CURL_LIBS, we
> > forgot to build this source file with CURL_CFLAGS, so if
> > the curl header is in a place that's not already on the
> > system include path then it will fail to build.
> >
> > Add a line specifying the cflags needed for download.o;
> > while we are here, bring the specification of the libs
> > into line with this, since using a per-object variable
> > setting is preferred over adding them to the final
> > executable link line.
> >
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Thanks. Since we don't have an obvious route for elf2dmp patches
to go into master I'll just put this in via the arm pullreq
I'm putting together for rc2.

-- PMM

Patch
diff mbox series

diff --git a/Makefile b/Makefile
index f9791dcb827..27dabb9b1a0 100644
--- a/Makefile
+++ b/Makefile
@@ -626,7 +626,6 @@  ifneq ($(EXESUF),)
 qemu-ga: qemu-ga$(EXESUF) $(QGA_VSS_PROVIDER) $(QEMU_GA_MSI)
 endif
 
-elf2dmp$(EXESUF): LIBS += $(CURL_LIBS)
 elf2dmp$(EXESUF): $(elf2dmp-obj-y)
 	$(call LINK, $^)
 
diff --git a/contrib/elf2dmp/Makefile.objs b/contrib/elf2dmp/Makefile.objs
index e3140f58cf7..15057169160 100644
--- a/contrib/elf2dmp/Makefile.objs
+++ b/contrib/elf2dmp/Makefile.objs
@@ -1 +1,4 @@ 
 elf2dmp-obj-y = main.o addrspace.o download.o pdb.o qemu_elf.o
+
+download.o-cflags := $(CURL_CFLAGS)
+download.o-libs   := $(CURL_LIBS)