kazu-yamamoto / Mew

Messaging in the Emacs World
http://www.mew.org/
Other
191 stars 51 forks source link

Make sure macros are defined during native compilation. #157

Closed tdanckaert closed 1 year ago

tdanckaert commented 1 year ago

Emacs 28 supports native compilation of byte-compiled modules. Native compilation is performed asynchronously in the background, and modules may be compiled in a different order than the order in which they are imported when running mew. This commit adds a number of (require) forms to make sure all macros are defined when they are used.

This commit seems to fix all errors, though I cannot guarantee that more thorough testing might uncover a few more missing imports.

I had to move a few macros to a new file mew-env0.el to avoid circular require statements. Perhaps there is a cleaner way to do it.

fixes #155

kazu-yamamoto commented 1 year ago

Merged. Thank you for your contribution!

aaa70 commented 1 year ago

Hi @tdanckaert and @kazu-yamamoto Thank you for your update!

But I'm still getting error on my environment:

Compiling /home/user/.emacs.d/elpa/mew-20221117.122/mew-vars3.el...
/home/user/.emacs.d/elpa/mew-20221117.122/mew-vars3.el: Error: Symbol's function definition is void mew-face-make-spec

So maybe, you should move mew-face-make-spec in mew-vars3.el to mew-env.el(or mew-env0.el?). That works fine for me. Thanks,

tdanckaert commented 1 year ago

Hi @aaa70 ,

I had noticed the same error in the compilation warning log, but was confused on how to fix it (I still do not understand how "Symbol's function definition is void" is possible if the function is defined in the same file...). Because everything seemed to work when running Mew (at least in my daily usage), I thought I'd submit what I had so far...

Interesting that moving the function to mew-env.el solves the problem (I do not understand why, but my elisp knowledge is very limited), but I don't know if that is a logical place for it. Perhaps @kazu-yamamoto can shed some light on it?

aaa70 commented 1 year ago

Hi @tdanckaert

I still do not understand how "Symbol's function definition is void" is possible if the function is defined in the same file...

Exactly.

Interesting that moving the function to mew-env.el solves the problem (I do not understand why, but my elisp knowledge is very limited), but I don't know if that is a logical place for it.

I don't know elisp well, especially native compilation. And I have realized mew-vars3.el won't be compiled if mew-face-make-spec has moved to mew-env.el.

tats commented 1 year ago

mew-vars3.el: Error: Symbol's function definition is void mew-face-make-spec

It seems mew-face-make-spec is evaluated and incomplete at compile time.

Adding (require 'mew-env) to mew-vars3.el may prevent this error.

kazu-yamamoto commented 1 year ago

@aaa70 Would you try the idea from @tats?

tdanckaert commented 1 year ago

I've just tried it, and can confirm that adding (eval-when-compile (require 'mew-env)) to mew-vars3.el makes the last compilation error go away. I assume it's due to the use of mew-face-spec-primitive, which is defalias-ed in mew-env.el?

kazu-yamamoto commented 1 year ago

@tdanckaert Very nice. For your credit, would you send a PR?

tdanckaert commented 1 year ago

Pull request submitted, though credit is due to @tats

Looking forward to using Mew in 2023!

aaa70 commented 1 year ago

@tats

Thank you for your advice. (require 'mew-env) works.

@tdanckaert

Thank you for submitting additional pr.

Actually, I got an another error when viewing attached images like:

mew-mime-image: Invalid function: mew-plet

Maybe, mew-elet too. So mew-gemacs.el needs

(eval-when-compile (require 'mew-func))
(eval-when-compile (require 'mew-mule3))

Best,

tats commented 1 year ago

mew-smime.el and mew-win32.el seem to have similar issues.

mew-func.el:(defmacro mew-elet (&rest body)
mew-gemacs.el:    (mew-elet

mew-func.el:(defmacro mew-rendezvous (who)
mew-smime.el:    (mew-rendezvous mew-smime-running)
mew-smime.el:    (mew-rendezvous mew-smime-running)
mew-smime.el:    (mew-rendezvous mew-smime-running)

mew-mule3.el:(defmacro mew-plet (&rest body)
mew-gemacs.el:      (mew-plet

mew-mule3.el:(defmacro mew-flet (&rest body)
mew-smime.el:   (mew-flet

mew-mule3.el:(defmacro mew-frwlet (read write &rest body)
mew-win32.el:    (mew-frwlet mew-cs-dummy mew-w32-cs-print

BTW, to simplify, (require 'mew) may be enough for mew-func, mew-env, mew-mule3, etc., though roughly dependency.

tats commented 1 year ago

Pull request submitted for mew-gemacs.el, mew-smime.el, and mew-win32. cf. https://github.com/kazu-yamamoto/Mew/pull/159