> 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 $@$(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 <t...@sss.pgh.pa.us> wrote: > > Donald Dong <xd...@csumb.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 $@$(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 $@$(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
explicit_link_rule_v2.patch
Description: Binary data
