[bug#50359] import: Add 'generic-git' updater.

On Tue, Sep 07 2021, Xinglu Chen wrote:

> On Mon, Sep 06 2021, Sarah Morgensen wrote:
>
>> Hi,
>>
>> Xinglu Chen <public@yoctocell.xyz> writes:
>>
>>> Any trick you used to find all of there weird version numbers?  :-)
>>
>> This monstrosity:
>>
>>   rg -U -B4 --pcre2 '(?!\(let.*(\n.*){0,1})\(version "([^\n"]*[^0-9\.][^"\n]*)".*(\n.*){0,10}commit.*version' gnu/packages
>>
>> and to show just the versions:
>>
>>   rg -Uor '$2' --pcre2 --no-filename --no-line-number 
>
> Wow!  I will try that and see for myself!  :-)
>
>>>> IMO, just get rid of the delimiter.  If we wanted to be *that* flexible,
>>>> we could make it so they provide a tag->version proc instead of (prefix,
>>>> suffix, delimiter).
>>>
>>> a ‘tag->version’ procedure would probably make things a bit too
>>> complicated for the people writing package definitions.  For example,
>>> having a delimiter would make it easy to match a tag like
>>> “2021-01-01-release”
>>>
>>> Delimiter is “.” (sorry if this hurts your eyes ;-))
>>>
>>> scheme@(guile-user)> (match:substring (string-match "^[^0-9]*([^\\.[:punct:]]+(\\.[^\\.[:punct:]]+)*).*$" "2021-01-01-release") 1)
>>> $28 = "2021"
>>>
>>> Delimiter is “-”
>>>
>>> scheme@(guile-user)> (match:substring (string-match "^[^0-9]*([^-[:punct:]]+(-[^-[:punct:]]+)*).*$" "2021-01-01-release") 1)
>>> $29 = "2021-01-01-release"
>>>
>>> And then, setting the suffix to “-release” would match just the version
>>> part.
>>
>> Right.  I missed that.
>>
>> In that vein, should we keep the dashes in "2021-01-01" or convert them
>> to periods?
>
> Having periods would be more consistent, then could have a
> ‘date->version’ procedure that replaces the hyphens with dots and have
>
>   (git-reference
>     (url "https://git.example.org")
>     (commit (date->version version)))
>
>> What about when a tag has underscores?
>
> Hmm, not sure about that, below is a list of packages I could find which had
> underscores as delimiters
>
> gnu/packages/graphics.scm
> 239:          (commit "DIRECTFB_1_7_7")))
> gnu/packages/gstreamer.scm
> 326:          (commit "ESOUND_0_2_41")))
> gnu/packages/java.scm
> 13925:                      (commit "jboss-transaction-api_1.2_spec-1.1.1.Final")))
>
> They all seem to use periods in the ‘version’ field, though, so I would
> say that the underscroes, should also be converted to periods.
>
>> What if a repo has tags in both formats?  Then "3.0.1" would be
>> considered older than "2011-01-01".
>
> That’s tricky, there isn’t really a way to know how old “3.0.1” is,
> without looking at the metadata of the tag.  Maybe this is one of those
> corner cases which can’t really automatically determine the latest
> release.  Should we have a ‘no-refresh?’ property to tell the refresh to
> not try to update the package?
>
>> Maybe we should just add an extra bit to detect a date format and only
>> consider it when there's no "proper versions"?
>
> That could be a good idea!
>
>> Aaaand I fell down a rabbit hole after that :) I've attached a patch
>> with what I've done.  It still has lots of issues--it requires the tag
>> to contain at least one version delimiter, it requires the first
>> character of the version to be a number... it might not even be better
>> than before I touched it, and even so the added complexity might not be
>> worth it.  But if you'd like to take it for a spin, I've attached it (it
>> applies straight on master).
>
> Great! I will try it out and see how it compares to my current WIP
> version.
>
> Not having characters in the first version number probably isn’t such a
> big deal, most version that contain characters end with a character.
> E.g., “1.2.3a” is not to uncommon, but “a1.2.3” is rarely seen.

I made some changes, to the patch and ran ‘guix refresh -t generic-git |
grep "^gnu/packages.*no valid’; I also modified the ‘github’ updater so
that it wouldn’t be used.
  
  (define %github-updater
    (upstream-updater
     (name 'github)
     (description "Updater for GitHub packages")
     (pred (const #f)) ;this right here
     (latest latest-release)))

Attached is a file with all the packages that didn’t have any valid
tags, and with a short description of perharps why no valid tags were
found.

Something I noticed was the a lot of Julia package use a version scheme
like this:

  (version "1.2.3+0")

The "+0" is included in the version field and acts like a “revision”; I
am not familiar with the Julia ecosystem, and I am not sure how we
should handle this situation.

The updated patch is also attached.

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’.

On Fri, Sep 10 2021, Ludovic Courtès wrote:

> Hello,
>
> This looks very cool!

Thanks for taking a look!

It’s still a WIP, but I think it’s getting there.  :-)

> 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.

Noted.

>> +@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?

Good idea.

>> +(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.)

Yep.  :-)

> 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.  :-)

Yeah, it was suggested by Sarah, and in my testing it seems to work
pretty well.  :-)

>> +  (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<?)

Agreed, I will use your suggested version.

>> +(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”.

Yeah, as the latest tag might not correspond to a release...

> 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.

Noted.

>> +(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?/

‘accept-pre-releases?’ ;-)

> 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.

Good point.

>> +(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.

Thanks for the pointers!  I will look into it.

> Could you send an updated patch?

Sure!  Thanks for the review!  :-)