[PATCH 1/2] aux-files: comp-integrity: Adjust for newer emacs.

  • Done
  • quality assurance status badge
Details
3 participants
  • Morgan Smith
  • Liliana Marie Prikler
  • Morgan Smith
Owner
unassigned
Submitted by
Morgan Smith
Severity
normal
M
M
Morgan Smith wrote on 28 Apr 19:07 +0200
(address . guix-patches@gnu.org)(name . Morgan Smith)(address . Morgan.J.Smith@outlook.com)
CH3PR84MB3424497FCBC7281536D266D0C5142@CH3PR84MB3424.NAMPRD84.PROD.OUTLOOK.COM
* gnu/packages/aux-files/emacs/comp-integrity.el (expect-help): Update with
new terms for function descriptions. Also return the description instead of
'nil' on failure to aid in debugging.

Change-Id: I63a3644c91dd7817a72ab11ae87ec4fc8fdb2c1b
---

Hello!

Trying to build the latest Emacs from source fails because they changed the
names of this stuff. I was able to successfully build the latest Emacs with this patch.

Thanks,

Morgan

gnu/packages/aux-files/emacs/comp-integrity.el | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)

Toggle diff (29 lines)
diff --git a/gnu/packages/aux-files/emacs/comp-integrity.el b/gnu/packages/aux-files/emacs/comp-integrity.el
index abe7e7c0c9..d969f58622 100644
--- a/gnu/packages/aux-files/emacs/comp-integrity.el
+++ b/gnu/packages/aux-files/emacs/comp-integrity.el
@@ -16,10 +16,16 @@
(let ((desc (substring-no-properties
(with-output-to-string
(help-fns-function-description-header ',fun)))))
- (cond ((string-search "native-compiled" desc) 'native)
- ((string-search "byte-compiled" desc) 'byte)
- ((string-search "built-in" desc) 'built-in)
- (t nil))))))))
+ (cond ((or (string-search "native-compiled" desc) ;; Emacs <= 29
+ (string-search "subr-native-elisp" desc)) ;; Emacs >= 30
+ 'native)
+ ((or (string-search "byte-compiled" desc) ;; Emacs <= 29
+ (string-search "compiled-function" desc)) ;; Emacs >= 30
+ 'byte)
+ ((or (string-search "built-in" desc) ;; Emacs <= 29
+ (string-search "primitive-function" desc)) ;; Emacs >= 30
+ 'built-in)
+ (t desc))))))))
(defmacro expect-native (fun &optional feature)
`(progn (expect-help ,fun native ,feature)))

base-commit: 9f183c3627a006e8fd3bb9708448bc05a6204e6d
--
2.41.0
M
M
Morgan Smith wrote on 28 Apr 19:18 +0200
[PATCH 2/2] teams: Register the comp-integrity.el file to the Emacs team.
(address . 70632@debbugs.gnu.org)(name . Morgan Smith)(address . Morgan.J.Smith@outlook.com)
LV8PR84MB343675692790213B0B374721C5142@LV8PR84MB3436.NAMPRD84.PROD.OUTLOOK.COM
* etc/teams.scm (emacs): Add the
"gnu/packages/aux-files/emacs/comp-integrity.el" file to the scope of the Emacs
team.

Change-Id: I66014b94e73fd87eeb3aceaf5f61f08abc875c44
---
etc/teams.scm | 1 +
1 file changed, 1 insertion(+)

Toggle diff (14 lines)
diff --git a/etc/teams.scm b/etc/teams.scm
index d537e83efc..73f57d7f1e 100755
--- a/etc/teams.scm
+++ b/etc/teams.scm
@@ -273,6 +273,7 @@ (define-team emacs
#:description "The extensible, customizable text editor and its
ecosystem."
#:scope (list "gnu/packages/aux-files/emacs/guix-emacs.el"
+ "gnu/packages/aux-files/emacs/comp-integrity.el"
(make-regexp* "^gnu/packages/emacs(-.+|)\\.scm$")
"gnu/packages/tree-sitter.scm"
"guix/build/emacs-build-system.scm"
--
2.41.0
M
M
Morgan Smith wrote on 28 Apr 19:40 +0200
[PATCH v2] aux-files: comp-integrity: Adjust for newer emacs.
(address . 70632@debbugs.gnu.org)(name . Morgan Smith)(address . Morgan.J.Smith@outlook.com)
CH3PR84MB34246A84BECA062339E1E922C5142@CH3PR84MB3424.NAMPRD84.PROD.OUTLOOK.COM
* gnu/packages/aux-files/emacs/comp-integrity.el (expect-help): Update with
new terms for function descriptions. Also return the description instead of
'nil' on failure to aid in debugging.

Change-Id: I63a3644c91dd7817a72ab11ae87ec4fc8fdb2c1b
---

I apologize. Two hours before I sent the original patch they changed the
descriptions once again. This patch is the same as before except instead of
"compiled-function" we now have "byte-code-function".

gnu/packages/aux-files/emacs/comp-integrity.el | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)

Toggle diff (29 lines)
diff --git a/gnu/packages/aux-files/emacs/comp-integrity.el b/gnu/packages/aux-files/emacs/comp-integrity.el
index abe7e7c0c9..0c11100d22 100644
--- a/gnu/packages/aux-files/emacs/comp-integrity.el
+++ b/gnu/packages/aux-files/emacs/comp-integrity.el
@@ -16,10 +16,16 @@
(let ((desc (substring-no-properties
(with-output-to-string
(help-fns-function-description-header ',fun)))))
- (cond ((string-search "native-compiled" desc) 'native)
- ((string-search "byte-compiled" desc) 'byte)
- ((string-search "built-in" desc) 'built-in)
- (t nil))))))))
+ (cond ((or (string-search "native-compiled" desc) ;; Emacs <= 29
+ (string-search "subr-native-elisp" desc)) ;; Emacs >= 30
+ 'native)
+ ((or (string-search "byte-compiled" desc) ;; Emacs <= 29
+ (string-search "byte-code-function" desc)) ;; Emacs >= 30
+ 'byte)
+ ((or (string-search "built-in" desc) ;; Emacs <= 29
+ (string-search "primitive-function" desc)) ;; Emacs >= 30
+ 'built-in)
+ (t desc))))))))
(defmacro expect-native (fun &optional feature)
`(progn (expect-help ,fun native ,feature)))

