[bug#39717] [PATCH 1/3] gnu: delft-icon-theme: Use the copy-build-system.

[bug#39717] [PATCH 1/3] gnu: delft-icon-theme: Use the copy-build-system.

[Leo Prikler]

* gnu/packages/gnome-xyz.scm (delft-icon-theme): Use the copy-build-system.
---
 gnu/packages/gnome-xyz.scm | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/gnu/packages/gnome-xyz.scm b/gnu/packages/gnome-xyz.scm
index 06af41aea6..882bd40c2e 100644
--- a/gnu/packages/gnome-xyz.scm
+++ b/gnu/packages/gnome-xyz.scm
@@ -33,7 +33,8 @@
   #:use-module (gnu packages gtk)
   #:use-module (gnu packages pkg-config)
   #:use-module (gnu packages ruby)
-  #:use-module (gnu packages xml))
+  #:use-module (gnu packages xml)
+  #:use-module (srfi srfi-1))
 
 (define-public matcha-theme
   (package
@@ -95,19 +96,17 @@ like Gnome, Unity, Budgie, Pantheon, XFCE, Mate and others.")
         (base32
          "0vw3yw9f9ygzfd2k3zrfih3r0vkzlhk1bmsk8sapvk7np24i1z9s"))
        (file-name (git-file-name name version))))
-    (build-system trivial-build-system)
+    (build-system copy-build-system)
     (arguments
-     `(#:modules ((guix build utils))
-       #:builder
-       (begin
-         (use-modules (guix build utils))
-         (copy-recursively (assoc-ref %build-inputs "source") "icons")
-         (substitute* "icons/Delft/index.theme"
-           (("gnome") "Adwaita"))
-         (delete-file "icons/README.md")
-         (delete-file "icons/LICENSE")
-         (delete-file "icons/logo.jpg")
-         (copy-recursively "icons" (string-append %output "/share/icons")))))
+     `(#:install-plan
+       '(,@(append-map (lambda (file)
+                         `((,file "share/icons/")
+                           (,(string-append file "-Dark") "share/icons/")
+                           (,(string-append file "-Darker") "share/icons/")
+                           (,(string-append file "-Darkest") "share/icons/")))
+                       '("Delft" "Delft-Amber" "Delft-Aqua" "Delft-Blue"
+                         "Delft-Gray" "Delft-Green" "Delft-Mint" "Delft-Purple"
+                         "Delft-Red" "Delft-Teal")))))
     (home-page "https://www.gnome-look.org/p/1199881/")
     (synopsis "Continuation of Faenza icon theme with up to date app icons")
     (description "Delft is a fork of the popular icon theme Faenza with up to
-- 
2.25.1

[bug#39716] [PATCH 2/3] gnu: gnome-shell-extension-appindicator: Use the copy-build-system.

[Leo Prikler]

* gnu/packages/gnome-xyz.scm (gnome-shell-extension-appindicator):
Use the copy-build-system.
---
 gnu/packages/gnome-xyz.scm | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/gnu/packages/gnome-xyz.scm b/gnu/packages/gnome-xyz.scm
index 882bd40c2e..0f945fc19d 100644
--- a/gnu/packages/gnome-xyz.scm
+++ b/gnu/packages/gnome-xyz.scm
@@ -129,19 +129,11 @@ the Obsidian icon theme.")
                (base32
                 "1fjhx23jqwv3d0smwhnjvc35gqhwk9p5f96ic22pfax653cn5vh8"))
               (file-name (git-file-name name version))))
-    (build-system trivial-build-system)
+    (build-system copy-build-system)
     (arguments
-     '(#:modules ((guix build utils))
-       #:builder
-       (begin
-         (use-modules (guix build utils))
-         (let* ((source (assoc-ref %build-inputs "source"))
-                (install-dir (string-append (assoc-ref %outputs "out")
-                                            "/share/gnome-shell/extensions"
-                                            "/appindicatorsupport@rgcjonas.gmail.com")))
-           (mkdir-p install-dir)
-           (copy-recursively source install-dir)
-           #t))))
+     `(#:install-plan
+       '(("." ,(string-append "share/gnome-shell/extensions/"
+                              "appindicatorsupport@rgcjonas.gmail.com")))))
     (synopsis "Adds KStatusNotifierItem support to GNOME Shell")
     (description "This extension integrates Ubuntu AppIndicators
 and KStatusNotifierItems (KDE's successor of the systray) into
-- 
2.25.1

[bug#39716] [PATCH 2/3] gnu: gnome-shell-extension-appindicator: Use the copy-build-system.

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 65 bytes --]

Merged, thanks!

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

[bug#39715] [PATCH 3/3] gnu: gnome-shell-extension-noannoyance: Use the copy-build-system.

[Leo Prikler]

* gnu/packages/gnome-xyz.scm (gnome-shell-extension-noannoyance):
Use the copy-build-system.
---
 gnu/packages/gnome-xyz.scm | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/gnu/packages/gnome-xyz.scm b/gnu/packages/gnome-xyz.scm
index 0f945fc19d..fa77fa79a7 100644
--- a/gnu/packages/gnome-xyz.scm
+++ b/gnu/packages/gnome-xyz.scm
@@ -279,18 +279,10 @@ into a single panel, similar to that found in KDE Plasma and Windows 7+.")
                (base32
                 "0fa8l3xlh8kbq07y4385wpb908zm6x53z81q16xlmin97dln32hh"))
               (file-name (git-file-name name version))))
