Обсуждение: Include ppc64le build type for back branches

Sandeep Thakkar <sandeep.thakkar@enterprisedb.com> writes:
> So, config.guess should be changed to include the build type for ppc64le
> like it is in 9.4+ branches.

So far as I can tell from a quick troll of the git history, we have never
ever updated config.guess/config.sub in released branches.  I'm a bit
hesitant to do it in this case either: it would amount to retroactively
adding support for a platform, which sure sounds like a new feature.

My vote would be to adjust your buildfarm critter to only try to build
9.4 and up.
        regards, tom lane

On 12/08/2015 10:27 AM, Tom Lane wrote:
> Sandeep Thakkar <sandeep.thakkar@enterprisedb.com> writes:
>> So, config.guess should be changed to include the build type for ppc64le
>> like it is in 9.4+ branches.
> So far as I can tell from a quick troll of the git history, we have never
> ever updated config.guess/config.sub in released branches.  I'm a bit
> hesitant to do it in this case either: it would amount to retroactively
> adding support for a platform, which sure sounds like a new feature.
>
> My vote would be to adjust your buildfarm critter to only try to build
> 9.4 and up.
>
>             



Or put what he said works in his critter's build-farm.conf in the 
config_opts section, something like:

    --build=powerpc64le-unknown-linux-gnu


cheers

andrew

On Tue, Dec 8, 2015 at 12:24 PM, Andrew Dunstan <andrew@dunslane.net> wrote:
>>> So, config.guess should be changed to include the build type for ppc64le
>>> like it is in 9.4+ branches.
>>
>> So far as I can tell from a quick troll of the git history, we have never
>> ever updated config.guess/config.sub in released branches.  I'm a bit
>> hesitant to do it in this case either: it would amount to retroactively
>> adding support for a platform, which sure sounds like a new feature.
>>
>> My vote would be to adjust your buildfarm critter to only try to build
>> 9.4 and up.
>>
>
> Or put what he said works in his critter's build-farm.conf in the
> config_opts section, something like:
>
>     --build=powerpc64le-unknown-linux-gnu

I don't really want to get into an argument about this, but is the
reason we haven't updated config.guess and config.sub in the past that
it presents an actual stability risk, or just that nobody's asked
before?  Because the first one is a good reason not to do it now, but
the second one isn't.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas@gmail.com> writes:
> I don't really want to get into an argument about this, but is the
> reason we haven't updated config.guess and config.sub in the past that
> it presents an actual stability risk, or just that nobody's asked
> before?  Because the first one is a good reason not to do it now, but
> the second one isn't.

Well, I see at least one case in the git history where we explicitly
declined to update config.guess/config.sub:

Author: Tom Lane <tgl@sss.pgh.pa.us>
Branch: master Release: REL9_3_BR [5c7603c31] 2013-06-04 15:42:02 -0400
Branch: REL9_2_STABLE Release: REL9_2_5 [612ecf311] 2013-06-04 15:42:21 -0400
   Add ARM64 (aarch64) support to s_lock.h.      Use the same gcc atomic functions as we do on newer ARM chips.
