GNU bug report logs - #50620
[PATCH 0/2] Unify 'computed-origin-method' (linux, icecat)

Previous Next

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

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

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

From: zimoun <zimon.toutoune <at> gmail.com>
To: guix-patches <at> gnu.org
Cc: zimoun <zimon.toutoune <at> gmail.com>
Subject: [PATCH 0/2] Unify 'computed-origin-method' (linux, icecat)
Date: Thu, 16 Sep 2021 13:45:05 +0200

Hi,

This patch follows the discussion from [0].  In short, it simplifies the code
generating the file 'sources.json' used by Software Heritage to ingest all the
tarballs.  It would partially fix the issue raised here [1].

The first patch moves the duplicate of 'computed-origin-method' and adds
comments.  Please proof-read. :-)

The second patch update the package guix.  Since I cannot know the commit hash
and checksum beforehand, it is set to xxxx and should be updated by the
commiter.  This update is necessary to get the correct Guix-as-library used by
the website generator of 'sources.json', IIUC.

All the best,
simon

0: <http://issues.guix.gnu.org/50515#5>
1: <https://lists.gnu.org/archive/html/guix-devel/2021-09/msg00050.html>

zimoun (2):
  guix: packages: Document 'computed-origin-method'.
  gnu: guix: Update to xxxx.

 gnu/packages/gnuzilla.scm           | 14 ++------------
 gnu/packages/linux.scm              | 14 ++------------
 gnu/packages/package-management.scm |  6 +++---
 guix/packages.scm                   | 23 ++++++++++++++++++++++-
 4 files changed, 29 insertions(+), 28 deletions(-)


base-commit: 33bc3fb2a5f30a6e21f1b8d6d43867d921bd951c
-- 
2.32.0

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

From: zimoun <zimon.toutoune <at> gmail.com>
To: 50620 <at> debbugs.gnu.org
Cc: zimoun <zimon.toutoune <at> gmail.com>
Subject: [PATCH 2/2] gnu: guix: Update to xxxx.
Date: Thu, 16 Sep 2021 13:47:34 +0200

* gnu/packages/package-management.scm (guix): Update to xxxx.
---
 gnu/packages/package-management.scm | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/gnu/packages/package-management.scm b/gnu/packages/package-management.scm
index 2611951a52..458e121ef2 100644
--- a/gnu/packages/package-management.scm
+++ b/gnu/packages/package-management.scm
@@ -137,8 +137,8 @@
   ;; Note: the 'update-guix-package.scm' script expects this definition to
   ;; start precisely like this.
   (let ((version "1.3.0")
-        (commit "6243ad3812f8c689599a19f0e8b9719ba14461f2")
-        (revision 5))
+        (commit "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx")
+        (revision 6))
     (package
       (name "guix")
 
@@ -154,7 +154,7 @@
                       (commit commit)))
                 (sha256
                  (base32
-                  "0i3sgk2w2yjy9ip47vk0h17afk16yl5ih3p3q75083kgjzyjdm3d"))
+                  "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"))
                 (file-name (string-append "guix-" version "-checkout"))))
       (build-system gnu-build-system)
       (arguments
-- 
2.32.0

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

From: zimoun <zimon.toutoune <at> gmail.com>
To: 50620 <at> debbugs.gnu.org
Cc: zimoun <zimon.toutoune <at> gmail.com>
Subject: [PATCH 1/2] guix: packages: Document 'computed-origin-method'.
Date: Thu, 16 Sep 2021 13:47:33 +0200

The 'computed-origin-method' had been introduced as a workaround limitations
in the 'snippet' mechanism.  The procedure is duplicated which makes hard to
automatically detects packages using it.

* guix/packages.scm (computed-origin-method): Move procedure from...
* gnu/packages/gnuzilla.scm: ...here and...
* gnu/packages/gnuzilla.scm: ...there.
---
 gnu/packages/gnuzilla.scm | 14 ++------------
 gnu/packages/linux.scm    | 14 ++------------
 guix/packages.scm         | 23 ++++++++++++++++++++++-
 3 files changed, 26 insertions(+), 25 deletions(-)

diff --git a/gnu/packages/gnuzilla.scm b/gnu/packages/gnuzilla.scm
index 431b487fd0..9f6e1f24e1 100644
--- a/gnu/packages/gnuzilla.scm
+++ b/gnu/packages/gnuzilla.scm
@@ -682,18 +682,8 @@ in C/C++.")
    ("1j6l66v1xw27z8w78mpsnmqgv8m277mf4r0hgqcrb4zx7xc2vqyy" "527e5e090608" "zh-CN")
    ("1frwx35klpyz3sdwrkz7945ivb2dwaawhhyfnz4092h9hn7rc4ky" "6cd366ad2947" "zh-TW")))
 
-(define* (computed-origin-method gexp-promise hash-algo hash
-                                 #:optional (name "source")
-                                 #:key (system (%current-system))
-                                 (guile (default-guile)))
-  "Return a derivation that executes the G-expression that results
-from forcing GEXP-PROMISE."
-  (mlet %store-monad ((guile (package->derivation guile system)))
-    (gexp->derivation (or name "computed-origin")
-                      (force gexp-promise)
-                      #:graft? #f       ;nothing to graft
-                      #:system system
-                      #:guile-for-build guile)))
+;; XXXX: Workaround 'snippet' limitations.
+(define computed-origin-method (@@ (guix packages) computed-origin-method))
 
 (define %icecat-version "78.14.0-guix0-preview1")
 (define %icecat-build-id "20210907000000") ;must be of the form YYYYMMDDhhmmss
diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index 285eb132f4..eb792be9a3 100644
--- a/gnu/packages/linux.scm
+++ b/gnu/packages/linux.scm
@@ -216,18 +216,8 @@ defconfig.  Return the appropriate make target if applicable, otherwise return
           (file-name (string-append "linux-libre-deblob-check-" version "-" gnu-revision))
           (sha256 deblob-check-hash))))
 
