[bug#33801] import: github: Support source URIs that redirect to GitHub

> Do you know how many packages fall into that category?

With this patch, we have a problem estimating the coverage using `guix
refresh -L'. Now, to estimate coverage, we need to make HTTP requests
for every single source tarball in Guix to determine if it redirects to
GitHub. This is an enormous number of HTTP requests! When I ran `guix
refresh -L', it took a very long time to finish coverage estimation. So,
I cancelled the command. Any better way to handle this?

>> +(define (follow-redirects-to-github uri)
>> +  "Follow redirects of URI until a GitHub URI is found. Return that GitHub
>> +URI. If no GitHub URI is found, return #f."
>
> Perhaps add the yt-dl.org example as a comment here.

I added a reference to the youtube-dl package in the comments. I also
added a few more comments in other places.

>> +  (define (follow-redirect uri)
>> +    (receive (response body) (http-get uri #:streaming? #t)
>
> Add: (close-port body).

I switched to using (http-head uri) instead of (http-get uri
#:streaming? #t). So, (close-port body) should no longer be required.

I also modified follow-redirects-to-github to avoid following redirects
on mirror and file URIs.

Please find attached a new patch.

Hi,

Arun Isaac <arunisaac@systemreboot.net> skribis:

>> Do you know how many packages fall into that category?
>
> With this patch, we have a problem estimating the coverage using `guix
> refresh -L'. Now, to estimate coverage, we need to make HTTP requests
> for every single source tarball in Guix to determine if it redirects to
> GitHub. This is an enormous number of HTTP requests! When I ran `guix
> refresh -L', it took a very long time to finish coverage estimation. So,
> I cancelled the command. Any better way to handle this?

Ouch, that’s a problem indeed.  We could maintain a cache of redirects,
although that wouldn’t be a real solution.

Hmm, now that I think about it, shouldn’t we store the github.com URL
directly for these packages?  We could add a new lint check along the
lines of the ‘check-mirror-url’ procedure that would allow us to find
out which URLs need to be changed.  If we took that route, the changes
you made to the importer would no longer be necessary.  :-/

WDYT?

Thanks,
Ludo’.

> Hmm, now that I think about it, shouldn’t we store the github.com URL
> directly for these packages?  We could add a new lint check along the
> lines of the ‘check-mirror-url’ procedure that would allow us to find
> out which URLs need to be changed.  If we took that route, the changes
> you made to the importer would no longer be necessary.  :-/

My changes only took a small amount of effort. I don't have much of an
issue with them being made obsolete. Shall I proceed with creating a
lint check along the lines of what you proposed?

Arun Isaac <arunisaac@systemreboot.net> skribis:

>> Hmm, now that I think about it, shouldn’t we store the github.com URL
>> directly for these packages?  We could add a new lint check along the
>> lines of the ‘check-mirror-url’ procedure that would allow us to find
>> out which URLs need to be changed.  If we took that route, the changes
>> you made to the importer would no longer be necessary.  :-/
>
> My changes only took a small amount of effort. I don't have much of an
> issue with them being made obsolete. Shall I proceed with creating a
> lint check along the lines of what you proposed?

Yes, if that sounds good to you, please do!

Ludo’.

> We could maintain a cache of redirects, although that wouldn’t be a
> real solution.
>
> Hmm, now that I think about it, shouldn’t we store the github.com URL
> directly for these packages?  We could add a new lint check along the
> lines of the ‘check-mirror-url’ procedure that would allow us to find
> out which URLs need to be changed.  If we took that route, the changes
> you made to the importer would no longer be necessary.  :-/

On the other hand, if we constrain our updater predicates to not perform
network operations, we would be restricting what we can achieve with
updaters. For example, suppose in the future we have a cgit-updater to
update packages hosted using cgit. The only way to detect that would be
using some kind of network operation. The way our updaters currently
work, we will only ever be able to properly handle centralized platforms
like GitHub, PyPI, etc. And, that's a pity.

Another solution, though somewhat inelegant and tedious, would be to add
an `updater' field to the package specification so that the packager can
explicitly specify what updater to use.

WDYT?

Arun Isaac <arunisaac@systemreboot.net> skribis:

>> We could maintain a cache of redirects, although that wouldn’t be a
>> real solution.
>>
>> Hmm, now that I think about it, shouldn’t we store the github.com URL
>> directly for these packages?  We could add a new lint check along the
>> lines of the ‘check-mirror-url’ procedure that would allow us to find
>> out which URLs need to be changed.  If we took that route, the changes
>> you made to the importer would no longer be necessary.  :-/
>
> On the other hand, if we constrain our updater predicates to not perform
> network operations, we would be restricting what we can achieve with
> updaters. For example, suppose in the future we have a cgit-updater to
> update packages hosted using cgit. The only way to detect that would be
> using some kind of network operation. The way our updaters currently
> work, we will only ever be able to properly handle centralized platforms
> like GitHub, PyPI, etc. And, that's a pity.

We can use heuristics in many cases to determine whether an updater
applies to a package; most of the time, that’s only based on the URL,
and it might also work for many cgit instances—those that have “/cgit”
in their URL.

Then again, I expect that a growing number of packages will be obtained
through ‘git-fetch’, and the updater for such a thing is trivial
(interestingly it’s not yet implemented though :-)).

So I feel like at present we don’t have strong evidence that that the
simple URL-based approach would not scale.

WDYT?

> Another solution, though somewhat inelegant and tedious, would be to add
> an `updater' field to the package specification so that the packager can
> explicitly specify what updater to use.

That could be useful to handle corner cases, but it should remain the
exception.

Thanks,
Ludo’.

> We can use heuristics in many cases to determine whether an updater
> applies to a package; most of the time, that’s only based on the URL,
> and it might also work for many cgit instances—those that have “/cgit”
> in their URL.
>
> Then again, I expect that a growing number of packages will be obtained
> through ‘git-fetch’, and the updater for such a thing is trivial
> (interestingly it’s not yet implemented though :-)).
>
> So I feel like at present we don’t have strong evidence that that the
> simple URL-based approach would not scale.

Agreed! Better to not complicate things beyond what is necessary at this
point. I am not confident that good heuristics exist. But, if we do use
git-fetch for the majority of our packages, like you said, updating will
be trivial.

I will proceed with the lint check as discussed earlier.

On Thu, 20 Dec 2018 17:28:42 +0100
Ludovic Courtès <ludo@gnu.org> wrote:

> Then again, I expect that a growing number of packages will be obtained
> through ‘git-fetch’, and the updater for such a thing is trivial
> (interestingly it’s not yet implemented though :-)).

See http://debbugs.gnu.org/cgi/bugreport.cgi?bug=33809

`~Eric