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

[Ramiro Polla]

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".

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

[Martin Storsjö]

On Thu, 20 Mar 2025, Ramiro Polla wrote:

> 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).

Fair enough, I can resonate with that argument.

> 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.

Yes, maybe. However there are a couple more cases that muddies the 
distinction.

If building with Windows tools from within WSL (e.g. if building with MSVC 
this way), then the host system, from within configure and make, looks 
like a unix, while the build tools actually still have the same 
limitation.

The same issue also potentially arises if you build with msvc-wine (MSVC, 
wrapped in scripts with wine).

Not sure what the best distinction here would be? If the toolchain is MSVC 
or if the host OS is windows?

> 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.

Yes, this does rely on that assumption. (I did bring this up in early 
review and asked Gyan investigate to make sure this really is the case, so 
we don't just gain the extra command line space of the linker options). I 
don't see it as a very high risk right now though.

>> @@ -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?

I guess that could be done, but it makes the code a little bit more messy 
with even more ifdefs, so I'm not sure if it's worth it?

// Martin
_______________________________________________
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".

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

[Gyan Doshi]

On 2025-03-20 05:39 am, Ramiro Polla wrote:
> 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 don't think most users want to log a ~30K character command for debugging.
How about an option, choices : yes, no, auto(default)?


> 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.

I think this permutation of tools is low-probability but I have a 
workaround in mind, so I'll send that.


>> @@ -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?

The command result won't abort the script so I avoided inserting more 
guards.

Regards,
Gyan
_______________________________________________
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".