Subject: bug#38261: Recent changes to emacs build system

From: [email-masked]
Date: Tue, 19 Nov 2019 22:16:43 +0100

On 19.11.2019 22:14, Maxim Cournoyer wrote:
> Hello Brett,
> 
> Thank you for the heads-up concerning the latest changes to the way
> Emacs find its libraries and the adjustments done to the
> emacs-build-system.
> 
> [email-masked] writes:
> 
>> On 19.11.2019 01:32, [email-masked] wrote:
>>> On 19.11.2019 01:27, [email-masked] wrote:
>>>> On 18.11.2019 22:34, [email-masked] wrote:
>>>>> On 18.11.2019 21:33, [email-masked] wrote:
>>>>>> On 18.11.2019 21:28, [email-masked] wrote:
>>>>>>> On 18.11.2019 20:01, [email-masked] wrote:
>>>>>>>> Hey all, the recent changes to the emacs build system result in
>>>>>>>> a few
>>>>>>>> broken packages like emacs-pdf-tools, or basically anything
>>>>>>>> that uses
>>>>>>>> a phase for emacs-set-emacs-load-path.
>>>>>>>> 
>>>>>>>> For example, I borrowed the technique used by emacs-pdf-tools to
>>>>>>>> package emacs-telega, resulting in both packages failing to 
>>>>>>>> build:
>>>>>>>> 
>>>>>>>> Here is the code for emacs-telega:
>>>>>>>> 
>>>>>>>> https://git.sr.ht/~brettgilio/cfg/tree/master/channel/non-gnu/packages/emacs-xyz.scm#L99
>>>>>>>> 
>>>>>>>> The issue is in this phase for both emacs-pdf-tools and my
>>>>>>>> channel package:
>>>>>>>> 
>>>>>>>>  (add-after 'compress-documentation 'emacs-set-emacs-load-path
>>>>>>>> 			  (assoc-ref emacs:%standard-phases 'set-emacs-load-path))
>>>>>>>> 
>>>>>>>> Resulting in:
>>>>>>>> 
>>>>>>>> starting phase `emacs-set-emacs-load-path'
>>>>>>>> Backtrace:
>>>>>>>>            5 (primitive-load
>>>>>>>> "/gnu/store/5b1p1gsvfyi4fbx4s42rhab2dns…")
>>>>>>>> In ice-9/eval.scm:
>>>>>>>>    191:35  4 (_ _)
>>>>>>>> In ice-9/boot-9.scm:
>>>>>>>>     829:9  3 (catch _ _ #<procedure 7ffff3bbb518 at
>>>>>>>> /gnu/store/zmkg…> …)
>>>>>>>> In srfi/srfi-1.scm:
>>>>>>>>    863:16  2 (every1 #<procedure 7ffff30ae160 at
>>>>>>>> /gnu/store/zmkgrvv…> …)
>>>>>>>> In
>>>>>>>> /gnu/store/zmkgrvvhmrix2b1z7id6zrg9bb7qxzdl-module-import/guix/build/gnu-build-system.scm:
>>>>>>>>    839:30  1 (_ _)
>>>>>>>> In unknown file:
>>>>>>>>            0 (_ #:source
>>>>>>>> "/gnu/store/qw8xbmk6ryl9a2jrp0gip3yffmsdix…" …)
>>>>>>>> 
>>>>>>>> ERROR: Wrong type to apply: #f
> 
> Sorry for failing to foresee this problem.  I've fixed the couple
> packages that were impacted on master now.  You can have a look at the
> fixes (they are fairly simple).
> 
>>>>>>>> If we suspect that changes are going to be non-backwards
>>>>>>>> compatible
>>>>>>>> could we use the news system to pass along that message? Much
>>>>>>>> appreciated. Thanks.
>>>>>>>> 
>>>>>>>> Brett Gilio
>>>>>>> 
>>>>>>> I applied a hotfix to the package by replacing 
>>>>>>> 'set-emacs-load-path
>>>>>>> with 'add-source-to-load-path. I believe this fix would work for
>>>>>>> all
>>>>>>> other affected packages.
>>>>>> 
>>>>>> https://git.sr.ht/~brettgilio/cfg/commit/c24a6db9d25151c487e9db675bd74e4bb3912173
> 
> Indeed!
> 
>>>>> Ricardo, just wanted to make you aware that this emacs-build-system
>>>>> change does break your guile-studio. I discovered this because I
>>>>> adopted some of your ideas of autoloading in the generated emacs 
>>>>> lisp
>>>>> from guile-studio when creating my own emacs configuration 
>>>>> dependent
>>>>> on guix, which resulted in
>>>>> 
>>>>> progn: Wrong number of arguments: (lambda nil "Autoload Emacs
>>>>> packages
>>>>> found in EMACSLOADPATH.
>>>>> 
>>>>> 'Autoload' means to load the 'autoloads' files matching
>>>>> `guix-emacs-autoloads-regexp'." (interactive) (let* 
>>>>> ((emacs-load-path
>>>>> (getenv "EMACSLOADPATH")) (emacs-non-core-load-path-directories
>>>>> (seq-filter (function (lambda (dir) (string-match-p
>>>>> "/share/emacs/site-lisp" dir))) (split-string emacs-load-path 
>>>>> ":")))
>>>>> (autoloads (mapcan (function guix-emacs-find-autoloads)
>>>>> emacs-non-core-load-path-directories))) (mapc (function (lambda (f)
>>>>> (load f (quote noerror)))) autoloads))), 78
>>>>> 
>>>>> I'll let you know if I can find any fix for this when I get some
>>>>> time.
>>>>> But just wanted to pass the message along.
>>>>> 
>>>>> Brett
>>>> 
>>>> I tried removing the arguments passed to
>>>> `guix-emacs-autoload-packages' as the updated version
>>>> (http://git.savannah.gnu.org/cgit/guix.git/tree/gnu/packages/aux-files/emacs/guix-emacs.el?id=47b3b4c2aa49e21f4cc32c97ff7bbbd069bb849c)
>>>> does not carry arguments anymore, instead depending on a variable
>>>> EMACSLOADPATH. However, whenever that is done it returns
>>>> 
>>>> split-string: Wrong type argument: stringp, nil
>>>> 
>>>> possibly because my system is not finding EMACSLOADPATH.
> 
> Yes, that's the reason.  Perhaps it could be worthwhile to give a 
> better
> message in this condition, but it's unclear to me why this condition
> would arise at all.  Would you mind giving some more background to how
> you got into that situation?  I'll try building guile-studio and see if
> I could get a hint or two.
> 
>>>> I will keep investigating.
>>> 
>>> I can confirm, when running something analogous to `guix environment
>>> --ad-hoc emacs-dashboard` no changes are being made to the
>>> $GUIX_ENVIRONMENT/etc/profile to resemble EMACSLOADPATH.
>> 
>> So I have investigated the issue with EMACSLOADPATH: Similar to
>> Ricardo's guile-studio,
>> my package enlign was using the autoload function from guix-emacs.el
>> to propagate a list
>> of packages called from inputs to be loaded with the start of emacs.
>> 
>> I have since revised this to just call the autoload function directly,
>> no longer passing any arguments, and had to modify my enlign package
>> to respect the EMACSLOADPATH variable. This is only possible if all of
>> the packages are passed as propagated-inputs though, which is less
>> than desirable in my opinion (though I am willing to be convinced.)
> 
> Is your package an Emacs library/package? If so, just defining the 
> other
> Elisp dependencies of this package as propagated inputs would be the
> correct solution.
> 
> For other cases (I can't think of how that would work exactly), perhaps
> wrapping the package's script with the computed EMACSLOADPATH
> environment variable could do?
> 
>> This additionally leads to having to require emacs as a propagated
>> input as well, as leading it native or regular input leads to an error
>> with emacs not recognizing simple.el or mule-util.
> 
> Do you mean, that your package somehow uses Emacs without having Emacs
> installed in the profile (not being propagated)?  If this is the case,
> wrapping your program with EMACSLOADPATH (and patching its reference to
> Emacs) should work, without having to propagate anything.
> 
>> Overall, I am not sure if I like the recent change to guix-emacs.el. I
>> understand where the incentive is to do this, but ultimately it seems
>> to lead to more headache when creating packages around emacs.
>> 
>> https://git.sr.ht/~brettgilio/cfg/commit/47bc9ae544ba10c0cacef1435625dcb0114e8cf5
>> 
>> Anyways,
>> 
>> If there is no other comment, this issue really just needs to fix
>> emacs-pdf-tools and other affected packages from the recent string of
>> changes.
> 
> I'm sorry you've had some problems with this recent change!  Thanks for
> bringing them out.  Please do continue, I'm interested to know about 
> any
> remaining issues you might find.
> 
> Maxim

Thank you for your work Maxim, I think most of them are resolved now. I 
do wonder about what prompted the change, though. Perhaps you might 
share?

Brett