Re: Merging guix.el

[Ludovic Courtès]

Alex Kost <address@hidden> skribis:

> Ludovic Courtès (2014-08-29 00:09 +0400) wrote:

[...]

> OK, I made "emacs.am" and modified "configure.ac" and "Makefile.am" (I
> pushed 2 new commits to “emacs-ui” branch).  Is there anything else to
> be done in a build part?

No, that looks good to me.

[...]

>>> I think it's a good idea, but if that "name" is just a package name,
>>> I can't use this optimization: I need to define entries by
>>> "name+version".
>>
>> Yes, makes sense.  So that ‘name->entries’ field would map a package
>> name to a list of entries; in the most common case, there’ll be just one
>> entry anyway.  How does that sound?

[...]

> Also I'm still not sure about the name ‘manifest-name->entries’ (or
> ‘manifest-name->entry’), as this function returns a vhash, it does not
> transform a name into a list of entries.  It may be confusing, no?

That would be only for internal use anyway, but if you prefer
‘manifest-table’ would do as well.

[...]

>>>> Given that ‘set-packages!’ has only on call site, what about removing
>>>> it, and instead writing directly:
>>>>
>>>>   (define %packages
>>>>     (fold-packages ... vlist-null))
>>>>
>>>>   (define %package-count
>>>>     (length %packages))
>>>>
>>>>   (define %package-table
>>>>     (vlist-fold ...))
>>>>
>>>> It’s also best to prefix global variable names with ‘%’.
>
> I recalled why I used that ugly ‘set-packages!’: I just didn't want to
> count packages again (i.e. to use ‘length’) for hash-table, so I
> combined setting ‘packages’ variable with counting.

OK, but even when there’s 1M packages, ‘length’ is still going to be
almost instantaneous, so no worries IMO.  ;-)

>> I would make %package-table a vhash instead of a hash table:
>>
>>   (define %package-table
>>     (vlist-fold (lambda (elem result)
>>                   (match elem
>>                     ((name . package)
>>                      (vhash-cons (cons (package-name package)
>>                                        (package-version package))
>>                                  package
>>                                  (if (vhash-assq name result)
>>                                      ...)))))
>>                 vlist-null
>>                 %packages))
>
> ... I left hash table, because I still don't understand how to use vhash
> for that: a name+version key should give a list of all matching
> packages, but AFAIU your variant would just replace one package with
> another (with the same name+version).

The key is ‘vhash-fold*’ (info "(guile) VHashes").  It allows you to
traverse all the entries associated with a given key:

--8<---------------cut here---------------start------------->8---
scheme@(guile-user)> (vhash-cons '("guile" "2.0") 'foo
                                 (vhash-cons '("guile" "2.0") 'bar
                                             vlist-null))
$12 = #<vhash 39240a0 2 pairs>
scheme@(guile-user)> (vhash-fold* cons '() '("guile" "2.0") $12)
$13 = (bar foo)
--8<---------------cut here---------------end--------------->8---

I think that answers your question, right?

> As for ‘set-current-manifest-maybe!’, I'm afraid it can't be deleted.  I
> found that I put it in some places where it shouldn't appear and I fixed
> that, but it still stays in functions returning “package information”
> for the elisp side (for info/list buffers).  Currently I can't invent a
> way how to get rid of this function completely.

I think the profile’s file name could be kept on the elisp side, and
passed to the Scheme code, which wouldn’t need to keep it in a global
variable.  That would also allow guix.el to be used on profiles other
than the default one.

That said, we can look into it later if you prefer.  WDYT?

Thanks!

Ludo’.