[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] draft addition of github updater
From: |
Ludovic Courtès |
Subject: |
Re: [PATCH] draft addition of github updater |
Date: |
Sun, 03 Jan 2016 21:46:37 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
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?
>>> 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" => "1.0"?
>> I guess so. What I would do is do that conversion when the tag matches
>> “^v[0-9]” and leave the tag as-is in other cases. WDYT?
>>
>> We can always add more heuristics later if we find that there’s 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’m not sure I completely follow ;-), but it’s 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’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.
>> 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! :-)
> 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.
> +;; 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 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).
> +(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…
> + (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))))
…)
> + (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-releases))
> + "tag_name"))
Rather:
(match proper-releases
(() ;empty release list
#f)
((release . rest) ;one or more releases
(let* ((tag (hash-ref release "tag_name")) …)
…)))
> +(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.
> - ((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’.
Could you send an updated patch? Looks like we’re almost there.
Thank you!
Ludo’.
- Re: [PATCH] draft addition of github updater,
Ludovic Courtès <=