-(define* (computed-origin-method gexp-promise hash-algo hash
-                                 #:optional (name "source")
-                                 #:key (system (%current-system))
-                                 (guile (default-guile)))
-  "Return a derivation that executes the G-expression that results
-from forcing GEXP-PROMISE."
-  (mlet %store-monad ((guile (package->derivation guile system)))
-    (gexp->derivation (or name "computed-origin")
-                      (force gexp-promise)
-                      #:graft? #f       ;nothing to graft
-                      #:system system
-                      #:guile-for-build guile)))
+;; XXXX: Workaround 'snippet' limitations
+(define computed-origin-method (@@ (guix packages) computed-origin-method))
 
 (define (make-linux-libre-source version
                                  upstream-source
diff --git a/guix/packages.scm b/guix/packages.scm
index ad7937b4fb..8c3a0b0b7b 100644
--- a/guix/packages.scm
+++ b/guix/packages.scm
@@ -1,6 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019, 2020, 2021 Ludovic Courtès <ludo <at> gnu.org>
-;;; Copyright © 2014, 2015, 2017, 2018 Mark H Weaver <mhw <at> netris.org>
+;;; Copyright © 2014, 2015, 2017, 2018, 2019 Mark H Weaver <mhw <at> netris.org>
 ;;; Copyright © 2015 Eric Bavier <bavier <at> member.fsf.org>
 ;;; Copyright © 2016 Alex Kost <alezost <at> gmail.com>
 ;;; Copyright © 2017, 2019, 2020 Efraim Flashner <efraim <at> flashner.co.il>
@@ -344,6 +344,27 @@ name of its URI."
          ;; git, svn, cvs, etc. reference
          #f))))
 
+;; Work around limitations in the 'snippet' mechanism.  It is not possible for
+;; a 'snippet' to produce a tarball with a different base name than the
+;; original downloaded source.  Moreover, cherry picking dozens of upsteam
+;; patches and applying them suddenly is often impractical; especially when a
+;; comprehensive code reformatting is done upstream.  Mainly designed for
+;; Linux and IceCat packages.
+;; XXXX: do not make part of public API (export) such radical capability
+;; before a detailed review process.
+(define* (computed-origin-method gexp-promise hash-algo hash
+                                 #:optional (name "source")
+                                 #:key (system (%current-system))
+                                 (guile (default-guile)))
+  "Return a derivation that executes the G-expression that results
+from forcing GEXP-PROMISE."
+  (mlet %store-monad ((guile (package->derivation guile system)))
+    (gexp->derivation (or name "computed-origin")
+                      (force gexp-promise)
+                      #:graft? #f       ;nothing to graft
+                      #:system system
+                      #:guile-for-build guile)))
+
 
 (define %supported-systems
   ;; This is the list of system types that are supported.  By default, we
-- 
2.32.0

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

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: zimoun <zimon.toutoune <at> gmail.com>, 50620 <at> debbugs.gnu.org
Subject: Re: [PATCH 1/2] guix: packages: Document 'computed-origin-method'.
Date: Thu, 16 Sep 2021 17:53:26 +0200

Hi,

Am Donnerstag, den 16.09.2021, 13:47 +0200 schrieb zimoun:
> The 'computed-origin-method' had been introduced as a workaround
> limitations in the 'snippet' mechanism.  The procedure is duplicated
> which makes hard to automatically detects packages using it.
> 
> * guix/packages.scm (computed-origin-method): Move procedure from...
> * gnu/packages/gnuzilla.scm: ...here and...
> * gnu/packages/gnuzilla.scm: ...there.
> ---
>  gnu/packages/gnuzilla.scm | 14 ++------------
>  gnu/packages/linux.scm    | 14 ++------------
>  guix/packages.scm         | 23 ++++++++++++++++++++++-
>  3 files changed, 26 insertions(+), 25 deletions(-)
> 
> diff --git a/gnu/packages/gnuzilla.scm b/gnu/packages/gnuzilla.scm
> index 431b487fd0..9f6e1f24e1 100644
> --- a/gnu/packages/gnuzilla.scm
> +++ b/gnu/packages/gnuzilla.scm
> @@ -682,18 +682,8 @@ in C/C++.")
>     ("1j6l66v1xw27z8w78mpsnmqgv8m277mf4r0hgqcrb4zx7xc2vqyy"
> "527e5e090608" "zh-CN")
>     ("1frwx35klpyz3sdwrkz7945ivb2dwaawhhyfnz4092h9hn7rc4ky"
> "6cd366ad2947" "zh-TW")))
>  
> -(define* (computed-origin-method gexp-promise hash-algo hash
> -                                 #:optional (name "source")
> -                                 #:key (system (%current-system))
> -                                 (guile (default-guile)))
> -  "Return a derivation that executes the G-expression that results
> -from forcing GEXP-PROMISE."
> -  (mlet %store-monad ((guile (package->derivation guile system)))
> -    (gexp->derivation (or name "computed-origin")
> -                      (force gexp-promise)
> -                      #:graft? #f       ;nothing to graft
> -                      #:system system
> -                      #:guile-for-build guile)))
> +;; XXXX: Workaround 'snippet' limitations.
> +(define computed-origin-method (@@ (guix packages) computed-origin-
> method))
>  
>  (define %icecat-version "78.14.0-guix0-preview1")
>  (define %icecat-build-id "20210907000000") ;must be of the form
> YYYYMMDDhhmmss
> diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
> index 285eb132f4..eb792be9a3 100644
> --- a/gnu/packages/linux.scm
> +++ b/gnu/packages/linux.scm
> @@ -216,18 +216,8 @@ defconfig.  Return the appropriate make target
> if applicable, otherwise return
>            (file-name (string-append "linux-libre-deblob-check-"
> version "-" gnu-revision))
>            (sha256 deblob-check-hash))))
>  
> -(define* (computed-origin-method gexp-promise hash-algo hash
> -                                 #:optional (name "source")
> -                                 #:key (system (%current-system))
> -                                 (guile (default-guile)))
> -  "Return a derivation that executes the G-expression that results
> -from forcing GEXP-PROMISE."
> -  (mlet %store-monad ((guile (package->derivation guile system)))
> -    (gexp->derivation (or name "computed-origin")
> -                      (force gexp-promise)
> -                      #:graft? #f       ;nothing to graft
> -                      #:system system
> -                      #:guile-for-build guile)))
> +;; XXXX: Workaround 'snippet' limitations
> +(define computed-origin-method (@@ (guix packages) computed-origin-
> method))
>  
>  (define (make-linux-libre-source version
>                                   upstream-source
> diff --git a/guix/packages.scm b/guix/packages.scm
> index ad7937b4fb..8c3a0b0b7b 100644
> --- a/guix/packages.scm
> +++ b/guix/packages.scm
> @@ -1,6 +1,6 @@
>  ;;; GNU Guix --- Functional package management for GNU
>  ;;; Copyright © 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019,
> 2020, 2021 Ludovic Courtès <ludo <at> gnu.org>
> -;;; Copyright © 2014, 2015, 2017, 2018 Mark H Weaver <mhw <at> netris.org
> >
> +;;; Copyright © 2014, 2015, 2017, 2018, 2019 Mark H Weaver <
> mhw <at> netris.org>
>  ;;; Copyright © 2015 Eric Bavier <bavier <at> member.fsf.org>
>  ;;; Copyright © 2016 Alex Kost <alezost <at> gmail.com>
>  ;;; Copyright © 2017, 2019, 2020 Efraim Flashner <
> efraim <at> flashner.co.il>
> @@ -344,6 +344,27 @@ name of its URI."
>           ;; git, svn, cvs, etc. reference
>           #f))))
>  
> +;; Work around limitations in the 'snippet' mechanism.  It is not
> possible for
> +;; a 'snippet' to produce a tarball with a different base name than
> the
> +;; original downloaded source.  Moreover, cherry picking dozens of
> upsteam
> +;; patches and applying them suddenly is often impractical;
> especially when a
> +;; comprehensive code reformatting is done upstream.  Mainly
> designed for
> +;; Linux and IceCat packages.
> +;; XXXX: do not make part of public API (export) such radical
> capability
> +;; before a detailed review process.
> +(define* (computed-origin-method gexp-promise hash-algo hash
> +                                 #:optional (name "source")
> +                                 #:key (system (%current-system))
> +                                 (guile (default-guile)))
> +  "Return a derivation that executes the G-expression that results
> +from forcing GEXP-PROMISE."
> +  (mlet %store-monad ((guile (package->derivation guile system)))
> +    (gexp->derivation (or name "computed-origin")
> +                      (force gexp-promise)
> +                      #:graft? #f       ;nothing to graft
> +                      #:system system
> +                      #:guile-for-build guile)))
> +
>  
>  (define %supported-systems
>    ;; This is the list of system types that are supported.  By
> default, we
I think that rather than putting this into (guix packages) itself, we
might want to put it into its own file like (guix computed-origins) and
choose a method name that is actually a verb, similar to git-fetch or
svn-fetch.  Perhaps simply call it compute-origin?

If done this way, there'd be the benefit that modules with packages
using this thing would have to explicitly request the presence of the
symbol through their use-modules clauses.  WDYT?

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

From: Mark H Weaver <mhw <at> netris.org>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Cc: 50620 <at> debbugs.gnu.org, zimoun <zimon.toutoune <at> gmail.com>
Subject: Re: [PATCH 1/2] guix: packages: Document 'computed-origin-method'.
Date: Thu, 16 Sep 2021 19:38:22 -0400

Hi Liliana,

Liliana Marie Prikler <liliana.prikler <at> gmail.com> wrote:
> I think that rather than putting this into (guix packages) itself, we
> might want to put it into its own file like (guix computed-origins) and
> choose a method name that is actually a verb, similar to git-fetch or
> svn-fetch.  Perhaps simply call it compute-origin?

These suggestions sound fine to me, although I don't have a strong
opinion either way.  I'm happy to leave these details to others to
decide.

> If done this way, there'd be the benefit that modules with packages
> using this thing would have to explicitly request the presence of the
> symbol through their use-modules clauses.

Actually, for better or worse, Guile's '@@' form does not require the
named module to be imported using 'use-modules', so I don't think this
benefit strictly exists as stated above.  However, I agree that it's
good practice to list all imported modules in the '#:use-module' clauses
at the top of the file wherever possible [*], and that there may be some
benefit in declaring the use of 'computed-origins' at the top of each
file.

      Thanks,
        Mark

[*] It's not always possible in the presence of cyclic module dependencies.

-- 
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about <https://stallmansupport.org>.

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

From: zimoun <zimon.toutoune <at> gmail.com>
To: Mark H Weaver <mhw <at> netris.org>
Cc: Liliana Marie Prikler <liliana.prikler <at> gmail.com>, 50620 <at> debbugs.gnu.org
Subject: Re: [PATCH 1/2] guix: packages: Document 'computed-origin-method'.
Date: Fri, 17 Sep 2021 10:41:01 +0200

Hi Liliana and Mark,

Thanks for the comments.

On Fri, 17 Sept 2021 at 01:40, Mark H Weaver <mhw <at> netris.org> wrote:
> Liliana Marie Prikler <liliana.prikler <at> gmail.com> wrote:

> > I think that rather than putting this into (guix packages) itself, we
> > might want to put it into its own file like (guix computed-origins) and
> > choose a method name that is actually a verb, similar to git-fetch or
> > svn-fetch.  Perhaps simply call it compute-origin?
>
> These suggestions sound fine to me, although I don't have a strong
> opinion either way.  I'm happy to leave these details to others to
> decide.

I do not have a strong opinion either.  If no one comes with a better
name, I will pick the suggested one. :-)

        There are only two hard things in Computer Science: cache
        invalidation and naming things.
                                                      -- Internet


> > If done this way, there'd be the benefit that modules with packages
> > using this thing would have to explicitly request the presence of the
> > symbol through their use-modules clauses.
>
> Actually, for better or worse, Guile's '@@' form does not require the
> named module to be imported using 'use-modules', so I don't think this
> benefit strictly exists as stated above.  However, I agree that it's
> good practice to list all imported modules in the '#:use-module' clauses
> at the top of the file wherever possible [*], and that there may be some
> benefit in declaring the use of 'computed-origins' at the top of each
> file.

I am not deeply familiar with Guile module.

I chose to put this in (guix packages) instead of its own module
because the module would contain only one function and nothing
exported.  The aim for now, as discussed, is to not make this 'method'
part of the public API.

Then if the function is not exported by the module, the '#:use-module'
does not have an effect, right?

In this case, does it make sense to put this in its own module?

Initially, I put the '@@' right after the '#:use-module's but then I
changed my mind; I do not remember why.  Anyway, yeah it is better at
the top.


Cheers,
simon

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

From: Mark H Weaver <mhw <at> netris.org>
To: zimoun <zimon.toutoune <at> gmail.com>
Cc: Liliana Marie Prikler <liliana.prikler <at> gmail.com>, 50620 <at> debbugs.gnu.org
Subject: Re: [PATCH 1/2] guix: packages: Document 'computed-origin-method'.
Date: Tue, 28 Sep 2021 05:36:48 -0400

Hi Simon,

zimoun <zimon.toutoune <at> gmail.com> writes:
> On Fri, 17 Sept 2021 at 01:40, Mark H Weaver <mhw <at> netris.org> wrote:
>> Liliana Marie Prikler <liliana.prikler <at> gmail.com> wrote:
[...]
>> > If done this way, there'd be the benefit that modules with packages
>> > using this thing would have to explicitly request the presence of the
>> > symbol through their use-modules clauses.
>>
>> Actually, for better or worse, Guile's '@@' form does not require the
>> named module to be imported using 'use-modules', so I don't think this
>> benefit strictly exists as stated above.  However, I agree that it's
>> good practice to list all imported modules in the '#:use-module' clauses
>> at the top of the file wherever possible [*], and that there may be some
>> benefit in declaring the use of 'computed-origins' at the top of each
>> file.
>
> I am not deeply familiar with Guile module.
>
> I chose to put this in (guix packages) instead of its own module
> because the module would contain only one function and nothing
> exported.  The aim for now, as discussed, is to not make this 'method'
> part of the public API.
>
> Then if the function is not exported by the module, the '#:use-module'
> does not have an effect, right?

It's true that it would have no effect on the set of available bindings,
and that's an excellent point.  Perhaps Liliana could clarify what she
had in mind, or better yet, propose a patch.

Please don't let me be a blocker on this thread.  I contributed a few
thoughts, but I don't have time right now to shepherd this issue, sorry.

      Regards,
        Mark

-- 
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about <https://stallmansupport.org>.

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

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: Mark H Weaver <mhw <at> netris.org>, zimoun <zimon.toutoune <at> gmail.com>
Cc: 50620 <at> debbugs.gnu.org
Subject: Re: [PATCH 1/2] guix: packages: Document 'computed-origin-method'.
Date: Tue, 28 Sep 2021 18:01:51 +0200

[Message part 1 (text/plain, inline)]

Hi everyone,

Am Dienstag, den 28.09.2021, 05:36 -0400 schrieb Mark H Weaver:
> Hi Simon,
> 
> zimoun <zimon.toutoune <at> gmail.com> writes:
> > On Fri, 17 Sept 2021 at 01:40, Mark H Weaver <mhw <at> netris.org>
> > wrote:
> > > Liliana Marie Prikler <liliana.prikler <at> gmail.com> wrote:
> [...]
> > > > If done this way, there'd be the benefit that modules with
> > > > packages
> > > > using this thing would have to explicitly request the presence
> > > > of the
> > > > symbol through their use-modules clauses.
> > > 
> > > Actually, for better or worse, Guile's '@@' form does not require
> > > the named module to be imported using 'use-modules', so I don't
> > > think this benefit strictly exists as stated above.  However, I
> > > agree that it's  good practice to list all imported modules in
> > > the '#:use-module' clauses at the top of the file wherever
> > > possible [*], and that there may be somebenefit in declaring the
> > > use of 'computed-origins' at the top of each file.
> > 
> > I am not deeply familiar with Guile module.
> > 
> > I chose to put this in (guix packages) instead of its own module
> > because the module would contain only one function and nothing
> > exported.  The aim for now, as discussed, is to not make this
> > 'method' part of the public API.
If so, one could argue that (gnu packages) is a better location to hide
it, but my main issue is that we still need to hide it!  This will
cause other channels to refer to it using @@ or roll their own
implementations.

> > Then if the function is not exported by the module, the '#:use-
> > module' does not have an effect, right?
> 
> It's true that it would have no effect on the set of available
> bindings, and that's an excellent point.  Perhaps Liliana could
> clarify what she had in mind, or better yet, propose a patch.
I would argue that something like computed-origin-method *does* deserve
to be an exported binding, but ought not to be grouped together into
(guix packages).  The latter only provides record types, not methods
(which are typically implemented elsewhere), and I'd like to keep it
that way.

I've attached a patch to illustrate my point, but please don't apply it
as is.  I have not put in the necessary git blame research to find out
who would need to be copyrighted here.

Regards,
Liliana

[0001-guix-Add-computed-origins.patch (text/x-patch, attachment)]

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

From: zimoun <zimon.toutoune <at> gmail.com>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Cc: Mark H Weaver <mhw <at> netris.org>, 50620 <at> debbugs.gnu.org
Subject: Re: [PATCH 1/2] guix: packages: Document 'computed-origin-method'.
Date: Tue, 28 Sep 2021 18:37:45 +0200

Hi,

On Tue, 28 Sept 2021 at 18:01, Liliana Marie Prikler
<liliana.prikler <at> gmail.com> wrote:
> > zimoun <zimon.toutoune <at> gmail.com> writes:

> > > I chose to put this in (guix packages) instead of its own module
> > > because the module would contain only one function and nothing
> > > exported.  The aim for now, as discussed, is to not make this
> > > 'method' part of the public API.

> If so, one could argue that (gnu packages) is a better location to hide

Ok.  I do not find it better than (guix packages) where 'origin' is
defined but anyway.
I will send a v2 considering this and the rename you proposed.

> it, but my main issue is that we still need to hide it!  This will
> cause other channels to refer to it using @@ or roll their own
> implementations.

This patch is not about discussing if this method should be public or
not.  It is private.  Please discuss that elsewhere.

Mark commented in [0]:

--8<---------------cut here---------------start------------->8---
The reason 'computed-origin-method' is not exported is because it
never went through the review process that such a radical new capability
in Guix should go through before becoming part of it's public API.
--8<---------------cut here---------------end--------------->8---

and this patch is about improving the situation (by removing the code
duplication).  That's all.  The aim of this improvement is related to
saving these IceCat and Linux Libre packages by Software Heritage [1].

0: <http://issues.guix.gnu.org/50515#3>
1: <http://issues.guix.gnu.org/50515>

> I've attached a patch to illustrate my point, but please don't apply it
> as is.  I have not put in the necessary git blame research to find out
> who would need to be copyrighted here.

As I said, the point of my patch is not to discuss if this
'compute-origin' should be part or not to the public API.  It is
simply a cleanup to ease the patch#50515 [1].  Therefore, I do not see
what is the point to create its own module.

All the best,
simon

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

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: zimoun <zimon.toutoune <at> gmail.com>
Cc: Mark H Weaver <mhw <at> netris.org>, 50620 <at> debbugs.gnu.org
Subject: Re: [PATCH 1/2] guix: packages: Document 'computed-origin-method'.
Date: Tue, 28 Sep 2021 19:24:23 +0200

Hi,

Am Dienstag, den 28.09.2021, 18:37 +0200 schrieb zimoun:
> Hi,
> 
> On Tue, 28 Sept 2021 at 18:01, Liliana Marie Prikler
> <liliana.prikler <at> gmail.com> wrote:
> > > zimoun <zimon.toutoune <at> gmail.com> writes:
> > > > I chose to put this in (guix packages) instead of its own
> > > > module
> > > > because the module would contain only one function and nothing
> > > > exported.  The aim for now, as discussed, is to not make this
> > > > 'method' part of the public API.
> > If so, one could argue that (gnu packages) is a better location to
> > hide
> 
> Ok.  I do not find it better than (guix packages) where 'origin' is
> defined but anyway.
> I will send a v2 considering this and the rename you proposed.
By that logic all of (guix git-download), (guix svn-download), etc.
could be inlined there as well.  Obviously that's a bad idea, but *why*
is it a bad idea?  I'd argue it's because we have a clear separation of
the record descriptor for an origin and the ways it can be computed
(the former in (guix packages), the latter elsewhere) and that it's
good to keep those concerns separate.

I also personally find the name "computed-origin" to be somewhat weird
naming choice.  I could just as well write the entire source code for
some given package in the snippet part of an origin, perhaps applying
some weird tricks in the category of Kolmogorov code golf – would that
origin not be computed?

> > it, but my main issue is that we still need to hide it!  This will
> > cause other channels to refer to it using @@ or roll their own
> > implementations.
> 
> This patch is not about discussing if this method should be public or
> not.  It is private.  Please discuss that elsewhere.
> 
> Mark commented in [0]:
> 
> --8<---------------cut here---------------start------------->8---
> The reason 'computed-origin-method' is not exported is because it
> never went through the review process that such a radical new
> capability in Guix should go through before becoming part of it's
> public API.
> --8<---------------cut here---------------end--------------->8---
> 
> and this patch is about improving the situation (by removing the code
> duplication).  That's all.  The aim of this improvement is related to
> saving these IceCat and Linux Libre packages by Software Heritage
> [1].
I don't think delaying this review is a good idea, though.  When you're
removing code duplication, you ought to do it in a way that all
duplicated code can indeed be removed, at least in my opinion.  As-is
this patch just invites practises otherwise discouraged by Guix.

Cheers

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

From: zimoun <zimon.toutoune <at> gmail.com>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Cc: Mark H Weaver <mhw <at> netris.org>, 50620 <at> debbugs.gnu.org
Subject: Re: [PATCH 1/2] guix: packages: Document 'computed-origin-method'.
Date: Wed, 29 Sep 2021 10:32:22 +0200

Hi,

On Tue, 28 Sept 2021 at 19:24, Liliana Marie Prikler
<liliana.prikler <at> gmail.com> wrote:

> > Ok.  I do not find it better than (guix packages) where 'origin' is
> > defined but anyway.
> > I will send a v2 considering this and the rename you proposed.
>
> By that logic all of (guix git-download), (guix svn-download), etc.
> could be inlined there as well.  Obviously that's a bad idea, but *why*
> is it a bad idea?  I'd argue it's because we have a clear separation of
> the record descriptor for an origin and the ways it can be computed
> (the former in (guix packages), the latter elsewhere) and that it's
> good to keep those concerns separate.
>
> I also personally find the name "computed-origin" to be somewhat weird
> naming choice.  I could just as well write the entire source code for
> some given package in the snippet part of an origin, perhaps applying
> some weird tricks in the category of Kolmogorov code golf – would that
> origin not be computed?

Are we bikeshedding here? ;-)

Again, the aim of this patch it not to fix the
'computed-origin-method'.  The aim of this patch is to improve the
readibility of the patch#50515 [1] which allows linux-libre and icecat
to be ingested by SWH from 'guix.gnu.org/sources.json'.

Maybe there is an original issue with 'computed-origin-method', as
Mark explained [0].  But that's another story than the SWH and
sources.json one!

--8<---------------cut here---------------start------------->8---
At the time that I added 'computed-origin-method', I was under time
pressure to push security updates for IceCat, and my previous method of
cherry picking dozens of upsteam patches and applying them to the most
recent IceCat release suddenly became impractical due to comprehensive
code reformatting done upstream.

I've always viewed 'computed-origin-method' as a temporary hack to work
around limitations in the 'snippet' mechanism.  Most importantly, last I
checked, it was not possible for a 'snippet' to produce a tarball with a
different base name than the original downloaded source.  I consider it
a *requirement* for the 'icecat' source tarball and it's unpacked
directory to be named "icecat-…" and not "firefox-…", and similarly
for'linux-libre'.
--8<---------------cut here---------------end--------------->8---

0: <http://issues.guix.gnu.org/50515#3>
1: <http://issues.guix.gnu.org/50515>

> > > it, but my main issue is that we still need to hide it!  This will
> > > cause other channels to refer to it using @@ or roll their own
> > > implementations.
> >
> > This patch is not about discussing if this method should be public or
> > not.  It is private.  Please discuss that elsewhere.
> >
> > Mark commented in [0]:
> >
> > --8<---------------cut here---------------start------------->8---
> > The reason 'computed-origin-method' is not exported is because it
> > never went through the review process that such a radical new
> > capability in Guix should go through before becoming part of it's
> > public API.
> > --8<---------------cut here---------------end--------------->8---
> >
> > and this patch is about improving the situation (by removing the code
> > duplication).  That's all.  The aim of this improvement is related to
> > saving these IceCat and Linux Libre packages by Software Heritage
> > [1].
>
> I don't think delaying this review is a good idea, though.  When you're
> removing code duplication, you ought to do it in a way that all
> duplicated code can indeed be removed, at least in my opinion.  As-is
> this patch just invites practises otherwise discouraged by Guix.

If you go this road, please push patch#50515 [1] as-is.  It perfectly
works and fixes the issue with 'guix.gnu.org/sources.json' and SWH.
This current patch#50620 is a way to improve the readibility of
patch#50515 but then reading all this discussion I miss why
patch#50620 is thus a blocker for patch#50515.  Because I feel we are
entering into the famous "Bigger problem" from xkcd. ;-)

Patch#50515 is the first part of a concrete answer to
<https://lists.gnu.org/archive/html/guix-devel/2021-09/msg00106.html>
and <https://lists.gnu.org/archive/html/guix-devel/2021-09/msg00290.html>.
It is discussed to use SWH for such situations but today our
linux-libre is not ingested by SWH.  Therefore, let first ingest
it--which does patch#50515.

All the best,
simon

PS: I disagree with your last statement.  Because I am in favour for
incremental improvements and not "The Right Thing or nothing".  That's
out of scope of the patch at hand. ;-)

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

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: zimoun <zimon.toutoune <at> gmail.com>
Cc: Mark H Weaver <mhw <at> netris.org>, 50620 <at> debbugs.gnu.org
Subject: Re: [PATCH 1/2] guix: packages: Document 'computed-origin-method'.
Date: Wed, 29 Sep 2021 12:10:16 +0200

Hi,

Am Mittwoch, den 29.09.2021, 10:32 +0200 schrieb zimoun:
> Hi,
> 
> On Tue, 28 Sept 2021 at 19:24, Liliana Marie Prikler
> <liliana.prikler <at> gmail.com> wrote:
> 
> > > Ok.  I do not find it better than (guix packages) where 'origin'
> > > is
> > > defined but anyway.
> > > I will send a v2 considering this and the rename you proposed.
> > 
> > By that logic all of (guix git-download), (guix svn-download), etc.
> > could be inlined there as well.  Obviously that's a bad idea, but
> > *why* is it a bad idea?  I'd argue it's because we have a clear
> > separation of the record descriptor for an origin and the ways it
> > can be computed (the former in (guix packages), the latter
> > elsewhere) and that it's good to keep those concerns separate.
> > 
> > I also personally find the name "computed-origin" to be somewhat
> > weird naming choice.  I could just as well write the entire source
> > code for some given package in the snippet part of an origin,
> > perhaps applying some weird tricks in the category of Kolmogorov
> > code golf – would that origin not be computed?
> 
> Are we bikeshedding here? ;-)
> 
> Again, the aim of this patch it not to fix the
> 'computed-origin-method'.  The aim of this patch is to improve the
> readibility of the patch#50515 [1] which allows linux-libre and
> icecat to be ingested by SWH from 'guix.gnu.org/sources.json'.
> 
> Maybe there is an original issue with 'computed-origin-method', as
> Mark explained [0].  But that's another story than the SWH and
> sources.json one!
Perhaps we're bikeshedding, but you started to shed the bike when you
chose to move this procedure to (guix packages) rather than
implementing one of Mark's suggestions in [0].  I do think we should
allow for bikeshedding comments from both sides if that is the case.

> [...]
> 
> > > > it, but my main issue is that we still need to hide it!  This
> > > > will
> > > > cause other channels to refer to it using @@ or roll their own
> > > > implementations.
> > > 
> > > This patch is not about discussing if this method should be
> > > public or
> > > not.  It is private.  Please discuss that elsewhere.
> > > 
> > > Mark commented in [0]:
> > > 
> > > --8<---------------cut here---------------start------------->8---
> > > The reason 'computed-origin-method' is not exported is because it
> > > never went through the review process that such a radical new
> > > capability in Guix should go through before becoming part of it's
> > > public API.
> > > --8<---------------cut here---------------end--------------->8---
> > > 
> > > and this patch is about improving the situation (by removing the
> > > code
> > > duplication).  That's all.  The aim of this improvement is
> > > related to
> > > saving these IceCat and Linux Libre packages by Software Heritage
> > > [1].
> > 
> > I don't think delaying this review is a good idea, though.  When
> > you're removing code duplication, you ought to do it in a way that
> > all duplicated code can indeed be removed, at least in my
> > opinion.  As-is this patch just invites practises otherwise
> > discouraged by Guix.
> 
> If you go this road, please push patch#50515 [1] as-is.  It perfectly
> works and fixes the issue with 'guix.gnu.org/sources.json' and SWH.
> This current patch#50620 is a way to improve the readibility of
> patch#50515 but then reading all this discussion I miss why
> patch#50620 is thus a blocker for patch#50515.  Because I feel we are
> entering into the famous "Bigger problem" from xkcd. ;-)
I don't think #50515 is "perfect as-is", though.  Mark's (1) suggestion
was to put computed-origin-method into its own module, which is the
same as my long-term position.  Mark suggested a short-term fix (2) of
still comparing by eq?, which I believe you dismissed for the wrong
reasons.  Yes, you would still need to check the promise, but you
wouldn't invert said check, i.e. you would still first look for the
method and then assert that the uri makes sense.  I think it is safer
to err on the side of conservatism here rather than claim that this
code will work for future computed-origin-esque methods.

Instead of doing either (1) or (2), you opted for your own solution
(3), which is to put this method into (guix packages).  In my personal
opinion, this is a half-baked solution, because it puts computed-
origin-method into the (guix ...) without addressing any of the more
fundamental issues raised by Mark.  If you really, really can't put
this into its own module, then I'd at least suggest (3a), which is to
put it into (gnu packages) instead.  That way, the definition is at
least closer to where it's used and it's clearer that it's a hack to
package certain things, not a core part of Guix.  Perhaps you can even
make use of it without needing to rebuild the guix package in [2/2],
but don't quote me on that.

> Patch#50515 is the first part of a concrete answer to
> <https://lists.gnu.org/archive/html/guix-devel/2021-09/msg00106.html>
> and <
> https://lists.gnu.org/archive/html/guix-devel/2021-09/msg00290.html>;
> .
> It is discussed to use SWH for such situations but today our
> linux-libre is not ingested by SWH.  Therefore, let first ingest
> it--which does patch#50515.
> 
> All the best,
> simon
> 
> PS: I disagree with your last statement.  Because I am in favour for
> incremental improvements and not "The Right Thing or
> nothing".  That's out of scope of the patch at hand. ;-)
Perhaps you are right in that we can't wait for a lengthy discussion of
whether computed-origin-method can be public (guix) API or not.  Either
way, it does not look as though this thread attracts enough attention
to that issue, which is why we can ignore Mark's option (1) for now.

For the right amount of incremental change, I'd suggest the following:
Try to see whether you can do with computed-origin-method in (gnu
packages) and without rebuilding the guix package, and if so, submit a
v2 to #50515, which implements that.  Otherwise, submit a v2 to #50515
that implements Mark's option (2).

If you are also interested in the more long-term discussion of allowing
computed origins into the (guix) tree, I suggest sending a v2 patch to
this thread, which creates a new module, adds documentation, and so on,
and so forth, and also link to it on guix-devel.  For the time being,
this v2 should also refrain from touching anything that uses the
current computed-origin-method, as that can be addressed in rather
simple follow-up commits, particularly if we also implement a #50515-v2 
before.  Then we can fully use this to bikeshed about making it a verb
and what not.

WDYT?

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Mark H Weaver <mhw <at> netris.org>
Cc: Liliana Marie Prikler <liliana.prikler <at> gmail.com>, 50620 <at> debbugs.gnu.org,
 zimoun <zimon.toutoune <at> gmail.com>
Subject: Re: bug#50620: [PATCH 0/2] Unify 'computed-origin-method' (linux,
 icecat)
Date: Wed, 29 Sep 2021 15:16:16 +0200

Hi there!

I’d rather go with zimoun’s original patch, which is focused and does
nothing more than what was originally intended, which is to factorize
the procedure.  I’ll go ahead and apply it shortly if there are no
objections.


As Mark wrote, ‘computed-origin-method’ remains a hack, so we can
discuss about the best way to improve on it, but that’s a separate
discussion.  What you propose, Liliana, is one possible way to improve
on the situation, but only on the surface: the hack remains, it just
gets its own module.

A better solution IMO would be to improve the ‘snippet’ mechanism in the
first place.  ‘computed-origin-method’ improves on it in two ways: (1)
lazy evaluation of the gexp, and (2) allows the use of a different base
name.

I would think #2 is addressed by the ‘file-name’ field (isn’t it?).

As for #1, it can be addressed by making the ‘snippet’ field delayed or
thunked.  It’s a one line change; the only thing we need is to measure,
or attempt to measure, the impact it has on module load time.

Thoughts?

Ludo’.

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

From: zimoun <zimon.toutoune <at> gmail.com>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Cc: Mark H Weaver <mhw <at> netris.org>, 50620 <at> debbugs.gnu.org
Subject: Re: [PATCH 1/2] guix: packages: Document 'computed-origin-method'.
Date: Wed, 29 Sep 2021 15:17:10 +0200

Hi,

On Wed, 29 Sept 2021 at 12:10, Liliana Marie Prikler
<liliana.prikler <at> gmail.com> wrote:

> Perhaps we're bikeshedding, but you started to shed the bike when you
> chose to move this procedure to (guix packages) rather than
> implementing one of Mark's suggestions in [0].  I do think we should
> allow for bikeshedding comments from both sides if that is the case.

I do not have time to bikeshed. :-)  (Even if I like to practise it. ;-))

(Note that Mark agreed on my proposal as a variant of one of their
suggestions [0].)

0: <http://issues.guix.gnu.org/50515#5>


> I don't think #50515 is "perfect as-is", though.  Mark's (1) suggestion
> was to put computed-origin-method into its own module, which is the
> same as my long-term position.  Mark suggested a short-term fix (2) of
> still comparing by eq?, which I believe you dismissed for the wrong
> reasons.  Yes, you would still need to check the promise, but you
> wouldn't invert said check, i.e. you would still first look for the
> method and then assert that the uri makes sense.  I think it is safer
> to err on the side of conservatism here rather than claim that this
> code will work for future computed-origin-esque methods.

Maybe.  Well, I commented there [1], reworded here:

The option (1) with its own module means make computed-origin-method
public which implies a lengthy discussion, therefore it is not an
option for me.  We agree an option (1), I guess. ;-)  From my point of
view, the long-term solution is to improve snippet and remove this
computed-origin-method; not make it public.

Perhaps I am wrong about option (2) -- my claim is that
computed-origin-method is *always* used with a promise so it is for
sure an half-baked guess but enough; and it avoids to hard code the
modules from where the packages come from.  Therefore, option (2) does
not improve, IMHO.

For sure, I am right about this [1]:

--8<---------------cut here---------------start------------->8---
However, I would not like that the sources.json situation stays blocked
by the computed-origin-method situation when sources.json is produced by
the website independently of Guix, somehow. :-)
--8<---------------cut here---------------end--------------->8---

because it is exactly what it is: blocked by the
computed-origin-method situation.

1: <http://issues.guix.gnu.org/50515#4>


> Instead of doing either (1) or (2), you opted for your own solution
> (3), which is to put this method into (guix packages).  In my personal
> opinion, this is a half-baked solution, because it puts computed-
> origin-method into the (guix ...) without addressing any of the more
> fundamental issues raised by Mark.  If you really, really can't put
> this into its own module, then I'd at least suggest (3a), which is to
> put it into (gnu packages) instead.  That way, the definition is at
> least closer to where it's used and it's clearer that it's a hack to
> package certain things, not a core part of Guix.  Perhaps you can even
> make use of it without needing to rebuild the guix package in [2/2],
> but don't quote me on that.

All the solutions are half-baked! :-)  Also, generating this
sources.json using the website is half-backed at first. ;-)

Options (1) and (2) are more half-baked than my initial submission (3)
(guix packages) or (3a) (gnu packages), IMHO.

I still think that (guix packages) is better than (gnu packages).
Maintainers, what do you think?

About update guix package [2/2], it has to be done, IIUC.  The file
sources.json contains what the package guix provides, not what the
current Guix has.  The website -- built using the Guile tool haunt --
uses Guix as a Guile library.  Maybe I miss something.

> For the right amount of incremental change, I'd suggest the following:
> Try to see whether you can do with computed-origin-method in (gnu
> packages) and without rebuilding the guix package, and if so, submit a

This is what I suggested earlier ;-) [2]: send a v2 moving to '(gnu
packages)' instead and rename to 'compute-origin'.  Although I
disagree on (gnu packages). :-)

