From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39600) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fhtY5-00029A-Qz for guix-patches@gnu.org; Tue, 24 Jul 2018 05:24:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fhtY2-0008MQ-KI for guix-patches@gnu.org; Tue, 24 Jul 2018 05:24:05 -0400 Received: from debbugs.gnu.org ([208.118.235.43]:49564) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1fhtY2-0008MD-En for guix-patches@gnu.org; Tue, 24 Jul 2018 05:24:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1fhtY2-0002FV-6W for guix-patches@gnu.org; Tue, 24 Jul 2018 05:24:02 -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> From: =?UTF-8?Q?Cl=C3=A9ment?= Lassieur In-reply-to: <20180723214328.18740-2-mail@cbaines.net> Date: Tue, 24 Jul 2018 11:23:23 +0200 Message-ID: <87o9ext2b8.fsf@lassieur.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable 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: Christopher Baines Cc: 30809@debbugs.gnu.org Hi Christopher, thank you for the update! 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' :-). > + (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. > + ;; 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 u= se > + ;; 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 for > + ;; initial setup, and will replace the previous key if run af= ter > + ;; 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. > + (begin I think that 'begin' is useless. > + ;; Exit with a non-zero status code if an exception i= s 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 been= 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. [...] > + (test-eq "cloning the admin repository" > + #t test-assert > + (invoke #$(file-append git "/bin/git") > + "clone" "-v" > + "ssh://git@localhost:2222/gitolite-admin" > + "/tmp/clone")) > + > + (with-directory-excursion "/tmp/clone" > + (invoke #$(file-append git "/bin/git") > + "-c" "user.name=3DGuix" "-c" "user.email=3Dguix" > + "commit" > + "-m" "Test commit" > + "--allow-empty") > + > + (test-eq "pushing, and the associated hooks" > + #t test-assert > + (invoke #$(file-append git "/bin/git") "push"))) Could you confirm that if a hook fails, that test will fail? > + (test-end) > + (exit (=3D (test-runner-fail-count (test-runner-current)) 0)))= )) > + > + (gexp->derivation "gitolite" test)) > + > +(define %test-gitolite > + (system-test > + (name "gitolite") > + (description "Clone the Gitolite admin repository.") > + (value (run-gitolite-test)))) Thanks! Cl=C3=A9ment