bug#12952: 24.3.50; Buggy handling of dependencies between customized variables

bug#12952: 24.3.50; Buggy handling of dependencies between customized variables

[Stephen Berman]

There's a bug in the handling of :set-after dependencies between customized
variables; details below.  But first, here's a recipe showing how the bug can
have annoying side effects:

0. Ensure that ~/.emacs and ~/.emacs.d/ do not exist or are empty.
1. Start Emacs (*without* -Q or -q).
2. Type `M-x customize-create-theme RET', then create a theme, give it a
   name and save it (it is helpful if the theme is readily recognizable
   when enabled, e.g., default face with the strike-through attribute in
   red).
3. Type `M-x customize-option RET custom-file RET', change the Value Menu from
   `Your Emacs init file' to `File', type a file name in the editable field
   (choosing `~/.emacs' as the value, though rather perverse, also triggers
   the bug), then select the State `Save for Future Sessions'.
4. Type `M-x customize-themes RET', click the check-box next to the name
   of the theme created in step 2, then click the button 'Save Theme
   Settings'.

Now the file named in step 3 should look like this (modulo the values):

(custom-set-variables
 ;; custom-set-variables was added by Custom.
 ;; If you edit it by hand, you could mess it up, so be careful.
 ;; Your init file should contain only one such instance.
 ;; If there is more than one, they won't work right.
 '(custom-enabled-themes (quote (srb-test)))
 '(custom-file "~/.emacs-custom.el")
 '(custom-safe-themes (quote ("c0e0b5618faae142b2a62d45174f9a0db392ee7ca12accd705a3a524e3490adf" default))))
(custom-set-faces
 ;; custom-set-faces was added by Custom.
 ;; If you edit it by hand, you could mess it up, so be careful.
 ;; Your init file should contain only one such instance.
 ;; If there is more than one, they won't work right.
 )

5. If you gave custom-file a different name than `~/.emacs' in step 3,
   now type `C-x C-f ~/.emacs RET' and insert this line:
   (load "~/.emacs-custom.el")
   where the file to be loaded is the one named in step 3, then save
   ~/.emacs.
6. Kill Emacs, then start it again (as in step 1).

=> The frame is split, with the *scratch* buffer above, the buffer
   *Custom Theme* below, containing the code of the theme created in
   step 2, and this is in the Minibuffer:
   "Loading a theme can run Lisp code.  Really load? (y or n) ".
   If you type `y', you should see the theme created in step 2, and this
   in the Minibuffer: 
   "Treat this theme as safe in future sessions? (y or n) ".
   Even if you answer `y', the next time you start Emacs (with the
   customizations made above), the same thing happens again, and every
   subsequent time.  In other words, even though you tell Emacs to save the
   theme as safe, Emacs does not treat it as safe the next time it starts. 

The patch below fixes at least this specific problem, and I think it
will work in most cases likely the arise in practice.  To motivate it,
here's my analysis of the bug's cause and this particular side effect:

The sort predicate in custom-theme-set-variables, which checks for
custom-dependencies between pairs of variables in the alphabetical list
of customized variables, returns nil if there is no dependency, so the
sort function leaves the order of such variables unchanged.  Now sort
assumes (as documented in the manual) that if a relation holds between
two variables, it is transitive; this allows saving come computations.
Thus, when the sort predicate etablishes that Rab and Rbc hold, it does
not check Rac, but assumes it holds.  However, while the
variable-setting dependency relation is transitive, its dual is not: if
var1 does not depend on var2 and var2 does not depend on var3, it does
not follow that var1 does not depend on var3.  Since the predicate
checking is short-circuited, a dependency relation may be missed and the
needed reordering of variables for setting omitted.  AFAICT, that is
what happens in the above case.

Specifically, custom-enabled-themes depends on custom-safe-themes, but
neither enters into a dependency relation with custom-file.  Now the
sort predicate checks this alphabetical list pairwise from the end: it
finds no dependency between custom-safe-themes and custom-file, nor
between custom-file and custom-enabled-themes, and assuming
transitivity, does not check the relation between custom-safe-themes and
custom-enabled-themes, missing their dependency and leaving them to be
set in alphabetical order.  Thus, when custom-enabled-themes is set, it
calls load-theme, which checks whether srb-test is safe, but since
custom-safe-themes has not yet been set, the check fails, resulting in
the theme not being enabled and the Minibuffer questions being asked.
If both questions are affirmed, srb-theme is enabled and added to
custom-safe-themes (by customize-push-and-save) and both this and
custom-enabled-themes are written to custom-file, which has already been
set (in step 3 of the above recipe).  Thus the customized variables are
set correctly anew.  But the next time Emacs is started the same steps
are repeated, and so on, ad infinitum.

I suspect a general solution to this problem would need to check all
pairs of variables for dependencies in arbitrarily long lists,
effectively circumventing sort's assumption of transivity; maybe it
would be better not to use sort in that case.  Anyway, I couldn't figure
out a general solution, but I did come up with a simple fix that at
least solves the case which prompted this bug report, by reordering
pairs of variables, neither of which depends on the other.  This makes
the sort predicate continue checking, until it finds a dependency or
exhausts the list.  I haven't tried to test if this approach will works
if there are dependency chains with more than one link, but I think such
cases are unlikely in practice, so until a better solution is found, I
think this fix improves on the current status.

This bug also exists on the emacs-24 branch, but I guess it's been
around since the Custom facility has recognized dependencies (but
customizations that result in noticeable bad side effects, like the one
reported here, are evidently quite rare).


In GNU Emacs 24.3.50.4 (x86_64-suse-linux-gnu, GTK+ Version 3.4.4)
 of 2012-11-21 on rosalinde
Bzr revision: 110970 rgm@gnu.org-20121121111740-z7q5jq8dmyh328a6
Windowing system distributor `The X.Org Foundation', version 11.0.11203000
System Description:	openSUSE 12.2 (x86_64)

Configured using:
 `configure '--without-toolkit-scroll-bars' 'CFLAGS=-g3 -O0''


2012-11-21  Stephen Berman  <stephen.berman@gmx.net>

	* custom.el (custom-theme-set-variables): Reorder variables if
	neither is a dependency of the other, in order to circumvent
	sort's assumption of transitivity, which may wrongly short-circuit
	the dependency check (bug#xxxx).


=== modified file 'lisp/custom.el'
*** lisp/custom.el	2012-09-28 12:18:38 +0000
--- lisp/custom.el	2012-11-21 13:52:42 +0000
***************
*** 970,976 ****
  		  (cond ((and 1-then-2 2-then-1)
  			 (error "Circular custom dependency between `%s' and `%s'"
  				sym1 sym2))