(Basicallythis is a copy and paste of the __arm__ code block,   but omitting the SWPB option since that definitely
won'twork.)      Back-patch to 9.2.  The patch would work further back, but we'd also   need to update
config.guess/config.subin older branches to make them   build out-of-the-box, and there hasn't been demand for it.
MarkSalter
 


More generally, I think "does updating config.guess, in itself, pose
a stability risk" is a false statement of the issue.  The real issue is
do we want to start supporting a new platform in 9.1-9.3; that is, IMO
if we accept this request then we are buying into doing *whatever is
needed* to support ppc64le on those branches.  Maybe that will stop with
config.guess/config.sub, or maybe it won't.

Setting this precedent will also make it quite hard to reject future
requests to back-patch support for other new platforms.

I'm not planning to go to war about this issue either.  But I do think
there's a slippery-slope hazard here, and we should be prepared for
the logical consequences of accepting such a request.  Or if we're
going to have a policy allowing this request but not every such request,
somebody had better enunciate what that policy is.
        regards, tom lane

(BTW, so far as direct stability hazards go, I would guess that the
key question is how much version skew can be tolerated between autoconf
and config.guess/config.sub. I have no idea about that; Peter E. might.
But I do note that pre-9.4 branches use an older autoconf version.)

On Tue, Dec 8, 2015 at 1:35 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> I don't really want to get into an argument about this, but is the
>> reason we haven't updated config.guess and config.sub in the past that
>> it presents an actual stability risk, or just that nobody's asked
>> before?  Because the first one is a good reason not to do it now, but
>> the second one isn't.
>
> Well, I see at least one case in the git history where we explicitly
> declined to update config.guess/config.sub:
>
> Author: Tom Lane <tgl@sss.pgh.pa.us>
> Branch: master Release: REL9_3_BR [5c7603c31] 2013-06-04 15:42:02 -0400
> Branch: REL9_2_STABLE Release: REL9_2_5 [612ecf311] 2013-06-04 15:42:21 -0400
>
>     Add ARM64 (aarch64) support to s_lock.h.
>
>     Use the same gcc atomic functions as we do on newer ARM chips.
>     (Basically this is a copy and paste of the __arm__ code block,
>     but omitting the SWPB option since that definitely won't work.)
>
>     Back-patch to 9.2.  The patch would work further back, but we'd also
>     need to update config.guess/config.sub in older branches to make them
>     build out-of-the-box, and there hasn't been demand for it.
>
>     Mark Salter

Right, but that commit cites lack of demand, rather than anything else.

> More generally, I think "does updating config.guess, in itself, pose
> a stability risk" is a false statement of the issue.  The real issue is
> do we want to start supporting a new platform in 9.1-9.3; that is, IMO
> if we accept this request then we are buying into doing *whatever is
> needed* to support ppc64le on those branches.  Maybe that will stop with
> config.guess/config.sub, or maybe it won't.

I'm sympathetic to that concern.  On the other hand, if somebody were
to discover, say, a bug in s_lock.h that causes a compile failure on
ppcle, I think we'd treat that as a back-patchable fix and whack it
into all supported branches.  If somebody were to come along and say,
you can't use GetWeirdWindowsCrap() on the latest verison of Windows,
you have to instead use GetDumbWindowsCrap(), we would presumably
back-patch that as far as convenient also.  Are those kind of changes
adding support for a new platform, or just fixing bugs?  Now, on the
flip side, the original port to Windows was not back-patched, nor
should it have been: that was clearly major new development, not just
tweaking.

With respect to this particular thing, it's hard for me to imagine
that anything will go wrong on ppcle that we wouldn't consider a
back-patchable fix, so there might be no harm in adjusting
config.guess and config.sub.  On the other hand, maybe it'd be better
to get the buildfarm critter set up first (using the workaround Andrew
suggested) and then reevaluate.  If it seems to work, I see little
harm in saying, oh yeah sure it's supported, but we don't actually
know that yet.

> Setting this precedent will also make it quite hard to reject future
> requests to back-patch support for other new platforms.
>
> I'm not planning to go to war about this issue either.  But I do think
> there's a slippery-slope hazard here, and we should be prepared for
> the logical consequences of accepting such a request.  Or if we're
> going to have a policy allowing this request but not every such request,
> somebody had better enunciate what that policy is.

I don't have a real clear opinion about this just at the moment, but I
think "those changes look scary to back-patch" is a pretty decent
heuristic.  "That looks like new development" is another one I'd feel
confident to deploy.

> (BTW, so far as direct stability hazards go, I would guess that the
> key question is how much version skew can be tolerated between autoconf
> and config.guess/config.sub. I have no idea about that; Peter E. might.
> But I do note that pre-9.4 branches use an older autoconf version.)

That's a good question, and I wonder if this is why it works from 9.4.
I don't remember an explicit decision to begin supporting ppcle, and
I'm not sure in any case that I'd endorse the view that whatever
config.guess has heard of, we support.  I thought the policy was more
like "whatever we have a working BF member for, we support".

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 12/8/15 1:06 PM, Robert Haas wrote:
> I don't really want to get into an argument about this, but is the
> reason we haven't updated config.guess and config.sub in the past that
> it presents an actual stability risk, or just that nobody's asked
> before?  Because the first one is a good reason not to do it now, but
> the second one isn't.

We had some incompatibility issues with these updates in the very
distant past, but I don't think this would be an issue anymore.  The
updates themselves are better and smaller now, and the buildfarm
coverage is quite good.  It would be prudent, however, to do a manual
verification of the changes, especially in the distant back branches.

I think there is a slippery slope argument, but I don't think we have
that many new platforms and such all the time.  If people start wanting
a new arm variant every six weeks, the we'll have to put a stop to it,
perhaps.

Robert Haas <robertmhaas@gmail.com> writes:
> With respect to this particular thing, it's hard for me to imagine
> that anything will go wrong on ppcle that we wouldn't consider a
> back-patchable fix, so there might be no harm in adjusting
> config.guess and config.sub.

FWIW, I also suspect that supporting ppc64le would not really take
much more than updating config.guess/config.sub; there's no evidence
in the git logs that we had to fix anything else in the newer branches.

My concern here is about establishing project policy about whether
we will or won't consider back-patching support for newer platforms.
I think that the default answer should be "no", and I'd like to
see us set down some rules about what it takes to override that.

Obviously, setting up a buildfarm member helps a good deal.  But
is that sufficient, or necessary?

> I'm not sure in any case that I'd endorse the view that whatever
> config.guess has heard of, we support.

config.guess support is a necessary but certainly not sufficient
condition.
        regards, tom lane

On Wed, Dec 9, 2015 at 2:54 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> With respect to this particular thing, it's hard for me to imagine
>> that anything will go wrong on ppcle that we wouldn't consider a
>> back-patchable fix, so there might be no harm in adjusting
>> config.guess and config.sub.
>
> FWIW, I also suspect that supporting ppc64le would not really take
> much more than updating config.guess/config.sub; there's no evidence
> in the git logs that we had to fix anything else in the newer branches.
>
> My concern here is about establishing project policy about whether
> we will or won't consider back-patching support for newer platforms.
> I think that the default answer should be "no", and I'd like to
> see us set down some rules about what it takes to override that.
>
> Obviously, setting up a buildfarm member helps a good deal.  But
> is that sufficient, or necessary?

I would say it's necessary but not sufficient.  The other criterion
I'd lay down is that you shouldn't need to really change anything in
the code except maybe to fix up s_lock.h breakage or some equally
minor wart.  For example, suppose that somebody wanted a new platform
which works just like UNIX except that open() takes 4 arguments
instead of 3, and we've always got to pass 0 for the last one.  Well,
even though that's minor and easily done, I'm not going to argue for
back-patching that to all supported branches.  I don't think that
would be good material for a back-patch; the necessary changes could
flummox third-party code or just turn out to be buggy, and apparently
this new platform wasn't that important up until now, so whatever.

But I feel differently about this case because we basically already do
support the platform, or so it seems.  We just didn't know we
supported it.  Really, we expect our stuff to work anywhere that has a
reasonably UNIX-like environment and, hey, it'll even use atomics
automatically if it has a reasonably modern gcc.  So, win.  I don't
think that we really gain anything by refusing to admit that the
product works on a platform where it does work.  We've put a lot of
effort into being portable and I don't think we should feel bad about
that.  If we test PostgreSQL on a new platform and it works with no
problems (or only trivial adjustments that seem like good back-patch
candidates anyway), then I think it's fine to just say, yep, it works.
I don't think that sets a precedent that we'll be willing to accept
arbitrary changes in back-branches to make it work when it doesn't
already.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company