Re: [PATCH] Skip llvm bytecode generation if LLVM is missing

Hi,

On 2020-03-12 14:08:31 +0800, Craig Ringer wrote:
> On Thu, 12 Mar 2020 at 03:43, Andres Freund <andres(at)anarazel(dot)de> wrote:
>
> > On 2020-03-11 11:25:28 +0800, Craig Ringer wrote:
> > > I propose that per the attached patch PGXS should simply skip adding
> > > the automatic dependency for .bc files if clang cannot be found.
> > > Extensions may still choose to explicitly declare the rule in their
> > > own Makefile if they want to force bitcode generation.
> >
> > Hm, that seems like it could also cause silent failures (e.g. after a
> > package upgrade or such).
> >
> > How about erroring out, but with an instruction that llvm can be
> > disabled with make NO_LLVM=1 or such?
>
> I thought about that at first, but that'll only benefit people who're
> hand-compiling things, and it's already possible with
>
> make with_llvm=no ...

Well, the difference is that you'd be told about it, instead of getting
a hard to parse error message.

> The proportion of people hand-compiling is an ever-shrinking
> proportion of the user base.

Those not building themselves aren't going to care either.

> When something's nested inside an rpm specfile inside a docker
> container inside a bash script inside another Docker container on an
> AWS instance .... not so fun. They might be able to inject it into the
> environment. But often not.

Uh, I have very little pity with that argument. If you're script
building stuff, you can also specify the full dependencies.

> Extensions that explicitly must generate bytecode may add their own
> dependency rule.

You're just moving to per-extension work with that. The likely outcome
is that you're just not going to have bitcode for numerous extensions,
because there's nothing warning you against an incomplete setup. And
that hard dependency then also has to take into account whether PG was
built with llvm enabled or not. That's not a good direction.

> Or we could make skipping bytecode generation if llvm cannot be found
> at build-time something the extension can turn off with a PGXS option,
> as suggested upthread.

See above.

> I'm reluctant to go with anything that requires each existing
> extension to be patched because that introduces a huge lag time for
> this change to actually help anyone out.

That's precisely what you're proposing, though. Just in the inverse.

Greetings,

Andres Freund