I need explanations if rebuild the guix package is not necessary.

2: <http://issues.guix.gnu.org/50620#8>

> If you are also interested in the more long-term discussion of allowing
> computed origins into the (guix) tree, I suggest sending a v2 patch to
> this thread, which creates a new module, adds documentation, and so on,
> and so forth, and also link to it on guix-devel.  For the time being,
> this v2 should also refrain from touching anything that uses the
> current computed-origin-method, as that can be addressed in rather
> simple follow-up commits, particularly if we also implement a #50515-v2
> before.  Then we can fully use this to bikeshed about making it a verb
> and what not.

For long-term, the road seems to improve the 'snippet' mechanism, not
make computed-origin-method public, IMHO.

All the best,
simon

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

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: zimoun <zimon.toutoune <at> gmail.com>
Cc: Mark H Weaver <mhw <at> netris.org>, 50620 <at> debbugs.gnu.org
Subject: Re: [PATCH 1/2] guix: packages: Document 'computed-origin-method'.
Date: Wed, 29 Sep 2021 16:36:38 +0200

Hi,

Am Mittwoch, den 29.09.2021, 15:17 +0200 schrieb zimoun:
> Hi,
> 
> On Wed, 29 Sept 2021 at 12:10, Liliana Marie Prikler
> <liliana.prikler <at> gmail.com> wrote:
> 
> > Perhaps we're bikeshedding, but you started to shed the bike when
> > you chose to move this procedure to (guix packages) rather than
> > implementing one of Mark's suggestions in [0].  I do think we
> > should allow for bikeshedding comments from both sides if that is
> > the case.
> 
> I do not have time to bikeshed. :-)  (Even if I like to practise it.
> ;-))
> 
> (Note that Mark agreed on my proposal as a variant of one of their
> suggestions [0].)
> 
> 0: <http://issues.guix.gnu.org/50515#5>
While Mark did agree to that, I still think that (guix packages) is the
wrong location for this procedure.  Multiple reviewers can hold
different opinions on that matter.

