From mboxrd@z Thu Jan 1 00:00:00 1970 From: ludo@gnu.org (Ludovic =?utf-8?Q?Court=C3=A8s?=) Subject: Re: [PATCH] Prototype register-path Date: Mon, 05 Jun 2017 22:34:34 +0200 Message-ID: <87poei9ns5.fsf@gnu.org> References: <87efv18nkt.fsf@cune.org> <87a85m96dg.fsf@cune.org> Mime-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:58816) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dHyi4-0008ME-Io for guix-devel@gnu.org; Mon, 05 Jun 2017 16:34:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dHyi3-0006dR-H6 for guix-devel@gnu.org; Mon, 05 Jun 2017 16:34:44 -0400 In-Reply-To: <87a85m96dg.fsf@cune.org> (Caleb Ristvedt's message of "Mon, 05 Jun 2017 03:38:19 -0500") 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" To: Caleb Ristvedt Cc: guix-devel@gnu.org --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Hi reepca! I gave this patch set a try and looked at the code, and it looks very good to me! =E2=80=9Cmake check TESTS=3Dtests/store.scm=E2=80=9D passes now with the fi= xes you posted today (=E2=80=98getenv=E2=80=99 & co.) Like you say, what=E2=80=99s missing from =E2=80=98register-path=E2=80=99 n= ow is a timestamp reset phase (=E2=80=98reset-timestamps=E2=80=99 in (gnu build install) shou= ld cover that), and a deduplication phase (which you=E2=80=99ll have to implement). = That would be a good next step IMO. For deduplication, you already know the (guix hash) API that will allow you to do that I guess. The =E2=80=9Cdeduplication database=E2=80=9D is si= mply the /gnu/store/.links directory. Each entry there is a hard link to a file; the name of the entry is the hex representation of the sha256 hash of that file. So when inserting a file in the store, all you need is to look up its hash in /gnu/store/.links and hard-link from there. See what I mean? Some minor suggestions about the code: =E2=80=A2 Please add a copyright line for yourself in files you modify, a= nd add the usual GPLv3+ header in new files. :-) =E2=80=A2 When you add new =E2=80=98with-something=E2=80=99 macros, you c= an tell Emacs (if that=E2=80=99s what you use) to ident them like the other =E2=80=98with= -=E2=80=99 forms by modifying .dir-locals.el; there are several examples of that there. =E2=80=A2 I think =E2=80=98register-path=E2=80=99 doesn=E2=80=99t have to= explicitly do a bunch of =E2=80=98getenv=E2=80=99 calls. Instead it should use the variables de= fined in (guix config). For instance =E2=80=98%store-directory=E2=80=99 and =E2=80=98%state-directory=E2=80=99 are already defined using =E2=80=98g= etenv=E2=80=99. There=E2=80=99s currently nothing for NIX_DB_DIR but you could define =E2=80=98%store-database-directory=E2=80=99 similarly. =E2=80=A2 It would probably make sense to use the (guix store =E2=80=A6) = name space for modules that implement the store. So we could have (guix store database) for the subset of (guix sql) that deals with /var/guix/db/db.sqlite, (guix store deduplication), and so on. =E2=80=A2 You get bonus points if you can squash =E2=80=9Cfixup=E2=80=9D = commits with the commit you=E2=80=99re conceptually amending. :-) Having a single patch that implements something makes it easier for others to review. If you=E2=80=99re not familiar with =E2=80=98git rebase -i=E2=80=99 & co.,= that=E2=80=99s OK though, don=E2=80=99t bother. =E2=80=A2 Extra bonus points if you follow our commit log conventions as discussed at . :-) What about pushing your changes to a WIP branch on Savannah or elsewhere? (If you have an account on Savannah we can give you access.) Thank you, and thumbs up for the quality work so far! Ludo=E2=80=99. --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEPORkVYqE/cadtAz7CQsRmT2a67UFAlk1wFoACgkQCQsRmT2a 67X2JhAAnD7BfAeYatgV/9d01mZL2Xp94UTct15xUB+3HUneJOQTWvmfTM49JPQX eo32PZrVa8o+1ZemKob28P0Vfog9hE1ln/XZsAO2tGFFW6p9WXaPywRxQb/MFRWY bz5gQ389T/TdLz3+iUtp8HHYpaLtRnIMVi2CQAxGTPtqBmKSDbqzQY4UIwxRwoIY bjpNcDDYm3EYKE1Ad6ypq4THlgTjhhc4feS+IKifxdo2Bxcbp97HK3rKs0viphc9 ioLivGFKn3ZGm4bmFYYlVRmbvOxUNEPYOT49JuceJiVg5QEv75WkcneN+YANdrDg jTmZls33i+7v0e6iF7CwkcYghHYYlm2+00VZeCH9xbr8qEBa5BpRDTAti3C3xEVc 2AhsMFkA7Q23Y3iGpSeEFtLn9+EyslN2fKTIuNiEwsbHQJh4iH7bbM2nwfqhWpQe u9Fxh802AWSqVQQS9fenShYmhtLtGOiGvv2IdYX+RsTv4xtgE4bjUtzTD7Y7jUwH 3gODfy44BcAy9EXBETvr3cqg8yTF92waJ7LMY/4TnxzwKRIA+uCI8wonHJ3phszB YSRCJK/4s3+v/E+KavB+jk102bOHcEK4SvsRtRZhBXQqrPfzTK8V3wqYEjIhC6m+ 9kaeEMWVw+nzo4yAj3AEFaOSzeb0j8YahqC3zfyKcdWUHPbgvpI= =TWIc -----END PGP SIGNATURE----- --=-=-=--