From mboxrd@z Thu Jan 1 00:00:00 1970 From: ludo@gnu.org (Ludovic =?utf-8?Q?Court=C3=A8s?=) Subject: Re: Merging guix.el Date: Thu, 28 Aug 2014 22:09:41 +0200 Message-ID: <87wq9s4bqy.fsf@gnu.org> References: <874mx3z9h2.fsf@gnu.org> <87fvgim60p.fsf@gmail.com> <87mwaobxbo.fsf@gnu.org> <87tx4wlbis.fsf@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:46586) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XN60z-00065V-FU for guix-devel@gnu.org; Thu, 28 Aug 2014 16:09:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XN60u-0004PF-VI for guix-devel@gnu.org; Thu, 28 Aug 2014 16:09:49 -0400 Received: from hera.aquilenet.fr ([2a01:474::1]:49808) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XN60u-0004Oz-GF for guix-devel@gnu.org; Thu, 28 Aug 2014 16:09:44 -0400 In-Reply-To: <87tx4wlbis.fsf@gmail.com> (Alex Kost's message of "Thu, 28 Aug 2014 22:22:35 +0400") List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org To: Alex Kost Cc: Guix-devel Alex Kost skribis: > Ludovic Court=C3=A8s (2014-08-28 16:41 +0400) wrote: > >> Alex Kost skribis: >> >>> Ludovic Court=C3=A8s (2014-08-23 16:17 +0400) wrote: [...] > OK, I think it would be good to make "emacs-ui" branch temporary, so > that after I'll fix everything that needs to be fixed, it may be > recreated with a nice and clean commit(s) for merging =E2=80=9Cguix.el=E2= =80=9D. This > way I could push commits there without a fear that I mess it all up. > WDYT? Definitely. What=E2=80=99s been done before for such work-in-progress bran= ches is to rewrite them until they=E2=80=99re ready, and then merge them. So if= you agree to adjust that commit and then delete+push the branch, it=E2=80=99s perfect. (By convention, we normally call these branches wip-* so that people understand that they may be rebased.) Now, I think we=E2=80=99re almost there anyway, so hopefully you=E2=80=99ll= merge real soon. :-) >>> I imagine there may be... for example vi users, who wouldn't want to >>> install this feature, so I made some changes in "configure.ac" to add >>> =E2=80=9C--disable-emacs-ui=E2=80=9D option. >> >> It seems that the only things that cannot be done when Emacs is not >> available is the generation of the autoloads file, right? > > Yes. > >> Then, what about adding $(AUTOLOADS) to the distribution? It would just >> need to be appended to dist_lisp_DATA, and not added to CLEANFILES. > > OK, will be done. And what about "configure.ac"? I thought a new > configure option is good. Should I delete it? I think so, yes, because it wouldn=E2=80=99t make any difference if the autoloads are already generated. Non-Emacs users would end up installing a few files that they would not use, but that=E2=80=99s not big = deal IMO. >>> Also I use almost the same code in "guix-helper.scm.in" that is used in >>> "scripts/guix.in", so I think it will be good to have some little >>> additional module with =E2=80=98config-lookup=E2=80=99 function. WDYT? >> >> It cannot be in a module, because at this point the module location >> isn=E2=80=99t known yet. I don=E2=80=99t really know how to factorize i= t, so I propose >> to leave it for later, with a FIXME. Maybe Mark has an idea? >> >> +(define %current-manifest) >> +(define current-manifest-entries-table) >> +(define packages) >> +(define packages-table) >> >> I didn=E2=80=99t know this was possible, but we shouldn=E2=80=99t rely o= n it. > > Do you mean definitions without initial values? Yes. [...] >> Also, my understanding is that =E2=80=98current-manifest-entries-table= =E2=80=99 is here >> to speed up lookups in =E2=80=98manifest-entries-by-name+version=E2=80= =99, right? > > Yes, =E2=80=98current-manifest-entries-table=E2=80=99 and =E2=80=98packag= es-table=E2=80=99 are there to > speed up the process of finding =E2=80=9Cmanifest entries=E2=80=9D/=E2=80= =9Cpackages=E2=80=9D by > name+version (it is a very general need). Also > =E2=80=98current-manifest-entries-table=E2=80=99 is used in =E2=80=98fold= -manifest-entries=E2=80=99. OK. >> Then, I think this optimization should go into (guix profiles): >> objects would carry that vhash, and =E2=80=98manifest-install= ed?=E2=80=99 >> etc. would make use of it. The constructor would be changed along these >> lines: [...] > 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 =E2=80=98name->entries=E2=80=99 field would map = a package name to a list of entries; in the most common case, there=E2=80=99ll be jus= t one entry anyway. How does that sound? > Also I think it should be =E2=80=98name->entries=E2=80=99 as there can be= several > manifest entries with the same name (or name/version). Yes, sure. >> Given that =E2=80=98set-packages!=E2=80=99 has only on call site, what a= bout removing >> it, and instead writing directly: >> >> (define %packages >> (fold-packages ... vlist-null)) >> >> (define %package-count >> (length %packages)) >> >> (define %package-table >> (vlist-fold ...)) >> >> It=E2=80=99s also best to prefix global variable names with =E2=80=98%= =E2=80=99. > > Yes, it's definitely better, except I don't know how to fill a table > with =E2=80=98vlist-fold=E2=80=99 and I don't see a better variant than t= his: > > (define %package-table > (let ((table (make-hash-table %package-count))) > (vlist-for-each (lambda (elem) > (let* ((pkg (cdr elem)) > (key (name+version->key > (package-name pkg) > (package-version pkg))) > (ref (hash-ref table key))) > (hash-set! table key > (if ref (cons pkg ref) (list pkg))))) > packages) > table)) 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)) > P.S. OMG! Thank you very much for reviewing all that crap. Elisp > code is much better (I hope :-)). Np, I trust you on the elisp code. :-) Ludo=E2=80=99.