From debbugs-submit-bounces@debbugs.gnu.org Sun Jul 29 16:45:35 2018 Received: (at 30809) by debbugs.gnu.org; 29 Jul 2018 20:45:35 +0000 Received: from localhost ([127.0.0.1]:34122 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fjsZL-0000d7-1U for submit@debbugs.gnu.org; Sun, 29 Jul 2018 16:45:35 -0400 Received: from li622-129.members.linode.com ([212.71.249.129]:33938 helo=mira.cbaines.net) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fjsZJ-0000cz-Ky for 30809@debbugs.gnu.org; Sun, 29 Jul 2018 16:45:34 -0400 Received: by mira.cbaines.net (Postfix, from userid 113) id E83EF16645; Sun, 29 Jul 2018 21:45:32 +0100 (BST) X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on mira.cbaines.net X-Spam-Level: X-Spam-Status: No, score=-2.9 required=5.0 tests=ALL_TRUSTED,BAYES_00, URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from localhost (cpc102582-walt20-2-0-cust14.13-2.cable.virginm.net [86.27.34.15]) by mira.cbaines.net (Postfix) with ESMTPSA id 0596C16626; Sun, 29 Jul 2018 21:45:31 +0100 (BST) Received: from capella (localhost [127.0.0.1]) by localhost (OpenSMTPD) with ESMTP id e9d5064e; Sun, 29 Jul 2018 20:45:31 +0000 (UTC) References: <20180723214328.18740-1-mail@cbaines.net> <20180723214328.18740-2-mail@cbaines.net> <87o9ext2b8.fsf@lassieur.org> User-agent: mu4e 1.0; emacs 26.1 From: Christopher Baines To: =?utf-8?Q?Cl=C3=A9ment?= Lassieur Subject: Re: [bug#30809] [PATCH 2/2] services: Add Gitolite. 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" X-Spam-Score: -0.0 (/) X-Debbugs-Envelope-To: 30809 Cc: 30809@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) --=-=-= 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----- --=-=-=--