GNU bug report logs - #33875
[PATCH] `guix lint' warn of GitHub autogenerated source tarballs

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 33875 in the body.
You can then email your comments to 33875 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Efraim Flashner <efraim <at> flashner.co.il>
To: guix-patches <at> gnu.org
Subject: [PATCH] `guix lint' warn of GitHub autogenerated source tarballs
Date: Wed, 26 Dec 2018 10:47:13 +0200

[Message part 1 (text/plain, inline)]

Forwarded from the discussion on guix-devel

-- 
Efraim Flashner   <efraim <at> flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[Message part 2 (message/rfc822, inline)]

From: Efraim Flashner <efraim <at> flashner.co.il>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: guix-devel <at> gnu.org
Subject: Re: `guix lint' warn of GitHub autogenerated source tarballs
Date: Tue, 25 Dec 2018 16:32:02 +0200

[Message part 3 (text/plain, inline)]

On Fri, Dec 21, 2018 at 09:50:51PM +0100, Ludovic Courtès wrote:
> Hi!
> 
> Efraim Flashner <efraim <at> flashner.co.il> skribis:
> 
> > Here's what I currently have. I don't think I've tried running the tests
> > I've written yet, and Ludo said there was a better way to check if the
> > download was a git-fetch or a url-fetch. As the logic is currently
> > written it'll flag any package hosted on github owned by 'archive' or
> > any package named 'archive' in addition to the ones we want.
> 
> OK.  I think you’re pretty much there anyway, so please don’t drop the
> ball.  ;-)
> 
> Some comments follow:
> 
> > From 8a07c8aea1f23db48a9e69956ad15f79f0f70e35 Mon Sep 17 00:00:00 2001
> > From: Efraim Flashner <efraim <at> flashner.co.il>
> > Date: Tue, 23 Oct 2018 12:01:53 +0300
> > Subject: [PATCH] lint: Add checker for unstable tarballs.
> >
> > * guix/scripts/lint.scm (check-source-unstable-tarball): New procedure.
> > (%checkers): Add it.
> > * tests/lint.scm ("source-unstable-tarball", source-unstable-tarball:
> > source #f", "source-unstable-tarball: valid", source-unstable-tarball:
> > not-github", source-unstable-tarball: git-fetch"): New tests.
> 
> [...]
> 
> > +(define (check-source-unstable-tarball package)
> > +  "Emit a warning if PACKAGE's source is an autogenerated tarball."
> > +  (define (github-tarball? origin)
> > +    (string-contains origin "github.com"))
> > +  (define (autogenerated-tarball? origin)
> > +    (string-contains origin "/archive/"))
> > +  (let ((origin (package-source package)))
> > +    (unless (not origin) ; check for '(source #f)'
> > +      (let ((uri       (origin-uri origin))
> > +            (dl-method (origin-method origin)))
> > +        (unless (not (pk dl-method "url-fetch"))
> > +        (when (and (github-tarball? uri)
> > +                   (autogenerated-tarball? uri))
> > +          (emit-warning package
> > +                        (G_ "the source URI should not be an autogenerated tarball")
> > +                        'source)))))))
> 
> You should use ‘origin-uris’ (plural), which always returns a list of
> URIs, and iterate on them (see ‘check-mirror-url’ as an example.)

That works really well

> 
> Also, when you have a URI, you can obtain just the host part and decode
> the path part like this:
> 
> --8<---------------cut here---------------start------------->8---
> scheme@(guile-user)> (string->uri "https://github.com/foo/bar/archive/whatnot")
> $2 = #<<uri> scheme: https userinfo: #f host: "github.com" port: #f path: "/foo/bar/archive/whatnot" query: #f fragment: #f>
> scheme@(guile-user)> (uri-host $2)
> $3 = "github.com"
> scheme@(guile-user)> (split-and-decode-uri-path (uri-path $2))
> $4 = ("foo" "bar" "archive" "whatnot")
> --8<---------------cut here---------------end--------------->8---
> 
> That way you should be able to get more accurate matching than with
> ‘string-contains’.  Does that make sense?

'third' from srfi-1 also helped a lot, considering how the github uris
are formatted.

> 
> The tests look good… but could you make sure they pass?  :-)

pfft, little things :) (forgot to export check-source-unstable-tarball)

> 
> Thank you!
> 
> Ludo’.

Next version attached

-- 
Efraim Flashner   <efraim <at> flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[0001-lint-Add-checker-for-unstable-tarballs.patch (text/plain, attachment)]

[signature.asc (application/pgp-signature, inline)]

[signature.asc (application/pgp-signature, inline)]

Message #8 received at 33875 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Efraim Flashner <efraim <at> flashner.co.il>
Cc: 33875 <at> debbugs.gnu.org
Subject: Re: [bug#33875] [PATCH] `guix lint' warn of GitHub autogenerated
 source tarballs