base-commit: 9f183c3627a006e8fd3bb9708448bc05a6204e6d
--
2.41.0
L
L
Liliana Marie Prikler wrote on 29 Apr 20:44 +0200
Re: [bug#70632] [PATCH 1/2] aux-files: comp-integrity: Adjust for newer emacs.
71e3b67343fca5dfce8633e9aba6414b74de2832.camel@gmail.com
Hi,

Am Sonntag, dem 28.04.2024 um 13:07 -0400 schrieb Morgan Smith:
Toggle quote (57 lines)
> * gnu/packages/aux-files/emacs/comp-integrity.el (expect-help):
> Update with new terms for function descriptions.  Also return the
> description instead of 'nil' on failure to aid in debugging.
>
> Change-Id: I63a3644c91dd7817a72ab11ae87ec4fc8fdb2c1b
> ---
>
> Hello!
>
> Trying to build the latest Emacs from source fails because they
> changed the names of this stuff.  I was able to successfully build
> the latest Emacs with this patch.
>
> Thanks,
>
> Morgan
>
>  gnu/packages/aux-files/emacs/comp-integrity.el | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/gnu/packages/aux-files/emacs/comp-integrity.el
> b/gnu/packages/aux-files/emacs/comp-integrity.el
> index abe7e7c0c9..d969f58622 100644
> --- a/gnu/packages/aux-files/emacs/comp-integrity.el
> +++ b/gnu/packages/aux-files/emacs/comp-integrity.el
> @@ -16,10 +16,16 @@
>                  (let ((desc (substring-no-properties
>                               (with-output-to-string
>                                 (help-fns-function-description-header
> ',fun)))))
> -                  (cond ((string-search "native-compiled" desc)
> 'native)
> -                        ((string-search "byte-compiled" desc) 'byte)
> -                        ((string-search "built-in" desc) 'built-in)
> -                        (t nil))))))))
> +                  (cond ((or (string-search "native-compiled" desc)
> ;; Emacs <= 29
> +                             (string-search "subr-native-elisp"
> desc)) ;; Emacs >= 30
> +                         'native)
> +                        ((or (string-search "byte-compiled" desc) ;;
> Emacs <= 29
> +                             (string-search "compiled-function"
> desc)) ;; Emacs >= 30
> +                         'byte)
> +                        ((or (string-search "built-in" desc) ;;
> Emacs <= 29
> +                             (string-search "primitive-function"
> desc)) ;; Emacs >= 30
> +                         'built-in)
> +                        (t desc))))))))
>  
>    (defmacro expect-native (fun &optional feature)
>      `(progn (expect-help ,fun native ,feature)))
>
> base-commit: 9f183c3627a006e8fd3bb9708448bc05a6204e6d

the change itself LGTM, but I think it should be accompanied by a
change to Emacs 30 and also we should really try to version it because
it rebuilds Emacs as a whole. The emacs-team branch hasn't been used
for a while and I think there's nothing big there; and neither is there
a need to exercise it if we just add another file and replace the
phase.

Cheers
M
M
Morgan Smith wrote on 29 Apr 22:43 +0200
(name . Liliana Marie Prikler)(address . liliana.prikler@gmail.com)
CH3PR84MB3424CB88BCC92AECD7E838B3C51B2@CH3PR84MB3424.NAMPRD84.PROD.OUTLOOK.COM
Liliana Marie Prikler <liliana.prikler@gmail.com> writes:

Toggle quote (11 lines)
> Hi,
>
> the change itself LGTM, but I think it should be accompanied by a
> change to Emacs 30 and also we should really try to version it because
> it rebuilds Emacs as a whole. The emacs-team branch hasn't been used
> for a while and I think there's nothing big there; and neither is there
> a need to exercise it if we just add another file and replace the
> phase.
>
> Cheers

I think some people might want to build newer Emacs's before the 30
release. Like how people wanted the pgtk and tree-sitter features
early. People might want to try the new GC that's being developed.

I'm not a fan of adding another file so I came up with this solution.
See attached patch.

If we believe that a core-updates merge will occur before Emacs 30 then
I would like to see my original patch applied there.

Thanks,

Morgan
From 0440fff5e554d442a113579fbc1330c05da98f6a Mon Sep 17 00:00:00 2001
Message-ID: <0440fff5e554d442a113579fbc1330c05da98f6a.1714423314.git.Morgan.J.Smith@outlook.com>
From: Morgan Smith <Morgan.J.Smith@outlook.com>
Date: Mon, 29 Apr 2024 15:27:59 -0400
Subject: [PATCH] gnu: emacs-next-minimal: Update to 30.0.50-3.ccb49ac.

* gnu/packages/emacs.scm (emacs-next-minimal): Update to 30.0.50-3.ccb49ac.
(emacs->emacs-next): Adjust 'validate-comp-integrity phase for newer Emacs.

Change-Id: Ib191d6044a4a3b56931f893c71dc998fc748245e
---
gnu/packages/emacs.scm | 34 ++++++++++++++++++++++++++++++----
1 file changed, 30 insertions(+), 4 deletions(-)

Toggle diff (61 lines)
diff --git a/gnu/packages/emacs.scm b/gnu/packages/emacs.scm
index 411bea3ab6..022aac416f 100644
--- a/gnu/packages/emacs.scm
+++ b/gnu/packages/emacs.scm
@@ -553,8 +553,8 @@ (define-public emacs-wide-int
#~(cons "--with-wide-int" #$flags))))))
(define-public emacs-next-minimal
- (let ((commit "170c6557922dad7e6e9bc0d6dadf6c080108fd42")
- (revision "2"))
+ (let ((commit "ccb49acd2afb8cec9cec1afba16e16420b9f9261")
+ (revision "3"))
(package
(inherit emacs-minimal)
(name "emacs-next-minimal")
@@ -567,7 +567,7 @@ (define-public emacs-next-minimal
(commit commit)))
(file-name (git-file-name name version))
(sha256
- (base32 "04carva3b6h9fnlzazrsxsj41hcnjc26kxjij07l159azi40l6sk"))
+ (base32 "1hxwaqjm596yykq42wl28jicd0b8rqcabyb5xp958sirr3yi884b"))
(patches
(search-patches "emacs-next-exec-path.patch"
"emacs-fix-scheme-indent-function.patch"
@@ -585,7 +585,33 @@ (define* (emacs->emacs-next emacs #:optional name
(string-drop (package-name emacs)
(string-length "emacs"))))))
(version version)
- (source source)))
+ (source source)
+ (arguments
+ (substitute-keyword-arguments (package-arguments emacs)
+ ((#:phases phases)
+ #~(modify-phases #$phases
+ (replace 'validate-comp-integrity
+ (lambda* (#:key outputs #:allow-other-keys)
+ #$(cond
+ ((%current-target-system)
+ #~(display "Cannot validate native-comp on cross builds.\n"))
+ ((member (%current-system) '("armhf-linux" "i686-linux"))
+ #~(display "Integrity test is broken on armhf.\n"))
+ (else
+ #~(begin
+ (copy-file #$(local-file
+ (search-auxiliary-file "emacs/comp-integrity.el"))
+ "comp-integrity.el")
+ (substitute* "comp-integrity.el"
+ (("\"native-compiled\"") "\"subr-native-elisp\"")
+ (("\"byte-compiled\"") "\"byte-code-function\"")
+ (("\"built-in\"") "\"primitive-function\""))
+ (invoke
+ (string-append (assoc-ref outputs "out") "/bin/emacs")
+ "--batch"
+ "--load"
+ "comp-integrity.el"
+ "-f" "ert-run-tests-batch-and-exit"))))))))))))
(define-public emacs-next (emacs->emacs-next emacs))
(define-public emacs-next-pgtk (emacs->emacs-next emacs-pgtk))
--
2.41.0
L
L
Liliana Marie Prikler wrote on 29 Apr 23:38 +0200
(name . Morgan Smith)(address . morgan.j.smith@outlook.com)
cb1611ac9414e4d0b0366d8f31bd41a3f300931a.camel@gmail.com
Am Montag, dem 29.04.2024 um 16:43 -0400 schrieb Morgan Smith:
Toggle quote (16 lines)
> Liliana Marie Prikler <liliana.prikler@gmail.com> writes:
>
> > Hi,
> >
> > the change itself LGTM, but I think it should be accompanied by a
> > change to Emacs 30 and also we should really try to version it
> > because it rebuilds Emacs as a whole.  The emacs-team branch hasn't
> > been used for a while and I think there's nothing big there; and
> > neither is there a need to exercise it if we just add another file
> > and replace the phase.
> >
> > Cheers
>
> I think some people might want to build newer Emacs's before the 30
> release.  Like how people wanted the pgtk and tree-sitter features
> early.  People might want to try the new GC that's being developed.
That's fair and we aim to support that case.

Toggle quote (2 lines)
> I'm not a fan of adding another file so I came up with this solution.
> See attached patch.
Hmm, I'm a bit torn on the solution. On one hand, it is a local
solution with just a phase, on the other having the file makes it
easier to just mv it.

Toggle quote (2 lines)
> If we believe that a core-updates merge will occur before Emacs 30
> then I would like to see my original patch applied there.
It'd be only emacs-team, not core-updates, but we could do this
"quickly" either way. But the point behind those is to keep them small
and manageable in a sense, so core-updates is typically not concerned
with leaves or leaf-like stuff.

Cheers
M
M
Morgan Smith wrote on 1 May 18:32 +0200
(name . Liliana Marie Prikler)(address . liliana.prikler@gmail.com)
CH3PR84MB3424627A6255F9DE51EF37B5C5192@CH3PR84MB3424.NAMPRD84.PROD.OUTLOOK.COM
Liliana Marie Prikler <liliana.prikler@gmail.com> writes:

Toggle quote (9 lines)
> Am Montag, dem 29.04.2024 um 16:43 -0400 schrieb Morgan Smith:
>> Liliana Marie Prikler <liliana.prikler@gmail.com> writes:
>>
>> I'm not a fan of adding another file so I came up with this solution.
>> See attached patch.
> Hmm, I'm a bit torn on the solution. On one hand, it is a local
> solution with just a phase, on the other having the file makes it
> easier to just mv it.

I don't understand the trade-offs here. I'm a fan of keeping data as
close to where it's used as possible (so ideally in emacs.scm). I'm not
sure what advantages putting it in a file gives over this solution.

Just to be clear: what do you mean by adding another file? I assume you
mean adding a comp-integrity-next.el file which is almost identical to
comp-integrity.el with these small changes in place.

Toggle quote (7 lines)
>> If we believe that a core-updates merge will occur before Emacs 30
>> then I would like to see my original patch applied there.
> It'd be only emacs-team, not core-updates, but we could do this
> "quickly" either way. But the point behind those is to keep them small
> and manageable in a sense, so core-updates is typically not concerned
> with leaves or leaf-like stuff.

I don't think I understand how our branch development stuff works. I
thought we put large dependency changes on core-updates so that the CI
could chew through everything and make sure it's good before merging.

Regardless, I trust the team to do the proper procedures. I simply
believe we might do more package fiddling before Emacs 30 and that
potential problems might be assuaged if the comp-integrity file was more
forgiving and supported every Emacs equally. Thus, I would encourage it
to be applied in the appropriate place now to avoid potential headache
but if we wish to wait for Emacs 30 that would also be a valid choice.
L
L
Liliana Marie Prikler wrote on 1 May 18:46 +0200
(name . Morgan Smith)(address . morgan.j.smith@outlook.com)
632d44c7dda06b0441f8c0ba88f47ea745c7f202.camel@gmail.com
Am Mittwoch, dem 01.05.2024 um 12:32 -0400 schrieb Morgan Smith:
Toggle quote (15 lines)
> Liliana Marie Prikler <liliana.prikler@gmail.com> writes:
>
> > Am Montag, dem 29.04.2024 um 16:43 -0400 schrieb Morgan Smith:
> > > Liliana Marie Prikler <liliana.prikler@gmail.com> writes:
> > >
> > > I'm not a fan of adding another file so I came up with this
> > > solution. See attached patch.
> > Hmm, I'm a bit torn on the solution.  On one hand, it is a local
> > solution with just a phase, on the other having the file makes it
> > easier to just mv it.
>
> I don't understand the trade-offs here.  I'm a fan of keeping data as
> close to where it's used as possible (so ideally in emacs.scm).  I'm
> not sure what advantages putting it in a file gives over this
> solution.
The advantage lies in only rebuilding Emacs 30.

Toggle quote (3 lines)
> Just to be clear: what do you mean by adding another file?  I assume
> you mean adding a comp-integrity-next.el file which is almost
> identical to comp-integrity.el with these small changes in place.
Yes.

Toggle quote (11 lines)
> > > If we believe that a core-updates merge will occur before Emacs
> > > 30 then I would like to see my original patch applied there.
> > It'd be only emacs-team, not core-updates, but we could do this
> > "quickly" either way.  But the point behind those is to keep them
> > small and manageable in a sense, so core-updates is typically not
> > concerned with leaves or leaf-like stuff.
>
> I don't think I understand how our branch development stuff works.  I
> thought we put large dependency changes on core-updates so that the
> CI could chew through everything and make sure it's good before
> merging.
Most stuff is now organized within teams that have "smaller"
responsibilities. I'm responsible for getting Emacs and Gnome updates
way later than we could…

Toggle quote (7 lines)
> Regardless, I trust the team to do the proper procedures.  I simply
> believe we might do more package fiddling before Emacs 30 and that
> potential problems might be assuaged if the comp-integrity file was
> more forgiving and supported every Emacs equally.  Thus, I would
> encourage it to be applied in the appropriate place now to avoid
> potential headache but if we wish to wait for Emacs 30 that would
> also be a valid choice.
There are tradeoffs to be made here. In principle, we could support
"every Emacs ever", in practice, it hardly makes sense. If you need an
old Emacs in the future, you might as well use the built-in time
machine.

The right place is a new comp-integrity.el. We can just mv it over the
old one once Emacs 30 is the default Emacs. We don't yet know how the
help changes for 31, so we can't really ask a crystal ball to insert
the right check.

Cheers
M
M
Morgan Smith wrote on 1 May 22:06 +0200
(name . Liliana Marie Prikler)(address . liliana.prikler@gmail.com)
CH3PR84MB34248523A04EC53481BB9740C5192@CH3PR84MB3424.NAMPRD84.PROD.OUTLOOK.COM
Liliana Marie Prikler <liliana.prikler@gmail.com> writes:

Toggle quote (18 lines)
> Am Mittwoch, dem 01.05.2024 um 12:32 -0400 schrieb Morgan Smith:
>> Liliana Marie Prikler <liliana.prikler@gmail.com> writes:
>>
>> > Am Montag, dem 29.04.2024 um 16:43 -0400 schrieb Morgan Smith:
>> > > Liliana Marie Prikler <liliana.prikler@gmail.com> writes:
>> > >
>> > > I'm not a fan of adding another file so I came up with this
>> > > solution. See attached patch.
>> > Hmm, I'm a bit torn on the solution.  On one hand, it is a local
>> > solution with just a phase, on the other having the file makes it
>> > easier to just mv it.
>>
>> I don't understand the trade-offs here.  I'm a fan of keeping data as
>> close to where it's used as possible (so ideally in emacs.scm).  I'm
>> not sure what advantages putting it in a file gives over this
>> solution.
> The advantage lies in only rebuilding Emacs 30.
>
I still feel like I'm conceptually missing something here. Emacs 30
doesn't actually exist, right? We are currently on Emacs 29.
emacs-next is the closest thing we have to Emacs 30. Regardless of if
we create a new file or use my phase I sent, we will only be rebuilding
the emacs-next stuff. The current emacs (29) is being left alone.

Toggle quote (17 lines)
>> Regardless, I trust the team to do the proper procedures.  I simply
>> believe we might do more package fiddling before Emacs 30 and that
>> potential problems might be assuaged if the comp-integrity file was
>> more forgiving and supported every Emacs equally.  Thus, I would
>> encourage it to be applied in the appropriate place now to avoid
>> potential headache but if we wish to wait for Emacs 30 that would
>> also be a valid choice.
> There are tradeoffs to be made here. In principle, we could support
> "every Emacs ever", in practice, it hardly makes sense. If you need an
> old Emacs in the future, you might as well use the built-in time
> machine.
>
> The right place is a new comp-integrity.el. We can just mv it over the
> old one once Emacs 30 is the default Emacs. We don't yet know how the
> help changes for 31, so we can't really ask a crystal ball to insert
> the right check.

Ok I think I now sort of see your point. You don't want a build up of
legacy support code in our files. I do understand and support that and
will send a patch of that nature if you would like. However, I do think
at least supporting all of the current Emacs packages in guix is a nice
thing to do.

In guix/build/emacs-utils.scm:emacs-generate-autoloads, there is a
condition to support emacs 28. I don't think we ever use that path
anymore but it is nice to have a robust function that "just works".
Espiaclly back when we did have emacs 28 and 29 packages in guix.

It is my personal opinion, that we should have the file support Emacs 29
and 30 for simplicity sake. But again, if you disagree with me (which
is valid), I'll send you a patch creating a new file.
L
L
Liliana Marie Prikler wrote on 2 May 06:24 +0200
(name . Morgan Smith)(address . morgan.j.smith@outlook.com)
518c80a73207138ede449fb24c72ac4b79764b1e.camel@gmail.com
Am Mittwoch, dem 01.05.2024 um 16:06 -0400 schrieb Morgan Smith:
Toggle quote (19 lines)
>
> I still feel like I'm conceptually missing something here.  Emacs 30
> doesn't actually exist, right?  We are currently on Emacs 29.
> emacs-next is the closest thing we have to Emacs 30.  Regardless of
> if we create a new file or use my phase I sent, we will only be
> rebuilding the emacs-next stuff.  The current emacs (29) is being
> left alone.
>
> >
> [...]
> Ok I think I now sort of see your point.  You don't want a build up
> of legacy support code in our files.  I do understand and support
> that and will send a patch of that nature if you would like. 
> However, I do think at least supporting all of the current Emacs
> packages in guix is a nice thing to do.
>
> It is my personal opinion, that we should have the file support Emacs
> 29 and 30 for simplicity sake. But again, if you disagree with me
> (which is valid), I'll send you a patch creating a new file.
TL;DR: If we do a new file or a phase, we only rebuild emacs-next. If
we modify the file in-place, we rebuild emacs, because it uses it.
Between a phase and a new file, a new file is preferable, because we
can then 'mv' it over the old one.

Toggle quote (4 lines)
> In guix/build/emacs-utils.scm:emacs-generate-autoloads, there is a
> condition to support emacs 28.  I don't think we ever use that path
> anymore but it is nice to have a robust function that "just works".
> Espiaclly back when we did have emacs 28 and 29 packages in guix.
This is somewhat legacy code that has grown that way back when Emacs 29
was emacs-next. There was no good reason to drop it with the switch,
but come Emacs 30, 31, and maintainability might be one.

Cheers
M
M
Morgan Smith wrote on 8 May 20:48 +0200
(name . Liliana Marie Prikler)(address . liliana.prikler@gmail.com)
CH3PR84MB3424B66223D97660AD97AFDAC5E52@CH3PR84MB3424.NAMPRD84.PROD.OUTLOOK.COM
Liliana Marie Prikler <liliana.prikler@gmail.com> writes:

Toggle quote (6 lines)
> Am Mittwoch, dem 01.05.2024 um 16:06 -0400 schrieb Morgan Smith:
> TL;DR: If we do a new file or a phase, we only rebuild emacs-next. If
> we modify the file in-place, we rebuild emacs, because it uses it.
> Between a phase and a new file, a new file is preferable, because we
> can then 'mv' it over the old one.

I apologize. In my mind we where talking about two different changes at
the same time. A change to emacs-next right now, and a change that
would rebuild all of emacs but live on a branch for now.

I still think that's the optimal route. Apply the phase patch right now
to main, and then the file modification to some branch.

On a related note, I do have two changes I'd like to make when we
rebuild all of emacs. I've attached the changes to this mail despite it
not being quite the right place to send them. Also I didn't actually
test them because I don't really want to rebuild all of that myself.

Toggle quote (8 lines)
>> In guix/build/emacs-utils.scm:emacs-generate-autoloads, there is a
>> condition to support emacs 28.  I don't think we ever use that path
>> anymore but it is nice to have a robust function that "just works".
>> Espiaclly back when we did have emacs 28 and 29 packages in guix.
> This is somewhat legacy code that has grown that way back when Emacs 29
> was emacs-next. There was no good reason to drop it with the switch,
> but come Emacs 30, 31, and maintainability might be one.

My issue isn't that you're wrong, but rather I would argue "it's not a
big deal". To maintain clean minimal code we should remove that
condition and so I see your point. But also, it is such a small
addition of code that adds significant backwards compatibility to the
function.
Attachment: 0001-build-system-emacs-Allow-usage-of-allowed-disallowed.patch
From 32ba147bbccaa21524487f2fa216cf1f4a18d884 Mon Sep 17 00:00:00 2001
Message-ID: <32ba147bbccaa21524487f2fa216cf1f4a18d884.1715193630.git.Morgan.J.Smith@outlook.com>
In-Reply-To: <bfb0492b28dddd69bc894de278b5ccbd0d12bd33.1715193630.git.Morgan.J.Smith@outlook.com>
References: <bfb0492b28dddd69bc894de278b5ccbd0d12bd33.1715193630.git.Morgan.J.Smith@outlook.com>
From: Morgan Smith <Morgan.J.Smith@outlook.com>
Date: Wed, 8 May 2024 14:40:00 -0400
Subject: [PATCH 2/2] guix: emacs-utils: Be more verbose in build phase.

* guix/build/emacs-utils.scm (emacs-compile-directory): Display the filename
of each file before compiling.

Change-Id: I275a086ed92f7cfb2907aea9b4feb59012cc8dd5
---
guix/build/emacs-utils.scm | 1 +
1 file changed, 1 insertion(+)

Toggle diff (14 lines)
diff --git a/guix/build/emacs-utils.scm b/guix/build/emacs-utils.scm
index aeb364133a..f35d9e690f 100644
--- a/guix/build/emacs-utils.scm
+++ b/guix/build/emacs-utils.scm
@@ -146,6 +146,7 @@ (define* (emacs-compile-directory dir)
'comp--write-bytecode-file)))))
(mapc
(lambda (file)
+ (message "Compiling `%s'")
(let (byte-to-native-output-buffer-file
;; First entry is the eln-cache of the homeless shelter,
;; second entry is the install directory.
--
2.41.0
L
L
Liliana Marie Prikler wrote on 7 Jul 08:36 +0200
(name . Morgan Smith)(address . morgan.j.smith@outlook.com)
7c27d998eb2d6ad50c4ff47d77305840ed9dc9d8.camel@gmail.com
Am Mittwoch, dem 08.05.2024 um 14:48 -0400 schrieb Morgan Smith:
Toggle quote (14 lines)
> Liliana Marie Prikler <liliana.prikler@gmail.com> writes:
>
> > Am Mittwoch, dem 01.05.2024 um 16:06 -0400 schrieb Morgan Smith:
> > TL;DR: If we do a new file or a phase, we only rebuild emacs-next. 
> > If we modify the file in-place, we rebuild emacs, because it uses
> > it. Between a phase and a new file, a new file is preferable,
> > because we can then 'mv' it over the old one.
>
> I apologize.  In my mind we where talking about two different changes
> at the same time.  A change to emacs-next right now, and a change
> that would rebuild all of emacs but live on a branch for now.
>
> I still think that's the optimal route.  Apply the phase patch right
> now to main, and then the file modification to some branch.
I've recently pushed an update to master that has a new integrity check
for emacs-next; using emacs-next intrinsics rather than going through
help-fns.

Thus marking this as done.

Toggle quote (5 lines)
> On a related note, I do have two changes I'd like to make when we
> rebuild all of emacs.  I've attached the changes to this mail despite
> it not being quite the right place to send them.  Also I didn't
> actually test them because I don't really want to rebuild all of that
> myself.
Sorry for the late review on those, but you might want to revisit them
and send them as a new patch series.

Toggle quote (1 lines)
> + (message "Compiling `%s'")
That doesn't look right.

Toggle quote (8 lines)
> [PATCH 1/2] build-system/emacs: Allow usage of
> #:{allowed/disallowed}-references key
>
> * guix/build-system/emacs.scm (emacs-build): Pass #:allowed-
> references and
>
> Change-Id: Ib9a35a7b2115365b96675fb7ca3914b0ae7e67c7
> #:disallowed-references keys to builder.
The references are actually being ignored here.

Cheers
Closed
?
Your comment

This issue is archived.

To comment on this conversation send an email to 70632@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 70632
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch