Re: [PATCH] draft addition of github updater

[Ben Woodcroft]

Hi again,

Thanks for the comments Ludo.

Unfortunately I found a further bug - the updated URL for the new 
package was actually the old URL not the updated one, and fixing this 
required some refactoring.

I'm afraid I'm almost out of time for this until the end of March, so if 
there are any further substantive changes we might have to let this slip 
the upcoming release, unless someone else can continue this work. Soz..

One way in which this could be improved in the future would be to accept 
odd source GitHub URLs and return the newest version, but error out when 
the URL needs to be guessed. That way, at least all GitHub-sourced 
packages can be checked for updates even if they cannot all be updated 
in place. I don't think this would be especially hard to implement and 
would be quite reliable.

On 04/01/16 06:46, Ludovic Courtès wrote:
> Ben Woodcroft<address@hidden>  skribis:
>
> > It seems I miscounted before, but now it is 129 of 146 github
> > "release" packages recognised with 28 suggesting an update - see the
> > end of email for details. There is one false positive:
> >
> > gnu/packages/ocaml.scm:202:13: camlp4 would be upgraded from 4.02+6 to
> > 4.02.0+1
> >
> > This happens because the newer versions were not made as official
> > releases just tags, so the newer versions are omitted from the API
> > response, plus there's the odd version numbering scheme. Guix is up to
> > date.
> I guess we could filter out such downgrades by adding a call to
> ‘version>?’, no?

My impression is that code elsewhere (yours?) already does this, but 
version>? does not work as intended for this corner case.

[...]

> > > I guess (guix import github) could contain something like:
> > >
> > >     (define %github-token
> > >       ;; Token to be passed to Github.com to avoid the 60-request per hour
> > >       ;; limit, or #f.
> > >       (make-parameter (getenv "GUIX_GITHUB_TOKEN")))
> > >
> > > and we’d need to document that, or maybe write a message hinting at it
> > > when we know the limit has been reached.
> > >
> > > WDYT?
> > Seems we were all thinking the same thing - I've integrated
> > this. Should we check that the token matches ^[0-9a-f]+$ for security
> > and UI?
> I think it’s fine as is.  There’s no security issue on the client side
> AFAICS.

OK

> > > I was thinking we could have a generic Git updater that would look
> > > for available tags upstream. I wonder how efficient that would be
> > > compared to using the GitHub-specific API, and if there would be
> > > other differences. What are your thoughts on this?
> > This sounds like an excellent idea, but I was unable to find any way
> > to fetch tags without a clone first. A clone could take a long time
> > and a lot of bandwidth I would imagine. Also there's no way to discern
> > regular releases from pre-releases I don't think. It is a bit unclear
> > to me how conservative these updaters should be, are tags sufficiently
> > synonymous with releases so as to be reported by refresh?
> I think we’d have to hard-code heuristics to distinguish release tags
> from other tags.  Typically, again, considering only tags that match
> ‘v[0-9\.]+’.
>
> Well, future work!  :-)

OK.

