[bug#54216] [PATCH] gnu: shepherd-for-guix: New package for use in Guix.

[bug#54216] [PATCH] gnu: shepherd-for-guix: New package for use in Guix.

[Attila Lendvai]

This increases the flexibility to inherit from this package, and to use
a non-yet-released version of Shepherd in a Guix build while working on
Shepherd.

IOW, it makes it easier to update Shepherd without going through staging
and/or rebuilding several dependant packages.

This also updates Shepherd to the latest commit, so that the two variants
have a different version.

* gnu/packages/admin.scm (shepherd-for-guix): New variable.
---

once it's merged, i'll send another patch to staging that will simplify this.

 gnu/packages/admin.scm    | 36 ++++++++++++++++++++++++++++++++++++
 gnu/services/shepherd.scm |  2 +-
 2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/admin.scm b/gnu/packages/admin.scm
index c8f91aab0d..3c02b16f34 100644
--- a/gnu/packages/admin.scm
+++ b/gnu/packages/admin.scm
@@ -307,6 +307,42 @@ (define-public shepherd
     (license license:gpl3+)
     (home-page "https://www.gnu.org/software/shepherd/")))
 
+(define-public shepherd-for-guix
+  (let* ((version "0.8.1")
+         ;; If it's an unreleased commit:
+         (commit "7c380590164ea8ee40de46059d07e08a48963577")
+         ;; Use the below if it's a release, and set REVISION to #f.
+         ;; (commit (string-append "v" version))
+         (revision "1"))
+    (package
+      (inherit shepherd)
+      (source
+       (origin
+         (method git-fetch)
+         (uri (git-reference
+               ;; Build from git and add Autotools inputs, to make developing
+               ;; Shepherd easier. It enables easier package inheritance.
+               (url "https://git.savannah.gnu.org/git/shepherd.git/")
+               (commit commit)))
+         (sha256
+          (base32
+           "1hgkbl3fyzwi5vw63kbkswnf3viyfl52c5dzkx7vbkr4sj5ysz1g"))
+         (modules '((guix build utils)))
+         (snippet
+          '(begin
+             ;; Build with -O1 to work around <https://bugs.gnu.org/48368>.
+             (substitute* "Makefile.am"
+               (("compile --target")
+                "compile -O1 --target"))))))
+      (version (if revision
+                   (git-version version revision commit)
+                   version))
+      (native-inputs
+       (modify-inputs (package-native-inputs shepherd)
+         (prepend autoconf automake gettext-minimal help2man texinfo)))
+      (description "A package variant for use in Guix. It helps lowering
+the build time of Guix when working on Shepherd."))))
+
 (define-public guile2.2-shepherd
   (package
     (inherit shepherd)
diff --git a/gnu/services/shepherd.scm b/gnu/services/shepherd.scm
index b44dbf9d9f..991194ffe6 100644
--- a/gnu/services/shepherd.scm
+++ b/gnu/services/shepherd.scm
@@ -88,7 +88,7 @@ (define-record-type* <shepherd-configuration>
   shepherd-configuration make-shepherd-configuration
   shepherd-configuration?
   (shepherd shepherd-configuration-shepherd
-            (default shepherd)) ; file-like
+            (default shepherd-for-guix)) ; file-like
   (services shepherd-configuration-services
             (default '()))) ; list of <shepherd-service>
 
-- 
2.34.0

[bug#54216] [PATCH] gnu: shepherd-for-guix: New package for use in Guix.

[Maxime Devos]

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

Attila Lendvai schreef op di 01-03-2022 om 19:28 [+0100]:
> This increases the flexibility to inherit from this package, and to use
> a non-yet-released version of Shepherd in a Guix build while working on
> Shepherd.
> 
> IOW, it makes it easier to update Shepherd without going through staging
> and/or rebuilding several dependant packages.
> 
> This also updates Shepherd to the latest commit, so that the two variants
> have a different version.
> 
> * gnu/packages/admin.scm (shepherd-for-guix): New variable.
> ---
> 
> once it's merged, i'll send another patch to staging that will simplify this.
> 
>  gnu/packages/admin.scm    | 36 ++++++++++++++++++++++++++++++++++++
>  gnu/services/shepherd.scm |  2 +-
>  2 files changed, 37 insertions(+), 1 deletion(-)
> 
> diff --git a/gnu/packages/admin.scm b/gnu/packages/admin.scm
> index c8f91aab0d..3c02b16f34 100644
> --- a/gnu/packages/admin.scm
> +++ b/gnu/packages/admin.scm
> @@ -307,6 +307,42 @@ (define-public shepherd
>      (license license:gpl3+)
>      (home-page "https://www.gnu.org/software/shepherd/")))
>  
> +(define-public shepherd-for-guix


Perhaps the explanation what's it for could be moved to a comment next
to 'shepherd-for-guix'?  Perhaps:

;; This is the Shepherd package used by Guix.  This package variant
;; allows us to add new features and fix bugs in Shepherd and use the
;; new features in Guix, without having to go through the 'staging'
;; branch and without having to wait for a new release, see
;; [...] in the manual.
(define-public shepherd-for-guix ...)

Greetings,
Maxime.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

[bug#54216] [PATCH v2] gnu: shepherd-for-guix: New package for use in Guix.

[Attila Lendvai]

This also updates shepherd-for-guix to the latest commit, so that the two
variants have a different version.

* gnu/packages/admin.scm (shepherd-for-guix): New variable.
---

v2: add the recommended comment and some edits to the other comments.

 gnu/packages/admin.scm    | 41 +++++++++++++++++++++++++++++++++++++++
 gnu/services/shepherd.scm |  2 +-
 2 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/admin.scm b/gnu/packages/admin.scm
index c8f91aab0d..f43526b7d9 100644
--- a/gnu/packages/admin.scm
+++ b/gnu/packages/admin.scm
@@ -307,6 +307,47 @@ (define-public shepherd
     (license license:gpl3+)
     (home-page "https://www.gnu.org/software/shepherd/")))
 
+;; This is the Shepherd package used by Guix.  This package variant
+;; allows us to add new features and fix bugs in Shepherd and use the
+;; new features in Guix, without having to go through the 'staging'
+;; branch, and without having to wait for a new release, see
+;; [TODO] in the manual.
+(define-public shepherd-for-guix
+  (let* ((version "0.8.1")
+         ;; If it's an unreleased commit:
+         (commit "7c380590164ea8ee40de46059d07e08a48963577")
+         ;; Use the below form if it's a release, and also set REVISION to #f.
+         ;; (commit (string-append "v" version))
+         (revision "1"))
+    (package
+      (inherit shepherd)
+      (source
+       (origin
+         (method git-fetch)
+         (uri (git-reference
+               ;; Build from git and add Autotools inputs, to make developing
+               ;; Shepherd easier. It enables easier package inheritance.
+               (url "https://git.savannah.gnu.org/git/shepherd.git/")
+               (commit commit)))
+         (sha256
+          (base32
+           "1hgkbl3fyzwi5vw63kbkswnf3viyfl52c5dzkx7vbkr4sj5ysz1g"))
+         (modules '((guix build utils)))
+         (snippet
+          '(begin
+             ;; Build with -O1 to work around <https://bugs.gnu.org/48368>.
+             (substitute* "Makefile.am"
+               (("compile --target")
+                "compile -O1 --target"))))))
+      (version (if revision
+                   (git-version version revision commit)
+                   version))
+      (native-inputs
+       (modify-inputs (package-native-inputs shepherd)
+         (prepend autoconf automake gettext-minimal help2man texinfo)))
+      (description "A package variant for use in Guix. It helps lowering
+the build time of Guix when working on Shepherd."))))
+
 (define-public guile2.2-shepherd
   (package
     (inherit shepherd)
diff --git a/gnu/services/shepherd.scm b/gnu/services/shepherd.scm
index b44dbf9d9f..991194ffe6 100644
--- a/gnu/services/shepherd.scm
+++ b/gnu/services/shepherd.scm
@@ -88,7 +88,7 @@ (define-record-type* <shepherd-configuration>
   shepherd-configuration make-shepherd-configuration
   shepherd-configuration?
   (shepherd shepherd-configuration-shepherd
-            (default shepherd)) ; file-like
+            (default shepherd-for-guix)) ; file-like
   (services shepherd-configuration-services
             (default '()))) ; list of <shepherd-service>
 
-- 
2.34.0

[bug#54216] [PATCH v2] gnu: shepherd-for-guix: New package for use in Guix.

[Maxime Devos]

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

Attila Lendvai schreef op di 01-03-2022 om 19:45 [+0100]:
> +       (origin
> +         (method git-fetch)
> +         (uri (git-reference
> +               ;; Build from git and add Autotools inputs, to make developing
> +               ;; Shepherd easier. It enables easier package inheritance.
> +               (url "https://git.savannah.gnu.org/git/shepherd.git/")
> +               (commit commit)))
> +         (sha256
> +          (base32
> +           "1hgkbl3fyzwi5vw63kbkswnf3viyfl52c5dzkx7vbkr4sj5ysz1g"))
> +         (modules '((guix build utils)))
> +         (snippet
> +          '(begin
> +             ;; Build with -O1 to work around <https://bugs.gnu.org/48368>.
> +             (substitute* "Makefile.am"
> +               (("compile --target")
> +                "compile -O1 --target")))))

This origin record can be simplified a bit by using inheritance:

(origin
  (inherit (package-source shepherd))
  (method git-fetch)
  (uri (git-reference [...]))
  (sha256 [...]))

'inherit' is not restricted to packages, it can be used for any record
type defined with (guix records)!

Greetings,
Maxime.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

[bug#54216] [PATCH v2] gnu: shepherd-for-guix: New package for use in Guix.

[Attila Lendvai]

> This origin record can be simplified a bit by using inheritance:
>
> (origin
> (inherit (package-source shepherd))
> (method git-fetch)
> (uri (git-reference [...]))
> (sha256 [...]))
>
> 'inherit' is not restricted to packages, it can be used for any record
> type defined with (guix records)!


oh, excellent point, thanks Maxime!

unfortunately, it won't help us much here, because one snippet modifies
Makefile.am, while the other Makefile.in.

once it's merged i'm planning to send another patch to staging that will clean
this up a little by building normal shepherd also from git. that patch will
include this inheritance.

--
• attila lendvai
• PGP: 963F 5D5F 45C7 DFCD 0A39
--
The truth cannot be told, only realized.

[bug#54216] [PATCH v2] gnu: shepherd-for-guix: New package for use in Guix.

[zimoun]

Hi Attila,

On Tue, 1 Mar 2022 at 20:42, Attila Lendvai <attila@lendvai.name> wrote:

> unfortunately, it won't help us much here, because one snippet modifies
> Makefile.am, while the other Makefile.in.

Guix does not have a clear line for patching (or I am not aware of the
update); what is going to source+snippet vs arguments+phase.  Pros and
cons for both; basically the question is what "guix build --source"
should return?

Option source+snippet means it returns the source of what Guix really
builds -- so many packages would not respect this rule of thumb.

Option arguments+phase means it returns the real unmodified upstream
source (modulo removal of non-free) -- so "guix shell -D foo" would
break for many packages.

Difficult tension. :-)  For most cases, not an unique answer; maybe
that's why we do not have a clear documentation. :-)  I just mention
it i.e., I am not saying you can do something. :-)  I am simply
pointing that Guix does not have a clear recommendation /
documentation where the patches should go; probably depending on their
nature.  Well, nothing related with your patch. :-)

That's said, personally, in this case, instead of having the Makefile*
patch in 'source', I would do the patching using a phase.

Cheers,
simon

[bug#54216] [PATCH v2] gnu: shepherd-for-guix: New package for use in Guix.

[Leo Famulari]

Origin snippets should only be used to remove nonfree things from the upstream source code. All other changes should use patch files or a build phase.

On Wed, Mar 2, 2022, at 04:14, zimoun wrote:
> Hi Attila,
>
> On Tue, 1 Mar 2022 at 20:42, Attila Lendvai <attila@lendvai.name> wrote:
>
>> unfortunately, it won't help us much here, because one snippet modifies
>> Makefile.am, while the other Makefile.in.
>
> Guix does not have a clear line for patching (or I am not aware of the
> update); what is going to source+snippet vs arguments+phase.  Pros and
> cons for both; basically the question is what "guix build --source"
> should return?
>
> Option source+snippet means it returns the source of what Guix really
> builds -- so many packages would not respect this rule of thumb.
>
> Option arguments+phase means it returns the real unmodified upstream
> source (modulo removal of non-free) -- so "guix shell -D foo" would
> break for many packages.
>
> Difficult tension. :-)  For most cases, not an unique answer; maybe
> that's why we do not have a clear documentation. :-)  I just mention
> it i.e., I am not saying you can do something. :-)  I am simply
> pointing that Guix does not have a clear recommendation /
> documentation where the patches should go; probably depending on their
> nature.  Well, nothing related with your patch. :-)
>
> That's said, personally, in this case, instead of having the Makefile*
> patch in 'source', I would do the patching using a phase.
>
> Cheers,
> simon

[bug#54216] [PATCH v2] gnu: shepherd-for-guix: New package for use in Guix.

[Maxime Devos]

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

Leo Famulari schreef op wo 02-03-2022 om 18:50 [-0500]:
> Origin snippets should only be used to remove nonfree things
> from the upstream source code. All other changes should use
> patch files or a build phase.

Why?  If it's a source code change and it fits an origin snippet,
why not an origin snippet?  Why would the source in Guix need to match
the source upstream?

Also, in this case, it was just copied from the 'shepherd' packages
with some slight adjustments.

Greetings,
Maxime. 

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

[bug#54216] [PATCH v2] gnu: shepherd-for-guix: New package for use in Guix.

[zimoun]

Hi Maxime,

On Thu, 3 Mar 2022 at 07:25, Maxime Devos <maximedevos@telenet.be> wrote:
> Leo Famulari schreef op wo 02-03-2022 om 18:50 [-0500]:

> > Origin snippets should only be used to remove nonfree things
> > from the upstream source code. All other changes should use
> > patch files or a build phase.
>
> Why?  If it's a source code change and it fits an origin snippet,
> why not an origin snippet?  Why would the source in Guix need to match
> the source upstream?

Because as I tried to explain here [1] ;-)

1: <https://issues.guix.gnu.org/54216#5>

a) the location of patches depending of their nature is **not well-documented**,
b) because it is a tension between two intentions,
c) and thus we never take the time to clearly document for consistency.

FWIW, it would be unfair for the patch to have the discussion here.

As Leo, I also think this source modification should go to a phase.


> Also, in this case, it was just copied from the 'shepherd' packages
> with some slight adjustments.

And I also think this recent commit
79be6a985799adc6d663890250f4fb7c12f015b4 introducing the 'snippet' is
unfortunate.  And it should be fixed, i.e., move the substitution to a
phase.


Cheers,
simon

[bug#54216] [PATCH v3] gnu: shepherd-for-guix: New package for use in Guix.

[Attila Lendvai]

This also updates shepherd-for-guix to the latest commit, so that the two
variants have a different version.

* gnu/packages/admin.scm (shepherd-for-guix): New variable.
---

v3: as per the discussion, implement the patching through a phase.

once it gets merged, i'll prepare another patch to staging that
cleans this all up by moving most of this into the parent package,
and also switch it to build from git.

 gnu/packages/admin.scm    | 48 +++++++++++++++++++++++++++++++++++++++
 gnu/services/shepherd.scm |  2 +-
 2 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/admin.scm b/gnu/packages/admin.scm
index c8f91aab0d..dbceaca5e5 100644
--- a/gnu/packages/admin.scm
+++ b/gnu/packages/admin.scm
@@ -307,6 +307,54 @@ (define-public shepherd
     (license license:gpl3+)
     (home-page "https://www.gnu.org/software/shepherd/")))
 
+;; This is the Shepherd package used by Guix.  This package variant
+;; allows us to add new features and fix bugs in Shepherd and use the
+;; new features in Guix, without having to go through the 'staging'
+;; branch, and without having to wait for a new release, see
+;; [TODO] in the manual.
+(define-public shepherd-for-guix
+  (let* ((version "0.8.1")
+         ;; If it's an unreleased commit:
+         (commit "7c380590164ea8ee40de46059d07e08a48963577")
+         ;; Use the below form if it's a release, and also set REVISION to #f.
+         ;; (commit (string-append "v" version))
+         (revision "1"))
+    (package
+      (inherit shepherd)
+      (version (if revision
+                   (git-version version revision commit)
+                   version))
+      (source
+       (origin
+         (inherit (package-source shepherd))
+         (method git-fetch)
+         (uri (git-reference
+               ;; Build from git and add Autotools inputs, to make developing
+               ;; Shepherd easier. It enables easier package inheritance.
+               (url "https://git.savannah.gnu.org/git/shepherd.git/")
+               (commit commit)))
+         (snippet #f) ; TODO delete this once parent is refactored.
+         (sha256
+          (base32
+           "1hgkbl3fyzwi5vw63kbkswnf3viyfl52c5dzkx7vbkr4sj5ysz1g"))))
+      ;; TODO merge this back into the parent with a patch to staging.
+      (arguments
+       (append
+        (package-arguments shepherd)
+        '(#:phases
+          (modify-phases %standard-phases
+            (add-after 'unpack 'patch-source
+              (lambda _
+                ;; Build with -O1 to work around <https://bugs.gnu.org/48368>.
+                (substitute* "Makefile.am"
+                  (("compile --target")
+                   "compile -O1 --target"))))))))
+      (native-inputs
+       (modify-inputs (package-native-inputs shepherd)
+         (prepend autoconf automake gettext-minimal help2man texinfo)))
+      (description "A package variant for use in Guix. It helps lowering
+the build time of Guix when working on Shepherd."))))
+
 (define-public guile2.2-shepherd
   (package
     (inherit shepherd)
diff --git a/gnu/services/shepherd.scm b/gnu/services/shepherd.scm
index b44dbf9d9f..991194ffe6 100644
--- a/gnu/services/shepherd.scm
+++ b/gnu/services/shepherd.scm
@@ -88,7 +88,7 @@ (define-record-type* <shepherd-configuration>
   shepherd-configuration make-shepherd-configuration
   shepherd-configuration?
   (shepherd shepherd-configuration-shepherd
-            (default shepherd)) ; file-like
+            (default shepherd-for-guix)) ; file-like
   (services shepherd-configuration-services
             (default '()))) ; list of <shepherd-service>
 
-- 
2.34.0

[bug#54216] [PATCH staging] gnu: shepherd: Build it from git, and clean up shepherd-for-guix.

[Attila Lendvai]

This cleanup is in a seprate patch because this one must go through staging.
---

as promised, this is the cleanup patch for staging.

 gnu/packages/admin.scm | 70 +++++++++++++++++-------------------------
 1 file changed, 28 insertions(+), 42 deletions(-)

diff --git a/gnu/packages/admin.scm b/gnu/packages/admin.scm
index dbceaca5e5..bcfef124d7 100644
--- a/gnu/packages/admin.scm
+++ b/gnu/packages/admin.scm
@@ -270,26 +270,31 @@ (define-public shepherd
   (package
     (name "shepherd")
     (version "0.8.1")
-    (source (origin
-              (method url-fetch)
-              (uri (string-append "mirror://gnu/shepherd/shepherd-"
-                                  version ".tar.gz"))
-              (sha256
-               (base32
-                "0x9zr0x3xvk4qkb6jnda451d5iyrl06cz1bjzjsm0lxvjj3fabyk"))
-              (modules '((guix build utils)))
-              (snippet
-               '(begin
-                  ;; Build with -O1 to work around <https://bugs.gnu.org/48368>.
-                  (substitute* "Makefile.in"
-                    (("compile --target")
-                     "compile -O1 --target"))))))
+    (source
+     (origin
+       (method git-fetch)
+       (uri (git-reference
+             (url "https://git.savannah.gnu.org/git/shepherd.git/")
+             (commit (string-append "v" version))))
+       (file-name (git-file-name name version))
+       (sha256
+        (base32 "1hgkbl3fyzwi5vw63kbkswnf3viyfl52c5dzkx7vbkr4sj5ysz1g"))
+       (modules '((guix build utils)))))
     (build-system gnu-build-system)
     (arguments
      '(#:configure-flags '("--localstatedir=/var")
-       #:make-flags '("GUILE_AUTO_COMPILE=0")))
+       #:make-flags '("GUILE_AUTO_COMPILE=0")
+       #:phases
+       (modify-phases %standard-phases
+         ;; TODO delete this once Guile is updated to v3.0.8+
+         (add-after 'unpack 'patch-source
+           (lambda _
+             ;; Build with -O1 to work around <https://bugs.gnu.org/48368>.
+             (substitute* "Makefile.am"
+               (("compile --target")
+                "compile -O1 --target")))))))
     (native-inputs
-     (list pkg-config
+     (list autoconf automake gettext-minimal help2man texinfo pkg-config
            ;; This is the Guile we use as a cross-compiler...
            guile-3.0))
     (inputs
@@ -314,45 +319,26 @@ (define-public shepherd
 ;; [TODO] in the manual.
 (define-public shepherd-for-guix
   (let* ((version "0.8.1")
-         ;; If it's an unreleased commit:
+         ;; A commit string, or #f if it's a release.
          (commit "7c380590164ea8ee40de46059d07e08a48963577")
-         ;; Use the below form if it's a release, and also set REVISION to #f.
-         ;; (commit (string-append "v" version))
-         (revision "1"))
+         (revision "1") ; Only relevant when COMMIT is specified.
+         (git-ref (or commit (string-append "v" version))))
     (package
       (inherit shepherd)
-      (version (if revision
+      (version (if commit
                    (git-version version revision commit)
                    version))
       (source
        (origin
          (inherit (package-source shepherd))
-         (method git-fetch)
          (uri (git-reference
                ;; Build from git and add Autotools inputs, to make developing
                ;; Shepherd easier. It enables easier package inheritance.
                (url "https://git.savannah.gnu.org/git/shepherd.git/")
-               (commit commit)))
-         (snippet #f) ; TODO delete this once parent is refactored.
+               (commit git-ref)))
          (sha256
-          (base32
-           "1hgkbl3fyzwi5vw63kbkswnf3viyfl52c5dzkx7vbkr4sj5ysz1g"))))
-      ;; TODO merge this back into the parent with a patch to staging.
-      (arguments
-       (append
-        (package-arguments shepherd)
-        '(#:phases
-          (modify-phases %standard-phases
-            (add-after 'unpack 'patch-source
-              (lambda _
-                ;; Build with -O1 to work around <https://bugs.gnu.org/48368>.
-                (substitute* "Makefile.am"
-                  (("compile --target")
-                   "compile -O1 --target"))))))))
-      (native-inputs
-       (modify-inputs (package-native-inputs shepherd)
-         (prepend autoconf automake gettext-minimal help2man texinfo)))
-      (description "A package variant for use in Guix. It helps lowering
+          (base32 "1hgkbl3fyzwi5vw63kbkswnf3viyfl52c5dzkx7vbkr4sj5ysz1g"))))
+      (description "A package variant for use in Guix.  It helps lowering
 the build time of Guix when working on Shepherd."))))
 
 (define-public guile2.2-shepherd
-- 
2.34.0

[bug#54216] [PATCH staging] gnu: shepherd: Build it from git, and clean up shepherd-for-guix.

[Maxime Devos]

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

zimoun schreef op wo 02-03-2022 om 10:14 [+0100]:
> Hi Attila,
> 
> On Tue, 1 Mar 2022 at 20:42, Attila Lendvai <attila@lendvai.name> wrote:
> 
> > unfortunately, it won't help us much here, because one snippet modifies
> > Makefile.am, while the other Makefile.in.
> 
> Guix does not have a clear line for patching (or I am not aware of the
> update); what is going to source+snippet vs arguments+phase.  Pros and
> cons for both; basically the question is what "guix build --source"
> should return?
> 
> Option source+snippet means it returns the source of what Guix really
> builds

Seems like a pro for source+snippet (or source+patch, though that
would be a bit more verbose) to me.

>  -- so many packages would not respect this rule of thumb.

In that case, it seems like there are plenty of package definitions to
improve!

> Option arguments+phase means it returns the real unmodified upstream
> source (modulo removal of non-free) -- so "guix shell -D foo" would
> break for many packages.

I assume you meant "guix build --source foo"?
This seems like a con for "phases+arguments" to me.
Sometimes, to hack on software, I download the source code with
"guix build --source the-package", unpack it and do
"guix shell -D the-package".

I don't see much value in returning the unmodified upstream source.
Especially since in this case the modified source fixes a bug
(well, works-around a Guile bug). As long as it's source code, it
builds, it doesn't do things like bundling, including binaries or
non-free things, and it avoids being Guix-specific and fixes known
bugs, it seems good source code to me.

Also, most packages don't modify upstream code, so I don't see
the ‘would break for many packages’ here ...

> Difficult tension. :-)

As implied from my explanations above, I don't see any tension here.

> That's said, personally, in this case, instead of having the Makefile*
> patch in 'source', I would do the patching using a phase.

It's ‘merely’ setting some compilation flags, so maybe.
Both options seem fine to me here but I don't see a point to _moving_
from the snippet-shed to phase-shed.


Greetings,
Maxime.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

[bug#54216] [PATCH staging] gnu: shepherd: Build it from git, and clean up shepherd-for-guix.

[zimoun]

Hi Maxime,

You are doing the discussion and we should stay focus on this patch.
:-)  Leo and me are considering better to have the substitution in a
phase, FWIW, and Attila submitted a new version.  Let's move on, again
for what my opinion is worth here.

I am totally fine to have the discussion on guix-devel or elsewhere --
I disagree on some points but I will not comment here; it appears to
me not fair to pollute by lengthy arguments the nice patch by Attila,
IMHO.


Cheers,
simon

[bug#54216] [PATCH shepherd staging v2] gnu: shepherd: Build it from git, and clean up shepherd-for-guix.

[Attila Lendvai]

* gnu/packages/admin.scm (shepherd): Change SOURCE to point to the git repository.
(shepherd-for-guix): Simplify by moving most of the customizations into the above.
---

i accidentally used the wrong hash in the first version. v2 fixes it, and
also adds a more conformant commit message.

my server is now building/reconfiguring into this codebase, that's how i've
noticed my mistake. no idea why it didn't surface earlier, probably due
to an unfortunate ordering of the test builds and the copy-paste mistake.

 gnu/packages/admin.scm | 70 +++++++++++++++++-------------------------
 1 file changed, 28 insertions(+), 42 deletions(-)

diff --git a/gnu/packages/admin.scm b/gnu/packages/admin.scm
index dbceaca5e5..882dc9f121 100644
--- a/gnu/packages/admin.scm
+++ b/gnu/packages/admin.scm
@@ -270,26 +270,31 @@ (define-public shepherd
   (package
     (name "shepherd")
     (version "0.8.1")
-    (source (origin
-              (method url-fetch)
-              (uri (string-append "mirror://gnu/shepherd/shepherd-"
-                                  version ".tar.gz"))
-              (sha256
-               (base32
-                "0x9zr0x3xvk4qkb6jnda451d5iyrl06cz1bjzjsm0lxvjj3fabyk"))
-              (modules '((guix build utils)))
-              (snippet
-               '(begin
-                  ;; Build with -O1 to work around <https://bugs.gnu.org/48368>.
-                  (substitute* "Makefile.in"
-                    (("compile --target")
-                     "compile -O1 --target"))))))
+    (source
+     (origin
+       (method git-fetch)
+       (uri (git-reference
+             (url "https://git.savannah.gnu.org/git/shepherd.git/")
+             (commit (string-append "v" version))))
+       (file-name (git-file-name name version))
+       (sha256
+        (base32 "109ha9rk4ycqcmx9cddlbym92c3fvbwp12q9p42h8sg8vr367w5j"))
+       (modules '((guix build utils)))))
     (build-system gnu-build-system)
     (arguments
      '(#:configure-flags '("--localstatedir=/var")
-       #:make-flags '("GUILE_AUTO_COMPILE=0")))
+       #:make-flags '("GUILE_AUTO_COMPILE=0")
+       #:phases
+       (modify-phases %standard-phases
+         ;; TODO delete this once Guile is updated to v3.0.8+
+         (add-after 'unpack 'patch-source
+           (lambda _
+             ;; Build with -O1 to work around <https://bugs.gnu.org/48368>.
+             (substitute* "Makefile.am"
+               (("compile --target")
+                "compile -O1 --target")))))))
     (native-inputs
-     (list pkg-config
+     (list autoconf automake gettext-minimal help2man texinfo pkg-config
            ;; This is the Guile we use as a cross-compiler...
            guile-3.0))
     (inputs
@@ -314,45 +319,26 @@ (define-public shepherd
 ;; [TODO] in the manual.
 (define-public shepherd-for-guix
   (let* ((version "0.8.1")
-         ;; If it's an unreleased commit:
+         ;; A commit string, or #f if it's a release.
          (commit "7c380590164ea8ee40de46059d07e08a48963577")
-         ;; Use the below form if it's a release, and also set REVISION to #f.
-         ;; (commit (string-append "v" version))
-         (revision "1"))
+         (revision "1") ; Only relevant when COMMIT is specified.
+         (git-ref (or commit (string-append "v" version))))
     (package
       (inherit shepherd)
-      (version (if revision
+      (version (if commit
                    (git-version version revision commit)
                    version))
       (source
        (origin
          (inherit (package-source shepherd))
-         (method git-fetch)
          (uri (git-reference
                ;; Build from git and add Autotools inputs, to make developing
                ;; Shepherd easier. It enables easier package inheritance.
                (url "https://git.savannah.gnu.org/git/shepherd.git/")
-               (commit commit)))
-         (snippet #f) ; TODO delete this once parent is refactored.
+               (commit git-ref)))
          (sha256
-          (base32
-           "1hgkbl3fyzwi5vw63kbkswnf3viyfl52c5dzkx7vbkr4sj5ysz1g"))))
-      ;; TODO merge this back into the parent with a patch to staging.
-      (arguments
-       (append
-        (package-arguments shepherd)
-        '(#:phases
-          (modify-phases %standard-phases
-            (add-after 'unpack 'patch-source
-              (lambda _
-                ;; Build with -O1 to work around <https://bugs.gnu.org/48368>.
-                (substitute* "Makefile.am"
-                  (("compile --target")
-                   "compile -O1 --target"))))))))
-      (native-inputs
-       (modify-inputs (package-native-inputs shepherd)
-         (prepend autoconf automake gettext-minimal help2man texinfo)))
-      (description "A package variant for use in Guix. It helps lowering
+          (base32 "1hgkbl3fyzwi5vw63kbkswnf3viyfl52c5dzkx7vbkr4sj5ysz1g"))))
+      (description "A package variant for use in Guix.  It helps lowering
 the build time of Guix when working on Shepherd."))))
 
 (define-public guile2.2-shepherd
-- 
2.34.0

[bug#54216] [PATCH v2] gnu: shepherd-for-guix: New package for use in Guix.

[Leo Famulari]

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

On Thu, Mar 03, 2022 at 07:25:22AM +0100, Maxime Devos wrote:
> Leo Famulari schreef op wo 02-03-2022 om 18:50 [-0500]:
> > Origin snippets should only be used to remove nonfree things
> > from the upstream source code. All other changes should use
> > patch files or a build phase.
> 
> Why?  If it's a source code change and it fits an origin snippet,
> why not an origin snippet?  Why would the source in Guix need to match
> the source upstream?

`guix build --source` is a tool to provide freely licensed source code
to be used for any purpose, including building on systems besides Guix.

Using the Guix tools, there is no way to access the upstream source code
without applying the snippets. The reason for that is that the origin
snippet mechanism was introduced specifically to remove non-free
components without making it easy to reverse the transformation.

Compare that to patch files, which are easily reversed, and build
phases, which do not apply to `guix build --source`.

So, we have to be careful when using snippets, to ensure that the result
of `guix build --source` is useful on any system, not just Guix.

More info:
https://guix.gnu.org/manual/en/html_node/Snippets-versus-Phases.html

Please let me know if these guidelines are still unclear.

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

gnu: shepherd: patch, snippet or phase

[Maxime Devos]

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

(as implied per zimoun's previous mail (‘FWIW, it would be unfair for
the patch to have the discussion here’), moved to guix-devel)

Leo Famulari schreef op za 05-03-2022 om 16:13 [-0500]:
> On Thu, Mar 03, 2022 at 07:25:22AM +0100, Maxime Devos wrote:
> > Leo Famulari schreef op wo 02-03-2022 om 18:50 [-0500]:
> > > Origin snippets should only be used to remove nonfree things
> > > from the upstream source code. All other changes should use
> > > patch files or a build phase.
> > 
> > Why?  If it's a source code change and it fits an origin snippet,
> > why not an origin snippet?  Why would the source in Guix need to
> match
> > the source upstream?
> 
> `guix build --source` is a tool to provide freely licensed source
> code
> to be used for any purpose, including building on systems besides
> Guix.
> 
> Using the Guix tools, there is no way to access the upstream source
> code
> without applying the snippets. The reason for that is that the origin
> snippet mechanism was introduced specifically to remove non-free
> components without making it easy to reverse the transformation.

It might be introduced for removing non-free components, that doesn't
mean it cannot be used for more.  Also, I don't see the point of ease
of reversing here. It's trivial to reverse the transformation induced
by the snippet: just delete the snippet in a git checkout.

> Compare that to patch files, which are easily reversed,

Removing a patch file by removing it from the 'patches' field is easy,
as easy as removing a snippet.  I assume you meant the additional
condition ‘... using only CLI tools’?

In that case, you have to: (1) run "guix build --source ..." to compute
the patched source (and maybe unpack), then (2) somehow know there's a
patch to revert, (3) clone guix, (4) find the location of the patch in
the git repo, (5) figure out the invocation of 'patch' command, and
possibly (6) repack the depatched source.  

As such, reversing patch files seems rather difficult to me, it seems
much simpler to just delete the patch inside Guix, and then we might as
well have used a snippet originally.

>  and build
> phases, which do not apply to `guix build --source`.
> 
> So, we have to be careful when using snippets, to ensure that the
> result
> of `guix build --source` is useful on any system, not just Guix.
> 
> More info:
> https://guix.gnu.org/manual/en/html_node/Snippets-versus-Phases.html
> 
> Please let me know if these guidelines are still unclear.

I am aware of the guideline of keeping the source usable outside Guix
systems.  AFAICT, in this case, the snippet modifying
Makefile.am/Makefile.in keeps the source usable on non-Guix systems.
In fact, it makes the source _more_ usable, both on Guix and non-Guix,
by working-around a Guile 3.0.5 compiler bug.  So I don't see any
problems here.

Greetings,
Maxime.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

[bug#54216] [PATCH v2] gnu: shepherd-for-guix: New package for use in Guix.

[Maxime Devos]

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

Leo Famulari schreef op za 05-03-2022 om 16:13 [-0500]:
> [...]

Replied to on guix-devel@ (see ‘gnu: shepherd: patch, snippet or
phase’).

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

[bug#54216] [PATCH v4] gnu: shepherd-for-guix: New package for use in Guix.

[Attila Lendvai]

This also updates shepherd-for-guix to the latest commit, so that the two
variants have a different version.

* gnu/packages/admin.scm (shepherd-for-guix): New variable.
---

v4: update to build the latest shepherd that now contains
the #:resource-limits commit.

 gnu/packages/admin.scm    | 49 +++++++++++++++++++++++++++++++++++++++
 gnu/services/shepherd.scm |  2 +-
 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/admin.scm b/gnu/packages/admin.scm
index c8f91aab0d..27fc33e990 100644
--- a/gnu/packages/admin.scm
+++ b/gnu/packages/admin.scm
@@ -307,6 +307,55 @@ (define-public shepherd
     (license license:gpl3+)
     (home-page "https://www.gnu.org/software/shepherd/")))
 
+;; This is the Shepherd package used by Guix.  This package variant
+;; allows us to add new features and fix bugs in Shepherd and use the
+;; new features in Guix, without having to go through the 'staging'
+;; branch, and without having to wait for a new release, see
+;; [TODO] in the manual.
+(define-public shepherd-for-guix
+  (let* ((version "0.8.1")
+         ;; If it's an unreleased commit:
+         (commit "3ee9a7193d73821d6f1dd76a745ed5e4bb1a78c8")
+         ;; Use the below form if it's a release, and also set REVISION to #f.
+         ;; (commit (string-append "v" version))
+         (revision "1"))
+    (package
+      (inherit shepherd)
+      (version (if revision
+                   (git-version version revision commit)
+                   version))
+      (source
+       (origin
+         (inherit (package-source shepherd))
+         (method git-fetch)
+         (uri (git-reference
+               ;; Build from git and add Autotools inputs, to make developing
+               ;; Shepherd easier. It enables easier package inheritance.
+               (url "https://git.savannah.gnu.org/git/shepherd.git/")
+               (commit commit)))
+         (snippet #f) ; TODO delete this once parent is refactored.
+         (sha256
+          (base32
+           "07zaazw33p98n38j9hgjaipjvbh9mf0x3b5nbri5nyrxbgn0vvvb"))))
+      ;; TODO merge this back into the parent with a patch to staging.
+      (arguments
+       (append
+        (package-arguments shepherd)
+        '(#:phases
+          (modify-phases %standard-phases
+            (delete 'strip) ; Avoid some warnings from stripping .go files.
+            (add-after 'unpack 'patch-source
+              (lambda _
+                ;; Build with -O1 to work around <https://bugs.gnu.org/48368>.
+                (substitute* "Makefile.am"
+                  (("compile --target")
+                   "compile -O1 --target"))))))))
+      (native-inputs
+       (modify-inputs (package-native-inputs shepherd)
+         (prepend autoconf automake gettext-minimal help2man texinfo)))
+      (description "A package variant for use in Guix. It helps lowering
+the build time of Guix when working on Shepherd."))))
+
 (define-public guile2.2-shepherd
   (package
     (inherit shepherd)
diff --git a/gnu/services/shepherd.scm b/gnu/services/shepherd.scm
index b44dbf9d9f..991194ffe6 100644
--- a/gnu/services/shepherd.scm
+++ b/gnu/services/shepherd.scm
@@ -88,7 +88,7 @@ (define-record-type* <shepherd-configuration>
   shepherd-configuration make-shepherd-configuration
   shepherd-configuration?
   (shepherd shepherd-configuration-shepherd
-            (default shepherd)) ; file-like
+            (default shepherd-for-guix)) ; file-like
   (services shepherd-configuration-services
             (default '()))) ; list of <shepherd-service>
 
-- 
2.34.0

[bug#54216] [PATCH] gnu: shepherd-for-guix: New package for use in Guix.

[Ludovic Courtès]

Hi Attila,

Attila Lendvai <attila@lendvai.name> skribis:

> This also updates shepherd-for-guix to the latest commit, so that the two
> variants have a different version.
>
> * gnu/packages/admin.scm (shepherd-for-guix): New variable.

Apologies for nor chiming in earlier.

Normally Guix provides packages for releases rather than snapshots.

In this case, since we’re also upstream, we could make a new Shepherd
release and update the ‘shepherd’ package in Guix.  I wasn’t sure this
was necessary yet, but may it is?  WDYT?

Thanks,
Ludo’.

[bug#54216] [PATCH] gnu: shepherd-for-guix: New package for use in Guix.

[zimoun]

Hi Ludo,

On Mon, 21 Mar 2022 at 17:03, Ludovic Courtès <ludo@gnu.org> wrote:

> Normally Guix provides packages for releases rather than snapshots.

Yes, but fetching from Git instead of URL tarball does not mean not a
release. ;-)  Well, fetching from Git would ease the use for snapshots
via transformation.  Other said, the source can point to a Git tagged
release, then people interested in very recent Shepherd can just apply
a transformation.

> In this case, since we’re also upstream, we could make a new Shepherd
> release and update the ‘shepherd’ package in Guix.  I wasn’t sure this
> was necessary yet, but may it is?  WDYT?

I think the switch of source would ease the cycle as Attila is proposing.


Cheers,
simon

[bug#54216] [PATCH staging v3] gnu: shepherd: Build it from git, and clean up shepherd-for-guix.

[Attila Lendvai]

* gnu/packages/admin.scm (shepherd): Change SOURCE to point to the git repository.
(shepherd-for-guix): Simplify by moving most of the customizations into the above.
---

also updated/rebased the cleanup patch to point to the commit that
includes #:resource-limit in Shepherd.

 gnu/packages/admin.scm | 72 +++++++++++++++++-------------------------
 1 file changed, 29 insertions(+), 43 deletions(-)

diff --git a/gnu/packages/admin.scm b/gnu/packages/admin.scm
index 27fc33e990..3f08447ba4 100644
--- a/gnu/packages/admin.scm
+++ b/gnu/packages/admin.scm
@@ -270,26 +270,32 @@ (define-public shepherd
   (package
     (name "shepherd")
     (version "0.8.1")
-    (source (origin
-              (method url-fetch)
-              (uri (string-append "mirror://gnu/shepherd/shepherd-"
-                                  version ".tar.gz"))
-              (sha256
-               (base32
-                "0x9zr0x3xvk4qkb6jnda451d5iyrl06cz1bjzjsm0lxvjj3fabyk"))
-              (modules '((guix build utils)))
-              (snippet
-               '(begin
-                  ;; Build with -O1 to work around <https://bugs.gnu.org/48368>.
-                  (substitute* "Makefile.in"
-                    (("compile --target")
-                     "compile -O1 --target"))))))
+    (source
+     (origin
+       (method git-fetch)
+       (uri (git-reference
+             (url "https://git.savannah.gnu.org/git/shepherd.git/")
+             (commit (string-append "v" version))))
+       (file-name (git-file-name name version))
+       (sha256
+        (base32 "109ha9rk4ycqcmx9cddlbym92c3fvbwp12q9p42h8sg8vr367w5j"))
+       (modules '((guix build utils)))))
     (build-system gnu-build-system)
     (arguments
      '(#:configure-flags '("--localstatedir=/var")
-       #:make-flags '("GUILE_AUTO_COMPILE=0")))
+       #:make-flags '("GUILE_AUTO_COMPILE=0")
+       #:phases
+       (modify-phases %standard-phases
+         (delete 'strip) ; Avoid some warnings from stripping .go files.
+         ;; TODO delete this once Guile is updated to v3.0.8+
+         (add-after 'unpack 'patch-source
+           (lambda _
+             ;; Build with -O1 to work around <https://bugs.gnu.org/48368>.
+             (substitute* "Makefile.am"
+               (("compile --target")
+                "compile -O1 --target")))))))
     (native-inputs
-     (list pkg-config
+     (list autoconf automake gettext-minimal help2man texinfo pkg-config
            ;; This is the Guile we use as a cross-compiler...
            guile-3.0))
     (inputs
@@ -314,46 +320,26 @@ (define-public shepherd
 ;; [TODO] in the manual.
 (define-public shepherd-for-guix
   (let* ((version "0.8.1")
-         ;; If it's an unreleased commit:
+         ;; A commit string, or #f if it's a release.
          (commit "3ee9a7193d73821d6f1dd76a745ed5e4bb1a78c8")
-         ;; Use the below form if it's a release, and also set REVISION to #f.
-         ;; (commit (string-append "v" version))
-         (revision "1"))
+         (revision "1") ; Only relevant when COMMIT is specified.
+         (git-ref (or commit (string-append "v" version))))
     (package
       (inherit shepherd)
-      (version (if revision
+      (version (if commit
                    (git-version version revision commit)
                    version))
       (source
        (origin
          (inherit (package-source shepherd))
-         (method git-fetch)
          (uri (git-reference
                ;; Build from git and add Autotools inputs, to make developing
                ;; Shepherd easier. It enables easier package inheritance.
                (url "https://git.savannah.gnu.org/git/shepherd.git/")
-               (commit commit)))
-         (snippet #f) ; TODO delete this once parent is refactored.
+               (commit git-ref)))
          (sha256
-          (base32
-           "07zaazw33p98n38j9hgjaipjvbh9mf0x3b5nbri5nyrxbgn0vvvb"))))
-      ;; TODO merge this back into the parent with a patch to staging.
-      (arguments
-       (append
-        (package-arguments shepherd)
-        '(#:phases
-          (modify-phases %standard-phases
-            (delete 'strip) ; Avoid some warnings from stripping .go files.
-            (add-after 'unpack 'patch-source
-              (lambda _
-                ;; Build with -O1 to work around <https://bugs.gnu.org/48368>.
-                (substitute* "Makefile.am"
-                  (("compile --target")
-                   "compile -O1 --target"))))))))
-      (native-inputs
-       (modify-inputs (package-native-inputs shepherd)
-         (prepend autoconf automake gettext-minimal help2man texinfo)))
-      (description "A package variant for use in Guix. It helps lowering
+          (base32 "07zaazw33p98n38j9hgjaipjvbh9mf0x3b5nbri5nyrxbgn0vvvb"))))
+      (description "A package variant for use in Guix.  It helps lowering
 the build time of Guix when working on Shepherd."))))
 
 (define-public guile2.2-shepherd
-- 
2.34.0

[bug#54216] [PATCH staging v3] gnu: shepherd: Build it from git, and clean up shepherd-for-guix.

[Attila Lendvai]

just a quick idea:

another solution could be to introduce a shepherd-tools package that only contains the command line tools, and packages would depend on this one, not shepherd itself.

that way shepherd could be updated cheaply, and as long as the socket interface remains compatible, the shepherd-tools package doesn't need to be updated (which is expensive in the current dependency graph).

--
• attila lendvai
• PGP: 963F 5D5F 45C7 DFCD 0A39
--
“I cannot imagine a torture more vicious and terrifying than to realize in the face of one’s final days that one wasted his entire life trying to please the plethora of idiots around him, instead of educating them and himself and molding tomorrow for the better.”
	— Brandon Smith, 'Alt-Market blog'

[bug#54216] [PATCH staging v3] gnu: shepherd: Build it from git, and clean up shepherd-for-guix.

[Attila Lendvai]

i really wish if this patch progressed, and if Guix master included the #:resource-limits commit sooner than later.

Ludo, can you please point out more precisely what is your concern? i'd be happy to reshape the patches accordingly.

1) including an unreleased Shepherd in Guix

2) building Shepherd from git instead of a release tarball

3) adding a secondary shepherd package

4) the currently proposed names/setup of the secondary shepherd package

5) ?

the rationale for this patch is discussed above, but to sum it up:

- several packages depend on shepherd, therefore updating it results in a long recompile (currently shepherd patches must also go into staging). this greatly increases the edit-compile-test cycle of hacking on shepherd and testing it "in place" using `guix system vm`.

- using git as source plays better with SWH and simplifies building a different version of shepherd.

--
• attila lendvai
• PGP: 963F 5D5F 45C7 DFCD 0A39
--
Every lie is a debt to the truth.

[bug#54216] [PATCH staging v3] gnu: shepherd: Build it from git, and clean up shepherd-for-guix.

[pelzflorian (Florian Pelz)]

On Tue, Mar 22, 2022 at 01:48:53PM +0000, Attila Lendvai wrote:
> - several packages depend on shepherd, therefore updating it results in a long recompile (currently shepherd patches must also go into staging). this greatly increases the edit-compile-test cycle of hacking on shepherd and testing it "in place" using `guix system vm`.

I'm not sure but have you checked if grafting is an option alternative to this shepherd-next package?

See <https://issues.guix.gnu.org/54199#3>.

The rest of the rationale may be reason enough for this patch though.

Regards,
Florian

[bug#54216] [PATCH staging v3] gnu: shepherd: Build it from git, and clean up shepherd-for-guix.

[Attila Lendvai]

> I'm not sure but have you checked if grafting is an option alternative to this shepherd-next package?
>
> See https://issues.guix.gnu.org/54199#3.

FTR, no, i haven't yet. the reason is that i still haven't
internalized a model of how grafting works.

i'm happy to refocus my attention to grafting if someone with a better
bird's eye view perspective tells me that it's the preferred solution
in this context.

your pointer is much appreciated, though! and recorded in my notes,
too.

--
• attila lendvai
• PGP: 963F 5D5F 45C7 DFCD 0A39
--
“Surprise exists in the map, not in the territory.”
	— Eliezer Yudkowsky (1979–)

Re: gnu: shepherd: patch, snippet or phase

[zimoun]

Hi Maxime,

On Sat, 05 Mar 2022 at 22:49, Maxime Devos <maximedevos@telenet.be> wrote:
> (as implied per zimoun's previous mail (‘FWIW, it would be unfair for
> the patch to have the discussion here’), moved to guix-devel)

Thanks. :-)

> Leo Famulari schreef op za 05-03-2022 om 16:13 [-0500]:
>> On Thu, Mar 03, 2022 at 07:25:22AM +0100, Maxime Devos wrote:
>> > Leo Famulari schreef op wo 02-03-2022 om 18:50 [-0500]:
>> > > Origin snippets should only be used to remove nonfree things
>> > > from the upstream source code. All other changes should use
>> > > patch files or a build phase.
>> > 
>> > Why?  If it's a source code change and it fits an origin snippet,
>> > why not an origin snippet?  Why would the source in Guix need to
>> match
>> > the source upstream?
>> 
>> `guix build --source` is a tool to provide freely licensed source
>> code
>> to be used for any purpose, including building on systems besides
>> Guix.
>> 
>> Using the Guix tools, there is no way to access the upstream source
>> code
>> without applying the snippets. The reason for that is that the origin
>> snippet mechanism was introduced specifically to remove non-free
>> components without making it easy to reverse the transformation.
>
> It might be introduced for removing non-free components, that doesn't
> mean it cannot be used for more.  Also, I don't see the point of ease
> of reversing here. It's trivial to reverse the transformation induced
> by the snippet: just delete the snippet in a git checkout.

Well, the point is the FSDG [1] frame, I guess.  From my understanding,
when --source had been introduced, it was a countermeasure to be able to
use hybrid source and still be compliant with an interpretation of: «A
free system distribution must not steer users towards obtaining any
nonfree information for practical use, or encourage them to do so.»

Therefore, using Guix tools, e.g., guix build --source, it is not easy
to reverse what ’snippet’ does.

I would not say it is trivial to reverse the transformation because the
user needs to run “guix edit”, then reassemble the URL, then fetch.
Otherwise, yes the user could go to the Guix repo, remove the snippet,
then run “guix shell -D guix”, do somehow “./pre-inst-env guix …”.

Well, I do not consider these steps “trivial”.  And if one user does
that, somehow they really want to obtain nonfree information. :-)



1: <https://www.gnu.org/distros/free-system-distribution-guidelines.en.html>

>> Compare that to patch files, which are easily reversed,
>
> Removing a patch file by removing it from the 'patches' field is easy,
> as easy as removing a snippet.  I assume you meant the additional
> condition ‘... using only CLI tools’?

Yes, somehow.


> I am aware of the guideline of keeping the source usable outside Guix
> systems.  AFAICT, in this case, the snippet modifying
> Makefile.am/Makefile.in keeps the source usable on non-Guix systems.
> In fact, it makes the source _more_ usable, both on Guix and non-Guix,
> by working-around a Guile 3.0.5 compiler bug.  So I don't see any
> problems here.

Well, the question without consensus is what “guix build --source”
should return?

  a) The source of what “guix build” concretely builds?
  b) The source of upstream (modulo the removal of nonfree part)?

The aim is to be as close as possible as b), IMHO.  The exception of
patches could be discussed. :-)


Back to Shepherd, because the question is originally from patch#54216
[2], the initial snippet was turning a flag:

--8<---------------cut here---------------start------------->8---
+         (snippet
+          '(begin
+             ;; Build with -O1 to work around <https://bugs.gnu.org/48368>.
+             (substitute* "Makefile.am"
+               (("compile --target")
+                "compile -O1 --target"))))))
--8<---------------cut here---------------end--------------->8---

Somehow, the snippet could be considered as a “patch“.  And, in the same
time, the upstream source will not compile without this ’-O1’, IIUC.

However, since “we“ are in the same time upstream and downstream, we
could fix that without introducing this kind of snippet.

Last, because the package is for building with Guix, then it seems more
appropriate to have the substitution in the ’arguments’, as v3 [3] is
doing.


2: <http://issues.guix.gnu.org/issue/54216>
3: <https://issues.guix.gnu.org/issue/54216#19>



Cheers,
simon

Re: gnu: shepherd: patch, snippet or phase

[Zhu Zihao]

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



IMO, It's cumbersome to add patches in build phase, you have to add a
new phase, and write something like:

```
(invoke "patch" "-p1" ...)
```

So packager will prefer to add it in the `patches` slot of <origin>
struct. I'd like to see if we have some build procedure like
`apply-patches` to help packager reduce the misnomer of `patches` slot.

Or we can add a new keyword argument #:patches-for-build to
gnu-build-system. For example

```
(package
  (name "XXX")
  (source
   (origin
    ...
    (patches (search-patches "...."))))
  (arguments
   (list
   ;; This patch only used when building the package 
   #:patches-for-build (search-patches ".....")
   ...
   )))
```  


zimoun <zimon.toutoune@gmail.com> writes:

> Hi Maxime,
>
> On Sat, 05 Mar 2022 at 22:49, Maxime Devos <maximedevos@telenet.be> wrote:
>> (as implied per zimoun's previous mail (‘FWIW, it would be unfair for
>> the patch to have the discussion here’), moved to guix-devel)
>
> Thanks. :-)
>
>> Leo Famulari schreef op za 05-03-2022 om 16:13 [-0500]:
>>> On Thu, Mar 03, 2022 at 07:25:22AM +0100, Maxime Devos wrote:
>>> > Leo Famulari schreef op wo 02-03-2022 om 18:50 [-0500]:
>>> > > Origin snippets should only be used to remove nonfree things
>>> > > from the upstream source code. All other changes should use
>>> > > patch files or a build phase.
>>> > 
>>> > Why?  If it's a source code change and it fits an origin snippet,
>>> > why not an origin snippet?  Why would the source in Guix need to
>>> match
>>> > the source upstream?
>>> 
>>> `guix build --source` is a tool to provide freely licensed source
>>> code
>>> to be used for any purpose, including building on systems besides
>>> Guix.
>>> 
>>> Using the Guix tools, there is no way to access the upstream source
>>> code
>>> without applying the snippets. The reason for that is that the origin
>>> snippet mechanism was introduced specifically to remove non-free
>>> components without making it easy to reverse the transformation.
>>
>> It might be introduced for removing non-free components, that doesn't
>> mean it cannot be used for more.  Also, I don't see the point of ease
>> of reversing here. It's trivial to reverse the transformation induced
>> by the snippet: just delete the snippet in a git checkout.
>
> Well, the point is the FSDG [1] frame, I guess.  From my understanding,
> when --source had been introduced, it was a countermeasure to be able to
> use hybrid source and still be compliant with an interpretation of: «A
> free system distribution must not steer users towards obtaining any
> nonfree information for practical use, or encourage them to do so.»
>
> Therefore, using Guix tools, e.g., guix build --source, it is not easy
> to reverse what ’snippet’ does.
>
> I would not say it is trivial to reverse the transformation because the
> user needs to run “guix edit”, then reassemble the URL, then fetch.
> Otherwise, yes the user could go to the Guix repo, remove the snippet,
> then run “guix shell -D guix”, do somehow “./pre-inst-env guix …”.
>
> Well, I do not consider these steps “trivial”.  And if one user does
> that, somehow they really want to obtain nonfree information. :-)
>
>
>
> 1: <https://www.gnu.org/distros/free-system-distribution-guidelines.en.html>
>
>>> Compare that to patch files, which are easily reversed,
>>
>> Removing a patch file by removing it from the 'patches' field is easy,
>> as easy as removing a snippet.  I assume you meant the additional
>> condition ‘... using only CLI tools’?
>
> Yes, somehow.
>
>
>> I am aware of the guideline of keeping the source usable outside Guix
>> systems.  AFAICT, in this case, the snippet modifying
>> Makefile.am/Makefile.in keeps the source usable on non-Guix systems.
>> In fact, it makes the source _more_ usable, both on Guix and non-Guix,
>> by working-around a Guile 3.0.5 compiler bug.  So I don't see any
>> problems here.
>
> Well, the question without consensus is what “guix build --source”
> should return?
>
>   a) The source of what “guix build” concretely builds?
>   b) The source of upstream (modulo the removal of nonfree part)?
>
> The aim is to be as close as possible as b), IMHO.  The exception of
> patches could be discussed. :-)
>
>
> Back to Shepherd, because the question is originally from patch#54216
> [2], the initial snippet was turning a flag:
>
> +         (snippet
> +          '(begin
> +             ;; Build with -O1 to work around <https://bugs.gnu.org/48368>.
> +             (substitute* "Makefile.am"
> +               (("compile --target")
> +                "compile -O1 --target"))))))
>
> Somehow, the snippet could be considered as a “patch“.  And, in the same
> time, the upstream source will not compile without this ’-O1’, IIUC.
>
> However, since “we“ are in the same time upstream and downstream, we
> could fix that without introducing this kind of snippet.
>
> Last, because the package is for building with Guix, then it seems more
> appropriate to have the substitution in the ’arguments’, as v3 [3] is
> doing.
>
>
> 2: <http://issues.guix.gnu.org/issue/54216>
> 3: <https://issues.guix.gnu.org/issue/54216#19>
>
>
>
> Cheers,
> simon


-- 
Retrieve my PGP public key:

  gpg --recv-keys D47A9C8B2AE3905B563D9135BE42B352A9F6821F

Zihao

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

[bug#54216] [PATCH v4] gnu: shepherd: Build Shepherd 0.9.0 from git.

[Attila Lendvai]

The rationale for this is that it makes it simpler to start VM's with a
Shepherd that is compiled from a local checkout.  This facilitates hacking on
Shepherd itself, and testing it in the Guix init environment, by greatly
reducing the edit-compile-test cycle.

* gnu/packages/admin.scm (shepherd-0.9): Source points to the git repo; adjust
native-inputs accordingly.  Also delete the strip build phase.
---
 gnu/packages/admin.scm | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/gnu/packages/admin.scm b/gnu/packages/admin.scm
index 94636faf8b..fd7df3f09f 100644
--- a/gnu/packages/admin.scm
+++ b/gnu/packages/admin.scm
@@ -314,19 +314,23 @@ (define-public shepherd shepherd-0.8)
 (define-public shepherd-0.9
   (package
     (inherit shepherd)
+    (name "shepherd") ; Only for git-file-name below; not ideal.
     (version "0.9.0")
-    (source (origin
-              (method url-fetch)
-              (uri (string-append "mirror://gnu/shepherd/shepherd-"
-                                  version ".tar.gz"))
-              (sha256
-               (base32
-                "1rdwhrcibs2ly4hjwwb5kmzb133ccjmrfvb0a70cqkv9jy1pg061"))))
+    (source
+     (origin
+       (method git-fetch)
+       (uri (git-reference
+             (url "https://git.savannah.gnu.org/git/shepherd.git/")
+             (commit (string-append "v" version))))
+       (file-name (git-file-name name version))
+       (sha256
+        (base32 "1caxw0549jfadnvjbh0jlawg6k535mb5mc226darcfihvjy3crma"))))
     (arguments
      (list #:configure-flags #~'("--localstatedir=/var")
            #:make-flags #~'("GUILE_AUTO_COMPILE=0")
            #:phases (if (%current-target-system)
                         #~(modify-phases %standard-phases
+                            (delete 'strip)  ; Warns when stripping .go files.
                             (add-before 'configure 'set-fibers-directory
                               (lambda _
                                 ;; When cross-compiling, refer to the target
@@ -341,8 +345,10 @@ (define-public shepherd-0.9
                                       (this-package-input "guile-fibers")
                                       "/lib/guile/3.0/site-ccache"))))))
                         #~%standard-phases)))
-    (native-inputs (list pkg-config guile-3.0
-                         guile-fibers-1.1))       ;for cross-compilation
+    (native-inputs
+     (list autoconf automake gettext-minimal help2man texinfo pkg-config
+           guile-fibers-1.1             ; for cross-compilation
+           guile-3.0))
     (inputs (list guile-3.0 guile-fibers-1.1))))
 
 (define-public guile2.2-shepherd
-- 
2.35.1

[bug#54216] [PATCH v5] gnu: shepherd: Build Shepherd 0.9.2 from git.

[Attila Lendvai]

The rationale for this is that it makes it simpler to start VM's with a
Shepherd that is compiled from a local checkout.  This facilitates hacking on
Shepherd itself, and testing it in the Guix init environment, by greatly
reducing the edit-compile-test cycle.

* gnu/packages/admin.scm (shepherd-0.9): Source points to the git repo; adjust
native-inputs accordingly.  Also delete the strip build phase.
---

since v4: rebased on master, avoid reindenting to shorten the diff,
avoid adding a (name "shepherd") field.

 gnu/packages/admin.scm | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/gnu/packages/admin.scm b/gnu/packages/admin.scm
index 5fb621e027..1a1378d64e 100644
--- a/gnu/packages/admin.scm
+++ b/gnu/packages/admin.scm
@@ -327,12 +327,13 @@ (define-public shepherd-0.9
     (inherit shepherd)
     (version "0.9.2")
     (source (origin
-              (method url-fetch)
-              (uri (string-append "mirror://gnu/shepherd/shepherd-"
-                                  version ".tar.gz"))
+              (method git-fetch)
+              (uri (git-reference
+                    (url "https://git.savannah.gnu.org/git/shepherd.git/")
+                    (commit (string-append "v" version))))
+              (file-name (git-file-name "shepherd" version))
               (sha256
-               (base32
-                "0mcby3ygh3bpns44rb1vnk8bz2km4nlw092nrcgkm3nkqfmbp4p1"))
+               (base32 "1b16qc7zmm1cz45gspcbng4djz9jy8db4awfw63b2hvf9b396mv2"))
               (modules '((guix build utils)))
               (snippet
                ;; Avoid continuation barriers so (@ (fibers) sleep) can be
@@ -349,6 +350,7 @@ (define-public shepherd-0.9
            #:make-flags #~'("GUILE_AUTO_COMPILE=0")
            #:phases (if (%current-target-system)
                         #~(modify-phases %standard-phases
+                            (delete 'strip)  ; Warns when stripping .go files.
                             (add-before 'configure 'set-fibers-directory
                               (lambda _
                                 ;; When cross-compiling, refer to the target
@@ -363,8 +365,10 @@ (define-public shepherd-0.9
                                       (this-package-input "guile-fibers")
                                       "/lib/guile/3.0/site-ccache"))))))
                         #~%standard-phases)))
-    (native-inputs (list pkg-config guile-3.0
-                         guile-fibers-1.1))       ;for cross-compilation
+    (native-inputs
+     (list autoconf automake gettext-minimal help2man texinfo pkg-config
+           guile-fibers-1.1             ; for cross-compilation
+           guile-3.0))
     (inputs (list guile-3.0 guile-fibers-1.1))))
 
 (define-public guile2.2-shepherd
-- 
2.35.1

[bug#54216] [PATCH v6] gnu: shepherd: Build Shepherd 0.9.2 from git.

[Attila Lendvai]

The rationale for this is that it makes it simpler to start VM's with a
Shepherd that is compiled from a local checkout.  This facilitates hacking on
Shepherd itself, and testing it in the Guix init environment, by greatly
reducing the edit-compile-test cycle.

* gnu/packages/admin.scm (shepherd-0.9): Source points to the git repo; adjust
native-inputs accordingly.  Also delete the strip build phase.
---

v6: fix the (delete 'strip) merge error to always include it. GExp
experts may be able to simplify it, or chose to drop it altogether.

 gnu/packages/admin.scm | 50 +++++++++++++++++++++++-------------------
 1 file changed, 27 insertions(+), 23 deletions(-)

diff --git a/gnu/packages/admin.scm b/gnu/packages/admin.scm
index 5fb621e027..e57f0436cb 100644
--- a/gnu/packages/admin.scm
+++ b/gnu/packages/admin.scm
@@ -327,12 +327,13 @@ (define-public shepherd-0.9
     (inherit shepherd)
     (version "0.9.2")
     (source (origin
-              (method url-fetch)
-              (uri (string-append "mirror://gnu/shepherd/shepherd-"
-                                  version ".tar.gz"))
+              (method git-fetch)
+              (uri (git-reference
+                    (url "https://git.savannah.gnu.org/git/shepherd.git/")
+                    (commit (string-append "v" version))))
+              (file-name (git-file-name "shepherd" version))
               (sha256
-               (base32
-                "0mcby3ygh3bpns44rb1vnk8bz2km4nlw092nrcgkm3nkqfmbp4p1"))
+               (base32 "1b16qc7zmm1cz45gspcbng4djz9jy8db4awfw63b2hvf9b396mv2"))
               (modules '((guix build utils)))
               (snippet
                ;; Avoid continuation barriers so (@ (fibers) sleep) can be
@@ -347,24 +348,27 @@ (define-public shepherd-0.9
     (arguments
      (list #:configure-flags #~'("--localstatedir=/var")
            #:make-flags #~'("GUILE_AUTO_COMPILE=0")
-           #:phases (if (%current-target-system)
-                        #~(modify-phases %standard-phases
-                            (add-before 'configure 'set-fibers-directory
-                              (lambda _
-                                ;; When cross-compiling, refer to the target
-                                ;; Fibers, not the native one.
-                                (substitute* '("herd.in" "shepherd.in")
-                                  (("%FIBERS_SOURCE_DIRECTORY%")
-                                   #$(file-append
-                                      (this-package-input "guile-fibers")
-                                      "/share/guile/site/3.0"))
-                                  (("%FIBERS_OBJECT_DIRECTORY%")
-                                   #$(file-append
-                                      (this-package-input "guile-fibers")
-                                      "/lib/guile/3.0/site-ccache"))))))
-                        #~%standard-phases)))
-    (native-inputs (list pkg-config guile-3.0
-                         guile-fibers-1.1))       ;for cross-compilation
+           #:phases #~(modify-phases %standard-phases
+                        (delete 'strip) ; Warns when stripping .go files.
+                        #$@(if (%current-target-system)
+                               #~((add-before 'configure 'set-fibers-directory
+                                    (lambda _
+                                      ;; When cross-compiling, refer to the target
+                                      ;; Fibers, not the native one.
+                                      (substitute* '("herd.in" "shepherd.in")
+                                        (("%FIBERS_SOURCE_DIRECTORY%")
+                                         #$(file-append
+                                            (this-package-input "guile-fibers")
+                                            "/share/guile/site/3.0"))
+                                        (("%FIBERS_OBJECT_DIRECTORY%")
+                                         #$(file-append
+                                            (this-package-input "guile-fibers")
+                                            "/lib/guile/3.0/site-ccache"))))))
+                               #~()))))
+    (native-inputs
+     (list autoconf automake gettext-minimal help2man texinfo pkg-config
+           guile-fibers-1.1             ; for cross-compilation
+           guile-3.0))
     (inputs (list guile-3.0 guile-fibers-1.1))))
 
 (define-public guile2.2-shepherd
-- 
2.35.1