-    (build-system trivial-build-system)
+    (build-system copy-build-system)
     (arguments
-     '(#:modules ((guix build utils))
-       #:builder
-       (begin
-         (use-modules (guix build utils))
-         (let ((dst (string-append
-                     (assoc-ref %outputs "out")
-                     "/share/gnome-shell/extensions/"
-                     "noannoyance@daase.net")))
-           (mkdir-p dst)
-           (copy-recursively (assoc-ref %build-inputs "source") dst)))))
+     '(#:install-plan
+       '(("." "share/gnome-shell/extensions/noannoyance@daase.net"))))
     (synopsis "Removes 'Window is ready' annotation")
     (description "One of the many extensions, that remove this message.
 It uses ES6 syntax and claims to be more actively maintained than others.")
-- 
2.25.1

[bug#39715] [PATCH 3/3] gnu: gnome-shell-extension-noannoyance: Use the copy-build-system.

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 65 bytes --]

Merged, thanks!

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

[bug#39717] [PATCH 1/3] gnu: delft-icon-theme: Use the copy-build-system.

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 1417 bytes --]


Thanks for the patch!
Comments below:

>      (arguments
> -     `(#:modules ((guix build utils))
> -       #:builder
> -       (begin
> -         (use-modules (guix build utils))
> -         (copy-recursively (assoc-ref %build-inputs "source") "icons")
> -         (substitute* "icons/Delft/index.theme"
> -           (("gnome") "Adwaita"))
> -         (delete-file "icons/README.md")
> -         (delete-file "icons/LICENSE")
> -         (delete-file "icons/logo.jpg")
> -         (copy-recursively "icons" (string-append %output "/share/icons")))))
> +     `(#:install-plan
> +       '(,@(append-map (lambda (file)
> +                         `((,file "share/icons/")
> +                           (,(string-append file "-Dark") "share/icons/")
> +                           (,(string-append file "-Darker") "share/icons/")
> +                           (,(string-append file "-Darkest") "share/icons/")))
> +                       '("Delft" "Delft-Amber" "Delft-Aqua" "Delft-Blue"
> +                         "Delft-Gray" "Delft-Green" "Delft-Mint" "Delft-Purple"
> +                         "Delft-Red" "Delft-Teal")))))

Interesting use of install-plan, but wouldn't it be to copy everything
and simply exclude the previous delete files?  E.g. (untested):

#:install-plan
`(("." "./" #:exclude ("README.md" "LICENSE" "logo.jpg")))

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

[bug#39717] [PATCH 1/3] gnu: delft-icon-theme: Use the copy-build-system.

[Leo Prikler]

Am Donnerstag, den 27.02.2020, 11:27 +0100 schrieb Pierre Neidhardt:
> Thanks for the patch!
> Comments below:
> 
> >      (arguments
> > -     `(#:modules ((guix build utils))
> > -       #:builder
> > -       (begin
> > -         (use-modules (guix build utils))
> > -         (copy-recursively (assoc-ref %build-inputs "source")
> > "icons")
> > -         (substitute* "icons/Delft/index.theme"
> > -           (("gnome") "Adwaita"))
> > -         (delete-file "icons/README.md")
> > -         (delete-file "icons/LICENSE")
> > -         (delete-file "icons/logo.jpg")
> > -         (copy-recursively "icons" (string-append %output
> > "/share/icons")))))
> > +     `(#:install-plan
> > +       '(,@(append-map (lambda (file)
> > +                         `((,file "share/icons/")
> > +                           (,(string-append file "-Dark")
> > "share/icons/")
> > +                           (,(string-append file "-Darker")
> > "share/icons/")
> > +                           (,(string-append file "-Darkest")
> > "share/icons/")))
> > +                       '("Delft" "Delft-Amber" "Delft-Aqua"
> > "Delft-Blue"
> > +                         "Delft-Gray" "Delft-Green" "Delft-Mint"
> > "Delft-Purple"
> > +                         "Delft-Red" "Delft-Teal")))))
> 
> Interesting use of install-plan, but wouldn't it be to copy
> everything
> and simply exclude the previous delete files?  E.g. (untested):
> 
> #:install-plan
> `(("." "./" #:exclude ("README.md" "LICENSE" "logo.jpg")))
It would indeed be nice if it worked that way.  However, Delft makes
heavy use of symbolic links, of which some are even dead, and that
causes install-file to fail.  
I tried patching copy-build-system, but the result was not usable,
probably because symlinks were not resolved correctly.  On top of that,
I don't think putting that much more work into copy-build-system is a
good idea.  copy-build-system should be a simple build-system that just
copies stuff.  Perhaps we can swallow the "file does not exist" errors,
but even that seems kinda wrong to me.
Anyways, since the original used copy-recursively, the only thing I had
to do was to make copy-build-system resort to copy-recursively as well,
hence this interesting use of install-plan.  Alternatively, one could
delete the files in a pre-install phase, but that seems even weirder to
me. 

Regards,
Leo

[bug#39717] [PATCH 1/3] gnu: delft-icon-theme: Use the copy-build-system.

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 247 bytes --]

Can you detail what does not work?  Which file, what error message?

In my opinion, the install-plan should be able to install any file,
symlink or not.  Failing to do so seems like a bug to me.

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

[bug#39717] [PATCH 1/3] gnu: delft-icon-theme: Use the copy-build-system.

[Leo Prikler]

Am Donnerstag, den 27.02.2020, 14:19 +0100 schrieb Pierre Neidhardt:
> Can you detail what does not work?  Which file, what error message?
$ find `guix build --source delft-icon-theme` -xtype l
This returns a list of broken symlinks.  On any of them, install-file
fails with "file not found".

> In my opinion, the install-plan should be able to install any file,
> symlink or not.  Failing to do so seems like a bug to me.
I think I agree with you.  Instead of simply doing (copy-file file
dest), we'd have to check for symlinks as is done in copy-recursively. 
Now that I have a more complete understanding of this bug, I'll try to
come up with a patch.

Regards,
Leo

[bug#39717] [PATCH 1/3] gnu: delft-icon-theme: Use the copy-build-system.

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 73 bytes --]

Thanks, and good catch!

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

[bug#39717] [PATCH 1/3] gnu: delft-icon-theme: Use the copy-build-system.

[Leo Prikler]

[-- Attachment #1: Type: text/plain, Size: 223 bytes --]

Am Donnerstag, den 27.02.2020, 15:02 +0100 schrieb Pierre Neidhardt:
> Thanks, and good catch!
Patch is ready.  I also noticed some inconsistencies with my previous
definition and updated the package to 1.11.

Regards,
Leo

[-- Attachment #2: 0001-build-system-copy-build-system-Keep-symlinks-symboli.patch --]
[-- Type: text/x-patch, Size: 1318 bytes --]

From 7147b298949e17b3b3af857d91437f9012149297 Mon Sep 17 00:00:00 2001
From: Leo Prikler <leo.prikler@student.tugraz.at>
Date: Thu, 27 Feb 2020 14:49:11 +0100
Subject: [PATCH 1/2] build-system: copy-build-system: Keep symlinks symbolic.

guix/build/copy-build-system.scm (install)[install-file]:
Read symlinks as is done in install-simple through copy-recursively.
---
 guix/build/copy-build-system.scm | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/guix/build/copy-build-system.scm b/guix/build/copy-build-system.scm
index 6d9dc8f93b..a86f0cde29 100644
--- a/guix/build/copy-build-system.scm
+++ b/guix/build/copy-build-system.scm
@@ -91,7 +91,13 @@ if TARGET ends with a '/', the source is installed underneath."
                                    file))))
       (format (current-output-port) "`~a' -> `~a'~%" file dest)
       (mkdir-p (dirname dest))
-      (copy-file file dest)))
+      (let ((stat (lstat file)))
+        (case (stat:type stat)
+          ((symlink)
+           (let ((target (readlink file)))
+             (symlink target dest)))
+          (else
+           (copy-file file dest))))))
 
   (define* (make-file-predicate suffixes matches-regexp #:optional (default-value #t))
     "Return a predicate that returns #t if its file argument matches the
-- 
2.25.1


[-- Attachment #3: 0002-gnu-delft-icon-theme-Use-the-copy-build-system.patch --]
[-- Type: text/x-patch, Size: 1934 bytes --]

From d51c90a241452ba03f9d2b5038233967bb94e844 Mon Sep 17 00:00:00 2001
From: Leo Prikler <leo.prikler@student.tugraz.at>
Date: Fri, 21 Feb 2020 17:17:56 +0100
Subject: [PATCH 2/2] gnu: delft-icon-theme: Use the copy-build-system.

* gnu/packages/gnome-xyz.scm (delft-icon-theme): Use the copy-build-system.
---
 gnu/packages/gnome-xyz.scm | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/gnu/packages/gnome-xyz.scm b/gnu/packages/gnome-xyz.scm
index bedaacf092..b0bc35c4b1 100644
--- a/gnu/packages/gnome-xyz.scm
+++ b/gnu/packages/gnome-xyz.scm
@@ -101,19 +101,17 @@ like Gnome, Unity, Budgie, Pantheon, XFCE, Mate and others.")
         (base32
          "0vw3yw9f9ygzfd2k3zrfih3r0vkzlhk1bmsk8sapvk7np24i1z9s"))
        (file-name (git-file-name name version))))
-    (build-system trivial-build-system)
+    (build-system copy-build-system)
     (arguments
-     `(#:modules ((guix build utils))
-       #:builder
-       (begin
-         (use-modules (guix build utils))
-         (copy-recursively (assoc-ref %build-inputs "source") "icons")
-         (substitute* "icons/Delft/index.theme"
-           (("gnome") "Adwaita"))
-         (delete-file "icons/README.md")
-         (delete-file "icons/LICENSE")
-         (delete-file "icons/logo.jpg")
-         (copy-recursively "icons" (string-append %output "/share/icons")))))
+     `(#:install-plan
+       `(("." "share/icons" #:exclude ("README.md" "LICENSE" "logo.jpg")))
+       #:phases
+       (modify-phases %standard-phases
+         (add-after 'unpack 'patch-index.theme
+           (lambda _
+            (substitute* "Delft/index.theme"
+              (("gnome") "Adwaita"))
+            #t)))))
     (home-page "https://www.gnome-look.org/p/1199881/")
     (synopsis "Continuation of Faenza icon theme with up to date app icons")
     (description "Delft is a fork of the popular icon theme Faenza with up to
-- 
2.25.1


[-- Attachment #4: 0003-gnu-delft-icon-theme-Update-to-1.11.patch --]
[-- Type: text/x-patch, Size: 1218 bytes --]

From c90ab439c7c79350fa783a9f20182a34fea95289 Mon Sep 17 00:00:00 2001
From: Leo Prikler <leo.prikler@student.tugraz.at>
Date: Thu, 27 Feb 2020 15:47:39 +0100
Subject: [PATCH 3/3] gnu: delft-icon-theme: Update to 1.11.

* gnu/packages/gnome-xyz.scm (delft-icon-theme): Update to 1.11.
---
 gnu/packages/gnome-xyz.scm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gnu/packages/gnome-xyz.scm b/gnu/packages/gnome-xyz.scm
index b0bc35c4b1..b8cd32db08 100644
--- a/gnu/packages/gnome-xyz.scm
+++ b/gnu/packages/gnome-xyz.scm
@@ -90,7 +90,7 @@ like Gnome, Unity, Budgie, Pantheon, XFCE, Mate and others.")
 (define-public delft-icon-theme
   (package
     (name "delft-icon-theme")
-    (version "1.10")
+    (version "1.11")
     (source
      (origin
        (method git-fetch)
@@ -99,7 +99,7 @@ like Gnome, Unity, Budgie, Pantheon, XFCE, Mate and others.")
              (commit (string-append "v" version))))
        (sha256
         (base32
-         "0vw3yw9f9ygzfd2k3zrfih3r0vkzlhk1bmsk8sapvk7np24i1z9s"))
+         "1m3r4i4m3y3xsjb5f4bik0ylmi64amkfyr0y8pjbvv6gyj492mi6"))
        (file-name (git-file-name name version))))
     (build-system copy-build-system)
     (arguments
-- 
2.25.1

[bug#39717] [PATCH 1/3] gnu: delft-icon-theme: Use the copy-build-system.

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 65 bytes --]

Merged, thanks!

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

[bug#39717] [PATCH 1/3] gnu: delft-icon-theme: Use the copy-build-system.

[Leo Prikler]

Am Donnerstag, den 27.02.2020, 11:27 +0100 schrieb Pierre Neidhardt:
> Thanks for the patch!
> Comments below:
> 
> >      (arguments
> > -     `(#:modules ((guix build utils))
> > -       #:builder
> > -       (begin
> > -         (use-modules (guix build utils))
> > -         (copy-recursively (assoc-ref %build-inputs "source")
> > "icons")
> > -         (substitute* "icons/Delft/index.theme"
> > -           (("gnome") "Adwaita"))
> > -         (delete-file "icons/README.md")
> > -         (delete-file "icons/LICENSE")
> > -         (delete-file "icons/logo.jpg")
> > -         (copy-recursively "icons" (string-append %output
> > "/share/icons")))))
> > +     `(#:install-plan
> > +       '(,@(append-map (lambda (file)
> > +                         `((,file "share/icons/")
> > +                           (,(string-append file "-Dark")
> > "share/icons/")
> > +                           (,(string-append file "-Darker")
> > "share/icons/")
> > +                           (,(string-append file "-Darkest")
> > "share/icons/")))
> > +                       '("Delft" "Delft-Amber" "Delft-Aqua"
> > "Delft-Blue"
> > +                         "Delft-Gray" "Delft-Green" "Delft-Mint"
> > "Delft-Purple"
> > +                         "Delft-Red" "Delft-Teal")))))
> 
> Interesting use of install-plan, but wouldn't it be to copy
> everything
> and simply exclude the previous delete files?  E.g. (untested):
> 
> #:install-plan
> `(("." "./" #:exclude ("README.md" "LICENSE" "logo.jpg")))
It would indeed be nice if it worked that way.  However, Delft makes
heavy use of symbolic links, of which some are even dead, and that
causes install-file to fail.  
I tried patching copy-build-system, but the result was not usable,
probably because symlinks were not resolved correctly.  On top of that,
I don't think putting that much more work into copy-build-system is a
good idea.  copy-build-system should be a simple build-system that just
copies stuff.  Perhaps we can swallow the "file does not exist" errors,
but even that seems kinda wrong to me.
Anyways, since the original used copy-recursively, the only thing I had
to do was to make copy-build-system resort to copy-recursively as well,
hence this interesting use of install-plan.  Alternatively, one could
delete the files in a pre-install phase, but that seems even weirder to
me. 

Regards,
Leo