Re: [FFmpeg-devel] [FFmpeg-cvslog] ffbuild: use response files only if ar accepts them

* Re: [FFmpeg-devel] [FFmpeg-cvslog] ffbuild: use response files only if ar accepts them
       [not found] <20250318100614.51261412756@natalya.videolan.org>
@ 2025-03-20  0:09 ` Ramiro Polla
  2025-03-20 10:59   ` Martin Storsjö
  2025-03-20 11:44   ` Gyan Doshi
  0 siblings, 2 replies; 3+ messages in thread
From: Ramiro Polla @ 2025-03-20  0:09 UTC (permalink / raw)
  To: ffmpeg-devel

On Tue, Mar 18, 2025 at 11:06 AM Gyan Doshi <git@videolan.org> wrote:
>
> ffmpeg | branch: master | Gyan Doshi <ffmpeg@gyani.pro> | Mon Mar 17 20:57:28 2025 +0530| [740d4009656abeb006e2d43001b59d875c1a7a6a] | committer: Gyan Doshi
>
> ffbuild: use response files only if ar accepts them

Sorry to come in late to the discussion...

I'm not a big fan of response files, because they make troubleshooting
more annoying (I like copying the command that failed and running it
again on its own with all arguments). IMO this shouldn't be the
default, but only used on systems where this could be a problem (i.e.:
Windows). Also, only based on host systems, so not while
cross-compiling from Linux.

I was wondering why ld.exe/ar.exe fail with a long argument list, but
echo doesn't. After a bit of investigation, it seems that cygwin/msys2
pass arguments through STARTUPINFO::lpReserved2 and shared memory. But
only programs built for cygwin/msys2 can make use of that, otherwise
the parameters from CreateProcess() are used. So echo is a
cygwin/msys2 program, but ld.exe/ar.exe are not. If, by any chance,
you have a non-cygwin/msys2 echo.exe on your path, this will fail
again.

> > http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=740d4009656abeb006e2d43001b59d875c1a7a6a
> ---
>
>  configure           | 7 +++++++
>  ffbuild/library.mak | 8 ++++++++
>  2 files changed, 15 insertions(+)
>
> diff --git a/configure b/configure
> index f6964c4ee1..d84e32196d 100755
> --- a/configure
> +++ b/configure
> @@ -5230,6 +5230,12 @@ else
>      ar_o='$@'
>  fi
>
> +if $ar 2>&1 | grep -qi  "@.*file"; then
> +    ar_objs="true"
> +else
> +    ar_objs=""
> +fi
> +

I know this has already had another fixup commit and has moved in the
file, but it's the equivalent chunk in configure that I think should
only be tested under $host_os = <some sort of windows>.

>  add_cflags $extra_cflags
>  add_cxxflags $extra_cxxflags
>  add_objcflags $extra_objcflags
> @@ -8133,6 +8139,7 @@ DEPX86ASM=$x86asmexe
>  DEPX86ASMFLAGS=\$(X86ASMFLAGS)
>  AR=$ar
>  ARFLAGS=$arflags
> +AR_OBJS=$ar_objs
>  AR_O=$ar_o
>  AR_CMD=$ar
>  NM_CMD=$nm
> diff --git a/ffbuild/library.mak b/ffbuild/library.mak
> index 72e3872157..7e1871b74c 100644
> --- a/ffbuild/library.mak
> +++ b/ffbuild/library.mak
> @@ -35,8 +35,12 @@ OBJS += $(SHLIBOBJS)
>  endif
>  $(SUBDIR)$(LIBNAME): $(OBJS) $(STLIBOBJS)
>         $(RM) $@
> +ifeq ($(AR_OBJS),true)
>         $(Q)echo $^ > $@.objs
>         $(AR) $(ARFLAGS) $(AR_O) @$@.objs
> +else
> +       $(AR) $(ARFLAGS) $(AR_O) $^
> +endif
>         $(RANLIB) $@
>         -$(RM) $@.objs
>
> @@ -68,8 +72,12 @@ $(SUBDIR)$(SLIBNAME): $(SUBDIR)$(SLIBNAME_WITH_MAJOR)
>
>  $(SUBDIR)$(SLIBNAME_WITH_MAJOR): $(OBJS) $(SHLIBOBJS) $(SLIBOBJS) $(SUBDIR)lib$(NAME).ver
>         $(SLIB_CREATE_DEF_CMD)
> +ifeq ($(AR_OBJS),true)
>         $(Q)echo $$(filter %.o,$$^) > $$@.objs
>         $$(LD) $(SHFLAGS) $(LDFLAGS) $(LDSOFLAGS) $$(LD_O) @$$@.objs $(FFEXTRALIBS)
> +else
> +       $$(LD) $(SHFLAGS) $(LDFLAGS) $(LDSOFLAGS) $$(LD_O) $$(filter %.o,$$^) $(FFEXTRALIBS)
> +endif
>         $(SLIB_EXTRA_CMD)
>         -$(RM) $$@.objs

Is there any reason why we don't ifeq the removal of the response file as well?

Ramiro
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 3+ messages in thread