GNU bug report logs - #44249
[PATCH] gnu: emacs: Make strip-double-wrap more robust

Previous Next

Package: guix-patches;

Reported by: Morgan.J.Smith <at> outlook.com

Date: Tue, 27 Oct 2020 02:18:01 UTC

Severity: normal

Tags: patch

Done: Ludovic Courtès <ludo <at> gnu.org>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 44249 in the body.
You can then email your comments to 44249 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to guix-patches <at> gnu.org:
bug#44249; Package guix-patches. (Tue, 27 Oct 2020 02:18:01 GMT) Full text and rfc822 format available.

Acknowledgement sent to Morgan.J.Smith <at> outlook.com:
New bug report received and forwarded. Copy sent to guix-patches <at> gnu.org. (Tue, 27 Oct 2020 02:18:01 GMT) Full text and rfc822 format available.

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

From: Morgan.J.Smith <at> outlook.com
To: guix-patches <at> gnu.org
Cc: Morgan Smith <Morgan.J.Smith <at> outlook.com>
Subject: [PATCH] gnu: emacs: Make strip-double-wrap more robust
Date: Mon, 26 Oct 2020 22:01:45 -0400
From: Morgan Smith <Morgan.J.Smith <at> outlook.com>

* gnu/packages/emacs.scm (emacs) [strip-double-wrap]:
Use regex to find emacs executable. This works even when the version is
changed by package transformations (ex: version=git.master)
---
 gnu/packages/emacs.scm | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/gnu/packages/emacs.scm b/gnu/packages/emacs.scm
