Created attachment 662161 [details, diff] [PATCH] Provide ebuild-run-mode and employ it in ebuild-run-command We offer a patch that introduces a simple major mode (`ebuild-run-mode') for buffers spawned by `ebuild-run-command'. The main benefit is, it allows jumping to errors from the *ebuild compile* buffer properly. At the moment errors are highlighted by compilation-mode but `compile-goto-error' can't find the relevant directory on its own. When it comes to ebuild-mode.el, this only requires one symbol to be added to `compile' call. This has only been tested on Emacs 28.0.9999. Caveats: - I define `ebuild-run-mode' in a separate file; macros and utility functions are placed into yet other two files. I think it's a good practice but it's not critical. - Docstring style is a little idiosyncratic in places. - `font-lock' settings highlight exit statuses, `compiled' in `Source compiled' and not much else. - `with-ebuild-compflation-buffer' performs variable capture. If you object to this, please consider whether prefixing symbols with ebuild-macs- would make it acceptable. - `ebuild-util-ebuild-eclasses' is not utilized. I added it assuming that sometimes knowledge of eclass could aid in determining "${S}" more accurately. (This is true for one eclass of mine but I don't know whether it's a good practice.)
Forgot to mention: this is for app-emacs/ebuild-mode
(In reply to akater from comment #0) > This has only been tested on Emacs 28.0.9999. So far, ebuild-mode has supported all versions of GNU Emacs that are in the Gentoo repository, as well as XEmacs (because there is an app-xemacs/ebuild-mode package). We have some add-ons that don't work with XEmacs (e.g., glep-mode) but they're not required by ebuild-mode itself. > - I define `ebuild-run-mode' in a separate file; macros and utility > functions are placed into yet other two files. I think it's a good > practice but it's not critical. Indeed I'd prefer having these in a single file. > - `ebuild-util-ebuild-eclasses' is not utilized. I added it assuming > that sometimes knowledge of eclass could aid in determining "${S}" > more accurately. (This is true for one eclass of mine but I don't > know whether it's a good practice.) Maybe omit the function for now then?
OK, should I just gather contents of -util and -macs in ebuild-run-mode.el then? Concerning tests for older Emacs, I could install an older version, slotted, eselect, build ebuild-mode, start, load, test ebuild-run-mode, repeat. Any known issues with this? Best practices? I can't afford breaking Emacs on any of my devices now. XEmacs tests will be a different adventure (I never used XEmacs). I will omit ebuild-eclasses.
(In reply to akater from comment #3) > OK, should I just gather contents of -util and -macs in ebuild-run-mode.el > then? Yes, please. > Concerning tests for older Emacs, I could install an older version, slotted, > eselect, build ebuild-mode, start, load, test ebuild-run-mode, repeat. Any > known issues with this? Best practices? That should work just fine. I normally start testing by simply loading the *.el file in the old Emacs version. The procedure you describe above would be a second step, to make sure that byte-compilation works properly.
Created attachment 663748 [details, diff] New revision of the patch
(the following uses org markup) I tested - loading ebuild-run-mode.el, ebuild-mode.el - compile-goto-error on emacs-$VERSION -q and compilation of the whole ebuild-mode with emacs-$VERSION. I could not test on some Emacsen. Tests for 23, 18 will most likely show the need for some more additions. I'll try again but I only have non-uclibc or multilib system ready. I'll try with xemacs too. Patch updated; new stuff is in a single file now. * DONE Emacs 28.0.9999 ** DONE Loads ** DONE compile-goto-error works ** DONE Compiles without new warnings * DONE Emacs 27.1-r1 ** DONE Loads ** DONE compile-goto-error works ** DONE Compiles without new warnings * HOLD Emacs 26.3-r2 Couldn't emerge: #+begin_example compilation ,************************************************** Warning: Your system has a gap between BSS and the heap (26975448 bytes). This usually means that exec-shield or something similar is in effect. The dump may fail because of this. See the section about exec-shield in etc/PROBLEMS for more information. ,************************************************** /bin/sh: line 1: 9485 Segmentation fault SANDBOX_ON=0 LD_PRELOAD= env ./temacs --batch --load loadup bootstrap #+end_example ** TODO Loads ** TODO compile-goto-error works ** TODO Compiles without new warnings * DONE Emacs 25.3-r7 ** DONE Loads ** DONE compile-goto-error works ** DONE Compiles without new warnings * DONE Emacs 24.5-r8 ** DONE Loads ** DONE compile-goto-error works ** DONE Compiles without new warnings * HOLD Emacs 23.4-r19 Won't build on uclibc profile. ** TODO Loads ** TODO compile-goto-error works ** TODO Compiles without new warnings * HOLD Emacs 18.59-r13 Won't build on no-multilib profile. ** TODO Loads ** TODO compile-goto-error works ** TODO Compiles without new warnings
Sorry, a typo: I only have non-multilib uclibc system ready.
(In reply to akater from comment #6) Have you tested with XEmacs too?
When it comes to XEmacs, even something like (with-current-buffer (find-file "/[sudo/portage@localhost]/usr/local/portage/app-emacs/ebuild-mode/ebuild-mode-9999.ebuild") (ebuild-run-command "clean")) won't work so I dropped all support for ebuild-run-mode from XEmacs. I will soon attach an update and a patch to app-xemacs/ebuild-mode that removes ebuild-run-mode.el at prepare stage. ebuild-run-command is thus hardly useful in XEmacs; had I known this in advance, I wouldn't spend time trying to add support for XEmacs tramp-related dependencies and so on. I repeated the tests with the new patch, results repeat those above. Tests for 23, 18 will most likely show the need for some more additions in dependencies provided by `eval-and-compile' form in `ebuild-run-mode'. If 26 needs anything at all, it will likely be additional condition to the wrapper around (defvar ebuild-run-mode).
Created attachment 664399 [details, diff] New revision of the patch
(In reply to akater from comment #9) > I repeated the tests with the new patch, results repeat those above. Tests > for 23, 18 will most likely show the need for some more additions in > dependencies provided by `eval-and-compile' form in `ebuild-run-mode'. We don't support external packages for Emacs 18, so no need to care about that slot.
Created attachment 664417 [details, diff] Version of ebuild-mode patch with version bump I should have updated the version. app-xemacs/ebuild-mode patch has to wait until ebuild-mode-1.52 is actually available, I guess.
(In reply to akater from comment #12) > I should have updated the version. Please don't. I shall update the version before cutting a new release, and having version changes in other commits will only cause trouble (e.g., merge conflicts). I would push your patch to a branch of the ebuild-mode repository for testing. Before doing so, here are two things that caught my eye: - In ebuild-run-mode.el, move the "lexical-binding: t" assignment to a local variables block, for consistency with the rest of the package. Also limit the length of lines to less than 80 characters, please. - In ebuild-mode.el, you have an XEmacs conditional that defines an alias for cl-macrolet, which is then used for another XEmacs conditional. Certainly there must be a simpler way of doing this? In fact, I'd rather avoid requiring cl-macs if it is just for one single command in ebuild-mode.el. (I think we need not care much about optimisation, there's nothing time critical with these high-level commands.)
* 80 chars, lexical-binding > - In ebuild-run-mode.el, move the "lexical-binding: t" assignment to > a local variables block, for consistency with the rest of the > package. Also limit the length of lines to less than 80 characters, > please. Ensured 80 chars everywhere except in one regexp: it's not clear what to do with string regexps in this situation, and I just borrowed this one. Re: lexical-binding. If moved to the bottom, it triggers a new warning (in GNU Emacs 28.0, at least): > Warning (files): ebuild-run-mode.el: ‘lexical-binding’ at end of > file unreliable Thus, I removed it. It is not necessary; I think enabling it by default is a good practice but I'd rather not have compiler warnings. * Tests I ran another series of tests; everything works. I managed to test on GNU Emacs 26 and 23, as provided by previous revision ebuilds. * DONE Emacs 28.0.9999 ** DONE Loads ** DONE compile-goto-error works ** DONE Compiles without new warnings * DONE Emacs 27.1-r1 ** DONE Loads ** DONE compile-goto-error works ** DONE Compiles without new warnings * DONE Emacs 26.3-r1 (current tree has r2) ** DONE Loads ** DONE compile-goto-error works ** DONE Compiles without new warnings * DONE Emacs 25.3-r7 ** DONE Loads ** DONE compile-goto-error works ** DONE Compiles without new warnings * DONE Emacs 24.5-r8 ** DONE Loads ** DONE compile-goto-error works ** DONE Compiles without new warnings * TODO Emacs 23.4-r18 (current tree has r19) ** DONE Loads ** TODO compile-goto-error works Even find-file /sudo:: ... won't work for me on emacs-23 -q. ** DONE Compiles with new warning Warning: Function `gensym' from cl package called at runtime (3 times) I don't know what to do with this warning and have little motivation to investigate right now. Anyway, cl is required at load time now in ebuild-run-mode.el in the (sort of) supported version of GNU Emacs 23. * No cl-macs dependency in ebuild-mode > - In ebuild-mode.el, you have an XEmacs conditional that defines an > alias for cl-macrolet, which is then used for another XEmacs > conditional. Certainly there must be a simpler way of doing this? In > fact, I'd rather avoid requiring cl-macs if it is just for one > single command in ebuild-mode.el. `cl-macs' dependency is avoided now: a global eval-when-compile'd macro is used. Alternatively, we could check for xemacs at runtime but checking which editor we're in (not even the version we're on) at runtime is not reasonable. I hope I won't have to support such a practice. Note: using a macro here is only necessary because Elisp doesn't have read-time conditionals. Had Elisp had them, noone would argue what the correct approach would be. Without them, two choices are left: either achieve or at least approximate the aforementioned correct approach by other means, or do something we wouldn't do in the presence of better tools. I'm always reluctant to do the latter or encourage it.
Created attachment 665185 [details, diff] Post-review patch
(In reply to akater from comment #14) > Ensured 80 chars everywhere except in one regexp: it's not clear what > to do with string regexps in this situation, and I just borrowed this one. Use backslash concatenation, e.g.: "^Ebuild \\(exited abnormally\\|interrupt\\|killed\\|terminated\ \\|segmentation fault\\)\\(?:.*with code \\([0-9]+\\)\\)?.*" > ** DONE Compiles without new warnings > * TODO Emacs 23.4-r18 (current tree has r19) It is stange that r18 works for you while r19 doesn't. Only change between the two is renaming of the "X" USE flag to "gui". > Alternatively, we could check for xemacs at runtime but checking which > editor we're in (not even the version we're on) at runtime is not > reasonable. I hope I won't have to support such a practice. > > Note: using a macro here is only necessary because Elisp doesn't have > read-time conditionals. Had Elisp had them, noone would argue what > the correct approach would be. Without them, two choices are left: > either achieve or at least approximate the aforementioned correct > approach by other means, or do something we wouldn't do in the > presence of better tools. I'm always reluctant to do the latter or > encourage it. I won't have a problem with an expression like the following, and I think it would be better readable than the current code in the patch: (compile (format "ebuild %s %s" file command) (unless (featurep 'xemacs) 'ebuild-run-mode)) Alternatively (and probably cleaner), define a variable and assign it at compile time: (defvar ebuild-mode-run-comint (eval-when-compile (unless (featurep 'xemacs) 'ebuild-run-mode)) "Mode to use for compilation buffer in ebuild-run-command.") ;; ... more stuff ... (compile (format "ebuild %s %s" file command) ebuild-mode-run-comint)
Please ignore the last part of my previous comment. I just see it already fails because "compile" in XEmacs doesn't have the optional second argument.
I also wonder, maybe autoload ebuild-run-mode instead of hard requiring it? (That would be done from the site-init file.) There are certainly users who use ebuild-mode only for its font-lock features, but never run ebuild commands from it.
Also, about testing Emacs versions: A test like (version= "23.4.1" emacs-version) is very brittle, because the build number (i.e. the last component) may be different on a user's system for some reason. Then again, (version<= emacs-version "26.3") may break because we cannot completely exclude that there will be a 26.4. Generally, I'd suggest testing for emacs-major-version whereever possible, or directly test for availability of functions and variables using fboundp and boundp, respectively.
Created attachment 674800 [details, diff] New version of the patch Sorry for the delay. Replaced version<= comparisons with <= comparisons w.r.t emacs-major-version. Made ebuild-run-mode autoload. Added the line (autoload 'ebuild-run-mode "ebuild-run-mode" "Major mode for non-interactive buffers spawned by `ebuild-run-command'." t) to my local FILESDIR in ebuild-mode. This time, my test was: (add-to-list 'load-path "/usr/share/emacs/site-lisp/ebuild-mode") (autoload 'ebuild-mode "ebuild-mode" "Major mode for Portage .ebuild and .eclass files." t) (autoload 'ebuild-run-mode "ebuild-run-mode" "Major mode for non-interactive buffers spawned by `ebuild-run-command'." t) (require 'ebuild-mode) (with-current-buffer (find-file "/sudo:portage@localhost:/usr/local/portage/app-emacs/ebuild-mode/ebuild-mode-9999.ebuild") (ebuild-run-command "clean")) Tests for Emacs 23 to 28 (load, compile-goto-error, compile ebuild-mode without new warnings) all yielded the same results. I did not understand whether you're satisfied with ebuild-run-compile-ignoring-mode-in-xemacs now. Regarding Emacs 26, I built it on a glibc Gentoo system. It only failed to build on my uclibc system so there's not much surprise here.
Could we please return to this? You didn't say yes and you didn't say no.
I am sorry, for some reason I had missed comment #20 with the updated patch. A few minor points remain: - Do we need the ebuild-run-compile-ignoring-mode-in-xemacs macro? It is used only once. I'd rather have something like the following in ebuild-run-command: (apply 'compile (format "ebuild %s %s" file command) (unless (featurep 'xemacs) '(ebuild-run-mode))) - "git am" complains about 2 lines with trailing whitespace. - The commit needs a "Signed-off-by" line: https://www.gentoo.org/glep/glep-0076.html#certificate-of-origin
Created attachment 721071 [details, diff] A new version of the patch I give up; here goes xemacs check at runtime. It's repulsive.
(In reply to akater from comment #23) > I give up; here goes xemacs check at runtime. > > It's repulsive. Why? Does it have a noticeable impact on perfomance? I'll gladly reconsider if you can show that it does.
Any runtime check as to which program / which Lisp we're running in, is a bad practice. One should adapt to a target at compile time, not at runtime. As you can see, even function signature is different. A compiler could issue warnings or even errors at compile time, rightfully so, but those would be totally irrelevant, as compiler was fed “alien” code. I have other features to add (to part, actually), like jump to eclass, jump to eclass function, and maybe others so let's just merge this; is case you reconsider later, I'll be happy to refactor all such code. One could introduce a universal macro with `case`-like syntax, not to be exported at runtime.
> to part to port, sorry
Looks like <= with more than 2 arguments isn't portable: $ emacs-23 -batch -q --no-site-file -f batch-byte-compile ebuild-run-mode.el In toplevel form: ebuild-run-mode.el:60:12:Warning: `<=' called with 3 args, but requires 2 Loading cl-macs... ebuild-run-mode.el:60:18:Error: Wrong number of arguments: <=, 3
(In reply to akater from comment #25) > Any runtime check as to which program / which Lisp we're running in, is a > bad practice. As long as such featurep tests are commonly used in GNU Emacs proper, I don't agree with that statement.
Created attachment 721110 [details, diff] A new version of the patch I re-run the test: (add-to-list 'load-path "/usr/share/emacs/site-lisp/ebuild-mode") (autoload 'ebuild-mode "ebuild-mode" "Major mode for Portage .ebuild and .eclass files." t) (autoload 'ebuild-run-mode "ebuild-run-mode" "Major mode for non-interactive buffers spawned by `ebuild-run-command'." t) (require 'ebuild-mode) (require 'ebuild-run-mode) (with-current-buffer (find-file "/sudo:portage@localhost:/usr/local/portage/app-emacs/ebuild-mode/ebuild-mode-9999.ebuild") (ebuild-run-command "clean")) ebuild-mode compiles and loads for Emacs 23 to 27. This time, on Emacs 23, I couldn't even find-file /sudo:portage@localhost:/whatever --- It asks for a password and freezes. But it nevertheless compiles and loads. I don't currently have a Gentoo installation that could emerge =app-editors/emacs-23.4-r20, =app-editors/emacs-24.5-r10 =app-editors/emacs-25.3-r9, =app-editors/emacs-26.3-r4. I checked with earlier revisions (the ebuilds are out of the tree for more than 1 year).
For 50ebuild-mode-gentoo, I suggest (autoload 'ebuild-run-mode "ebuild-run-mode" "Major mode for non-interactive buffers spawned by `ebuild-run-command'." t)
Thank you, committed to branch ebuild-run-mode for now: https://gitweb.gentoo.org/proj/ebuild-mode.git/commit/?h=ebuild-run-mode This will still require some work before it's ready for mainline: - Could tramp be made optional? I think in the typical use case the relevant directories should be accessible, when running "ebuild" as a user. (Or to ask differently, what is your workflow so that you need tramp?) - Some documentation on what the mode does should be added to ebuild-mode.texi. Maybe some usage examples could be included too?
I was also wondering if we shouldn't include this as a separate package, like app-emacs/ebuild-run-mode, which might be easier to maintain for both sides. We'd need some way to activate it in ebuild-run-command, e.g. a variable ebuild-compilation-mode which would be set either to nil or to 'ebuild-run-mode when the package is loaded.
I'm fine with a separate package. I'm not against “ebuild-compilation-mode” either but nevertheless would like to point out that it is a misleading name, as ebuild-run-mode is intended to be useful not just for ebuild .. compile but in other commands too (IIRC it already is). I understand that it inherits from compilation-mode so the name is formally correct but I still find it misleading. The core reason is “compilation-mode” being an unfortunate name in the first place as #'compile is useful for more general shell commands, as ebuild-mode itself exemplifies. OTOH, comint means “interactive mode” so it's not always a good fit either, for ebuild-run-mode in particular. That said, I will accept ebuild-compilation-mode variable. I would try to point out the name-related subtlety in the docstring though (better be in Info node but it does not exist) but it's OK without such notes too. An alternative: Right now ebuild-run-command does not use any non-standard mode and it doesn't look like it's actively being augmented with such, so it could be just (if xemacs .. (compile ebuild-command (when (fboundp 'ebuild-run-mode) 'ebuild-run-mode))) without introducing a variable. It is also natural for “ebuild-run” to be a separate entity from ebuild-mode in the first place as they are applied in totally independent cases. One could use ebuild-mode and never touch ebuild-run-command (though it's a loss for a user) and vice versa (not necessarily a loss).
(In reply to akater from comment #33) > I'm fine with a separate package. Good. It would be best if you'd host your code somewhere (gitlab, github, etc.). > I'm not against “ebuild-compilation-mode” either but nevertheless would like > to point out that it is a misleading name, [...] Do you have a better suggestion? ebuild-run-mode? ebuild-log-buffer-mode? > (if xemacs .. > (compile ebuild-command (when (fboundp 'ebuild-run-mode) > 'ebuild-run-mode))) > > without introducing a variable. I was thinking about a custom variable, so the user would have full control. The default could still be as above. Something along the lines of: (defcustom ebuild-run-mode (if (fboundp 'ebuild-run-mode) 'ebuild-run-mode) "Major mode to use for the log buffer of `ebuild-run-command'. If nil, `compilation-mode' will be used." :type '(choice boolean function) :group 'ebuild)
I used “ebuild-log-buffer-mode” in the pre(?)-release. Here goes the package: https://gitlab.com/akater/emacs-ebuild-run-mode/-/blob/master/ebuild-run-mode.org It starts with the short overview. ebuild-log-buffer-mode is set in the site-lisp configuration file so it works out of the box. Ebuilds are suggested in the last section extras: there's live one and stable one, ebuild-run-mode-20210707 https://gitlab.com/akater/emacs-ebuild-run-mode/-/blob/1d5e28f043b76a059547864313678958f07ee428/ebuild-run-mode.org#L516
The bug has been referenced in the following commit(s): https://gitweb.gentoo.org/proj/ebuild-mode.git/commit/?id=6f9c7ef415e2244cac3126ebdee49f6b0e9503e0 commit 6f9c7ef415e2244cac3126ebdee49f6b0e9503e0 Author: Ulrich Müller <ulm@gentoo.org> AuthorDate: 2021-07-08 16:48:25 +0000 Commit: Ulrich Müller <ulm@gentoo.org> CommitDate: 2021-07-08 16:48:25 +0000 New variable ebuild-log-buffer-mode * ebuild-mode.el (ebuild-log-buffer-mode): New variable. (ebuild-run-command): Use it. Bug: https://bugs.gentoo.org/744370 Signed-off-by: Ulrich Müller <ulm@gentoo.org> ebuild-mode.el | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-)
(In reply to akater from comment #35) > I used “ebuild-log-buffer-mode” in the pre(?)-release. Now also in ebuild-mode: https://gitweb.gentoo.org/proj/ebuild-mode.git/commit/?id=6f9c7ef415e2244cac3126ebdee49f6b0e9503e0 I am currently waiting for some eclass updates that are pending, then I'm going to make a new release of ebuild-mode and add the ebuild-run-mode package. (In reply to akater from comment #25) > Any runtime check as to which program / which Lisp we're running in, is a > bad practice. > > One should adapt to a target at compile time, not at runtime. As you can > see, even function signature is different. A compiler could issue warnings > or even errors at compile time, rightfully so, but those would be totally > irrelevant, as compiler was fed “alien” code. Coming back to this. It turns out that the (featurep 'xemacs) check is optimized away by the byte-compiler, both in GNU Emacs and XEmacs. So effectively there won't be any runtime check, even if we take the easy approach. (If you're interested in the details, check for the compiler-macro property of featurep in GNU Emacs. For XEmacs things are done in a different way, but to the same effect.)
> turns out that the (featurep 'xemacs) check > is optimized away by the byte-compiler I have nothing against such checks, then. Thank you for letting me know!
The bug has been referenced in the following commit(s): https://gitweb.gentoo.org/proj/ebuild-mode.git/commit/?id=95f35fcddec648a4d22f2848c99fe351b15b7a37 commit 95f35fcddec648a4d22f2848c99fe351b15b7a37 Author: Ulrich Müller <ulm@gentoo.org> AuthorDate: 2021-07-08 16:48:25 +0000 Commit: Ulrich Müller <ulm@gentoo.org> CommitDate: 2021-07-12 18:39:11 +0000 New variable ebuild-log-buffer-mode * ebuild-mode.el (ebuild-log-buffer-mode): New variable. (ebuild-run-command): Use it. Bug: https://bugs.gentoo.org/744370 Signed-off-by: Ulrich Müller <ulm@gentoo.org> ChangeLog | 5 +++++ ebuild-mode.el | 19 +++++++++++++------ 2 files changed, 18 insertions(+), 6 deletions(-)
The bug has been closed via the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=61717bbc9b1a16c49b235fb6d3a676d33049bf64 commit 61717bbc9b1a16c49b235fb6d3a676d33049bf64 Author: Ulrich Müller <ulm@gentoo.org> AuthorDate: 2021-07-12 19:35:31 +0000 Commit: Ulrich Müller <ulm@gentoo.org> CommitDate: 2021-07-12 19:36:31 +0000 app-emacs/ebuild-run-mode: Initial import of version 20210707 Closes: https://bugs.gentoo.org/744370 Suggested-by: Dima Akater <nuclearspace@gmail.com> Package-Manager: Portage-3.0.20, Repoman-3.0.3 Signed-off-by: Ulrich Müller <ulm@gentoo.org> app-emacs/ebuild-run-mode/Manifest | 1 + .../ebuild-run-mode-20210707.ebuild | 26 ++++++++++++++++++++++ .../files/50ebuild-run-mode-gentoo.el | 4 ++++ app-emacs/ebuild-run-mode/metadata.xml | 12 ++++++++++ 4 files changed, 43 insertions(+)