> > I don't think #50515 is "perfect as-is", though.  Mark's (1)
> > suggestion was to put computed-origin-method into its own module,
> > which is the same as my long-term position.  Mark suggested a
> > short-term fix (2) of still comparing by eq?, which I believe you
> > dismissed for the wrong reasons.  Yes, you would still need to
> > check the promise, but you wouldn't invert said check, i.e. you
> > would still first look for the method and then assert that the uri
> > makes sense.  I think it is safer to err on the side of
> > conservatism here rather than claim that this
> > code will work for future computed-origin-esque methods.
> 
> Maybe.  Well, I commented there [1], reworded here:
> 
> The option (1) with its own module means make computed-origin-method
> public which implies a lengthy discussion, therefore it is not an
> option for me.  We agree an option (1), I guess. ;-)  From my point
> of view, the long-term solution is to improve snippet and remove this
> computed-origin-method; not make it public.
I personally think there are certain cases where it would make sense to
compute origins, but they are not widely applied because computed-
origin-method is hidden and clunky.  For instance, there are several
packages, that pull in extra origins as inputs, which could however be
argued to be part of the source closure.

Again, there is room to bikeshed far and wide here and all we can agree
to currently is that we need some solution long-term.

> Perhaps I am wrong about option (2) -- my claim is that
> computed-origin-method is *always* used with a promise so it is for
> sure an half-baked guess but enough; and it avoids to hard code the
> modules from where the packages come from.  Therefore, option (2)
> does not improve, IMHO.
The probability of having a promise when using computed-origin-method
is 100%.  What is the probability of having computed-origin-method when
you see a promise?  The answer is: we don't know.  We can see from the
existing two copies of computed-origin-method, that they use promises,
but you could imagine an origin method that takes a promise only as
part of its input and then transforms it in some way under the hood. 
You could also imagine a different computed-origin-method that is
actually thunked or uses a raw gexp.

> For sure, I am right about this [1]:
> 
> --8<---------------cut here---------------start------------->8---
> However, I would not like that the sources.json situation stays
> blocked by the computed-origin-method situation when sources.json is
> produced by the website independently of Guix, somehow. :-)
> --8<---------------cut here---------------end--------------->8---
> 
> because it is exactly what it is: blocked by the
> computed-origin-method situation.
> 
> 1: <http://issues.guix.gnu.org/50515#4>
Sure, I agree that we need to divorce these discussions if this one is
going to take longer – which from the looks of it, it is.

> > Instead of doing either (1) or (2), you opted for your own solution
> > (3), which is to put this method into (guix packages).  In my
> > personal opinion, this is a half-baked solution, because it puts
> > computed-origin-method into the (guix ...) without addressing any
> > of the more fundamental issues raised by Mark.  If you really,
> > really can't put this into its own module, then I'd at least
> > suggest (3a), which is to put it into (gnu packages) instead.  That
> > way, the definition is at least closer to where it's used and it's
> > clearer that it's a hack to package certain things, not a core part
> > of Guix.  Perhaps you can even make use of it without needing to
> > rebuild the guix package in [2/2], but don't quote me on that.
> 
> All the solutions are half-baked! :-)  Also, generating this
> sources.json using the website is half-backed at first. ;-)
> 
> Options (1) and (2) are more half-baked than my initial submission
> (3) (guix packages) or (3a) (gnu packages), IMHO.
I don't think option (1) is half-baked, but it does entail making
computed-origin-method somewhat public API, which would need more
careful review than initially assumed.  (2) is half-baked indeed, but
because it is minimal effort.  Instead of touching the modules in which
the definitions are made, it just references them.

> About update guix package [2/2], it has to be done, IIUC.  The file
> sources.json contains what the package guix provides, not what the
> current Guix has.  The website -- built using the Guile tool haunt --
> uses Guix as a Guile library.  Maybe I miss something.
What I was trying to say was that you wouldn't need to rebuild the guix
package before applying the 50515 changes, which this one seems to
require.  Again, I'm not 100% sure that's the case, but IIUC (gnu
packages) is its own realm in this regard.

> > For the right amount of incremental change, I'd suggest the
> > following:  Try to see whether you can do with computed-origin-
> > method in (gnu packages) and without rebuilding the guix package,
> > and if so, submit a
> 
> This is what I suggested earlier ;-) [2]: send a v2 moving to '(gnu
> packages)' instead and rename to 'compute-origin'.  Although I
> disagree on (gnu packages). :-)
> 
> I need explanations if rebuild the guix package is not necessary.
> 
> 2: <http://issues.guix.gnu.org/50620#8>
I don't think a rename is necessary if we want a "quick and dirty"
version, this suggestion was rather made in the intention that it would
be public API.  However, if you like the naming choice, feel free to
apply it.

> > If you are also interested in the more long-term discussion of
> > allowing computed origins into the (guix) tree, I suggest sending a
> > v2 patch to this thread, which creates a new module, adds
> > documentation, and so on, and so forth, and also link to it on
> > guix-devel.  For the time being, this v2 should also refrain from
> > touching anything that uses the current computed-origin-method, as
> > that can be addressed in rather simple follow-up commits,
> > particularly if we also implement a #50515-v2
> > before.  Then we can fully use this to bikeshed about making it a
> > verb and what not.
> 
> For long-term, the road seems to improve the 'snippet' mechanism, not
> make computed-origin-method public, IMHO.
I do have some opinions on that, but I'll type them out in response to
Ludo, so as to not repeat myself too often.

Cheers

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

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>, Mark H Weaver
 <mhw <at> netris.org>
Cc: 50620 <at> debbugs.gnu.org, zimoun <zimon.toutoune <at> gmail.com>
Subject: Re: bug#50620: [PATCH 0/2] Unify 'computed-origin-method' (linux,
 icecat)
Date: Wed, 29 Sep 2021 17:34:27 +0200

Hi there,

Am Mittwoch, den 29.09.2021, 15:16 +0200 schrieb Ludovic Courtès:
> Hi there!
> 
> I’d rather go with zimoun’s original patch, which is focused and does
> nothing more than what was originally intended, which is to factorize
> the procedure.  I’ll go ahead and apply it shortly if there are no
> objections.
I have trouble understanding this paragraph.  What exactly is "this
patch" and what do you mean by "factorizing"?  If it means moving
computed-origin-method elsewhere, then yes, for a short-time solution
only moving it is a wise choice in my opinion, but zimoun and I still
disagree on the target.  zimoun says (guix packages) for reasons
unknown to me, whereas I say (gnu packages), because it's closer to
where it's used and doesn't imply that this is going to be a part of
the (guix) download schemes anytime soon.

> As Mark wrote, ‘computed-origin-method’ remains a hack, so we can
> discuss about the best way to improve on it, but that’s a separate
> discussion.  What you propose, Liliana, is one possible way to
> improve on the situation, but only on the surface: the hack remains,
> it just gets its own module.
I don't necessarily perceive computed-origin-method as a hack, though,
at least not in concept.  The current implementation may be a hack
indeed, but I don't think the very concept of expressing an input as a
function is.  We are functional programmers after all :)

> A better solution IMO would be to improve the ‘snippet’ mechanism in
> the first place.  ‘computed-origin-method’ improves on it in two
> ways: (1) lazy evaluation of the gexp, and (2) allows the use of a
> different base name.
> 
> I would think #2 is addressed by the ‘file-name’ field (isn’t it?).
> 
> As for #1, it can be addressed by making the ‘snippet’ field delayed
> or thunked.  It’s a one line change; the only thing we need is to
> measure, or attempt to measure, the impact it has on module load
> time.
> 
> Thoughts?
This would work for packages, whose source are some base source with
patches or snippets applied, as is indeed the case for linux and
icecat.  However, there are also other potential uses for computed
origins.  Grepping for the string "'unpack 'unpack", which indicates
that more sources are unpacked at build-time returns more than fifty
instances in a number of locations.  This is potentially dangerous, as
for one, we will probably want to phase out the "origins as inputs"
idiom once we have short-form inputs and also it could be argued, that
`guix build -S` returns something wrong in those packages.

