bug#25236: 26.0.50; sql-postgres not working on master

bug#25236: 26.0.50; sql-postgres not working on master

[sunshavi]

Hi.

This is working on 25.1 but not on master compiled toda.

I tried it with with 'emacs -Q'

Regards




In GNU Emacs 26.0.50.1 (x86_64-unknown-linux-gnu, X toolkit, Xaw3d scroll bars)
 of 2016-12-19 built on caral
System Description:	Arch Linux

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
process-lines: psql exited with status 2
Quit
Configured using:
 'configure '--program-transform-name=s/^ctags$/ctags.emacs/'
 --prefix=/usr --sysconfdir=/etc --libexecdir=/usr/lib
 --localstatedir=/var --with-x-toolkit=lucid --mandir=/usr/share/man
 --pdfdir=/usr/share/doc/emacs --with-sound --without-gconf --with-xft
 --without-xwidgets --with-gameuser=:games 'CFLAGS=-march=x86-64
 -mtune=generic -O2 -pipe -fstack-protector-strong'
 CPPFLAGS=-D_FORTIFY_SOURCE=2
 LDFLAGS=-Wl,-O1,--sort-common,--as-needed,-z,relro'

Configured features:
XAW3D XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GSETTINGS
NOTIFY ACL GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS LUCID X11 LIBSYSTEMD

Important settings:
  value of $LC_COLLATE: C
  value of $LANG: C
  locale-coding-system: nil

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message subr-x puny seq byte-opt gv
bytecomp byte-compile cl-extra help-mode cconv dired dired-loaddefs
format-spec rfc822 mml mml-sec password-cache epa derived epg epg-config
gnus-util rmail rmail-loaddefs mm-decode mm-bodies mm-encode mail-parse
rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045
ietf-drums mm-util mail-prsvr mail-utils sql easymenu view thingatpt
comint ansi-color ring cl-loaddefs pcase cl-lib term/xterm xterm
time-date disp-table mule-util tooltip eldoc electric uniquify
ediff-hook vc-hooks lisp-float-type mwheel term/x-win x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode elisp-mode lisp-mode
prog-mode register page menu-bar rfn-eshadow isearch timer select
scroll-bar mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors frame cl-generic cham georgian utf-8-lang misc-lang
vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms cp51932
hebrew greek romanian slovak czech european ethiopic indian cyrillic
chinese composite charscript case-table epa-hook jka-cmpr-hook help
simple abbrev obarray minibuffer cl-preloaded nadvice loaddefs button
faces cus-face macroexp files text-properties overlay sha1 md5 base64
format env code-pages mule custom widget hashtable-print-readable
backquote dbusbind inotify dynamic-setting system-font-setting
font-render-setting x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 107196 6967)
 (symbols 48 20913 1)
 (miscs 40 33 121)
 (strings 32 20197 3828)
 (string-bytes 1 673838)
 (vectors 16 12775)
 (vector-slots 8 412029 5278)
 (floats 8 187 568)
 (intervals 56 226 66)
 (buffers 976 11))

bug#25236: 26.0.50; sql-postgres not working on master

[Simen Heggestøyl]

This bug seems to be caused by a change I made in
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=24646, making
`sql-postgres` able to complete the database name by looking at the
result of `psql -ltX`.

psql exits with status code 2 if the connection to the server went
bad, for instance if permissions are missing [1].

I think we can fix this by being more lax in
`sql-postgres-list-databases`, and just don't provide any completions
should the command fail. Then we'd also need to change the value of
the `REQUIRE-MATCH` parameter to `completing-read` on line 2966, since
requiring a match when there are no completions would make it
impossible to continue.

What do you think, Michael?

-- Simen

[1] 
https://www.postgresql.org/docs/current/static/app-psql.html#AEN98634

bug#25236: 26.0.50; sql-postgres not working on master

[Michael Mauger]

