Broken `map-derivation' procedure

  • Open
  • quality assurance status badge
Details
2 participants
  • Sergio Pastor Pérez
  • Sergio Pastor Pérez
Owner
unassigned
Submitted by
Sergio Pastor Pérez
Severity
normal
S
S
Sergio Pastor Pérez wrote on 4 Jul 2024 16:59
(address . bug-guix@gnu.org)
PAXP251MB03489F8E853184BD49C9D310F3DE2@PAXP251MB0348.EURP251.PROD.OUTLOOK.COM
Hello.

The procedure `map-derivation` from `(guix derivations)` seems broken.

Evaluating this yields an error, it probably shouldn't:
Toggle snippet (19 lines)
scheme@(guix-user)> (use-modules (guix)
(guix derivations)
(gnu packages)
(gnu packages perl)
(gnu packages games))
scheme@(guix-user)> (with-store store
(let ((cowsay-drv (package-derivation store cowsay))
(perl-drv (package-derivation store perl))
(perl-5.6-drv (package-derivation store perl-5.6)))
(map-derivation store
cowsay-drv
`((,perl-drv . ,perl-5.6-drv)))))
ice-9/boot-9.scm:1685:16: In procedure raise-exception:
In procedure fport_read: Is a directory

Entering a new prompt. Type `,bt' for a backtrace or `,q' to continue.
scheme@(guix-user) [1]>

If you inspect the `cowsay` derivation, you will see that the mapping
should be possible since it contains the `perl` derivation.

Does anyone have an idea on what could be the issue or how to investigate
further?

Thanks,
Sergio.
S
S
Sergio Pastor Pérez wrote on 1 Sep 2024 18:15
[PATCH] guix: fix map-derivation not handling directories
(address . 71941@debbugs.gnu.org)(name . Sergio Pastor Pérez)(address . sergio.pastorperez@outlook.es)
PAXP251MB0348632A500D4E1B1A078B75F3912@PAXP251MB0348.EURP251.PROD.OUTLOOK.COM
The `map-derivation` procedure was trying to process directories as files.
When a derivation had a 'module import' directory as input, it threw an
exception since it tried to open it as a file.

Change-Id: I9b766f9aaa03ea9307f73e8abb36bc347af4b5e6
---
Hi, as far as I know 'module import' directories don't contain derivation
references, so it should not be needed to apply `substitute-file` on the files of
those directories. This fix just returns the 'module import' directories
untouched. Thoughts?

Note that `map-derivation` is very slow. I could only test it with tiny
derivations, such as the ones provided in the '(gnu packages commencement)'
module.

