diff mbox series

[bug#70632,1/2] aux-files: comp-integrity: Adjust for newer emacs.

Message ID CH3PR84MB3424CB88BCC92AECD7E838B3C51B2@CH3PR84MB3424.NAMPRD84.PROD.OUTLOOK.COM
State New
Headers show
Series [bug#70632,1/2] aux-files: comp-integrity: Adjust for newer emacs. | expand

Commit Message

Morgan Smith April 29, 2024, 8:43 p.m. UTC
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.

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

Comments

Liliana Marie Prikler April 29, 2024, 9:38 p.m. UTC | #1
Am Montag, dem 29.04.2024 um 16:43 -0400 schrieb Morgan Smith:
> 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.

> 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.

> 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
Morgan Smith May 1, 2024, 4:32 p.m. UTC | #2
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.

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.

>> 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.
Liliana Marie Prikler May 1, 2024, 4:46 p.m. UTC | #3
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.

> 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.

> > > 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…

> 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
Morgan Smith May 1, 2024, 8:06 p.m. UTC | #4
Liliana Marie Prikler <liliana.prikler@gmail.com> writes:

> 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.

>> 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.
Liliana Marie Prikler May 2, 2024, 4:24 a.m. UTC | #5
Am Mittwoch, dem 01.05.2024 um 16:06 -0400 schrieb Morgan Smith:
> 
> 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.

> 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
diff mbox series

Patch

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(-)

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