[Ffmpeg-devel] Makefile consolidation

[Diego Biurrun]

On Sun, Feb 12, 2006 at 12:45:54AM +0000, M?ns Rullg?rd wrote:
> Inspired by Diego's makefile cleanup today, I took it one step
> further, and moved lots of common bits into a single file.

:-))

> I haven't committed it in case anyone should have objections.  The
> diffstat is rather convincing though:
> 
>  Makefile                        |   18 +-----
>  common.mak                      |   98 +++++++++++++++++++++++++++++++++++
>  configure                       |    2 
>  libavcodec/Makefile             |   84 +++---------------------------
>  libavcodec/libpostproc/Makefile |   67 ++---------------------
>  libavformat/Makefile            |   80 ++--------------------------
>  libavutil/Makefile              |   63 +---------------------
>  7 files changed, 133 insertions(+), 279 deletions(-)
> 
> Patch attached.  Comments welcome.

I think I really like this.  Hey, it would even be a perfect moment for
taking a stab at the auto* tools :-)

> --- configure	11 Feb 2006 20:37:12 -0000	1.248
> +++ configure	12 Feb 2006 00:39:12 -0000
> @@ -1894,6 +1894,8 @@
>      echo "SRC_PATH='$source_path'" >> config.mak
>  fi
>  
> +echo "BUILD_ROOT=$PWD" >> config.mak

Hmm, is this portable?  And how does it relate to source_path?

> --- libavcodec/libpostproc/Makefile	11 Feb 2006 20:54:40 -0000	1.42
> +++ libavcodec/libpostproc/Makefile	12 Feb 2006 00:39:12 -0000
> @@ -1,81 +1,28 @@
>  
> +CFLAGS  = $(OPTFLAGS) $(MLIB_INC) -I. -I.. -I$(SRC_PATH)/libavcodec -I../.. $(EXTRA_INC)

I wonder whether all those -I options are not redundant, but this is
outside the scope of your patch for the moment..

> --- /dev/null	2005-09-30 08:27:56.000000000 +0100
> +++ common.mak	2006-02-12 00:30:22.032867585 +0000
> @@ -0,0 +1,98 @@
> +
> +INSTLIBTARGETS = 
> +ifeq ($(BUILD_SHARED),yes)
> +INSTLIBTARGETS += install-lib-shared
> +endif
> +ifeq ($(BUILD_STATIC),yes)
> +INSTLIBTARGETS += install-lib-static
> +endif

I think you can leave out the empty variable declaration.  There's also
some trailing whitespace on that line.

> +install-headers:
> +	install -m 644 $(addprefix "$(SRC_DIR)"/,$(HEADERS)) "$(incdir)"

Oh, I notice that I likely broke out-of-tree header installation..

Diego