Hello Guix,

Thanks for the feedback!

Note: @civodul, assuming you are subscribed to the ml, I currently kept
you as a `To:` recipient but I can drop you from it, right? I'm "also"
subscribed to the ml so you may drop me from the `To:` too (if i'm not
mistaken).

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

> Hi Antoine!
> "Antoine R. Dumont (@ardumont)" <antoine.romain.dumont@gmail.com>
> skribis:
>
>> Here is the rough changelog:
>>
>> - The local db cache is now versioned. Migration will transparently
>>   happen for users at each index command calls (if need be).
>
> Perfect!
>
>> - The cli parsing got rewritten to be more flexible (inspired from
>>   existing code from guix, notably `guix home`).
>>
>> - We can now choose the indexation method using the
>>   `--with-method={store|manifests}` flag. The "manifests" method is the
>>   default, seel the help message for more details).
>
> Excellent.  (I think we can call it ‘--method’, without “with”.)

sure.

>> - Finally, the indexation methods are displayed using a progress bar.
>
> Yay, I love progress bars.  :-)

I have some work to improve the implementation to have more details in
the messages (typically make the "prefix" parameter of
`progress-report/bar` be a callable/function [1]...). If that's of some
interest, i'll push forward (in another patch maybe?).  I stopped at the
moment 'cause i had some strange issues with my env (where i could not
make guile see the changes for some reasons...). Anyway, that's of
lesser priority than the rest so...

[1] or whatever the name is in guile context ;)

>> Heads up, I did not yet address the "output" part. Thanks @zimoun for
>> the clarification btw ;)
>
> Future work.  ;-)

ok!

If i'm getting out of all the modifications i need to do, and if I have
some energy left, I might attend to it too ;)

>>> In the package case, the number of packages is known ahead.
>>
>> @civodul For the index 'store' implementation, ^ I did not find that
>> information.
>
> (length (all-packages)) gives you the total number of packages you’re
> going to traverse.  ‘all-packages’ is not instantaneous, but as a good
> approximation the time spent in ‘all-packages’ can be ignored.

ok. I missed that.

Although, the current call to `fold-packages` does some package
filtering first. So, I guess that's why you call `(length
(all-packages))` an approximation (no filtering on that call), right?

>> So, as a costly implementation detail, I'm folding over all packages
>> first to know the total number of packages (for the progress bar). And
>> then another round trip to actually do the insert.
>
> You could build up the package list just once and call ‘length’ on it.

I explained myself wrongly. That's what it is doing currenly. It does
that ^ folding and keep the packages list, then do a `length` call on it
to have the exact number of entries. And then does the actual loop on
that list to insert them in the db cache.

I naively thought that the `length` call on the list would cost one
round trip O(n), isn't it so? Or is there some memoization somewhere?

>> Hope you'll find it mostly to your taste!
>
> I do!

\o/

>> Note: I gather we'll rework the commits at some point (when it's ready)
>> so I did not bother too much right now.
>
> I think at this point we could consider integration in Guix proper,
> under ‘guix/scripts’.  For that we could dismiss commit history.

Fine with me. I'll do the adaptations to make it a script then.

> That’ll entail extra work (d’oh!) such as fine-tuning, writing tests,
> and writing a section for the manual.

Yes, i'm fine with that.

FWIW, I tried to have a look at how current unit tests were written last
week. I did not grok it entirely yet. I saw some script tests generate
some guile and I got lost there ;) I'll have to double check.

I'll probalby need some help for testing and documentation. I guess
asking questions on irc is fine for that part, right?

> The other option, if you prefer, would be to keep it in a separate repo
> as an extension that people can install.  To me that would be more of a
> temporary solution because I think it’s a useful feature that ought to
> be provided by Guix proper eventually.
> WDYT?  :-)

If it's temporary then i'm fine with trying to do the extra work to
merge the work with proper Guix ;).

Although, zimoun, down thread has some interesting remarks too.  I'll
let you discuss those.

I have another extension idea [1] that might help anyway. So we'll have
another opportunity to entertain the guix extensions features (if the
idea is interesting to proper Guix).

[1] `guix bug-report [--with-uname|--with-version|...]`

> Ludo’.

Cheers,
--
tony / Antoine R. Dumont (@ardumont)

-----------------------------------------------------------------
gpg fingerprint BF00 203D 741A C9D5 46A8 BE07 52E2 E984 0D10 C3B8