! 			(2-then-1 nil)
  			;; 1 is a dependency of 2, so needs to be set first.
  			(1-then-2)
  			;; Put minor modes and symbols with :require last.
--- 970,980 ----
  		  (cond ((and 1-then-2 2-then-1)
  			 (error "Circular custom dependency between `%s' and `%s'"
  				sym1 sym2))
! 			;; Reorder 1 and 2 if neither is a dependency of the
! 			;; other, in order to circumvent sort's assumption of
! 			;; transitivity, which may wrongly short-circuit the
! 			;; dependency check (bug#xxxx).
! 			((not (or 1-then-2 2-then-1)) t)
  			;; 1 is a dependency of 2, so needs to be set first.
  			(1-then-2)
  			;; Put minor modes and symbols with :require last.

bug#12952: 24.3.50; Buggy handling of dependencies between customized variables

[Chong Yidong]

Stephen Berman <stephen.berman@gmx.net> writes:

> The patch below fixes at least this specific problem, and I think it
> will work in most cases likely the arise in practice.

I'd rather fix it right, which basically involves a topological sort.

bug#12952: 24.3.50; Buggy handling of dependencies between customized variables

[Stephen Berman]

On Fri, 23 Nov 2012 23:49:41 +0800 Chong Yidong <cyd@gnu.org> wrote:

> Stephen Berman <stephen.berman@gmx.net> writes:
>
>> The patch below fixes at least this specific problem, and I think it
>> will work in most cases likely the arise in practice.
>
> I'd rather fix it right, which basically involves a topological sort.

Sounds good; I hope someone does it before I'd be able to.

Steve Berman

bug#12952: 24.3.50; Buggy handling of dependencies between customized variables

[Chong Yidong]

Stephen Berman <stephen.berman@gmx.net> writes:

>> I'd rather fix it right, which basically involves a topological sort.
>
> Sounds good; I hope someone does it before I'd be able to.

Fixed in trunk.  Thanks for the bug report.

bug#12952: 24.3.50; Buggy handling of dependencies between customized variables

[Stephen Berman]

On Sat, 24 Nov 2012 11:46:55 +0800 Chong Yidong <cyd@gnu.org> wrote:

> Stephen Berman <stephen.berman@gmx.net> writes:
>
>>> I'd rather fix it right, which basically involves a topological sort.
>>
>> Sounds good; I hope someone does it before I'd be able to.
>
> Fixed in trunk.  Thanks for the bug report.

Excellent; thanks.  For the record, I confirm it fixes the problem I
reported.

Steve Berman