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:

Previous
From: David Fetter
Date:
Subject: Re: Parallel copy
Next
From: Tom Lane
Date:
Subject: Re: assert pg_class.relnatts is consistent