GNU bug report logs - #72045
Emacs graft lookup still fails

Previous Next

Full log

Message #50 received at 72045 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: Suhail Singh <suhailsingh247 <at> gmail.com>
Cc: 72045 <at> debbugs.gnu.org
Subject: Re: [PATCH v2 1/2] gnu: Add system test for Emacs.
Date: Fri, 19 Jul 2024 17:42:52 +0200

Am Freitag, dem 19.07.2024 um 11:23 -0400 schrieb Suhail Singh:
> Liliana Marie Prikler <liliana.prikler <at> gmail.com> writes:
> 
> > +          (test-equal "native-comp-dir"
> > +            (emacs-native-comp-dir
> > +             #$(file-append old-emacs "/bin/emacs"))
> > +            (emacs-native-comp-dir
> > +             #$(file-append new-emacs "/bin/emacs")))
> 
> I like that there is a test that focuses on the native-comp-dir
> directly.  Having only a test that focuses on ABI_VERSION wouldn't
> have been sufficient IMO.
> 
> Minor nitpick: However, there may still be some utility in either
> having an additional test for ABI_VERSION or adding a comment that a
> successful evaluation of the above test also implies that the
> ABI_VERSION matches.
We can test `comp-abi-hash` as well, should be no biggie.

> > +          (test-assert "old emacs has hierarchical layout"
> > +            (file-exists?
> > +             (string-append #$new-emacs "/lib/emacs/"
> > +                            (emacs-effective-version old-emacs-
> > bin)
> > +                            "/native-lisp/"
> > +                            (emacs-native-comp-dir old-emacs-bin)
> > +                            "/preloaded/emacs-lisp/comp.eln")))
> 
> Should that say #$old-emacs instead of #$new-emacs ?
Yes, it should.

> > +          (test-assert "new emacs has hierarchical layout"
> > +            (file-exists?
> > +             (string-append #$new-emacs "/lib/emacs/"
> > +                            (emacs-effective-version new-emacs-
> > bin)
> > +                            "/native-lisp/"
> > +                            (emacs-native-comp-dir new-emacs-bin)
> > +                            "/preloaded/emacs-lisp/comp.eln")))
> 
> Do we need to additionally ensure that the new emacs' "hierarchical
> layout" matches the old emacs' "hierarchical layout" in some way
> (over and above both having them)?
We could do so, but that'd be an expensive test.  In practice, we
assume the same layout and just poke a single file.

> > +(define %test-emacs-native-comp-replacable
> > +  (system-test
> > +   (name "emacs-native-comp")
> > +   (description "Test whether an emacs replacement (if any) is
> > valid.")
> > +   (value (run-native-comp-replacable-test
> > +           (package-without-replacement emacs)
> > +           emacs))))
> 
> Ah!  So that's how it's done.  I am not qualified to review this
> part, but this looks to be in the right spirit.  Hoping this is
> merged soon.™
You can verify this part by running "make check-system TESTS=emacs-
native-comp" and seeing that it fails for this commit but succeeds on
the next.  That's TDD :D

Cheers

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.