all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Mark H Weaver <mhw@netris.org>
To: zimoun <zimon.toutoune@gmail.com>
Cc: 50515@debbugs.gnu.org
Subject: [bug#50515] [PATCH 2/2] website: Add 'computed-origin-method' packages to 'sources.json'.
Date: Sat, 11 Sep 2021 20:54:50 -0400	[thread overview]
Message-ID: <87a6kid34a.fsf@netris.org> (raw)
In-Reply-To: <20210911002608.14074-2-zimon.toutoune@gmail.com>

Hi Simon,

> With Guix 9875f9bca3976bf3576eab9be42164fde454597e, the packages considered
> are IceCat and the Linux kernel; see: gnu/packages/gnuzilla.scm and
> gnu/packages/linux.scm.
>
> * website/apps/packages/builder.scm (gexp-references): Unexported variable
> from the module '(guix gexp)'.
> (origin->json): Add 'computed-origin-method' case.

Thanks for working on this.

> diff --git a/website/apps/packages/builder.scm b/website/apps/packages/builder.scm
> index fb53215..ecf958a 100644
> --- a/website/apps/packages/builder.scm
> +++ b/website/apps/packages/builder.scm
[...]
> @@ -106,53 +109,67 @@
>           (append-map (cut maybe-expand-mirrors <> %mirrors)
>                       (map string->uri urls))))
> 
> -  `((type . ,(cond ((or (eq? url-fetch method)
> -                        (eq? url-fetch/tarbomb method)
> -                        (eq? url-fetch/zipbomb method)) 'url)
> -                   ((eq? git-fetch method) 'git)
> -                   ((or (eq? svn-fetch method)
> -                        (eq? svn-multi-fetch method)) 'svn)
> -                   ((eq? hg-fetch method) 'hg)
> -                   (else                   #nil)))
> -    ,@(cond ((or (eq? url-fetch method)
> +  (match uri
> +    ((? promise? promise)               ;computed-origin-method
> +     (match (force promise)

Here, you're implicitly assuming that 'computed-origin-method' is the
only origin method that puts a promise in the 'uri' field.  That may be
true today, but it will not necessarily be true tomorrow, and therefore
it seems suboptimal to make that assumption in the code.

Instead, I would suggest checking for "computed origins" in the same way
that is done for the other cases: using 'eq?'.  It's not ideal, but it's
more future-proof than checking for a promise in the 'url' field, and
anyway it's the way things are currently being done.

However, there's a difficulty, and I suspect you're already aware of it
and that it's why you used the suboptimal approach above:

At present, 'computed-origin-method' is not exported by any Guix module,
nor is there even a unique definition of it.  Instead, there are two
copies of it, one in gnuzilla.scm and one in linux.scm.

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.

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'.

I'm sorry that I never found the energy to clean this up properly.

Anyway, regarding your proposed patch: for now, I would suggest the
following options:

(1) In a separate preceding commit, move 'computed-origin-method' to its
    own module, export it, use the exported one in gnuzilla.scm and
    linux.scm, and use 'eq?' to test for it in the code above.  There
    should probably also be a comment next to the definition of
    'computed-origin-method' pointing out that it's a temporary hack,
    hopefully to be superceded by snippets when they have gained the
    required functionality.

(2) Alternatively, for now, use 'eq?' against the two private copies
    (accessed using @@, see below), along with a "FIXME" comment.

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

What do you think?

I'm not on the guix-patches list, so please CC me on replies that you'd
like me to see.

       Thanks,
         Mark

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




  parent reply	other threads:[~2021-09-12  0:57 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-11  0:14 [bug#50515] (guix-artwork)[PATCH 0/2] List linux origins in 'sources.json' zimoun
2021-09-11  0:26 ` [bug#50515] [PATCH 1/2] website: Tweak 'GUIX_WEB_SITE_LOCAL' zimoun
2021-09-11  0:26   ` [bug#50515] [PATCH 2/2] website: Add 'computed-origin-method' packages to 'sources.json' zimoun
2021-10-01 14:16     ` zimoun
2021-10-04  7:53       ` Ludovic Courtès
2021-10-05 14:09         ` zimoun
2021-09-12  0:54   ` Mark H Weaver [this message]
2021-09-13  7:01     ` zimoun
2021-09-16  0:07       ` Mark H Weaver
2021-09-16 11:48         ` zimoun
2021-10-05 14:09 ` [bug#50515] [PATCH v2 1/2] website: Tweak 'GUIX_WEB_SITE_LOCAL' zimoun
2021-10-05 14:09   ` [bug#50515] [PATCH v2 2/2] website: Add 'computed-origin-method' packages to 'sources.json' zimoun
2021-10-18 12:23     ` [bug#50515] (guix-artwork)[PATCH 0/2] List linux origins in 'sources.json' Ludovic Courtès
2021-10-21  9:42       ` zimoun
2021-10-21  9:41 ` [bug#50515] [PATCH v3 1/2] website: Tweak 'GUIX_WEB_SITE_LOCAL' zimoun
2021-10-21  9:41   ` [bug#50515] [PATCH v3 2/2] website: Add 'computed-origin-method' packages to 'sources.json' zimoun
2021-10-21 20:58   ` bug#50515: (guix-artwork)[PATCH 0/2] List linux origins in 'sources.json' Ludovic Courtès

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87a6kid34a.fsf@netris.org \
    --to=mhw@netris.org \
    --cc=50515@debbugs.gnu.org \
    --cc=zimon.toutoune@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/guix.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.