emacs-bug-tracker
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[debbugs-tracker] bug#33875: closed ([PATCH] `guix lint' warn of GitHub


From: GNU bug Tracking System
Subject: [debbugs-tracker] bug#33875: closed ([PATCH] `guix lint' warn of GitHub autogenerated source tarballs)
Date: Thu, 27 Dec 2018 12:59:02 +0000

Your message dated Thu, 27 Dec 2018 14:58:22 +0200
with message-id <address@hidden>
and subject line Re: [bug#33875] [PATCH] `guix lint' warn of GitHub 
autogenerated source tarballs
has caused the debbugs.gnu.org bug report #33875,
regarding [PATCH] `guix lint' warn of GitHub autogenerated source tarballs
to be marked as done.

(If you believe you have received this mail in error, please contact
address@hidden)


-- 
33875: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=33875
GNU Bug Tracking System
Contact address@hidden with problems
--- Begin Message --- Subject: [PATCH] `guix lint' warn of GitHub autogenerated source tarballs Date: Wed, 26 Dec 2018 10:47:13 +0200 User-agent: Mutt/1.11.0 (2018-11-25)
Forwarded from the discussion on guix-devel

-- 
Efraim Flashner   <address@hidden>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted
--- Begin Message --- Subject: Re: `guix lint' warn of GitHub autogenerated source tarballs Date: Tue, 25 Dec 2018 16:32:02 +0200 User-agent: Mutt/1.11.0 (2018-11-25)
On Fri, Dec 21, 2018 at 09:50:51PM +0100, Ludovic Courtès wrote:
> Hi!
> 
> Efraim Flashner <address@hidden> 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 <address@hidden>
> > 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   <address@hidden>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

Attachment: 0001-lint-Add-checker-for-unstable-tarballs.patch
Description: Text document

Attachment: signature.asc
Description: PGP signature


--- End Message ---

Attachment: signature.asc
Description: PGP signature


--- End Message ---
--- Begin Message --- Subject: Re: [bug#33875] [PATCH] `guix lint' warn of GitHub autogenerated source tarballs Date: Thu, 27 Dec 2018 14:58:22 +0200 User-agent: Mutt/1.11.0 (2018-11-25)
On Wed, Dec 26, 2018 at 06:40:22PM +0100, Ludovic Courtès wrote:
> Hello!
> 
> Efraim Flashner <address@hidden> skribis:
> 
> > From dcd8b207f932289cb3b35720af45f49f849b7c27 Mon Sep 17 00:00:00 2001
> > From: Efraim Flashner <address@hidden>
> > 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   <address@hidden>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

Attachment: signature.asc
Description: PGP signature


--- End Message ---

reply via email to

[Prev in Thread] Current Thread [Next in Thread]