[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#30809] [PATCH 2/2] services: Add Gitolite.
From: |
Clément Lassieur |
Subject: |
[bug#30809] [PATCH 2/2] services: Add Gitolite. |
Date: |
Tue, 24 Jul 2018 11:23:23 +0200 |
User-agent: |
mu4e 1.0; emacs 26.1 |
Hi Christopher, thank you for the update!
Christopher Baines <address@hidden> writes:
[...]
> +(define gitolite-setup
> + (match-lambda
> + (($ <gitolite-configuration> 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 use
> + ;; the hostname command, as the network might not be up yet
> + (with-output-to-file #$(string-append home "/.gitconfig")
> + (lambda ()
> + (display "[user]
> + name = GNU Guix
> + email = address@hidden
> +"))))
> + ;; 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 after
> + ;; 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 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 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://address@hidden:2222/gitolite-admin"
> + "/tmp/clone"))
> +
> + (with-directory-excursion "/tmp/clone"
> + (invoke #$(file-append git "/bin/git")
> + "-c" "user.name=Guix" "-c" "user.email=guix"
> + "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 (= (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ément
[bug#30809] [PATCH 1/2] gnu: Modify the gitolite package to support the Guix service., Christopher Baines, 2018/07/29
[bug#30809] [PATCH 1/2] gnu: Modify the gitolite package to support the Guix service., Christopher Baines, 2018/07/31