all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* [bug#68420] [PATCH 0/1] scripts: size: Add ‘system’ argument to ‘ensure-store-item’.
@ 2024-01-13  7:25 Hilton Chain via Guix-patches via
  2024-01-13  7:29 ` [bug#68420] [PATCH 1/1] " Hilton Chain via Guix-patches via
  0 siblings, 1 reply; 4+ messages in thread
From: Hilton Chain via Guix-patches via @ 2024-01-13  7:25 UTC (permalink / raw)
  To: 68420
  Cc: Hilton Chain, Christopher Baines, Josselin Poiret,
	Ludovic Courtès, Mathieu Othacehe, Ricardo Wurmus,
	Simon Tournier, Tobias Geerinckx-Rice, Zheng Junjie

This patch is a follow-up to <https://issues.guix.gnu.org/68363>.

Since the issue hasn't been found for a long time, I think it's better to pass
the system explicitly.

Thanks

Hilton Chain (1):
  scripts: size: Add ‘system’ argument to ‘ensure-store-item’.

 guix/scripts/size.scm | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)


base-commit: c0b303aaa3d6154acbe054120d11467eb98e6d33
--
2.41.0




^ permalink raw reply	[flat|nested] 4+ messages in thread

* [bug#68420] [PATCH 1/1] scripts: size: Add ‘system’ argument to ‘ensure-store-item’.
  2024-01-13  7:25 [bug#68420] [PATCH 0/1] scripts: size: Add ‘system’ argument to ‘ensure-store-item’ Hilton Chain via Guix-patches via
@ 2024-01-13  7:29 ` Hilton Chain via Guix-patches via
  2024-01-13  9:36   ` Mathieu Othacehe
  0 siblings, 1 reply; 4+ messages in thread
From: Hilton Chain via Guix-patches via @ 2024-01-13  7:29 UTC (permalink / raw)
  To: 68420
  Cc: Hilton Chain, Christopher Baines, Josselin Poiret,
	Ludovic Courtès, Mathieu Othacehe, Ricardo Wurmus,
	Simon Tournier, Tobias Geerinckx-Rice, Zheng Junjie

This is a follow-up to c245a54aab348642178129a9aad501b81a3089b4.

‘%current-system’ is already parameterized within ‘run-with-store’, the root
cause is that ‘mlet*’ bindings are evaluated before the parameterization.

* guix/scripts/size.scm (ensure-store-item): Add ‘system’ argument and pass it
to ‘package->derivation’.
(guix-size): Adjust accordingly.

Change-Id: I910af7c137737bcd0ee079e57a81c4114ab5ae32
---
 guix/scripts/size.scm | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/guix/scripts/size.scm b/guix/scripts/size.scm
index 8a8676a16f..0603c0498a 100644
--- a/guix/scripts/size.scm
+++ b/guix/scripts/size.scm
@@ -168,7 +168,7 @@ (define (store-profile items)
                (return (profile item size dependencies)))))
           sizes)))

-(define* (ensure-store-item spec-or-item)
+(define* (ensure-store-item spec-or-item #:optional (system (%current-system)))
   "Return a store file name.  If SPEC-OR-ITEM is a store file name, return it
 as is.  Otherwise, assume SPEC-OR-ITEM is a package output specification such
 as \"guile:debug\" or \"gcc-4.8\" and return its store file name."
@@ -177,7 +177,7 @@ (define* (ensure-store-item spec-or-item)
         (return spec-or-item)
         (let-values (((package output)
                       (specification->package+output spec-or-item)))
-          (mlet %store-monad ((drv (package->derivation package)))
+          (mlet %store-monad ((drv (package->derivation package system)))
             ;; Note: we don't try building DRV like 'guix archive' does
             ;; because we don't have to since we can instead rely on
             ;; substitute meta-data.
@@ -317,8 +317,7 @@ (define-command (guix-size . args)
           ;; Turn off grafts because (1) substitute servers do not serve grafted
           ;; packages, and (2) they do not make any difference on the
           ;; resulting size.
-          (parameterize ((%graft? #f)
-                         (%current-system system))
+          (parameterize ((%graft? #f))
             (with-store store
               (set-build-options store
                                  #:use-substitutes? #t
@@ -326,7 +325,9 @@ (define-command (guix-size . args)

               (run-with-store store
                 (mlet* %store-monad ((items   (mapm %store-monad
-                                                    ensure-store-item files))
+                                                    (cut ensure-store-item <>
+                                                         system)
+                                                  files))
                                      (profile (store-profile items)))
                   (if map-file
                       (begin
--
2.41.0




^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [bug#68420] [PATCH 1/1] scripts: size: Add ‘system’ argument to ‘ensure-store-item’.
  2024-01-13  7:29 ` [bug#68420] [PATCH 1/1] " Hilton Chain via Guix-patches via
@ 2024-01-13  9:36   ` Mathieu Othacehe
  2024-02-01  6:02     ` Hilton Chain via Guix-patches via
  0 siblings, 1 reply; 4+ messages in thread
From: Mathieu Othacehe @ 2024-01-13  9:36 UTC (permalink / raw)
  To: Hilton Chain
  Cc: Josselin Poiret, Tobias Geerinckx-Rice, Simon Tournier,
	Ludovic Courtès, 68420, Zheng Junjie, Ricardo Wurmus,
	Christopher Baines


Hey,

> -(define* (ensure-store-item spec-or-item)

When I have a look to the derivation that is computed in that procedure,
it looks like it has the expected system (the one passed as a cli
argument). Do you have any evidence of `guix size` doing the wrong thing
after c245a54aab?

Thanks,

Mathieu




^ permalink raw reply	[flat|nested] 4+ messages in thread

* [bug#68420] [PATCH 1/1] scripts: size: Add ‘system’ argument to ‘ensure-store-item’.
  2024-01-13  9:36   ` Mathieu Othacehe
@ 2024-02-01  6:02     ` Hilton Chain via Guix-patches via
  0 siblings, 0 replies; 4+ messages in thread
From: Hilton Chain via Guix-patches via @ 2024-02-01  6:02 UTC (permalink / raw)
  To: Mathieu Othacehe
  Cc: Josselin Poiret, Tobias Geerinckx-Rice, Simon Tournier,
	Ludovic Courtès, 68420, Zheng Junjie, Ricardo Wurmus,
	Christopher Baines

Hi Mathieu,

On Sat, 13 Jan 2024 17:36:37 +0800,
Mathieu Othacehe wrote:
>
>
> Hey,
>
> > -(define* (ensure-store-item spec-or-item)
>
> When I have a look to the derivation that is computed in that procedure,
> it looks like it has the expected system (the one passed as a cli
> argument). Do you have any evidence of `guix size` doing the wrong thing
> after c245a54aab?

Sorry for the confusion, c245a54aab did fix the issue, I should remove the
"follow-up" line in the commit message.

This patch is more of a style change:

Since system has been passed to ‘run-with-store’ and parameterized within it, we
only have to deal with the ‘mlet*’ bindings which are evaluated earlier.

For this specefic case, only ‘ensure-store-item’ used in ‘mlet*’ bindings needs
the system, so we can simply handle it there.  And I think parameterizing system
around ‘run-with-store’ again made the procedure less clear.

Thanks




^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2024-02-01  6:03 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-13  7:25 [bug#68420] [PATCH 0/1] scripts: size: Add ‘system’ argument to ‘ensure-store-item’ Hilton Chain via Guix-patches via
2024-01-13  7:29 ` [bug#68420] [PATCH 1/1] " Hilton Chain via Guix-patches via
2024-01-13  9:36   ` Mathieu Othacehe
2024-02-01  6:02     ` Hilton Chain via Guix-patches via

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/guix.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.