Date: Wed, 26 Dec 2018 18:40:22 +0100

Hello!

Efraim Flashner <efraim <at> flashner.co.il> skribis:

> From dcd8b207f932289cb3b35720af45f49f849b7c27 Mon Sep 17 00:00:00 2001
> From: Efraim Flashner <efraim <at> flashner.co.il>
> Date: Tue, 25 Dec 2018 16:29:12 +0200
> Subject: [PATCH] lint: Add checker for unstable tarballs.
>
> * guix/scripts/lint.scm (check-source-unstable-tarball): New procedure.
> (%checkers): Add it.
> * tests/lint.scm ("source-unstable-tarball", "source-unstable-tarball:
> source #f", "source-unstable-tarball: valid", "source-unstable-tarball:
> package named archive", "source-unstable-tarball: not-github",
> "source-unstable-tarball: git-fetch"): New tests.

Could you mention ‘source-unstable-tarball’ in doc/guix.texi?

Otherwise LGTM!

Please double-check that “make check TESTS=tests/lint.scm” passes and
that ‘./pre-inst-env guix lint -c source-unstable-tarball’ works.

Thanks!

Ludo’.

Message #13 received at 33875-done <at> debbugs.gnu.org (full text, mbox):

From: Efraim Flashner <efraim <at> flashner.co.il>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 33875-done <at> debbugs.gnu.org
Subject: Re: [bug#33875] [PATCH] `guix lint' warn of GitHub autogenerated
 source tarballs
Date: Thu, 27 Dec 2018 14:58:22 +0200

[Message part 1 (text/plain, inline)]

On Wed, Dec 26, 2018 at 06:40:22PM +0100, Ludovic Courtès wrote:
> Hello!
> 
> Efraim Flashner <efraim <at> flashner.co.il> skribis:
> 
> > From dcd8b207f932289cb3b35720af45f49f849b7c27 Mon Sep 17 00:00:00 2001
> > From: Efraim Flashner <efraim <at> flashner.co.il>
> > Date: Tue, 25 Dec 2018 16:29:12 +0200
> > Subject: [PATCH] lint: Add checker for unstable tarballs.
> >
> > * guix/scripts/lint.scm (check-source-unstable-tarball): New procedure.
> > (%checkers): Add it.
> > * tests/lint.scm ("source-unstable-tarball", "source-unstable-tarball:
> > source #f", "source-unstable-tarball: valid", "source-unstable-tarball:
> > package named archive", "source-unstable-tarball: not-github",
> > "source-unstable-tarball: git-fetch"): New tests.
> 
> Could you mention ‘source-unstable-tarball’ in doc/guix.texi?

Added

> 
> Otherwise LGTM!
> 
> Please double-check that “make check TESTS=tests/lint.scm” passes and
> that ‘./pre-inst-env guix lint -c source-unstable-tarball’ works.

‘./pre-inst-env guix lint -c source-unstable-tarball’ shows many many
autogenerated tarballs, and I made the checks pass.

> 
> Thanks!
> 
> Ludo’.

Pushed as c180017b6f7e9b6d23238c1fbaac986c435cd35e

-- 
Efraim Flashner   <efraim <at> flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[signature.asc (application/pgp-signature, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.