> >  From a42eda6b9631cc28dfdd02d2c8bb02eabb2626b9 Mon Sep 17 00:00:00 2001
> > From: Ben Woodcroft<address@hidden>
> > Date: Sun, 15 Nov 2015 10:18:05 +1000
> > Subject: [PATCH] import: Add github-updater.
> >
> > * guix/import/github.scm: New file.
> > * guix/scripts/refresh.scm (%updaters): Add %GITHUB-UPDATER.
> > * doc/guix.texi (Invoking guix refresh): Mention it.
> [...]
>
> > +The @code{github} updater uses the
> > address@hidden://developer.github.com/v3/, GitHub API} to query for new
> > +releases. When used repeatedly e.g. when refreshing all packages, GitHub
> > +will eventually refuse to answer any further API requests. By default 60
> > +API requests per hour are allowed, and a full refresh on all GitHub
> > +packages in Guix requires more than this. Authentication with GitHub
> > +through the use of an API token alleviates these limits. To use an API
> > +token, set the environment variable @code{GUIX_GITHUB_TOKEN} to a token
> > +procured from @uref{https://github.com/settings/tokens} or otherwise.
> Good!  Please make sure to leave two spaces after end-of-sentence periods.
>
> Also, maybe this paragraph should be moved after the @table that lists
> updaters?  Otherwise it mentions the ‘github’ updater before it has been
> introduced.

OK. I moved it to the end of the refresh section, not just after the table.

[...]
> > +(define (json-fetch* url)
> > +  "Return a list/hash representation of the JSON resource URL, or #f on
> > +failure."
> > +  (call-with-output-file "/dev/null"
> > +    (lambda (null)
> > +      (with-error-to-port null
> > +        (lambda ()
> > +          (call-with-temporary-output-file
> > +           (lambda (temp port)
> > +             (and (url-fetch url temp)
> > +                  (call-with-input-file temp json->scm)))))))))
> Rather use (guix http-client) and something like:
>
>    (let ((port (http-fetch url)))
>      (dynamic-wind
>        (const #t)
>        (lambda ()
>          (json->scm port))
>        (lambda ()
>          (close-port port))))
>
> This avoids the temporary file creation etc.

This sounds preferable but did not work as I kept getting 403 forbidden. 
Displaying the URI for instance with (string-append uri "\n") gives the 
below. I understand the error is trivial, but just wanted to communicate 
the URI object.

ERROR: In procedure string-append:
ERROR: In procedure string-append: Wrong type (expecting string): 
#<<uri> scheme: https userinfo: #f host: "api.github.com" port: #f path: 
"/repos/torognes/vsearch/releases" query: 
"access_token=27907952ef87f3691d592b9dcd93cd4b6f20625f" fragment: #f>

> > +;; TODO: is there some code from elsewhere in guix that can be used instead of
> > +;; redefining?
> > +(define (find-extension url)
> > +  "Return the extension of the archive e.g. '.tar.gz' given a URL, or
> > +false if none is recognized"
> > +  (find (lambda x (string-suffix? (first x) url))
> > +        (list ".tar.gz" ".tar.bz2" ".tar.xz" ".zip" ".tar")))
> Remove this procedure and use (file-extension url) instead, from (guix utils).

I figured there was something out there. The problem is file-extension 
returns, for example, "gz" when we are after "tar.gz".

> > +(define (github-user-slash-repository url)
> > +  "Return a string e.g. arq5x/bedtools2 of the owner and the name of the
> > +repository separated by a forward slash, from a string URL of the form
> > +'https://github.com/arq5x/bedtools2/archive/v2.24.0.tar.gz'"
> > +  (let ((splits (string-split url #\/)))
> > +    (string-append (list-ref splits 3) "/" (list-ref splits 4))))
> Rather write it as:
>
>    (match (string-split (uri-path (string->uri url)) #\/)
>      ((owner project . rest)
>       (string-append owner "/" project)))
>
> > +    (if (eq? json #f)
> Rather: (if (not json).
>
> However, ‘http-fetch’ raises an &http-error condition when something
> goes wrong (it never returns #f.)  So…

Since we aren't using http-fetch for the above reasons I tried to use 
(if (not json)), but this did not work because "(if (list of hashes))" 
throws a Wrong type to apply error.

> > +        (if token
> > +            (error "Error downloading release information through the GitHub
> > +API when using a GitHub token")
> > +            (error "Error downloading release information through the GitHub
> > +API. This may be fixed by using an access token and setting the environment
> > +variable GUIX_GITHUB_TOKEN, for instance one procured from
> > +https://github.com/settings/tokens";))
> … this can be removed, and the whole thing becomes:
>
>    (guard (c ((http-get-error? c)
>               (warning (_ "failed to access ~a: ~a (~a)~%")
>                        (uri->string (http-get-error-uri c))
>                        (http-get-error-code c)
>                        (http-get-error-reason c))))
>      …)

I've not used for now.

> > +        (let ((proper-releases
> > +               (filter
> > +                (lambda (x)
> > +                  ;; example pre-release:
> > +                  ;;https://github.com/wwood/OrfM/releases/tag/v0.5.1
> > +                  ;; or an all-prerelease set
> > +                  ;;https://github.com/powertab/powertabeditor/releases
> > +                  (eq? (assoc-ref (hash-table->alist x) "prerelease") #f))
> Simply: (not (hash-ref x "prerelease")).

OK.

> > +          (if (eq? (length proper-releases) 0) #f ;empty releases list
> > +              (let*
> > +                  ((tag (assoc-ref (hash-table->alist (first proper-releases))
> > +                                   "tag_name"))
> Rather:
>
>    (match proper-releases
>      (()                       ;empty release list
>       #f)
>      ((release . rest)         ;one or more releases
>       (let* ((tag (hash-ref release "tag_name")) …)
>         …)))

OK.

> > +(define (latest-release guix-package)
> > +  "Return an <upstream-source> for the latest release of GUIX-PACKAGE."
> > +  (let* ((pkg (specification->package guix-package))
> Someone (Ricardo?) proposed recently to pass a package object instead of
> a package name to ‘latest-release’.
>
> We should do that ideally before this patch goes in, or otherwise soon.

As discussed in the other thread, let's just proceed without waiting for 
Ricardo's efforts.

> > -                 ((guix import pypi) => %pypi-updater)))
> > +                 ((guix import pypi) => %pypi-updater)
> > +                 %github-updater))
> Write it as:
>
>    ((guix import github) => %github-updater)
>
> so that users who do not have guile-json can still use ‘guix refresh’.

OK.

> Could you send an updated patch?  Looks like we’re almost there.

Not quite there it seems.

Thanks.

0001-import-Add-github-updater.patch
Description: Text Data