pg.ddx.io  pgsql-committers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
pgsql: Add basic TAP tests for psql's tab-completion logic.
64+ messages / 9 participants
[nested] [flat]

* pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-02 20:02  Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 2 replies; 64+ messages in thread

From: Tom Lane @ 2020-01-02 20:02 UTC (permalink / raw)
  To: pgsql-committers@lists.postgresql.org

Add basic TAP tests for psql's tab-completion logic.

Up to now, psql's tab-complete.c has had exactly no regression test
coverage.  This patch is an experimental attempt to add some.

This needs Perl's IO::Pty module, which isn't installed everywhere,
so the test script just skips all tests if that's not present.
There may be other portability gotchas too, so I await buildfarm
results with interest.

So far this just covers a few very basic keyword-completion and
query-driven-completion scenarios, which should be enough to let us
get a feel for whether this is practical at all from a portability
standpoint.  If it is, there's lots more that can be done.

Discussion: https://postgr.es/m/10967.1577562752@sss.pgh.pa.us

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/7c015045b9141cc30272930ea88cfa5df47240b7

Modified Files
--------------
configure                            |   2 +
configure.in                         |   1 +
src/Makefile.global.in               |   1 +
src/bin/psql/.gitignore              |   2 +-
src/bin/psql/Makefile                |  10 +++
src/bin/psql/t/010_tab_completion.pl | 122 +++++++++++++++++++++++++++++++++++
src/test/perl/PostgresNode.pm        |  67 +++++++++++++++++++
7 files changed, 204 insertions(+), 1 deletion(-)



^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-03 11:01  Christoph Berg <myon@debian.org>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  1 sibling, 2 replies; 64+ messages in thread

From: Christoph Berg @ 2020-01-03 11:01 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>

Re: Tom Lane 2020-01-02 <E1in6ft-0004zR-6l@gemulon.postgresql.org>
> Add basic TAP tests for psql's tab-completion logic.

The \DRD test fails on Debian/unstable:

# check case-sensitive keyword replacement
# XXX the output here might vary across readline versions
check_completion(
    "\\DRD\t",
    "\\DRD\b\b\bdrds ",
    "complete \\DRD<tab> to \\drds");


