[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#50359] [PATCH] import: Add 'generic-git' updater.
From: |
Ludovic Courtès |
Subject: |
[bug#50359] [PATCH] import: Add 'generic-git' updater. |
Date: |
Fri, 10 Sep 2021 10:36:12 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) |
Hello,
This looks very cool!
Xinglu Chen <public@yoctocell.xyz> skribis:
> From f924dbb835425f6b9a5796918125592870391405 Mon Sep 17 00:00:00 2001
> Message-Id:
> <f924dbb835425f6b9a5796918125592870391405.1631125652.git.public@yoctocell.xyz>
> From: Xinglu Chen <public@yoctocell.xyz>
> Date: Fri, 3 Sep 2021 17:50:56 +0200
> Subject: [PATCH] import: Add 'generic-git' updater.
>
> * guix/import/git.scm: New file.
> * doc/guix.texi (Invoking guix refresh): Document it.
> * Makefile.am (MODULES): Register it.
> * guix/git.scm (ls-remote-refs): New procedure.
>
> Co-authored-by: Sarah Morgensen <iskarian@mgsn.dev>
Overall LGTM; comments below.
> diff --git a/doc/guix.texi b/doc/guix.texi
> index 36a0c7f5ec..26afb1607a 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -11920,6 +11920,33 @@ the updater for @uref{https://launchpad.net,
> Launchpad} packages.
> @item generic-html
> a generic updater that crawls the HTML page where the source tarball of
> the package is hosted, when applicable.
> +@item generic-git
Please add a newline above.
> +@lisp
> +(package
> + (name "foo")
> + ;; ...
> + (properties
> + '((tag-prefix . "^release0-")
> + (tag-suffix . "[a-z]?$")
> + (tag-version-delimiter . ":"))))
> +@end lisp
Very nice. Perhaps s/tag-/release-tag-/ for clarity?
> +(define* (ls-remote-refs url #:key tags?)
> + "Return the list of references advertised at Git repository URL. If TAGS?
> +is true, limit to only refs/tags."
To remain consistent with existing naming conventions, I’d call it
‘remote-refs’.
> + (with-libgit2
> + (call-with-temporary-directory
> + (lambda (cache-directory)
> + (let* ((repository (repository-init cache-directory))
> + ;; Create an in-memory remote so we don't touch disk.
> + (remote (remote-create-anonymous repository url)))
Too bad we need to create an empty repo; hopefully it costs next to
nothing though.
> + (remote-connect remote)
> + (remote-disconnect remote)
> + (repository-close! repository)
> +
> + (filter include? (map remote-head-name (remote-ls remote))))))))
Use ‘filter-map’.
> +(define* (get-version-mapping tags #:key prefix suffix delim pre-releases?)
Please add a docstring and remove ‘get-’ from the name. :-)
> + (define (guess-delim)
> + (let ((total (length tags))
> + (dots (reduce + 0 (map (cut string-count <> #\.) tags)))
> + (dashes (reduce + 0 (map (cut string-count <> #\-) tags)))
> + (underscores (reduce + 0 (map (cut string-count <> #\_) tags))))
> + (display (format #t "total: ~d, dots: ~d, dashes ~d, underscores ~d~%"
> + total dots dashes underscores))
Leftover? (Also display + format.)
Please spell out ‘delimiter’ (info "(guix) Formatting Code").
> + (cond
> + ((>= dots (* total 0.35)) ".")
> + ((>= dashes (* total 0.8)) "-")
> + ((>= underscores (* total 0.8)) "_")
> + (else ""))))
That’s a fancy heuristic. :-)
> + (let ((mapping (fold alist-cons '() (map get-version tags) tags)))
> + (stable-sort! (filter car mapping) entry<?)))
It’s perhaps clearer written like this:
(stable-sort (filter-map (lambda (tag)
(let ((version (get-version tag)))
(and version (cons version tag))))
tags)
entry<?)
> +(define* (get-latest-tag url #:key prefix suffix delim pre-releases?)
> + "Return the latest tag available from the Git repository at URL."
Maybe “the tag corresponding to the latest version”.
s/get-latest-tag/latest-tag/
> + (define (pre-release? tag)
> + (any (lambda (rx) (regexp-exec (make-regexp rx regexp/icase) tag))
> + %pre-release-words))
Better call ‘make-regexp’ only once; so you could change
‘%pre-release-words’ to be a list of regexp objects instead of a list of
strings.
> +(define (latest-git-tag-version package tag-prefix tag-suffix
> + tag-version-delimiter refresh-pre-releases?)
> + "Given a PACKAGE, the TAG-PREFIX, TAG-SUFFIX, TAG-VERSION-DELIMITER, and
> +REFRESH-PRE-RELEASES? properties of PACKAGE, returns the latest version of
> +PACKAGE."
Maybe s/refresh-pre-releases?/accept-pre-preleases?/
Since this procedure takes a package, it probably doesn’t need the other
arguments: it can extract them from the package properties, rather than
doing it at the call site.
> +(define (latest-git-release package)
> + "Return the latest release of PACKAGE."
> + (let* ((name (package-name package))
> + (properties (package-properties package))
> + (tag-prefix (assq-ref properties 'tag-prefix))
> + (tag-suffix (assq-ref properties 'tag-suffix))
> + (tag-version-delimiter (assq-ref properties 'tag-version-delimiter))
> + (refresh-pre-releases? (assq-ref properties 'refresh-pre-releases?))
> + (old-version (package-version package))
> + (url (git-reference-url (origin-uri (package-source package))))
> + (new-version (latest-git-tag-version package
> + tag-prefix
> + tag-suffix
> + tag-version-delimiter
> + refresh-pre-releases?)))
> +
> + (if new-version
> + (upstream-source
> + (package name)
> + (version new-version)
> + (urls (list url)))
> + ;; No new release or no tags available.
> + #f)))
Simply: (and new-version (upstream-source …)).
It would have been nice to have tests. I think testing
‘latest-git-release’ should be feasible without too much hassle using
the (guix tests git) infrastructure, as is done in tests/git.scm, with a
package referring to a locally-created repo using a git-reference with a
file:// URL.
Could you send an updated patch?
Thanks,
Ludo’.
- [bug#50359] [PATCH] import: Add 'generic-git' updater., Xinglu Chen, 2021/09/03
- [bug#50359] [PATCH] import: Add 'generic-git' updater., Sarah Morgensen, 2021/09/04
- [bug#50359] [PATCH] import: Add 'generic-git' updater., Sarah Morgensen, 2021/09/04
- [bug#50359] [PATCH] import: Add 'generic-git' updater., Xinglu Chen, 2021/09/05
- [bug#50359] [PATCH] import: Add 'generic-git' updater., Sarah Morgensen, 2021/09/06
- [bug#50359] [PATCH] import: Add 'generic-git' updater., Xinglu Chen, 2021/09/06
- [bug#50359] [PATCH] import: Add 'generic-git' updater., Sarah Morgensen, 2021/09/06
- [bug#50359] [PATCH] import: Add 'generic-git' updater., Xinglu Chen, 2021/09/07
- [bug#50359] [PATCH] import: Add 'generic-git' updater., Xinglu Chen, 2021/09/08
- [bug#50359] [PATCH] import: Add 'generic-git' updater.,
Ludovic Courtès <=
- [bug#50359] [PATCH] import: Add 'generic-git' updater., Xinglu Chen, 2021/09/10
[bug#50359] [PATCH] import: Add 'generic-git' updater., Xinglu Chen, 2021/09/05
[bug#50359] [PATCH 0/3] Add 'generic-git' updater., Xinglu Chen, 2021/09/10