[FFmpeg-devel] [PATCH] Non-recursive makefiles

[Måns Rullgård]

Diego Biurrun <diego at biurrun.de> writes:

> On Sat, Mar 29, 2008 at 01:05:59PM +0000, M?ns Rullg?rd wrote:
>> M?ns Rullg?rd <mans at mansr.com> writes:
>> 
>> > This splendid patch makes the makefiles non-recursive, which is good
>> > in a number of ways.  Through some clever tricks, it is still possible
>> > to invoke make in a subdirectory to build only that library.
>> >
>> > Please test.  If you're too lazy to help me test it, at least comment.
>> 
>> And here's the latest incarnation.
>
> Has anybody tested this to work on MinGW (including shared libs and
> installation), I don't remember hearing a report of this system.
>
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -24,11 +22,19 @@ ALLPROGS    = $(addsuffix   $(EXESUF), $(BASENAMES))
>>  
>> +FF_LDFLAGS   := $(FFLDFLAGS)
>> +FF_EXTRALIBS := $(FFEXTRALIBS)
>
> Is this indirection really necessary?

Yes, or something equivalent.  Those variables get new values for each
subdir, so the proper values for the top dir must be saved.

>> +S := $(BUILD_SHARED:yes=S)
>
> Maybe you could use a better variable name here..

That would make the next line so long, and that same line reads so
nicely when the variable is called S.

>> @@ -40,21 +46,28 @@ INSTALL_TARGETS-$(BUILD_DOC)    += install-man
>>  
>> -vhook/%.o: vhook/%.c
>> -	$(CC) $(VHOOKCFLAGS) -c -o $@ $<
>> -
>> +vhook/%.o: CFLAGS:=$(VHOOKCFLAGS)
>
> This could be committed separately.  Or maybe not, it did not work as
> expected in my tests right now...

Perhaps worth investigating, then.

>> +define DOSUBDIR
>> +$(foreach V,$(SUBDIR_VARS),$(eval $(call RESET,$(V))))
>> +SUBDIR := $(1)/
>> +include $(1)/Makefile
>> +endef
>> +
>> +$(foreach D,$(FFLIBS),$(eval $(call DOSUBDIR,lib$(D))))
>
> I'm not terribly happy with D and V as variable names here.  "dirname"
> or "libname" or similar might be better instead of D.

Those names only exist within the foreach, and I prefer to keep the
lines of reasonable length.

>> --- a/configure
>> +++ b/configure
>> @@ -930,7 +930,7 @@ enable stripping
>>  
>> -SHFLAGS='-shared -Wl,-soname,$@'
>> +SHFLAGS='-shared -Wl,-soname,$(SLIBNAME_WITH_MAJOR)'
>
> This looks like it could be committed separately.

It could, but it's not necessary.

>> --- /dev/null
>> +++ b/subdir.mak
>> @@ -0,0 +1,75 @@
>> +
>> +ifeq ($(BUILD_STATIC),yes)
>
> I wonder if this can be done without the ifeq..

I can't.

> This file should be created with 'svn cp'.

If you insist, although the diff is much longer than the file.

-- 
M?ns Rullg?rd
mans at mansr.com