I think that some version of `computed-origin-method' will eventually
need to become public API as such packages may not always be best
described as "a base package with a snippet".  If we had recursive
origins – i.e. origins, that can take origins as inputs – we might be
able to do some of that, but I don't think it would necessarily work
for linux-libre or icecat, as with those you don't want the tainted
versions to be kept around.  Perhaps this could be worked around by not
interning the intermediate origins, but only using their file-names
inside the temporary directory in which the snippet is applied?

Another thing is that the final act of the linux-libre promise is not
the packing of the tarball, but the deblob-check.  Guix currently lacks
a way of modeling such checks in their origin, but I'd argue it would
need one if we wanted to do computed origins via snippets.  This is not
required by icecat and so one "simplification" could be that computed-
origin-method would not require the user to create a tarball, but
instead simply provide a name for the tarball and a directory to create
it from (via a promise again).

A combination of the above might make computed origins obsolete for
good, but the question remains whether that is a better design.  What
do y'all think?

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

From: zimoun <zimon.toutoune <at> gmail.com>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Cc: Mark H Weaver <mhw <at> netris.org>, 50620 <at> debbugs.gnu.org
Subject: Re: [PATCH 1/2] guix: packages: Document 'computed-origin-method'.
Date: Wed, 29 Sep 2021 19:48:41 +0200

Hi,

On Wed, 29 Sept 2021 at 16:36, Liliana Marie Prikler
<liliana.prikler <at> gmail.com> wrote:

> > Perhaps I am wrong about option (2) -- my claim is that
> > computed-origin-method is *always* used with a promise so it is for
> > sure an half-baked guess but enough; and it avoids to hard code the
> > modules from where the packages come from.  Therefore, option (2)
> > does not improve, IMHO.
>
> The probability of having a promise when using computed-origin-method
> is 100%.  What is the probability of having computed-origin-method when
> you see a promise?  The answer is: we don't know.  We can see from the

You mean, what is the probability of having a computed-origin-method
when the origin-uri is a promise?  We do not know, but pragmatically,
for now 100%. :-)

Option (2) is:

 ___ (or (eq? method (@@ (gnu packages gnuzilla) computed-origin-method))
 _______ (eq? method (@@ (gnu packages linux) computed-origin-method)))

then I ask you similarly: what is the probability of having packages
using computed-origin-method in these 2 modules only?  We do not know,
but pragmatically, for now 100%. :-)

The hypothetical probabilities to evaluate are:

 - what would be the probability that a new package having a promise
as origin-uri is not indeed a package with a computed-origin-method?
vs
 - what would be the probability that a new package using
computed-origin-method is not part of either (gnu packages gnuzilla)
or (gnu packages linux)?

Anyway!  Well, I am not convinced that it is worth to tackle these
hypothetical issues. :-)

That's why the option (3):

   (eq? method (@@ (guix packages) computed-origin-method))

which means refactorize*.  It is somehow the two worlds: check i.e.,
safer, no modules hard-coded and keep private the time to have The
Right Plan for this computed-origin-method.

*refactorize: I think (guix packages) is better because it defines
'<origin>' and other tooling friends.  Because
'computed-origin-method' is somehow a temporary tool about origin,
i.e., a mechanism to define packages, it makes sense to me to put it
there.  However, (gnu packages) is about tools to deal with packages,
not to define them; although one could argue that 'search-patch' is
there is used to define package. For what my rationale behind the
choice of (guix packages) is worth.  And indeed, I have only
half-mentioned this rationale.


As I said, generating this sources.json file by the website is clunky.
Somehow, it is a quick hack to have something up waiting The Right
Way; the long-term generations should be done through the Data
Service, as it had been already discussed but not yet implemented.
Help welcome. :-)


> > About update guix package [2/2], it has to be done, IIUC.  The file
> > sources.json contains what the package guix provides, not what the
> > current Guix has.  The website -- built using the Guile tool haunt --
> > uses Guix as a Guile library.  Maybe I miss something.
>
> What I was trying to say was that you wouldn't need to rebuild the guix
> package before applying the 50515 changes, which this one seems to
> require.  Again, I'm not 100% sure that's the case, but IIUC (gnu
> packages) is its own realm in this regard.

Hum, maybe there is a misunderstanding here.  On one hand 50620
applies to the guix.git repo and on the other hand 50515 applies to
guix-artwork.git repo.

To have the sources of linux-libre and icecat reported in sources.json
and thus to get a chance to have them archived by SWH, we need:

 a- if computed-origin-method is factorized then update the guix
package (Guix as a library) else do nothing; patch applied to guix.git
 b- tweak how sources.json is built; patch applied to guix-artwork.git

Well, the aim of 50620 is to deal with a) whereas the aim of 50515 is
to deal with b).  Note that 50515 requires a v2 if
computed-origin-method is factorized.

Maybe I miss something.  From my understanding, all the modules are
part of Guix as a library.  Therefore, it does not depends on where we
refactorize.



To be honest, I thought that this tiny improvement of the SWH coverage
would have been much more easier and that that trivial task would not
have taken more than 15 days with lengthy discussions. :-)

> I do have some opinions on that, but I'll type them out in response to
> Ludo, so as to not repeat myself too often.

Thanks.  I will comment overthere or maybe raise the discussion to guix-devel.

All the best,
simon

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

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: zimoun <zimon.toutoune <at> gmail.com>
Cc: Mark H Weaver <mhw <at> netris.org>, 50620 <at> debbugs.gnu.org
Subject: Re: [PATCH 1/2] guix: packages: Document 'computed-origin-method'.
Date: Wed, 29 Sep 2021 21:10:43 +0200

Hi,

Am Mittwoch, den 29.09.2021, 19:48 +0200 schrieb zimoun:
> Hi,
> 
> On Wed, 29 Sept 2021 at 16:36, Liliana Marie Prikler
> <liliana.prikler <at> gmail.com> wrote:
> 
> > > Perhaps I am wrong about option (2) -- my claim is that
> > > computed-origin-method is *always* used with a promise so it is
> > > for sure an half-baked guess but enough; and it avoids to hard
> > > code the modules from where the packages come from.  Therefore,
> > > option (2) does not improve, IMHO.
> > 
> > The probability of having a promise when using computed-origin-
> > method is 100%.  What is the probability of having computed-origin-
> > method when you see a promise?  The answer is: we don't know.  We
> > can see from the
> 
> You mean, what is the probability of having a computed-origin-method
> when the origin-uri is a promise?  We do not know, but pragmatically,
> for now 100%. :-)
> 
> Option (2) is:
> 
>  ___ (or (eq? method (@@ (gnu packages gnuzilla) computed-origin-
> method))
>  _______ (eq? method (@@ (gnu packages linux) computed-origin-
> method)))
> 
> then I ask you similarly: what is the probability of having packages
> using computed-origin-method in these 2 modules only?  We do not
> know,
> but pragmatically, for now 100%. :-)
> 
> The hypothetical probabilities to evaluate are:
> 
>  - what would be the probability that a new package having a promise
> as origin-uri is not indeed a package with a computed-origin-method?
> vs
>  - what would be the probability that a new package using
> computed-origin-method is not part of either (gnu packages gnuzilla)
> or (gnu packages linux)?
> 
> Anyway!  Well, I am not convinced that it is worth to tackle these
> hypothetical issues. :-)
I meant that only in reference to a comparison of your original patch
in #50515 vs. option (2).  Option (2) is conservative, it only detects
computed origin which it knows how to handle.  Your original patch is
more liberal in that it detects anything that has a promise as uri as a
computed origin, which might however not have the same semantics as
those two copies of the same code.  Obviously, both (3) and (3a) don't
have this problem, but they're also conservative in that e.g. I could
roll my own channel with the exact same computed-origin-method
copypasta'd once more and it wouldn't be detected, though that's
probably off-topic.[1]

> That's why the option (3):
> 
>    (eq? method (@@ (guix packages) computed-origin-method))
> 
> which means refactorize*.  It is somehow the two worlds: check i.e.,
> safer, no modules hard-coded and keep private the time to have The
> Right Plan for this computed-origin-method.
I was a little confused when I read factorize from Ludo or refactorize
from you.  I know this technique under the name "refactoring".

> *refactorize: I think (guix packages) is better because it defines
> '<origin>' and other tooling friends.  Because
> 'computed-origin-method' is somehow a temporary tool about origin,
> i.e., a mechanism to define packages, it makes sense to me to put it
> there.  However, (gnu packages) is about tools to deal with packages,
> not to define them; although one could argue that 'search-patch' is
> there is used to define package. For what my rationale behind the
> choice of (guix packages) is worth.  And indeed, I have only
> half-mentioned this rationale.
To that I would counter, that (guix packages) only defines package and
origin records and how to compile them to bags and derivations.  All
the methods through which those fields are filled are defined outside,
be it the occasional local-file used in "Build it with Guix"-style
recipes or the closer to our methods svn-fetch and git-fetch.

Were it not for the fact that it uses procedures defined in (guix
packages), you might have a better time reasoning that this should be a
hidden part of (guix gexp), but since it does, it would introduce a
cycle if you did.  While (gnu packages) does offer "tools to deal with
packages" as you put it, I'd argue the way it does is in establishing a
namespace for them (fold-packages etc.) and that this namespace (the
"GNU namespace") is the best location for computed-origin-method.

The reason we use computed-origin-method at all, is because as GNU Guix
we take a hard stance on the FSDG and do our damndest not to lead users
to nonfree software.  This includes producing clean --source tarballs. 
Were it not for that, we could easily deblob at build time, and this is
an important observation to make, because it means that projects that
don't need this level of control can "safely" defer it the way we do
currently for "unpack more after unpack".[2]  In other words, I
strongly believe that users of compute-origin-method do the hard work
of computing origins to follow GNU standards and will thereby have no
issue referencing the GNU namespace to get to it.

> As I said, generating this sources.json file by the website is
> clunky.
> Somehow, it is a quick hack to have something up waiting The Right
> Way; the long-term generations should be done through the Data
> Service, as it had been already discussed but not yet implemented.
> Help welcome. :-)
I have no opinion on that as I don't work on the Data Service.

> > > About update guix package [2/2], it has to be done, IIUC.  The
> > > file
> > > sources.json contains what the package guix provides, not what
> > > the
> > > current Guix has.  The website -- built using the Guile tool
> > > haunt --
> > > uses Guix as a Guile library.  Maybe I miss something.
> > 
> > What I was trying to say was that you wouldn't need to rebuild the
> > guix package before applying the 50515 changes, which this one
> > seems to require.  Again, I'm not 100% sure that's the case, but
> > IIUC (gnu packages) is its own realm in this regard.
> 
> Hum, maybe there is a misunderstanding here.  On one hand 50620
> applies to the guix.git repo and on the other hand 50515 applies to
> guix-artwork.git repo.
Oh, I somehow missed that completely.  Oops.

> To have the sources of linux-libre and icecat reported in
> sources.json and thus to get a chance to have them archived by SWH,
> we need:
> 
>  a- if computed-origin-method is factorized then update the guix
> package (Guix as a library) else do nothing; patch applied to
> guix.git
>  b- tweak how sources.json is built; patch applied to guix-
> artwork.git
> 
> Well, the aim of 50620 is to deal with a) whereas the aim of 50515 is
> to deal with b).  Note that 50515 requires a v2 if
> computed-origin-method is factorized.
> 
> Maybe I miss something.  From my understanding, all the modules are
> part of Guix as a library.  Therefore, it does not depends on where
> we refactorize.
No, no, I was wrongly under the impression that this sources.json would
be generated by Guix itself what with all the other SWH interaction we
already have.  Again, a mistake on my part.

> To be honest, I thought that this tiny improvement of the SWH
> coverage would have been much more easier and that that trivial task
> would not have taken more than 15 days with lengthy discussions. :-)
To be honest, part of the lengthy discussion was me being confused
about your intent – in multiple ways.  If you wanted a "quick and dirty
fix" you could have went with checking those two modules explicitly on
the guix-artwork side and it would have had a fairly small impact. 
Reading this patch first and the discussion second, I had assumed your
intent was rather to formalize a method that had hitherto only been
used informally and the move to the guix namespace amplifies that imo.

All the best,
Liliana

[1] The only solution is of course to compare via equal? instead of
eq?, which if implemented for functions would be Turing-hard :)
[2] Of course, this assumes that other projects don't want to actually
unpack dependencies when using --source, but I think it's likelier they
will just use recursive submodule checkout.

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

From: zimoun <zimon.toutoune <at> gmail.com>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Cc: Mark H Weaver <mhw <at> netris.org>, 50620 <at> debbugs.gnu.org
Subject: Re: [PATCH 1/2] guix: packages: Document 'computed-origin-method'.
Date: Wed, 29 Sep 2021 22:15:02 +0200

Hi Liliana,

On Wed, 29 Sep 2021 at 21:10, Liliana Marie Prikler <liliana.prikler <at> gmail.com> wrote:

>                                                               I could
> roll my own channel with the exact same computed-origin-method
> copypasta'd once more and it wouldn't be detected, though that's
> probably off-topic.[1]

If it is in your own channel, then it will not be part of the file
https://guix.gnu.org/sources.json.

From my understanding, you are arguing about corner cases that does not
happen now.  And if it happens in the near future, we will fix it,
depending on what will really happen in this very future. ;-)


> I was a little confused when I read factorize from Ludo or refactorize
> from you.  I know this technique under the name "refactoring".

Indeed.  Maybe a false-friend from French. :-)


>> *refactorize: I think (guix packages) is better because it defines

[...]

>> half-mentioned this rationale.
>
> To that I would counter, that (guix packages) only defines package and

[...]

> issue referencing the GNU namespace to get to it.

I hear your argument.  Well, I will not discuss it.  Raise as an answer
to Ludo, maybe.


>> To be honest, I thought that this tiny improvement of the SWH
>> coverage would have been much more easier and that that trivial task
>> would not have taken more than 15 days with lengthy discussions. :-)
>
> To be honest, part of the lengthy discussion was me being confused
> about your intent – in multiple ways.  If you wanted a "quick and dirty
> fix" you could have went with checking those two modules explicitly on
> the guix-artwork side and it would have had a fairly small impact.
> Reading this patch first and the discussion second, I had assumed your
> intent was rather to formalize a method that had hitherto only been
> used informally and the move to the guix namespace amplifies that imo.

The cover letter [1] says: «This patch follows the discussion from [0].»
where [0] points to the Mark’s approval as an answer to a patch which
applies to website/apps/packages/builder.scm.

Then the cover letter [1] says: «In short, it simplifies the code
generating the file 'sources.json' used by Software Heritage to ingest
all the tarballs.»

1: <http://issues.guix.gnu.org/50620#0>

I am sorry if this cover letter was not enough explicit about my intent.
From my point of view, the aim of this cover letter was to invite to
read first the discussion and second read the patch.  My bad if this aim
had been missed.  I apologize for the confusion.

Being optimistic, this discussion leads to some concerns about this
’computed-origin-method’ and ideas for improving.  IMHO, it is worth to
open another issue providing the wish of multi-origin packages and
reference to this.  WDYT?

All the best,
simon

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

From: Mark H Weaver <mhw <at> netris.org>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: Liliana Marie Prikler <liliana.prikler <at> gmail.com>, 50620 <at> debbugs.gnu.org,
 zimoun <zimon.toutoune <at> gmail.com>
Subject: Re: bug#50620: [PATCH 0/2] Unify 'computed-origin-method' (linux,
 icecat)
Date: Wed, 29 Sep 2021 16:42:21 -0400

Hi Ludovic,

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

> A better solution IMO would be to improve the ‘snippet’ mechanism in the
> first place.  ‘computed-origin-method’ improves on it in two ways: (1)
> lazy evaluation of the gexp, and (2) allows the use of a different base
> name.
>
> I would think #2 is addressed by the ‘file-name’ field (isn’t it?).

Using the 'file-name' field would not satisfy my requirements.  It has
two problems:

(1) It would cause the unmodified upstream Firefox source tarball to be
    named "icecat-…" in the store.

(2) Although the resulting tarball would be named "icecat-…", the
    toplevel directory name within that tarball would still be named
    "firefox-…".

I consider each of these flaws to be unacceptable.

What do you think?

      Thanks,
        Mark

-- 
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about <https://stallmansupport.org>.

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Mark H Weaver <mhw <at> netris.org>
Cc: Liliana Marie Prikler <liliana.prikler <at> gmail.com>, 50620 <at> debbugs.gnu.org,
 zimoun <zimon.toutoune <at> gmail.com>
Subject: Re: bug#50620: [PATCH 0/2] Unify 'computed-origin-method' (linux,
 icecat)
Date: Wed, 29 Sep 2021 23:34:16 +0200

Hi Mark,

Mark H Weaver <mhw <at> netris.org> skribis:

> Ludovic Courtès <ludo <at> gnu.org> writes:
>
>> A better solution IMO would be to improve the ‘snippet’ mechanism in the
>> first place.  ‘computed-origin-method’ improves on it in two ways: (1)
>> lazy evaluation of the gexp, and (2) allows the use of a different base
>> name.
>>
>> I would think #2 is addressed by the ‘file-name’ field (isn’t it?).
>
> Using the 'file-name' field would not satisfy my requirements.  It has
> two problems:
>
> (1) It would cause the unmodified upstream Firefox source tarball to be
>     named "icecat-…" in the store.
>
> (2) Although the resulting tarball would be named "icecat-…", the
>     toplevel directory name within that tarball would still be named
>     "firefox-…".
>
> I consider each of these flaws to be unacceptable.

Oh I see.  I agree it’d be nice to have a way to fix those.

Perhaps by allowing for custom pack-and-repack procedures?

Thanks,
Ludo’.

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

From: Mark H Weaver <mhw <at> netris.org>
To: zimoun <zimon.toutoune <at> gmail.com>, Liliana Marie Prikler
 <liliana.prikler <at> gmail.com>
Cc: 50620 <at> debbugs.gnu.org
Subject: Re: [PATCH 1/2] guix: packages: Document 'computed-origin-method'.
Date: Wed, 29 Sep 2021 17:40:39 -0400

Hi Simon,

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

> On Wed, 29 Sept 2021 at 16:36, Liliana Marie Prikler
> <liliana.prikler <at> gmail.com> wrote:
>
>> > Perhaps I am wrong about option (2) -- my claim is that
>> > computed-origin-method is *always* used with a promise so it is for
>> > sure an half-baked guess but enough; and it avoids to hard code the
>> > modules from where the packages come from.  Therefore, option (2)
>> > does not improve, IMHO.
>>
>> The probability of having a promise when using computed-origin-method
>> is 100%.  What is the probability of having computed-origin-method when
>> you see a promise?  The answer is: we don't know.  We can see from the
>
> You mean, what is the probability of having a computed-origin-method
> when the origin-uri is a promise?  We do not know, but pragmatically,
> for now 100%. :-)

To my mind, that's not good enough.  I consider it unsafe, and poor
programming practice, to force a promise without first knowing what that
promise represents and what are the implications of forcing it.

In projects as large as Guix, if it becomes accepted practice to
introduce lots of assumptions scattered around the code that are
"for now 100%" true, the result is eventually a very brittle project
where it's difficult to make changes without random stuff breaking.

> Option (2) is:
>
>  ___ (or (eq? method (@@ (gnu packages gnuzilla) computed-origin-method))
>  _______ (eq? method (@@ (gnu packages linux) computed-origin-method)))
>
> then I ask you similarly: what is the probability of having packages
> using computed-origin-method in these 2 modules only?  We do not know,
> but pragmatically, for now 100%. :-)

The potential failure mode here is far less bad.  In this case, if
someone else makes another clone of 'computed-origin-method' in another
module and forgets to update this code, the worst case is that some
source code fails to be added to SWH.  Incidentally, I guess that's the
same outcome that would happen if someone adds a brand new
'origin-method' and forgets to update this code.

Incidentally, I have a suggestion for how to avoid that failure mode
properly, once and for all: issue a warning if we're unable to identify
the 'method' of the origin at hand, calling attention to the fact that
there's an unhandled case in this code.  This is precisely analogous to
Standard ML's *very* useful feature of issuing warnings at compile time
in case of an non-exhaustive 'match' form.

What do you think?

In any case, thanks very much for your efforts to push this issue toward
resolution.

      Regards,
        Mark

-- 
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about <https://stallmansupport.org>.

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Cc: Mark H Weaver <mhw <at> netris.org>, 50620 <at> debbugs.gnu.org,
 zimoun <zimon.toutoune <at> gmail.com>
Subject: Re: bug#50620: [PATCH 0/2] Unify 'computed-origin-method' (linux,
 icecat)
Date: Wed, 29 Sep 2021 23:47:40 +0200

Hi Liliana,

Liliana Marie Prikler <liliana.prikler <at> gmail.com> skribis:

> Am Mittwoch, den 29.09.2021, 15:16 +0200 schrieb Ludovic Courtès:
>> Hi there!
>> 
>> I’d rather go with zimoun’s original patch, which is focused and does
>> nothing more than what was originally intended, which is to factorize
>> the procedure.  I’ll go ahead and apply it shortly if there are no
>> objections.
> I have trouble understanding this paragraph.  What exactly is "this
> patch" and what do you mean by "factorizing"?  If it means moving
> computed-origin-method elsewhere, then yes, for a short-time solution
> only moving it is a wise choice in my opinion,

OK, I agree too.

> but zimoun and I still disagree on the target.  zimoun says (guix
> packages) for reasons unknown to me, whereas I say (gnu packages),
> because it's closer to where it's used and doesn't imply that this is
> going to be a part of the (guix) download schemes anytime soon.

(gnu packages) is higher-level: it’s part of the distro and includes CLI
helpers such as ‘specification->package’.  So I think (guix …) is
somewhat more appropriate.

(That said, what matters more to me is how we’re going to replace it
with a proper solution.)

[...]

>> A better solution IMO would be to improve the ‘snippet’ mechanism in
>> the first place.  ‘computed-origin-method’ improves on it in two
>> ways: (1) lazy evaluation of the gexp, and (2) allows the use of a
>> different base name.
>> 
>> I would think #2 is addressed by the ‘file-name’ field (isn’t it?).
>> 
>> As for #1, it can be addressed by making the ‘snippet’ field delayed
>> or thunked.  It’s a one line change; the only thing we need is to
>> measure, or attempt to measure, the impact it has on module load
>> time.
>> 
>> Thoughts?
> This would work for packages, whose source are some base source with
> patches or snippets applied, as is indeed the case for linux and
> icecat.  However, there are also other potential uses for computed
> origins.

It’s hard for me to talk about potential uses in the abstract.  :-)

There might be cases where an origin simply isn’t the right tool and one
would prefer ‘computed-file’ or something else.  It really depends on
the context.

[...]

> I think that some version of `computed-origin-method' will eventually
> need to become public API as such packages may not always be best
> described as "a base package with a snippet".  If we had recursive
> origins – i.e. origins, that can take origins as inputs – we might be
> able to do some of that, but I don't think it would necessarily work
> for linux-libre or icecat, as with those you don't want the tainted
> versions to be kept around.  Perhaps this could be worked around by not
> interning the intermediate origins, but only using their file-names
> inside the temporary directory in which the snippet is applied?

