[FFmpeg-devel] [PATCH] Check for bugged make

[Diego Biurrun]

On Mon, Oct 27, 2008 at 09:27:07PM +0100, Stefano Sabatini wrote:
> On date Sunday 2008-10-26 11:00:12 +0100, Stefano Sabatini encoded:
> > On date Wednesday 2008-10-22 20:18:00 -0200, Ramiro Polla encoded:
> > > Stefano Sabatini wrote:
> > > > On date Wednesday 2008-08-20 10:45:35 +0200, Dominik 'Rathann' Mierzejewski encoded:
> > > >> On Wednesday, 20 August 2008 at 09:46, Chris Cormie wrote:
> > > >> [...]
> > > >>> I've also got a patch for configure that catches the 3.79.1 bug and 
> > > >>> warns the developer (below.) Could this be considered for testing and 
> > > >>> merging in?
> > > [...]
> > > > I'm reviving this patch.
> > > > 
> > > > I considered the possibility of checking the version of make, but to
> > > > check for the version of a program parsing the --version output is a
> > > > braindead solution, furthermore we have to check here for the bugged
> > > > behaviour rather than for the version.
> > > > 
> > > > I just removed the stray hunk and applied some other cosmetics.
> > > 
> > > [...]
> > > 
> > > > @@ -619,6 +619,13 @@
> > > >      check_lib $header $func "$@" || die "ERROR: $name not found"
> > > >  }
> > > >  
> > > > +check_make(){
> > > > +    log check_make "$@"
> > > > +    cat >$TMPM
> > > > +    log_file $TMPM
> > > > +    check_cmd make -f $TMPM
> > > > +}
> > > > +
> > > 
> > > Why "make"? What if the user wants to use make-3.81.exe, or gmake?
> > > 
> > > >  require2(){
> > > >      name="$1"
> > > >      headers="$2"
> > > > @@ -1139,6 +1146,7 @@
> > > >  TMPC="${TMPDIR1}/ffmpeg-conf-${RANDOM}-$$-${RANDOM}.c"
> > > >  TMPE="${TMPDIR1}/ffmpeg-conf-${RANDOM}-$$-${RANDOM}${EXESUF}"
> > > >  TMPH="${TMPDIR1}/ffmpeg-conf-${RANDOM}-$$-${RANDOM}.h"
> > > > +TMPM="${TMPDIR1}/ffmpeg-conf-${RANDOM}-$$-${RANDOM}.mak"
> > > >  TMPO="${TMPDIR1}/ffmpeg-conf-${RANDOM}-$$-${RANDOM}.o"
> > > >  TMPS="${TMPDIR1}/ffmpeg-conf-${RANDOM}-$$-${RANDOM}.S"
> > > >  TMPSH="${TMPDIR1}/ffmpeg-conf-${RANDOM}-$$-${RANDOM}.sh"
> > > > @@ -1716,6 +1724,23 @@
> > > >  EOF
> > > >  done
> > > >  
> > > > +check_make <<'EOF' || \
> > > > +die "ERROR: broken make utility. If using MinGW try a later version of
> > > > +make. make-3.81-MSYS-1.0.11-2 is known to work."
> > > 
> > > Is this bug only present on msys make? Is it not also on Linux or some 
> > > other system?
> > 
> > I have no idea, all that I know is that GNU make 3.79.1 doesn't work on
> > Windows, GNU Make 3.81 works on both.
> >  
> > > The version known to work is on the documentation, so there's no need to 
> > > put it here again.
> > 
> > Not a string opinion on this, but I think print a working make version
> > here may slightly simplify the life to the user (no need to check
> > documentation), but if someone insists I'll drop the reference to a
> > working make version.
> > 
> > > Checking on configure isn't a good idea because we don't know what make 
> > > the user will call. Maybe a better place to test would be in some 
> > > makefile...
> > 
> > Perfectly fine point.
> > 
> > > I agree that we should check for buggy makes though...
> > 
> > New version attached.
> [...]
> 
> Sligthy simplified.
> 
> Please check it, regards.

I have very mixed feelings about this.

> --- check-make.mak	(revision 0)
> +++ check-make.mak	(revision 0)

I don't like a separate file for this at the top level.

> @@ -0,0 +1,14 @@
> +# MinGW make version 3.79.1 (and earlier?) cannot handle calling a
> +# macro within a foreach statement, we check for this behaviour

behavior

Also, this is not at all specific to MinGW, GNU make 3.81 is required
everywhere.  Earlier versions do not work on any system AFAIR.

> +define a
> +  $(eval var = val)
> +endef
> +
> +TEST_LIST = a b
> +$(foreach D,$(TEST_LIST),$(eval($(call a))))
> +
> +all:
> +ifneq ($(var),val)
> +	@echo "Broken make utility detected. GNU make 3.81 is known to work, try with that."

It's not known to work, it's required.

> --- Makefile	(revision 15731)
> +++ Makefile	(working copy)
> @@ -40,8 +40,11 @@
>  
> -all: $(FF_DEP_LIBS) $(PROGS) $(ALL_TARGETS-yes)
> +all: check-make $(FF_DEP_LIBS) $(PROGS) $(ALL_TARGETS-yes)
>  
> +check-make:
> +	@$(MAKE) -f check-make.mak

Once again, very mixed feelings.

I'm not sure if this is worth all the clutter.  The place to detect such
a thing is configure, if at all.

Diego