[Mesa-dev] [PATCH 1/2] meson: fix LLVM version detection when <= 3.4

[Andres Gomez]

On Wed, 2018-02-28 at 17:12 +0000, Eric Engestrom wrote:
> On Wednesday, 2018-02-28 17:08:41 +0000, Eric Engestrom wrote:
> > On Wednesday, 2018-02-28 17:02:50 +0000, Eric Engestrom wrote:
> > > On Wednesday, 2018-02-28 17:52:05 +0200, Andres Gomez wrote:
> > > > 3 digits versions in LLVM only started from 3.4.1 on. Hence, if you
> > > > have installed 3.4 or below, meson will fail even when we may not make
> > > > use of LLVM.
> > > > 
> > > > Cc: Dylan Baker <dylan at pnwbakers.com>
> > > > Cc: Eric Engestrom <eric.engestrom at imgtec.com>
> > > > Signed-off-by: Andres Gomez <agomez at igalia.com>
> > > > ---
> > > >  meson.build | 13 ++++++++++++-
> > > >  1 file changed, 12 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/meson.build b/meson.build
> > > > index 308f64cf811..b8c0b04893c 100644
> > > > --- a/meson.build
> > > > +++ b/meson.build
> > > > @@ -1037,7 +1037,18 @@ if with_llvm
> > > >    # that for our version checks.
> > > >    # svn suffixes are stripped by meson as of 0.43, and git suffixes are
> > > >    # strippped as of 0.44, but we support older meson versions.
> > > > -  _llvm_patch = _llvm_version[2]
> > > > +
> > > > +  # 3 digits versions in LLVM only started from 3.4.1 on
> > > > +  if dep_llvm.version() <= '3.3'
> > > 
> > > The correct 'meson way' to compare version strings is
> > >   if dep_llvm.version().version_compare('<= 3.3')
> > > 
> > > > +    _llvm_patch = _llvm_version[1]
> > > > +  elif dep_llvm.version() >= '3.5'
> > > > +    _llvm_patch = _llvm_version[2]
> > > > +  elif dep_llvm.version().startswith('3.4.1') or dep_llvm.version().startswith('3.4.2')
> > > > +    _llvm_patch = _llvm_version[2]
> > > > +  else
> > > > +    _llvm_patch = _llvm_version[1]
> > > > +  endif
> > > 
> > > This whole logic seems overly complicated, and I don't think duplicating
> > > the minor version as the patch version is the right thing either.

Ouch! Right, minor version should be 0 in those cases.

> > > How about this instead?
> > > 
> > >   if dep_llvm.version().version_compare('>= 3.4.1')
> > >     _llvm_patch = _llvm_version[2]
> > >   else
> > >     _llvm_patch = '0'
> > > 	endif
> > 
> > Actually, do we still support llvm < 3.4? Didn't we just bump the
> > minimum to 4.0?
> > I think we did, in which case this patch is not necessary at all :)
> 
> Correction: the minimum is 3.9, which is still >= 3.4, so NAK on this
> patch, it would be dead code anyway :)

The purpose of this patch is not to provide support for older versions
of LLVM but avoiding meson to fail when an older version is present in
the system.

In other words, you can perfectly build with an old LLVM (< 3.4.1) in
the system while not needing LLVM at all (auto). When passing through
this detection code, meson will fail when accessing "_llvm_version[2]"
due to:

"Index 2 out of bounds of array of size 2."

You can see an example of this error at:
https://travis-ci.org/Igalia/release-mesa/builds/347267445


I'll send a new version following your snippet. Thanks! ☺

-- 
Br,

Andres