“Recursive origins” are a bit of a stretch as a concept IMO; what you
describe is a case where I’d probably use ‘computed-file’ instead.

> Another thing is that the final act of the linux-libre promise is not
> the packing of the tarball, but the deblob-check.  Guix currently lacks
> a way of modeling such checks in their origin, but I'd argue it would
> need one if we wanted to do computed origins via snippets.  This is not
> required by icecat and so one "simplification" could be that computed-
> origin-method would not require the user to create a tarball, but
> instead simply provide a name for the tarball and a directory to create
> it from (via a promise again).

Ah, I had overlooked that ‘deblob-check’ bit.  It could be that allowing
for custom pack-and-repack procedures would be enough to address it.

> A combination of the above might make computed origins obsolete for
> good, but the question remains whether that is a better design.  What
> do y'all think?

The design goal is to have clearly identified types: <package>,
<origin>, <operating-system>.  For each of these, we want some
flexibility: build system, origin method, etc.  However, beyond some
level of stretching, it may be clearer to just use the catch-all
‘computed-file’ or to devise a new type.  After all, that’s how <origin>
came to be (we could have used <package> instead with a suitable build
system).

There’s a tension between “purely declarative” and “flexible”, and it’s
about striking a balance, subjectively.

Hope that makes sense!

Ludo’.

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

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: zimoun <zimon.toutoune <at> gmail.com>
Cc: Mark H Weaver <mhw <at> netris.org>, 50620 <at> debbugs.gnu.org
Subject: Re: [PATCH 1/2] guix: packages: Document 'computed-origin-method'.
Date: Thu, 30 Sep 2021 00:13:48 +0200

Hi zimoun,

Am Mittwoch, den 29.09.2021, 22:15 +0200 schrieb zimoun:
> Hi Liliana,
> 
> On Wed, 29 Sep 2021 at 21:10, Liliana Marie Prikler <
> liliana.prikler <at> gmail.com> wrote:
> 
> > I could roll my own channel with the exact same computed-origin-
> > method copypasta'd once more and it wouldn't be detected, though
> > that's probably off-topic.[1]
> 
> If it is in your own channel, then it will not be part of the file
> https://guix.gnu.org/sources.json.
> 
> From my understanding, you are arguing about corner cases that does
> not happen now.  And if it happens in the near future, we will fix
> it, depending on what will really happen in this very future. ;-)
The patch mentions "automatic detection of computed-origin-method"
which I would assume has implication beyond this sources.json.  But
yeah, I can see that it's off-topic to this discussion, hence why I
wrote that it's probably off-topic to this dicussion.

> > > *refactorize: I think (guix packages) is better because it
> > > defines
> 
> [...]
> 
> > > half-mentioned this rationale.
> > 
> > To that I would counter, that (guix packages) only defines package
> > and
> 
> [...]
> 
> > issue referencing the GNU namespace to get to it.
> 
> I hear your argument.  Well, I will not discuss it.  Raise as an
> answer to Ludo, maybe.
I did already mention that in my reply to Ludo, so we'll see.

> > > To be honest, I thought that this tiny improvement of the SWH
> > > coverage would have been much more easier and that that trivial
> > > task
> > > would not have taken more than 15 days with lengthy discussions.
> > > :-)
> > 
> > To be honest, part of the lengthy discussion was me being confused
> > about your intent – in multiple ways.  If you wanted a "quick and
> > dirty fix" you could have went with checking those two modules
> > explicitly on the guix-artwork side and it would have had a fairly
> > small impact.
> > Reading this patch first and the discussion second, I had assumed
> > your intent was rather to formalize a method that had hitherto only
> > been used informally and the move to the guix namespace amplifies
> > that imo.
> 
> The cover letter [1] says: «This patch follows the discussion from
> [0].» where [0] points to the Mark’s approval as an answer to a patch
> which applies to website/apps/packages/builder.scm.
> 
> Then the cover letter [1] says: «In short, it simplifies the code
> generating the file 'sources.json' used by Software Heritage to
> ingest all the tarballs.»
> 
> 1: <http://issues.guix.gnu.org/50620#0>
> 
> I am sorry if this cover letter was not enough explicit about my
> intent.  From my point of view, the aim of this cover letter was to
> invite to read first the discussion and second read the patch.  My
> bad if this aim had been missed.  I apologize for the confusion.
Again, I read the patch itself first and the context second, but
speaking about "simplifying the code generating sources.json", the real
change were we to compare (2) and (3) or (3a) to each other would be a
3 line diff (two deletions, one insertion).  So I do think it is fair
to also talk about implications beyond those three lines.

Also, even with this context in mind, the patch at first appeared to me
as a way of sneaking (1) past the radar, rather than the three-line
diff that one would see when looking at it from 50515 with (2) applied.

> Being optimistic, this discussion leads to some concerns about this
> ’computed-origin-method’ and ideas for improving.  IMHO, it is worth
> to open another issue providing the wish of multi-origin packages and
> reference to this.  WDYT?
Since it's but an idea sketch in my head at the moment, I think the
best we could muster discussing this outside of this thread would be on
guix-devel.  Which is fine and all, but since we're looking in this
thread for something comparatively small in scale I'd say let's look at
the small issue first and the big issue once we've fixed the small one.

Let's shortly recap what options we have.

A: Push a v2 of 50515 guix-artwork, which references (gnu packages
linux) and (gnu packages gnuzilla) using @@.  Then decide on which
module we want to have computed-origin-method to be in and update the
guix package.  Finally, update the sources.json generator to use the
singular reference.

B: Push the lazy v2 as above, but instead hold up the cleaning up part
until we find a solution for the computed-origin-method in this thread
or guix-devel.  

C: Discuss the (gnu packages) vs. (guix packages) thing some more,
merge this patch (with perhaps a move), update the guix package and
then do a v2 of 50515.

