Re: Implicit make rules break test examples

> I observe that ecpg's Makefile.regress is already doing #3:
> %: %.o
> $(CC) $(CFLAGS) $< $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@
> so what we'd be talking about is moving that to some more global spot,
> probably Makefile.global. (I wonder why the target is not specified
> as $(at)$(X) here?)

Thank you for pointing that out!
I think #3 is a better choice since it's less invasive and would
potentially avoid similar problems in the future. I think may worth
the risks of breaking some extensions. I moved the rule to the
Makefile.global and added $(X) in case it's set.

I also updated the order in Makefile.linux in the same patch since
they have the same cause. I'm not sure if changes are necessary for
other platforms, and I am not able to test it, unfortunately.

I've built it again on Ubuntu and tested src/test/examples and
src/interfaces/libpq/test. There are no errors.

Thank you again for the awesome explanation,
Donald Dong

On Tue, Jan 1, 2019 at 11:54 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Donald Dong <xdong(at)csumb(dot)edu> writes:
> > Thank you for the explanation! That makes sense. It is strange that it does
> > not work for me.
>
> Yeah, I still can't account for the difference in behavior between your
> platform and mine (I tried several different ones here, and they all
> manage to build src/test/examples). However, I'm now convinced that
> we do have an issue, because I found another place that does fail on my
> platforms: src/interfaces/libpq/test gives failures like
>
> uri-regress.o: In function `main':
> uri-regress.c:58: undefined reference to `pg_printf'
>
> the reason being that the link command lists -lpgport before
> uri-regress.o, and since we only make the .a flavor of libpgport, it's
> definitely going to be order-sensitive. (This has probably been busted
> since we changed over to using snprintf.c everywhere, but nobody noticed
> because this test isn't normally run.)
>
> The reason we haven't noticed this already seems to be that in all the
> places where it matters, we have explicit link rules that order things
> like this:
>
> $(CC) $(CFLAGS) $(OBJS) $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $(at)$(X)
>
> However, the places that are having problems are trying to rely on
> gmake's implicit rule, which according to their manual is
>
> Linking a single object file
> `N' is made automatically from `N.o' by running the linker
> (usually called `ld') via the C compiler. The precise command
> used is `$(CC) $(LDFLAGS) N.o $(LOADLIBES) $(LDLIBS)'.
>
> So really the problem here is that we're doing the wrong thing by
> injecting -l switches into LDFLAGS; it would be more standard to
> put them into LDLIBS (or maybe LOADLIBES, though I think that's
> not commonly used).
>
> I hesitate to try to change that though. The places that are messing with
> that are injecting both -L and -l switches, and we want to keep putting
> the -L switches into LDFLAGS because of the strong likelihood that the
> initial (autoconf-derived) value of LDFLAGS contains -L switches; our
> switches pointing at within-tree directories need to come first.
> So the options seem to be:
>
> 1. Redefine our makefile conventions as being that internal -L switches
> go into LDFLAGS_INTERNAL but internal -l switches go into LDLIBS_INTERNAL,
> and we use the same recursive-expansion dance for LDLIBS[_INTERNAL] as for
> LDFLAGS[_INTERNAL], and we have to start mentioning LDLIBS in our explicit
> link rules. This would be a pretty invasive patch, I'm afraid, and would
> almost certainly break some third-party extensions' Makefiles. It'd be
> the cleanest solution in a green field, I think, but our Makefiles are
> hardly a green field.
>
> 2. Be sure to override the gmake implicit link rule with an explicit
> link rule everywhere --- basically like your patch, but touching more
> places. This seems like the least risky alternative in the short
> run, but we'd be highly likely to reintroduce such problems in future.
>
> 3. Replace the default implicit link rule with one of our own.
> Conceivably this also breaks some extensions' Makefiles, though
> I think that's less likely.
>
> I observe that ecpg's Makefile.regress is already doing #3:
>
> %: %.o
> $(CC) $(CFLAGS) $< $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@
>
> so what we'd be talking about is moving that to some more global spot,
> probably Makefile.global. (I wonder why the target is not specified
> as $(at)$(X) here?)
>
> I notice that the platform-specific makefiles in src/makefiles
> are generally also getting it wrong in their implicit rules for
> building shlibs, eg in Makefile.linux:
>
> # Rule for building a shared library from a single .o file
> %.so: %.o
> $(CC) $(CFLAGS) $(LDFLAGS) $(LDFLAGS_SL) -shared -o $@ $<
>
> Per this discussion, that needs to be more like
>
> $(CC) $(CFLAGS) $< $(LDFLAGS) $(LDFLAGS_SL) -shared -o $@
>
> in case a reference to libpgport or libpgcommon has been inserted
> into LDFLAGS. I'm a bit surprised that that hasn't bitten us
> already.
>
> regards, tom lane

--
Donald Dong