diff mbox series

[FFmpeg-devel] ffbuild/common.mak: explicitly pass windres preprocessor args

Message ID 20210608050838.1071844-1-zeranoe@gmail.com
State New
Headers show
Series [FFmpeg-devel] ffbuild/common.mak: explicitly pass windres preprocessor args
Related show

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Kyle Schwarz June 8, 2021, 5:08 a.m. UTC
Binutils 2.36 no longer supports bundling args with the preprocessor
option.
---
 ffbuild/common.mak | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Martin Storsjö June 14, 2021, 12:02 p.m. UTC | #1
On Tue, 8 Jun 2021, Kyle Schwarz wrote:

> Binutils 2.36 no longer supports bundling args with the preprocessor
> option.
> ---
> ffbuild/common.mak | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/ffbuild/common.mak b/ffbuild/common.mak
> index 32f5b997b5..9fbbf89130 100644
> --- a/ffbuild/common.mak
> +++ b/ffbuild/common.mak
> @@ -90,7 +90,7 @@ COMPILE_MSA = $(call COMPILE,CC,MSAFLAGS)
> 	-$(if $(ASMSTRIPFLAGS), $(STRIP) $(ASMSTRIPFLAGS) $@)
>
> %.o: %.rc
> -	$(WINDRES) $(IFLAGS) --preprocessor "$(DEPWINDRES) -E -xc-header -DRC_INVOKED $(CC_DEPFLAGS)" -o $@ $<
> +	$(WINDRES) $(IFLAGS) --preprocessor "$(DEPWINDRES)" $(foreach ARG,-E -xc-header -DRC_INVOKED $(CC_DEPFLAGS),--preprocessor-arg "$(ARG)") -o $@ $<
>
> %.i: %.c
> 	$(CC) $(CCFLAGS) $(CC_E) $<
> -- 
> 2.31.1

If doing this, we no longer need to pass --preprocessor "$(DEPWINDRES)" at 
all (and one can remove DEPWINDRES from configure), and by not setting 
that option, we don't need to pass "-E -xcheader -DRC_INVOKED" either.

We could also just hardcode the options to pass (as it's probably safe to 
assume that windres calls a gcc compatible preprocessor), e.g. like this:

     --preprocessor-arg -MMD --preprocessor-arg -MF --preprocessor-arg 
$(@:.o=.d) --preprocessor-arg -MT --preprocessor-arg $@

But your approach of $(foreach) also seems neat. In that case, it'd be 
something like this:

$(foreach ARG,$(CC_DEPFLAGS),--preprocessor-arg "$(ARG)")

I've been considering sending a patch like this, but have held off so far, 
as it's still under discussion in 
https://sourceware.org/bugzilla/show_bug.cgi?id=27594 whether this 
behaviour change is supposed to be reverted or not. (It's under discussion 
whether it was correct of users to use the --preprocessor option like 
this, even if the documentation used to explicitly say that it was 
allowed.)

// Martin
diff mbox series

Patch

diff --git a/ffbuild/common.mak b/ffbuild/common.mak
index 32f5b997b5..9fbbf89130 100644
--- a/ffbuild/common.mak
+++ b/ffbuild/common.mak
@@ -90,7 +90,7 @@  COMPILE_MSA = $(call COMPILE,CC,MSAFLAGS)
 	-$(if $(ASMSTRIPFLAGS), $(STRIP) $(ASMSTRIPFLAGS) $@)
 
 %.o: %.rc
-	$(WINDRES) $(IFLAGS) --preprocessor "$(DEPWINDRES) -E -xc-header -DRC_INVOKED $(CC_DEPFLAGS)" -o $@ $<
+	$(WINDRES) $(IFLAGS) --preprocessor "$(DEPWINDRES)" $(foreach ARG,-E -xc-header -DRC_INVOKED $(CC_DEPFLAGS),--preprocessor-arg "$(ARG)") -o $@ $<
 
 %.i: %.c
 	$(CC) $(CCFLAGS) $(CC_E) $<