Hi, On Fri, 18 Sep 2020 23:40:13 -0400 Mark H Weaver wrote: > I think it's important that Guix core functionality should be usable > without installing a collection of patented media codecs. Those plugins > should be purely optional. In my opinion, we should find a way to > eliminate those dependencies. I agree that it would be good to prevent weird dependencies from creeping in--for your stated reasons, but also for a lot of other reasons, chief of which is that the most secure source code is the source code that has been eliminated. Also, Guix sometimes pulls in transitive dependencies for the weirdest things. The "*-minimal" packages we have make it less bad. Still, it would nice to also have something that automatically checks whether there are weird transitive inputs of a package, for each package (*especially* in order to use that for "-minimal" packages). I sometimes add #:disallowed-references after tracking down problems of unintended transitive inputs (for example see f2fs-tools/static). But even that disallows just one specific reference (one package version). What I want is to disallow any package of that name entirely in the dependency graph--or even disallow references to specific source files (or other groups of packages) entirely. And I want it to keep disallowing it mechanically without me having to remember it. Guix lint already does something like I want, but for direct (non-transitive) inputs. It should be possible to add a "guix lint" check that also checks transitive inputs of packages for suspicious packages. The maintenance of a transitive-whitelist/-blacklist per package would then probably be have to be done inside guix lint, though. It could be nicer if there were package fields for those for it eventually. But for now, I guess inside guix lint is good enough. That said, for practicality one has to find some kind of groups of packages, in order to keep the whitelist/blacklist from ballooning. For now, I assume that each group has an extra source file--which we know is not true in Guix right now. But we could make it true. I started to add something to guix lint (possible procedures to use: package-transitive-inputs, package-transitive-propagated-inputs, package-transitive-native-inputs)--see patch below. But note that it just complains about everything now--we would still have to specify what is "bad". I would suggest to have a whitelist (of file names) and a blacklist (of file names), and the following: If a package has a whitelist and a transitive dependency is not on the whitelist, complain. If a package does not have a whitelist but does have a blacklist and a transitive dependency is on the blacklist, complain. I still find it illuminating as it is now. Try: $ guix lint qemu [...] gnu/packages/virtualization.scm:260:5: qemu@5.0.0: 'gnu/packages/dbm.scm' should probably not be referred to (but it is--because of packages (gdbm)) [...] gnu/packages/virtualization.scm:260:5: qemu@5.0.0: 'gnu/packages/spice.scm' should probably not be referred to (but it is by packages (libcacard spice usbredir virglrenderer spice-protocol)) gnu/packages/virtualization.scm:260:5: qemu@5.0.0: 'gnu/packages/gl.scm' should probably not be referred to (but it is by packages (libepoxy mesa)) [...] gnu/packages/virtualization.scm:260:5: qemu@5.0.0: 'gnu/packages/dbm.scm' should probably not be referred to (but it is by packages (gdbm)) [...] gnu/packages/virtualization.scm:260:5: qemu@5.0.0: 'gnu/packages/pulseaudio.scm' should probably not be referred to (but it is because of packages (pulseaudio)) WTF! Also, for the special case where no package in a source file A should refer to any package in a source file B, it could be enough to establish a convention of commenting out the respective "#:use-module (...)" in source file A (and adding a "DO NOT USE" text to it), and never deleting that comment. That way, once somebody had found what module one should not ever import, he could document that fact. diff --git a/guix/lint.scm b/guix/lint.scm index ec43a4dcad..d65ac34441 100644 --- a/guix/lint.scm +++ b/guix/lint.scm @@ -72,6 +72,7 @@ #:export (check-description-style check-inputs-should-be-native check-inputs-should-not-be-an-input-at-all + check-transitive-input-sanity check-patch-file-names check-synopsis-style check-derivation @@ -287,6 +288,38 @@ of a package, and INPUT-NAMES, a list of package specifications such as input)))) packages outputs)))) +(define (check-transitive-input-sanity package) + (let* ((examined-package-name (package-name package)) + (examined-package-location (package-location package)) + (examined-package-source-file-name (location-file examined-package-location)) + (examined-package-dependency-source-file-names + (delete examined-package-source-file-name + (delete-duplicates + (map (match-lambda + ((key dependency . rest) (location-file (package-location dependency)))) + (package-transitive-target-inputs package)))))) + (map (lambda (source-file-name) + (let ((packages-in-source-file + (filter (match-lambda + ((key dependency . rest) (string=? source-file-name + (location-file + (package-location dependency))))) + (package-transitive-target-inputs package)))) + (make-warning package + (G_ "'~a' should probably not be referred to (but it is--because of packages ~a)") + (list source-file-name (map (match-lambda + ((key dependency . rest) + (package-name dependency))) + packages-in-source-file)) + #:field 'inputs))) + examined-package-dependency-source-file-names))) + (delete examined-package-source-file-name + (delete-duplicates + (map (match-lambda + ((key dependency . rest) (location-file (package-location dependency)))) + (package-transitive-target-inputs package)))))) + (map (lambda (source-file-name) + (let ((packages-in-source-file + (filter (match-lambda + ((key dependency . rest) (string=? source-file-name + (location-file + (package-location dependency))))) + (package-transitive-target-inputs package)))) + (make-warning package + (G_ "'~a' should probably not be referred to (but it is--because of packages ~a)") + (list source-file-name (map (match-lambda + ((key dependency . rest) + (package-name dependency))) + packages-in-source-file)) + #:field 'inputs))) + examined-package-dependency-source-file-names))) + + ;; if examined-package-name like '%qemu%': + ;; (package-name (map package-transitive-inputs)) no gstreamer; source file not + ;; gstreamer.scm, gtk.scm. + ;; allowed references to location-files + ;; TODO: gstreamer should not be anywhere in any transitive inputs of any qemu + (define (check-inputs-should-be-native package) ;; Emit a warning if some inputs of PACKAGE are likely to belong to its ;; native inputs. @@ -1378,6 +1411,10 @@ them for PACKAGE." (define %local-checkers (list + (lint-checker + (name 'transitive-inputs) + (description "Checks transitive inputs") + (check check-transitive-input-sanity)) (lint-checker (name 'description) (description "Validate package descriptions")