index 4963379d74..5c89e4c6b6 100644
--- a/gnu/packages/emacs.scm
+++ b/gnu/packages/emacs.scm
@@ -196,17 +196,13 @@
            (lambda* (#:key outputs #:allow-other-keys)
              ;; Directly copy emacs-X.Y to emacs, so that it is not wrapped
              ;; twice.  This also fixes a minor issue, where WMs would not be
-             ;; able to track emacs back to emacs.desktop.  The version is
-             ;; accessed using using THIS-PACKAGE so it "just works" for
-             ;; inherited Emacs packages of different versions.
+             ;; able to track emacs back to emacs.desktop.
              (with-directory-excursion (assoc-ref outputs "out")
-               (copy-file (string-append
-                           "bin/emacs-"
-                           ,(let ((this-version (package-version this-package)))
-                              (or (false-if-exception
-                                   (version-major+minor+point this-version))
-                                  (version-major+minor this-version))))
-                          "bin/emacs")
+               (copy-file
+                (car
+                 (find-files
+                  "bin" (file-name-predicate "^emacs-([0-9]+\\.)+[0-9]+$")))
+                "bin/emacs")
                #t)))
          (add-before 'reset-gzip-timestamps 'make-compressed-files-writable
            ;; The 'reset-gzip-timestamps phase will throw a permission error
-- 
2.29.1





Information forwarded to guix-patches <at> gnu.org:
bug#44249; Package guix-patches. (Tue, 27 Oct 2020 21:14:02 GMT) Full text and rfc822 format available.

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

From: Nicolas Goaziou <mail <at> nicolasgoaziou.fr>
To: Morgan.J.Smith <at> outlook.com
Cc: 44249 <at> debbugs.gnu.org
Subject: Re: [bug#44249] [PATCH] gnu: emacs: Make strip-double-wrap more robust
Date: Tue, 27 Oct 2020 22:13:29 +0100
Hello,

Morgan.J.Smith <at> outlook.com writes:

> * gnu/packages/emacs.scm (emacs) [strip-double-wrap]:
> Use regex to find emacs executable. This works even when the version is
> changed by package transformations (ex: version=git.master)

Thank you.

> +               (copy-file
> +                (car

Please use pattern matching, i.e. `match', instead of `car'.

> +                 (find-files
> +                  "bin" (file-name-predicate "^emacs-([0-9]+\\.)+[0-9]+$")))
> +                "bin/emacs")

Would it be even more robust to simply catch any "emacs-" prefixed file
name?

Regards,
-- 
Nicolas Goaziou




Information forwarded to guix-patches <at> gnu.org:
bug#44249; Package guix-patches. (Mon, 02 Nov 2020 04:36:02 GMT) Full text and rfc822 format available.

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

From: Morgan.J.Smith <at> outlook.com
To: mail <at> nicolasgoaziou.fr
Cc: 44249 <at> debbugs.gnu.org, Morgan Smith <Morgan.J.Smith <at> outlook.com>
Subject: [PATCH v2] gnu: emacs: Make strip-double-wrap more robust
Date: Sun,  1 Nov 2020 23:35:04 -0500
From: Morgan Smith <Morgan.J.Smith <at> outlook.com>

* gnu/packages/emacs.scm (emacs) [strip-double-wrap]:
Use regex to find emacs executable. This works even when the version is
changed by package transformations (ex: version=git.master)
---
 gnu/packages/emacs.scm | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/gnu/packages/emacs.scm b/gnu/packages/emacs.scm
index 4963379d74..00441dee45 100644
--- a/gnu/packages/emacs.scm
+++ b/gnu/packages/emacs.scm
@@ -123,6 +123,9 @@
     (build-system glib-or-gtk-build-system)
     (arguments
      `(#:tests? #f                      ; no check target
+       #:modules ((guix build glib-or-gtk-build-system)
+                  (guix build utils)
+                  (ice-9 match))
        #:configure-flags (list "--with-modules"
                                "--with-cairo"
                                "--disable-build-details")
@@ -196,17 +199,13 @@
            (lambda* (#:key outputs #:allow-other-keys)
              ;; Directly copy emacs-X.Y to emacs, so that it is not wrapped
              ;; twice.  This also fixes a minor issue, where WMs would not be
-             ;; able to track emacs back to emacs.desktop.  The version is
-             ;; accessed using using THIS-PACKAGE so it "just works" for
-             ;; inherited Emacs packages of different versions.
+             ;; able to track emacs back to emacs.desktop.
              (with-directory-excursion (assoc-ref outputs "out")
-               (copy-file (string-append
-                           "bin/emacs-"
-                           ,(let ((this-version (package-version this-package)))
-                              (or (false-if-exception
-                                   (version-major+minor+point this-version))
-                                  (version-major+minor this-version))))
-                          "bin/emacs")
+               (copy-file
+                (match
+                    (find-files "bin" (file-name-predicate "^emacs-"))
+                  (((? string? string)) string))
+                "bin/emacs")
                #t)))
          (add-before 'reset-gzip-timestamps 'make-compressed-files-writable
            ;; The 'reset-gzip-timestamps phase will throw a permission error
-- 
2.29.1





Reply sent to Nicolas Goaziou <mail <at> nicolasgoaziou.fr>:
You have taken responsibility. (Tue, 03 Nov 2020 09:46:01 GMT) Full text and rfc822 format available.

Notification sent to Morgan.J.Smith <at> outlook.com:
bug acknowledged by developer. (Tue, 03 Nov 2020 09:46:02 GMT) Full text and rfc822 format available.

Message #16 received at 44249-done <at> debbugs.gnu.org (full text, mbox):

From: Nicolas Goaziou <mail <at> nicolasgoaziou.fr>
To: Morgan.J.Smith <at> outlook.com
Cc: 44249-done <at> debbugs.gnu.org
Subject: Re: [PATCH v2] gnu: emacs: Make strip-double-wrap more robust
Date: Tue, 03 Nov 2020 10:45:30 +0100
Hello,

Morgan.J.Smith <at> outlook.com writes:

> * gnu/packages/emacs.scm (emacs) [strip-double-wrap]:
> Use regex to find emacs executable. This works even when the version is
> changed by package transformations (ex: version=git.master)

I added missing final full stops in the commit message, and tweaked your
patter a bit. In particular, I removed the call to `string?', since
I don't think `find-files' can return a non-empty list with anything not
being a string.Let me know if you think I'm wrong.

Patch applied. Thank you.

Regards,
-- 
Nicolas Goaziou




Information forwarded to guix-patches <at> gnu.org:
bug#44249; Package guix-patches. (Tue, 03 Nov 2020 12:49:02 GMT) Full text and rfc822 format available.

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

From: Nicolas Goaziou <mail <at> nicolasgoaziou.fr>
To: 44249 <at> debbugs.gnu.org
Cc: Morgan.J.Smith <at> outlook.com
Subject: Re: bug#44249: [PATCH v2] gnu: emacs: Make strip-double-wrap more
 robust
Date: Tue, 03 Nov 2020 13:48:04 +0100
Nicolas Goaziou <mail <at> nicolasgoaziou.fr> writes:

> Patch applied. Thank you.

And patch reverted… It generates a build error: "No code
for module (guix build glib-or-gtk-build-system)".

What is the purpose of loading (guix build glib-or-gtk-build-system)?

Regards,




Information forwarded to guix-patches <at> gnu.org:
bug#44249; Package guix-patches. (Tue, 03 Nov 2020 14:50:01 GMT) Full text and rfc822 format available.

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

From: Morgan Smith <Morgan.J.Smith <at> outlook.com>
To: Nicolas Goaziou <mail <at> nicolasgoaziou.fr>, 44249 <at> debbugs.gnu.org
Subject: Re: bug#44249: [PATCH v2] gnu: emacs: Make strip-double-wrap more
 robust
Date: Tue, 3 Nov 2020 09:49:30 -0500
So I need to use the module (ice-9 match) there to get the definition of
match. However, it seems to override the modules that where previously
available there so I have to add them back.

Can you confirm how you create the error? I did a checkout to the commit
before you did the revert (51482b93b6) and I couldn't find any errors.
This is what I did:

guix environment guix -C --pure -- make distclean
git clean -xfd
guix environment guix -C --pure -- ./bootstrap
guix environment guix -C --pure -- ./configure --localstatedir=/var
guix environment guix -C --pure -- make
./pre-inst-env guix build emacs
./pre-inst-env guix build emacs-next


On 11/3/20 7:48 AM, Nicolas Goaziou wrote:
> Nicolas Goaziou <mail <at> nicolasgoaziou.fr> writes:
> 
>> Patch applied. Thank you.
> 
> And patch reverted… It generates a build error: "No code
> for module (guix build glib-or-gtk-build-system)".
> 
> What is the purpose of loading (guix build glib-or-gtk-build-system)?
> 
> Regards,
> 




Information forwarded to guix-patches <at> gnu.org:
bug#44249; Package guix-patches. (Tue, 03 Nov 2020 21:40:01 GMT) Full text and rfc822 format available.

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

From: Nicolas Goaziou <mail <at> nicolasgoaziou.fr>
To: Morgan Smith <Morgan.J.Smith <at> outlook.com>
Cc: 44249 <at> debbugs.gnu.org
Subject: Re: bug#44249: [PATCH v2] gnu: emacs: Make strip-double-wrap more
 robust
Date: Tue, 03 Nov 2020 22:38:58 +0100
Morgan Smith <Morgan.J.Smith <at> outlook.com> writes:

> So I need to use the module (ice-9 match) there to get the definition of
> match. However, it seems to override the modules that where previously
> available there so I have to add them back.

Ah. True.

> Can you confirm how you create the error?

I cannot. I tested your patch before applying it, and could compile
Emacs just fine. However, as Ludovic reported it on IRC this commit had
introduced issues in `emacs-minimal' package, hence the revert. See, if
I understand Guix Data correctly,

http://data.guix.gnu.org/revision/b107a19ffb6a6abb7bde3436f3fa359071bd1f5c/package/emacs-minimal/27.1

Regards,




Information forwarded to guix-patches <at> gnu.org:
bug#44249; Package guix-patches. (Tue, 03 Nov 2020 22:11:03 GMT) Full text and rfc822 format available.

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

From: zimoun <zimon.toutoune <at> gmail.com>
To: Nicolas Goaziou <mail <at> nicolasgoaziou.fr>, Morgan Smith
 <Morgan.J.Smith <at> outlook.com>
Cc: 44249 <at> debbugs.gnu.org
Subject: Re: [bug#44249] [PATCH v2] gnu: emacs: Make strip-double-wrap more
 robust
Date: Tue, 03 Nov 2020 23:09:56 +0100
Dear,

On Tue, 03 Nov 2020 at 22:38, Nicolas Goaziou <mail <at> nicolasgoaziou.fr> wrote:

>> Can you confirm how you create the error?
>
> I cannot. I tested your patch before applying it, and could compile
> Emacs just fine. However, as Ludovic reported it on IRC this commit had
> introduced issues in `emacs-minimal' package, hence the revert. See, if
> I understand Guix Data correctly,
>
> http://data.guix.gnu.org/revision/b107a19ffb6a6abb7bde3436f3fa359071bd1f5c/package/emacs-minimal/27.1

Another entry point is:

https://data.guix.gnu.org/repository/1/branch/master/package/emacs-minimal/output-history

Then click on “2020-11-03 09:43:20“ which is the (commit) date of the
first failing commit and you get the revision
b107a19ffb6a6abb7bde3436f3fa359071bd1f5c

https://data.guix.gnu.org/revision/b107a19ffb6a6abb7bde3436f3fa359071bd1f5c

then click on “(View cgit)” leads to:

https://git.savannah.gnu.org/cgit/guix.git/commit/?id=b107a19ffb6a6abb7bde3436f3fa359071bd1f5c

QED. :-)


Hope that helps,
simon

PS:
The attentive reader notice the difference of hours:
“2020-11-03 09:43:20“
    vs
2020-11-03 10:30:03 +0100 
Hum?!




Information forwarded to guix-patches <at> gnu.org:
bug#44249; Package guix-patches. (Wed, 04 Nov 2020 19:51:01 GMT) Full text and rfc822 format available.

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

From: Morgan.J.Smith <at> outlook.com
To: mail <at> nicolasgoaziou.fr
Cc: 44249 <at> debbugs.gnu.org, Morgan Smith <Morgan.J.Smith <at> outlook.com>
Subject: [PATCH v3] gnu: emacs: Make strip-double-wrap more robust.
Date: Wed,  4 Nov 2020 14:47:13 -0500
From: Morgan Smith <Morgan.J.Smith <at> outlook.com>

* gnu/packages/emacs.scm (emacs) [strip-double-wrap]: Use regex to find emacs
executable.  This works even when the version is changed by package
transformations (e.g., version=git.master).
---

(Can you reopen this bug report please?)

So I see 3 possible solutions:
1. Accept my first patch and give up on match
2. Accept this patch and modify almost every emacs varient (I did test building them all)
3. Figure out some proper module inheritence

I think option 3 is the most correct, but I'm lazy so I'm leaning towards option 1.
---
 gnu/packages/emacs.scm | 42 ++++++++++++++++++++++++++++--------------
 1 file changed, 28 insertions(+), 14 deletions(-)

diff --git a/gnu/packages/emacs.scm b/gnu/packages/emacs.scm
index 4963379d74..4d1080f9dd 100644
--- a/gnu/packages/emacs.scm
+++ b/gnu/packages/emacs.scm
@@ -123,6 +123,9 @@
     (build-system glib-or-gtk-build-system)
     (arguments
      `(#:tests? #f                      ; no check target
+       #:modules ((guix build glib-or-gtk-build-system)
+                  (guix build utils)
+                  (ice-9 match))
        #:configure-flags (list "--with-modules"
                                "--with-cairo"
                                "--disable-build-details")
@@ -196,17 +199,12 @@
            (lambda* (#:key outputs #:allow-other-keys)
              ;; Directly copy emacs-X.Y to emacs, so that it is not wrapped
              ;; twice.  This also fixes a minor issue, where WMs would not be
-             ;; able to track emacs back to emacs.desktop.  The version is
-             ;; accessed using using THIS-PACKAGE so it "just works" for
-             ;; inherited Emacs packages of different versions.
+             ;; able to track emacs back to emacs.desktop.
              (with-directory-excursion (assoc-ref outputs "out")
-               (copy-file (string-append
-                           "bin/emacs-"
-                           ,(let ((this-version (package-version this-package)))
-                              (or (false-if-exception
-                                   (version-major+minor+point this-version))
-                                  (version-major+minor this-version))))
-                          "bin/emacs")
+               (copy-file
+                (match (find-files "bin" "^emacs-")
+                  ((executable . _) executable))
+                "bin/emacs")
                #t)))
          (add-before 'reset-gzip-timestamps 'make-compressed-files-writable
            ;; The 'reset-gzip-timestamps phase will throw a permission error
@@ -328,7 +326,11 @@ languages.")
        ((#:phases phases)
         `(modify-phases ,phases
            (delete 'restore-emacs-pdmp)
-           (delete 'strip-double-wrap)))))
+           (delete 'strip-double-wrap)))
+       ((#:modules modules)
+        `((guix build gnu-build-system)
+          (guix build utils)
+          (ice-9 match)))))
     (inputs
      `(("guix-emacs.el" ,(search-auxiliary-file "emacs/guix-emacs.el"))
        ("ncurses" ,ncurses)))
@@ -348,7 +350,11 @@ editor (with xwidgets support)")
        ((#:phases phases)
         `(modify-phases ,phases
            (delete 'restore-emacs-pdmp)
-           (delete 'strip-double-wrap)))))
+           (delete 'strip-double-wrap)))
+       ((#:modules modules)
+        `((guix build gnu-build-system)
+          (guix build utils)
+          (ice-9 match)))))
     (inputs
      `(("webkitgtk" ,webkitgtk)
        ("libxcomposite" ,libxcomposite)
@@ -375,7 +381,11 @@ editor (console only)")
        ((#:phases phases)
         `(modify-phases ,phases
            (delete 'restore-emacs-pdmp)
-           (delete 'strip-double-wrap)))))))
+           (delete 'strip-double-wrap)))
+       ((#:modules modules)
+        `((guix build gnu-build-system)
+          (guix build utils)
+          (ice-9 match)))))))
 
 (define-public emacs-no-x-toolkit
   (package/inherit emacs
@@ -392,7 +402,11 @@ editor (without an X toolkit)" )
        ((#:phases phases)
         `(modify-phases ,phases
            (delete 'restore-emacs-pdmp)
-           (delete 'strip-double-wrap)))))))
+           (delete 'strip-double-wrap)))
+       ((#:modules modules)
+        `((guix build gnu-build-system)
+          (guix build utils)
+          (ice-9 match)))))))
 
 (define-public emacs-wide-int
   (package/inherit emacs
-- 
2.29.1





Information forwarded to guix-patches <at> gnu.org:
bug#44249; Package guix-patches. (Thu, 05 Nov 2020 22:34:01 GMT) Full text and rfc822 format available.

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

From: Nicolas Goaziou <mail <at> nicolasgoaziou.fr>
To: Morgan.J.Smith <at> outlook.com
Cc: 44249 <at> debbugs.gnu.org
Subject: Re: [PATCH v3] gnu: emacs: Make strip-double-wrap more robust.
Date: Thu, 05 Nov 2020 23:33:19 +0100
Hello,

> (Can you reopen this bug report please?)

I think you need to open a new one.

> So I see 3 possible solutions:
> 1. Accept my first patch and give up on match
> 2. Accept this patch and modify almost every emacs varient (I did test building them all)
> 3. Figure out some proper module inheritence
>
> I think option 3 is the most correct, but I'm lazy so I'm leaning
> towards option 1.

I think I cannot help you, as I'm not sure about how module inheritance
is handled. Maybe someone else can chime in.

Regards,
-- 
Nicolas Goaziou




Did not alter fixed versions and reopened. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Thu, 05 Nov 2020 22:38:02 GMT) Full text and rfc822 format available.

Information forwarded to guix-patches <at> gnu.org:
bug#44249; Package guix-patches. (Sat, 07 Nov 2020 20:49:02 GMT) Full text and rfc822 format available.

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

From: Marius Bakke <marius <at> gnu.org>
To: Nicolas Goaziou <mail <at> nicolasgoaziou.fr>, Morgan.J.Smith <at> outlook.com
Cc: 44249 <at> debbugs.gnu.org
Subject: Re: [bug#44249] [PATCH v3] gnu: emacs: Make strip-double-wrap more
 robust.
Date: Sat, 07 Nov 2020 21:48:49 +0100
[Message part 1 (text/plain, inline)]
Nicolas Goaziou <mail <at> nicolasgoaziou.fr> writes:

> Hello,
>
>> (Can you reopen this bug report please?)
>
> I think you need to open a new one.
>
>> So I see 3 possible solutions:
>> 1. Accept my first patch and give up on match
>> 2. Accept this patch and modify almost every emacs varient (I did test building them all)
>> 3. Figure out some proper module inheritence
>>
>> I think option 3 is the most correct, but I'm lazy so I'm leaning
>> towards option 1.
>
> I think I cannot help you, as I'm not sure about how module inheritance
> is handled. Maybe someone else can chime in.

I haven't followed the discussion in detail, but from skimming through
the thread, is it just about adding (ice-9 match) to the build system
modules?

In that case I think

  #:modules ((ice-9 match) ,@%glib-or-gtk-build-system-modules)

...should do the trick.
[signature.asc (application/pgp-signature, inline)]

Information forwarded to guix-patches <at> gnu.org:
bug#44249; Package guix-patches. (Fri, 15 Jan 2021 13:29:02 GMT) Full text and rfc822 format available.

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Morgan.J.Smith <at> outlook.com
Cc: 44249 <at> debbugs.gnu.org, mail <at> nicolasgoaziou.fr
Subject: Re: bug#44249: [PATCH] gnu: emacs: Make strip-double-wrap more robust
Date: Fri, 15 Jan 2021 14:28:26 +0100
Hi Morgan,

Morgan.J.Smith <at> outlook.com skribis:

> From: Morgan Smith <Morgan.J.Smith <at> outlook.com>
>
> * gnu/packages/emacs.scm (emacs) [strip-double-wrap]: Use regex to find emacs
> executable.  This works even when the version is changed by package
> transformations (e.g., version=git.master).

[...]

>               (with-directory-excursion (assoc-ref outputs "out")
> -               (copy-file (string-append
> -                           "bin/emacs-"
> -                           ,(let ((this-version (package-version this-package)))
> -                              (or (false-if-exception
> -                                   (version-major+minor+point this-version))
> -                                  (version-major+minor this-version))))
> -                          "bin/emacs")
> +               (copy-file
> +                (match (find-files "bin" "^emacs-")
> +                  ((executable . _) executable))

If we assume there should be just one “^emacs-” executable, you can
change the match clause to reflect it:

  (match (find-files "bin" "^emacs-")
    ((executable) executable))

To be even more defensive, you could refine the regexp to
“^emacs-[0-9]”.

> +                "bin/emacs")

[...]

> +       ((#:modules modules)
> +        `((guix build gnu-build-system)
> +          (guix build utils)
> +          (ice-9 match)))))

Unless I’m missing something, you don’t need to repeat #:modules in
every variant: the ‘arguments’ field is inherited by those variants, and
that includes #:modules.

You can check easily that re-adding #:modules has no effect by checking
the output of, say:

  ./pre-inst-env guix build emacs-xwidgets -d --no-grafts

before and after removing the ((#:modules modules) …) bit.

Could you send an updated patch?

This is the last missing bit before one can run things like:

  guix install emacs-next --with-branch=emacs-next=master

:-)

Thanks,
Ludo’.




Information forwarded to guix-patches <at> gnu.org:
bug#44249; Package guix-patches. (Fri, 15 Jan 2021 19:50:02 GMT) Full text and rfc822 format available.

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

From: Morgan Smith <Morgan.J.Smith <at> outlook.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 44249 <at> debbugs.gnu.org, mail <at> nicolasgoaziou.fr
Subject: Re: bug#44249: [PATCH] gnu: emacs: Make strip-double-wrap more robust
Date: Fri, 15 Jan 2021 14:49:15 -0500
I've actually been sorta half working on this for a while now.

The problem is exactly that the modules field is inherited. See each
build system includes its own module in the modules field. The various
emacsen are built with different build systems. So emacs is going to
need to import (guix build glib-or-gtk-build-system) and emacs-minimal
is going to want (guix build gnu-build-system). By setting the modules
to be the glib-or-gtk-build-system, we override the default modules in
each inherited package. This means building emacs-minimal would result
in this error:

no code for module (guix build glib-or-gtk-build-system)

I'm not entirely certain why it worked for you but it looks like maybe
you included the gnu-build system instead of the glib-or-gtk-build-system.

I think to solve this issue proper, we need to come up with a way to use
%default-modules. Currently this variable isn't usable in this context,
but as gnu/packages/code.scm:791 says: ";; FIXME use %default-modules"




Information forwarded to guix-patches <at> gnu.org:
bug#44249; Package guix-patches. (Sat, 16 Jan 2021 21:55:01 GMT) Full text and rfc822 format available.

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Morgan Smith <Morgan.J.Smith <at> outlook.com>
Cc: 44249 <at> debbugs.gnu.org, mail <at> nicolasgoaziou.fr
Subject: Re: bug#44249: [PATCH] gnu: emacs: Make strip-double-wrap more robust
Date: Sat, 16 Jan 2021 22:54:29 +0100
Hi,

Morgan Smith <Morgan.J.Smith <at> outlook.com> skribis:

> I've actually been sorta half working on this for a while now.
>
> The problem is exactly that the modules field is inherited. See each
> build system includes its own module in the modules field. The various
> emacsen are built with different build systems. So emacs is going to
> need to import (guix build glib-or-gtk-build-system) and emacs-minimal
> is going to want (guix build gnu-build-system). By setting the modules
> to be the glib-or-gtk-build-system, we override the default modules in
> each inherited package. This means building emacs-minimal would result
> in this error:
>
> no code for module (guix build glib-or-gtk-build-system)

Ooh, my bad, I had completely overlooked this “detail”.

Then I guess the patch is fine though… in this case you could
exceptionally ;-) write (car (find-files …)) so you don’t even need to
both importing (ice-9 match).  That’d save quite a few lines of code.

WDYT?

Thanks!

Ludo’.




Information forwarded to guix-patches <at> gnu.org:
bug#44249; Package guix-patches. (Sat, 16 Jan 2021 22:05:01 GMT) Full text and rfc822 format available.

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

From: Morgan Smith <Morgan.J.Smith <at> outlook.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 44249 <at> debbugs.gnu.org, mail <at> nicolasgoaziou.fr
Subject: Re: bug#44249: [PATCH] gnu: emacs: Make strip-double-wrap more robust
Date: Sat, 16 Jan 2021 17:03:55 -0500
Hi Ludo,

I think that's an elegant and wonderful idea. Assuming the first patch
in this thread still applies, I vote we just apply that one.

There has been some discussion in this thread on if the regex should
actually look for numbers or not (notably Nicolas and you). I could go
either way. I'm pretty sure the regex that's already there that matches
2 or more period separated numbers will always hold true, but feel free
to loosen up the regex a little if you feel otherwise.

Thanks,

Morgan

On 1/16/21 4:54 PM, Ludovic Courtès wrote:
> Hi,
> 
> Morgan Smith <Morgan.J.Smith <at> outlook.com> skribis:
> 
>> I've actually been sorta half working on this for a while now.
>>
>> The problem is exactly that the modules field is inherited. See each
>> build system includes its own module in the modules field. The various
>> emacsen are built with different build systems. So emacs is going to
>> need to import (guix build glib-or-gtk-build-system) and emacs-minimal
>> is going to want (guix build gnu-build-system). By setting the modules
>> to be the glib-or-gtk-build-system, we override the default modules in
>> each inherited package. This means building emacs-minimal would result
>> in this error:
>>
>> no code for module (guix build glib-or-gtk-build-system)
> 
> Ooh, my bad, I had completely overlooked this “detail”.
> 
> Then I guess the patch is fine though… in this case you could
> exceptionally ;-) write (car (find-files …)) so you don’t even need to
> both importing (ice-9 match).  That’d save quite a few lines of code.
> 
> WDYT?
> 
> Thanks!
> 
> Ludo’.
> 




Reply sent to Ludovic Courtès <ludo <at> gnu.org>:
You have taken responsibility. (Sun, 31 Jan 2021 20:31:02 GMT) Full text and rfc822 format available.

Notification sent to Morgan.J.Smith <at> outlook.com:
bug acknowledged by developer. (Sun, 31 Jan 2021 20:31:02 GMT) Full text and rfc822 format available.

Message #56 received at 44249-done <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Morgan.J.Smith <at> outlook.com
Cc: 44249-done <at> debbugs.gnu.org
Subject: Re: bug#44249: [PATCH] gnu: emacs: Make strip-double-wrap more robust
Date: Sun, 31 Jan 2021 21:30:51 +0100
Hi Morgan,

Morgan.J.Smith <at> outlook.com skribis:

> From: Morgan Smith <Morgan.J.Smith <at> outlook.com>
>
> * gnu/packages/emacs.scm (emacs) [strip-double-wrap]:
> Use regex to find emacs executable. This works even when the version is
> changed by package transformations (ex: version=git.master)

Somehow I had forgotten about this patch, but I finally applied it!

>               (with-directory-excursion (assoc-ref outputs "out")
> -               (copy-file (string-append
> -                           "bin/emacs-"
> -                           ,(let ((this-version (package-version this-package)))
> -                              (or (false-if-exception
> -                                   (version-major+minor+point this-version))
> -                                  (version-major+minor this-version))))
> -                          "bin/emacs")
> +               (copy-file
> +                (car
> +                 (find-files
> +                  "bin" (file-name-predicate "^emacs-([0-9]+\\.)+[0-9]+$")))

Here I just remove ‘file-name-predicate’ because it’s implicit.  I
checked that it works with the currently-packaged version as well as
‘--with-branch=emacs-next=master’.

Thank you, and apologies for the long delay!

Ludo’.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Mon, 01 Mar 2021 12:24:05 GMT) Full text and rfc822 format available.

This bug report was last modified 3 years and 57 days ago.

Previous Next


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