guix-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[bug#50620] [PATCH 1/2] guix: packages: Document 'computed-origin-method


From: Liliana Marie Prikler
Subject: [bug#50620] [PATCH 1/2] guix: packages: Document 'computed-origin-method'.
Date: Wed, 29 Sep 2021 16:36:38 +0200
User-agent: Evolution 3.34.2

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@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






reply via email to

[Prev in Thread] Current Thread [Next in Thread]