[FFmpeg-devel] [PATCH] Alternative OS/2 patch

[Diego Biurrun]

On Mon, Nov 26, 2007 at 06:11:54PM -0800, Dave Yeo wrote:
> On 11/26/07 04:53 pm, Diego Biurrun wrote:
>> On Fri, Nov 23, 2007 at 09:58:42PM -0800, Dave Yeo wrote:
>>> On 11/08/07 09:16 pm, Dave Yeo wrote:
>>>> Attached patch cleans up some of the nasty escaping by putting the 
>>>> DESCRIPTION field into a separate variable. This has the extra advantage 
>>>> that it makes it simpler to change the description. (adding things like 
>>>> vendor or bldlevel info).
>>>> Also makes config.mak more readable :)
>> The DESCRIPTION variable is only used once.  How does that make things
>> simpler?
>
> The DESCRIPTION needs to be quoted (according to the docs with single 
> quotes though double quotes do work) so we need more escaping for the 
> quotes to be written to the DEF file. Separating it allows only the 
> DESCRIPTION variable to contain escapes. It also makes it simpler to expand 
> the DESCRIPTION to be expanded by anyone who is distributing FFmpeg.

I am against moving stuff into variables that is only used once.

> --- configure	(revision 11093)
> +++ configure	(working copy)
> @@ -1217,6 +1219,35 @@
>          ;;
> +    os/2*)
> +        DESCRIPTION="'\"\$(SLIBNAME_WITH_VERSION)\"'"
> +        SHFLAGS='$(FULLNAME).def -Zdll -Zomf'
> +        SLIB_CREATE_DEF_CMD='echo LIBRARY $(FULLNAME) INITINSTANCE TERMINSTANCE > $(FULLNAME).def; \
> +          echo DESCRIPTION $(DESCRIPTION) >> $(FULLNAME).def; \
> +          echo PROTMODE >> $(FULLNAME).def; \
> +          echo CODE PRELOAD MOVEABLE DISCARDABLE >> $(FULLNAME).def; \
> +          echo DATA PRELOAD MOVEABLE MULTIPLE NONSHARED >> $(FULLNAME).def; \
> +          echo EXPORTS >> $(FULLNAME).def; \
> +          emxexp -o $(OBJS) >> $(FULLNAME).def'
> +        SLIB_EXTRA_CMD='emximp -o $(LIBPREF)$(FULLNAME)_dll.a $(FULLNAME).def; emximp -o $(LIBPREF)$(FULLNAME)_dll.lib $(FULLNAME).def'
> +        SLIB_INSTALL_EXTRA_CMD='install -m 644 $(LIBPREF)$(FULLNAME)_dll.lib $(LIBPREF)$(FULLNAME)_dll.a "$(LIBDIR)"'
> +        SLIB_UNINSTALL_EXTRA_CMD='rm -f "$(LIBDIR)"/$(LIBPREF)$(FULLNAME)_dll.lib;rm -f "$(LIBDIR)"/$(LIBPREF)$(FULLNAME)_dll.a'

nit: Please keep the same order of _dll.lib and _dll.a in the last three
lines.

I am starting to think that we should introduce a variable for DEF
files, MinGW could use it as well.

Otherwise the patch looks OK.

Diego