On Friday, December 30, 2016 1:17 PM, Simen Heggestøyl <simenheg@gmail.com> wrote:
>
>This bug seems to be caused by a change I made in
>https://debbugs.gnu.org/cgi/bugreport.cgi?bug=24646, making
>`sql-postgres` able to complete the database name by looking at the
>result of `psql -ltX`.
>
>psql exits with status code 2 if the connection to the server went
>bad, for instance if permissions are missing [1].
>
>I think we can fix this by being more lax in
>`sql-postgres-list-databases`, and just don't provide any completions
>should the command fail. Then we'd also need to change the value of
>the `REQUIRE-MATCH` parameter to `completing-read` on line 2966, since
>requiring a match when there are no completions would make it
>impossible to continue.
>
>What do you think, Michael?
>
>-- Simen
>
>[1] 
>https://www.postgresql.org/docs/current/static/app-psql.html#AEN98634
>


Just got back after the holiday. Makes sense that could be a problem. I'm getting ready to push a change related to completing-read so let me take a look.

Thanks for staying on top of this...

-- 
MICHAEL MAUGER // FSF Member // GNU Emacs sql-mode maintainer // GNU Linux, GNU Emacs

bug#25236: 26.0.50; sql-postgres not working on master

[Simen Heggestøyl]

[-- Attachment #1: Type: text/plain, Size: 498 bytes --]

On Wed, Jan 4, 2017 at 4:16 AM, Michael Mauger <michael@mauger.com> 
wrote:
> Just got back after the holiday. Makes sense that could be a problem. 
> I'm getting ready to push a change related to completing-read so let 
> me take a look.
> 
> Thanks for staying on top of this...

I think the attached patch fixes half of the problem. Then we need to
do something about the `completing-read' to make it possible to still
select a database when `sql-postgres-list-databases' returns nil.

-- Simen

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Handle-non-zero-exit-status-from-psql-more-gracefull.patch --]
[-- Type: text/x-patch, Size: 2119 bytes --]

From 100fc2038511a10edbda1fbf0c877bef33efbe4c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Simen=20Heggest=C3=B8yl?= <simenheg@gmail.com>
Date: Sat, 18 Feb 2017 17:50:16 +0100
Subject: [PATCH] Handle non-zero exit status from psql more gracefully

