Hi again, Thanks Ricardo for the reminder. :-)
Christopher Baines <m...@cbaines.net> skribis: > I've included a very rough patch which detects and informs the user what > has happened, this is an example of what this looks like (with a version > of the wip-rails branch I've broken): > > > $ guix build ruby-rails > > error: input loop detected, error generating a derivation for #<package > ruby-rails@5.0.0 > /home/chris/Projects/Guix/guix-wip-rails/gnu/packages/rails.scm:136 2d9f300> > > This shouldn't happen with Guix packages, please consider reporting a bug. > > Report bugs to: bug-g...@gnu.org. > GNU Guix home page: <https://www.gnu.org/software/guix/> > General help using GNU software: <http://www.gnu.org/gethelp/> > > If any of the packages below are not included in Guix, it could be that one of > them is causing the loop. The packages are listed in reverse order, so the > first package listed is a input to the second package for example, and the > start and end of the detected loop is highlighted with an arrow (--->). > > ---> #<package ruby-rspec@3.5.0 gnu/packages/ruby.scm:446 2b82d80> > #<package ruby-thread-order@1.1.0 gnu/packages/ruby.scm:6865 2bcc300> > #<package ruby-rspec-support@3.5.0 gnu/packages/ruby.scm:6822 2bcc3c0> > #<package ruby-rspec-core@3.5.4 gnu/packages/ruby.scm:325 2b6d300> > ---> #<package ruby-rspec@3.5.0 gnu/packages/ruby.scm:446 2b82d80> > #<package ruby-concurrent@1.0.5 gnu/packages/ruby.scm:4562 2bb3c00> > #<package ruby-activesupport@5.1.4 gnu/packages/ruby.scm:2794 2ba0900> > #<package ruby-actionview@5.0.0 > /home/chris/Projects/Guix/guix-wip-rails/gnu/packages/rails.scm:267 3b619c0> > #<package ruby-actionpack@5.0.0 > /home/chris/Projects/Guix/guix-wip-rails/gnu/packages/rails.scm:237 3b61c00> > #<package ruby-actioncable@5.0.0 > /home/chris/Projects/Guix/guix-wip-rails/gnu/packages/rails.scm:183 2d9f0c0> > #<package ruby-rails@5.0.0 > /home/chris/Projects/Guix/guix-wip-rails/gnu/packages/rails.scm:136 2d9f300> Neat. > I'm not particularly fond of the implementation, because the > package-derivation function is called from expand-input called from > bag->derivation, the information about the part of the graph that has > been traversed is passed through each function. > > The seen-package-list argument could be removed, but the ordering > information is really useful when printing out the error message. I > think it should be still possible to generate this after finding the > issue by searching through the graph of packages, which would allow > removing this one argument. ‘set->list’ preserves the order (actually the reverse order, but we could fix that) of insertion, because it’s just a vhash, and a vhash is just a list, which has a notion of ordering obviously: --8<---------------cut here---------------start------------->8--- scheme@(guile-user)> (set->list (setq 'a 'b 'c 'd)) $4 = (d c b a) scheme@(guile-user)> (set->list (apply set (map list (iota 4)))) $5 = ((3) (2) (1) (0)) scheme@(guile-user)> (set->list (apply set (iota 4))) $6 = (3 2 1 0) --8<---------------cut here---------------end--------------->8--- Thus we can get rid of ‘seen-package-list’. Another comment: > -(define* (expand-input store package input system #:optional cross-system) > +(define* (expand-input store package input system #:optional cross-system > + #:key seen-packages seen-package-list) Maybe s/seen-packages/visited/ (I’m not fond of passing an extra parameter around, but the only way to avoid it would be to use a state monad, and that in turn would work better once we’ve finally merged ‘wip-build-systems-gexp’…) > + (if (set-contains? seen-packages package) > + (begin > + (simple-format #t "\nerror: input loop detected, error generating a > derivation for ~A\n" > + (last seen-package-list)) > + (display " > +This shouldn't happen with Guix packages, please consider reporting a > bug.\n") > + (show-bug-report-information) > + (display " > +If any of the packages below are not included in Guix, it could be that one > of > +them is causing the loop. The packages are listed in reverse order, so the > +first package listed is a input to the second package for example, and the > +start and end of the detected loop is highlighted with an arrow (--->).\n\n") > + (for-each (lambda (seen-package) > + (if (eq? package seen-package) > + (display " --->")) > + (simple-format #t "\t~A\n" seen-package)) > + (cons package > + seen-package-list)) > + (exit 1))) Please just define a condition type and: (raise (condition (&package-cycle …))) with the UI part of it moved to (guix ui) in ‘call-with-error-handling’ with proper i18n. I think we’d need two more things: 1. Timing and memory reported by, say: time guix build libreoffice certbot pigx -d --no-grafts before and after the change. We should make sure the overhead in time and space is minimal. 2. One or two tests in tests/packages.scm that check whether the exception is raised when it should. Could you look into it, Chris? Thanks, and apologies for the long delay! Ludo’.
