[FFmpeg-devel] [PATCH] Add versioning information to dlls

[Diego Biurrun]

On Thu, May 22, 2008 at 03:27:19PM -0400, Jeremy Kolb wrote:
>
> This should take care of the trailing whitespaces.

Ignoring for a moment that this patch has other more serious problems
because of which it was reverted...

> --- configure	(revision 13216)
> +++ configure	(working copy)
> @@ -1236,12 +1237,15 @@
>          SLIB_EXTRA_CMD='-lib /machine:$(LIBTARGET) /def:$$(@:$(SLIBSUF)=.def) /out:$(SUBDIR)$(SLIBNAME_WITH_MAJOR:$(SLIBSUF)=.lib)'
> +        SLIB_EXTRA_OBJS="dllinfo.rco"
> +        SLIB_EXTRA_DEP='$(SUBDIR)../version.h'

nit: Reverse the order to sort this alphabetically.

> @@ -2069,6 +2073,8 @@
>      echo "SLIB_EXTRA_CMD=${SLIB_EXTRA_CMD}" >> config.mak
> +    echo "SLIB_EXTRA_OBJS=${SLIB_EXTRA_OBJS}" >> config.mak
> +    echo "SLIB_EXTRA_DEP=${SLIB_EXTRA_DEP}" >> config.mak

ditto

> @@ -2215,3 +2221,69 @@
> +
> +# build dll configuration for windows.

Build DLL configuration for Windows.

> +dllinfo_generate() {
> +    name=$1
> +    description=$2
> +    version=$3
> +    major=${version%%.*}
> +    version_commas=`echo $version | tr . ,`,0
> +    dllname=`echo $name | sed s/lib//`

dllname=${name#lib}

> +
> +    license_str="This FFmpeg build is distributed under the terms of the GNU $license.\r\n"
> +    test "$license" = "unredistributable" &&
> +        license_str="This FFmpeg build may not be distributed publicly.\r\n"

This FFmpeg build contains non-free code and is not legally redistributable.

> +    cat <<EOF >$TMPRC

Space after the > please.

> +#include <WinVer.h>
> +#include "version.h"
> +VS_VERSION_INFO VERSIONINFO
> + FILEVERSION $version_commas
> + PRODUCTVERSION $version_commas
> + FILEFLAGSMASK 0x17L
> + FILEFLAGS 0x0L
> + FILEOS VOS__WINDOWS32
> + FILETYPE VFT_DLL
> + FILESUBTYPE 0x0L

Is there a reason for this weird 1-space indentation?

> +BEGIN
> +    BLOCK "StringFileInfo"
> +    BEGIN
> +        BLOCK "040904b0"
> +        BEGIN
> +            VALUE "Comments",            "$license_str"
> +                                         "Source code is available at http://ffmpeg.org"

Add a trailing / to the URL and a period to the sentence.

> +    if ! cmp -s $TMPRC $name/dllinfo.rc; then
> +        mv -f $TMPRC $name/dllinfo.rc
> +    fi
> +    rm -f $TMPRC

Is this necessary?  We regenerate config.mak and the pkg-config files
unconditionally.  config.h is treated this way because unnecessarily
recreating it triggers a lot of pointless recompiles.

> +if test "$dllinfo" = "yes"; then
> +    dllinfo_generate libavutil "FFmpeg utility library" "$LIBAVUTIL_VERSION"
> +    dllinfo_generate libavcodec "FFmpeg codec library" "$LIBAVCODEC_VERSION"
> +    dllinfo_generate libavformat "FFmpeg container format library" "$LIBAVFORMAT_VERSION"
> +    dllinfo_generate libavdevice "FFmpeg device handling library" "$LIBAVDEVICE_VERSION"
> +    dllinfo_generate libavfilter "FFmpeg video filtering library" "$LIBAVFILTER_VERSION"
> +    dllinfo_generate libpostproc "FFmpeg post processing library" "$LIBPOSTPROC_VERSION"

postprocessing

Also, you could align all those lines.

> --- subdir.mak	(revision 13216)
> +++ subdir.mak	(working copy)
> @@ -27,7 +27,9 @@
>  
> +$(SUBDIR)$(SLIB_EXTRA_OBJS): $(SLIB_EXTRA_DEP)

I think you could just generate a proper .d file from the .rco file and
be done with it.  This would simplify things considerably.

> -$(SUBDIR)$(SLIBNAME_WITH_MAJOR): $(OBJS)
> +$(SUBDIR)$(SLIBNAME_WITH_MAJOR): $(OBJS) $(SUBDIR)$(SLIB_EXTRA_OBJS)

Alternatively, add that extra object to OBJS.  This should make things
simpler.  It also likely fixes the build failures we were seeing on some
platforms.

Diego