bug#21971: 25.1.50; EUDC conversion to BBDB 3 incompatibility

bug#21971: 25.1.50; EUDC conversion to BBDB 3 incompatibility

[George McNinch]

I'm reporting an incompatibility between functions found in
eudc-export.el and BBDB3.

The version of eudc should be implied by the emacs-25 version info
below. The version of bbdb is

bbdb-20151114.1541

installed from MELPA.

Here is how to reproduce the issue.

I initialize/configure bbdb, eudc and ldap as follows:

;;---------------------------------------------------------------------------
;; bbdb

(use-package bbdb
  :config
  (bbdb-initialize 'gnus 'message 'sc 'aliases))

;;---------------------------------------------------------------------------
;; eudc and ldap

(use-package eudc
  :config
  (eudc-set-server "ldap.tufts.edu" 'ldap t)
  (setq eudc-server-hotlist '(("ldap.tufts.edu" . ldap)))
  (setq eudc-inline-expansion-servers 'hotlist)
  (setq eudc-inline-query-format '((name)
				   (firstname)
				   (firstname name)
				   (email)
				   ))
  (setq eudc-ldap-bbdb-conversion-alist
	`((name . cn)
	  (net . mail)
	  (address eudc-bbdbify-address postaladdress "Address"))))

(use-package ldap
  :config
  (setq ldap-host-parameters-alist
	'(("ldap.tufts.edu" base "dc=tufts,dc=edu")))
  (setq ldap-ldapsearch-args (quote ("-tt" "-LLL" "-x"))))

;;---------------------------------------------------------------------------

Now do

M-x eudc-query server

Any hit will do; with the above server setting, you can e.g. find *me*
by entering McNinch as NAME in the query form.

After pushing the "query server" button, enter the *Directory Query
Results* buffer, put point on a record (for an individual not already in
the bbdb), and press 'b' (or do M-x eudc-try-bbdb-insert ).

I get an error, which is reported in messages as 

bbdb-change-record: Wrong type argument: listp, t

In fact, a record *has* been inserted into the bbdb; in this case it
looks like

["George" "McNinch" nil "George.McNinch@tufts.edu" nil nil nil nil ((creation-date . "2015-11-21 14:13:29 +0000") (timestamp . "2015-11-21 14:13:29 +0000")) nil]

(assuming McNinch was not already in the bbdb, of course).

This record seems "mangled" from the point-of-view of BBDB3 - the email
address should be "in a list" inside this record-vector. Perhaps other
things are amiss, as well.


Note also that the setting of eudc-ldap-bbdb-conversion-alist I made
above isn't quite the original value. If I instead use the original
value, e.g. by instead using the original value

[which amounts to:

  (setq eudc-ldap-bbdb-conversion-alist
	`((name . cn)
	  (net . mail)
	  (address eudc-bbdbify-address postaladdress "Address")
	  (phone
	   (eudc-bbdbify-phone telephonenumber "Phone")))


in the initialization of EUDC],

then when I press "b" on a record
in the *Directory Query Results* buffer, I get the error

eudc-bbdbify-phone: Symbol’s function definition is void:
bbdb-parse-phone-number


;;---------------------------------------------------------------------------
In GNU Emacs 25.1.50.1 (x86_64-unknown-linux-gnu, GTK+ Version 3.14.5)
 of 2015-11-15
Repository revision: 70f1fda4ae6abb5e11dcf281738c25f6f5b06061
Windowing system distributor 'The X.Org Foundation', version 11.0.11604000
System Description:	Debian GNU/Linux 8.2 (jessie)

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

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Messages

Minor modes in effect:
  override-global-mode: t
  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
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  buffer-read-only: t
  line-number-mode: t
  transient-mark-mode: t

Recent messages:
Parsing results... done
eudc-bbdbify-phone: Symbol’s function definition is void: bbdb-parse-phone-number
Quit

Load-path shadows:
None found.

Features:
(shadow sort bbdb-message mail-extr emacsbug sendmail bbdb-com crm
goto-addr thingatpt eudcb-ldap ldap eudc cus-edit cus-start cus-load
eudc-vars wid-edit bbdb bbdb-site timezone message dired format-spec
rfc822 mml mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231
rfc2047 rfc2045 ietf-drums mailabbrev mail-utils gmm-utils mailheader
use-package diminish bind-key easy-mmode url-handlers url-parse
auth-source cl-seq eieio byte-opt bytecomp byte-compile cl-extra cconv
eieio-core cl-macs gv gnus-util mm-util help-fns help-mode cl-loaddefs
pcase cl-lib mail-prsvr password-cache url-vars finder-inf info package
easymenu epg-config time-date mule-util tooltip eldoc electric uniquify
ediff-hook vc-hooks lisp-float-type mwheel x-win term/common-win x-dnd
tool-bar dnd fontset image regexp-opt fringe tabulated-list newcomment
elisp-mode lisp-mode prog-mode register page menu-bar rfn-eshadow timer
select scroll-bar mouse jit-lock font-lock syntax facemenu font-core
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 charscript
case-table epa-hook jka-cmpr-hook help simple abbrev 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 move-toolbar gtk
x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 135459 17741)
 (symbols 48 24227 0)
 (miscs 40 125 165)
 (strings 32 27735 3761)
 (string-bytes 1 835086)
 (vectors 16 16572)
 (vector-slots 8 486561 5903)
 (floats 8 210 32)
 (intervals 56 364 0)
 (buffers 976 16)
 (heap 1024 48378 1131))

bug#21971: 25.1.50; EUDC conversion to BBDB 3 incompatibility

[Thomas Fitzsimmons]

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

Hi George,

George McNinch <gmcninch@gmail.com> writes:

[...]

> In fact, a record *has* been inserted into the bbdb; in this case it
> looks like
>
> ["George" "McNinch" nil "George.McNinch@tufts.edu" nil nil nil nil ((creation-date . "2015-11-21 14:13:29 +0000") (timestamp . "2015-11-21 14:13:29 +0000")) nil]
>
> (assuming McNinch was not already in the bbdb, of course).
>
> This record seems "mangled" from the point-of-view of BBDB3 - the email
> address should be "in a list" inside this record-vector. Perhaps other
> things are amiss, as well.

Thank you for the detailed description of how to replicate the issue.

> Note also that the setting of eudc-ldap-bbdb-conversion-alist I made
> above isn't quite the original value. If I instead use the original
> value, e.g. by instead using the original value
>
> [which amounts to:
>
>   (setq eudc-ldap-bbdb-conversion-alist
> 	`((name . cn)
> 	  (net . mail)
> 	  (address eudc-bbdbify-address postaladdress "Address")
> 	  (phone
> 	   (eudc-bbdbify-phone telephonenumber "Phone")))
>
>
> in the initialization of EUDC],
>
> then when I press "b" on a record
> in the *Directory Query Results* buffer, I get the error
>
> eudc-bbdbify-phone: Symbol’s function definition is void:
> bbdb-parse-phone-number

OK, this seems like a separate issue.  I haven't investigated it yet.

With your eudc-ldap-bbdb-conversion-alist workaround still enabled, can
you try the attached patch to see if it fixes exporting for you?

Thanks,
Thomas


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-BBDB-3-support-for-EUDC-export.patch --]
[-- Type: text/x-patch, Size: 3451 bytes --]

From 3a4870a3076467c8465df9ba5dd0646acf1d6fd0 Mon Sep 17 00:00:00 2001
From: Thomas Fitzsimmons <tfitzsim@tfitzsim-lnx2.cisco.com>
Date: Sat, 21 Nov 2015 21:24:27 -0500
Subject: [PATCH] Add BBDB 3 support for EUDC export

* eudc.el: Add bbdb-version defvar.
(eudc--using-bbdb-3-or-newer-p): New function.
* eudc-export.el (eudc-create-bbdb-record): Add support for
bbdb-create-internal argument list changes introduced in BBDB 3.
* eudcb-bbdb.el: Remove bbdb-version defvar.
(eudc-bbdb-field): Call eudc--using-bbdb-3-or-newer-p.
---
 lisp/net/eudc-export.el | 19 +++++++++++++------
 lisp/net/eudc.el        | 12 ++++++++++++
 lisp/net/eudcb-bbdb.el  | 10 +---------
 3 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/lisp/net/eudc-export.el b/lisp/net/eudc-export.el
index c60911f..a65f555 100644
--- a/lisp/net/eudc-export.el
+++ b/lisp/net/eudc-export.el
@@ -86,12 +86,19 @@ eudc-create-bbdb-record
 					      (cons (car mapping) value))))
 				       conversion-alist)))
       (setq bbdb-notes (delq nil bbdb-notes))
-      (setq bbdb-record (bbdb-create-internal bbdb-name
-					      bbdb-company
-					      bbdb-net
-					      bbdb-address
-					      bbdb-phones
-					      bbdb-notes))
+      (setq bbdb-record (bbdb-create-internal
+			 bbdb-name
+			 ,@(when (eudc--using-bbdb-3-or-newer-p)
+			     '(nil
+			       nil))
+			 bbdb-company
+			 bbdb-net
+			 ,@(if (eudc--using-bbdb-3-or-newer-p)
+			       '(bbdb-phones
+				 bbdb-address)
+			     '(bbdb-address
+			       bbdb-phones))
+			 bbdb-notes))
       (or silent
 	  (bbdb-display-records (list bbdb-record))))))
 
diff --git a/lisp/net/eudc.el b/lisp/net/eudc.el
index 7280d9d..25a26bd 100644
--- a/lisp/net/eudc.el
+++ b/lisp/net/eudc.el
@@ -107,6 +107,18 @@ eudc-switch-from-server-hook
 ;; attribute name
 (defvar eudc-protocol-has-default-query-attributes nil)
 
+(defvar bbdb-version)
+
+(defun eudc--using-bbdb-3-or-newer-p ()
+  "Return non-nil if BBDB version is 3 or greater."
+  (or
+   ;; MELPA versions of BBDB may have a bad package version, but
+   ;; they're all version 3 or later.
+   (equal bbdb-version "@PACKAGE_VERSION@")
+   ;; Development versions of BBDB can have the format "X.YZ devo".
+   ;; Split the string just in case.
+   (version<= "3" (car (split-string bbdb-version)))))
+
 (defun eudc-plist-member (plist prop)
   "Return t if PROP has a value specified in PLIST."
   (if (not (= 0 (% (length plist) 2)))
diff --git a/lisp/net/eudcb-bbdb.el b/lisp/net/eudcb-bbdb.el
index 0545304..1972fc1 100644
--- a/lisp/net/eudcb-bbdb.el
+++ b/lisp/net/eudcb-bbdb.el
@@ -42,21 +42,13 @@
 (defvar eudc-bbdb-current-query nil)
 (defvar eudc-bbdb-current-return-attributes nil)
 
-(defvar bbdb-version)
-
 (defun eudc-bbdb-field (field-symbol)
   "Convert FIELD-SYMBOL so that it is recognized by the current BBDB version.
 BBDB < 3 used `net'; BBDB >= 3 uses `mail'."
   ;; This just-in-time translation permits upgrading from BBDB 2 to
   ;; BBDB 3 without restarting Emacs.
   (if (and (eq field-symbol 'net)
-	   (or
-	    ;; MELPA versions of BBDB may have a bad package version,
-	    ;; but they're all version 3 or later.
-	    (equal bbdb-version "@PACKAGE_VERSION@")
-	    ;; Development versions of BBDB can have the format "X.YZ
-	    ;; devo".  Split the string just in case.
-	    (version<= "3" (car (split-string bbdb-version)))))
+	   (eudc--using-bbdb-3-or-newer-p))
       'mail
     field-symbol))
 
-- 
1.8.3.1

bug#21971: 25.1.50; EUDC conversion to BBDB 3 incompatibility

[George McNinch]

Hi Thomas,

    TF> With your eudc-ldap-bbdb-conversion-alist workaround still
    TF> enabled, can you try the attached patch to see if it fixes
    TF> exporting for you?

Indeed, this works fine for me; thanks!

    >> Note also that the setting of eudc-ldap-bbdb-conversion-alist I
    >> made above isn't quite the original value. If I instead use the
    >> original value, e.g. by instead using the original value
    >> 
    >> [which amounts to:
    >> 
    >> (setq eudc-ldap-bbdb-conversion-alist `((name . cn) (net . mail)
    >> (address eudc-bbdbify-address postaladdress "Address") (phone
    >> (eudc-bbdbify-phone telephonenumber "Phone")))
    >> 
    >> 
    >> in the initialization of EUDC],
    >> 
    >> then when I press "b" on a record in the *Directory Query
    >> Results* buffer, I get the error
    >> 
    >> eudc-bbdbify-phone: Symbol’s function definition is void:
    >> bbdb-parse-phone-number

    TF> OK, this seems like a separate issue.  I haven't investigated it
    TF> yet.

Right; should I have made a separate report, or is this OK?  

--------------------------------------------
By the way:

I wonder: if bbdb really now uses "mail" instead of "net", I'd think
that (?) that eudc-ldap-bbdb-conversion-alist should instead read:

  (setq eudc-ldap-bbdb-conversion-alist
   `((name . cn)
     (mail . mail)
     (address eudc-bbdbify-address postaladdress "Address")))

But that doesn't seem to work (I tried... I get then BBDB record with
*no* mail field at all).

In general, I find it difficult to work out what can be used in that
alist. E.g. it seems that there is a key symbol ou for the record
passed to eudc-create-bbdb-record.

So I tried adding the pair

 (Organizations . ou)

to that alist, but it doens't seem to populate the "right part of the
BBDB record" - in fact, the BBDB record gets a new line with an
Organizations field, but it makes the BBDB unhappy (it eventually
complains of an "illegal field" or something). I'd hoped to populate the
bit that shows up to the right of the name when displaying the BBDB
record, which I thought was called "Organizations".

I also tried (organization . ou) and (company . ou) and maybe a few
others, but nothing seemed to work and guessing doesn't seem so healthy,
and looking at the code in eudc-export.el doesn't seem like

Well, this is a distraction, and only tangentially related to the patch,
but it does point out that it'd be nice to have a clearer definition of
the fields on both sides of this conversion...!

Best,
george

-- 
   + + +   George McNinch <gmcninch@gmail.com>                        
   + + +   http://gmcninch.math.tufts.edu                             

bug#21971: 25.1.50; EUDC conversion to BBDB 3 incompatibility

[Thomas Fitzsimmons]

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

George McNinch <gmcninch@gmail.com> writes:

> Hi Thomas,
>
>     TF> With your eudc-ldap-bbdb-conversion-alist workaround still
>     TF> enabled, can you try the attached patch to see if it fixes
>     TF> exporting for you?
>
> Indeed, this works fine for me; thanks!
>
>     >> Note also that the setting of eudc-ldap-bbdb-conversion-alist I
>     >> made above isn't quite the original value. If I instead use the
>     >> original value, e.g. by instead using the original value
>     >> 
>     >> [which amounts to:
>     >> 
>     >> (setq eudc-ldap-bbdb-conversion-alist `((name . cn) (net . mail)
>     >> (address eudc-bbdbify-address postaladdress "Address") (phone
>     >> (eudc-bbdbify-phone telephonenumber "Phone")))
>     >> 
>     >> 
>     >> in the initialization of EUDC],
>     >> 
>     >> then when I press "b" on a record in the *Directory Query
>     >> Results* buffer, I get the error
>     >> 
>     >> eudc-bbdbify-phone: Symbol’s function definition is void:
>     >> bbdb-parse-phone-number
>
>     TF> OK, this seems like a separate issue.  I haven't investigated it
>     TF> yet.
>
> Right; should I have made a separate report, or is this OK?  

It's fine, we can work on it here.

> --------------------------------------------
> By the way:
>
> I wonder: if bbdb really now uses "mail" instead of "net", I'd think
> that (?) that eudc-ldap-bbdb-conversion-alist should instead read:
>
>   (setq eudc-ldap-bbdb-conversion-alist
>    `((name . cn)
>      (mail . mail)
>      (address eudc-bbdbify-address postaladdress "Address")))
>
> But that doesn't seem to work (I tried... I get then BBDB record with
> *no* mail field at all).
>
> In general, I find it difficult to work out what can be used in that
> alist. E.g. it seems that there is a key symbol ou for the record
> passed to eudc-create-bbdb-record.
>
> So I tried adding the pair
>
>  (Organizations . ou)
>
> to that alist, but it doens't seem to populate the "right part of the
> BBDB record" - in fact, the BBDB record gets a new line with an
> Organizations field, but it makes the BBDB unhappy (it eventually
> complains of an "illegal field" or something). I'd hoped to populate the
> bit that shows up to the right of the name when displaying the BBDB
> record, which I thought was called "Organizations".
>
> I also tried (organization . ou) and (company . ou) and maybe a few
> others, but nothing seemed to work and guessing doesn't seem so healthy,
> and looking at the code in eudc-export.el doesn't seem like
>
> Well, this is a distraction, and only tangentially related to the patch,
> but it does point out that it'd be nice to have a clearer definition of
> the fields on both sides of this conversion...!

Can you remove your eudc-ldap-bbdb-conversion-alist customization and
try the attached patch?  I think it will fix both the phone parsing and
organization missing issues.

I wouldn't worry about 'net unless you're customizing it directly.  It's
a change and I left the old 'net as the customization entry point, but
it will get converted to 'mail depending on which version of BBDB is in
use.

Thomas


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Improve-EUDC-to-BBDB-3-export.patch --]
[-- Type: text/x-patch, Size: 2255 bytes --]

From 25b742df574fc9ae8340a37409fc72ec2f40574b Mon Sep 17 00:00:00 2001
From: Thomas Fitzsimmons <fitzsim@fitzsim.org>
Date: Sun, 22 Nov 2015 11:29:13 -0500
Subject: [PATCH] Improve EUDC to BBDB 3 export

* eudc-vars.el (eudc-ldap-bbdb-conversion-alist): Change phone
entry to single item.  Add company conversion.
* eudc-export.el (eudc-bbdbify-company): New function.
(eudc-bbdbify-phone): Add BBDB 3 support.
(bug#21971)
---
 lisp/net/eudc-export.el | 8 +++++++-
 lisp/net/eudc-vars.el   | 3 ++-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/lisp/net/eudc-export.el b/lisp/net/eudc-export.el
index a65f555..f91f4ae 100644
--- a/lisp/net/eudc-export.el
+++ b/lisp/net/eudc-export.el
@@ -169,6 +169,10 @@ eudc-bbdbify-address
                   (string &optional number-type))
 (declare-function bbdb-string-trim "ext:bbdb" (string))
 
+(defun eudc-bbdbify-company (&rest organizations)
+  "Return ORGANIZATIONS as a list compatible with BBDB."
+  organizations)
+
 (defun eudc-bbdbify-phone (phone location)
   "Parse PHONE into a vector compatible with BBDB.
 PHONE is either a string supposedly containing a phone number or
@@ -179,7 +183,9 @@ eudc-bbdbify-phone
    ((stringp phone)
     (let (phone-list)
       (condition-case err
-	  (setq phone-list (bbdb-parse-phone-number phone))
+	  (setq phone-list (if (eudc--using-bbdb-3-or-newer-p)
+			       (bbdb-parse-phone phone)
+			     (bbdb-parse-phone-number phone)))
 	(error
 	 (if (string= "phone number unparsable." (cadr err))
 	     (if (not (y-or-n-p (format "BBDB claims %S to be unparsable--insert anyway? " phone)))
diff --git a/lisp/net/eudc-vars.el b/lisp/net/eudc-vars.el
index 8cffa8e..a08d175 100644
--- a/lisp/net/eudc-vars.el
+++ b/lisp/net/eudc-vars.el
@@ -369,7 +369,8 @@ eudc-ldap-bbdb-conversion-alist
   '((name . cn)
     (net . mail)
     (address . (eudc-bbdbify-address postaladdress "Address"))
-    (phone . ((eudc-bbdbify-phone telephonenumber "Phone"))))
+    (phone . (eudc-bbdbify-phone telephonenumber "Phone"))
+    (company . (eudc-bbdbify-company o)))
   "A mapping from BBDB to LDAP attributes.
 This is a list of cons cells (BBDB-FIELD . SPEC-OR-LIST) where
 BBDB-FIELD is the name of a field that must be defined in your BBDB
-- 
1.8.3.1

bug#21971: 25.1.50; EUDC conversion to BBDB 3 incompatibility

[George McNinch]

Hi Thomas,

    TF> Can you remove your eudc-ldap-bbdb-conversion-alist
    TF> customization and try the attached patch?  I think it will fix
    TF> both the phone parsing and organization missing issues.

Both issues are resolved for me; thanks!

Best,
george

-- 
   φ-φ-φ   George McNinch <gmcninch@gmail.com>                        
   φ-φ-φ   http://gmcninch.math.tufts.edu                             

bug#21971: 25.1.50; EUDC conversion to BBDB 3 incompatibility

[Thomas Fitzsimmons]

fixed 21971 25.1
thanks

George McNinch <gmcninch@gmail.com> writes:

> Hi Thomas,
>
>     TF> Can you remove your eudc-ldap-bbdb-conversion-alist
>     TF> customization and try the attached patch?  I think it will fix
>     TF> both the phone parsing and organization missing issues.
>
> Both issues are resolved for me; thanks!

I pushed the fixes to the emacs-25 branch.  They'll subsequently get
merged from there to master.

Thanks for the report.  Closing.

Thomas