Re: Byte-compilation of custom themes

[Basil L. Contovounesios]

Eli Zaretskii <address@hidden> writes:

>> From: "Basil L. Contovounesios" <address@hidden>
>> Cc: <address@hidden>,  <address@hidden>
>> Date: Sun, 03 Jun 2018 17:08:22 +0100
>> 
>> So, if we can't load cl-lib at the top-level of custom.el, and we don't
>> load cl-lib in custom-available-themes, then the featurep check says
>> "only call cl-assert if some other library has already loaded cl-lib for
>> us", which doesn't make sense to me.  What is the benefit of
>> conditionally calling an assertion at runtime?
>
> In most sessions cl-lib is loaded pretty close to startup, so the
> problem doesn't exist in practice, IMO.
>
>> Why wouldn't we want the assertion to be exercised every time
>> custom-available-themes is called?
>
> Because it's complicated, and I don't see any reason to justify that
> complication?
>
> But this is not a string opinion, so we could just drop the issue and
> move on.

Fair enough, though it's looking like the simplest solution is to keep
the original (when (file-directory-p dir) ...) check and avoid cl-lib
altogether.

Anyway, I have updated the relevant patch according to your suggestion
(the fboundp check is effectively equivalent to the featurep one, but
additionally silences the byte-compiler) and reattach the set of
patches; let me know if there are any outstanding issues with them.

>> Just to be clear, including any one of the following three top-level
>> forms in custom.el breaks 'make bootstrap', irrespective of the NOERROR
>> argument passed to require, and even when there is no subsequent mention
>> of cl-assert or any other cl-lib feature in custom.el:
>> 
>> (require 'cl-lib)
>> (eval-when-compile (require 'cl-lib))
>> (when t (require 'cl-lib))
>> 
>> In all three cases, 'make bootstrap' reports the following:
>> 
>> Loading custom (source)...
>> Warning: Unknown defun property ‘gv-setter’ in cl-fifth
>> Warning: Unknown defun property ‘gv-setter’ in cl-sixth
>> Warning: Unknown defun property ‘gv-setter’ in cl-seventh
>> Warning: Unknown defun property ‘gv-setter’ in cl-eighth
>> Warning: Unknown defun property ‘gv-setter’ in cl-ninth
>> Warning: Unknown defun property ‘gv-setter’ in cl-tenth
>> Symbol’s function definition is void: gv-define-simple-setter
>> Makefile:745: recipe for target 'bootstrap-emacs' failed
>
> Any reason you require cl-lib and not cl-macs?

I was following the suggestion of (info "(cl) Usage"), which implies
that the internal organisation of cl-lib is a bit of an implementation
detail.  I realise now that this suggestion is aimed more at external
packages, not Emacs core, though.  Bootstrap fails either way.

Thanks for indulging me,

-- 
Basil

0001-Improve-loading-of-byte-compiled-custom-themes.patch
Description: Text Data

0002-Disable-no-byte-compile-in-built-in-themes.patch
Description: Text Data

0003-Fix-custom-available-themes-file-expansion.patch
Description: Text Data

0004-lisp-custom.el-Use-lexical-binding.patch
Description: Text Data

0005-lisp-cus-theme.el-Use-lexical-binding.patch
Description: Text Data

0006-Minor-custom.el-simplifications.patch
Description: Text Data

0007-Minor-cus-theme.el-simplifications.patch
Description: Text Data

0008-Tweak-subr-x.el-substring-functions.patch
Description: Text Data