Re: Use LN_S instead of "ln -s" in Makefile - Mailing list pgsql-hackers
From | Tom Lane |
---|---|
Subject | Re: Use LN_S instead of "ln -s" in Makefile |
Date | |
Msg-id | 710.1581791363@sss.pgh.pa.us Whole thread Raw |
In response to | Re: Use LN_S instead of "ln -s" in Makefile (Andreas Karlsson <andreas@proxel.se>) |
List | pgsql-hackers |
Andreas Karlsson <andreas@proxel.se> writes: > On 2/15/20 1:57 AM, Tom Lane wrote: >> Hm. So, these oversights are certainly bugs in narrow terms. However, >> they've all been like that for a *long* time --- the three instances >> you found date to 2005, 2006, and 2008 according to "git blame". >> The complete lack of complaints shows that nobody cares. So I think >> a fairly strong case could be made for going the other way, ie >> s/$(LN_S)/ln -s/g and get rid of the configure-time cycles wasted in >> setting up that variable. > Here is a patch which does that. I was just about to push that when I noticed something that gave me pause: the "ln -s" in backend/Makefile is wrapped in ifneq ($(PORTNAME), win32) This means there's one popular platform where we *don't* know for sure that people aren't building in environments that don't support "ln -s". (The other two direct uses that Ashwin found are in test code that a non-developer person very likely would never exercise, so I don't think they prove much.) I'm still on balance inclined to push this. We have no buildfarm animals exercising the case (they all report "ln -s" as supported, even the Windows animals), and these days I think most people who are building for Windows use the MSVC scripts not the makefiles. Moreover, $(LN_S) is a horribly error-prone macro, because of the fact that "ln -s" and "cp" don't have the same semantics for the source argument. The Autoconf manual says If you make a link in a directory other than the current directory, its meaning depends on whether `ln' or `ln -s' is used. To safely create links using `$(LN_S)', either find out which form is used and adjust the arguments, or always invoke `ln' in the directory where the link is to be created. In other words, it does not work to do: $(LN_S) foo /x/bar Instead, do: (cd /x && $(LN_S) foo bar) So Ashwin's original patch would, far from fixing the code for symlink-less systems, just have caused them to fail in a different way. I could do without having that sort of gotcha in our build system, especially if the set of people it would help is so close to empty, and most especially when we have no testing that would catch mistakes. Nonetheless, it looks like the current makefiles do work, for moderate values of "work", on non-symlink Windows. If we apply this patch then they won't. An alternative we could consider is to go back to Ashwin's patch, after fixing it to use the "cd && ln" approach. I noticed though while chasing through the git history that that approach was in place there originally and was specifically rejected in commit ccca61b5f. That commit is quite old enough to drink, so maybe the underlying concern no longer applies --- certainly we're using "cd && ln" elsewhere. But this seems like another point in favor of the whole business being too complex/error-prone to want to support. regards, tom lane
pgsql-hackers by date: