From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57693) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fjsZp-0001ZL-7B for guix-patches@gnu.org; Sun, 29 Jul 2018 16:46:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fjsZm-0003TP-1x for guix-patches@gnu.org; Sun, 29 Jul 2018 16:46:05 -0400 Received: from debbugs.gnu.org ([208.118.235.43]:57337) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1fjsZl-0003TL-Ti for guix-patches@gnu.org; Sun, 29 Jul 2018 16:46:01 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1fjsZl-0000dp-Nh for guix-patches@gnu.org; Sun, 29 Jul 2018 16:46:01 -0400 Subject: [bug#30809] [PATCH 2/2] services: Add Gitolite. Resent-Message-ID: References: <20180723214328.18740-1-mail@cbaines.net> <20180723214328.18740-2-mail@cbaines.net> <87o9ext2b8.fsf@lassieur.org> From: Christopher Baines In-reply-to: <87o9ext2b8.fsf@lassieur.org> Date: Sun, 29 Jul 2018 21:45:29 +0100 Message-ID: <87zhy9kbyu.fsf@cbaines.net> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+kyle=kyleam.com@gnu.org Sender: "Guix-patches" To: =?UTF-8?Q?Cl=C3=A9ment?= Lassieur Cc: 30809@debbugs.gnu.org --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Cl=C3=A9ment Lassieur writes: > Hi Christopher, thank you for the update! Thanks for taking another look. I've just send another set of revised patches. > Christopher Baines writes: > > [...] > >> +(define gitolite-setup >> + (match-lambda >> + (($ package user group home >> + rc-file admin-pubkey) >> + #~(let ((user-info (getpwnam #$user))) >> + (use-modules (guix build utils)) >> + >> + (simple-format #t "guix: gitolite: installing ~A\n" #$rc-file) >> + (copy-file #$rc-file #$(string-append home "/.gitolite.rc")) >> + >> + (let ((admin-pubkey #$admin-pubkey) > > What's the point of that 'let'? Afterwards you reuse '$admin-pubkey' > :-). Ah yeah, I've fixed that now. >> + (pubkey-file #$(string-append home "/id_rsa.pub"))) >> + (when admin-pubkey > > If we are 'gitolite-setup', that means 'admin-pubkey' is true, I think, > so that 'when' is useless. Indeed. I've removed the gitolite-setup function now, along with this conditional. >> + ;; The key must be writable, so copy it from the store >> + (copy-file #$admin-pubkey pubkey-file) >> + >> + (chmod pubkey-file #o500) >> + (chown pubkey-file >> + (passwd:uid user-info) >> + (passwd:gid user-info)) >> + >> + ;; Set the git configuration, to avoid gitolite trying to = use >> + ;; the hostname command, as the network might not be up yet >> + (with-output-to-file #$(string-append home "/.gitconfig") >> + (lambda () >> + (display "[user] >> + name =3D GNU Guix >> + email =3D guix@localhost >> +")))) >> + ;; Run Gitolite setup, as this updates the hooks and include= the >> + ;; admin pubkey if specified. The admin pubkey is required f= or >> + ;; initial setup, and will replace the previous key if run a= fter >> + ;; initial setup >> + (let ((pid (primitive-fork))) >> + (if (eq? pid 0) > > I have a slight preference for the previous 'match' expression you used > before, because it's used elsewhere this way and it requires less code. While I agree with both your points, I tried for quite a while last weekend to get match to work, and couldn't. I couldn't even tell why it suddenly wasn't. Unfortunately, Linux panicing when anything fails makes debugging the system test a bit tricky. >> + (begin > > I think that 'begin' is useless. Yeah, I think I added that while trying to get match to work. I've removed it now. >> + ;; Exit with a non-zero status code if an exception = is thrown. >> + (dynamic-wind >> + (const #t) >> + (lambda () >> + (setenv "HOME" (passwd:dir user-info)) >> + (setenv "USER" #$user) >> + (setgid (passwd:gid user-info)) >> + (setuid (passwd:uid user-info)) >> + (primitive-exit >> + (apply system* >> + #$(file-append package "/bin/gitolite") >> + "setup" >> + (if admin-pubkey >> + `("-pk" ,pubkey-file) >> + '())))) >> + (lambda () >> + (primitive-exit 1)))) >> + (waitpid pid))) >> + >> + (when (file-exists? pubkey-file) >> + (delete-file pubkey-file))))))) >> + >> +(define (gitolite-activation config) >> + (if (gitolite-configuration-admin-pubkey config) >> + (gitolite-setup config) >> + #~(display >> + "guix: Skipping gitolite setup as the admin-pubkey has not bee= n provided\n"))) > > I'm not fan of the idea that a user might: > 1. setup an initial configuration with 'admin-pubkey', > 2. change that configuration once the initial activation has been > done. > > What is the drawback to forcing the user to setup an 'admin-pubkey'? > Maybe you think that doing the activation is annoying and it should only > be done when necessary? If that's the case, maybe what we need is an > ad-hoc command instead of the activation, a bit like the > 'certbot-command' of the Certbot service. I wrote it this way as this is how I've been using Gitolite so far. On Debian, I think debconf prompts you for the key when you install the package, and runs gitolite setup. I've actually read the gitolite setup script now, and its behaviour it pretty reasonable if it's run frequently. As I understand it, it ensures that the provided admin-pubkey exists in the keydir directory in the gitolite-admin repository, and will commit to the repository if it changes anything. So, I think I've now changed both the service and the documentation to describe adding the admin-pubkey always. >> + (test-eq "cloning the admin repository" >> + #t > > test-assert > >> + (test-eq "pushing, and the associated hooks" >> + #t > > test-assert I've changed these now :) >> + (invoke #$(file-append git "/bin/git") "push"))) > > Could you confirm that if a hook fails, that test will fail? Yep, I added this check when I realised that the hooks were broken. --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQKTBAEBCgB9FiEEPonu50WOcg2XVOCyXiijOwuE9XcFAlteJ2lfFIAAAAAALgAo aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldDNF ODlFRUU3NDU4RTcyMEQ5NzU0RTBCMjVFMjhBMzNCMEI4NEY1NzcACgkQXiijOwuE 9XeW3xAAntZSanenOHsLxeMnfXl+IHhS5mzNNDbm3WG0G5J+u8xq2KLJFUKASD1t I4FYoTCtzAdFZVHKe/dmZCzp3ydFrC38UtHGbx4L4tfEiuX8HHQIUqz/QI3P+/lU XvsZuFsHWYhapVj278KcJN4O3YLHNt+NstKpTm8h5/upkRM9hB69H3Ku65Z16xLs VG1rmBn5JAf+nw2JLJXgBNybzwGkIoT5FNlXth4io4rPwItAhTGfK+pfqfI16eUE ZcI0O99/UH5pQI2f92HkJe/NK9l1RdNYEA+QKViGUHC0hr6lruQon6QLCpCLrVnE RiBMXC4x94uB6AoPfNfzaSfdzEdffX+crxIPtr67Ehv9989H7PZo+xeWlM6IC+cr YIIGCBykyAz0AkGBn0EVG9qy0jzOI7VEgJBpMznc4xzK+qPPUgnJK2mp1R5gzLeE UBp38uyuiHATTmOgrr+S8z//iHugW4atlbsREpKKnkzaHXYIUpVXhJ4d5n5JPoDw 0iwHSun5jIstAndHSF2CnGMfRNiluGlZpfUrBUwnvIuV2G3foQABCXQ1GpmuWK/C CF958dzzD+rWlX9+6dpp2BMZafnDaU+2LEWUjQIiK8oiiST7/Y+DDCpgaBeer9dS 3D37wIC9+LEnxy3/WcayxwDTO34Tb8RwhhwIKRBFb4prgVb7fzo= =YoxP -----END PGP SIGNATURE----- --=-=-=--