| Message ID | TYCP286MB18973CA5D0DD1447744E30FAA30D9@TYCP286MB1897.JPNP286.PROD.OUTLOOK.COM |
|---|---|
| State | New |
| Headers | show |
| Series | [bug#54293,v2] home: Add home-git-service-type. | expand |
| Context | Check | Description |
|---|---|---|
| cbaines/comparison | success | View comparision |
| cbaines/git branch | success | View Git branch |
| cbaines/applying patch | success | View Laminar job |
| cbaines/issue | success | View issue |
iyzsong@outlook.com schreef op za 12-03-2022 om 10:22 [+0800]: > +(define (serialize-git-option-value value) > + (cond > + ((string? value) (with-output-to-string (lambda () (write value)))) Does git follow the same escaping rules as Guile? > + ((integer? value) (number->string value)) > + ((boolean? value) (if value "true" "false")))) This (and more generally, the home-git-service-type) needs to be documented in the manual. Greetings, Maxime.
iyzsong@outlook.com schreef op za 12-03-2022 om 10:22 [+0800]: > + (user.name > + (git-option-value *unspecified*)) I think *unspecified* is considered an implementation detail. Proposal: let it be #false by default, and don't serialize the option whenever it is #false. Also, I don't think that user names can be #false, #true or integers. Perhaps a 'git-string-value' would be useful? Greetings, Maxime.
iyzsong@outlook.com schreef op za 12-03-2022 om 10:22 [+0800]: > + (user.email [...]) > + (user.signingKey [...]) > + (author.name [...]) > + (author.email [...]) > + (committer.name [...]) > + (committer.email [...]) Conventionally (Guile) Scheme symbols are lowercase and don't use dots. I suppose you could do 'user-signing-key'. However, there seems to be quite some duplication between 'user', 'author' and 'committer'. Perhaps you could abstract things a little by e.g. doing something like the git-user-info record I proposed? (home-git-configuration (user (git-user-info (name ...) (e-mail ...) (signing-key ...)) (author user) (committer user) [other options]) Greetings, Maxime.
iyzsong@outlook.com schreef op za 12-03-2022 om 10:22 [+0800]: > +(define (home-git-profile config) > + (let ((package (home-git-configuration-package config))) > + (if (home-git-configuration-enable-send-email? config) > + `(,package (,package "send-email")) > + `(,package)))) Why is git added to the profile? E.g., what if, say, I want to use "guix home" only for setting up environment variables, dot files, etc, but not for installing software, and I want to use "guix install" for installing software? Alternatively, what if most of the time I don't use Guix, so I have no need for "git" in the environment, and when I do need git, I'll just do "guix shell git" to temporarily add it? Greetings, Maxime.
iyzsong@outlook.com schreef op za 12-03-2022 om 10:22 [+0800]: > + "Whether to install git email tools from the package's @code{send-email} > +output.") Perhaps add a little text about benefits/downsides? E.g., "Enabling this allows sending patch e-mails from git with @code{git send-email}, at the cost of an increase in closure size." Greetings, Maxime.
iyzsong@outlook.com schreef op za 12-03-2022 om 10:22 [+0800]: > + "The human-readable name used in the author and committer identity when > +creating commit or tag objects, or when writing reflogs. If you need the > +author or committer to be different, the @code{author.name} or > +@code{committer.name} can be set.") It does not need to he human-readable AFAIK, space aliens could write a name in some space alphabet not understood by humans here (as long as it's in Unicode). This sentence could be simplified a lot: "The name of the committer and author to record in commits and tags. By default, git assumes the committer and author name to be the same. To override this, the field @code{author.name} or @code{committer.name} can be set." This technically ignores reflogs, but I expect most people using git to not have to know what reflogs are. Greetings, Maxime.
iyzsong@outlook.com schreef op za 12-03-2022 om 10:22 [+0800]:
> + "If @command{git-tag} or @command{git-commit}
I don't have a 'git-tag' or 'git-commit', but I do have 'git tag' and
'git commit'. What are the hyphen-minuses doing here?
Greetings,
Maxime.
iyzsong@outlook.com schreef op za 12-03-2022 om 10:22 [+0800]: > + "If @command{git-tag} or @command{git-commit} is not selecting the key you > +want it to automatically when creating a signed tag or commit, you can > +override the default selection with this variable. This option is passed > +unchanged to gpg’s @code{--local-user} parameter, so you may specify a key > +using any method that gpg supports.") I would do this in the third person (I think the manual recommends this somewhere but I wouldn't know where), and avoid mentioning the implementation details (‘this option is passed to’) in favour of the meaning (basically, ‘declarative’ documentation instead of ‘procedural’ documentation). Proposal: "The PGP key, if any, to sign commmits and tags with. This can be in any format that gpg's @code{--local-user} parameter support, e.g. a PGP fingerprint or an e-mail (*) address. If set to @code{#false}, this defaults to a private key corresponding to the name and e-mail of the committer." (*) I don't know if Guix uses "email" or "e-mail". Greetings, Maxime.
iyzsong@outlook.com schreef op za 12-03-2022 om 10:22 [+0800]: > + (merge-options (lambda (options) ;merge options by section > + (fold > + (lambda (e prev) > + (match e > + ((section variables ..1) > + (begin > + (let ((v (assv-ref prev section))) > + (assv-set! prev section > + (if v (append v variables) > + variables))))))) > + '() options)))) Seems rather imperative. Can this be avoided, e.g. using vhashes? Excerpt from the manual: > 18.5.1 Programming Paradigm > --------------------------- > > Scheme code in Guix is written in a purely functional style. One > exception is code that involves input/output, and procedures that > implement low-level concepts, such as the ‘memoize’ procedure. Greetings, Maxime.
iyzsong@outlook.com schreef op za 12-03-2022 om 10:22 [+0800]: > + "Specify the encryption to use, either @code{ssl} or @code{tls}. Any other > +value reverts to plain SMTP.") This seems easy to go wrong, e.g. what if I accidentally pass a value in uppercase, e.g. "SSL" or "TLS"? I recommend validating this value instead of silently not encrypting. Also, does this need to be a symbol, or a string? Looking at the documentation, I would expect a symbol, but looking at the implementation, it seems to have to be a string. Greetings, Maxime.
iyzsong@outlook.com schreef op za 12-03-2022 om 10:22 [+0800]: > + (commit.gpgSign > + (git-option-value *unspecified*) Would be nice to validate it's a boolean, and not, say, a string or number. > + "A boolean to specify whether all commits should be GPG signed.") I'm wondering, should 'gpg.program' be set instead of relying on $PATH? Also, this can be overriden with "-S", so perhaps "A boolean specifying whether to sign commits by default, with OpenPGP." Also, the exact implementation (GPG) of OpenPGP used seems an implementation detail, hence my suggestion GPG->OpenPGP. Greetings, Maxime.
iyzsong@outlook.com schreef op za 12-03-2022 om 10:22 [+0800]: > + "If set, specifies the outgoing SMTP server to > +use (e.g. @code{smtp.example.com} or a raw IP address). How does smtp.example.com specify the SMTP server? As a DNS address? Does that mean I can do (sendemail.smtpServer (make-socket-address AF_INET INADDR_LOOPBACK some-number ...))? 'make-socket-address' is Guile's procedure for making an address, 'AF_INET' indicates IP is used, INADDR_LOOPBACK some-number indicates which IP address ... Also, wouldn't a raw IP address be a 32-bit integer? (or 128-bit in case of IPv6)? And in case of IPv6, is this with or without surrounding [brackets]? Perhaps a reference to the relevant Internet RFC would be useful. Greetings, Maxime.
Maxime Devos <maximedevos@telenet.be> skribis: > iyzsong@outlook.com schreef op za 12-03-2022 om 10:22 [+0800]: >> + (user.name >> + (git-option-value *unspecified*)) > > I think *unspecified* is considered an implementation detail. Yes, but it may be useful to be able to distinguish between “not specified” and “false”. To do that you can either use ‘*unspecified*’ or some other unique value that you’d compare with ‘eq?’, say: (define ¬-specified (list 'not 'specified)) (define (specified? value) (not (eq? value ¬-specified))) But really, ‘*unspecified*’ is okay IMO. Ludo’.
Hi 宋文武, iyzsong@outlook.com skribis: > * gnu/home/services/git.scm: New file. > * gnu/local.mk (GNU_SYSTEM_MODULES): Add it. It’s been a while already but I think it’d be a nice addition. I think Maxime raised good points while reviewing, in particular regarding naming conventions (lowercase and without dots) and possible grouping (all the ‘user.’ options might fit in a single <git-user> record or similar?). How do you feel about making hopefully final changes so you can push it? Thanks in advance! :-) Ludo’.
diff --git a/gnu/home/services/git.scm b/gnu/home/services/git.scm new file mode 100644 index 0000000000..f39c931c38 --- /dev/null +++ b/gnu/home/services/git.scm @@ -0,0 +1,214 @@ +;;; GNU Guix --- Functional package management for GNU +;;; Copyright © 2022 宋文武 <iyzsong@member.fsf.org> +;;; +;;; This file is part of GNU Guix. +;;; +;;; GNU Guix is free software; you can redistribute it and/or modify it +;;; under the terms of the GNU General Public License as published by +;;; the Free Software Foundation; either version 3 of the License, or (at +;;; your option) any later version. +;;; +;;; GNU Guix is distributed in the hope that it will be useful, but +;;; WITHOUT ANY WARRANTY; without even the implied warranty of +;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +;;; GNU General Public License for more details. +;;; +;;; You should have received a copy of the GNU General Public License +;;; along with GNU Guix. If not, see <http://www.gnu.org/licenses/>. + +(define-module (gnu home services git) + #:use-module (gnu home services) + #:use-module (gnu services configuration) + #:use-module (gnu packages version-control) + #:use-module (guix packages) + #:use-module (guix gexp) + #:use-module (srfi srfi-1) + #:use-module (ice-9 match) + #:export (home-git-service-type + home-git-configuration)) + +(define (git-option-value? value) + (or (unspecified? value) + (string? value) + (integer? value) + (boolean? value))) + +(define (serialize-git-option-value value) + (cond + ((string? value) (with-output-to-string (lambda () (write value)))) + ((integer? value) (number->string value)) + ((boolean? value) (if value "true" "false")))) + +(define (git-options? options) + "Return #t if OPTIONS is a well-formed sexp for git options." + (define git-variable? + (match-lambda + ((key value) (and (symbol? key) (git-option-value? value))) + (_ #f))) + (every + (match-lambda + (((section subsection) variables ..1) + (and (symbol? section) + (string? subsection) + (every git-variable? variables))) + ((section variables ..1) + (and (symbol? section) + (every git-variable? variables))) + (_ #f)) + options)) + +(define (serialize-git-options options) + "Return the @command{git-config} text form for OPTIONS." + (define serialize-section + (match-lambda + ((section variables ..1) + (with-output-to-string + (lambda () + (match section + ((section subsection) + (simple-format #t "[~a ~s]~%" section subsection)) + (_ + (simple-format #t "[~a]~%" section))) + (for-each + (match-lambda + ((key value) + (simple-format #t "\t~a = ~a~%" + key (serialize-git-option-value value)))) + variables)))))) + (string-concatenate (map serialize-section options))) + +(define-configuration/no-serialization home-git-configuration + (package + (package git) + "The Git package to use.") + (enable-send-email? + (boolean #t) + "Whether to install git email tools from the package's @code{send-email} +output.") + (user.name + (git-option-value *unspecified*) + "The human-readable name used in the author and committer identity when +creating commit or tag objects, or when writing reflogs. If you need the +author or committer to be different, the @code{author.name} or +@code{committer.name} can be set.") + (user.email + (git-option-value *unspecified*) + "The email address used in the author and committer identity when creating +commit or tag objects, or when writing reflogs. If you need the author or +committer to be different, the @code{author.email} or @code{committer.email} +can be set.") + (user.signingKey + (git-option-value *unspecified*) + "If @command{git-tag} or @command{git-commit} is not selecting the key you +want it to automatically when creating a signed tag or commit, you can +override the default selection with this variable. This option is passed +unchanged to gpg’s @code{--local-user} parameter, so you may specify a key +using any method that gpg supports.") + (author.name + (git-option-value *unspecified*) + "The human-readable name used in the author identity when creating commit +or tag objects, or when writing reflogs.") + (author.email + (git-option-value *unspecified*) + "The email address used in the author identity when creating commit or tag +objects, or when writing reflogs.") + (committer.name + (git-option-value *unspecified*) + "The human-readable name used in the committer identity when creating +commit or tag objects, or when writing reflogs.") + (committer.email + (git-option-value *unspecified*) + "The email address used in the author identity when creating commit or tag +objects, or when writing reflogs.") + (commit.gpgSign + (git-option-value *unspecified*) + "A boolean to specify whether all commits should be GPG signed.") + (sendemail.smtpServer + (git-option-value *unspecified*) + "If set, specifies the outgoing SMTP server to +use (e.g. @code{smtp.example.com} or a raw IP address). If unspecified, and if +@var{sendemail.sendmailcmd} is also unspecified, the default is to search for +@command{sendmail} in $PATH if such a program is available, falling back to +@code{localhost} otherwise.") + (sendemail.smtpServerPort + (git-option-value *unspecified*) + "Specifies a port different from the default port (SMTP servers typically +listen to smtp port 25, but may also listen to submission port 587, or the +common SSL smtp port 465); symbolic port names (e.g. @code{submission} instead +of 587) are also accepted.") + (sendemail.smtpUser + (git-option-value *unspecified*) + "Username for SMTP-AUTH. If a username is not specified, then +authentication is not attempted.") + (sendemail.smtpPass + (git-option-value *unspecified*) + "Password for SMTP-AUTH. If not specified, then a password is obtained +using @command{git-credential}.") + (sendemail.smtpEncryption + (git-option-value *unspecified*) + "Specify the encryption to use, either @code{ssl} or @code{tls}. Any other +value reverts to plain SMTP.") + (sendemail.sendmailcmd + (git-option-value *unspecified*) + "Specify a command to run to send the email. The command should be +sendmail-like; specifically, it must support the @code{-i} option. The +command will be executed in the shell if necessary.") + (extra-options + (git-options '()) + "Extra configuration options for Git.")) + +(define (home-git-configuration-final-options config) + (let* ((fields + (filter + (lambda (field) + (eq? (configuration-field-type field) 'git-option-value)) + home-git-configuration-fields)) + (options + (filter + (match-lambda + ((section (key value)) (not (unspecified? value)))) + (map (lambda (field) + (let* ((name (configuration-field-name field)) + (section+key (map string->symbol + (string-split (symbol->string name) #\.))) + (value ((configuration-field-getter field) config))) + `(,(car section+key) (,(cadr section+key) ,value)))) + fields))) + (extra-options (home-git-configuration-extra-options config)) + (merge-options (lambda (options) ;merge options by section + (fold + (lambda (e prev) + (match e + ((section variables ..1) + (begin + (let ((v (assv-ref prev section))) + (assv-set! prev section + (if v (append v variables) + variables))))))) + '() options)))) + (merge-options (append options extra-options)))) + +(define (home-git-environment-variables config) + (let ((gitconfig (serialize-git-options + (home-git-configuration-final-options config)))) + `(("GIT_CONFIG_SYSTEM" . ,(plain-file "gitconfig" gitconfig))))) + +(define (home-git-profile config) + (let ((package (home-git-configuration-package config))) + (if (home-git-configuration-enable-send-email? config) + `(,package (,package "send-email")) + `(,package)))) + +(define home-git-service-type + (service-type (name 'home-git) + (extensions + (list (service-extension + home-environment-variables-service-type + home-git-environment-variables) + (service-extension + home-profile-service-type + home-git-profile))) + (default-value (home-git-configuration)) + (description + "Install and configure the Git distributed revision control +system."))) diff --git a/gnu/local.mk b/gnu/local.mk index 9bfeede60f..a5ea94b3a1 100644 --- a/gnu/local.mk +++ b/gnu/local.mk @@ -80,6 +80,7 @@ GNU_SYSTEM_MODULES = \ %D%/home.scm \ %D%/home/services.scm \ %D%/home/services/desktop.scm \ + %D%/home/services/git.scm \ %D%/home/services/symlink-manager.scm \ %D%/home/services/fontutils.scm \ %D%/home/services/shells.scm \