* Re: [Emacs-diffs] master 28b7dd4 2/2: Fix build error in bytecomp.el from previous change [not found] ` <20191001144137.1032C20927@vcs0.savannah.gnu.org> @ 2019-10-01 15:08 ` Stefan Monnier 2019-10-01 15:22 ` Lars Ingebrigtsen 2019-10-02 16:57 ` Sven Joachim 0 siblings, 2 replies; 5+ messages in thread From: Stefan Monnier @ 2019-10-01 15:08 UTC (permalink / raw) To: emacs-devel; +Cc: Lars Ingebrigtsen Hi Lars, > --- a/lisp/emacs-lisp/bytecomp.el > +++ b/lisp/emacs-lisp/bytecomp.el > @@ -124,7 +124,7 @@ > (require 'backquote) > (require 'macroexp) > (require 'cconv) > -(eval-when-compile (require 'compile)) > +(require 'compile) This is a bad change. It brings in many more packages, and every additional package brought in reduces the quality of our warnings (e.g. if any of the packages (transitively) loaded load cl-lib, then we fail to signal when a package uses cl-lib functionality without requiring cl-lib). We should try and find some other solution. Stefan ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Emacs-diffs] master 28b7dd4 2/2: Fix build error in bytecomp.el from previous change 2019-10-01 15:08 ` [Emacs-diffs] master 28b7dd4 2/2: Fix build error in bytecomp.el from previous change Stefan Monnier @ 2019-10-01 15:22 ` Lars Ingebrigtsen 2019-10-01 21:30 ` Stefan Monnier 2019-10-02 16:57 ` Sven Joachim 1 sibling, 1 reply; 5+ messages in thread From: Lars Ingebrigtsen @ 2019-10-01 15:22 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: >> -(eval-when-compile (require 'compile)) >> +(require 'compile) > > This is a bad change. > > It brings in many more packages, and every additional package brought > in reduces the quality of our warnings (e.g. if any of the packages > (transitively) loaded load cl-lib, then we fail to signal when > a package uses cl-lib functionality without requiring cl-lib). I'm not quite sure I followed the last bit -- if something transitively required cl-lib, then there's no point for the "top" file to require cl-lib, surely? Or is this a style issue. But I agree that requiring compile.el runtime is probably not good, but: > We should try and find some other solution. No other solution I thought of at the top of my head seemed good. Moving either of those keymaps to a different place seemed logical, either... The problem is that the mode now inherits from the parent defined in compile.el: (defvar emacs-lisp-compilation-mode-map (let ((map (make-sparse-keymap))) (set-keymap-parent map compilation-minor-mode-map) (define-key map "g" 'emacs-lisp-compilation-recompile) map)) -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Emacs-diffs] master 28b7dd4 2/2: Fix build error in bytecomp.el from previous change 2019-10-01 15:22 ` Lars Ingebrigtsen @ 2019-10-01 21:30 ` Stefan Monnier 2019-10-03 15:18 ` Lars Ingebrigtsen 0 siblings, 1 reply; 5+ messages in thread From: Stefan Monnier @ 2019-10-01 21:30 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: emacs-devel > I'm not quite sure I followed the last bit -- if something transitively > required cl-lib, then there's no point for the "top" file to require > cl-lib, surely? Or is this a style issue. If a file (transitively) required by bytecomp.el requires cl-lib, then when compiling *another* file any missing (require 'cl-lib) in that other file won't be noticed. > (defvar emacs-lisp-compilation-mode-map > (let ((map (make-sparse-keymap))) > (set-keymap-parent map compilation-minor-mode-map) > (define-key map "g" 'emacs-lisp-compilation-recompile) > map)) Do we need this set-keymap-parent? define-derived-mode should do it for us (when the mode is called). Stefan ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Emacs-diffs] master 28b7dd4 2/2: Fix build error in bytecomp.el from previous change 2019-10-01 21:30 ` Stefan Monnier @ 2019-10-03 15:18 ` Lars Ingebrigtsen 0 siblings, 0 replies; 5+ messages in thread From: Lars Ingebrigtsen @ 2019-10-03 15:18 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: >> I'm not quite sure I followed the last bit -- if something transitively >> required cl-lib, then there's no point for the "top" file to require >> cl-lib, surely? Or is this a style issue. > > If a file (transitively) required by bytecomp.el requires cl-lib, > then when compiling *another* file any missing (require 'cl-lib) in that > other file won't be noticed. Oh, this is just special for bytecomp.el; I get it. >> (defvar emacs-lisp-compilation-mode-map >> (let ((map (make-sparse-keymap))) >> (set-keymap-parent map compilation-minor-mode-map) >> (define-key map "g" 'emacs-lisp-compilation-recompile) >> map)) > > Do we need this set-keymap-parent? define-derived-mode should do it for > us (when the mode is called). I didn't know that. I've now made the change (and reverted the `require'), and this do indeed work as they should. -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Emacs-diffs] master 28b7dd4 2/2: Fix build error in bytecomp.el from previous change 2019-10-01 15:08 ` [Emacs-diffs] master 28b7dd4 2/2: Fix build error in bytecomp.el from previous change Stefan Monnier 2019-10-01 15:22 ` Lars Ingebrigtsen @ 2019-10-02 16:57 ` Sven Joachim 1 sibling, 0 replies; 5+ messages in thread From: Sven Joachim @ 2019-10-02 16:57 UTC (permalink / raw) To: Stefan Monnier; +Cc: Lars Ingebrigtsen, emacs-devel On 2019-10-01 11:08 -0400, Stefan Monnier wrote: > Hi Lars, > >> --- a/lisp/emacs-lisp/bytecomp.el >> +++ b/lisp/emacs-lisp/bytecomp.el >> @@ -124,7 +124,7 @@ >> (require 'backquote) >> (require 'macroexp) >> (require 'cconv) >> -(eval-when-compile (require 'compile)) >> +(require 'compile) > > This is a bad change. > > It brings in many more packages, and every additional package brought > in reduces the quality of our warnings (e.g. if any of the packages > (transitively) loaded load cl-lib, then we fail to signal when > a package uses cl-lib functionality without requiring cl-lib). Indeed, this is bug #30635, mentioned in the comments you had written just around Lars' changes: ,---- | ;; Refrain from using cl-lib at run-time here, since it otherwise prevents | ;; us from emitting warnings when compiling files which use cl-lib without | ;; requiring it! (bug#30635) `---- > We should try and find some other solution. Cheers, Sven ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-10-03 15:18 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20191001144135.32717.19241@vcs0.savannah.gnu.org> [not found] ` <20191001144137.1032C20927@vcs0.savannah.gnu.org> 2019-10-01 15:08 ` [Emacs-diffs] master 28b7dd4 2/2: Fix build error in bytecomp.el from previous change Stefan Monnier 2019-10-01 15:22 ` Lars Ingebrigtsen 2019-10-01 21:30 ` Stefan Monnier 2019-10-03 15:18 ` Lars Ingebrigtsen 2019-10-02 16:57 ` Sven Joachim
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).