Bug 36651

Created attachment 46450 [details] [review]
patch configure.ac, Makefiles, etc to check for flex/bison

I'm not an autoconf expert but I think the attached patch will do the job.  Can you review/test?  Thanks.

With this patch I could specify what flex and bison would Mesa use but it still configures with no flex nor bison.

checking build system type... x86_64-unknown-linux-gnu
checking host system type... x86_64-unknown-linux-gnu
checking for gcc... gcc
checking whether the C compiler works... yes
checking for C compiler default output file name... a.out
checking for suffix of executables... 
checking whether we are cross compiling... no
checking for suffix of object files... o
checking whether we are using the GNU C compiler... yes
checking whether gcc accepts -g... yes
checking for gcc option to accept ISO C89... none needed
checking how to run the C preprocessor... gcc -E
checking for gcc... (cached) gcc
checking whether we are using the GNU C compiler... (cached) yes
checking whether gcc accepts -g... (cached) yes
checking for gcc option to accept ISO C89... (cached) none needed
checking for g++... g++
checking whether we are using the GNU C++ compiler... yes
checking whether g++ accepts -g... yes
checking for gmake... no
checking for make... make
checking for python2... no
checking for python... python
checking for flex... no
checking for bison... no
checking for makedepend... /usr/bin/makedepend
checking for sed... /bin/sed
checking for a BSD-compatible install... /usr/bin/install -c
checking if compiling with clang... no
checking whether gcc version is sufficient... yes
checking for pkg-config... /usr/bin/pkg-config
checking pkg-config is at least version 0.9.0... yes
checking whether gcc supports -fvisibility=hidden... yes
checking whether g++ supports -fvisibility=hidden... yes
checking whether to enable assembly... yes, x86
checking for gcc option to produce PIC... -fPIC
checking for dlopen... no
checking for dlopen in -ldl... yes
checking for posix_memalign... yes
checking pkg-config files for X11 are available... yes
checking for X11... yes
checking for GLPROTO... yes
checking for LIBDRM... yes
checking for DRI2PROTO... yes
checking for XF86VIDMODE... yes
checking for DRIGL... yes
checking for LIBDRM_RADEON... yes
checking expat.h usability... yes
checking expat.h presence... yes
checking for expat.h... yes
checking for XML_ParserCreate in -lexpat... yes
checking for llvm-config... /usr/bin/llvm-config
configure: creating ./config.status
config.status: creating configs/autoconf
config.status: executing configs commands
configure: WARNING: unrecognized options: --enable-gallium-radeon

        prefix:          /usr/local
        exec_prefix:     ${prefix}
        libdir:          ${exec_prefix}/lib
        includedir:      ${prefix}/include

        OpenGL:          yes (ES1: no ES2: no)
        OpenVG:          no

        Driver:          dri
        OSMesa:          no
        DRI drivers:     no
        DRI driver dir:  ${libdir}/dri
        Use XCB:         no
        Shared dricore:  no

        GLU:             no
        GLw:             no (Motif: no)
        glut:            no

        EGL:             no

        llvm:            yes
        llvm-config:     /usr/bin/llvm-config
        llvm-version:    2.6

        Gallium:         yes

Review of attachment 46450 [details] [review]:

In order to make the substitution work, you need to add a place for configure to put the definition of found programs. In configs/autoconf, add these two lines somewhere:

FLEX = @FLEX@
BISON = @BISON@

The rest looks good to me. There are ways to make this more portable with autoconf, but we can save that for another patch.

::: configure.ac
@@ +34,3 @@
 AC_CHECK_PROGS([PYTHON2], [python2 python])
+AC_CHECK_PROGS([FLEX], [flex])
+AC_CHECK_PROGS([BISON], [bison])

There are some autoconf macros specifically for lex/yacc, but this should do for now. However, what's missing here is that we're not checking if the tools are not available and stopping if they're not available. configure will set the program to "no" if it finds nothing. Also, I'm not sure it's a big deal, but I prefer AC_PATH_PROG if we're not checking for multiple program names. Something like this might work:

AC_PATH_PROG([FLEX], [flex])
test "x$FLEX" = "xno" && AC_MSG_ERROR([flex is needed to build mesa])
AC_PATH_PROG([BISON], [bison])
test "x$BISON" = "xno" && AC_MSG_ERROR([bison is needed to build mesa])

Created attachment 46650 [details] [review]
updated patch

Thanks for the tips, Dan.  Here's an updated patch.  Note that AC_PATH_PROG returns "" not "no" when the program is not found.

I've committed the patch: de1df26b5c11a45f2b1ff2ddc7b8ec764356aa94

AFAIK this is only a part of the solution.

The patch makes configure check for flex but does not make it fail when flex is not found.

Are you sure you're looking at the updated patch?

See:

+AC_PATH_PROG([FLEX], [flex])
+test "x$FLEX" = "x" && AC_MSG_ERROR([flex is needed to build Mesa])
+
+AC_PATH_PROG([BISON], [bison])
+test "x$BISON" = "x" && AC_MSG_ERROR([bison is needed to build Mesa])
+

I removed flex/bison and tested and configure errors/exits as expected.

Yes, with the updated patch it works as expected.

I missed the previous one where the check was introduced.

Thanks

This appears to be have regressed, and be broken again. I'm using a fresh check-out of mesa-git (head).

A recursive grep in the root of mesa-git yields nothing which matches "flex is needed to" or "bison is needed to", as seen in comment 7 below.

(In reply to Alex Perez from comment #9)
> This appears to be have regressed, and be broken again. I'm using a fresh
> check-out of mesa-git (head).
> 
> A recursive grep in the root of mesa-git yields nothing which matches "flex
> is needed to" or "bison is needed to", as seen in comment 7 below.

Are you saying that you don't get any configure error(s) when building from git without flex/bison ?

As we don't check-in any auto-generated files you _will_ need bison/flex/python/etc.. On the other hand one can build mesa without any of them when using the distribution tarballs.

Whichever the issue, please open separate bug. The original has been resolved and the code has diverged massively over the last 4 years.

Thanks