* lisp/progmodes/sql.el (sql-postgres-list-databases): Handle non-zero
exit statuses from `psql -ltX' more gracefully by returning nil.

* test/lisp/progmodes/sql-tests.el
(sql-tests-postgres-list-databases-error): New test.
---
 lisp/progmodes/sql.el            |  7 ++++---
 test/lisp/progmodes/sql-tests.el | 10 ++++++++++
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/lisp/progmodes/sql.el b/lisp/progmodes/sql.el
index 8868343129..4a936a6b45 100644
--- a/lisp/progmodes/sql.el
+++ b/lisp/progmodes/sql.el
@@ -1087,9 +1087,10 @@ sql-postgres-list-databases
   "Return a list of available PostgreSQL databases."
   (when (executable-find sql-postgres-program)
     (let ((res '()))
-      (dolist (row (process-lines sql-postgres-program "-ltX"))
-        (when (string-match "^ \\([[:alnum:]-_]+\\) +|.*" row)
-          (push (match-string 1 row) res)))
+      (ignore-errors
+        (dolist (row (process-lines sql-postgres-program "-ltX"))
+          (when (string-match "^ \\([[:alnum:]-_]+\\) +|.*" row)
+            (push (match-string 1 row) res))))
       (nreverse res))))
 
 ;; Customization for Interbase
diff --git a/test/lisp/progmodes/sql-tests.el b/test/lisp/progmodes/sql-tests.el
index 27a72aa2c2..f75005f737 100644
--- a/test/lisp/progmodes/sql-tests.el
+++ b/test/lisp/progmodes/sql-tests.el
@@ -43,5 +43,15 @@
     (should (equal (sql-postgres-list-databases)
                    '("db-name-1" "db_name_2")))))
 
+(ert-deftest sql-tests-postgres-list-databases-error ()
+  "Test that nil is returned when `psql -ltX' fails."
+  (cl-letf
+      (((symbol-function 'executable-find)
+        (lambda (_command) t))
+       ((symbol-function 'process-lines)
+        (lambda (_program &rest _args)
+          (error))))
+    (should-not (sql-postgres-list-databases))))
+
 (provide 'sql-tests)
 ;;; sql-tests.el ends here
-- 
2.11.0

bug#25236: 26.0.50; sql-postgres not working on master

[erreina]

Hi, any further progress on this? The database name completion is
awesome...but the require match on the database name make it impossible to
continue when working on a remote database server.

Best,
Ernesto



--
View this message in context: http://emacs.1067599.n8.nabble.com/bug-25236-26-0-50-sql-postgres-not-working-on-master-tp416235p426363.html
Sent from the Emacs - Bugs mailing list archive at Nabble.com.

bug#25236: 26.0.50; sql-postgres not working on master

[Simen Heggestøyl]

[-- Attachment #1: Type: text/plain, Size: 311 bytes --]

On Wed, Jan 4, 2017 at 4:16 AM, Michael Mauger <michael@mauger.com> 
wrote:
> I'm getting ready to push a change related to completing-read so let 
> me take a look.

Hi Michael, any progress on this? Otherwise, does the attached patch
look OK to install? I think it should fix the described problem.

-- Simen

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Handle-non-zero-exit-status-from-psql-more-gracefull.patch --]
[-- Type: text/x-patch, Size: 2546 bytes --]

From 6a203bdf63bda37a28d5f4ae51386f552e337895 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Simen=20Heggest=C3=B8yl?= <simenheg@gmail.com>
Date: Sat, 29 Jul 2017 18:20:43 +0200
Subject: [PATCH] Handle non-zero exit status from psql more gracefully

* lisp/progmodes/sql.el (sql-postgres-list-databases): Handle non-zero
exit statuses from `psql -ltX' more gracefully by returning nil.
(sql-get-login-ext): Don't require a database name match.

* test/lisp/progmodes/sql-tests.el
(sql-tests-postgres-list-databases-error): New test.
---
 lisp/progmodes/sql.el            |  9 +++++----
 test/lisp/progmodes/sql-tests.el | 10 ++++++++++
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/lisp/progmodes/sql.el b/lisp/progmodes/sql.el
index 68ca37207e..05370ba0aa 100644
--- a/lisp/progmodes/sql.el
+++ b/lisp/progmodes/sql.el
@@ -1090,9 +1090,10 @@ sql-postgres-list-databases
   "Return a list of available PostgreSQL databases."
   (when (executable-find sql-postgres-program)
     (let ((res '()))
-      (dolist (row (process-lines sql-postgres-program "-ltX"))
-        (when (string-match "^ \\([[:alnum:]-_]+\\) +|.*" row)
-          (push (match-string 1 row) res)))
+      (ignore-errors
+        (dolist (row (process-lines sql-postgres-program "-ltX"))
+          (when (string-match "^ \\([[:alnum:]-_]+\\) +|.*" row)
+            (push (match-string 1 row) res))))
       (nreverse res))))
 
 ;; Customization for Interbase
@@ -2971,7 +2972,7 @@ sql-get-login-ext
            (expand-file-name file-name))))
 
       ((plist-member plist :completion)
-       (completing-read prompt-def (plist-get plist :completion) nil t
+       (completing-read prompt-def (plist-get plist :completion) nil nil
                         last-value history-var default))
 
       ((plist-get plist :number)
diff --git a/test/lisp/progmodes/sql-tests.el b/test/lisp/progmodes/sql-tests.el
index 27a72aa2c2..f75005f737 100644
--- a/test/lisp/progmodes/sql-tests.el
+++ b/test/lisp/progmodes/sql-tests.el
@@ -43,5 +43,15 @@
     (should (equal (sql-postgres-list-databases)
                    '("db-name-1" "db_name_2")))))
 
+(ert-deftest sql-tests-postgres-list-databases-error ()
+  "Test that nil is returned when `psql -ltX' fails."
+  (cl-letf
+      (((symbol-function 'executable-find)
+        (lambda (_command) t))
+       ((symbol-function 'process-lines)
+        (lambda (_program &rest _args)
+          (error))))
+    (should-not (sql-postgres-list-databases))))
+
 (provide 'sql-tests)
 ;;; sql-tests.el ends here
-- 
2.13.2

bug#25236: new mail address (was: bug#25236: 26.0.50; sql-postgres not working on master)

[andrés ramírez]

Hi Simen, Michael.

My fastmail account is going to be closed on 2 days. On the future send
me a copy to this mail address (gmail).

AR

bug#25236: 26.0.50; sql-postgres not working on master

[Michael Mauger]

[-- Attachment #1: Type: text/plain, Size: 991 bytes --]

Sorry for the delay; ADD keeps in a perpetual state of: "what was it that I was doing...?"
I had made the changes locally a while ago, but hedn't pushed them to the reporsity. They are there now. Let me know if you have any issues.
Again, sorry for the delay.

--
MICHAEL MAUGER // FSF and EFF member // GNU Emacs sql.el maintainer

> -------- Original Message --------
> Subject: Re: bug#25236: 26.0.50; sql-postgres not working on master
> Local Time: July 29, 2017 12:26 PM
> UTC Time: July 29, 2017 4:26 PM
> From: simenheg@gmail.com
> To: Michael Mauger <michael@mauger.com>
> sunshavi@fastmail.fm <sunshavi@fastmail.fm>, 25236@debbugs.gnu.org <25236@debbugs.gnu.org>
> On Wed, Jan 4, 2017 at 4:16 AM, Michael Mauger <michael@mauger.com>
> wrote:
>> I"m getting ready to push a change related to completing-read so let
>> me take a look.
> Hi Michael, any progress on this? Otherwise, does the attached patch
> look OK to install? I think it should fix the described problem.
> -- Simen

[-- Attachment #2: Type: text/html, Size: 1634 bytes --]

bug#25236: 26.0.50; sql-postgres not working on master

[Simen Heggestøyl]

[-- Attachment #1: Type: text/plain, Size: 146 bytes --]

Thanks Michael. There's still a problem when psql exists with a
non-zero exit status. I think the attached patch is needed in
addition.

-- Simen

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Handle-non-zero-exit-status-from-psql-more-gracefull.patch --]
[-- Type: text/x-patch, Size: 2118 bytes --]

From a12099e443e9ab1a26715daafca6abb6c99f29ab Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Simen=20Heggest=C3=B8yl?= <simenheg@gmail.com>
Date: Wed, 9 Aug 2017 15:34:34 +0200
Subject: [PATCH] Handle non-zero exit status from psql more gracefully

* lisp/progmodes/sql.el (sql-postgres-list-databases): Handle non-zero
exit statuses from `psql -ltX' more gracefully by returning nil.

* test/lisp/progmodes/sql-tests.el
(sql-tests-postgres-list-databases-error): New test.
---
 lisp/progmodes/sql.el            |  7 ++++---
 test/lisp/progmodes/sql-tests.el | 10 ++++++++++
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/lisp/progmodes/sql.el b/lisp/progmodes/sql.el
index b176e52950..48e21605a3 100644
--- a/lisp/progmodes/sql.el
+++ b/lisp/progmodes/sql.el
@@ -1095,9 +1095,10 @@ sql-postgres-list-databases
   "Return a list of available PostgreSQL databases."
   (when (executable-find sql-postgres-program)
     (let ((res '()))
-      (dolist (row (process-lines sql-postgres-program "-ltX"))
-        (when (string-match "^ \\([[:alnum:]-_]+\\) +|.*" row)
-          (push (match-string 1 row) res)))
+      (ignore-errors
+        (dolist (row (process-lines sql-postgres-program "-ltX"))
+          (when (string-match "^ \\([[:alnum:]-_]+\\) +|.*" row)
+            (push (match-string 1 row) res))))
       (nreverse res))))
 
 ;; Customization for Interbase
diff --git a/test/lisp/progmodes/sql-tests.el b/test/lisp/progmodes/sql-tests.el
index 27a72aa2c2..f75005f737 100644
--- a/test/lisp/progmodes/sql-tests.el
+++ b/test/lisp/progmodes/sql-tests.el
@@ -43,5 +43,15 @@
     (should (equal (sql-postgres-list-databases)
                    '("db-name-1" "db_name_2")))))
 
+(ert-deftest sql-tests-postgres-list-databases-error ()
+  "Test that nil is returned when `psql -ltX' fails."
+  (cl-letf
+      (((symbol-function 'executable-find)
+        (lambda (_command) t))
+       ((symbol-function 'process-lines)
+        (lambda (_program &rest _args)
+          (error))))
+    (should-not (sql-postgres-list-databases))))
+
 (provide 'sql-tests)
 ;;; sql-tests.el ends here
-- 
2.13.2

bug#25236: 26.0.50; sql-postgres not working on master

[Eli Zaretskii]

> Date: Thu, 10 Aug 2017 23:26:13 +0200
> From: Simen Heggestøyl <simenheg@gmail.com>
> Cc: Michael Mauger <michael@mauger.com>,
>  andrés ramírez <rrandresf@gmail.com>,
>  "25236@debbugs.gnu.org" <25236@debbugs.gnu.org>,
>  "sunshavi@fastmail.fm" <sunshavi@fastmail.fm>
> 
> Thanks Michael. There's still a problem when psql exists with a
> non-zero exit status. I think the attached patch is needed in
> addition.

Michael, any reasons not to push this patch?

bug#25236: 26.0.50; sql-postgres not working on master

[Michael Mauger]

[-- Attachment #1: Type: text/plain, Size: 940 bytes --]

No. Go ahead and push it.

--
MICHAEL@MAUGER.COM // FSF and EFF member // GNU Emacs sql.el maintainer

> -------- Original Message --------
> Subject: Re: bug#25236: 26.0.50; sql-postgres not working on master
> Local Time: September 2, 2017 9:09 AM
> UTC Time: September 2, 2017 1:09 PM
> From: eliz@gnu.org
> To: Simen Heggestøyl <simenheg@gmail.com>
> mmauger@protonmail.com, michael@mauger.com, rrandresf@gmail.com, 25236@debbugs.gnu.org, sunshavi@fastmail.fm
>
>> Date: Thu, 10 Aug 2017 23:26:13 +0200
>> From: Simen Heggestøyl <simenheg@gmail.com>
>> Cc: Michael Mauger <michael@mauger.com>,
>> andrés ramírez <rrandresf@gmail.com>,
>> "25236@debbugs.gnu.org" <25236@debbugs.gnu.org>,
>> "sunshavi@fastmail.fm" <sunshavi@fastmail.fm>
>>
>> Thanks Michael. There"s still a problem when psql exists with a
>> non-zero exit status. I think the attached patch is needed in
>> addition.
>
> Michael, any reasons not to push this patch?

[-- Attachment #2: Type: text/html, Size: 1600 bytes --]

bug#25236: 26.0.50; sql-postgres not working on master

[Simen Heggestøyl]

On Tue, Sep 5, 2017 at 2:18 AM, Michael Mauger <mmauger@protonmail.com> 
wrote:
> No. Go ahead and push it.

Pushed!

-- Simen