C2: Have Ludo flip a coin and decide.

D: Have computed-origin-method block the sources.json generator until
it is completely resolved.

We obviously want to avoid D here and are somewhat aiming for C at the
moment instead.  However, we are kinda stuck here as even though we
don't want this situation to continue indefinitely, we can't seem to
reach a consensus quickly.

WDYT?  Does it make sense to do the "redundant test" [1], knowing that
it'll be soon simplified?  Can we hold off more computed-origin-method
clones until we find a way of making do without it or actually decide
that it's public API?

All the best,
Liliana

[1] <http://issues.guix.gnu.org/50515#4>

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

From: zimoun <zimon.toutoune <at> gmail.com>
To: Mark H Weaver <mhw <at> netris.org>, Liliana Marie Prikler
 <liliana.prikler <at> gmail.com>
Cc: 50620 <at> debbugs.gnu.org
Subject: Re: [PATCH 1/2] guix: packages: Document 'computed-origin-method'.
Date: Thu, 30 Sep 2021 00:45:58 +0200

Hi Mark,

On Wed, 29 Sep 2021 at 17:40, Mark H Weaver <mhw <at> netris.org> wrote:
> zimoun <zimon.toutoune <at> gmail.com> writes:

> To my mind, that's not good enough.  I consider it unsafe, and poor
> programming practice, to force a promise without first knowing what that
> promise represents and what are the implications of forcing it.
>
> In projects as large as Guix, if it becomes accepted practice to
> introduce lots of assumptions scattered around the code that are
> "for now 100%" true, the result is eventually a very brittle project
> where it's difficult to make changes without random stuff breaking.

I agree…

>> Option (2) is:
>>
>>  ___ (or (eq? method (@@ (gnu packages gnuzilla) computed-origin-method))
>>  _______ (eq? method (@@ (gnu packages linux) computed-origin-method)))
>>
>> then I ask you similarly: what is the probability of having packages
>> using computed-origin-method in these 2 modules only?  We do not know,
>> but pragmatically, for now 100%. :-)
>
> The potential failure mode here is far less bad.  In this case, if
> someone else makes another clone of 'computed-origin-method' in another
> module and forgets to update this code, the worst case is that some
> source code fails to be added to SWH.  Incidentally, I guess that's the
> same outcome that would happen if someone adds a brand new
> 'origin-method' and forgets to update this code.

…and I also agree.  That’s why, right after this quotation, I wrote:

        That's why the option (3):

           (eq? method (@@ (guix packages) computed-origin-method))

        which means refactorize*.  It is somehow the two worlds: check i.e.,
        safer, no modules hard-coded and keep private the time to have The
        Right Plan for this computed-origin-method.

which is *exactly* what you are asking, IIUC.

To be honest, I do not understand why we are discussing at length this
trivial path:

for guix.git:

 1. move the duplicate computed-origin-method to a single place
 2. keep it private
 3. add comments about that

for guix-artwork.git:

 4. guard the promise using a check against:
      (@@ (module somewhere) computed-origin-method)

for guix.git

 5. update the package guix

done! :-)

It changes nothing on the Guix side.

Do we not agree on this trivial path?  Re-reading from the starting
point 50515 and then 50620, the trivial road appears to me clear.  I
apologize if it was not or to not make it explicit earlier.

From my understanding, we all agree, somehow; because it fixes the
current situation and let the time to cook The Right Plan for this
computed-origin-method.  Where we can discuss is #2 but as it is already
mentioned, it is out of scope for sources.json, IMHO.

If I knew all what would happen, then I would send a v2 for 50515 using
what you described as option (2). :-) My aim with this 50620 was just to
simplify at very low cost the code (belonging to the repo
guix-artwork.git) which generates <https://guix.gnu.org/sources.json>.

The v2 (adding a guard) for 50515 is simply waiting the output of this
50620; because this guard depends if computed-origin-method is defined
at an unique location or at two different locations.
  

> Incidentally, I have a suggestion for how to avoid that failure mode
> properly, once and for all: issue a warning if we're unable to identify
> the 'method' of the origin at hand, calling attention to the fact that
> there's an unhandled case in this code.  This is precisely analogous to
> Standard ML's *very* useful feature of issuing warnings at compile time
> in case of an non-exhaustive 'match' form.

The SWH reader which consumes this sources.json file does not care about
a warning.  And AFAIK no one is reviewing by hand this sources.json
file.  Specifically, the only purpose of this very file is to be
consumed by the SWH infrastructure.  It is automatically generated when
the Guix website rebuilds; the content of this file depends on the
version of the package guix.

That’s said, there is room of improvement to track what is the archival
coverage by SWH.  Today, we do not have a clear picture of the packages
archived by SWH.  By archived, it means packages for which Guix is able
to fallback and lookup using the SWH archive.  Well, now one foot, now
the other. :-)

(On a side note, I agree that this ML feature is very useful.  From my
understanding, it requires a kind of type system that Guile does not
have.  Sadly.)

> In any case, thanks very much for your efforts to push this issue toward
> resolution.

Thanks.  At one point, I felt demotivated then reconsidering the energy
we all are putting it, we have to resolve. :-)

All the best,
simon

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

From: zimoun <zimon.toutoune <at> gmail.com>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Cc: Mark H Weaver <mhw <at> netris.org>, 50620 <at> debbugs.gnu.org
Subject: Re: [PATCH 1/2] guix: packages: Document 'computed-origin-method'.
Date: Thu, 30 Sep 2021 01:31:04 +0200

Hi,

On Thu, 30 Sep 2021 at 00:13, Liliana Marie Prikler <liliana.prikler <at> gmail.com> wrote:

> C: Discuss the (gnu packages) vs. (guix packages) thing some more,
> merge this patch (with perhaps a move), update the guix package and
> then do a v2 of 50515.

This is the option I am for.  Even, the patch is ready and waiting since
«Fri, 10 Sep 2021 18:01:22 +0200». ;-)

The patch reads:

