From mboxrd@z Thu Jan 1 00:00:00 1970 From: Efraim Flashner Subject: Re: `guix lint' warn of GitHub autogenerated source tarballs Date: Tue, 25 Dec 2018 16:32:02 +0200 Message-ID: <20181225143202.GO2581@macbook41> References: <87pntxwqx0.fsf@gnu.org> <08635A1A-EDA5-44B0-8C8A-532F16683154@flashner.co.il> <20181219192926.GB2581@macbook41> <87imzmmwno.fsf@gnu.org> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="PPxI8paQBs33t8dK" Return-path: Received: from eggs.gnu.org ([208.118.235.92]:38595) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gbnkg-0002sL-E7 for guix-devel@gnu.org; Tue, 25 Dec 2018 09:32:12 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gbnkc-0006lD-3t for guix-devel@gnu.org; Tue, 25 Dec 2018 09:32:10 -0500 Content-Disposition: inline In-Reply-To: <87imzmmwno.fsf@gnu.org> 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" To: Ludovic =?utf-8?Q?Court=C3=A8s?= Cc: guix-devel@gnu.org --PPxI8paQBs33t8dK Content-Type: multipart/mixed; boundary="RCJLo13VlymhPcEi" Content-Disposition: inline --RCJLo13VlymhPcEi Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Dec 21, 2018 at 09:50:51PM +0100, Ludovic Court=C3=A8s wrote: > Hi! >=20 > Efraim Flashner skribis: >=20 > > 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. >=20 > OK. I think you=E2=80=99re pretty much there anyway, so please don=E2=80= =99t drop the > ball. ;-) >=20 > Some comments follow: >=20 > > From 8a07c8aea1f23db48a9e69956ad15f79f0f70e35 Mon Sep 17 00:00:00 2001 > > From: Efraim Flashner > > 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. >=20 > [...] >=20 > > +(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 autogener= ated tarball") > > + 'source))))))) >=20 > You should use =E2=80=98origin-uris=E2=80=99 (plural), which always retur= ns a list of > URIs, and iterate on them (see =E2=80=98check-mirror-url=E2=80=99 as an e= xample.) That works really well >=20 > Also, when you have a URI, you can obtain just the host part and decode > the path part like this: >=20 > --8<---------------cut here---------------start------------->8--- > scheme@(guile-user)> (string->uri "https://github.com/foo/bar/archive/wha= tnot") > $2 =3D #< scheme: https userinfo: #f host: "github.com" port: #f pat= h: "/foo/bar/archive/whatnot" query: #f fragment: #f> > scheme@(guile-user)> (uri-host $2) > $3 =3D "github.com" > scheme@(guile-user)> (split-and-decode-uri-path (uri-path $2)) > $4 =3D ("foo" "bar" "archive" "whatnot") > --8<---------------cut here---------------end--------------->8--- >=20 > That way you should be able to get more accurate matching than with > =E2=80=98string-contains=E2=80=99. Does that make sense? 'third' from srfi-1 also helped a lot, considering how the github uris are formatted. >=20 > The tests look good=E2=80=A6 but could you make sure they pass? :-) pfft, little things :) (forgot to export check-source-unstable-tarball) >=20 > Thank you! >=20 > Ludo=E2=80=99. Next version attached --=20 Efraim Flashner =D7=90=D7=A4=D7=A8=D7=99=D7=9D = =D7=A4=D7=9C=D7=A9=D7=A0=D7=A8 GPG key =3D A28B F40C 3E55 1372 662D 14F7 41AA E7DC CA3D 8351 Confidentiality cannot be guaranteed on emails sent or received unencrypted --RCJLo13VlymhPcEi Content-Type: text/plain; charset=utf-8 Content-Disposition: attachment; filename="0001-lint-Add-checker-for-unstable-tarballs.patch" Content-Transfer-Encoding: quoted-printable =46rom dcd8b207f932289cb3b35720af45f49f849b7c27 Mon Sep 17 00:00:00 2001 =46rom: Efraim Flashner 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. --- guix/scripts/lint.scm | 23 ++++++++++++- tests/lint.scm | 80 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+), 1 deletion(-) diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm index 354f6f703..2c1c7ec66 100644 --- a/guix/scripts/lint.scm +++ b/guix/scripts/lint.scm @@ -7,7 +7,7 @@ ;;; Copyright =C2=A9 2016 Hartmut Goebel ;;; Copyright =C2=A9 2017 Alex Kost ;;; Copyright =C2=A9 2017 Tobias Geerinckx-Rice -;;; Copyright =C2=A9 2017 Efraim Flashner +;;; Copyright =C2=A9 2017, 2018 Efraim Flashner ;;; Copyright =C2=A9 2018 Arun Isaac ;;; ;;; This file is part of GNU Guix. @@ -76,6 +76,7 @@ check-home-page check-source check-source-file-name + check-source-unstable-tarball check-mirror-url check-github-url check-license @@ -752,6 +753,22 @@ descriptions maintained upstream." (G_ "the source file name should contain the package n= ame") 'source)))) =20 +(define (check-source-unstable-tarball package) + "Emit a warning if PACKAGE's source is an autogenerated tarball." + (define (check-source-uri uri) + (when (and (string=3D? (uri-host (string->uri uri)) "github.com") + (string=3D? (third (split-and-decode-uri-path + (uri-path (string->uri uri)))) + "archive")) + (emit-warning package + (G_ "the source URI should not be an autogenerated tar= ball") + 'source))) + (let ((origin (package-source package))) + (when (and (origin? origin) + (eqv? (origin-method origin) url-fetch)) + (let ((uris (origin-uris origin))) + (for-each check-source-uri uris))))) + (define (check-mirror-url package) "Check whether PACKAGE uses source URLs that should be 'mirror://'." (define (check-mirror-uri uri) ;XXX: could be optimized @@ -1098,6 +1115,10 @@ or a list thereof") (name 'source-file-name) (description "Validate file names of sources") (check check-source-file-name)) + (lint-checker + (name 'source-unstable-tarball) + (description "Check for autogenerated tarballs") + (check check-source-unstable-tarball)) (lint-checker (name 'derivation) (description "Report failure to compile a package to a derivation") diff --git a/tests/lint.scm b/tests/lint.scm index d4aa7c0e8..fe12bebd8 100644 --- a/tests/lint.scm +++ b/tests/lint.scm @@ -572,6 +572,86 @@ (check-source-file-name pkg))) "file name should contain the package name")))) =20 +(test-assert "source-unstable-tarball" + (string-contains + (with-warnings + (let ((pkg (dummy-package "x" + (source + (origin + (method url-fetch) + (uri "https://github.com/example/example/archive/v0.= 0.tar.gz") + (sha256 %null-sha256)))))) + (check-source-unstable-tarball pkg))) + "source URI should not be an autogenerated tarball")) + +(test-assert "source-unstable-tarball: source #f" + (not + (->bool + (string-contains + (with-warnings + (let ((pkg (dummy-package "x" + (source #f)))) + (check-source-unstable-tarball pkg))) + "source URI should not be an autogenerated tarball")))) + +(test-assert "source-unstable-tarball: valid" + (not + (->bool + (string-contains + (with-warnings + (let ((pkg (dummy-package "x" + (source + (origin + (method url-fetch) + (uri "https://github.com/example/example/releases= /download/x-0.0/x-0.0.tar.gz") + (sha256 %null-sha256)))))) + (check-source-unstable-tarball pkg))) + "source URI should not be an autogenerated tarball")))) + +(test-assert "source-unstable-tarball: package named archive" + (not + (->bool + (string-contains + (with-warnings + (let ((pkg (dummy-package "x" + (source + (origin + (method url-fetch) + (uri "https://github.com/example/archive/releases= /download/x-0.0/x-0.0.tar.gz") + (sha256 %null-sha256)))))) + (check-source-unstable-tarball pkg))) + "source URI should not be an autogenerated tarball")))) + +(test-assert "source-unstable-tarball: not-github" + (not + (->bool + (string-contains + (with-warnings + (let ((pkg (dummy-package "x" + (source + (origin + (method url-fetch) + (uri "https://bitbucket.org/archive/example/downl= oad/x-0.0.tar.gz") + (sha256 %null-sha256)))))) + (check-source-unstable-tarball pkg))) + "source URI should not be an autogenerated tarball")))) + +(test-assert "source-unstable-tarball: git-fetch" + (not + (->bool + (string-contains + (with-warnings + (let ((pkg (dummy-package "x" + (source + (origin + (method git-fetch) + (uri (git-reference + (url "https://github.com/archive/example.g= it") + (commit "0"))) + (sha256 %null-sha256)))))) + (check-source-unstable-tarball pkg))) + "source URI should not be an autogenerated tarball")))) + (test-skip (if (http-server-can-listen?) 0 1)) (test-equal "source: 200" "" --=20 2.20.1 --RCJLo13VlymhPcEi-- --PPxI8paQBs33t8dK Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCgAdFiEEoov0DD5VE3JmLRT3Qarn3Mo9g1EFAlwiP2IACgkQQarn3Mo9 g1H1Pg/7B3BjFfKmjOm9FylVKH3oHaWeu0RzF2BIFZwkiX83LliAtQb7PClBnh6N 6q13tJlR2EfqSeAJijryLtucmPlqLXJQU+WXpLeOGXpBMFTJk6BOmF/ukP0vBgrK 7GhPFOCldjCnCrebKekzkkpTChL7BygFLtc2kmxt7CDdPXZNNcN1p0W1a14Od4vk IvhcpM2wVRVhlw38PH2e0/X/q3DtKjrwQZchW7/9P6Lv1lim1rPFSTINd6gW9hqi bI1rYXjvYdoXhm9q0z/WYb5t9eb0bURBveol3fhaskLOCG2TSCw97lwP1UgfDbRP PziaaHMj4i/rjrJXxfkpjceBZGwHULGciqCNO6jO+w7wO75nSCnenXYqG6/++Wkw T9DCaOwJVvFbD20fmj6kCDOILPBiRWwr+fgW0qPI8KRrEowdwcSotSGrpS7ucH0E ogWzxiLFv+z5N9jPlkyXFDhqu525CK/A9XVcd4k4CTMbtXS9bNYTtu/D0+SRcR0b Tsdy4unxA0m4XABYkjXgWJJEV09j8yqt1N0IuupjP5NcNvS3jOHWcdbznoYZJRGg 6MQGW04IOsImbcpaH5ER1FdYA6HgjLsVnMiUKhi8FDZslX20OMB1RdYRYz2g+3Op PHr7mgRvqWl9qrn5kIcjfzyaYAm7/RRqajy+EMg2dBzmm6mn5cs= =kiHO -----END PGP SIGNATURE----- --PPxI8paQBs33t8dK--