00:58:02 rm -rf '/<<PKGBUILDDIR>>/build/src/bin/psql'/tmp_check
00:58:02 /bin/mkdir -p '/<<PKGBUILDDIR>>/build/src/bin/psql'/tmp_check
00:58:02 cd /<<PKGBUILDDIR>>/build/../src/bin/psql && TESTDIR='/<<PKGBUILDDIR>>/build/src/bin/psql' PATH="/<<PKGBUILDDIR>>/build/tmp_install/usr/lib/postgresql/13/bin:$PATH" LD_LIBRARY_PATH="/<<PKGBUILDDIR>>/build/tmp_install/usr/lib/x86_64-linux-gnu"  PGPORT='65432' PG_REGRESS='/<<PKGBUILDDIR>>/build/src/bin/psql/../../../src/test/regress/pg_regress' REGRESS_SHLIB='/<<PKGBUILDDIR>>/build/src/test/regress/regress.so' /usr/bin/prove -I /<<PKGBUILDDIR>>/build/../src/test/perl/ -I /<<PKGBUILDDIR>>/build/../src/bin/psql  t/*.pl
00:58:09
00:58:09 #   Failed test 'complete \DRD<tab> to \drds'
00:58:09 #   at t/010_tab_completion.pl line 64.
00:58:09 # Looks like you failed 1 test of 12.
00:58:09 t/010_tab_completion.pl ..
00:58:09 Dubious, test returned 1 (wstat 256, 0x100)
00:58:09 Failed 1/12 subtests
00:58:09
00:58:09 Test Summary Report
00:58:09 -------------------
00:58:09 t/010_tab_completion.pl (Wstat: 256 Tests: 12 Failed: 1)
00:58:09   Failed test:  11
00:58:09   Non-zero exit status: 1
00:58:09 Files=1, Tests=12,  7 wallclock secs ( 0.01 usr  0.01 sys +  0.77 cusr  0.23 csys =  1.02 CPU)
00:58:09 Result: FAIL
00:58:09 make[2]: *** [Makefile:87: check] Error 1

https://pgdgbuild.dus.dg-i.net/job/postgresql-13-binaries/architecture=amd64,distribution=sid/444/co...


Shouldn't this print some "expected foo, got bar" diagnostics instead
of just dying?

Christoph





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-03 11:46  Michael Paquier <michael@paquier.xyz>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  1 sibling, 1 reply; 64+ messages in thread

From: Michael Paquier @ 2020-01-03 11:46 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-committers@lists.postgresql.org

On Thu, Jan 02, 2020 at 08:02:29PM +0000, Tom Lane wrote:
> Add basic TAP tests for psql's tab-completion logic.
> 
> Up to now, psql's tab-complete.c has had exactly no regression test
> coverage.  This patch is an experimental attempt to add some.
> 
> This needs Perl's IO::Pty module, which isn't installed everywhere,
> so the test script just skips all tests if that's not present.
> There may be other portability gotchas too, so I await buildfarm
> results with interest.

Reading through the commit logs, I am not a fan of this part:
+if ($ENV{with_readline} ne 'yes')
+{
+   plan skip_all => 'readline is not supported by this build';
+}
+
+# If we don't have IO::Pty, forget it, because IPC::Run depends on that
+# to support pty connections
+eval { require IO::Pty; };
+if ($@)
+{
+   plan skip_all => 'IO::Pty is needed to run this test';
+}

This has the disadvantage to have people never actually notice if the
tests are running or not because this does not generate a dependency
error.  Skipping things if libreadline is not around is perfectly fine
IMO, but I think that we should harden things for IO::Pty by removing
this skipping part, and by adding a test in configure.in's
AX_PROG_PERL_MODULES.  That would be also more consistent with the
approach we take with other tests.
--
Michael

Attachments:

  [application/pgp-signature] signature.asc (832B, ../../20200103114607.GA14380@paquier.xyz/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-03 13:52  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Christoph Berg <myon@debian.org>
  1 sibling, 2 replies; 64+ messages in thread

From: Tom Lane @ 2020-01-03 13:52 UTC (permalink / raw)
  To: Christoph Berg <myon@debian.org>; +Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>

Christoph Berg <myon@debian.org> writes:
> Re: Tom Lane 2020-01-02 <E1in6ft-0004zR-6l@gemulon.postgresql.org>
>> Add basic TAP tests for psql's tab-completion logic.

> The \DRD test fails on Debian/unstable:

Indeed.  It appears that recent libedit breaks tab-completion for
words involving a backslash, which is the fault of this upstream
commit:

http://cvsweb.netbsd.org/bsdweb.cgi/src/lib/libedit/filecomplete.c.diff?r1=1.52&r2=1.53

Basically what that's doing is applying de-backslashing to EVERY
word that completion is attempted on, whether it might be a filename
or not.  So what psql_complete sees in this test case is just "DRD"
which of course it does not recognize as a possible psql backslash
command.

I found out while investigating this that the libedit version shipping
with buster (3.1-20181209) is differently broken for the same case:
instead of inapproriate forced de-escaping of the input of the
application-specific completion function, it applies inapproriate
forced escaping to the output of said function, so that when we see
"\DRD" and return "\drds", what comes out to the user is "\\drds".
libedit apparently needs a regression test suite even worse than we do.

I was kind of despairing of fixing this last night, but in the light
of morning it occurs to me that there's a possible workaround for the
de-escape bug: we could make psql_completion ignore the passed "text"
string and look at the original input buffer, as
get_previous_words() is already doing.  I don't see any way to
dodge buster's bug, though.

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-03 14:03  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Christoph Berg <myon@debian.org>
  1 sibling, 1 reply; 64+ messages in thread

From: Tom Lane @ 2020-01-03 14:03 UTC (permalink / raw)
  To: Christoph Berg <myon@debian.org>; +Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>

Christoph Berg <myon@debian.org> writes:
> Shouldn't this print some "expected foo, got bar" diagnostics instead
> of just dying?

BTW, as far as that goes, we do: see for instance the tail end of

https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=desmoxytes&dt=2020-01-02%2020%3A04%3A03

ok 8 - offer multiple table choices
ok 9 - finish completion of one of multiple table choices
ok 10 - \r works
not ok 11 - complete \DRD<tab> to \drds

#   Failed test 'complete \DRD<tab> to \drds'
#   at t/010_tab_completion.pl line 64.
# Actual output was "\DRD"
ok 12 - \r works

Not sure why you are not seeing the "Actual output" bit in your log.
I used a "note" command to print it, maybe that's not best practice?

Also, while I'm asking for Perl advice: I can see in my editor that
there's a control-G bell character in that string, but this is far
from obvious on the web page.  I'd kind of like to get the report
to escapify control characters so that what comes out is more like

	# Actual output was "\DRD^G"
or
	# Actual output was "\\DRD\007"

or some such.  Anybody know an easy way to do that in Perl?

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-03 14:10  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Michael Paquier <michael@paquier.xyz>
  0 siblings, 0 replies; 64+ messages in thread

From: Tom Lane @ 2020-01-03 14:10 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; +Cc: pgsql-committers@lists.postgresql.org

Michael Paquier <michael@paquier.xyz> writes:
> This has the disadvantage to have people never actually notice if the
> tests are running or not because this does not generate a dependency
> error.  Skipping things if libreadline is not around is perfectly fine
> IMO, but I think that we should harden things for IO::Pty by removing
> this skipping part, and by adding a test in configure.in's
> AX_PROG_PERL_MODULES.  That would be also more consistent with the
> approach we take with other tests.

I do not think that requiring IO::Pty is practical.  It's not going
to be present in Windows installations, for starters, because it's
nonfunctional there.  I've also found that it fails to compile on
some of my older buildfarm dinosaurs.

In the case of IPC::Run, having a hard dependency is sensible because
the TAP tests pretty much can't do anything at all without it.
However, we don't need IO::Pty except for testing a few psql behaviors,
so it's fine with me if some buildfarm members don't run those tests.

There's precedent, too: see for instance
src/test/recovery/t/017_shm.pl
which is where I stole this coding technique from.

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-03 17:02  Christoph Berg <myon@debian.org>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  1 sibling, 1 reply; 64+ messages in thread

From: Christoph Berg @ 2020-01-03 17:02 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>

Re: Tom Lane 2020-01-03 <13708.1578059577@sss.pgh.pa.us>
> I found out while investigating this that the libedit version shipping
> with buster (3.1-20181209) is differently broken for the same case:

(Fwiw this wasn't spotted before because we have this LD_PRELOAD hack
that replaces libedit with readline at psql runtime. I guess that
means that the hack is pretty stable... Still, looking forward to the
day that OpenSSL is finally relicensing so we can properly link to
readline.)


Re: Tom Lane 2020-01-03 <14261.1578060227@sss.pgh.pa.us>
> > Shouldn't this print some "expected foo, got bar" diagnostics instead
> > of just dying?
> 
> BTW, as far as that goes, we do: see for instance the tail end of
> 
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=desmoxytes&dt=2020-01-02%2020%3A04%3A03
> 
> ok 8 - offer multiple table choices
> ok 9 - finish completion of one of multiple table choices
> ok 10 - \r works
> not ok 11 - complete \DRD<tab> to \drds
> 
> #   Failed test 'complete \DRD<tab> to \drds'
> #   at t/010_tab_completion.pl line 64.
> # Actual output was "\DRD"
> ok 12 - \r works
> 
> Not sure why you are not seeing the "Actual output" bit in your log.
> I used a "note" command to print it, maybe that's not best practice?

I think best practice is to use something like

like($out, qr/$pattern/, $annotation)

instead of plain "ok()" which doesn't know about the actual values
compared. The "&& !$timer->is_expired" condition can be dropped from
the test because all we care about is if the output matches.

I never really grasped in which contexts TAP is supposed to print the
full test output ("ok 10 -..."). Apparently the way the testsuite is
invoked at package build time only prints the terse failure summary in
which "note"s aren't included. Is there a switch to configure that?

> Also, while I'm asking for Perl advice: I can see in my editor that
> there's a control-G bell character in that string, but this is far
> from obvious on the web page.  I'd kind of like to get the report
> to escapify control characters so that what comes out is more like
> 
> 	# Actual output was "\DRD^G"
> or
> 	# Actual output was "\\DRD\007"
> 
> or some such.  Anybody know an easy way to do that in Perl?

I don't know for note(), but maybe like() would do that automatically.

Christoph





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-03 17:12  Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 64+ messages in thread

From: ilmari@ilmari.org @ 2020-01-03 17:12 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Christoph Berg <myon@debian.org>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

Tom Lane <tgl@sss.pgh.pa.us> writes:

> Also, while I'm asking for Perl advice: I can see in my editor that
> there's a control-G bell character in that string, but this is far
> from obvious on the web page.  I'd kind of like to get the report
> to escapify control characters so that what comes out is more like
>
> 	# Actual output was "\DRD^G"
> or
> 	# Actual output was "\\DRD\007"
>
> or some such.  Anybody know an easy way to do that in Perl?

I was going to suggest using Test::More's like() function to do the
regex check, but sadly that only escapes things that would break the TAP
stream syntax, not non-printables in general.  The next obvious thing is
Data::Dumper with the 'Useqq' option enabled, which makes it use
double-quoted-string escapes (e.g. "\a" for ^G).

The attaced patch does that, and also bumps $Test::Builder::Level so the
diagnostic references the calling line, and uses diag() instad of
note(), so it shows even in non-verbose mode.

- ilmari
-- 
"A disappointingly low fraction of the human race is,
 at any given time, on fire." - Stig Sandbeck Mathisen

Attachments:

  [text/x-diff] 0001-Escape-non-printable-characters-in-psql-tab-completi.patch (0B, ../../87o8vk1n2u.fsf@wibble.ilmari.org/2-0001-Escape-non-printable-characters-in-psql-tab-completi.patch)
  download

^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-03 17:35  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Christoph Berg <myon@debian.org>
  0 siblings, 1 reply; 64+ messages in thread

From: Tom Lane @ 2020-01-03 17:35 UTC (permalink / raw)
  To: Christoph Berg <myon@debian.org>; +Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>

Christoph Berg <myon@debian.org> writes:
> Re: Tom Lane 2020-01-03 <13708.1578059577@sss.pgh.pa.us>
>> I found out while investigating this that the libedit version shipping
>> with buster (3.1-20181209) is differently broken for the same case:

> (Fwiw this wasn't spotted before because we have this LD_PRELOAD hack
> that replaces libedit with readline at psql runtime.

You do?  I went looking in the Debian package source repo just the
other day for some evidence that that was true, and couldn't find
any, so I concluded that it was only an urban legend.  Where is that
done exactly?

Perhaps more importantly, *why* is it done?  It seems to me that it
takes a pretty fevered imagination to suppose that using libreadline
that way meets the terms of its license but just building against
the library normally would not.  Certainly when I worked for Red Hat,
their lawyers did not think there was any problem with building
Postgres using both openssl and readline.

The reason I'm concerned about this is that there's a patch on the
table [1] that will probably not behave nicely at all if it's
compiled against libedit headers and then executed with libreadline,
because it will draw the wrong conclusions about whether the
filename quoting hooks are available.  So that hack is going to
fail on you soon, especially after I add regression testing around
the filename completion stuff ;-)

>> I used a "note" command to print it, maybe that's not best practice?

> I think best practice is to use something like
> like($out, qr/$pattern/, $annotation)

I'll check into that, thanks!

			regards, tom lane

[1] https://www.postgresql.org/message-id/flat/16059-8836946734c02b84@postgresql.org





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-03 17:48  Christoph Berg <myon@debian.org>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 2 replies; 64+ messages in thread

From: Christoph Berg @ 2020-01-03 17:48 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>

Re: Tom Lane 2020-01-03 <26339.1578072930@sss.pgh.pa.us>
> Christoph Berg <myon@debian.org> writes:
> > Re: Tom Lane 2020-01-03 <13708.1578059577@sss.pgh.pa.us>
> >> I found out while investigating this that the libedit version shipping
> >> with buster (3.1-20181209) is differently broken for the same case:
> 
> > (Fwiw this wasn't spotted before because we have this LD_PRELOAD hack
> > that replaces libedit with readline at psql runtime.
> 
> You do?  I went looking in the Debian package source repo just the
> other day for some evidence that that was true, and couldn't find
> any, so I concluded that it was only an urban legend.  Where is that
> done exactly?

/usr/share/postgresql-common/pg_wrapper

https://salsa.debian.org/postgresql/postgresql-common/blob/master/pg_wrapper#L129-157

> Perhaps more importantly, *why* is it done?  It seems to me that it
> takes a pretty fevered imagination to suppose that using libreadline

Tom, claiming that things are "fevered" just because you didn't like
them is not appropriate. It's not fun working with PostgreSQL when the
tone is like that.

> that way meets the terms of its license but just building against
> the library normally would not.  Certainly when I worked for Red Hat,
> their lawyers did not think there was any problem with building
> Postgres using both openssl and readline.

I'm not starting that debate here, but Debian thinks otherwise:

https://lwn.net/Articles/428111/

> The reason I'm concerned about this is that there's a patch on the
> table [1] that will probably not behave nicely at all if it's
> compiled against libedit headers and then executed with libreadline,
> because it will draw the wrong conclusions about whether the
> filename quoting hooks are available.  So that hack is going to
> fail on you soon, especially after I add regression testing around
> the filename completion stuff ;-)

Well, so far, it worked well. (The biggest problem used to be that
libedit didn't have the history append function so it wasn't used even
with readline, but that got implemented ~2 years ago.)

Christoph





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-03 17:55  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
  0 siblings, 0 replies; 64+ messages in thread

From: Tom Lane @ 2020-01-03 17:55 UTC (permalink / raw)
  To: ilmari@ilmari.org; +Cc: Christoph Berg <myon@debian.org>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

ilmari@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes:
> Tom Lane <tgl@sss.pgh.pa.us> writes:
>> Anybody know an easy way to do that in Perl?

> I was going to suggest using Test::More's like() function to do the
> regex check, but sadly that only escapes things that would break the TAP
> stream syntax, not non-printables in general.  The next obvious thing is
> Data::Dumper with the 'Useqq' option enabled, which makes it use
> double-quoted-string escapes (e.g. "\a" for ^G).

> The attaced patch does that, and also bumps $Test::Builder::Level so the
> diagnostic references the calling line, and uses diag() instad of
> note(), so it shows even in non-verbose mode.

LGTM, pushed (along with a fix to deal with what hopefully is the
only remaining obstacle for Andres' critters).

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-03 17:59  Robert Haas <robertmhaas@gmail.com>
  parent: Christoph Berg <myon@debian.org>
  1 sibling, 1 reply; 64+ messages in thread

From: Robert Haas @ 2020-01-03 17:59 UTC (permalink / raw)
  To: Christoph Berg <myon@debian.org>; Tom Lane <tgl@sss.pgh.pa.us>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

On Fri, Jan 3, 2020 at 12:48 PM Christoph Berg <myon@debian.org> wrote:
> > Perhaps more importantly, *why* is it done?  It seems to me that it
> > takes a pretty fevered imagination to suppose that using libreadline
>
> Tom, claiming that things are "fevered" just because you didn't like
> them is not appropriate. It's not fun working with PostgreSQL when the
> tone is like that.

+1.

> > that way meets the terms of its license but just building against
> > the library normally would not.  Certainly when I worked for Red Hat,
> > their lawyers did not think there was any problem with building
> > Postgres using both openssl and readline.
>
> I'm not starting that debate here, but Debian thinks otherwise:
>
> https://lwn.net/Articles/428111/

I take no position on whether Debian is correct in its assessment of
such things, but I reiterate my previous opposition to breaking it
just because we don't agree with it, or because Tom specifically
doesn't. It's too mainstream a platform to arbitrarily break. And it
will probably just have the effect of increasing the number of patches
they're carrying against our sources, which will not make things
better for anybody.

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





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-03 18:17  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Christoph Berg <myon@debian.org>
  1 sibling, 0 replies; 64+ messages in thread

From: Tom Lane @ 2020-01-03 18:17 UTC (permalink / raw)
  To: Christoph Berg <myon@debian.org>; +Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>

Christoph Berg <myon@debian.org> writes:
> Re: Tom Lane 2020-01-03 <26339.1578072930@sss.pgh.pa.us>
>> You do?  I went looking in the Debian package source repo just the
>> other day for some evidence that that was true, and couldn't find
>> any, so I concluded that it was only an urban legend.  Where is that
>> done exactly?

> /usr/share/postgresql-common/pg_wrapper
> https://salsa.debian.org/postgresql/postgresql-common/blob/master/pg_wrapper#L129-157

Oh, so not in the Postgres package per se.

What that means is that our regression tests will pass (as it's
just a regular libedit install while they're running) but then filename
completion will not work well for actual users.  And there's not really
anything I can do about that from this end.

(On the other hand, filename completion is already kind of buggy,
which is why that patch exists in the first place.  So maybe it
won't get any worse.  Hard to say.)

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-04 01:32  Peter Geoghegan <pg@bowt.ie>
  parent: Robert Haas <robertmhaas@gmail.com>
  0 siblings, 1 reply; 64+ messages in thread

From: Peter Geoghegan @ 2020-01-04 01:32 UTC (permalink / raw)
  To: Robert Haas <robertmhaas@gmail.com>; +Cc: Christoph Berg <myon@debian.org>; Tom Lane <tgl@sss.pgh.pa.us>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

On Fri, Jan 3, 2020 at 9:59 AM Robert Haas <robertmhaas@gmail.com> wrote:
> I take no position on whether Debian is correct in its assessment of
> such things, but I reiterate my previous opposition to breaking it
> just because we don't agree with it, or because Tom specifically
> doesn't. It's too mainstream a platform to arbitrarily break. And it
> will probably just have the effect of increasing the number of patches
> they're carrying against our sources, which will not make things
> better for anybody.

Even with commit 56a3921a, "make check-world" is broken on my Ubuntu
18.04 workstation. This is now adversely impacting my work, so I hope
it can be resolved soon.

Not sure if the specifics matter, but FWIW "make check-world" ended
with the following failure just now:

make[2]: Entering directory '/code/postgresql/patch/build/src/bin/psql'
rm -rf '/code/postgresql/patch/build/src/bin/psql'/tmp_check
/bin/mkdir -p '/code/postgresql/patch/build/src/bin/psql'/tmp_check
cd /code/postgresql/patch/build/../source/src/bin/psql &&
TESTDIR='/code/postgresql/patch/build/src/bin/psql'
PATH="/code/postgresql/patch/build/tmp_install/code/postgresql/patch/install/bin:$PATH"
LD_LIBRARY_PATH="/code/postgresql/patch/build/tmp_install/code/postgresql/patch/install/lib"
 PGPORT='65432'
PG_REGRESS='/code/postgresql/patch/build/src/bin/psql/../../../src/test/regress/pg_regress'
REGRESS_SHLIB='/code/postgresql/patch/build/src/test/regress/regress.so'
/usr/bin/prove -I
/code/postgresql/patch/build/../source/src/test/perl/ -I
/code/postgresql/patch/build/../source/src/bin/psql  t/*.pl
t/010_tab_completion.pl .. 8/?
#   Failed test 'offer multiple table choices'
#   at t/010_tab_completion.pl line 105.
# Actual output was "\r\n\e[01;35mmytab\e[0m\e[K123\e[0m\e[K
\e[01;35mmytab\e[0m\e[K246\e[0m\e[K  \r\npostgres=# select * from
mytab\r\n\e[01;35mmytab\e[0m\e[K123\e[0m\e[K
\e[01;35mmytab\e[0m\e[K246\e[0m\e[K  \r\npostgres=# select * from
mytab"
#
# Looks like you failed 1 test of 12.
t/010_tab_completion.pl .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/12 subtests

Test Summary Report
-------------------
t/010_tab_completion.pl (Wstat: 256 Tests: 12 Failed: 1)
  Failed test:  8
  Non-zero exit status: 1
Files=1, Tests=12,  7 wallclock secs ( 0.02 usr  0.00 sys +  0.80 cusr
 0.09 csys =  0.91 CPU)
Result: FAIL
Makefile:87: recipe for target 'check' failed
make[2]: *** [check] Error 1
make[2]: Leaving directory '/code/postgresql/patch/build/src/bin/psql'
Makefile:41: recipe for target 'check-psql-recurse' failed
make[1]: *** [check-psql-recurse] Error 2
make[1]: Leaving directory '/code/postgresql/patch/build/src/bin'
GNUmakefile:70: recipe for target 'check-world-src/bin-recurse' failed
make: *** [check-world-src/bin-recurse] Error 2

--
Peter Geoghegan





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-04 02:16  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Peter Geoghegan <pg@bowt.ie>
  0 siblings, 1 reply; 64+ messages in thread

From: Tom Lane @ 2020-01-04 02:16 UTC (permalink / raw)
  To: Peter Geoghegan <pg@bowt.ie>; +Cc: Robert Haas <robertmhaas@gmail.com>; Christoph Berg <myon@debian.org>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

Peter Geoghegan <pg@bowt.ie> writes:
> Not sure if the specifics matter, but FWIW "make check-world" ended
> with the following failure just now:

> #   Failed test 'offer multiple table choices'
> #   at t/010_tab_completion.pl line 105.
> # Actual output was "\r\n\e[01;35mmytab\e[0m\e[K123\e[0m\e[K
> \e[01;35mmytab\e[0m\e[K246\e[0m\e[K  \r\npostgres=# select * from
> mytab\r\n\e[01;35mmytab\e[0m\e[K123\e[0m\e[K
> \e[01;35mmytab\e[0m\e[K246\e[0m\e[K  \r\npostgres=# select * from
> mytab"

Huh.  What readline or libedit version are you using, on what
platform?  I'm curious also what is your prevailing setting
of TERM?  (I've been wondering if the test doesn't need to
force that to something standard.  The buildfarm hasn't shown
any signs of needing that, but manual invocations might be
a different story.)

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-04 02:37  Peter Geoghegan <pg@bowt.ie>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 64+ messages in thread

From: Peter Geoghegan @ 2020-01-04 02:37 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Robert Haas <robertmhaas@gmail.com>; Christoph Berg <myon@debian.org>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

On Fri, Jan 3, 2020 at 6:16 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Huh.  What readline or libedit version are you using, on what
> platform?

Ubuntu 18.04. I used ldd to verify that psql links to the system
libreadline, which is libreadline7:amd64 -- that's what Debian
packages as "7.0-3".

> I'm curious also what is your prevailing setting
> of TERM?

I use zsh, with a fairly customized setup. $TERM is "xterm-256color"
in the affected shell. (I have a feeling that this has something to do
with my amazing technicolor terminal.)

-- 
Peter Geoghegan





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-04 02:51  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Peter Geoghegan <pg@bowt.ie>
  0 siblings, 1 reply; 64+ messages in thread

From: Tom Lane @ 2020-01-04 02:51 UTC (permalink / raw)
  To: Peter Geoghegan <pg@bowt.ie>; +Cc: Robert Haas <robertmhaas@gmail.com>; Christoph Berg <myon@debian.org>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

Peter Geoghegan <pg@bowt.ie> writes:
>> I'm curious also what is your prevailing setting
>> of TERM?

> I use zsh, with a fairly customized setup. $TERM is "xterm-256color"
> in the affected shell. (I have a feeling that this has something to do
> with my amazing technicolor terminal.)

Hmm.   If you set it to plain "xterm", does the test pass?

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-04 03:06  Peter Geoghegan <pg@bowt.ie>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 64+ messages in thread

From: Peter Geoghegan @ 2020-01-04 03:06 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Robert Haas <robertmhaas@gmail.com>; Christoph Berg <myon@debian.org>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

On Fri, Jan 3, 2020 at 6:51 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Hmm.   If you set it to plain "xterm", does the test pass?

No. Also tried setting PG_COLOR="off" and CLICOLOR=0 -- that also
didn't help. (This was based on possibly-relevant vars that "env"
showed were set).

-- 
Peter Geoghegan





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-04 03:28  Peter Geoghegan <pg@bowt.ie>
  parent: Peter Geoghegan <pg@bowt.ie>
  0 siblings, 1 reply; 64+ messages in thread

From: Peter Geoghegan @ 2020-01-04 03:28 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Robert Haas <robertmhaas@gmail.com>; Christoph Berg <myon@debian.org>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

On Fri, Jan 3, 2020 at 7:06 PM Peter Geoghegan <pg@bowt.ie> wrote:
> No. Also tried setting PG_COLOR="off" and CLICOLOR=0 -- that also
> didn't help. (This was based on possibly-relevant vars that "env"
> showed were set).

Removing the single check_completion() test from 010_tab_completion.pl
that actually fails on my system ("offer multiple table choices")
fixes the problem for me -- everything else passes.

I suppose that this means that the problem is in "offer multiple table
choices" specifically.

-- 
Peter Geoghegan





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-04 05:30  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Peter Geoghegan <pg@bowt.ie>
  0 siblings, 1 reply; 64+ messages in thread

From: Tom Lane @ 2020-01-04 05:30 UTC (permalink / raw)
  To: Peter Geoghegan <pg@bowt.ie>; +Cc: Robert Haas <robertmhaas@gmail.com>; Christoph Berg <myon@debian.org>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

Peter Geoghegan <pg@bowt.ie> writes:
> On Fri, Jan 3, 2020 at 7:06 PM Peter Geoghegan <pg@bowt.ie> wrote:
>> No. Also tried setting PG_COLOR="off" and CLICOLOR=0 -- that also
>> didn't help. (This was based on possibly-relevant vars that "env"
>> showed were set).

Yeah, that's not terribly surprising, because if I'm reading those
escape sequences correctly they're not about color.  They seem to be
just cursor movement and line clearing, according to [1].

What I'm mystified by is why your copy of libreadline is choosing to
do that, rather than just space over to where the word should be printed
which is what every other copy seems to be doing.  I have a fresh new
Debian installation at hand, with

$ dpkg -l | grep readline
ii  libreadline-dev:amd64                 7.0-5                                amd64        GNU readline and history libraries, development files
ii  libreadline5:amd64                    5.2+dfsg-3+b13                       amd64        GNU readline and history libraries, run-time libraries
ii  libreadline7:amd64                    7.0-5                                amd64        GNU readline and history libraries, run-time libraries
ii  readline-common                       7.0-5                                all          GNU readline and history libraries, common files

and I'm not seeing the failure on it, either with TERM=xterm
or with TERM=xterm-256color.  So what's the missing ingredient?

> Removing the single check_completion() test from 010_tab_completion.pl
> that actually fails on my system ("offer multiple table choices")
> fixes the problem for me -- everything else passes.
> I suppose that this means that the problem is in "offer multiple table
> choices" specifically.

I'd hate to conclude that we can't test any completion behavior that
involves offering a list.

If we can't coerce libreadline into being less avant-garde in its
screen management, I suppose we could write a regex to recognize
xterm escape sequences and ignore those.  But I'd be happier about
this if I could reproduce the behavior.  I don't like the feeling
that there's something going on here that I don't understand.

BTW, it seems somewhat likely that this is less about libreadline
than about its dependency libtinfo.  On my machine that's from

ii  libtinfo6:amd64                       6.1+20181013-2+deb10u2               amd64        shared low-level terminfo library for terminal handling

what about yours?

			regards, tom lane

[1] https://www.xfree86.org/current/ctlseqs.html





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-04 05:38  Peter Geoghegan <pg@bowt.ie>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 64+ messages in thread

From: Peter Geoghegan @ 2020-01-04 05:38 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Robert Haas <robertmhaas@gmail.com>; Christoph Berg <myon@debian.org>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

On Fri, Jan 3, 2020 at 9:30 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> BTW, it seems somewhat likely that this is less about libreadline
> than about its dependency libtinfo.  On my machine that's from
>
> ii  libtinfo6:amd64                       6.1+20181013-2+deb10u2               amd64        shared low-level terminfo library for terminal handling

This seems promising. By following the same ldd + dpkg -S workflow as
before, I can see that my libtinfo is "libtinfo5:amd64". This libtinfo
appears to be a Ubuntu-specific package:

$ dpkg -l libtinfo5:amd64
Desired=Unknown/Install/Remove/Purge/Hold
| Status=Not/Inst/Conf-files/Unpacked/halF-conf/Half-inst/trig-aWait/Trig-pend
|/ Err?=(none)/Reinst-required (Status,Err: uppercase=bad)
||/ Name                         Version             Architecture
  Description
+++-============================-===================-===================-==============================================================
ii  libtinfo5:amd64              6.1-1ubuntu1.18.04  amd64
  shared low-level terminfo library for terminal handling

-- 
Peter Geoghegan





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-04 06:39  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Peter Geoghegan <pg@bowt.ie>
  0 siblings, 2 replies; 64+ messages in thread

From: Tom Lane @ 2020-01-04 06:39 UTC (permalink / raw)
  To: Peter Geoghegan <pg@bowt.ie>; +Cc: Robert Haas <robertmhaas@gmail.com>; Christoph Berg <myon@debian.org>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

Peter Geoghegan <pg@bowt.ie> writes:
> On Fri, Jan 3, 2020 at 9:30 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> BTW, it seems somewhat likely that this is less about libreadline
>> than about its dependency libtinfo.  On my machine that's from
>> ii  libtinfo6:amd64                       6.1+20181013-2+deb10u2               amd64        shared low-level terminfo library for terminal handling

> This seems promising. By following the same ldd + dpkg -S workflow as
> before, I can see that my libtinfo is "libtinfo5:amd64".

Hmm.  Usually this sort of software gets more weird in newer
versions, not less so ;-).  Still, it's a starting point.

Attached is a blind attempt to fix this by allowing escape
sequence(s) instead of spaces between the words.  Does this
work for you?

			regards, tom lane

Attachments:

  [text/x-diff] allow-escape-sequences-1.patch (1.2K, ../../8038.1578119995@sss.pgh.pa.us/2-allow-escape-sequences-1.patch)
  download | inline diff:
diff --git a/src/bin/psql/t/010_tab_completion.pl b/src/bin/psql/t/010_tab_completion.pl
index 96221f8..7f1797c 100644
--- a/src/bin/psql/t/010_tab_completion.pl
+++ b/src/bin/psql/t/010_tab_completion.pl
@@ -38,6 +38,12 @@ $node->safe_psql('postgres',
 my $historyfile = "${TestLib::log_path}/010_psql_history.txt";
 $ENV{PSQL_HISTORY} = $historyfile;
 
+# Ensure that readline/libedit puts out xterm escapes, not something else.
+$ENV{TERM} = 'xterm';
+
+# regexp to match one xterm escape sequence (CSI style only, for now)
+my $escseq = "(\e\\[[0-9;]*[A-Za-z])";
+
 # fire up an interactive psql session
 my $in  = '';
 my $out = '';
@@ -101,8 +107,12 @@ check_completion(
 	"select \\* from my\a?tab",
 	"complete my<tab> to mytab when there are multiple choices");
 
-# some versions of readline/libedit require two tabs here, some only need one
-check_completion("\t\t", "mytab123 +mytab246",
+# some versions of readline/libedit require two tabs here, some only need one.
+# also, some might issue escape sequences to reposition the cursor, instead
+# of just printing some spaces.
+check_completion(
+	"\t\t",
+	"mytab$escseq*123( +|$escseq+)mytab$escseq*246",
 	"offer multiple table choices");
 
 check_completion("2\t", "246 ",

^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-04 06:49  Peter Geoghegan <pg@bowt.ie>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  1 sibling, 1 reply; 64+ messages in thread

From: Peter Geoghegan @ 2020-01-04 06:49 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Robert Haas <robertmhaas@gmail.com>; Christoph Berg <myon@debian.org>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

On Fri, Jan 3, 2020 at 10:39 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Attached is a blind attempt to fix this by allowing escape
> sequence(s) instead of spaces between the words.  Does this
> work for you?

I'm afraid not; no apparent change. No change in the "Actual output
was" line, either.

-- 
Peter Geoghegan





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-04 07:00  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Peter Geoghegan <pg@bowt.ie>
  0 siblings, 1 reply; 64+ messages in thread

From: Tom Lane @ 2020-01-04 07:00 UTC (permalink / raw)
  To: Peter Geoghegan <pg@bowt.ie>; +Cc: Robert Haas <robertmhaas@gmail.com>; Christoph Berg <myon@debian.org>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

Peter Geoghegan <pg@bowt.ie> writes:
> On Fri, Jan 3, 2020 at 10:39 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Attached is a blind attempt to fix this by allowing escape
>> sequence(s) instead of spaces between the words.  Does this
>> work for you?

> I'm afraid not; no apparent change. No change in the "Actual output
> was" line, either.

Meh.  I must be too tired to get the regexp syntax right.
Will try tomorrow.

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-04 07:01  Peter Geoghegan <pg@bowt.ie>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  1 sibling, 0 replies; 64+ messages in thread

From: Peter Geoghegan @ 2020-01-04 07:01 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Robert Haas <robertmhaas@gmail.com>; Christoph Berg <myon@debian.org>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

On Fri, Jan 3, 2020 at 10:39 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Hmm.  Usually this sort of software gets more weird in newer
> versions, not less so ;-).  Still, it's a starting point.

In case I was unclear: I meant to suggest that this may have something
to do with Ubuntu having patched the Debian package for who-knows-what
reason. This is indicated by the fact that the Version string is
"6.1-1ubuntu1.18.04", as opposed to a Debian style Version without the
"ubuntu" (I believe that that's the convention they follow).

-- 
Peter Geoghegan





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-04 18:19  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 64+ messages in thread

From: Tom Lane @ 2020-01-04 18:19 UTC (permalink / raw)
  To: Peter Geoghegan <pg@bowt.ie>; +Cc: Robert Haas <robertmhaas@gmail.com>; Christoph Berg <myon@debian.org>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

I wrote:
> Meh.  I must be too tired to get the regexp syntax right.

Looking closer, I see that your actual output included *both*
spaces and escape sequences between the table names, so it
needs to be more like the attached.

Also, I apparently misread the control sequences.  What they
look like in the light of morning is

\e[0m		Character Attributes = Normal (no bold, color, etc)
\e[K		Erase in Line to Right

So now I'm thinking again that there must be something about
your colorized setup that triggers use of at least the first one.
But why didn't clearing the relevant environment variables
change anything?

			regards, tom lane

Attachments:

  [text/x-diff] allow-escape-sequences-2.patch (1.2K, ../../25739.1578161968@sss.pgh.pa.us/2-allow-escape-sequences-2.patch)
  download | inline diff:
diff --git a/src/bin/psql/t/010_tab_completion.pl b/src/bin/psql/t/010_tab_completion.pl
index 96221f8..ed9e9e1 100644
--- a/src/bin/psql/t/010_tab_completion.pl
+++ b/src/bin/psql/t/010_tab_completion.pl
@@ -38,6 +38,12 @@ $node->safe_psql('postgres',
 my $historyfile = "${TestLib::log_path}/010_psql_history.txt";
 $ENV{PSQL_HISTORY} = $historyfile;
 
+# Ensure that readline/libedit puts out xterm escapes, not something else.
+$ENV{TERM} = 'xterm';
+
+# regexp to match one xterm escape sequence (CSI style only, for now)
+my $escseq = "(\e\\[[0-9;]*[A-Za-z])";
+
 # fire up an interactive psql session
 my $in  = '';
 my $out = '';
@@ -101,8 +107,12 @@ check_completion(
 	"select \\* from my\a?tab",
 	"complete my<tab> to mytab when there are multiple choices");
 
-# some versions of readline/libedit require two tabs here, some only need one
-check_completion("\t\t", "mytab123 +mytab246",
+# some versions of readline/libedit require two tabs here, some only need one.
+# also, some might issue escape sequences to reposition the cursor, clear the
+# line, etc, instead of just printing some spaces.
+check_completion(
+	"\t\t",
+	"mytab$escseq*123( |$escseq)+mytab$escseq*246",
 	"offer multiple table choices");
 
 check_completion("2\t", "246 ",

^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-04 18:38  Peter Geoghegan <pg@bowt.ie>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 64+ messages in thread

From: Peter Geoghegan @ 2020-01-04 18:38 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Robert Haas <robertmhaas@gmail.com>; Christoph Berg <myon@debian.org>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

On Sat, Jan 4, 2020 at 10:19 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Looking closer, I see that your actual output included *both*
> spaces and escape sequences between the table names, so it
> needs to be more like the attached.

This patch makes the tests pass. Thanks!

> So now I'm thinking again that there must be something about
> your colorized setup that triggers use of at least the first one.
> But why didn't clearing the relevant environment variables
> change anything?

I don't know. It also failed with bash, which doesn't have any of that stuff.

-- 
Peter Geoghegan





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-04 19:50  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Peter Geoghegan <pg@bowt.ie>
  0 siblings, 2 replies; 64+ messages in thread

From: Tom Lane @ 2020-01-04 19:50 UTC (permalink / raw)
  To: Peter Geoghegan <pg@bowt.ie>; +Cc: Robert Haas <robertmhaas@gmail.com>; Christoph Berg <myon@debian.org>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

Peter Geoghegan <pg@bowt.ie> writes:
> On Sat, Jan 4, 2020 at 10:19 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Looking closer, I see that your actual output included *both*
>> spaces and escape sequences between the table names, so it
>> needs to be more like the attached.

> This patch makes the tests pass. Thanks!

Ah, good.  Pushed.

>> So now I'm thinking again that there must be something about
>> your colorized setup that triggers use of at least the first one.
>> But why didn't clearing the relevant environment variables
>> change anything?

> I don't know. It also failed with bash, which doesn't have any of that stuff.

I got another data point just now.  I was experimenting with a proposed
fix for the current libedit problem [1], so I needed to build libedit
from source, and the path of least resistance was to do so on my RHEL6
workstation.  I find that that conglomeration fails the current
010_tab_completion.pl tests, even with this patch, with symptoms like

not ok 2 - complete SEL<tab> to SELECT

#   Failed test 'complete SEL<tab> to SELECT'
#   at t/010_tab_completion.pl line 91.
# Actual output was "postgres=# SEL\a\r\e[15GECT "

and similarly in a couple other tests.  I know where the bell (\a)
is coming from: there's a different logic bug in libedit that causes
it to do el_beep() even when there's a unique completion.  But why the
unnecessary cursor repositioning?  Looking closer, I realize that
libedit on this software stack is depending on

        libtinfo.so.5 => /lib64/libtinfo.so.5 (0x00007fa39a5e2000)

which is from

$ rpm -qf /lib64/libtinfo.so.5
ncurses-libs-5.7-4.20090207.el6.x86_64

Seeing that you're also having issues with a stack involving
libtinfo.so.5, here's my theory: libtinfo version 5 is brain-dead
about whether it needs to issue cursor repositioning commands, and
tends to do so even when the cursor is in the right place already.
Version 6 fixed that, which is why we're not seeing these escape
sequences on any of the libedit-using buildfarm critters.

So we're going to have to make some decisions about what range of
libedit builds we want to cater for in the tab-completion tests.
Given that the plan is to considerably increase the coverage of
those tests, I'm hesitant to promise that we'll keep passing with
anything that isn't explicitly tested in the buildfarm, or by active
developers such as yourself.  Maybe that's fine given that end users
probably don't run the TAP tests.

It would likely help if we could constrain the behavior to avoid
variations such as colorization, which is why I'm concerned that
we don't seem to have figured out how to turn that off in your
setup.  I think that bears more investigation, even though we've
managed to work around it for the moment.

			regards, tom lane

[1] http://gnats.netbsd.org/cgi-bin/query-pr-single.pl?number=54510





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-04 19:58  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  1 sibling, 2 replies; 64+ messages in thread

From: Tom Lane @ 2020-01-04 19:58 UTC (permalink / raw)
  To: Peter Geoghegan <pg@bowt.ie>; +Cc: Robert Haas <robertmhaas@gmail.com>; Christoph Berg <myon@debian.org>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

I wrote:
> Seeing that you're also having issues with a stack involving
> libtinfo.so.5, here's my theory: libtinfo version 5 is brain-dead
> about whether it needs to issue cursor repositioning commands, and
> tends to do so even when the cursor is in the right place already.
> Version 6 fixed that, which is why we're not seeing these escape
> sequences on any of the libedit-using buildfarm critters.

Nope, the buildfarm just blew up that theory: Andres' critters are
failing in the wake of fac1c04fe, with symptoms exactly like those
of my franken-libedit build.  So newer libtinfo doesn't fix it.

What has to have broken those machines was the change to explicitly
force TERM to "xterm".  Now I'm wondering what their prevailing
setting was before that.  Maybe it was undef, or some absolutely
vanilla thing that prevents libtinfo from thinking it can use any
escape sequences at all.  I'm going to go find out, because if we
can use that behavior globally, it'd be a heck of a lot safer
solution than the path of dealing with escape sequences explicitly.

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-04 20:31  Andrew Dunstan <andrew.dunstan@2ndquadrant.com>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  1 sibling, 0 replies; 64+ messages in thread

From: Andrew Dunstan @ 2020-01-04 20:31 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Peter Geoghegan <pg@bowt.ie>; Robert Haas <robertmhaas@gmail.com>; Christoph Berg <myon@debian.org>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

On Sun, Jan 5, 2020 at 6:29 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> I wrote:
> > Seeing that you're also having issues with a stack involving
> > libtinfo.so.5, here's my theory: libtinfo version 5 is brain-dead
> > about whether it needs to issue cursor repositioning commands, and
> > tends to do so even when the cursor is in the right place already.
> > Version 6 fixed that, which is why we're not seeing these escape
> > sequences on any of the libedit-using buildfarm critters.
>
> Nope, the buildfarm just blew up that theory: Andres' critters are
> failing in the wake of fac1c04fe, with symptoms exactly like those
> of my franken-libedit build.  So newer libtinfo doesn't fix it.
>
> What has to have broken those machines was the change to explicitly
> force TERM to "xterm".  Now I'm wondering what their prevailing
> setting was before that.  Maybe it was undef, or some absolutely
> vanilla thing that prevents libtinfo from thinking it can use any
> escape sequences at all.  I'm going to go find out, because if we
> can use that behavior globally, it'd be a heck of a lot safer
> solution than the path of dealing with escape sequences explicitly.
>


You can see what settings it started with, although only certain
values are whitelisted. See orig_env in the config. e.g. crake, which
is now failing, has no TERM setting.

cheers

andrew


-- 
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-04 20:46  Peter Geoghegan <pg@bowt.ie>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  1 sibling, 1 reply; 64+ messages in thread

From: Peter Geoghegan @ 2020-01-04 20:46 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Robert Haas <robertmhaas@gmail.com>; Christoph Berg <myon@debian.org>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

On Sat, Jan 4, 2020 at 11:50 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> So we're going to have to make some decisions about what range of
> libedit builds we want to cater for in the tab-completion tests.
> Given that the plan is to considerably increase the coverage of
> those tests, I'm hesitant to promise that we'll keep passing with
> anything that isn't explicitly tested in the buildfarm, or by active
> developers such as yourself.  Maybe that's fine given that end users
> probably don't run the TAP tests.

This sounds similar to "EXTRA_TESTS=collate.linux.utf8". I think that
it's fine to go that way, provided it isn't hard to work around.

FWIW, I find it very surprising that it was possible for the test to
fail on my workstation/server, without it failing on any buildfarm
animals.

-- 
Peter Geoghegan





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-04 20:48  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  1 sibling, 0 replies; 64+ messages in thread

From: Tom Lane @ 2020-01-04 20:48 UTC (permalink / raw)
  To: Andrew Dunstan <andrew.dunstan@2ndquadrant.com>; +Cc: Peter Geoghegan <pg@bowt.ie>; Robert Haas <robertmhaas@gmail.com>; Christoph Berg <myon@debian.org>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
> On Sun, Jan 5, 2020 at 6:29 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> What has to have broken those machines was the change to explicitly
>> force TERM to "xterm".  Now I'm wondering what their prevailing
>> setting was before that.  Maybe it was undef, or some absolutely
>> vanilla thing that prevents libtinfo from thinking it can use any
>> escape sequences at all.  I'm going to go find out, because if we
>> can use that behavior globally, it'd be a heck of a lot safer
>> solution than the path of dealing with escape sequences explicitly.

> You can see what settings it started with, although only certain
> values are whitelisted. See orig_env in the config. e.g. crake, which
> is now failing, has no TERM setting.

Ah, right.  The one-off patch I just pushed also confirms that the
TAP tests are seeing TERM-not-set on Andres' machines.

I'm currently investigating the possibility of just unsetting it
in the tab-completion test and then being able to revert all the
escape-sequence hocus-pocus.  It's looking promising in local
testing, although Peter said off-list that it didn't seem to
work in his setup.

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-04 20:56  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Peter Geoghegan <pg@bowt.ie>
  0 siblings, 2 replies; 64+ messages in thread

From: Tom Lane @ 2020-01-04 20:56 UTC (permalink / raw)
  To: Peter Geoghegan <pg@bowt.ie>; +Cc: Robert Haas <robertmhaas@gmail.com>; Christoph Berg <myon@debian.org>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

Peter Geoghegan <pg@bowt.ie> writes:
> FWIW, I find it very surprising that it was possible for the test to
> fail on my workstation/server, without it failing on any buildfarm
> animals.

Yeah, there is still something unexplained about that.  We've so far
failed to pin the blame on either readline version or environment
settings ... but what else could be causing you to get different
results?

For the record, I'm currently running around and trying the attached
(on top of latest HEAD, 60ab7c80b) on the various configurations
I have here.  Could you confirm that it works, or doesn't, in your
environment --- and if it doesn't, what's the output?

			regards, tom lane

Attachments:

  [text/x-diff] unset-TERM-in-tab-completion-test.patch (1.6K, ../../4659.1578171401@sss.pgh.pa.us/2-unset-TERM-in-tab-completion-test.patch)
  download | inline diff:
diff --git a/src/bin/psql/t/010_tab_completion.pl b/src/bin/psql/t/010_tab_completion.pl
index 1dc87b5..ebf603f 100644
--- a/src/bin/psql/t/010_tab_completion.pl
+++ b/src/bin/psql/t/010_tab_completion.pl
@@ -38,14 +38,12 @@ $node->safe_psql('postgres',
 my $historyfile = "${TestLib::log_path}/010_psql_history.txt";
 $ENV{PSQL_HISTORY} = $historyfile;
 
-# Debug investigation
-note "TERM is set to '" . ($ENV{TERM} || "<undef>") . "'";
-
-# Ensure that readline/libedit puts out xterm escapes, not something else.
-$ENV{TERM} = 'xterm';
-
-# regexp to match one xterm escape sequence (CSI style only, for now)
-my $escseq = "(\e\\[[0-9;]*[A-Za-z])";
+# Unset $TERM so that readline/libedit won't use any terminal-dependent
+# escape sequences; that leads to way too many cross-version variations
+# in the output.
+delete $ENV{TERM};
+# Some versions of readline inspect LS_COLORS, so for luck unset that too.
+delete $ENV{LS_COLORS};
 
 # fire up an interactive psql session
 my $in  = '';
@@ -110,12 +108,8 @@ check_completion(
 	"select \\* from my\a?tab",
 	"complete my<tab> to mytab when there are multiple choices");
 
-# some versions of readline/libedit require two tabs here, some only need one.
-# also, some might issue escape sequences to reposition the cursor, clear the
-# line, etc, instead of just printing some spaces.
-check_completion(
-	"\t\t",
-	"mytab$escseq*123( |$escseq)+mytab$escseq*246",
+# some versions of readline/libedit require two tabs here, some only need one
+check_completion("\t\t", "mytab123 +mytab246",
 	"offer multiple table choices");
 
 check_completion("2\t", "246 ",

^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-04 21:09  Peter Geoghegan <pg@bowt.ie>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  1 sibling, 0 replies; 64+ messages in thread

From: Peter Geoghegan @ 2020-01-04 21:09 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Robert Haas <robertmhaas@gmail.com>; Christoph Berg <myon@debian.org>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

On Sat, Jan 4, 2020 at 12:56 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Yeah, there is still something unexplained about that.  We've so far
> failed to pin the blame on either readline version or environment
> settings ... but what else could be causing you to get different
> results?

Beats me. Could have something to do with the fact that my libtinfo5
is in fact a patched version -- "6.1-1ubuntu1.18.04". (Whatever that
means.)

> For the record, I'm currently running around and trying the attached
> (on top of latest HEAD, 60ab7c80b) on the various configurations
> I have here.  Could you confirm that it works, or doesn't, in your
> environment --- and if it doesn't, what's the output?

This patch shows the same old failure as before:

cd /code/postgresql/patch/build/../source/src/bin/psql &&
TESTDIR='/code/postgresql/patch/build/src/bin/psql'
PATH="/code/postgresql/patch/build/tmp_install/code/postgresql/patch/install/bin:$PATH"
LD_LIBRARY_PATH="/code/postgresql/patch/build/tmp_install/code/postgresql/patch/install/lib"
 PGPORT='65432'
PG_REGRESS='/code/postgresql/patch/build/src/bin/psql/../../../src/test/regress/pg_regress'
REGRESS_SHLIB='/code/postgresql/patch/build/src/test/regress/regress.so'
/usr/bin/prove -I
/code/postgresql/patch/build/../source/src/test/perl/ -I
/code/postgresql/patch/build/../source/src/bin/psql  t/*.pl
t/010_tab_completion.pl .. 8/?
#   Failed test 'offer multiple table choices'
#   at t/010_tab_completion.pl line 112.
# Actual output was "\r\n\e[01;35mmytab\e[0m\e[K123\e[0m\e[K
\e[01;35mmytab\e[0m\e[K246\e[0m\e[K  \r\npostgres=# select * from
mytab\r\n\e[01;35mmytab\e[0m\e[K123\e[0m\e[K
\e[01;35mmytab\e[0m\e[K246\e[0m\e[K  \r\npostgres=# select * from
mytab"
#
# Looks like you failed 1 test of 12.
t/010_tab_completion.pl .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/12 subtests

Test Summary Report
-------------------
t/010_tab_completion.pl (Wstat: 256 Tests: 12 Failed: 1)
  Failed test:  8
  Non-zero exit status: 1
Files=1, Tests=12,  7 wallclock secs ( 0.01 usr  0.00 sys +  0.37 cusr
 0.09 csys =  0.47 CPU)
Result: FAIL
Makefile:87: recipe for target 'check' failed
make: *** [check] Error 1

-- 
Peter Geoghegan





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-04 23:29  Peter Geoghegan <pg@bowt.ie>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  1 sibling, 1 reply; 64+ messages in thread

From: Peter Geoghegan @ 2020-01-04 23:29 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Robert Haas <robertmhaas@gmail.com>; Christoph Berg <myon@debian.org>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

On Sat, Jan 4, 2020 at 12:56 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Yeah, there is still something unexplained about that.  We've so far
> failed to pin the blame on either readline version or environment
> settings ... but what else could be causing you to get different
> results?

Tom just advised me via private e-mail that my termcap database may be
the issue here. Looks like he was right. I did a "sudo aptitude
reinstall ncurses-base", and now the tests pass against HEAD. (I
probably should have tried to preserve things before going ahead with
that, but that didn't happen -- I had no reason to think that the
system was affected by any kind of corruption.)

I am very sorry for having wasted your time on this wild goose chase,
Tom. The only explanation I can think of is that maybe it relates to
my upgrading the OS in-place. That is, maybe my system was affected by
a subtle, low probability bug due to a major version change in
ncurses-base.

-- 
Peter Geoghegan





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-04 23:39  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Peter Geoghegan <pg@bowt.ie>
  0 siblings, 1 reply; 64+ messages in thread

From: Tom Lane @ 2020-01-04 23:39 UTC (permalink / raw)
  To: Peter Geoghegan <pg@bowt.ie>; +Cc: Robert Haas <robertmhaas@gmail.com>; Christoph Berg <myon@debian.org>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

Peter Geoghegan <pg@bowt.ie> writes:
> Tom just advised me via private e-mail that my termcap database may be
> the issue here. Looks like he was right. I did a "sudo aptitude
> reinstall ncurses-base", and now the tests pass against HEAD. (I
> probably should have tried to preserve things before going ahead with
> that, but that didn't happen -- I had no reason to think that the
> system was affected by any kind of corruption.)

Hm, well, HEAD still has the hackery with explicit accounting for
escape sequences.  Could you try it with the patch I showed to unset
TERM and remove that stuff?  (It won't apply exactly to HEAD, but
the diffs are simple enough, or you could revert to 60ab7c80b first.)

> I am very sorry for having wasted your time on this wild goose chase,

Don't worry about it --- even if it was some upgrade glitch, there
was no way to know that in advance.

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-05 00:09  Peter Geoghegan <pg@bowt.ie>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 64+ messages in thread

From: Peter Geoghegan @ 2020-01-05 00:09 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Robert Haas <robertmhaas@gmail.com>; Christoph Berg <myon@debian.org>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

On Sat, Jan 4, 2020 at 3:39 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Hm, well, HEAD still has the hackery with explicit accounting for
> escape sequences.  Could you try it with the patch I showed to unset
> TERM and remove that stuff?  (It won't apply exactly to HEAD, but
> the diffs are simple enough, or you could revert to 60ab7c80b first.)

With the attached patch against HEAD (which is based on your earlier
unset-TERM-in-tab-completion-test.patch), I find that the tests fail
as follows:

cd /code/postgresql/patch/build/../source/src/bin/psql &&
TESTDIR='/code/postgresql/patch/build/src/bin/psql'
PATH="/code/postgresql/patch/build/tmp_install/code/postgresql/patch/install/bin:$PATH"
LD_LIBRARY_PATH="/code/postgresql/patch/build/tmp_install/code/postgresql/patch/install/lib"
 PGPORT='65432'
PG_REGRESS='/code/postgresql/patch/build/src/bin/psql/../../../src/test/regress/pg_regress'
REGRESS_SHLIB='/code/postgresql/patch/build/src/test/regress/regress.so'
/usr/bin/prove -I
/code/postgresql/patch/build/../source/src/test/perl/ -I
/code/postgresql/patch/build/../source/src/bin/psql  t/*.pl
t/010_tab_completion.pl .. 8/?
#   Failed test 'offer multiple table choices'
#   at t/010_tab_completion.pl line 112.
# Actual output was "\r\n\e[01;35mmytab\e[0m\e[K123\e[0m\e[K
\e[01;35mmytab\e[0m\e[K246\e[0m\e[K  \r\npostgres=# select * from
mytab\r\n\e[01;35mmytab\e[0m\e[K123\e[0m\e[K
\e[01;35mmytab\e[0m\e[K246\e[0m\e[K  \r\npostgres=# select * from
mytab"
#
# Looks like you failed 1 test of 12.
t/010_tab_completion.pl .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/12 subtests

Test Summary Report
-------------------
t/010_tab_completion.pl (Wstat: 256 Tests: 12 Failed: 1)
  Failed test:  8
  Non-zero exit status: 1
Files=1, Tests=12,  7 wallclock secs ( 0.02 usr  0.00 sys +  0.36 cusr
 0.12 csys =  0.50 CPU)
Result: FAIL
Makefile:87: recipe for target 'check' failed
make: *** [check] Error 1

-- 
Peter Geoghegan

Attachments:

  [application/octet-stream] HEAD-unset-TERM-in-tab-completion-test.patch (1.5K, ../../CAH2-WzkqStzPVGCQXdd428GEUCs_VGc9kY_VhcCVt6bKNEc1zQ@mail.gmail.com/2-HEAD-unset-TERM-in-tab-completion-test.patch)
  download | inline diff:
diff --git a/src/bin/psql/t/010_tab_completion.pl b/src/bin/psql/t/010_tab_completion.pl
index 0f1d596b2b..ebf603f82e 100644
--- a/src/bin/psql/t/010_tab_completion.pl
+++ b/src/bin/psql/t/010_tab_completion.pl
@@ -38,11 +38,12 @@ $node->safe_psql('postgres',
 my $historyfile = "${TestLib::log_path}/010_psql_history.txt";
 $ENV{PSQL_HISTORY} = $historyfile;
 
-# Debug investigation
-note "TERM is set to '" . ($ENV{TERM} || "<undef>") . "'";
-
-# regexp to match one xterm escape sequence (CSI style only, for now)
-my $escseq = "(\e\\[[0-9;]*[A-Za-z])";
+# Unset $TERM so that readline/libedit won't use any terminal-dependent
+# escape sequences; that leads to way too many cross-version variations
+# in the output.
+delete $ENV{TERM};
+# Some versions of readline inspect LS_COLORS, so for luck unset that too.
+delete $ENV{LS_COLORS};
 
 # fire up an interactive psql session
 my $in  = '';
@@ -107,12 +108,8 @@ check_completion(
 	"select \\* from my\a?tab",
 	"complete my<tab> to mytab when there are multiple choices");
 
-# some versions of readline/libedit require two tabs here, some only need one.
-# also, some might issue escape sequences to reposition the cursor, clear the
-# line, etc, instead of just printing some spaces.
-check_completion(
-	"\t\t",
-	"mytab$escseq*123( |$escseq)+mytab$escseq*246",
+# some versions of readline/libedit require two tabs here, some only need one
+check_completion("\t\t", "mytab123 +mytab246",
 	"offer multiple table choices");
 
 check_completion("2\t", "246 ",


^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-05 00:14  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Peter Geoghegan <pg@bowt.ie>
  0 siblings, 1 reply; 64+ messages in thread

From: Tom Lane @ 2020-01-05 00:14 UTC (permalink / raw)
  To: Peter Geoghegan <pg@bowt.ie>; +Cc: Robert Haas <robertmhaas@gmail.com>; Christoph Berg <myon@debian.org>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

Peter Geoghegan <pg@bowt.ie> writes:
> With the attached patch against HEAD (which is based on your earlier
> unset-TERM-in-tab-completion-test.patch), I find that the tests fail
> as follows:

Um, well, that's the same behavior you were seeing before.
So the terminfo reinstall didn't really do anything.

I'm still curious about which terminfo file your psql actually
reads if TERM is unset, and whether that file is visibly
different from the xterm-related files.

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-05 00:42  Peter Geoghegan <pg@bowt.ie>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 64+ messages in thread

From: Peter Geoghegan @ 2020-01-05 00:42 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Robert Haas <robertmhaas@gmail.com>; Christoph Berg <myon@debian.org>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

On Sat, Jan 4, 2020 at 4:14 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Um, well, that's the same behavior you were seeing before.
> So the terminfo reinstall didn't really do anything.

Sigh.

> I'm still curious about which terminfo file your psql actually
> reads if TERM is unset, and whether that file is visibly
> different from the xterm-related files.

I've found the actual problem -- it's my ~/.inputrc. Which is read in by
libreadline at some point (determined this using ltrace).

Once I comment out the following two lines from ~/.inputrc, everything
works fine on
HEAD + HEAD-unset-TERM-in-tab-completion-test.patch:

set colored-completion-prefix on
set colored-stats on

--
Peter Geoghegan





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-05 01:57  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Peter Geoghegan <pg@bowt.ie>
  0 siblings, 2 replies; 64+ messages in thread

From: Tom Lane @ 2020-01-05 01:57 UTC (permalink / raw)
  To: Peter Geoghegan <pg@bowt.ie>; +Cc: Robert Haas <robertmhaas@gmail.com>; Christoph Berg <myon@debian.org>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

Peter Geoghegan <pg@bowt.ie> writes:
> I've found the actual problem -- it's my ~/.inputrc. Which is read in by
> libreadline at some point (determined this using ltrace).

Ah-hah!

So what we really want here, I guess, is for the test script to suppress
reading of ~/.inputrc, on the same principle that it suppresses reading
of ~/.psqlrc.  A quick look at the readline docs suggests that the
best way to do that would be to set envar INPUTRC to /dev/null --- could
you confirm that that works for you?

> Once I comment out the following two lines from ~/.inputrc, everything
> works fine on
> HEAD + HEAD-unset-TERM-in-tab-completion-test.patch:
> set colored-completion-prefix on
> set colored-stats on

Hm.  I wonder how it is that that leads to ignoring the TERM environment?
Still, it's just an academic point if we can suppress reading the file.

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-05 02:15  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  1 sibling, 0 replies; 64+ messages in thread

From: Tom Lane @ 2020-01-05 02:15 UTC (permalink / raw)
  To: ; +Cc: Peter Geoghegan <pg@bowt.ie>; Robert Haas <robertmhaas@gmail.com>; Christoph Berg <myon@debian.org>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

I wrote:
> Peter Geoghegan <pg@bowt.ie> writes:
>> Once I comment out the following two lines from ~/.inputrc, everything
>> works fine on
>> HEAD + HEAD-unset-TERM-in-tab-completion-test.patch:
>> set colored-completion-prefix on
>> set colored-stats on

> Hm.  I wonder how it is that that leads to ignoring the TERM environment?

A bit of digging says that readline's color support is just hard-wired
to use xterm-style escapes -- it doesn't look like there's any connection
to terminfo at all.  See the  _rl_color_indicator[] data structure.
The \e[...m and \e[K escape sequences can both be found there.

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-05 02:16  Peter Geoghegan <pg@bowt.ie>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  1 sibling, 1 reply; 64+ messages in thread

From: Peter Geoghegan @ 2020-01-05 02:16 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Robert Haas <robertmhaas@gmail.com>; Christoph Berg <myon@debian.org>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

On Sat, Jan 4, 2020 at 5:57 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> So what we really want here, I guess, is for the test script to suppress
> reading of ~/.inputrc, on the same principle that it suppresses reading
> of ~/.psqlrc.

That was what I thought of, too.

> A quick look at the readline docs suggests that the
> best way to do that would be to set envar INPUTRC to /dev/null --- could
> you confirm that that works for you?

Yes -- "export INPUTRC=/dev/null" also makes it work for me (on HEAD +
HEAD-unset-TERM-in-tab-completion-test.patch, but with my
original/problematic ~/.inputrc).

-- 
Peter Geoghegan





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-05 02:17  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Peter Geoghegan <pg@bowt.ie>
  0 siblings, 2 replies; 64+ messages in thread

From: Tom Lane @ 2020-01-05 02:17 UTC (permalink / raw)
  To: Peter Geoghegan <pg@bowt.ie>; +Cc: Robert Haas <robertmhaas@gmail.com>; Christoph Berg <myon@debian.org>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

Peter Geoghegan <pg@bowt.ie> writes:
> On Sat, Jan 4, 2020 at 5:57 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> A quick look at the readline docs suggests that the
>> best way to do that would be to set envar INPUTRC to /dev/null --- could
>> you confirm that that works for you?

> Yes -- "export INPUTRC=/dev/null" also makes it work for me (on HEAD +
> HEAD-unset-TERM-in-tab-completion-test.patch, but with my
> original/problematic ~/.inputrc).

Cool, I'll go commit a fix along those lines.  Thanks for tracing
this down!

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-05 02:18  Peter Geoghegan <pg@bowt.ie>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  1 sibling, 0 replies; 64+ messages in thread

From: Peter Geoghegan @ 2020-01-05 02:18 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Robert Haas <robertmhaas@gmail.com>; Christoph Berg <myon@debian.org>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

On Sat, Jan 4, 2020 at 6:17 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Cool, I'll go commit a fix along those lines.  Thanks for tracing
> this down!

Glad to help!

-- 
Peter Geoghegan





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-05 13:35  Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  1 sibling, 2 replies; 64+ messages in thread

From: ilmari@ilmari.org @ 2020-01-05 13:35 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Peter Geoghegan <pg@bowt.ie>; Robert Haas <robertmhaas@gmail.com>; Christoph Berg <myon@debian.org>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

Tom Lane <tgl@sss.pgh.pa.us> writes:

> Cool, I'll go commit a fix along those lines.  Thanks for tracing
> this down!

Here's one final style cleanup for the TAP test.

- use like() for the banner test
- pass the regexes around as qr// objects, so they can be
  syntax-highlighted properly, and don't need regex
  metacharacter-escaping backslashes doubled.
- include the regex that didn't match in the diagnostic

- ilmari
-- 
"The surreality of the universe tends towards a maximum" -- Skud's Law
"Never formulate a law or axiom that you're not prepared to live with
 the consequences of."                              -- Skud's Meta-Law






^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-05 13:39  Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
  parent: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
  1 sibling, 0 replies; 64+ messages in thread

From: ilmari@ilmari.org @ 2020-01-05 13:39 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Peter Geoghegan <pg@bowt.ie>; Robert Haas <robertmhaas@gmail.com>; Christoph Berg <myon@debian.org>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

ilmari@ilmari.org (Dagfinn Ilmari Mannsåker) writes:

> Tom Lane <tgl@sss.pgh.pa.us> writes:
>
>> Cool, I'll go commit a fix along those lines.  Thanks for tracing
>> this down!
>
> Here's one final style cleanup for the TAP test.
>
> - use like() for the banner test
> - pass the regexes around as qr// objects, so they can be
>   syntax-highlighted properly, and don't need regex
>   metacharacter-escaping backslashes doubled.
> - include the regex that didn't match in the diagnostic

This time with the actual attachment...

- ilmari
-- 
- Twitter seems more influential [than blogs] in the 'gets reported in
  the mainstream press' sense at least.               - Matt McLeod
- That'd be because the content of a tweet is easier to condense down
  to a mainstream media article.                      - Calle Dybedahl

Attachments:

  [text/x-diff] 0001-Use-qr-for-passed-in-regexes-in-tab-completion-TAP-t.patch (0B, ../../87h81a10qo.fsf@wibble.ilmari.org/2-0001-Use-qr-for-passed-in-regexes-in-tab-completion-TAP-t.patch)
  download

^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-05 16:38  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
  1 sibling, 1 reply; 64+ messages in thread

From: Tom Lane @ 2020-01-05 16:38 UTC (permalink / raw)
  To: ilmari@ilmari.org; +Cc: Peter Geoghegan <pg@bowt.ie>; Robert Haas <robertmhaas@gmail.com>; Christoph Berg <myon@debian.org>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

ilmari@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes:
> Here's one final style cleanup for the TAP test.

LGTM, pushed.

One minor note: you wanted to change the \DRD test to

+check_completion("\\DRD\t", qr/\\drds /, "complete \\DRD<tab> to \\drds");

but that doesn't work everywhere, unfortunately.  On my machine
what comes out is

# Actual output was "\\DRD\b\b\bdrds "
# Did not match "(?-xism:\\drds )"

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-05 16:48  Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 0 replies; 64+ messages in thread

From: Dagfinn Ilmari Mannsåker @ 2020-01-05 16:48 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Peter Geoghegan <pg@bowt.ie>; Robert Haas <robertmhaas@gmail.com>; Christoph Berg <myon@debian.org>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>



On 5 January 2020 16:38:36 GMT, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>ilmari@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes:
>> Here's one final style cleanup for the TAP test.
>
>LGTM, pushed.

Thanks!

>One minor note: you wanted to change the \DRD test to
>
>+check_completion("\\DRD\t", qr/\\drds /, "complete \\DRD<tab> to
>\\drds");
>
>but that doesn't work everywhere, unfortunately.  On my machine
>what comes out is
>
># Actual output was "\\DRD\b\b\bdrds "
># Did not match "(?-xism:\\drds )"

Sorry, that was something I left in for testing the diagnostic and forgot to remove before committing.

>			regards, tom lane

- ilmari





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-05 18:30  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  1 sibling, 1 reply; 64+ messages in thread

From: Tom Lane @ 2020-01-05 18:30 UTC (permalink / raw)
  To: Andres Freund <andres@anarazel.de>; +Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>; Christoph Berg <myon@debian.org>

I wrote:
> Indeed.  It appears that recent libedit breaks tab-completion for
> words involving a backslash, which is the fault of this upstream
> commit:
> http://cvsweb.netbsd.org/bsdweb.cgi/src/lib/libedit/filecomplete.c.diff?r1=1.52&r2=1.53
> Basically what that's doing is applying de-backslashing to EVERY
> word that completion is attempted on, whether it might be a filename
> or not.  So what psql_complete sees in this test case is just "DRD"
> which of course it does not recognize as a possible psql backslash
> command.

The current state of play on this is that I committed a hacky workaround
[1], but there is now a fix for it in libedit upstream [2][3].  I gather
from looking at Debian's package page that the fix could be expected to
propagate to Debian unstable within a few weeks, at which point I'd like
to revert the hack.  The libedit bug's only been there a few months
(it was evidently introduced on 2019-03-31) so we can hope that it hasn't
propagated into any long-term-support distros.

> I found out while investigating this that the libedit version shipping
> with buster (3.1-20181209) is differently broken for the same case:
> instead of inapproriate forced de-escaping of the input of the
> application-specific completion function, it applies inapproriate
> forced escaping to the output of said function, so that when we see
> "\DRD" and return "\drds", what comes out to the user is "\\drds".

There's little we can do about that one, but it doesn't affect the
regression test as currently constituted.

Another libedit bug that the regression test *is* running into is that
some ancient versions fail to emit a trailing space after a successful
completion.  snapper is hitting that [4], and I believe locust would
be if it were running the TAP tests.  While we could work around that
by removing the trailing spaces from the match regexps, I really
don't wish to do so, because that destroys the test's ability to
distinguish correct outputs from incorrect-but-longer ones.  (That's
not a killer problem for any of the current test cases, perhaps, but
I think it will be in future.)  So I'd like to define this problem as
being out of scope.  This bug was fixed eleven years ago upstream
(see change in _rl_completion_append_character_function in [5]), so
it seems reasonable to insist that people get a newer libedit or not
run this test.

Another issue visible in [4] is that ancient libedit fails to sort
the offered completion strings as one would expect.  I don't see
much point in working around that either.  prairiedog's host has
that bug but not the space bug (see [6], from before I suppressed
running the test on that machine), so it affects a larger range of
libedit versions, but they're probably all way too old for anyone
to care.  If anyone can point to a new-enough-to-matter libedit
that still behaves that way, we can reconsider.

			regards, tom lane

[1] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=ddd87d564508bb1c80aac0a4439cfe74a...
[2] http://gnats.netbsd.org/cgi-bin/query-pr-single.pl?number=54510
[3] http://cvsweb.netbsd.org/bsdweb.cgi/src/lib/libedit/filecomplete.c.diff?r1=1.63&r2=1.64
[4] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=snapper&dt=2020-01-05%2013%3A01%3A46
[5] http://cvsweb.netbsd.org/bsdweb.cgi/src/lib/libedit/readline.c.diff?r1=1.75&r2=1.76
[6] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=prairiedog&dt=2020-01-02%2022%3A52%3A36





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-06 10:56  Christoph Berg <myon@debian.org>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 2 replies; 64+ messages in thread

From: Christoph Berg @ 2020-01-06 10:56 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Andres Freund <andres@anarazel.de>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

Re: Tom Lane 2020-01-05 <25771.1578249042@sss.pgh.pa.us>
> The current state of play on this is that I committed a hacky workaround
> [1], but there is now a fix for it in libedit upstream [2][3].  I gather
> from looking at Debian's package page that the fix could be expected to
> propagate to Debian unstable within a few weeks, at which point I'd like
> to revert the hack.  The libedit bug's only been there a few months
> (it was evidently introduced on 2019-03-31) so we can hope that it hasn't
> propagated into any long-term-support distros.
[...]

I lost track of what bug is supposed to be where, so here's a summary
of the state at apt.postgresql.org:

PG13 head work on Debian unstable, buster, stretch.
Does not work on Ubuntu bionic, xenial. (Others not tested.)

Ubuntu xenial:

07:24:42 #   Failed test 'complete SEL<tab> to SELECT'
07:24:42 #   at t/010_tab_completion.pl line 98.
07:24:42 # Actual output was "SEL\tpostgres=# SEL\a"
07:24:42 # Did not match "(?^:SELECT )"
07:24:48
07:24:48 #   Failed test 'complete sel<tab> to select'
07:24:48 #   at t/010_tab_completion.pl line 103.
07:24:48 # Actual output was "sel\b\b\bSELECT "
07:24:48 # Did not match "(?^:select )"
07:24:54
07:24:54 #   Failed test 'complete t<tab> to tab1'
07:24:54 #   at t/010_tab_completion.pl line 106.
07:24:54 # Actual output was "* from t "
07:24:54 # Did not match "(?^:\* from tab1 )"
07:25:00
07:25:00 #   Failed test 'complete my<tab> to mytab when there are multiple choices'
07:25:00 #   at t/010_tab_completion.pl line 112.
07:25:00 # Actual output was "select * from my "
07:25:00 # Did not match "(?^:select \* from my\a?tab)"
07:25:06
07:25:06 #   Failed test 'offer multiple table choices'
07:25:06 #   at t/010_tab_completion.pl line 118.
07:25:06 # Actual output was "\r\n\r\n\r\r\npostgres=# select * from my \r\n\r\n\r\r\npostgres=# select * from my "
07:25:06 # Did not match "(?^:mytab123 +mytab246)"
07:25:12
07:25:12 #   Failed test 'finish completion of one of multiple table choices'
07:25:12 #   at t/010_tab_completion.pl line 123.
07:25:12 # Actual output was "2 "
07:25:12 # Did not match "(?^:246 )"
07:25:18
07:25:18 #   Failed test 'complete \DRD<tab> to \drds'
07:25:18 #   at t/010_tab_completion.pl line 131.
07:25:18 # Actual output was "\\DRD\b\b\b\bselect "
07:25:18 # Did not match "(?^:drds )"
07:25:18 # Looks like you failed 7 tests of 12.
07:25:18 t/010_tab_completion.pl ..
07:25:18 Dubious, test returned 7 (wstat 1792, 0x700)
07:25:18 Failed 7/12 subtests

Ubuntu bionic fails elsewhere:

07:19:51 t/001_stream_rep.pl .................. ok
07:19:53 t/002_archiving.pl ................... ok
07:19:59 t/003_recovery_targets.pl ............ ok
07:20:01 t/004_timeline_switch.pl ............. ok
07:20:08 t/005_replay_delay.pl ................ ok
07:20:10 Bailout called.  Further testing stopped:  system pg_ctl failed
07:20:10 FAILED--Further testing stopped: system pg_ctl failed

07:20:10 2020-01-06 06:19:41.285 UTC [26415] LOG:  received fast shutdown request
07:20:10 2020-01-06 06:19:41.285 UTC [26415] LOG:  aborting any active transactions
07:20:10 2020-01-06 06:19:41.287 UTC [26415] LOG:  background worker "logical replication launcher" (PID 26424) exited with exit code 1
07:20:10 2020-01-06 06:19:41.287 UTC [26419] LOG:  shutting down
07:20:10 2020-01-06 06:19:41.287 UTC [26419] LOG:  checkpoint starting: shutdown immediate

(It didn't get to the 010_tab_completion.pl test.)

Libedit versions are:

Debian:
libedit2   | 3.1-20140620-2 | oldoldstable | amd64, armel, armhf, i386 (jessie)
libedit2   | 3.1-20160903-3 | oldstable    | amd64, arm64, armel, armhf, i386, mips, mips64el, m (stretch)
libedit2   | 3.1-20181209-1 | stable       | amd64, arm64, armel, armhf, i386, mips, mips64el, m (buster)
libedit2   | 3.1-20191211-1 | testing      | amd64, arm64, armel, armhf, i386, mips64el, mipsel, (bullseye)
libedit2   | 3.1-20191231-1 | unstable     | amd64, arm64, armel, armhf, i386, mips64el, mipsel,

Ubuntu:
 libedit2 | 2.11-20080614-3ubuntu2 | precise        | amd64, armel, armhf, i386, powerpc
 libedit2 | 3.1-20130712-2         | trusty         | amd64, arm64, armhf, i386, powerpc, ppc64e
 libedit2 | 3.1-20150325-1ubuntu2  | xenial         | amd64, arm64, armhf, i386, powerpc, ppc64e
 libedit2 | 3.1-20170329-1         | bionic         | amd64, arm64, armhf, i386, ppc64el, s390x
 libedit2 | 3.1-20181209-1         | disco          | amd64, arm64, armhf, i386, ppc64el, s390x
 libedit2 | 3.1-20190324-1         | eoan           | amd64, arm64, armhf, i386, ppc64el, s390x
 libedit2 | 3.1-20191211-1         | focal          | amd64, arm64, armhf, i386, ppc64el, s390x
 libedit2 | 3.1-20191231-1         | focal-proposed | amd64, arm64, armhf, i386, ppc64el, s390x

Christoph





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-06 15:15  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Christoph Berg <myon@debian.org>
  1 sibling, 1 reply; 64+ messages in thread

From: Tom Lane @ 2020-01-06 15:15 UTC (permalink / raw)
  To: Christoph Berg <myon@debian.org>; +Cc: Andres Freund <andres@anarazel.de>; Amit Kapila <amit.kapila16@gmail.com>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

Christoph Berg <myon@debian.org> writes:
> I lost track of what bug is supposed to be where, so here's a summary
> of the state at apt.postgresql.org:

> PG13 head work on Debian unstable, buster, stretch.

Cool.

> Does not work on Ubuntu bionic, xenial. (Others not tested.)

Hmm ... do we care?  The test output seems to show that xenial's
3.1-20150325-1ubuntu2 libedit is completely broken.  Maybe there's
a way to work around that, but it's not clear to me that that'd
be a useful expenditure of time.  You're not really going to be
building PG13 for that release are you?

> Ubuntu bionic fails elsewhere [ apparently in 006_logical_decoding.pl ]

Hmm.  Not related to this thread then.  But if that's reproducible,
somebody should have a look.  Maybe related to

https://www.postgresql.org/message-id/CAA4eK1LMDx6vK8Kdw8WUeW1MjToN2xVffL2kvtHvZg17%3DY6QQg%40mail.g...

??? (cc'ing Amit for that)

Meanwhile, as to the point I was really concerned about, your table of
current versions looks promising -- libedit's premature-dequote bug is
evidently only in unstable and the last stable branch, and I presume the
last-stables are still getting updated, since those have libedit versions
that are less than a month old.  I looked at Fedora's git repo and the
situation seems similar over there.

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-06 19:13  Christoph Berg <myon@debian.org>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 2 replies; 64+ messages in thread

From: Christoph Berg @ 2020-01-06 19:13 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Andres Freund <andres@anarazel.de>; Amit Kapila <amit.kapila16@gmail.com>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

Re: Tom Lane 2020-01-06 <3764.1578323719@sss.pgh.pa.us>
> > Does not work on Ubuntu bionic, xenial. (Others not tested.)
> 
> Hmm ... do we care?  The test output seems to show that xenial's
> 3.1-20150325-1ubuntu2 libedit is completely broken.  Maybe there's
> a way to work around that, but it's not clear to me that that'd
> be a useful expenditure of time.  You're not really going to be
> building PG13 for that release are you?

xenial (16.04) is a LTS release with support until 2021-04, and the
current plan was to support it. I now realize that's semi-close to the
13 release date, but so far we have tried to really support all
PG-Distro combinations.

I could probably arrange for that test to be disabled when building
for xenial, but it'd be nice if there were a configure switch or
environment variable for it so we don't have to invent it.

> > Ubuntu bionic fails elsewhere [ apparently in 006_logical_decoding.pl ]
> 
> Hmm.  Not related to this thread then.  But if that's reproducible,

It has been failing with the same output since Jan 2, 2020, noon.

> somebody should have a look.  Maybe related to
> 
> https://www.postgresql.org/message-id/CAA4eK1LMDx6vK8Kdw8WUeW1MjToN2xVffL2kvtHvZg17%3DY6QQg%40mail.g...

The only git change in that build was d207038053837 "Fix running out
of file descriptors for spill files."

Christoph





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-06 23:06  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Christoph Berg <myon@debian.org>
  1 sibling, 1 reply; 64+ messages in thread

From: Tom Lane @ 2020-01-06 23:06 UTC (permalink / raw)
  To: Christoph Berg <myon@debian.org>; +Cc: Andres Freund <andres@anarazel.de>; Amit Kapila <amit.kapila16@gmail.com>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

Christoph Berg <myon@debian.org> writes:
> Re: Tom Lane 2020-01-06 <3764.1578323719@sss.pgh.pa.us>
>>> Does not work on Ubuntu bionic, xenial. (Others not tested.)

>> Hmm ... do we care?  The test output seems to show that xenial's
>> 3.1-20150325-1ubuntu2 libedit is completely broken.  Maybe there's
>> a way to work around that, but it's not clear to me that that'd
>> be a useful expenditure of time.  You're not really going to be
>> building PG13 for that release are you?

> xenial (16.04) is a LTS release with support until 2021-04, and the
> current plan was to support it. I now realize that's semi-close to the
> 13 release date, but so far we have tried to really support all
> PG-Distro combinations.

I installed libedit_3.1-20150325.orig.tar.gz from source here, and it
passes our current regression test and seems to behave just fine in
light manual testing.  (I did not apply any of the Debian-specific
patches at [1], but they don't look like they'd explain much.)
So I'm a bit at a loss as to what's going wrong for you.  Is the test
environment for Xenial the same as for the other branches?

			regards, tom lane

[1] https://launchpad.net/ubuntu/+source/libedit/3.1-20150325-1ubuntu2





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-07 06:18  Amit Kapila <amit.kapila16@gmail.com>
  parent: Christoph Berg <myon@debian.org>
  1 sibling, 0 replies; 64+ messages in thread

From: Amit Kapila @ 2020-01-07 06:18 UTC (permalink / raw)
  To: Christoph Berg <myon@debian.org>; Tom Lane <tgl@sss.pgh.pa.us>; Andres Freund <andres@anarazel.de>; Amit Kapila <amit.kapila16@gmail.com>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

On Tue, Jan 7, 2020 at 12:43 AM Christoph Berg <myon@debian.org> wrote:
>
> Re: Tom Lane 2020-01-06 <3764.1578323719@sss.pgh.pa.us>
> > > Does not work on Ubuntu bionic, xenial. (Others not tested.)
> >
> > Hmm ... do we care?  The test output seems to show that xenial's
> > 3.1-20150325-1ubuntu2 libedit is completely broken.  Maybe there's
> > a way to work around that, but it's not clear to me that that'd
> > be a useful expenditure of time.  You're not really going to be
> > building PG13 for that release are you?
>
> xenial (16.04) is a LTS release with support until 2021-04, and the
> current plan was to support it. I now realize that's semi-close to the
> 13 release date, but so far we have tried to really support all
> PG-Distro combinations.
>
> I could probably arrange for that test to be disabled when building
> for xenial, but it'd be nice if there were a configure switch or
> environment variable for it so we don't have to invent it.
>
> > > Ubuntu bionic fails elsewhere [ apparently in 006_logical_decoding.pl ]
> >
> > Hmm.  Not related to this thread then.  But if that's reproducible,
>
> It has been failing with the same output since Jan 2, 2020, noon.
>
> > somebody should have a look.  Maybe related to
> >
> > https://www.postgresql.org/message-id/CAA4eK1LMDx6vK8Kdw8WUeW1MjToN2xVffL2kvtHvZg17%3DY6QQg%40mail.g...
>
> The only git change in that build was d207038053837 "Fix running out
> of file descriptors for spill files."
>

Thanks, for reporting.  Is it possible to get a call stack as we are
not able to reproduce this failure?  Also, if you don't mind, let's
discuss this on the thread provided by Tom.


-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-07 22:28  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 64+ messages in thread

From: Tom Lane @ 2020-01-07 22:28 UTC (permalink / raw)
  To: Christoph Berg <myon@debian.org>; +Cc: Andres Freund <andres@anarazel.de>; Amit Kapila <amit.kapila16@gmail.com>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

I wrote:
> I installed libedit_3.1-20150325.orig.tar.gz from source here, and it
> passes our current regression test and seems to behave just fine in
> light manual testing.  (I did not apply any of the Debian-specific
> patches at [1], but they don't look like they'd explain much.)
> So I'm a bit at a loss as to what's going wrong for you.  Is the test
> environment for Xenial the same as for the other branches?

To dig deeper, I set up an actual installation of xenial, and on that
I can replicate the tab-completion misbehavior you reported.  The cause
appears to be that libedit's rl_line_buffer does not contain the current
line as expected, but the previous line (or, initially, an empty string).
Thus, the hack I put in to make things pass on current libedit actually
makes things worse on this version --- although it doesn't fully pass
even if I revert ddd87d564, since there are other places where we
depend on rl_line_buffer to be valid.

So that raises the question: why does xenial's version of libedit
not match either its documentation or the distributed source code?
Because it doesn't.

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-08 02:59  Amit Kapila <amit.kapila16@gmail.com>
  parent: Christoph Berg <myon@debian.org>
  1 sibling, 0 replies; 64+ messages in thread

From: Amit Kapila @ 2020-01-08 02:59 UTC (permalink / raw)
  To: Christoph Berg <myon@debian.org>; Tom Lane <tgl@sss.pgh.pa.us>; Andres Freund <andres@anarazel.de>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

On Mon, Jan 6, 2020 at 4:26 PM Christoph Berg <myon@debian.org> wrote:
>
> Re: Tom Lane 2020-01-05 <25771.1578249042@sss.pgh.pa.us>
>
> Ubuntu bionic fails elsewhere:
>
> 07:19:51 t/001_stream_rep.pl .................. ok
> 07:19:53 t/002_archiving.pl ................... ok
> 07:19:59 t/003_recovery_targets.pl ............ ok
> 07:20:01 t/004_timeline_switch.pl ............. ok
> 07:20:08 t/005_replay_delay.pl ................ ok
> 07:20:10 Bailout called.  Further testing stopped:  system pg_ctl failed
> 07:20:10 FAILED--Further testing stopped: system pg_ctl failed
>
> 07:20:10 2020-01-06 06:19:41.285 UTC [26415] LOG:  received fast shutdown request
> 07:20:10 2020-01-06 06:19:41.285 UTC [26415] LOG:  aborting any active transactions
> 07:20:10 2020-01-06 06:19:41.287 UTC [26415] LOG:  background worker "logical replication launcher" (PID 26424) exited with exit code 1
> 07:20:10 2020-01-06 06:19:41.287 UTC [26419] LOG:  shutting down
> 07:20:10 2020-01-06 06:19:41.287 UTC [26419] LOG:  checkpoint starting: shutdown immediate
>

It looks like this failure is more of what we are getting on
"sidewinder" where it failed because of "insufficient file descriptors
available to start server process".  Can you check in the log
(probably in 006_logical_decoding_master.log) if this is the same you
are getting or something else.


-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-09 14:45  Robert Haas <robertmhaas@gmail.com>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 2 replies; 64+ messages in thread

From: Robert Haas @ 2020-01-09 14:45 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Christoph Berg <myon@debian.org>; Andres Freund <andres@anarazel.de>; Amit Kapila <amit.kapila16@gmail.com>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

On Tue, Jan 7, 2020 at 5:29 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> So that raises the question: why does xenial's version of libedit
> not match either its documentation or the distributed source code?
> Because it doesn't.

The level of effort you've put into this is extremely impressive, but
I can't shake the feeling that you're going to keep finding issues in
the test setup, the operating system, or the upstream libraries,
rather than bugs in PostgreSQL. Maybe this is all just one-time
stabilization effort, but...

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





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-09 14:59  Christoph Berg <myon@debian.org>
  parent: Robert Haas <robertmhaas@gmail.com>
  1 sibling, 1 reply; 64+ messages in thread

From: Christoph Berg @ 2020-01-09 14:59 UTC (permalink / raw)
  To: Robert Haas <robertmhaas@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Andres Freund <andres@anarazel.de>; Amit Kapila <amit.kapila16@gmail.com>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

Re: Robert Haas 2020-01-09 <CA+TgmoZC_6z3d5+6n1GVfy898uoBHMPPJU4-nAQLG5ZkaEyqdQ@mail.gmail.com>
> > So that raises the question: why does xenial's version of libedit
> > not match either its documentation or the distributed source code?
> > Because it doesn't.
> 
> The level of effort you've put into this is extremely impressive, but
> I can't shake the feeling that you're going to keep finding issues in
> the test setup, the operating system, or the upstream libraries,
> rather than bugs in PostgreSQL. Maybe this is all just one-time
> stabilization effort, but...

Fwiw if libedit in xenial is Just Broken and fixing the tests would
just codify the brokenness without adding any value, I'll just disable
that test in that particular build. It looks like setting
with_readline=no on the make command line should work.

Christoph





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-09 15:46  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Robert Haas <robertmhaas@gmail.com>
  1 sibling, 0 replies; 64+ messages in thread

From: Tom Lane @ 2020-01-09 15:46 UTC (permalink / raw)
  To: Robert Haas <robertmhaas@gmail.com>; +Cc: Christoph Berg <myon@debian.org>; Andres Freund <andres@anarazel.de>; Amit Kapila <amit.kapila16@gmail.com>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

Robert Haas <robertmhaas@gmail.com> writes:
> The level of effort you've put into this is extremely impressive, but
> I can't shake the feeling that you're going to keep finding issues in
> the test setup, the operating system, or the upstream libraries,
> rather than bugs in PostgreSQL. Maybe this is all just one-time
> stabilization effort, but...

Yeah, this is pretty nearly what I feared would happen, given libedit's
apparently-well-earned evil reputation.  I suspect that the filename
completion tests I posted over in that thread may expose another round
of issues.  If we can get past that, I think that expanding our test
coverage for the rest of tab-complete.c should be relatively
unproblematic, because there aren't very many other readline behaviors
that tab-complete.c is depending on.

One alternative is to give up on the idea of ever having any test
coverage for tab-complete.c, which doesn't seem very desirable.

Or we could disable the tests when using libedit, or provide some
switch to make it easier for the user/packager to do so.

In Debian's case, I suspect that they don't care about the behavior
when using libedit, so skipping the test would be just fine for them.
What they really ought to test is what happens after they sub in
libreadline at runtime ... but I imagine that their dubious legal
theories about all this would prevent them from actually doing that
during the package build.

			regards, tom lane

PS:  Stepping back from the immediate problem, it's obviously better
for all concerned if libedit is usable with Postgres.  So if they're
willing to patch problems, which we've found out they are, then coping
with this stuff is a win in the long run.





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-09 15:50  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Christoph Berg <myon@debian.org>
  0 siblings, 1 reply; 64+ messages in thread

From: Tom Lane @ 2020-01-09 15:50 UTC (permalink / raw)
  To: Christoph Berg <myon@debian.org>; +Cc: Robert Haas <robertmhaas@gmail.com>; Andres Freund <andres@anarazel.de>; Amit Kapila <amit.kapila16@gmail.com>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

Christoph Berg <myon@debian.org> writes:
> Fwiw if libedit in xenial is Just Broken and fixing the tests would
> just codify the brokenness without adding any value, I'll just disable
> that test in that particular build. It looks like setting
> with_readline=no on the make command line should work.

That would disable psql's tab completion, command editing, and history
altogether, which I doubt is what you want for production builds.
If we conclude we can't work around the testing issues for ancient
libedit, probably the right answer is to provide a switch to
disable just the test.  I've been trying to dance around that
conclusion, but maybe we should just do it and move on.

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-09 16:10  Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 64+ messages in thread

From: Peter Eisentraut @ 2020-01-09 16:10 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; Christoph Berg <myon@debian.org>; +Cc: Robert Haas <robertmhaas@gmail.com>; Andres Freund <andres@anarazel.de>; Amit Kapila <amit.kapila16@gmail.com>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

On 2020-01-09 16:50, Tom Lane wrote:
> Christoph Berg <myon@debian.org> writes:
>> Fwiw if libedit in xenial is Just Broken and fixing the tests would
>> just codify the brokenness without adding any value, I'll just disable
>> that test in that particular build. It looks like setting
>> with_readline=no on the make command line should work.
> 
> That would disable psql's tab completion, command editing, and history
> altogether, which I doubt is what you want for production builds.
> If we conclude we can't work around the testing issues for ancient
> libedit, probably the right answer is to provide a switch to
> disable just the test.  I've been trying to dance around that
> conclusion, but maybe we should just do it and move on.

I think he means something like

make check with_readline=no

not for the actual build.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-09 16:19  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
  0 siblings, 1 reply; 64+ messages in thread

From: Tom Lane @ 2020-01-09 16:19 UTC (permalink / raw)
  To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; +Cc: Christoph Berg <myon@debian.org>; Robert Haas <robertmhaas@gmail.com>; Andres Freund <andres@anarazel.de>; Amit Kapila <amit.kapila16@gmail.com>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> On 2020-01-09 16:50, Tom Lane wrote:
>> That would disable psql's tab completion, command editing, and history
>> altogether, which I doubt is what you want for production builds.
>> If we conclude we can't work around the testing issues for ancient
>> libedit, probably the right answer is to provide a switch to
>> disable just the test.  I've been trying to dance around that
>> conclusion, but maybe we should just do it and move on.

> I think he means something like
> 	make check with_readline=no
> not for the actual build.

Oh, I see.  I'd rather not codify that though, because it risks
problems if that symbol ever gets used any other way.  I was
thinking of making the test script check for some independent
environment variable, say SKIP_READLINE_TESTS.

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-09 21:51  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 64+ messages in thread

From: Tom Lane @ 2020-01-09 21:51 UTC (permalink / raw)
  To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; +Cc: Christoph Berg <myon@debian.org>; Robert Haas <robertmhaas@gmail.com>; Andres Freund <andres@anarazel.de>; Amit Kapila <amit.kapila16@gmail.com>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

I wrote:
> Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
>> I think he means something like
>> 	make check with_readline=no
>> not for the actual build.

> Oh, I see.  I'd rather not codify that though, because it risks
> problems if that symbol ever gets used any other way.  I was
> thinking of making the test script check for some independent
> environment variable, say SKIP_READLINE_TESTS.

I thought of another problem with the with_readline=no method,
which is that it requires the user to be issuing "make check"
directly; it wouldn't be convenient for a buildfarm owner, say.
(*Perhaps* it'd work to set with_readline=no throughout a
buildfarm run, but I think that's asking for trouble with the
build part.)  I pushed a patch using SKIP_READLINE_TESTS.
Christoph should be able to set that for the Ubuntu branches
where the test is failing.

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 64+ messages in thread

* Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
@ 2020-01-15 20:28  Christoph Berg <myon@debian.org>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 0 replies; 64+ messages in thread

From: Christoph Berg @ 2020-01-15 20:28 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; Robert Haas <robertmhaas@gmail.com>; Andres Freund <andres@anarazel.de>; Amit Kapila <amit.kapila16@gmail.com>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>

Re: Tom Lane 2020-01-09 <16328.1578606690@sss.pgh.pa.us>
> build part.)  I pushed a patch using SKIP_READLINE_TESTS.
> Christoph should be able to set that for the Ubuntu branches
> where the test is failing.

That "fixed" the problem on xenial, thanks.

Christoph





^ permalink  raw  reply  [nested|flat] 64+ messages in thread


end of thread, other threads:[~2020-01-15 20:28 UTC | newest]

Thread overview: 64+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-01-02 20:02 pgsql: Add basic TAP tests for psql's tab-completion logic. Tom Lane <tgl@sss.pgh.pa.us>
2020-01-03 11:01 ` Christoph Berg <myon@debian.org>
2020-01-03 13:52   ` Tom Lane <tgl@sss.pgh.pa.us>
2020-01-03 17:02     ` Christoph Berg <myon@debian.org>
2020-01-03 17:35       ` Tom Lane <tgl@sss.pgh.pa.us>
2020-01-03 17:48         ` Christoph Berg <myon@debian.org>
2020-01-03 17:59           ` Robert Haas <robertmhaas@gmail.com>
2020-01-04 01:32             ` Peter Geoghegan <pg@bowt.ie>
2020-01-04 02:16               ` Tom Lane <tgl@sss.pgh.pa.us>
2020-01-04 02:37                 ` Peter Geoghegan <pg@bowt.ie>
2020-01-04 02:51                   ` Tom Lane <tgl@sss.pgh.pa.us>
2020-01-04 03:06                     ` Peter Geoghegan <pg@bowt.ie>
2020-01-04 03:28                       ` Peter Geoghegan <pg@bowt.ie>
2020-01-04 05:30                         ` Tom Lane <tgl@sss.pgh.pa.us>
2020-01-04 05:38                           ` Peter Geoghegan <pg@bowt.ie>
2020-01-04 06:39                             ` Tom Lane <tgl@sss.pgh.pa.us>
2020-01-04 06:49                               ` Peter Geoghegan <pg@bowt.ie>
2020-01-04 07:00                                 ` Tom Lane <tgl@sss.pgh.pa.us>
2020-01-04 18:19                                   ` Tom Lane <tgl@sss.pgh.pa.us>
2020-01-04 18:38                                     ` Peter Geoghegan <pg@bowt.ie>
2020-01-04 19:50                                       ` Tom Lane <tgl@sss.pgh.pa.us>
2020-01-04 19:58                                         ` Tom Lane <tgl@sss.pgh.pa.us>
2020-01-04 20:31                                           ` Andrew Dunstan <andrew.dunstan@2ndquadrant.com>
2020-01-04 20:48                                           ` Tom Lane <tgl@sss.pgh.pa.us>
2020-01-04 20:46                                         ` Peter Geoghegan <pg@bowt.ie>
2020-01-04 20:56                                           ` Tom Lane <tgl@sss.pgh.pa.us>
2020-01-04 21:09                                             ` Peter Geoghegan <pg@bowt.ie>
2020-01-04 23:29                                             ` Peter Geoghegan <pg@bowt.ie>
2020-01-04 23:39                                               ` Tom Lane <tgl@sss.pgh.pa.us>
2020-01-05 00:09                                                 ` Peter Geoghegan <pg@bowt.ie>
2020-01-05 00:14                                                   ` Tom Lane <tgl@sss.pgh.pa.us>
2020-01-05 00:42                                                     ` Peter Geoghegan <pg@bowt.ie>
2020-01-05 01:57                                                       ` Tom Lane <tgl@sss.pgh.pa.us>
2020-01-05 02:15                                                         ` Tom Lane <tgl@sss.pgh.pa.us>
2020-01-05 02:16                                                         ` Peter Geoghegan <pg@bowt.ie>
2020-01-05 02:17                                                           ` Tom Lane <tgl@sss.pgh.pa.us>
2020-01-05 02:18                                                             ` Peter Geoghegan <pg@bowt.ie>
2020-01-05 13:35                                                             ` Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2020-01-05 13:39                                                               ` Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2020-01-05 16:38                                                               ` Tom Lane <tgl@sss.pgh.pa.us>
2020-01-05 16:48                                                                 ` Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2020-01-04 07:01                               ` Peter Geoghegan <pg@bowt.ie>
2020-01-03 18:17           ` Tom Lane <tgl@sss.pgh.pa.us>
2020-01-05 18:30     ` Tom Lane <tgl@sss.pgh.pa.us>
2020-01-06 10:56       ` Christoph Berg <myon@debian.org>
2020-01-06 15:15         ` Tom Lane <tgl@sss.pgh.pa.us>
2020-01-06 19:13           ` Christoph Berg <myon@debian.org>
2020-01-06 23:06             ` Tom Lane <tgl@sss.pgh.pa.us>
2020-01-07 22:28               ` Tom Lane <tgl@sss.pgh.pa.us>
2020-01-09 14:45                 ` Robert Haas <robertmhaas@gmail.com>
2020-01-09 14:59                   ` Christoph Berg <myon@debian.org>
2020-01-09 15:50                     ` Tom Lane <tgl@sss.pgh.pa.us>
2020-01-09 16:10                       ` Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
2020-01-09 16:19                         ` Tom Lane <tgl@sss.pgh.pa.us>
2020-01-09 21:51                           ` Tom Lane <tgl@sss.pgh.pa.us>
2020-01-15 20:28                             ` Christoph Berg <myon@debian.org>
2020-01-09 15:46                   ` Tom Lane <tgl@sss.pgh.pa.us>
2020-01-07 06:18             ` Amit Kapila <amit.kapila16@gmail.com>
2020-01-08 02:59         ` Amit Kapila <amit.kapila16@gmail.com>
2020-01-03 14:03   ` Tom Lane <tgl@sss.pgh.pa.us>
2020-01-03 17:12     ` Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2020-01-03 17:55       ` Tom Lane <tgl@sss.pgh.pa.us>
2020-01-03 11:46 ` Michael Paquier <michael@paquier.xyz>
2020-01-03 14:10   ` Tom Lane <tgl@sss.pgh.pa.us>

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox