[Ffmpeg-devel] Makefile consolidation

[Måns Rullgård]

Diego Biurrun <diego at biurrun.de> writes:

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

Creating a useful alternative to autotools is one of my longterm
todos.  Unfortunately, I seem to never get around to actually doing
it.

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

It's in POSIX, and is equivalent to `pwd`, but I see now that it needs
some quotes.  The variable is needed to identify the top of the build
tree, i.e. where configure was launched.  source_path is the top of
the source tree.

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

I noticed a few other potential targets for cleanup too, but I think
it's better to leave those for later.

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

You're probably right.  I just seem to recall getting some message
about += being used with an unassigned variable some time.  Maybe that
was a non-gnu make (all bets are off an AIX).

> There's also some trailing whitespace on that line.

How bad of me.

>> +install-headers:
>> +	install -m 644 $(addprefix "$(SRC_DIR)"/,$(HEADERS)) "$(incdir)"
>
> Oh, I notice that I likely broke out-of-tree header installation..

Yes, you did.  That was why I started looking at the makefiles in the
first place.  To fix it I'd have to make the same change in all of the
makefiles.  I thought it would be a better idea to collect all the
common parts in one place.

-- 
M?ns Rullg?rd
mru at inprovide.com