--8<---------------cut here---------------start------------->8---
+  (if (eq? method (@@ (guix packages) computed-origin-method))
+      ;; Packages in gnu/packages/gnuzilla.scm and gnu/packages/linux.scm
+      ;; represent their 'uri' as 'promise'.
+      (match uri
+        ((? promise? promise)

[...]

+           (_ `((type . #nil))))))
+      ;;Regular packages represent 'uri' as string.
+      `((type . ,(cond ((or (eq? url-fetch method)
--8<---------------cut here---------------end--------------->8---

and I find better (guix packages) but I do not have a strong opinion; I
accepted previously in this thread to send a v2 with (gnu packages) or
whatever other location.


> WDYT?  Does it make sense to do the "redundant test" [1], knowing that
> it'll be soon simplified?

I do not mind about option (2) which reads:

--8<---------------cut here---------------start------------->8---
+  (if (or (eq? method (@@ (gnu packages linux) computed-origin-method))
+          (eq? method (@@ (gnu packages gnuzilla) computed-origin-method)))
+      (match uri
+        ((? promise? promise)

[...]

+           (_ `((type . #nil))))))
+      ;;Regular packages represent 'uri' as string.
+      `((type . ,(cond ((or (eq? url-fetch method)
--8<---------------cut here---------------end--------------->8---

Whatever.

However, since it is me who takes care about how this sources.json is
generated, I find easier to have one location and forget about this
case.  The only thing I am asking here with this patch 50620 is to
locate computed-origin-method to one unique place.  If people strongly
disagree, then let do this option (2) and move on.

Last, I am confused why all this is so complicated when it is trivial
and for something outside Guix proper.  I do not understand what we are
discussing when my request is trivial, IMHO.

This discussion has eaten all my energy allowed for Guix.
See you next week.

All the best,
simon

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

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: Mark H Weaver <mhw <at> netris.org>, 50620 <at> debbugs.gnu.org,
 zimoun <zimon.toutoune <at> gmail.com>
Subject: Re: bug#50620: [PATCH 0/2] Unify 'computed-origin-method' (linux,
 icecat)
Date: Thu, 30 Sep 2021 01:44:37 +0200

Hi Ludo,

Am Mittwoch, den 29.09.2021, 23:47 +0200 schrieb Ludovic Courtès:
> [...]
> 
> > but zimoun and I still disagree on the target.  zimoun says (guix
> > packages) for reasons unknown to me, whereas I say (gnu packages),
> > because it's closer to where it's used and doesn't imply that this
> > is
> > going to be a part of the (guix) download schemes anytime soon.
> 
> (gnu packages) is higher-level: it’s part of the distro and includes
> CLI helpers such as ‘specification->package’.  So I think (guix …) is
> somewhat more appropriate.
> 
> (That said, what matters more to me is how we’re going to replace it
> with a proper solution.)
(gnu packages) being high-level is part of the reason I want it there. 
Stuff that's hidden quite deep inside (guix something) will be slower
to change and replace with the proper solution.  When you pull on a
lever, the outside moves faster :)

> > > A better solution IMO would be to improve the ‘snippet’ mechanism
> > > in the first place.  ‘computed-origin-method’ improves on it in
> > > two ways: (1) lazy evaluation of the gexp, and (2) allows the use
> > > of a different base name.
> > > 
> > > I would think #2 is addressed by the ‘file-name’ field (isn’t
> > > it?).
> > > 
> > > As for #1, it can be addressed by making the ‘snippet’ field
> > > delayed or thunked.  It’s a one line change; the only thing we
> > > need is to measure, or attempt to measure, the impact it has on
> > > module load time.
> > > 
> > > Thoughts?
> > This would work for packages, whose source are some base source
> > with patches or snippets applied, as is indeed the case for linux
> > and icecat.  However, there are also other potential uses for
> > computed origins.
> 
> It’s hard for me to talk about potential uses in the abstract.  :-)
> 
> There might be cases where an origin simply isn’t the right tool and
> one would prefer ‘computed-file’ or something else.  It really
> depends on the context.
> 
> [...]
> 
> > I think that some version of `computed-origin-method' will
> > eventually need to become public API as such packages may not
> > always be best described as "a base package with a snippet".  If we
> > had recursive origins – i.e. origins, that can take origins as
> > inputs – we might be able to do some of that, but I don't think it
> > would necessarily work for linux-libre or icecat, as with those you
> > don't want the tainted versions to be kept around.  Perhaps this
> > could be worked around by not interning the intermediate origins,
> > but only using their file-names inside the temporary directory in
> > which the snippet is applied?
> 
> “Recursive origins” are a bit of a stretch as a concept IMO; what you
> describe is a case where I’d probably use ‘computed-file’ instead.
In other words, we could/should use computed-file for linux-libre and
icecat?  If we reasonably can, would it make sense to use that in lieu
of computed-origin-method to actually advertise the existence of
computed-file to Guix users/packagers?

> > Another thing is that the final act of the linux-libre promise is
> > not the packing of the tarball, but the deblob-check.  Guix
> > currently lacks a way of modeling such checks in their origin, but
> > I'd argue it would need one if we wanted to do computed origins via
> > snippets.  This is not required by icecat and so one
> > "simplification" could be that computed-origin-method would not
> > require the user to create a tarball, but instead simply provide a
> > name for the tarball and a directory to create it from (via a
> > promise again).
> 
> Ah, I had overlooked that ‘deblob-check’ bit.  It could be that
> allowing for custom pack-and-repack procedures would be enough to
> address it.
I think asking users to supply their own implementation of a 200 line
long function to be a bit much to only do part of the job.  On the
other hand, the promise for linux-libre takes 400 lines and for icecat
more than 600, but I think there are some things we ought to factor
out.  Particularly, looking up tools like tar or gzip and even the
actual packing are always the same.

What we can't currently control is the top directory name and the
output name.  Both of that could be customized by supplying a "repack-
name" field, which is used as basis for the directory name and the
tarball name.
Another thing we can't easily control are extraneous inputs to the
patches, although the patch-inputs field *does* exist.

> > A combination of the above might make computed origins obsolete for
> > good, but the question remains whether that is a better
> > design.  What do y'all think?
> 
> The design goal is to have clearly identified types: <package>,
> <origin>, <operating-system>.  For each of these, we want some
> flexibility: build system, origin method, etc.  However, beyond some
> level of stretching, it may be clearer to just use the catch-all
> ‘computed-file’ or to devise a new type.  After all, that’s how
> <origin> came to be (we could have used <package> instead with a
> suitable build system).
> 
> There’s a tension between “purely declarative” and “flexible”, and
> it’s about striking a balance, subjectively.
To be fair, I did think that "computed-tarball" might be a good
abstraction in some sense, but on another hand origins are computed
tarballs with a record interface.

On a somewhat related note, origins have this weird situation going on
where some things like git or svn checkouts need to be defined through
them, whereas others may pass unhindered.  I feel that this contributes
to the equation of source = origin, that might have caused "computed-
origin-method" to exist in the first place.

What do you think?

Liliana

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

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: zimoun <zimon.toutoune <at> gmail.com>, Mark H Weaver <mhw <at> netris.org>
Cc: 50620 <at> debbugs.gnu.org
Subject: Re: [PATCH 1/2] guix: packages: Document 'computed-origin-method'.
Date: Thu, 30 Sep 2021 09:11:15 +0200

Hi zimoun,

Am Donnerstag, den 30.09.2021, 00:45 +0200 schrieb zimoun:
> To be honest, I do not understand why we are discussing at length
> this trivial path:
> 
> for guix.git:
> 
>  1. move the duplicate computed-origin-method to a single place
>  2. keep it private
>  3. add comments about that
> 
> for guix-artwork.git:
> 
>  4. guard the promise using a check against:
>       (@@ (module somewhere) computed-origin-method)
> 
> for guix.git
> 
>  5. update the package guix
> 
> done! :-)
> 
> It changes nothing on the Guix side.
I've started discussing this path because we are currently stuck at 1.
for some time.  Given that this drags on for so long and you are
looking for a "quick solution" for guix-artwork.git, I called into
question whether it is really necessary to modify guix.git first.  This
does not change nothing for the Guix side either, it changes the
location of a hack most of us wish to rather avoid than to give more
attention to in the code base :)

Sorry for forcing you through this, I had not intended to spark this
lengthy debates.

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Cc: Mark H Weaver <mhw <at> netris.org>, 50620 <at> debbugs.gnu.org,
 zimoun <zimon.toutoune <at> gmail.com>
Subject: Re: bug#50620: [PATCH 0/2] Unify 'computed-origin-method' (linux,
 icecat)
Date: Thu, 30 Sep 2021 10:28:24 +0200

Hi!

Liliana Marie Prikler <liliana.prikler <at> gmail.com> skribis:

> I think asking users to supply their own implementation of a 200 line
> long function to be a bit much to only do part of the job.  On the
> other hand, the promise for linux-libre takes 400 lines and for icecat
> more than 600, but I think there are some things we ought to factor
> out.  Particularly, looking up tools like tar or gzip and even the
> actual packing are always the same.

True, there’s a lot going on there, though that’s partly because it’s
generic.

> What we can't currently control is the top directory name and the
> output name.  Both of that could be customized by supplying a "repack-
> name" field, which is used as basis for the directory name and the
> tarball name.
> Another thing we can't easily control are extraneous inputs to the
> patches, although the patch-inputs field *does* exist.

It’s possible to use a gexp as the snippet, where you can refer to
additional things in there (though in practice this is currently
impractical due to snippets not being thunks/promises.)

>> > A combination of the above might make computed origins obsolete for
>> > good, but the question remains whether that is a better
>> > design.  What do y'all think?
>> 
>> The design goal is to have clearly identified types: <package>,
>> <origin>, <operating-system>.  For each of these, we want some
>> flexibility: build system, origin method, etc.  However, beyond some
>> level of stretching, it may be clearer to just use the catch-all
>> ‘computed-file’ or to devise a new type.  After all, that’s how
>> <origin> came to be (we could have used <package> instead with a
>> suitable build system).
>> 
>> There’s a tension between “purely declarative” and “flexible”, and
>> it’s about striking a balance, subjectively.
> To be fair, I did think that "computed-tarball" might be a good
> abstraction in some sense, but on another hand origins are computed
> tarballs with a record interface.
>
> On a somewhat related note, origins have this weird situation going on
> where some things like git or svn checkouts need to be defined through
> them, whereas others may pass unhindered.  I feel that this contributes
> to the equation of source = origin, that might have caused "computed-
> origin-method" to exist in the first place.

I’m not sure what you mean by “others may pass unhindered”?  You mean
other VCS checkouts?

> What do you think?

I think the situation of IceCat and Linux-libre is unusual: 2 packages
out of 18K.  That probably explains why we have a hard time figuring out
how to generalize the issues that ‘computed-origin-method’ addresses.

What you propose (IIUC) sounds interesting: we’d provide a
<computed-tarball> data type, which would make the source URL manifest
(something that’s useful for <https://issues.guix.gnu.org/50515>, for
instance), but the lowering step would be entirely custom, similar to
what it already looks like:

  (define-record-type* <computed-tarball> computed-tarball make-computed-tarball
    computed-tarball?
    this-computed-tarball
    (url      computed-tarball-url)  ;or could be an <origin>
    (builder  computer-tarball-builder (thunked)) ;gexp
    (location computed-tarball-location (innate) (default (current-source-location))))

Is this what you had in mind?

Thanks,
Ludo’.

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

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: Mark H Weaver <mhw <at> netris.org>, 50620 <at> debbugs.gnu.org,
 zimoun <zimon.toutoune <at> gmail.com>
Subject: Re: bug#50620: [PATCH 0/2] Unify 'computed-origin-method' (linux,
 icecat)
Date: Thu, 30 Sep 2021 16:17:48 +0200

Hi,

Am Donnerstag, den 30.09.2021, 10:28 +0200 schrieb Ludovic Courtès:
> [...]
> > What we can't currently control is the top directory name and the
> > output name.  Both of that could be customized by supplying a
> > "repack-name" field, which is used as basis for the directory name
> > and the tarball name.
> > Another thing we can't easily control are extraneous inputs to the
> > patches, although the patch-inputs field *does* exist.
> 
> It’s possible to use a gexp as the snippet, where you can refer to
> additional things in there (though in practice this is currently
> impractical due to snippets not being thunks/promises.)
Which is a practical issue because it'd mean that the tarball gets
built as soon as the source is interpreted?

> > > > A combination of the above might make computed origins obsolete
> > > > for
> > > > good, but the question remains whether that is a better
> > > > design.  What do y'all think?
> > > 
> > > The design goal is to have clearly identified types: <package>,
> > > <origin>, <operating-system>.  For each of these, we want some
> > > flexibility: build system, origin method, etc.  However, beyond
> > > some
> > > level of stretching, it may be clearer to just use the catch-all
> > > ‘computed-file’ or to devise a new type.  After all, that’s how
> > > <origin> came to be (we could have used <package> instead with a
> > > suitable build system).
> > > 
> > > There’s a tension between “purely declarative” and “flexible”,
> > > and
> > > it’s about striking a balance, subjectively.
> > To be fair, I did think that "computed-tarball" might be a good
> > abstraction in some sense, but on another hand origins are computed
> > tarballs with a record interface.
> > 
> > On a somewhat related note, origins have this weird situation going
> > on where some things like git or svn checkouts need to be defined
> > through them, whereas others may pass unhindered.  I feel that this
> > contributes to the equation of source = origin, that might have
> > caused "computed-origin-method" to exist in the first place.
> 
> I’m not sure what you mean by “others may pass unhindered”?  You mean
> other VCS checkouts?
I mean that we don't need to wrap local-file inside an origin for
example whereas we do need to wrap e.g. svn-fetch instead of having an
svn-checkout constructor at the top.  It's not really that noticable
normally, but weird once you start thinking a little too hard about it.

> > What do you think?
> 
> I think the situation of IceCat and Linux-libre is unusual: 2
> packages out of 18K.  That probably explains why we have a hard time
> figuring out how to generalize the issues that ‘computed-origin-
> method’ addresses.
> 
> What you propose (IIUC) sounds interesting: we’d provide a
> <computed-tarball> data type, which would make the source URL
> manifest (something that’s useful for <
> https://issues.guix.gnu.org/50515>;,
> for instance), but the lowering step would be entirely custom,
> similar to what it already looks like:
> 
>   (define-record-type* <computed-tarball> computed-tarball make-
> computed-tarball
>     computed-tarball?
>     this-computed-tarball
>     (url      computed-tarball-url)  ;or could be an <origin>
>     (builder  computer-tarball-builder (thunked)) ;gexp
>     (location computed-tarball-location (innate) (default (current-
> source-location))))
> 
> Is this what you had in mind?
Slightly similar, but I don't think I'd want a singular source url. 
Instead

  (define-record-type* <computed-tarball> computed-tarball 
    make-computed-tarball
    computed-tarball?
    this-computed-tarball
    (sources  computed-tarball-sources)  ; list of origins, local 
                                         ; files or other things
    (builder  computer-tarball-builder (thunked)) ; gexp
    (name     computed-tarball-name) ; perhaps?
    (location computed-tarball-location (innate) 
              (default (current-source-location))))

At the start of BUILDER, SOURCES are already unpacked to the current
working directory under their stripped file names.  After builder
returns, we either package the contents of the current working
directory up into a tarball (variant A) or we have builder return a
list of files to pack up (variant B) which we then post-process maybe.
 
WDYT?

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Cc: Mark H Weaver <mhw <at> netris.org>, 50620 <at> debbugs.gnu.org,
 zimoun <zimon.toutoune <at> gmail.com>
Subject: Re: bug#50620: [PATCH 0/2] Unify 'computed-origin-method' (linux,
 icecat)
Date: Thu, 30 Sep 2021 22:09:12 +0200

Liliana Marie Prikler <liliana.prikler <at> gmail.com> skribis:

> Am Donnerstag, den 30.09.2021, 10:28 +0200 schrieb Ludovic Courtès:
>> [...]
>> > What we can't currently control is the top directory name and the
>> > output name.  Both of that could be customized by supplying a
>> > "repack-name" field, which is used as basis for the directory name
>> > and the tarball name.
>> > Another thing we can't easily control are extraneous inputs to the
>> > patches, although the patch-inputs field *does* exist.
>> 
>> It’s possible to use a gexp as the snippet, where you can refer to
>> additional things in there (though in practice this is currently
>> impractical due to snippets not being thunks/promises.)
> Which is a practical issue because it'd mean that the tarball gets
> built as soon as the source is interpreted?

It’s impractical because typical usage introduces top-level circular
references (e.g., if you write #$gzip).

> I mean that we don't need to wrap local-file inside an origin for
> example whereas we do need to wrap e.g. svn-fetch instead of having an
> svn-checkout constructor at the top.  It's not really that noticable
> normally, but weird once you start thinking a little too hard about it.

Hmm yeah, I must not be thinking hard enough.  :-)

> Slightly similar, but I don't think I'd want a singular source url. 
> Instead
>
>   (define-record-type* <computed-tarball> computed-tarball 
>     make-computed-tarball
>     computed-tarball?
>     this-computed-tarball
>     (sources  computed-tarball-sources)  ; list of origins, local 
>                                          ; files or other things
>     (builder  computer-tarball-builder (thunked)) ; gexp
>     (name     computed-tarball-name) ; perhaps?
>     (location computed-tarball-location (innate) 
>               (default (current-source-location))))
>
> At the start of BUILDER, SOURCES are already unpacked to the current
> working directory under their stripped file names.  After builder
> returns, we either package the contents of the current working
> directory up into a tarball (variant A) or we have builder return a
> list of files to pack up (variant B) which we then post-process maybe.
>  
> WDYT?

Overall LGTM!  IWBN to see if there are other potential users in the
tree (I can’t think of any), but for IceCat and Linux-libre, it could
already improve the situation.

Thanks,
Ludo’.

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

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: Mark H Weaver <mhw <at> netris.org>, 50620 <at> debbugs.gnu.org,
 zimoun <zimon.toutoune <at> gmail.com>
Subject: Re: bug#50620: [PATCH 0/2] Unify 'computed-origin-method' (linux,
 icecat)
Date: Thu, 30 Sep 2021 23:49:02 +0200

Hi,

Am Donnerstag, den 30.09.2021, 22:09 +0200 schrieb Ludovic Courtès:
> Liliana Marie Prikler <liliana.prikler <at> gmail.com> skribis:
> 
> > [...]
> > Which is a practical issue because it'd mean that the tarball gets
> > built as soon as the source is interpreted?
> 
> It’s impractical because typical usage introduces top-level circular
> references (e.g., if you write #$gzip).
Ah, right.

> > I mean that we don't need to wrap local-file inside an origin for
> > example whereas we do need to wrap e.g. svn-fetch instead of having
> > an svn-checkout constructor at the top.  It's not really that
> > noticable normally, but weird once you start thinking a little too
> > hard about it.
> 
> Hmm yeah, I must not be thinking hard enough.  :-)
Perhaps it's just me who finds it weird tho.  ¯\_(ツ)_/¯

> > Slightly similar, but I don't think I'd want a singular source
> > url. Instead
> > 
> >   (define-record-type* <computed-tarball> computed-tarball 
> >     make-computed-tarball
> >     computed-tarball?
> >     this-computed-tarball
> >     (sources  computed-tarball-sources)  ; list of origins, local 
> >                                          ; files or other things
> >     (builder  computer-tarball-builder (thunked)) ; gexp
> >     (name     computed-tarball-name) ; perhaps?
> >     (location computed-tarball-location (innate) 
> >               (default (current-source-location))))
> > 
> > At the start of BUILDER, SOURCES are already unpacked to the
> > current working directory under their stripped file names.  After
> > builder returns, we either package the contents of the current
> > working directory up into a tarball (variant A) or we have builder
> > return a list of files to pack up (variant B) which we then post-
> > process maybe.
> >  
> > WDYT?
> 
> Overall LGTM!  IWBN to see if there are other potential users in the
> tree (I can’t think of any), but for IceCat and Linux-libre, it could
> already improve the situation.
Concrete examples which currently use "unpack more after unpack":

- chez-scheme with nanopass and stex
- xen's mini-os
- lbzip2's gnulib (and probably gnulib in other locations)
- similarly libgd in gedit and gnome-recipes (same origin for both)

The builder for those would always be a simple (series of) directory
rename(s) as well :)

This list might not be complete, at least I haven't checked whether it
is.  Also, packages which have  (package-source some-other-package) as
input somewhere don't count here, as the missing sources can trivially
be found and inserted imo.

Regards,
Liliana

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: zimoun <zimon.toutoune <at> gmail.com>
Cc: 50620-done <at> debbugs.gnu.org
Subject: Re: bug#50620: [PATCH 0/2] Unify 'computed-origin-method' (linux,
 icecat)
Date: Fri, 01 Oct 2021 00:17:01 +0200

zimoun <zimon.toutoune <at> gmail.com> skribis:

> The 'computed-origin-method' had been introduced as a workaround limitations
> in the 'snippet' mechanism.  The procedure is duplicated which makes hard to
> automatically detects packages using it.
>
> * guix/packages.scm (computed-origin-method): Move procedure from...
> * gnu/packages/gnuzilla.scm: ...here and...
> * gnu/packages/gnuzilla.scm: ...there.

I tweaked the commit log and applied, thanks!

Hopefully we’ll grow a “proper” solution thanks to the discussions about
<computed-tarball> & co. that we’ve had.

Thanks everyone,
Ludo’.

Previous Next

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