From mboxrd@z Thu Jan 1 00:00:00 1970 From: ludo@gnu.org (Ludovic =?utf-8?Q?Court=C3=A8s?=) Subject: Re: [PATCH] draft addition of github updater Date: Sun, 03 Jan 2016 21:46:37 +0100 Message-ID: <87ziwmqtle.fsf@gnu.org> References: <5647D2A8.8040603@uq.edu.au> <87h9kmb8zs.fsf@gnu.org> <5675F96E.4090609@uq.edu.au> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:38666) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aFpY7-0003RF-KP for guix-devel@gnu.org; Sun, 03 Jan 2016 15:46:49 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aFpY5-00030m-Rw for guix-devel@gnu.org; Sun, 03 Jan 2016 15:46:47 -0500 In-Reply-To: <5675F96E.4090609@uq.edu.au> (Ben Woodcroft's message of "Sun, 20 Dec 2015 10:42:22 +1000") List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org To: Ben Woodcroft Cc: "guix-devel@gnu.org" Ben Woodcroft 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 =E2=80=98version>?=E2=80=99, no? >>> I have two questions: >>> >>> 1. Some guess-work is required to get between the version as it is >>> defined in guix, and that presented in the github json, where only the >>> "tag_name" is available. Is it OK to be a little speculative in this >>> conversion e.g. "v1.0" =3D> "1.0"? >> I guess so. What I would do is do that conversion when the tag matches >> =E2=80=9C^v[0-9]=E2=80=9D and leave the tag as-is in other cases. WDYT? >> >> We can always add more heuristics later if we find that there=E2=80=99s = another >> widely-used convention for tag names. > Most seem to follow those few conventions, but there's still repos > that decided to be different e.g. > > https://github.com/vapoursynth/vapoursynth/archive/R28.tar.gz > https://github.com/synergy/synergy/archive/v1.7.4-stable.tar.gz > > Having gotten this far, I wonder if I've gone about it > backwards. Currently the updater works by asserting it is a > refreshable package by interrogating the source URI only. But it might > be easier to determine this with an API response on hand, by matching > the current release version number to a tag. Then if we assume the > same transformation of tag to version holds in the newest release, the > reverse transformation can be used on the newest tag to convert it > back into a version number. By transformation I mean addition of > [a-z\.\-] characters before and after the version number. This is > easier because guesswork is only needed to convert between the tag and > version number, without reference to a URI. > > This means more work for me, is it a good idea? As I understand it > would involve returning #t more often from "github-package?". If #f is > returned by an updater, do the updaters further down the chain get a > bite at the cherry too? It doesn't matter for now since the github > updater is last, but it might in the future. I=E2=80=99m not sure I completely follow ;-), but it=E2=80=99s fine to hard= -code the v[0-9\.]+ convention for now, esp. if it works for most packages. >> 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=E2=80=99d 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=E2=80=99s fine as is. There=E2=80=99s no security issue on the = client side AFAICS. >> 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=E2=80=99d have to hard-code heuristics to distinguish release ta= gs from other tags. Typically, again, considering only tags that match =E2=80=98v[0-9\.]+=E2=80=99. Well, future work! :-) > From a42eda6b9631cc28dfdd02d2c8bb02eabb2626b9 Mon Sep 17 00:00:00 2001 > From: Ben Woodcroft > 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 > +@uref{https://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 =E2=80=98github=E2=80=99 updater befor= e it has been introduced. > +;; TODO: Are all of these imports used? > +(define-module (guix import github) Should be easily checked. ;-) > +(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. > +;; TODO: is there some code from elsewhere in guix that can be used inst= ead 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 util= s). > +(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, =E2=80=98http-fetch=E2=80=99 raises an &http-error condition when = something goes wrong (it never returns #f.) So=E2=80=A6 > + (if token > + (error "Error downloading release information through the Gi= tHub > +API when using a GitHub token") > + (error "Error downloading release information through the Gi= tHub > +API. This may be fixed by using an access token and setting the environm= ent > +variable GUIX_GITHUB_TOKEN, for instance one procured from > +https://github.com/settings/tokens")) =E2=80=A6 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)))) =E2=80=A6) > + (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")). > + (if (eq? (length proper-releases) 0) #f ;empty releases list > + (let* > + ((tag (assoc-ref (hash-table->alist (first proper-rele= ases)) > + "tag_name")) Rather: (match proper-releases (() ;empty release list #f) ((release . rest) ;one or more releases (let* ((tag (hash-ref release "tag_name")) =E2=80=A6) =E2=80=A6))) > +(define (latest-release guix-package) > + "Return an 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 =E2=80=98latest-release=E2=80=99. We should do that ideally before this patch goes in, or otherwise soon. > - ((guix import pypi) =3D> %pypi-updater))) > + ((guix import pypi) =3D> %pypi-updater) > + %github-updater)) Write it as: ((guix import github) =3D> %github-updater) so that users who do not have guile-json can still use =E2=80=98guix refres= h=E2=80=99. Could you send an updated patch? Looks like we=E2=80=99re almost there. Thank you! Ludo=E2=80=99.