You can test it with:
Toggle snippet (17 lines)
scheme@(guix-user)> (use-modules (guix store)
(guix packages)
(guix derivations)
(gnu packages games)
(gnu packages bootstrap))
scheme@(guix-user)> (with-store store
(let ((bootar-drv (package-derivation store (@@ (gnu packages commencement) bootar)))
(guile-bootstrap-drv (package-derivation store %bootstrap-guile))
(cowsay-drv (package-derivation store cowsay)))
(map-derivation store
bootar-drv
`((,guile-bootstrap-drv . ,cowsay-drv)))))
$1 = #<derivation /gnu/store/qwn18yxc1ccdxq1mgg863lfxsfwng3wk-bootar-1b.drv => /gnu/store/852xy3bhck2sd1hq1rmzai0px7fplxfq-bootar-1b 7fcfc3f05b90>
scheme@(guix-user)> (derivation-inputs $1)
$2 = (#<<derivation-input> drv: #<derivation /gnu/store/5rx5dn2xnkjs3q0rzpm66q79ndwrafp7-module-import-compiled.drv => /gnu/store/472plnlfm8yrb3axwy16fydq01idbkv1-module-import-compiled 7fcfc3f05d70> sub-derivations: ("out")> #<<derivation-input> drv: #<derivation /gnu/store/fhqh9f3lmf8wd9mh0bzavpkjnmsb0bg0-cowsay-3.7.0.drv => /gnu/store/vwa9vh21l68ivnwxj18s2gxd1v71w43r-cowsay-3.7.0 7fcfb73a50f0> sub-derivations: ("out")> #<<derivation-input> drv: #<derivation /gnu/store/k6852ja7cvdvbbdxh24ph711gm74m3qq-bootar-1b.ses.drv => /gnu/store/xmw3h03svpw6rwfg03f0m608zkm24qx8-bootar-1b.ses 7fcfc3f05f00> sub-derivations: ("out")>)

As you can see, with this fix, the new derivation has the `cowsay` package a an
input.

I would like to encourage people to discuss ways to improve the performance of
this procedure. It would be very useful for system wide package rewriting as
discussed in this thread[1].


Regards,
Sergio.


guix/derivations.scm | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

Toggle diff (21 lines)
diff --git a/guix/derivations.scm b/guix/derivations.scm
index a91c1ae984..c16e1c2be3 100644
--- a/guix/derivations.scm
+++ b/guix/derivations.scm
@@ -1062,8 +1062,10 @@ (define* (map-derivation store drv mapping
((_ . replacement)
replacement)
(#f
- (substitute-file source
- initial replacements))))
+ (if (file-is-directory? source)
+ source
+ (substitute-file source
+ initial replacements)))))
(derivation-sources drv)))
;; Now augment the lists of initials and replacements.

base-commit: e1c92c98f7afff13fb7060199ba0dd4d9c5c2c53
--
2.45.2
S
S
Sergio Pastor Pérez wrote 5 days ago
[PATCH v2 1/2] guix: fix map-derivation not handling directories
(address . 71941@debbugs.gnu.org)
bd6d0cd65d321f03aad528391cc782564a405130.1738758574.git.sergio.pastorperez@gmail.com
The `map-derivation` procedure was trying to process directories as files.
When a derivation had a 'module import' directory as input, it threw an
exception since it tried to open it as a file.

Change-Id: I9b766f9aaa03ea9307f73e8abb36bc347af4b5e6
---
guix/derivations.scm | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

Toggle diff (21 lines)
diff --git a/guix/derivations.scm b/guix/derivations.scm
index bef98cd26a..9c019a35bb 100644
--- a/guix/derivations.scm
+++ b/guix/derivations.scm
@@ -1074,8 +1074,10 @@ (define* (map-derivation store drv mapping
((_ . replacement)
replacement)
(#f
- (substitute-file source
- initial replacements))))
+ (if (file-is-directory? source)
+ source
+ (substitute-file source
+ initial replacements)))))
(derivation-sources drv)))
;; Now augment the lists of initials and replacements.

base-commit: d0dbba3053123ee623d8a5889f1a0946859a205e
--
2.48.1
S
S
Sergio Pastor Pérez wrote 5 days ago
[PATCH v2 2/2] guix: fix: slow `map-derivation' procedure
(address . 71941@debbugs.gnu.org)
00a6dd1e400f182156e8fa5a1de062944aa6a37e.1738758574.git.sergio.pastorperez@gmail.com
Implement caching to speed up computation.

Change-Id: I186e2a62f6655e3b0738dd6e0f628faccd8b855e
---
guix/derivations.scm | 108 +++++++++++++++++++++++--------------------
1 file changed, 58 insertions(+), 50 deletions(-)

Toggle diff (128 lines)
diff --git a/guix/derivations.scm b/guix/derivations.scm
index 9c019a35bb..aa7f55ee92 100644
--- a/guix/derivations.scm
+++ b/guix/derivations.scm
@@ -1044,7 +1044,8 @@ (define* (map-derivation store drv mapping
((file . replacement)
(vhash-cons file replacement result))))
vlist-null
- mapping)))
+ mapping))
+ (computed-drvs (make-hash-table 100)))
(define rewritten-input
;; Rewrite the given input according to MAPPING, and return an input
;; in the format used in 'derivation' calls.
@@ -1060,55 +1061,62 @@ (define* (map-derivation store drv mapping
(derivation-input (loop drv) sub-drvs)))))))
(let loop ((drv drv))
- (let* ((inputs (map (cut rewritten-input <> loop)
- (derivation-inputs drv)))
- (initial (append-map derivation-input-output-paths
- (derivation-inputs drv)))
- (replacements (append-map input->output-paths inputs))
-
- ;; Sources typically refer to the output directories of the
- ;; original inputs, INITIAL. Rewrite them by substituting
- ;; REPLACEMENTS.
- (sources (map (lambda (source)
- (match (vhash-assoc source mapping)
- ((_ . replacement)
- replacement)
- (#f
- (if (file-is-directory? source)
- source
- (substitute-file source
- initial replacements)))))
- (derivation-sources drv)))
-
- ;; Now augment the lists of initials and replacements.
- (initial (append (derivation-sources drv) initial))
- (replacements (append sources replacements))
- (name (store-path-package-name
- (string-drop-right (derivation-file-name drv)
- 4))))
- (derivation store name
- (substitute (derivation-builder drv)
- initial replacements)
- (map (cut substitute <> initial replacements)
- (derivation-builder-arguments drv))
- #:system system
- #:env-vars (map (match-lambda
- ((var . value)
- `(,var
- . ,(substitute value initial
- replacements))))
- (derivation-builder-environment-vars drv))
- #:inputs (filter derivation-input? inputs)
- #:sources (append sources (filter string? inputs))
- #:outputs (derivation-output-names drv)
- #:hash (match (derivation-outputs drv)
- ((($ <derivation-output> _ algo hash))
- hash)
- (_ #f))
- #:hash-algo (match (derivation-outputs drv)
- ((($ <derivation-output> _ algo hash))
- algo)
- (_ #f)))))))
+ (let ((cached-drv (hash-ref computed-drvs drv)))
+ (if cached-drv
+ cached-drv
+ (let* ((inputs (map (cut rewritten-input <> loop)
+ (derivation-inputs drv)))
+ (initial (append-map derivation-input-output-paths
+ (derivation-inputs drv)))
+ (replacements (append-map input->output-paths inputs))
+
+ ;; Sources typically refer to the output directories of the
+ ;; original inputs, INITIAL. Rewrite them by substituting
+ ;; REPLACEMENTS.
+ (sources (map (lambda (source)
+ (match (vhash-assoc source mapping)
+ ((_ . replacement)
+ replacement)
+ (#f
+ (if (file-is-directory? source)
+ source
+ (substitute-file source
+ initial replacements)))))
+ (derivation-sources drv)))
+
+ ;; Now augment the lists of initials and replacements.
+ (initial (append (derivation-sources drv) initial))
+ (replacements (append sources replacements))
+ (name (store-path-package-name
+ (string-drop-right (derivation-file-name drv)
+ 4))))
+
+ (hash-set!
+ computed-drvs
+ drv
+ (derivation store name
+ (substitute (derivation-builder drv)
+ initial replacements)
+ (map (cut substitute <> initial replacements)
+ (derivation-builder-arguments drv))
+ #:system system
+ #:env-vars (map (match-lambda
+ ((var . value)
+ `(,var
+ . ,(substitute value initial
+ replacements))))
+ (derivation-builder-environment-vars drv))
+ #:inputs (filter derivation-input? inputs)
+ #:sources (append sources (filter string? inputs))
+ #:outputs (derivation-output-names drv)
+ #:hash (match (derivation-outputs drv)
+ ((($ <derivation-output> _ algo hash))
+ hash)
+ (_ #f))
+ #:hash-algo (match (derivation-outputs drv)
+ ((($ <derivation-output> _ algo hash))
+ algo)
+ (_ #f))))))))))
;;;
--
2.48.1
?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 71941@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 71941
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch