pgxs and bison, flex

There are some extensions that build with pgxs that use bison and flex.
Their makefiles are set up to use the variables BISON and FLEX that pgxs
provides. Except that that depends on how PostgreSQL was built. A
binary package that was built in a clean chroot would probably not have
those variables set, because the programs were not present in the build
process. There have been a number of bugs related to those extensions
because of that.

I propose that we apply the attached patch to make sure those variables
are set to a usable default value in any case.

Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> There are some extensions that build with pgxs that use bison and flex.
> Their makefiles are set up to use the variables BISON and FLEX that pgxs
> provides. Except that that depends on how PostgreSQL was built. A
> binary package that was built in a clean chroot would probably not have
> those variables set, because the programs were not present in the build
> process. There have been a number of bugs related to those extensions
> because of that.

> I propose that we apply the attached patch to make sure those variables
> are set to a usable default value in any case.

Won't this break usages such as in contrib/cube?

cubeparse.c: cubeparse.y
ifdef BISON
$(BISON) $(BISONFLAGS) -o $@ $<
else
@$(missing) bison $< $@
endif

IMO, people building distribution packages in clean chroots are best
advised to include bison and flex even if they're not strictly
necessary. Otherwise the build could fall over unexpectedly and
unnecessarily, depending on file timestamps and other phase-of-the-moon
issues. If the package maker has adopted that elementary bit of
self-defense, the whole thing is a non problem.

regards, tom lane

On ons, 2012-03-28 at 22:12 -0400, Tom Lane wrote:
> Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> > I propose that we apply the attached patch to make sure those variables
> > are set to a usable default value in any case.
>
> Won't this break usages such as in contrib/cube?
>
> cubeparse.c: cubeparse.y
> ifdef BISON
> $(BISON) $(BISONFLAGS) -o $@ $<
> else
> @$(missing) bison $< $@
> endif

No, the code in my patch is conditional on 'ifdef PGXS'. (Not visible
in the patch, unfortunately.)

I don't think we want to support external use of $(missing), since the
text it refers to is specific to PostgreSQL's distribution mechanisms.

> IMO, people building distribution packages in clean chroots are best
> advised to include bison and flex even if they're not strictly
> necessary. Otherwise the build could fall over unexpectedly and
> unnecessarily, depending on file timestamps and other phase-of-the-moon
> issues. If the package maker has adopted that elementary bit of
> self-defense, the whole thing is a non problem.

I don't agree with that. If this were a problem, dozens of packages
would be liable to break randomly, and this knowledge would have passed
into best practices and policies decades ago.

In any case, it won't really help because we can't enforce it, and we
can't tell the average person who installs from source to install
additional build dependencies that the installation instructions
explicitly say are not needed.

Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> On ons, 2012-03-28 at 22:12 -0400, Tom Lane wrote:
>> Won't this break usages such as in contrib/cube?

> No, the code in my patch is conditional on 'ifdef PGXS'. (Not visible
> in the patch, unfortunately.)

Oh, okay.

> I don't think we want to support external use of $(missing), since the
> text it refers to is specific to PostgreSQL's distribution mechanisms.

Fair enough. Objection withdrawn.

regards, tom lane