all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* Outreachy - Guix Data Service: implementing basic json output for derivation comparison page
@ 2021-04-14 19:48 Luciana Lima Brito
  2021-04-15  8:46 ` Christopher Baines
  0 siblings, 1 reply; 22+ messages in thread
From: Luciana Lima Brito @ 2021-04-14 19:48 UTC (permalink / raw)
  To: guix-devel

[-- Attachment #1: Type: text/plain, Size: 270 bytes --]

Hi,

I implemented a basic json output for the derivation comparison page,
for my first contribution as an Outreachy applicant.

The patch for the code I've changed is attached.
I'm waiting your reviews :)

-- 
Best Regards,

Luciana Lima Brito
MSc. in Computer Science

[-- Attachment #2: controller.patch --]
[-- Type: text/x-patch, Size: 6884 bytes --]

diff --git a/guix-data-service/web/compare/controller.scm b/guix-data-service/web/compare/controller.scm
index a6aa198..b7788cb 100644
--- a/guix-data-service/web/compare/controller.scm
+++ b/guix-data-service/web/compare/controller.scm
@@ -584,19 +584,115 @@
                       (derivation-differences-data conn
                                                    base-derivation
                                                    target-derivation)))))
-          (case (most-appropriate-mime-type
-                 '(application/json text/html)
-                 mime-types)
-            ((application/json)
-             (render-json
-              '((error . "unimplemented")) ; TODO
-              #:extra-headers http-headers-for-unchanging-content))
-            (else
-             (render-html
-              #:sxml (compare/derivation
-                      query-parameters
-                      data)
-              #:extra-headers http-headers-for-unchanging-content)))))))
+          (let ((outputs (assq-ref data 'outputs))
+                (inputs  (assq-ref data 'inputs))
+                (sources (assq-ref data 'sources))
+                (system  (assq-ref data 'system))
+                (builder (assq-ref data 'builder))
+                (args    (assq-ref data 'arguments))
+                (environment-variables (assq-ref data 'environment-variables))
+                (get-derivation-data 
+                 (lambda (items)
+                   (map
+                    (match-lambda
+                      ((name path hash-alg hash recursive)
+                       `(,@(if (null? name)
+                               '()
+                               `((name . ,name)))
+                         ,@(if (null? path)
+                               '()
+                               `((path . ,path))
+                               )
+                         ,@(if (or (null? hash-alg) (not (string? hash-alg)))
+                               '()
+                               `((hash-algorithm . ,hash-alg))
+                               )
+                         ,@(if (or (null? hash) (not (string? hash)))
+                               '()
+                               `((hash . ,hash))
+                               )
+                         ,@(if (null? recursive)
+                               '()
+                               `((recursive . ,(string=? recursive "t"))))))
+                      ((derivation output)
+                       `(,@(if (null? derivation)
+                               '()
+                               `((derivation . ,derivation)))
+                         ,@(if (null? output)
+                               '()
+                               `((output . ,output)))))
+                      ((derivation)
+                       `(,@(if (null? derivation)
+                               '()
+                               `((derivation . ,derivation))))))
+                    (or items '())))))
+            
+            (let ((base-system (assq-ref system 'base))
+                  (target-system (assq-ref system 'target))
+                  (common-system (assq-ref system 'common))
+
+                  (base-builder (assq-ref builder 'base))
+                  (target-builder (assq-ref builder 'target))
+                  (common-builder (assq-ref builder 'common))
+
+                  (base-args (assq-ref args 'base))
+                  (target-args (assq-ref args 'target))
+                  (common-args (assq-ref args 'common)))
+
+              (let ((matched-outputs (append-map get-derivation-data
+                                                 (list (assq-ref outputs 'base)
+                                                       (assq-ref outputs 'target)
+                                                       (assq-ref outputs 'common))))
+                    (matched-inputs (append-map get-derivation-data
+                                                (list (assq-ref inputs 'base)
+                                                      (assq-ref inputs 'target))))
+                    (matched-sources (append-map get-derivation-data
+                                                 (list (assq-ref sources 'base)
+                                                       (assq-ref sources 'target)
+                                                       (assq-ref sources 'common)))))
+                (case (most-appropriate-mime-type
+                       '(application/json text/html)
+                       mime-types)
+                  ((application/json)
+                   (render-json
+                    `((revision
+                       . ((base
+                           . ((derivation . ,base-derivation)))
+                          (target
+                           . ((derivation . ,target-derivation)))))
+                      (outputs
+                       . ,((lambda (l) (cond
+                                        ((= (length l) 3) `((base . ,(first l))
+                                                            (target . ,(second l))
+                                                            (common . ,(third l))))
+                                        ((= (length l) 2) `((base . ,(first l))
+                                                            (target . ,(second l))))
+                                        (else `((common . ,(first l))))))
+                           matched-outputs))
+                      (inputs
+                       . ((base . ,(first matched-inputs))
+                          (target . ,(second matched-inputs))))
+                      (source
+                       . ((base . ,(first matched-sources))
+                          (target . ,(second matched-sources))
+                          (common . ,(third matched-sources))))                      
+                      (system
+                       . ((common . ,common-system)))
+                      (builder-and-arguments
+                       . ((builder . ,common-builder)
+                          (arguments
+                           . ((base . ,(list->vector
+                                        base-args))
+                              (target . ,(list->vector
+                                          target-args))))))
+                      (environment-variables . ,environment-variables))
+                    #:extra-headers http-headers-for-unchanging-content))
+                  (else
+                   (render-html
+                    #:sxml (compare/derivation
+                            query-parameters
+                            data)
+                    #:extra-headers http-headers-for-unchanging-content))))))))))
 
 (define (render-compare/package-derivations mime-types
                                             query-parameters)

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

* Re: Outreachy - Guix Data Service: implementing basic json output for derivation comparison page
  2021-04-14 19:48 Outreachy - Guix Data Service: implementing basic json output for derivation comparison page Luciana Lima Brito
@ 2021-04-15  8:46 ` Christopher Baines
  2021-04-15 16:09   ` Luciana Lima Brito
  0 siblings, 1 reply; 22+ messages in thread
From: Christopher Baines @ 2021-04-15  8:46 UTC (permalink / raw)
  To: Luciana Lima Brito; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 8713 bytes --]


Luciana Lima Brito <lubrito@posteo.net> writes:

> I implemented a basic json output for the derivation comparison page,
> for my first contribution as an Outreachy applicant.
>
> The patch for the code I've changed is attached.
> I'm waiting your reviews :)

Hi Luciana,

I'm not quite sure how to apply this, I'd suggest using git format-patch
to generate the file next time as I think there would normally be some
metadata along with the diff.

Looking at the diff though:

> diff --git a/guix-data-service/web/compare/controller.scm b/guix-data-service/web/compare/controller.scm
> index a6aa198..b7788cb 100644
> --- a/guix-data-service/web/compare/controller.scm
> +++ b/guix-data-service/web/compare/controller.scm
> @@ -584,19 +584,115 @@
>                        (derivation-differences-data conn
>                                                     base-derivation
>                                                     target-derivation)))))
> -          (case (most-appropriate-mime-type
> -                 '(application/json text/html)
> -                 mime-types)
> -            ((application/json)
> -             (render-json
> -              '((error . "unimplemented")) ; TODO
> -              #:extra-headers http-headers-for-unchanging-content))
> -            (else
> -             (render-html
> -              #:sxml (compare/derivation
> -                      query-parameters
> -                      data)
> -              #:extra-headers http-headers-for-unchanging-content)))))))
> +          (let ((outputs (assq-ref data 'outputs))
> +                (inputs  (assq-ref data 'inputs))
> +                (sources (assq-ref data 'sources))
> +                (system  (assq-ref data 'system))
> +                (builder (assq-ref data 'builder))
> +                (args    (assq-ref data 'arguments))
> +                (environment-variables (assq-ref data 'environment-variables))
> +                (get-derivation-data
> +                 (lambda (items)
> +                   (map
> +                    (match-lambda
> +                      ((name path hash-alg hash recursive)
> +                       `(,@(if (null? name)
> +                               '()
> +                               `((name . ,name)))
> +                         ,@(if (null? path)
> +                               '()
> +                               `((path . ,path))
> +                               )
> +                         ,@(if (or (null? hash-alg) (not (string? hash-alg)))
> +                               '()
> +                               `((hash-algorithm . ,hash-alg))
> +                               )
> +                         ,@(if (or (null? hash) (not (string? hash)))
> +                               '()
> +                               `((hash . ,hash))
> +                               )
> +                         ,@(if (null? recursive)
> +                               '()
> +                               `((recursive . ,(string=? recursive "t"))))))
> +                      ((derivation output)
> +                       `(,@(if (null? derivation)
> +                               '()
> +                               `((derivation . ,derivation)))
> +                         ,@(if (null? output)
> +                               '()
> +                               `((output . ,output)))))
> +                      ((derivation)
> +                       `(,@(if (null? derivation)
> +                               '()
> +                               `((derivation . ,derivation))))))
> +                    (or items '())))))
> +            
> +            (let ((base-system (assq-ref system 'base))
> +                  (target-system (assq-ref system 'target))
> +                  (common-system (assq-ref system 'common))
> +
> +                  (base-builder (assq-ref builder 'base))
> +                  (target-builder (assq-ref builder 'target))
> +                  (common-builder (assq-ref builder 'common))
> +
> +                  (base-args (assq-ref args 'base))
> +                  (target-args (assq-ref args 'target))
> +                  (common-args (assq-ref args 'common)))
> +
> +              (let ((matched-outputs (append-map get-derivation-data
> +                                                 (list (assq-ref outputs 'base)
> +                                                       (assq-ref outputs 'target)
> +                                                       (assq-ref outputs 'common))))
> +                    (matched-inputs (append-map get-derivation-data
> +                                                (list (assq-ref inputs 'base)
> +                                                      (assq-ref inputs 'target))))
> +                    (matched-sources (append-map get-derivation-data
> +                                                 (list (assq-ref sources 'base)
> +                                                       (assq-ref sources 'target)
> +                                                       (assq-ref sources 'common)))))

I would consider whether it's useful to have all these let blocks, and
whether here is the right place for them.

Taking a binding like outputs, it's only used in a later let. You can do
something like this (with let*) to remove the need to have multiple let
blocks.

  (let* ((outputs (assq-ref data 'outputs))
         (matched-outputs (append-map get-derivation-data
                           (list (assq-ref outputs 'base)
                                 (assq-ref outputs 'target)
                                 (assq-ref outputs 'common))))

Also, since matched-outputs is only used when rendering the JSON, I'd
move all the bindings that are only used for the JSON output within that
part of the case statement, so that it's clearer that they only apply to
that bit of the code.

Does that make sense?

> +                (case (most-appropriate-mime-type
> +                       '(application/json text/html)
> +                       mime-types)
> +                  ((application/json)
> +                   (render-json
> +                    `((revision

I'm not sure what revision here referrs to.

> +                       . ((base
> +                           . ((derivation . ,base-derivation)))
> +                          (target
> +                           . ((derivation . ,target-derivation)))))
> +                      (outputs
> +                       . ,((lambda (l) (cond
> +                                        ((= (length l) 3) `((base . ,(first l))
> +                                                            (target . ,(second l))
> +                                                            (common . ,(third l))))
> +                                        ((= (length l) 2) `((base . ,(first l))
> +                                                            (target . ,(second l))))
> +                                        (else `((common . ,(first l))))))
> +                           matched-outputs))
> +                      (inputs
> +                       . ((base . ,(first matched-inputs))
> +                          (target . ,(second matched-inputs))))
> +                      (source
> +                       . ((base . ,(first matched-sources))
> +                          (target . ,(second matched-sources))
> +                          (common . ,(third matched-sources))))
> +                      (system
> +                       . ((common . ,common-system)))
> +                      (builder-and-arguments
> +                       . ((builder . ,common-builder)
> +                          (arguments
> +                           . ((base . ,(list->vector
> +                                        base-args))
> +                              (target . ,(list->vector
> +                                          target-args))))))
> +                      (environment-variables . ,environment-variables))
> +                    #:extra-headers http-headers-for-unchanging-content))
> +                  (else
> +                   (render-html
> +                    #:sxml (compare/derivation
> +                            query-parameters
> +                            data)
> +                    #:extra-headers http-headers-for-unchanging-content))))))))))
>
>  (define (render-compare/package-derivations mime-types
>                                              query-parameters)

I hope that helps, just let me know if you have any questions,

Chris

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

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

* Re: Outreachy - Guix Data Service: implementing basic json output for derivation comparison page
  2021-04-15  8:46 ` Christopher Baines
@ 2021-04-15 16:09   ` Luciana Lima Brito
  2021-04-15 23:19     ` Christopher Baines
  0 siblings, 1 reply; 22+ messages in thread
From: Luciana Lima Brito @ 2021-04-15 16:09 UTC (permalink / raw)
  To: Christopher Baines; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 1517 bytes --]

On Thu, 15 Apr 2021 09:46:12 +0100
Christopher Baines <mail@cbaines.net> wrote:

Hi,
  
> I'm not quite sure how to apply this, I'd suggest using git
> format-patch to generate the file next time as I think there would
> normally be some metadata along with the diff.

I tried using git format-patch and I got 7 patches from my 7 commits,
then I generate a single patch output, which is attached.
The last commit before my modifications is this:
410f58cb43f083623885a430700c6818a187cadc

> I would consider whether it's useful to have all these let blocks, and
> whether here is the right place for them.
 
> Taking a binding like outputs, it's only used in a later let. You can
> do something like this (with let*) to remove the need to have
> multiple let blocks.

> Also, since matched-outputs is only used when rendering the JSON, I'd
> move all the bindings that are only used for the JSON output within
> that part of the case statement, so that it's clearer that they only
> apply to that bit of the code.
> 
> Does that make sense?

I did it, I used the let* and this helped a lot. I also moved
everything into the case branch of the json.

> I'm not sure what revision here referrs to.

It was a placeholder, but now I removed it.
 
> I hope that helps, just let me know if you have any questions,

The function get-derivation-data does not depend on anything, don't you
think it goes better in another place outside render-compare/derivation?

-- 
Best Regards,

Luciana Lima Brito
MSc. in Computer Science

[-- Attachment #2: controller-0415.patch --]
[-- Type: text/x-patch, Size: 65186 bytes --]

From c7af970c677a8d97cb4841a748e343c03e0bc886 Mon Sep 17 00:00:00 2001
From: Luciana Brito <lubrito@posteo.net>
Date: Sun, 11 Apr 2021 11:06:06 -0300
Subject: [PATCH 1/7] Include base-derivation and target-derivation on json of
 render-compare/derivation

---
 guix-data-service/web/compare/controller.scm | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/guix-data-service/web/compare/controller.scm b/guix-data-service/web/compare/controller.scm
index a6aa198..355ce03 100644
--- a/guix-data-service/web/compare/controller.scm
+++ b/guix-data-service/web/compare/controller.scm
@@ -589,7 +589,12 @@
                  mime-types)
             ((application/json)
              (render-json
-              '((error . "unimplemented")) ; TODO
+              `((revision
+                 . ((base
+                     . ((derivation . ,base-derivation)))
+                    (target
+                     . ((derivation . ,target-derivation))))))
+              ;'((error . "unimplemented")) ; TODO
               #:extra-headers http-headers-for-unchanging-content))
             (else
              (render-html
-- 
2.30.2


From d91856c6b4a0e998288b2291f9a9f420f9916dc4 Mon Sep 17 00:00:00 2001
From: Luciana Brito <lubrito@posteo.net>
Date: Mon, 12 Apr 2021 11:05:20 -0300
Subject: [PATCH 2/7] Include base and target outputs to json of
 render-compare/derivation

---
 guix-data-service/web/compare/controller.scm | 127 ++++++++++++++++---
 1 file changed, 109 insertions(+), 18 deletions(-)

diff --git a/guix-data-service/web/compare/controller.scm b/guix-data-service/web/compare/controller.scm
index 355ce03..2f54310 100644
--- a/guix-data-service/web/compare/controller.scm
+++ b/guix-data-service/web/compare/controller.scm
@@ -584,24 +584,115 @@
                       (derivation-differences-data conn
                                                    base-derivation
                                                    target-derivation)))))
-          (case (most-appropriate-mime-type
-                 '(application/json text/html)
-                 mime-types)
-            ((application/json)
-             (render-json
-              `((revision
-                 . ((base
-                     . ((derivation . ,base-derivation)))
-                    (target
-                     . ((derivation . ,target-derivation))))))
-              ;'((error . "unimplemented")) ; TODO
-              #:extra-headers http-headers-for-unchanging-content))
-            (else
-             (render-html
-              #:sxml (compare/derivation
-                      query-parameters
-                      data)
-              #:extra-headers http-headers-for-unchanging-content)))))))
+
+          (let ((outputs (assq-ref data 'outputs))
+                (inputs (assq-ref data 'inputs))
+                (sources (assq-ref data 'sources))
+                (system (assq-ref data 'system))
+                (build-and-args (assq-ref data 'build-and-args))
+                (envronment-and-variables (assq-ref data 'envronment-and-variables))
+                )
+            (let ((base-outputs (assq-ref outputs 'base))
+                  (target-outputs (assq-ref outputs 'target))
+                  
+                  (base-inputs (assq-ref inputs 'base))
+                  (target-inputs (assq-ref inputs 'target))
+
+                  (base-sources (assq-ref sources 'base))
+                  (target-sources (assq-ref sources 'target))
+
+                  (base-system (assq-ref system 'base))
+                  (target-system (assq-ref system 'target))
+
+                  (base-build-and-args (assq-ref build-and-args 'base))
+                  (target-build-and-args (assq-ref build-and-args 'target))
+
+                  (base-environment-and-variables (assq-ref envronment-and-variables 'base))
+                  (target-environment-and-variables (assq-ref envronment-and-variables 'target))
+                  
+                  )
+
+          ;(let-values
+          ;   (((base-derivation-output target-derivation-output)
+          ;      (assq-ref data 'outputs)))
+          ;  (let ((derivation-changes
+          ;         (package-derivation-data-changes names-and-versions
+          ;                                          base-packages-vhash
+          ;                                          target-packages-vhash)))
+          
+              (case (most-appropriate-mime-type
+                     '(application/json text/html)
+                     mime-types)
+                ((application/json)
+                 (render-json
+                  `((revision
+                     . ((base
+                         . ((derivation . ,base-derivation)))
+                        (target
+                         . ((derivation . ,target-derivation)))))
+                    (outputs . ((base . ,(list->vector
+                                         (map
+                                          (match-lambda
+                                            ((name path hash-alg hash recursive)
+                                             `(,@(if (null? name)
+                                                     '()
+                                                     `((name . ,name)))
+                                               ,@(if (null? path)
+                                                     '()
+                                                     `((path . ,path))
+                                                     )
+                                               ,@(if (null? hash-alg)
+                                                     '()
+                                                     `((hash-algorithm . ,hash-alg))
+                                                     )
+                                               ,@(if (null? hash)
+                                                     '()
+                                                     `((hash . ,hash))
+                                                     )
+                                               ,@(if (null? recursive)
+                                                     '()
+                                                     `((recursive . ,recursive))
+                                                     )
+                                        ;(change . ,change)
+                                               )))
+                                          base-outputs)))
+
+                             (target . ,(list->vector
+                                         (map
+                                          (match-lambda
+                                            ((name path hash-alg hash recursive)
+                                             `(,@(if (null? name)
+                                                     '()
+                                                     `((name . ,name)))
+                                               ,@(if (null? path)
+                                                     '()
+                                                     `((path . ,path))
+                                                     )
+                                               ,@(if (null? hash-alg)
+                                                     '()
+                                                     `((hash-algorithm . ,hash-alg))
+                                                     )
+                                               ,@(if (null? hash)
+                                                     '()
+                                                     `((hash . ,hash))
+                                                     )
+                                               ,@(if (null? recursive)
+                                                     '()
+                                                     `((recursive . ,recursive))
+                                                     )
+                                        ;(change . ,change)
+                                               )))
+                                          target-outputs))))
+                             
+                             )
+                    )
+                  #:extra-headers http-headers-for-unchanging-content))
+                (else
+                 (render-html
+                  #:sxml (compare/derivation
+                          query-parameters
+                          data)
+                  #:extra-headers http-headers-for-unchanging-content)))))))))
 
 (define (render-compare/package-derivations mime-types
                                             query-parameters)
-- 
2.30.2


From 3810e457e75a24ae1c1582a35956c6bc9e37c614 Mon Sep 17 00:00:00 2001
From: Luciana Brito <lubrito@posteo.net>
Date: Mon, 12 Apr 2021 15:52:17 -0300
Subject: [PATCH 3/7] Include base and target outputs to json in same anonymous
 function of render-compare/derivation

---
 guix-data-service/web/compare/controller.scm | 84 +++++++-------------
 1 file changed, 30 insertions(+), 54 deletions(-)

diff --git a/guix-data-service/web/compare/controller.scm b/guix-data-service/web/compare/controller.scm
index 2f54310..7fa2fc0 100644
--- a/guix-data-service/web/compare/controller.scm
+++ b/guix-data-service/web/compare/controller.scm
@@ -630,62 +630,38 @@
                          . ((derivation . ,base-derivation)))
                         (target
                          . ((derivation . ,target-derivation)))))
-                    (outputs . ((base . ,(list->vector
-                                         (map
-                                          (match-lambda
-                                            ((name path hash-alg hash recursive)
-                                             `(,@(if (null? name)
-                                                     '()
-                                                     `((name . ,name)))
-                                               ,@(if (null? path)
-                                                     '()
-                                                     `((path . ,path))
-                                                     )
-                                               ,@(if (null? hash-alg)
-                                                     '()
-                                                     `((hash-algorithm . ,hash-alg))
-                                                     )
-                                               ,@(if (null? hash)
-                                                     '()
-                                                     `((hash . ,hash))
-                                                     )
-                                               ,@(if (null? recursive)
-                                                     '()
-                                                     `((recursive . ,recursive))
-                                                     )
+                    (outputs . ,(list->vector
+                                  (append-map
+                                   (lambda (items)
+                                     (map
+                                      (match-lambda
+                                        ((name path hash-alg hash recursive)
+                                         `(,@(if (null? name)
+                                                 '()
+                                                 `((name . ,name)))
+                                           ,@(if (null? path)
+                                                 '()
+                                                 `((path . ,path))
+                                                 )
+                                           ,@(if (null? hash-alg)
+                                                 '()
+                                                 `((hash-algorithm . ,hash-alg))
+                                                 )
+                                           ,@(if (null? hash)
+                                                 '()
+                                                 `((hash . ,hash))
+                                                 )
+                                           ,@(if (null? recursive)
+                                                 '()
+                                                 `((recursive . ,recursive))
+                                                 )
                                         ;(change . ,change)
-                                               )))
-                                          base-outputs)))
+                                           )))
+                                      (or items '())))
 
-                             (target . ,(list->vector
-                                         (map
-                                          (match-lambda
-                                            ((name path hash-alg hash recursive)
-                                             `(,@(if (null? name)
-                                                     '()
-                                                     `((name . ,name)))
-                                               ,@(if (null? path)
-                                                     '()
-                                                     `((path . ,path))
-                                                     )
-                                               ,@(if (null? hash-alg)
-                                                     '()
-                                                     `((hash-algorithm . ,hash-alg))
-                                                     )
-                                               ,@(if (null? hash)
-                                                     '()
-                                                     `((hash . ,hash))
-                                                     )
-                                               ,@(if (null? recursive)
-                                                     '()
-                                                     `((recursive . ,recursive))
-                                                     )
-                                        ;(change . ,change)
-                                               )))
-                                          target-outputs))))
-                             
-                             )
-                    )
+                                   (list base-outputs
+                                         target-outputs))))
+                    );revision
                   #:extra-headers http-headers-for-unchanging-content))
                 (else
                  (render-html
-- 
2.30.2


From aa5292177bbe70564306cebdebff417754b807b6 Mon Sep 17 00:00:00 2001
From: Luciana Brito <lubrito@posteo.net>
Date: Wed, 14 Apr 2021 14:04:21 -0300
Subject: [PATCH 4/7] Fix rendering for outputs on json from
 render-compare/derivation

---
 guix-data-service/web/compare/controller.scm | 231 +++++++++++++------
 1 file changed, 165 insertions(+), 66 deletions(-)

diff --git a/guix-data-service/web/compare/controller.scm b/guix-data-service/web/compare/controller.scm
index 7fa2fc0..dcb0175 100644
--- a/guix-data-service/web/compare/controller.scm
+++ b/guix-data-service/web/compare/controller.scm
@@ -589,86 +589,185 @@
                 (inputs (assq-ref data 'inputs))
                 (sources (assq-ref data 'sources))
                 (system (assq-ref data 'system))
-                (build-and-args (assq-ref data 'build-and-args))
-                (envronment-and-variables (assq-ref data 'envronment-and-variables))
+                (builder (assq-ref data 'builder))
+                (args (assq-ref data 'arguments))
+                (environment-variables (assq-ref data 'environment-variables))
                 )
-            (let ((base-outputs (assq-ref outputs 'base))
-                  (target-outputs (assq-ref outputs 'target))
-                  
+
+            (let (;(base-outputs (assq-ref outputs 'base))
+                  ;(target-outputs (assq-ref outputs 'target))
+                  ;(common-outputs (assq-ref outputs 'common))
+
+                  (matched-outputs (append-map
+                                       (lambda (items)
+                                         (map
+                                          (match-lambda
+                                            ((name path hash-alg hash recursive)
+                                             `(,@(if (null? name)
+                                                     '()
+                                                     `((name . ,name)))
+                                               ,@(if (null? path)
+                                                     '()
+                                                     `((path . ,path))
+                                                     )
+                                               ,@(if (or (null? hash-alg) (not (string? hash-alg)))
+                                                     '()
+                                                     `((hash-algorithm . ,hash-alg))
+                                                     )
+                                               ,@(if (or (null? hash) (not (string? hash)))
+                                                     '()
+                                                     `((hash . ,hash))
+                                                     )
+                                               ,@(if (null? recursive)
+                                                     '()
+                                                     `((recursive . ,(string=? recursive "t")))))
+                                             ))
+                                          (or items '())))
+                                       (list (assq-ref outputs 'base)
+                                             (assq-ref outputs 'target)
+                                             (assq-ref outputs 'common))))
+              
                   (base-inputs (assq-ref inputs 'base))
                   (target-inputs (assq-ref inputs 'target))
 
                   (base-sources (assq-ref sources 'base))
                   (target-sources (assq-ref sources 'target))
+                  (common-sources (assq-ref sources 'common))
 
                   (base-system (assq-ref system 'base))
                   (target-system (assq-ref system 'target))
+                  (common-system (assq-ref system 'common))
 
-                  (base-build-and-args (assq-ref build-and-args 'base))
-                  (target-build-and-args (assq-ref build-and-args 'target))
+                  (base-builder (assq-ref builder 'base))
+                  (target-builder (assq-ref builder 'target))
+                  (common-builder (assq-ref builder 'common))
 
-                  (base-environment-and-variables (assq-ref envronment-and-variables 'base))
-                  (target-environment-and-variables (assq-ref envronment-and-variables 'target))
-                  
+                  (base-args (assq-ref args 'base))
+                  (target-args (assq-ref args 'target))
+                  (common-args (assq-ref args 'common))
                   )
 
-          ;(let-values
-          ;   (((base-derivation-output target-derivation-output)
-          ;      (assq-ref data 'outputs)))
-          ;  (let ((derivation-changes
-          ;         (package-derivation-data-changes names-and-versions
-          ;                                          base-packages-vhash
-          ;                                          target-packages-vhash)))
-          
-              (case (most-appropriate-mime-type
-                     '(application/json text/html)
-                     mime-types)
-                ((application/json)
-                 (render-json
-                  `((revision
-                     . ((base
-                         . ((derivation . ,base-derivation)))
-                        (target
-                         . ((derivation . ,target-derivation)))))
-                    (outputs . ,(list->vector
-                                  (append-map
-                                   (lambda (items)
-                                     (map
-                                      (match-lambda
-                                        ((name path hash-alg hash recursive)
-                                         `(,@(if (null? name)
-                                                 '()
-                                                 `((name . ,name)))
-                                           ,@(if (null? path)
-                                                 '()
-                                                 `((path . ,path))
-                                                 )
-                                           ,@(if (null? hash-alg)
-                                                 '()
-                                                 `((hash-algorithm . ,hash-alg))
-                                                 )
-                                           ,@(if (null? hash)
-                                                 '()
-                                                 `((hash . ,hash))
-                                                 )
-                                           ,@(if (null? recursive)
-                                                 '()
-                                                 `((recursive . ,recursive))
-                                                 )
-                                        ;(change . ,change)
-                                           )))
-                                      (or items '())))
+              (match environment-variables
+                ((name . values)
+                 (let ((environment-variables-name name)
+                       (common-environment-variables (assq-ref values 'common))
+                       (base-environment-variables (assq-ref values 'base))
+                       (target-environment-variables (assq-ref values 'target))
+                       )
+                 
+                   (case (most-appropriate-mime-type
+                          '(application/json text/html)
+                          mime-types)
+                     ((application/json)
+                      (render-json
+                       `((revision
+                          . ((base
+                              . ((derivation . ,base-derivation)))
+                             (target
+                              . ((derivation . ,target-derivation)))))
+                         ;(outputs . ,(list->vector
+                         ;             (append-map
+                         ;              (lambda (items)
+                         ;                (map
+                         ;                 (match-lambda
+                         ;                   ((name path hash-alg hash recursive)
+                         ;                    `(,@(if (null? name)
+                         ;                            '()
+                         ;                            `((name . ,name)))
+                         ;                      ,@(if (null? path)
+                         ;                            '()
+                         ;                            `((path . ,path))
+                         ;                            )
+                         ;                      ,@(if (or (null? hash-alg) (not (string? hash-alg)))
+                         ;                            '()
+                         ;                            `((hash-algorithm . ,hash-alg))
+                         ;                            )
+                         ;                      ,@(if (or (null? hash) (not (string? hash)))
+                         ;                            '()
+                         ;                            `((hash . ,hash))
+                         ;                            )
+                         ;                      ,@(if (null? recursive)
+                         ;                            '()
+                                        ;if there is other representation for "true"
+                                        ;this parse won't work
+                         ;                            `((recursive . ,(string=? recursive "t")))
+                         ;                            )
+                         ;                      )))
+                         ;                 (or items '()))
+                         ;                );inner lambda
 
-                                   (list base-outputs
-                                         target-outputs))))
-                    );revision
-                  #:extra-headers http-headers-for-unchanging-content))
-                (else
-                 (render-html
-                  #:sxml (compare/derivation
-                          query-parameters
-                          data)
-                  #:extra-headers http-headers-for-unchanging-content)))))))))
+                         ;              (list base-outputs
+                         ;                    target-outputs
+                                        ;                    common-outputs))))
+
+                         (output
+                          . ,((lambda (x) (cond
+                                           ((= (length x) 3) `((base . ,(first x))
+                                                               (target . ,(second x))
+                                                               (common . ,(third x))))
+                                           ((= (length x) 2) `((base . ,(first x))
+                                                               (target . ,(second x))))
+                                           (else `((common . ,(first x))))))
+
+                              matched-outputs))
+                         
+                         (inputs . ,(list->vector
+                                     (append-map
+                                      (lambda (items)
+                                        (map
+                                         (match-lambda
+                                           ((derivation output)
+                                            `(,@(if (null? derivation)
+                                                    '()
+                                                    `((derivation . ,derivation)))
+                                              ,@(if (null? output)
+                                                    '()
+                                                    `((output . ,output))))))
+                                         (or items '())))
+                                      (list base-inputs
+                                            target-inputs)
+                                      )))
+
+                         (source
+                          . ((base . ,base-sources)))
+                        
+;                         (sources . ,(list->vector
+;                                     (append-map
+;                                      (lambda (items)
+;                                        (map
+;                                         (match-lambda
+;                                           ((derivation)
+;                                            `(,@(if (null? derivation)
+;                                                    '()
+;                                                    `((derivation . ,derivation))))))
+;                                         (or items '())))
+;                                      (list base-sources
+;                                            target-sources
+;                                            common-sources)))
+;                          )                     
+
+                         (system
+                          . ((common . ,common-system)))
+
+                         (builder-and-arguments
+                          . ((builder . ,common-builder)
+                             (arguments
+                              . ((base . ,(list->vector
+                                           base-args))
+                                 (target . ,(list->vector
+                                             target-args))))))
+
+                         (environment-variables . ,environment-variables)
+
+                         
+                         );revision
+                       #:extra-headers http-headers-for-unchanging-content))
+                     (else
+                      (render-html
+                       #:sxml (compare/derivation
+                               query-parameters
+                               data)
+                       #:extra-headers http-headers-for-unchanging-content))))))))))))
 
 (define (render-compare/package-derivations mime-types
                                             query-parameters)
-- 
2.30.2


From b61825ca583b1a3d598575f12aa8c6e279ca60ad Mon Sep 17 00:00:00 2001
From: Luciana Brito <lubrito@posteo.net>
Date: Wed, 14 Apr 2021 15:37:19 -0300
Subject: [PATCH 5/7] Fix rendering for sources on json from
 remder-compare/derivation

---
 guix-data-service/web/compare/controller.scm | 169 +++++++------------
 1 file changed, 61 insertions(+), 108 deletions(-)

diff --git a/guix-data-service/web/compare/controller.scm b/guix-data-service/web/compare/controller.scm
index dcb0175..bfec00c 100644
--- a/guix-data-service/web/compare/controller.scm
+++ b/guix-data-service/web/compare/controller.scm
@@ -594,46 +594,46 @@
                 (environment-variables (assq-ref data 'environment-variables))
                 )
 
-            (let (;(base-outputs (assq-ref outputs 'base))
-                  ;(target-outputs (assq-ref outputs 'target))
-                  ;(common-outputs (assq-ref outputs 'common))
-
-                  (matched-outputs (append-map
-                                       (lambda (items)
-                                         (map
-                                          (match-lambda
-                                            ((name path hash-alg hash recursive)
-                                             `(,@(if (null? name)
-                                                     '()
-                                                     `((name . ,name)))
-                                               ,@(if (null? path)
-                                                     '()
-                                                     `((path . ,path))
-                                                     )
-                                               ,@(if (or (null? hash-alg) (not (string? hash-alg)))
-                                                     '()
-                                                     `((hash-algorithm . ,hash-alg))
-                                                     )
-                                               ,@(if (or (null? hash) (not (string? hash)))
-                                                     '()
-                                                     `((hash . ,hash))
-                                                     )
-                                               ,@(if (null? recursive)
-                                                     '()
-                                                     `((recursive . ,(string=? recursive "t")))))
-                                             ))
-                                          (or items '())))
-                                       (list (assq-ref outputs 'base)
-                                             (assq-ref outputs 'target)
-                                             (assq-ref outputs 'common))))
+            (let ((get-derivation-data 
+                   (lambda (items)
+                     (map
+                      (match-lambda
+                        ((name path hash-alg hash recursive)
+                         `(,@(if (null? name)
+                                 '()
+                                 `((name . ,name)))
+                           ,@(if (null? path)
+                                 '()
+                                 `((path . ,path))
+                                 )
+                           ,@(if (or (null? hash-alg) (not (string? hash-alg)))
+                                 '()
+                                 `((hash-algorithm . ,hash-alg))
+                                 )
+                           ,@(if (or (null? hash) (not (string? hash)))
+                                 '()
+                                 `((hash . ,hash))
+                                 )
+                           ,@(if (null? recursive)
+                                 '()
+                                 `((recursive . ,(string=? recursive "t"))))))
+                        ((derivation output)
+                         `(,@(if (null? derivation)
+                                 '()
+                                 `((derivation . ,derivation)))
+                           ,@(if (null? output)
+                                 '()
+                                 `((output . ,output)))))
+                        ((derivation)
+                         `(,@(if (null? derivation)
+                                 '()
+                                 `((derivation . ,derivation)))
+                           ))
+                         )
+                      (or items '())                                        
+                      ))
+                   )
               
-                  (base-inputs (assq-ref inputs 'base))
-                  (target-inputs (assq-ref inputs 'target))
-
-                  (base-sources (assq-ref sources 'base))
-                  (target-sources (assq-ref sources 'target))
-                  (common-sources (assq-ref sources 'common))
-
                   (base-system (assq-ref system 'base))
                   (target-system (assq-ref system 'target))
                   (common-system (assq-ref system 'common))
@@ -647,6 +647,19 @@
                   (common-args (assq-ref args 'common))
                   )
 
+              (let ((matched-outputs (append-map get-derivation-data
+                                                 (list (assq-ref outputs 'base)
+                                                       (assq-ref outputs 'target)
+                                                       (assq-ref outputs 'common))))
+                    (matched-inputs (append-map get-derivation-data
+                                                (list (assq-ref inputs 'base)
+                                                      (assq-ref inputs 'target))))
+                    (matched-sources (append-map get-derivation-data
+                                                (list (assq-ref sources 'base)
+                                                      (assq-ref sources 'target)
+                                                      (assq-ref sources 'common))))
+                    )
+
               (match environment-variables
                 ((name . values)
                  (let ((environment-variables-name name)
@@ -665,42 +678,7 @@
                               . ((derivation . ,base-derivation)))
                              (target
                               . ((derivation . ,target-derivation)))))
-                         ;(outputs . ,(list->vector
-                         ;             (append-map
-                         ;              (lambda (items)
-                         ;                (map
-                         ;                 (match-lambda
-                         ;                   ((name path hash-alg hash recursive)
-                         ;                    `(,@(if (null? name)
-                         ;                            '()
-                         ;                            `((name . ,name)))
-                         ;                      ,@(if (null? path)
-                         ;                            '()
-                         ;                            `((path . ,path))
-                         ;                            )
-                         ;                      ,@(if (or (null? hash-alg) (not (string? hash-alg)))
-                         ;                            '()
-                         ;                            `((hash-algorithm . ,hash-alg))
-                         ;                            )
-                         ;                      ,@(if (or (null? hash) (not (string? hash)))
-                         ;                            '()
-                         ;                            `((hash . ,hash))
-                         ;                            )
-                         ;                      ,@(if (null? recursive)
-                         ;                            '()
-                                        ;if there is other representation for "true"
-                                        ;this parse won't work
-                         ;                            `((recursive . ,(string=? recursive "t")))
-                         ;                            )
-                         ;                      )))
-                         ;                 (or items '()))
-                         ;                );inner lambda
-
-                         ;              (list base-outputs
-                         ;                    target-outputs
-                                        ;                    common-outputs))))
-
-                         (output
+                         (outputs
                           . ,((lambda (x) (cond
                                            ((= (length x) 3) `((base . ,(first x))
                                                                (target . ,(second x))
@@ -711,41 +689,16 @@
 
                               matched-outputs))
                          
-                         (inputs . ,(list->vector
-                                     (append-map
-                                      (lambda (items)
-                                        (map
-                                         (match-lambda
-                                           ((derivation output)
-                                            `(,@(if (null? derivation)
-                                                    '()
-                                                    `((derivation . ,derivation)))
-                                              ,@(if (null? output)
-                                                    '()
-                                                    `((output . ,output))))))
-                                         (or items '())))
-                                      (list base-inputs
-                                            target-inputs)
-                                      )))
+                         (inputs
+                          . ((base . ,(first matched-inputs))
+                             (target . ,(second matched-inputs))
+                              ))
 
                          (source
-                          . ((base . ,base-sources)))
+                          . ((base . ,(first matched-sources))
+                             (target . ,(second matched-sources))
+                             (common . ,(third matched-sources))))
                         
-;                         (sources . ,(list->vector
-;                                     (append-map
-;                                      (lambda (items)
-;                                        (map
-;                                         (match-lambda
-;                                           ((derivation)
-;                                            `(,@(if (null? derivation)
-;                                                    '()
-;                                                    `((derivation . ,derivation))))))
-;                                         (or items '())))
-;                                      (list base-sources
-;                                            target-sources
-;                                            common-sources)))
-;                          )                     
-
                          (system
                           . ((common . ,common-system)))
 
@@ -767,7 +720,7 @@
                        #:sxml (compare/derivation
                                query-parameters
                                data)
-                       #:extra-headers http-headers-for-unchanging-content))))))))))))
+                       #:extra-headers http-headers-for-unchanging-content)))))))))))))
 
 (define (render-compare/package-derivations mime-types
                                             query-parameters)
-- 
2.30.2


From 2c8d95aea7ef3f2950bbb125c995e009f61edf12 Mon Sep 17 00:00:00 2001
From: Luciana Brito <lubrito@posteo.net>
Date: Wed, 14 Apr 2021 16:28:43 -0300
Subject: [PATCH 6/7] Generate a complete json output for
 render-compare/derivation procedure for the derivation comparison page.

---
 guix-data-service/web/compare/controller.scm | 202 ++++++++-----------
 1 file changed, 87 insertions(+), 115 deletions(-)

diff --git a/guix-data-service/web/compare/controller.scm b/guix-data-service/web/compare/controller.scm
index bfec00c..b7788cb 100644
--- a/guix-data-service/web/compare/controller.scm
+++ b/guix-data-service/web/compare/controller.scm
@@ -584,57 +584,50 @@
                       (derivation-differences-data conn
                                                    base-derivation
                                                    target-derivation)))))
-
           (let ((outputs (assq-ref data 'outputs))
-                (inputs (assq-ref data 'inputs))
+                (inputs  (assq-ref data 'inputs))
                 (sources (assq-ref data 'sources))
-                (system (assq-ref data 'system))
+                (system  (assq-ref data 'system))
                 (builder (assq-ref data 'builder))
-                (args (assq-ref data 'arguments))
+                (args    (assq-ref data 'arguments))
                 (environment-variables (assq-ref data 'environment-variables))
-                )
-
-            (let ((get-derivation-data 
-                   (lambda (items)
-                     (map
-                      (match-lambda
-                        ((name path hash-alg hash recursive)
-                         `(,@(if (null? name)
-                                 '()
-                                 `((name . ,name)))
-                           ,@(if (null? path)
-                                 '()
-                                 `((path . ,path))
-                                 )
-                           ,@(if (or (null? hash-alg) (not (string? hash-alg)))
-                                 '()
-                                 `((hash-algorithm . ,hash-alg))
-                                 )
-                           ,@(if (or (null? hash) (not (string? hash)))
-                                 '()
-                                 `((hash . ,hash))
-                                 )
-                           ,@(if (null? recursive)
-                                 '()
-                                 `((recursive . ,(string=? recursive "t"))))))
-                        ((derivation output)
-                         `(,@(if (null? derivation)
-                                 '()
-                                 `((derivation . ,derivation)))
-                           ,@(if (null? output)
-                                 '()
-                                 `((output . ,output)))))
-                        ((derivation)
-                         `(,@(if (null? derivation)
-                                 '()
-                                 `((derivation . ,derivation)))
-                           ))
-                         )
-                      (or items '())                                        
-                      ))
-                   )
-              
-                  (base-system (assq-ref system 'base))
+                (get-derivation-data 
+                 (lambda (items)
+                   (map
+                    (match-lambda
+                      ((name path hash-alg hash recursive)
+                       `(,@(if (null? name)
+                               '()
+                               `((name . ,name)))
+                         ,@(if (null? path)
+                               '()
+                               `((path . ,path))
+                               )
+                         ,@(if (or (null? hash-alg) (not (string? hash-alg)))
+                               '()
+                               `((hash-algorithm . ,hash-alg))
+                               )
+                         ,@(if (or (null? hash) (not (string? hash)))
+                               '()
+                               `((hash . ,hash))
+                               )
+                         ,@(if (null? recursive)
+                               '()
+                               `((recursive . ,(string=? recursive "t"))))))
+                      ((derivation output)
+                       `(,@(if (null? derivation)
+                               '()
+                               `((derivation . ,derivation)))
+                         ,@(if (null? output)
+                               '()
+                               `((output . ,output)))))
+                      ((derivation)
+                       `(,@(if (null? derivation)
+                               '()
+                               `((derivation . ,derivation))))))
+                    (or items '())))))
+            
+            (let ((base-system (assq-ref system 'base))
                   (target-system (assq-ref system 'target))
                   (common-system (assq-ref system 'common))
 
@@ -644,8 +637,7 @@
 
                   (base-args (assq-ref args 'base))
                   (target-args (assq-ref args 'target))
-                  (common-args (assq-ref args 'common))
-                  )
+                  (common-args (assq-ref args 'common)))
 
               (let ((matched-outputs (append-map get-derivation-data
                                                  (list (assq-ref outputs 'base)
@@ -655,72 +647,52 @@
                                                 (list (assq-ref inputs 'base)
                                                       (assq-ref inputs 'target))))
                     (matched-sources (append-map get-derivation-data
-                                                (list (assq-ref sources 'base)
-                                                      (assq-ref sources 'target)
-                                                      (assq-ref sources 'common))))
-                    )
-
-              (match environment-variables
-                ((name . values)
-                 (let ((environment-variables-name name)
-                       (common-environment-variables (assq-ref values 'common))
-                       (base-environment-variables (assq-ref values 'base))
-                       (target-environment-variables (assq-ref values 'target))
-                       )
-                 
-                   (case (most-appropriate-mime-type
-                          '(application/json text/html)
-                          mime-types)
-                     ((application/json)
-                      (render-json
-                       `((revision
-                          . ((base
-                              . ((derivation . ,base-derivation)))
-                             (target
-                              . ((derivation . ,target-derivation)))))
-                         (outputs
-                          . ,((lambda (x) (cond
-                                           ((= (length x) 3) `((base . ,(first x))
-                                                               (target . ,(second x))
-                                                               (common . ,(third x))))
-                                           ((= (length x) 2) `((base . ,(first x))
-                                                               (target . ,(second x))))
-                                           (else `((common . ,(first x))))))
-
-                              matched-outputs))
-                         
-                         (inputs
-                          . ((base . ,(first matched-inputs))
-                             (target . ,(second matched-inputs))
-                              ))
-
-                         (source
-                          . ((base . ,(first matched-sources))
-                             (target . ,(second matched-sources))
-                             (common . ,(third matched-sources))))
-                        
-                         (system
-                          . ((common . ,common-system)))
-
-                         (builder-and-arguments
-                          . ((builder . ,common-builder)
-                             (arguments
-                              . ((base . ,(list->vector
-                                           base-args))
-                                 (target . ,(list->vector
-                                             target-args))))))
-
-                         (environment-variables . ,environment-variables)
-
-                         
-                         );revision
-                       #:extra-headers http-headers-for-unchanging-content))
-                     (else
-                      (render-html
-                       #:sxml (compare/derivation
-                               query-parameters
-                               data)
-                       #:extra-headers http-headers-for-unchanging-content)))))))))))))
+                                                 (list (assq-ref sources 'base)
+                                                       (assq-ref sources 'target)
+                                                       (assq-ref sources 'common)))))
+                (case (most-appropriate-mime-type
+                       '(application/json text/html)
+                       mime-types)
+                  ((application/json)
+                   (render-json
+                    `((revision
+                       . ((base
+                           . ((derivation . ,base-derivation)))
+                          (target
+                           . ((derivation . ,target-derivation)))))
+                      (outputs
+                       . ,((lambda (l) (cond
+                                        ((= (length l) 3) `((base . ,(first l))
+                                                            (target . ,(second l))
+                                                            (common . ,(third l))))
+                                        ((= (length l) 2) `((base . ,(first l))
+                                                            (target . ,(second l))))
+                                        (else `((common . ,(first l))))))
+                           matched-outputs))
+                      (inputs
+                       . ((base . ,(first matched-inputs))
+                          (target . ,(second matched-inputs))))
+                      (source
+                       . ((base . ,(first matched-sources))
+                          (target . ,(second matched-sources))
+                          (common . ,(third matched-sources))))                      
+                      (system
+                       . ((common . ,common-system)))
+                      (builder-and-arguments
+                       . ((builder . ,common-builder)
+                          (arguments
+                           . ((base . ,(list->vector
+                                        base-args))
+                              (target . ,(list->vector
+                                          target-args))))))
+                      (environment-variables . ,environment-variables))
+                    #:extra-headers http-headers-for-unchanging-content))
+                  (else
+                   (render-html
+                    #:sxml (compare/derivation
+                            query-parameters
+                            data)
+                    #:extra-headers http-headers-for-unchanging-content))))))))))
 
 (define (render-compare/package-derivations mime-types
                                             query-parameters)
-- 
2.30.2


From e8d99e4a422dc9ee01625e5a3206768ba09a2d54 Mon Sep 17 00:00:00 2001
From: Luciana Brito <lubrito@posteo.net>
Date: Thu, 15 Apr 2021 12:47:46 -0300
Subject: [PATCH 7/7] Improve json output for render-data/derivation

---
 guix-data-service/web/compare/controller.scm | 213 +++++++++----------
 1 file changed, 103 insertions(+), 110 deletions(-)

diff --git a/guix-data-service/web/compare/controller.scm b/guix-data-service/web/compare/controller.scm
index b7788cb..882652a 100644
--- a/guix-data-service/web/compare/controller.scm
+++ b/guix-data-service/web/compare/controller.scm
@@ -583,116 +583,109 @@
                     (lambda (conn)
                       (derivation-differences-data conn
                                                    base-derivation
-                                                   target-derivation)))))
-          (let ((outputs (assq-ref data 'outputs))
-                (inputs  (assq-ref data 'inputs))
-                (sources (assq-ref data 'sources))
-                (system  (assq-ref data 'system))
-                (builder (assq-ref data 'builder))
-                (args    (assq-ref data 'arguments))
-                (environment-variables (assq-ref data 'environment-variables))
-                (get-derivation-data 
-                 (lambda (items)
-                   (map
-                    (match-lambda
-                      ((name path hash-alg hash recursive)
-                       `(,@(if (null? name)
-                               '()
-                               `((name . ,name)))
-                         ,@(if (null? path)
-                               '()
-                               `((path . ,path))
-                               )
-                         ,@(if (or (null? hash-alg) (not (string? hash-alg)))
-                               '()
-                               `((hash-algorithm . ,hash-alg))
-                               )
-                         ,@(if (or (null? hash) (not (string? hash)))
-                               '()
-                               `((hash . ,hash))
-                               )
-                         ,@(if (null? recursive)
-                               '()
-                               `((recursive . ,(string=? recursive "t"))))))
-                      ((derivation output)
-                       `(,@(if (null? derivation)
-                               '()
-                               `((derivation . ,derivation)))
-                         ,@(if (null? output)
-                               '()
-                               `((output . ,output)))))
-                      ((derivation)
-                       `(,@(if (null? derivation)
-                               '()
-                               `((derivation . ,derivation))))))
-                    (or items '())))))
-            
-            (let ((base-system (assq-ref system 'base))
-                  (target-system (assq-ref system 'target))
-                  (common-system (assq-ref system 'common))
-
-                  (base-builder (assq-ref builder 'base))
-                  (target-builder (assq-ref builder 'target))
-                  (common-builder (assq-ref builder 'common))
-
-                  (base-args (assq-ref args 'base))
-                  (target-args (assq-ref args 'target))
-                  (common-args (assq-ref args 'common)))
-
-              (let ((matched-outputs (append-map get-derivation-data
-                                                 (list (assq-ref outputs 'base)
-                                                       (assq-ref outputs 'target)
-                                                       (assq-ref outputs 'common))))
-                    (matched-inputs (append-map get-derivation-data
-                                                (list (assq-ref inputs 'base)
-                                                      (assq-ref inputs 'target))))
-                    (matched-sources (append-map get-derivation-data
-                                                 (list (assq-ref sources 'base)
-                                                       (assq-ref sources 'target)
-                                                       (assq-ref sources 'common)))))
-                (case (most-appropriate-mime-type
-                       '(application/json text/html)
-                       mime-types)
-                  ((application/json)
-                   (render-json
-                    `((revision
-                       . ((base
-                           . ((derivation . ,base-derivation)))
-                          (target
-                           . ((derivation . ,target-derivation)))))
-                      (outputs
-                       . ,((lambda (l) (cond
-                                        ((= (length l) 3) `((base . ,(first l))
-                                                            (target . ,(second l))
-                                                            (common . ,(third l))))
-                                        ((= (length l) 2) `((base . ,(first l))
-                                                            (target . ,(second l))))
-                                        (else `((common . ,(first l))))))
-                           matched-outputs))
-                      (inputs
-                       . ((base . ,(first matched-inputs))
-                          (target . ,(second matched-inputs))))
-                      (source
-                       . ((base . ,(first matched-sources))
-                          (target . ,(second matched-sources))
-                          (common . ,(third matched-sources))))                      
-                      (system
-                       . ((common . ,common-system)))
-                      (builder-and-arguments
-                       . ((builder . ,common-builder)
-                          (arguments
-                           . ((base . ,(list->vector
-                                        base-args))
-                              (target . ,(list->vector
-                                          target-args))))))
-                      (environment-variables . ,environment-variables))
-                    #:extra-headers http-headers-for-unchanging-content))
-                  (else
-                   (render-html
-                    #:sxml (compare/derivation
-                            query-parameters
-                            data)
-                    #:extra-headers http-headers-for-unchanging-content))))))))))
+                                                   target-derivation))))
+                  (get-derivation-data 
+                   (lambda (items)
+                     (map
+                      (match-lambda
+                        ((name path hash-alg hash recursive)
+                         `(,@(if (null? name)
+                                 '()
+                                 `((name . ,name)))
+                           ,@(if (null? path)
+                                 '()
+                                 `((path . ,path))
+                                 )
+                           ,@(if (or (null? hash-alg) (not (string? hash-alg)))
+                                 '()
+                                 `((hash-algorithm . ,hash-alg))
+                                 )
+                           ,@(if (or (null? hash) (not (string? hash)))
+                                 '()
+                                 `((hash . ,hash))
+                                 )
+                           ,@(if (null? recursive)
+                                 '()
+                                 `((recursive . ,(string=? recursive "t"))))))
+                        ((derivation output)
+                         `(,@(if (null? derivation)
+                                 '()
+                                 `((derivation . ,derivation)))
+                           ,@(if (null? output)
+                                 '()
+                                 `((output . ,output)))))
+                        ((derivation)
+                         `(,@(if (null? derivation)
+                                 '()
+                                 `((derivation . ,derivation))))))
+                      (or items '())))))
+          
+          (case (most-appropriate-mime-type
+                 '(application/json text/html)
+                 mime-types)
+            ((application/json)
+             (let* ((outputs (assq-ref data 'outputs))
+                    (base-outputs (append-map get-derivation-data (list (assq-ref outputs 'base))))
+                    (target-outputs (append-map get-derivation-data (list (assq-ref outputs 'target))))
+                    (common-outputs (append-map get-derivation-data (list (assq-ref outputs 'common))))
+                    
+                    (inputs  (assq-ref data 'inputs))
+                    (base-inputs (append-map get-derivation-data (list (assq-ref inputs 'base))))
+                    (target-inputs (append-map get-derivation-data (list (assq-ref inputs 'target))))
+                    
+                    (sources (assq-ref data 'sources))
+                    (base-sources (append-map get-derivation-data (list (assq-ref sources 'base))))
+                    (target-sources (append-map get-derivation-data (list (assq-ref sources 'target))))
+                    (common-sources (append-map get-derivation-data (list (assq-ref sources 'common))))
+                    
+                    (system  (assq-ref data 'system))
+                    (base-system (assq-ref system 'base))
+                    (target-system (assq-ref system 'target))
+                    (common-system (assq-ref system 'common))
+                    
+                    (builder (assq-ref data 'builder))
+                    (base-builder (assq-ref builder 'base))
+                    (target-builder (assq-ref builder 'target))
+                    (common-builder (assq-ref builder 'common))
+                    
+                    (args    (assq-ref data 'arguments))
+                    (base-args (assq-ref args 'base))
+                    (target-args (assq-ref args 'target))
+                    (common-args (assq-ref args 'common))
+                    (environment-variables (assq-ref data 'environment-variables)))              
+               (render-json
+                `((base
+                   . ((derivation . ,base-derivation)))
+                  (target
+                   . ((derivation . ,target-derivation)))
+                  (outputs
+                   . ((base . ,(list->vector base-outputs))
+                      (target . ,(list->vector target-outputs))
+                      (common . ,(list->vector common-outputs))))
+                  (inputs
+                   . ((base . ,(list->vector base-inputs))
+                      (target . ,(list->vector target-inputs))))                                
+                  (sources
+                   . ((base . ,(list->vector base-sources))
+                      (target . ,(list->vector target-sources))
+                      (common . ,(list->vector common-sources))))                  
+                  (system
+                   . ((common . ,common-system)))
+                  (builder-and-arguments
+                   . ((builder . ,common-builder)
+                      (arguments
+                       . ((base . ,(list->vector
+                                    base-args))
+                          (target . ,(list->vector
+                                      target-args))))))
+                  (environment-variables . ,environment-variables))
+                #:extra-headers http-headers-for-unchanging-content)))
+            (else
+             (render-html
+              #:sxml (compare/derivation
+                      query-parameters
+                      data)
+              #:extra-headers http-headers-for-unchanging-content)))))))
 
 (define (render-compare/package-derivations mime-types
                                             query-parameters)
-- 
2.30.2


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

* Re: Outreachy - Guix Data Service: implementing basic json output for derivation comparison page
  2021-04-15 16:09   ` Luciana Lima Brito
@ 2021-04-15 23:19     ` Christopher Baines
  2021-04-16 15:07       ` Luciana Lima Brito
  0 siblings, 1 reply; 22+ messages in thread
From: Christopher Baines @ 2021-04-15 23:19 UTC (permalink / raw)
  To: Luciana Lima Brito; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 2370 bytes --]


Luciana Lima Brito <lubrito@posteo.net> writes:

> On Thu, 15 Apr 2021 09:46:12 +0100
> Christopher Baines <mail@cbaines.net> wrote:
>
> Hi,
>
>> I'm not quite sure how to apply this, I'd suggest using git
>> format-patch to generate the file next time as I think there would
>> normally be some metadata along with the diff.
>
> I tried using git format-patch and I got 7 patches from my 7 commits,
> then I generate a single patch output, which is attached.
> The last commit before my modifications is this:
> 410f58cb43f083623885a430700c6818a187cadc

Ok, I looked at the overall diff, and it looks to me like these changes
should probably be one commit.

>> I would consider whether it's useful to have all these let blocks, and
>> whether here is the right place for them.
>
>> Taking a binding like outputs, it's only used in a later let. You can
>> do something like this (with let*) to remove the need to have
>> multiple let blocks.
>
>> Also, since matched-outputs is only used when rendering the JSON, I'd
>> move all the bindings that are only used for the JSON output within
>> that part of the case statement, so that it's clearer that they only
>> apply to that bit of the code.
>>
>> Does that make sense?
>
> I did it, I used the let* and this helped a lot. I also moved
> everything into the case branch of the json.
>
>> I'm not sure what revision here referrs to.
>
> It was a placeholder, but now I removed it.
>
>> I hope that helps, just let me know if you have any questions,
>
> The function get-derivation-data does not depend on anything, don't you
> think it goes better in another place outside render-compare/derivation?

On the get-derivation-data function, I wouldn't use the same function to
process the inputs, outputs and sources. The data for each is different,
so I would separate the code as well.

To avoid having to call a procedure three times, on the base, target and
common items, I'd consider following the same pattern in the HTML
generating code, map over a list of the lists, so something like:

  (map (lambda (name data)
         (cons name
               (match data
                 ((name path hash-alg hash recursive)
                  ...))))
       '(base target common)
       (list (assq-ref outputs 'base)
             (assq-ref outputs 'target)
             (assq-ref outputs 'common)))

Does that make sense?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

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

* Re: Outreachy - Guix Data Service: implementing basic json output for derivation comparison page
  2021-04-15 23:19     ` Christopher Baines
@ 2021-04-16 15:07       ` Luciana Lima Brito
  2021-04-16 15:47         ` Christopher Baines
  0 siblings, 1 reply; 22+ messages in thread
From: Luciana Lima Brito @ 2021-04-16 15:07 UTC (permalink / raw)
  To: Christopher Baines; +Cc: guix-devel

On Fri, 16 Apr 2021 00:19:46 +0100
Christopher Baines <mail@cbaines.net> wrote:

Hi,
 
> Ok, I looked at the overall diff, and it looks to me like these
> changes should probably be one commit.

I don't actually understand what you mean saying it should be one
commit. Do I have to make my seven commits become a single one? How do
I do that?
 
> On the get-derivation-data function, I wouldn't use the same function
> to process the inputs, outputs and sources. The data for each is
> different, so I would separate the code as well.

I understand that, but the logic to map the values for these three
bindings is the same, wouldn't it generate redundancies implementing
the same logic separately? 
> 
> To avoid having to call a procedure three times, on the base, target
> and common items, I'd consider following the same pattern in the HTML
> generating code, map over a list of the lists, so something like:
> 
>   (map (lambda (name data)
>          (cons name
>                (match data
>                  ((name path hash-alg hash recursive)
>                   ...))))
>        '(base target common)
>        (list (assq-ref outputs 'base)
>              (assq-ref outputs 'target)
>              (assq-ref outputs 'common)))
> 
> Does that make sense?

Actually I did it in a similar way before, but it resulted in a list
with all the values for base, target and common together, in which
I had to have another way to separate them and render on json, for
example, I tried appending "base", "target" or "common" names to each
list (similar to your function?), but them I had to convert this list to
a vector. Calling the function for each separately gave me a cleaner
output. Also, I think that sometimes you might have more than one output
for base, target like it does for common, and I fail to see how your
example function addresses this. In short, I couldn't see the benefit
of this over calling the function three times. Is it for organizational
purpose or am I simply wrong? This time I'm just not understanding.

:)

-- 
Best Regards,

Luciana Lima Brito
MSc. in Computer Science


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

* Re: Outreachy - Guix Data Service: implementing basic json output for derivation comparison page
  2021-04-16 15:07       ` Luciana Lima Brito
@ 2021-04-16 15:47         ` Christopher Baines
  2021-04-16 18:46           ` Luciana Lima Brito
  0 siblings, 1 reply; 22+ messages in thread
From: Christopher Baines @ 2021-04-16 15:47 UTC (permalink / raw)
  To: Luciana Lima Brito; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 3170 bytes --]


Luciana Lima Brito <lubrito@posteo.net> writes:

> On Fri, 16 Apr 2021 00:19:46 +0100
> Christopher Baines <mail@cbaines.net> wrote:
>
> Hi,
>
>> Ok, I looked at the overall diff, and it looks to me like these
>> changes should probably be one commit.
>
> I don't actually understand what you mean saying it should be one
> commit. Do I have to make my seven commits become a single one? How do
> I do that?

From looking at the content of your commits, I think they should be
merged together.

There's some information about that here for example:

  https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History#_squashing

>> On the get-derivation-data function, I wouldn't use the same function
>> to process the inputs, outputs and sources. The data for each is
>> different, so I would separate the code as well.
>
> I understand that, but the logic to map the values for these three
> bindings is the same, wouldn't it generate redundancies implementing
> the same logic separately?

I'm unsure three bindings are you referring to, can you clairfy?

>> To avoid having to call a procedure three times, on the base, target
>> and common items, I'd consider following the same pattern in the HTML
>> generating code, map over a list of the lists, so something like:
>>
>>   (map (lambda (name data)
>>          (cons name
>>                (match data
>>                  ((name path hash-alg hash recursive)
>>                   ...))))
>>        '(base target common)
>>        (list (assq-ref outputs 'base)
>>              (assq-ref outputs 'target)
>>              (assq-ref outputs 'common)))
>>
>> Does that make sense?
>
> Actually I did it in a similar way before, but it resulted in a list
> with all the values for base, target and common together, in which
> I had to have another way to separate them and render on json, for
> example, I tried appending "base", "target" or "common" names to each
> list (similar to your function?), but them I had to convert this list to
> a vector.

Getting a list with all of the values in individually was possibly due
to using append-map rather than map.

> Calling the function for each separately gave me a cleaner
> output. Also, I think that sometimes you might have more than one
> output for base, target like it does for common, and I fail to see how
> your example function addresses this. In short, I couldn't see the
> benefit of this over calling the function three times. Is it for
> organizational purpose or am I simply wrong? This time I'm just not
> understanding.

It's an organisational thing, code is generally more readable if the
scope for variables is tight and there's less indirection. Defining a
procedure is one form of indirection. It's often really helpful, but I
think it's unnecessary here.

You're right though about the above example not handling data being a
list, I think that's a fixable problem though, rather than the (match
data ...) bit, I'd suggest using map with match-lambda, probably wrapped
with list->vector if you want a vector which will be outputted as a JSON
array.

Does that make sense?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

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

* Re: Outreachy - Guix Data Service: implementing basic json output for derivation comparison page
  2021-04-16 15:47         ` Christopher Baines
@ 2021-04-16 18:46           ` Luciana Lima Brito
  2021-04-16 19:17             ` Christopher Baines
  0 siblings, 1 reply; 22+ messages in thread
From: Luciana Lima Brito @ 2021-04-16 18:46 UTC (permalink / raw)
  To: Christopher Baines; +Cc: guix-devel

On Fri, 16 Apr 2021 16:47:10 +0100
Christopher Baines <mail@cbaines.net> wrote:

Hi 


> From looking at the content of your commits, I think they should be
> merged together.
> 
> There's some information about that here for example:
> 
>   https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History#_squashing

Thanks, I'll look into it.

> >> On the get-derivation-data function, I wouldn't use the same
> >> function to process the inputs, outputs and sources. The data for
> >> each is different, so I would separate the code as well.  
> >
> > I understand that, but the logic to map the values for these three
> > bindings is the same, wouldn't it generate redundancies implementing
> > the same logic separately?  
> 
> I'm unsure three bindings are you referring to, can you clairfy?

The bindings I was talking about are "outputs", "inputs" and "sources",
the only difference between them are the number of elements each one
has, that is why I simply made a match clause for each one. If you think
it is better I can separate them. Also, do you think it would be clearer
if I move each one inside the json case branch?

> >> To avoid having to call a procedure three times, on the base,
> >> target and common items, I'd consider following the same pattern
> >> in the HTML generating code, map over a list of the lists, so
> >> something like:
> >>
> >>   (map (lambda (name data)
> >>          (cons name
> >>                (match data
> >>                  ((name path hash-alg hash recursive)
> >>                   ...))))
> >>        '(base target common)
> >>        (list (assq-ref outputs 'base)
> >>              (assq-ref outputs 'target)
> >>              (assq-ref outputs 'common)))
> >>
> >> Does that make sense?  
> >
> > Actually I did it in a similar way before, but it resulted in a list
> > with all the values for base, target and common together, in which
> > I had to have another way to separate them and render on json, for
> > example, I tried appending "base", "target" or "common" names to
> > each list (similar to your function?), but them I had to convert
> > this list to a vector.  
> 
> Getting a list with all of the values in individually was possibly due
> to using append-map rather than map.
> 
> > Calling the function for each separately gave me a cleaner
> > output. Also, I think that sometimes you might have more than one
> > output for base, target like it does for common, and I fail to see
> > how your example function addresses this. In short, I couldn't see
> > the benefit of this over calling the function three times. Is it for
> > organizational purpose or am I simply wrong? This time I'm just not
> > understanding.  
> 
> It's an organisational thing, code is generally more readable if the
> scope for variables is tight and there's less indirection. Defining a
> procedure is one form of indirection. It's often really helpful, but I
> think it's unnecessary here.
> 
> You're right though about the above example not handling data being a
> list, I think that's a fixable problem though, rather than the (match
> data ...) bit, I'd suggest using map with match-lambda, probably
> wrapped with list->vector if you want a vector which will be
> outputted as a JSON array.
> 
> Does that make sense?

I still don't quite get it. The function I had before was like this
(using inputs as example):

(append-map
 (lambda (label items) 
   (map
    (match-lambda
      ((derivation output)
       (...))
      (or items '()))))
 (list "base" "target" "common")
 (list (assq-ref inputs 'base)
       (assq-ref inputs 'target)
       (assq-ref inputs 'common)))

Which indeed I made based on the html code. However this outputs me
something like:

(("base" derivation output)
 ("target" derivation output)
 ("common" derivation output)
 ...)

where "..." are lots of different ("common" derivation output) lists.

The only way I could think of showing this, was transforming it to a
vector, which gives us the indexes from 0, and inside each one we
would have the label showing where it came from. Is that the way you
think it is better? (is this what your proposed function should
accomplish?)

I think the above output produces a bit less clean json,
that is why I changed this function to the last one I sent you, so I
don't need to pass any labels, because if I pass the base, target, and
common lists separately the output is already correct and we don't need
any vectors.

-- 
Best Regards,

Luciana Lima Brito
MSc. in Computer Science


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

* Re: Outreachy - Guix Data Service: implementing basic json output for derivation comparison page
  2021-04-16 18:46           ` Luciana Lima Brito
@ 2021-04-16 19:17             ` Christopher Baines
  2021-04-16 22:47               ` Luciana Lima Brito
  0 siblings, 1 reply; 22+ messages in thread
From: Christopher Baines @ 2021-04-16 19:17 UTC (permalink / raw)
  To: Luciana Lima Brito; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 6063 bytes --]


Luciana Lima Brito <lubrito@posteo.net> writes:

> On Fri, 16 Apr 2021 16:47:10 +0100
> Christopher Baines <mail@cbaines.net> wrote:
>
>> >> On the get-derivation-data function, I wouldn't use the same
>> >> function to process the inputs, outputs and sources. The data for
>> >> each is different, so I would separate the code as well.
>> >
>> > I understand that, but the logic to map the values for these three
>> > bindings is the same, wouldn't it generate redundancies implementing
>> > the same logic separately?
>>
>> I'm unsure three bindings are you referring to, can you clairfy?
>
> The bindings I was talking about are "outputs", "inputs" and "sources",
> the only difference between them are the number of elements each one
> has, that is why I simply made a match clause for each one. If you think
> it is better I can separate them. Also, do you think it would be clearer
> if I move each one inside the json case branch?

Ok, I think the data in the inputs, outputs and sources will be quite
different, because they're different things, right?

Looking at this page as an example [1], the inputs are each a derivation
and a string saying which output of that derivation is being used as the
input. The outputs have a number of bits of information, the name, path,
hash algorithm, hash and whether it's recursive, and the sources are
just a single string, the file.

1: https://data.guix.gnu.org/compare/derivation?base_derivation=/gnu/store/5s9mi0k3mk6cmakyckj2fcp4qb75rn79-debootstrap-1.0.123.drv&target_derivation=/gnu/store/vqhfs0dmfm4q0lr1w6sdhrbc2pa2jrar-debootstrap-1.0.123.drv

If it's the same function being used to process the inputs, outputs and
sources, then it's harder to quickly tell from the code what's going on,
as you don't know which code inside get-derivation-data applies to the
inputs, outputs or sources. Matching the code up directly with the data
it should process will make this much more obvious.

>> >> To avoid having to call a procedure three times, on the base,
>> >> target and common items, I'd consider following the same pattern
>> >> in the HTML generating code, map over a list of the lists, so
>> >> something like:
>> >>
>> >>   (map (lambda (name data)
>> >>          (cons name
>> >>                (match data
>> >>                  ((name path hash-alg hash recursive)
>> >>                   ...))))
>> >>        '(base target common)
>> >>        (list (assq-ref outputs 'base)
>> >>              (assq-ref outputs 'target)
>> >>              (assq-ref outputs 'common)))
>> >>
>> >> Does that make sense?
>> >
>> > Actually I did it in a similar way before, but it resulted in a list
>> > with all the values for base, target and common together, in which
>> > I had to have another way to separate them and render on json, for
>> > example, I tried appending "base", "target" or "common" names to
>> > each list (similar to your function?), but them I had to convert
>> > this list to a vector.
>>
>> Getting a list with all of the values in individually was possibly due
>> to using append-map rather than map.
>>
>> > Calling the function for each separately gave me a cleaner
>> > output. Also, I think that sometimes you might have more than one
>> > output for base, target like it does for common, and I fail to see
>> > how your example function addresses this. In short, I couldn't see
>> > the benefit of this over calling the function three times. Is it for
>> > organizational purpose or am I simply wrong? This time I'm just not
>> > understanding.
>>
>> It's an organisational thing, code is generally more readable if the
>> scope for variables is tight and there's less indirection. Defining a
>> procedure is one form of indirection. It's often really helpful, but I
>> think it's unnecessary here.
>>
>> You're right though about the above example not handling data being a
>> list, I think that's a fixable problem though, rather than the (match
>> data ...) bit, I'd suggest using map with match-lambda, probably
>> wrapped with list->vector if you want a vector which will be
>> outputted as a JSON array.
>>
>> Does that make sense?
>
> I still don't quite get it. The function I had before was like this
> (using inputs as example):
>
> (append-map
>  (lambda (label items)
>    (map
>     (match-lambda
>       ((derivation output)
>        (...))
>       (or items '()))))
>  (list "base" "target" "common")
>  (list (assq-ref inputs 'base)
>        (assq-ref inputs 'target)
>        (assq-ref inputs 'common)))
>
> Which indeed I made based on the html code. However this outputs me
> something like:
>
> (("base" derivation output)
>  ("target" derivation output)
>  ("common" derivation output)
>  ...)
>
> where "..." are lots of different ("common" derivation output) lists.
>
> The only way I could think of showing this, was transforming it to a
> vector, which gives us the indexes from 0, and inside each one we
> would have the label showing where it came from. Is that the way you
> think it is better? (is this what your proposed function should
> accomplish?)
>
> I think the above output produces a bit less clean json,
> that is why I changed this function to the last one I sent you, so I
> don't need to pass any labels, because if I pass the base, target, and
> common lists separately the output is already correct and we don't need
> any vectors.

So, append-map expects the procedure it runs on the elements in the
lists its provided to return a list. Whereas map would give you a list
of lists in this case, append-map effectively runs append on that list
of lists, so you end up with a list.

While a flatter list is what you want when building an HTML table, I
think you were looking to get a JSON object separating the common, base
and target elements, right? If so, then map, rather than append-map
should be more useful to you here. Since above you're passing in two
lists of three things, if the procedure passed to map returns a pair
with a string in the first position, you'll end up producing the scheme
version of a JSON object (an alist).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

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

* Re: Outreachy - Guix Data Service: implementing basic json output for derivation comparison page
  2021-04-16 19:17             ` Christopher Baines
@ 2021-04-16 22:47               ` Luciana Lima Brito
  2021-04-17  8:40                 ` Christopher Baines
  0 siblings, 1 reply; 22+ messages in thread
From: Luciana Lima Brito @ 2021-04-16 22:47 UTC (permalink / raw)
  To: Christopher Baines; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 1024 bytes --]

On Fri, 16 Apr 2021 20:17:45 +0100
Christopher Baines <mail@cbaines.net> wrote:

Hi,

I hope the patch is correct this time.
I considered all you said, so I separated the
functions to get outputs, inputs and sources. I also implemented
everything inside the case of the json/application.

> While a flatter list is what you want when building an HTML table, I
> think you were looking to get a JSON object separating the common,
> base and target elements, right? If so, then map, rather than
> append-map should be more useful to you here. Since above you're
> passing in two lists of three things, if the procedure passed to map
> returns a pair with a string in the first position, you'll end up
> producing the scheme version of a JSON object (an alist).

You were right about that, I'm using map now.

Please, let me know if I missed something.
Thanks in advance, I'm learning a great deal! :)
-- 
Best Regards,

Luciana Lima Brito
MSc. in Computer Science
Federal University of Uberlândia

[-- Attachment #2: 0001-Implement-basic-json-output-for-the-derivation-compa.patch --]
[-- Type: text/x-patch, Size: 7404 bytes --]

From b51dd007180e69f7da479a857afd48dfa60e32e7 Mon Sep 17 00:00:00 2001
From: Luciana Brito <lubrito@posteo.net>
Date: Sun, 11 Apr 2021 11:06:06 -0300
Subject: [PATCH] Implement basic json output for the derivation comparison
 page

---
 guix-data-service/web/compare/controller.scm | 122 ++++++++++++++++++-
 1 file changed, 119 insertions(+), 3 deletions(-)

diff --git a/guix-data-service/web/compare/controller.scm b/guix-data-service/web/compare/controller.scm
index a6aa198..1b0fc2f 100644
--- a/guix-data-service/web/compare/controller.scm
+++ b/guix-data-service/web/compare/controller.scm
@@ -588,9 +588,125 @@
                  '(application/json text/html)
                  mime-types)
             ((application/json)
-             (render-json
-              '((error . "unimplemented")) ; TODO
-              #:extra-headers http-headers-for-unchanging-content))
+             (let* ((outputs (assq-ref data 'outputs))
+                    (matched-outputs
+                     (map
+                      (lambda (label items)
+                        (cons label
+                              (map
+                               (match-lambda
+                                 ((name path hash-alg hash recursive)
+                                  `(,@(if (null? name)
+                                          '()
+                                          `((name . ,name)))
+                                    ,@(if (null? path)
+                                          '()
+                                          `((path . ,path))
+                                          )
+                                    ,@(if (or (null? hash-alg) (not (string? hash-alg)))
+                                          '()
+                                          `((hash-algorithm . ,hash-alg))
+                                          )
+                                    ,@(if (or (null? hash) (not (string? hash)))
+                                          '()
+                                          `((hash . ,hash))
+                                          )
+                                    ,@(if (null? recursive)
+                                          '()
+                                          `((recursive . ,(string=? recursive "t")))))))
+                               (or items '()))))
+                      (list "base" "target" "common")
+                      (list (assq-ref outputs 'base)
+                            (assq-ref outputs 'target)
+                            (assq-ref outputs 'common))))
+                    (base-outputs (list->vector (cdr (first matched-outputs))))
+                    (target-outputs (list->vector (cdr (second matched-outputs))))
+                    (common-outputs (list->vector (cdr (third matched-outputs))))
+
+                    (inputs  (assq-ref data 'inputs))
+                    (matched-inputs
+                     (map
+                      (lambda (label items)
+                        (cons label
+                              (map 
+                               (match-lambda
+                                 ((derivation output)
+                                  `(,@(if (null? derivation)
+                                          '()
+                                          `((derivation . ,derivation)))
+                                    ,@(if (null? output)
+                                          '()
+                                          `((output . ,output))))))
+                               (or items '()))))
+                      (list "base" "target" "common")
+                      (list (assq-ref inputs 'base)
+                            (assq-ref inputs 'target))))
+                    (base-inputs (list->vector (cdr (first matched-inputs))))
+                    (target-inputs (list->vector (cdr (second matched-inputs))))
+                    
+                    (sources (assq-ref data 'sources))
+                    (matched-sources
+                     (map
+                      (lambda (label items)
+                        (cons label
+                              (map
+                               (match-lambda
+                                 ((derivation)
+                                  `(,@(if (null? derivation)
+                                          '()
+                                          `((derivation . ,derivation))))))
+                               (or items '())))) 
+                      (list "base" "target" "common")
+                      (list (assq-ref sources 'base)
+                            (assq-ref sources 'target)
+                            (assq-ref sources 'common))))
+                    (base-sources (list->vector (cdr (first matched-sources))))
+                    (target-sources (list->vector (cdr (second matched-sources))))
+                    (common-sources (list->vector (cdr (third matched-sources))))
+                    
+                    (system  (assq-ref data 'system))
+                    (base-system (assq-ref system 'base))
+                    (target-system (assq-ref system 'target))
+                    (common-system (assq-ref system 'common))
+                    
+                    (builder (assq-ref data 'builder))
+                    (base-builder (assq-ref builder 'base))
+                    (target-builder (assq-ref builder 'target))
+                    (common-builder (assq-ref builder 'common))
+                    
+                    (args    (assq-ref data 'arguments))
+                    (base-args (assq-ref args 'base))
+                    (target-args (assq-ref args 'target))
+                    (common-args (assq-ref args 'common))
+                    (environment-variables (assq-ref data 'environment-variables)))
+               
+               (render-json
+                `((base
+                   . ((derivation . ,base-derivation)))
+                  (target
+                   . ((derivation . ,target-derivation)))
+                  (outputs
+                   . ((,(first (first matched-outputs)) . ,base-outputs)
+                      (,(first (second matched-outputs)) . ,target-outputs)
+                      (,(first (third matched-outputs)) . ,common-outputs)))
+                  (inputs
+                   . ((,(first (first matched-inputs)) . ,base-inputs)
+                      (,(first (second matched-inputs)) . ,target-inputs)))
+                  (sources                   
+                   . ((,(first (first matched-sources)) . ,base-sources)
+                      (,(first (second matched-sources)) . ,target-sources)
+                      (,(first (third matched-sources)) . ,common-sources)))
+                  (system
+                   . ((common . ,common-system)))
+                  (builder-and-arguments
+                   . ((builder . ,common-builder)
+                      (arguments
+                       . ((base . ,(list->vector
+                                    base-args))
+                          (target . ,(list->vector
+                                      target-args))))))
+                  (environment-variables . ,environment-variables))
+                #:extra-headers http-headers-for-unchanging-content)))
             (else
              (render-html
               #:sxml (compare/derivation
-- 
2.30.2


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

* Re: Outreachy - Guix Data Service: implementing basic json output for derivation comparison page
  2021-04-16 22:47               ` Luciana Lima Brito
@ 2021-04-17  8:40                 ` Christopher Baines
  2021-04-17 12:48                   ` Luciana Lima Brito
  0 siblings, 1 reply; 22+ messages in thread
From: Christopher Baines @ 2021-04-17  8:40 UTC (permalink / raw)
  To: Luciana Lima Brito; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 1428 bytes --]


Luciana Lima Brito <lubrito@posteo.net> writes:

> On Fri, 16 Apr 2021 20:17:45 +0100
> Christopher Baines <mail@cbaines.net> wrote:
>
> Hi,
>
> I hope the patch is correct this time.
> I considered all you said, so I separated the
> functions to get outputs, inputs and sources. I also implemented
> everything inside the case of the json/application.

Yep, that's looking good, much neater.

>> While a flatter list is what you want when building an HTML table, I
>> think you were looking to get a JSON object separating the common,
>> base and target elements, right? If so, then map, rather than
>> append-map should be more useful to you here. Since above you're
>> passing in two lists of three things, if the procedure passed to map
>> returns a pair with a string in the first position, you'll end up
>> producing the scheme version of a JSON object (an alist).
>
> You were right about that, I'm using map now.
>
> Please, let me know if I missed something.
> Thanks in advance, I'm learning a great deal! :)

I think you're getting there, but it looks like you're close to what you
want with matched-outputs say, and then later you pick bits out of that
alist, generate vectors from the lists, and then rebuild the alist. I
think you can remove all that complexity by just tweaking what you're
doing up when you generate matched-outputs. I think this is true for
matched-outputs, matched-inputs and matched-sources.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

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

* Re: Outreachy - Guix Data Service: implementing basic json output for derivation comparison page
  2021-04-17  8:40                 ` Christopher Baines
@ 2021-04-17 12:48                   ` Luciana Lima Brito
  2021-04-17 13:11                     ` Christopher Baines
  0 siblings, 1 reply; 22+ messages in thread
From: Luciana Lima Brito @ 2021-04-17 12:48 UTC (permalink / raw)
  To: Christopher Baines; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 773 bytes --]

On Sat, 17 Apr 2021 09:40:22 +0100
Christopher Baines <mail@cbaines.net> wrote:

> I think you're getting there, but it looks like you're close to what
> you want with matched-outputs say, and then later you pick bits out
> of that alist, generate vectors from the lists, and then rebuild the
> alist. I think you can remove all that complexity by just tweaking
> what you're doing up when you generate matched-outputs. I think this
> is true for matched-outputs, matched-inputs and matched-sources.

It's beautiful!
Now I could understand better the alist, and things make much more
sense to me. I simplified this part of getting properly the values from
matched-outputs, matched-inputs and matched-sources.

-- 
Best Regards,

Luciana Lima Brito
MSc. in Computer Science

[-- Attachment #2: 0001-Implement-basic-json-output-for-the-derivation-compa.patch --]
[-- Type: text/x-patch, Size: 6782 bytes --]

From 8d91269ead953c7d087242fbce5857af89af3025 Mon Sep 17 00:00:00 2001
From: Luciana Brito <lubrito@posteo.net>
Date: Sun, 11 Apr 2021 11:06:06 -0300
Subject: [PATCH] Implement basic json output for the derivation comparison
 page

---
 guix-data-service/web/compare/controller.scm | 114 ++++++++++++++++++-
 1 file changed, 111 insertions(+), 3 deletions(-)

diff --git a/guix-data-service/web/compare/controller.scm b/guix-data-service/web/compare/controller.scm
index a6aa198..d05c177 100644
--- a/guix-data-service/web/compare/controller.scm
+++ b/guix-data-service/web/compare/controller.scm
@@ -588,9 +588,117 @@
                  '(application/json text/html)
                  mime-types)
             ((application/json)
-             (render-json
-              '((error . "unimplemented")) ; TODO
-              #:extra-headers http-headers-for-unchanging-content))
+             (let* ((outputs (assq-ref data 'outputs))
+                    (matched-outputs
+                     (map
+                      (lambda (label items)
+                        (cons label
+                              (map
+                               (match-lambda
+                                 ((name path hash-alg hash recursive)
+                                  `(,@(if (null? name)
+                                          '()
+                                          `((name . ,name)))
+                                    ,@(if (null? path)
+                                          '()
+                                          `((path . ,path))
+                                          )
+                                    ,@(if (or (null? hash-alg) (not (string? hash-alg)))
+                                          '()
+                                          `((hash-algorithm . ,hash-alg))
+                                          )
+                                    ,@(if (or (null? hash) (not (string? hash)))
+                                          '()
+                                          `((hash . ,hash))
+                                          )
+                                    ,@(if (null? recursive)
+                                          '()
+                                          `((recursive . ,(string=? recursive "t")))))))
+                               (or items '()))))
+                      '(base target common)
+                      (list (assq-ref outputs 'base)
+                            (assq-ref outputs 'target)
+                            (assq-ref outputs 'common))))
+
+                    (inputs  (assq-ref data 'inputs))
+                    (matched-inputs
+                     (map
+                      (lambda (label items)
+                        (cons label
+                              (map 
+                               (match-lambda
+                                 ((derivation output)
+                                  `(,@(if (null? derivation)
+                                          '()
+                                          `((derivation . ,derivation)))
+                                    ,@(if (null? output)
+                                          '()
+                                          `((output . ,output))))))
+                               (or items '()))))
+                      '(base target common)
+                      (list (assq-ref inputs 'base)
+                            (assq-ref inputs 'target))))
+                    
+                    (sources (assq-ref data 'sources))
+                    (matched-sources
+                     (map
+                      (lambda (label items)
+                        (cons label
+                              (map
+                               (match-lambda
+                                 ((derivation)
+                                  `(,@(if (null? derivation)
+                                          '()
+                                          `((derivation . ,derivation))))))
+                               (or items '())))) 
+                      '(base target common)
+                      (list (assq-ref sources 'base)
+                            (assq-ref sources 'target)
+                            (assq-ref sources 'common))))
+                    
+                    (system  (assq-ref data 'system))
+                    (base-system (assq-ref system 'base))
+                    (target-system (assq-ref system 'target))
+                    (common-system (assq-ref system 'common))
+                    
+                    (builder (assq-ref data 'builder))
+                    (base-builder (assq-ref builder 'base))
+                    (target-builder (assq-ref builder 'target))
+                    (common-builder (assq-ref builder 'common))
+                    
+                    (args    (assq-ref data 'arguments))
+                    (base-args (assq-ref args 'base))
+                    (target-args (assq-ref args 'target))
+                    (common-args (assq-ref args 'common))
+                    (environment-variables (assq-ref data 'environment-variables)))
+               
+               (render-json
+                `((base
+                   . ((derivation . ,base-derivation)))
+                  (target
+                   . ((derivation . ,target-derivation)))
+                  (outputs
+                   . ((base . ,(list->vector (assq-ref matched-outputs 'base)))
+                      (target . ,(list->vector (assq-ref matched-outputs 'target)))
+                      (common . ,(list->vector (assq-ref matched-outputs 'common)))))
+                  (inputs
+                   . ((base . ,(list->vector (assq-ref matched-inputs 'base)))
+                      (target . ,(list->vector (assq-ref matched-inputs 'target)))))
+                  (sources                   
+                   . ((base . ,(list->vector (assq-ref matched-sources 'base)))
+                      (target . ,(list->vector (assq-ref matched-sources 'target)))
+                      (common . ,(list->vector (assq-ref matched-sources 'common)))))
+                  (system
+                   . ((common . ,common-system)))
+                  (builder-and-arguments
+                   . ((builder . ,common-builder)
+                      (arguments
+                       . ((base . ,(list->vector
+                                    base-args))
+                          (target . ,(list->vector
+                                      target-args))))))
+                  (environment-variables . ,environment-variables))
+                #:extra-headers http-headers-for-unchanging-content)))
             (else
              (render-html
               #:sxml (compare/derivation
-- 
2.30.2


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

* Re: Outreachy - Guix Data Service: implementing basic json output for derivation comparison page
  2021-04-17 12:48                   ` Luciana Lima Brito
@ 2021-04-17 13:11                     ` Christopher Baines
  2021-04-17 14:08                       ` Luciana Lima Brito
  0 siblings, 1 reply; 22+ messages in thread
From: Christopher Baines @ 2021-04-17 13:11 UTC (permalink / raw)
  To: Luciana Lima Brito; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 1279 bytes --]


Luciana Lima Brito <lubrito@posteo.net> writes:

> On Sat, 17 Apr 2021 09:40:22 +0100
> Christopher Baines <mail@cbaines.net> wrote:
>
>> I think you're getting there, but it looks like you're close to what
>> you want with matched-outputs say, and then later you pick bits out
>> of that alist, generate vectors from the lists, and then rebuild the
>> alist. I think you can remove all that complexity by just tweaking
>> what you're doing up when you generate matched-outputs. I think this
>> is true for matched-outputs, matched-inputs and matched-sources.
>
> It's beautiful!
> Now I could understand better the alist, and things make much more
> sense to me. I simplified this part of getting properly the values from
> matched-outputs, matched-inputs and matched-sources.

I think that's better, but you're still taking the matched-outputs,
matched-inputs and matched-sources alists, then later making the values
vectors rather than lists. The code would be even simpler if you
converted to a vector just after the map that produces the relevant
lists.

I haven't looked to closely at the rest, but it looks like the
environment-variables binding is unnecessary, given how simple the bit
of code used is, it can just be inlined in the one place where the
binding is used.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

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

* Re: Outreachy - Guix Data Service: implementing basic json output for derivation comparison page
  2021-04-17 13:11                     ` Christopher Baines
@ 2021-04-17 14:08                       ` Luciana Lima Brito
  2021-04-17 17:45                         ` Christopher Baines
  0 siblings, 1 reply; 22+ messages in thread
From: Luciana Lima Brito @ 2021-04-17 14:08 UTC (permalink / raw)
  To: Christopher Baines; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 739 bytes --]

On Sat, 17 Apr 2021 14:11:37 +0100
Christopher Baines <mail@cbaines.net> wrote:

Hi,

> I think that's better, but you're still taking the matched-outputs,
> matched-inputs and matched-sources alists, then later making the
> values vectors rather than lists. The code would be even simpler if
> you converted to a vector just after the map that produces the
> relevant lists.
> 
> I haven't looked to closely at the rest, but it looks like the
> environment-variables binding is unnecessary, given how simple the bit
> of code used is, it can just be inlined in the one place where the
> binding is used.

Done! :)
I even simplified some other stuff, based on what you said.

-- 
Best Regards,

Luciana Lima Brito
MSc. in Computer Science

[-- Attachment #2: 0001-Implement-basic-json-output-for-the-derivation-compa.patch --]
[-- Type: text/x-patch, Size: 5754 bytes --]

From 701c89e8f039a6bc7d9b616acded54eac26fb0a7 Mon Sep 17 00:00:00 2001
From: Luciana Brito <lubrito@posteo.net>
Date: Sun, 11 Apr 2021 11:06:06 -0300
Subject: [PATCH] Implement basic json output for the derivation comparison
 page

---
 guix-data-service/web/compare/controller.scm | 100 ++++++++++++++++++-
 1 file changed, 97 insertions(+), 3 deletions(-)

diff --git a/guix-data-service/web/compare/controller.scm b/guix-data-service/web/compare/controller.scm
index a6aa198..14a25ee 100644
--- a/guix-data-service/web/compare/controller.scm
+++ b/guix-data-service/web/compare/controller.scm
@@ -588,9 +588,103 @@
                  '(application/json text/html)
                  mime-types)
             ((application/json)
-             (render-json
-              '((error . "unimplemented")) ; TODO
-              #:extra-headers http-headers-for-unchanging-content))
+             (let* ((outputs (assq-ref data 'outputs))
+                    (matched-outputs
+                     (map
+                      (lambda (label items)
+                        (cons label
+                              (list->vector
+                               (map
+                                (match-lambda
+                                  ((name path hash-alg hash recursive)
+                                   `(,@(if (null? name)
+                                           '()
+                                           `((name . ,name)))
+                                     ,@(if (null? path)
+                                           '()
+                                           `((path . ,path))
+                                           )
+                                     ,@(if (or (null? hash-alg) (not (string? hash-alg)))
+                                           '()
+                                           `((hash-algorithm . ,hash-alg))
+                                           )
+                                     ,@(if (or (null? hash) (not (string? hash)))
+                                           '()
+                                           `((hash . ,hash))
+                                           )
+                                     ,@(if (null? recursive)
+                                           '()
+                                           `((recursive . ,(string=? recursive "t")))))))
+                                (or items '())))))
+                      '(base target common)
+                      (list (assq-ref outputs 'base)
+                            (assq-ref outputs 'target)
+                            (assq-ref outputs 'common))))
+
+                    (inputs  (assq-ref data 'inputs))
+                    (matched-inputs
+                     (map
+                      (lambda (label items)
+                        (cons label
+                              (list->vector
+                               (map 
+                                (match-lambda
+                                  ((derivation output)
+                                   `(,@(if (null? derivation)
+                                           '()
+                                           `((derivation . ,derivation)))
+                                     ,@(if (null? output)
+                                           '()
+                                           `((output . ,output))))))
+                                (or items '())))))
+                      '(base target common)
+                      (list (assq-ref inputs 'base)
+                            (assq-ref inputs 'target))))
+                    
+                    (sources (assq-ref data 'sources))
+                    (matched-sources
+                     (map
+                      (lambda (label items)
+                        (cons label
+                              (list->vector
+                               (map
+                                (match-lambda
+                                  ((derivation)
+                                   `(,@(if (null? derivation)
+                                           '()
+                                           `((derivation . ,derivation))))))
+                                (or items '())))))
+                      '(base target common)
+                      (list (assq-ref sources 'base)
+                            (assq-ref sources 'target)
+                            (assq-ref sources 'common))))
+                    
+                    (args    (assq-ref data 'arguments))
+                    (base-args (assq-ref args 'base))
+                    (target-args (assq-ref args 'target)))
+               
+               (render-json
+                `((base
+                   . ((derivation . ,base-derivation)))
+                  (target
+                   . ((derivation . ,target-derivation)))
+                  (outputs
+                   . ,matched-outputs)
+                  (inputs
+                   . ,matched-inputs)
+                  (sources                   
+                   . ,matched-sources)
+                  (system
+                   . ,(assq-ref data 'system))
+                  (builder-and-arguments
+                   . ((builder . ,(assq-ref data 'builder))
+                      (arguments
+                       . ((base . ,(list->vector
+                                    base-args))
+                          (target . ,(list->vector
+                                      target-args))))))
+                  (environment-variables . ,(assq-ref data 'environment-variables)))
+                #:extra-headers http-headers-for-unchanging-content)))
             (else
              (render-html
               #:sxml (compare/derivation
-- 
2.30.2


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

* Re: Outreachy - Guix Data Service: implementing basic json output for derivation comparison page
  2021-04-17 14:08                       ` Luciana Lima Brito
@ 2021-04-17 17:45                         ` Christopher Baines
  2021-04-18 13:12                           ` Luciana Lima Brito
  0 siblings, 1 reply; 22+ messages in thread
From: Christopher Baines @ 2021-04-17 17:45 UTC (permalink / raw)
  To: Luciana Lima Brito; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 1285 bytes --]


Luciana Lima Brito <lubrito@posteo.net> writes:

> On Sat, 17 Apr 2021 14:11:37 +0100
> Christopher Baines <mail@cbaines.net> wrote:
>
> Hi,
>
>> I think that's better, but you're still taking the matched-outputs,
>> matched-inputs and matched-sources alists, then later making the
>> values vectors rather than lists. The code would be even simpler if
>> you converted to a vector just after the map that produces the
>> relevant lists.
>>
>> I haven't looked to closely at the rest, but it looks like the
>> environment-variables binding is unnecessary, given how simple the bit
>> of code used is, it can just be inlined in the one place where the
>> binding is used.
>
> Done! :)
> I even simplified some other stuff, based on what you said.

Great, I think that looks much simpler.

Some more things to think about:

 - Variable naming, what does the "matched" in matched outputs mean?
   (same goes for the other "matched" things)

 - (if (null? ...), I'm unsure if all of those checks are necessary, I
   believe some fields at least will never be "null?".

 - Builder and arguments grouping, I think this makes sense on the HTML
   page, as they're connected, but does it make sense in the JSON?

I think you're getting close to something that's ready to merge though.

Chris

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

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

* Re: Outreachy - Guix Data Service: implementing basic json output for derivation comparison page
  2021-04-17 17:45                         ` Christopher Baines
@ 2021-04-18 13:12                           ` Luciana Lima Brito
  2021-04-18 13:19                             ` Luciana Lima Brito
  2021-04-18 16:34                             ` Christopher Baines
  0 siblings, 2 replies; 22+ messages in thread
From: Luciana Lima Brito @ 2021-04-18 13:12 UTC (permalink / raw)
  To: Christopher Baines; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 1193 bytes --]

Hi,

On Sat, 17 Apr 2021 18:45:14 +0100
Christopher Baines <mail@cbaines.net> wrote:
 
> Some more things to think about:
> 
>  - Variable naming, what does the "matched" in matched outputs mean?
>    (same goes for the other "matched" things)

The name matched would refer to the match function, but I changed to
*-values. The names I wanted were "outputs", "inputs"
and "sources", but I already used them. If you have anything in mind,
please let me know.

>  - (if (null? ...), I'm unsure if all of those checks are necessary, I
>    believe some fields at least will never be "null?".

I revised it, I think now it's better.
About the "recursive" field, apparently it assumes a string value "t"
or "f", and I convert this to a boolean. Are there other values
possible?

>  - Builder and arguments grouping, I think this makes sense on the
> HTML page, as they're connected, but does it make sense in the JSON?

indeed, I separated them.

> I think you're getting close to something that's ready to merge
> though.


One last thing, I see that on the html the commom inputs are ommited.
Does this make sense for the json too?

-- 
Best Regards,

Luciana Lima Brito
MSc. in Computer Science

[-- Attachment #2: 0001-Implement-basic-json-output-for-the-derivation-compa.patch --]
[-- Type: text/x-patch, Size: 4993 bytes --]

From 614ea09fc6a2d96a9dc2955ae08736740d08f8f8 Mon Sep 17 00:00:00 2001
From: Luciana Brito <lubrito@posteo.net>
Date: Sun, 11 Apr 2021 11:06:06 -0300
Subject: [PATCH] Implement basic json output for the derivation comparison
 page

---
 guix-data-service/web/compare/controller.scm | 89 +++++++++++++++++++-
 1 file changed, 86 insertions(+), 3 deletions(-)

diff --git a/guix-data-service/web/compare/controller.scm b/guix-data-service/web/compare/controller.scm
index a6aa198..09caa82 100644
--- a/guix-data-service/web/compare/controller.scm
+++ b/guix-data-service/web/compare/controller.scm
@@ -588,9 +588,92 @@
                  '(application/json text/html)
                  mime-types)
             ((application/json)
-             (render-json
-              '((error . "unimplemented")) ; TODO
-              #:extra-headers http-headers-for-unchanging-content))
+             (let* ((outputs (assq-ref data 'outputs))
+                    (output-values
+                     (map
+                      (lambda (label items)
+                        (cons label
+                              (list->vector
+                               (map
+                                (match-lambda
+                                  ((name path hash-alg hash recursive)
+                                   `((name . ,name)
+                                     (path . ,path)
+                                     ,@(if (not (string? hash-alg))
+                                           '()
+                                           `((hash-algorithm . ,hash-alg))
+                                           )
+                                     ,@(if (not (string? hash))
+                                           '()
+                                           `((hash . ,hash))
+                                           )
+                                     (recursive . ,(string=? recursive "t"))
+                                     )
+                                   ))
+                                (or items '())))))
+                      '(base target common)
+                      (list (assq-ref outputs 'base)
+                            (assq-ref outputs 'target)
+                            (assq-ref outputs 'common))))
+
+                    (inputs  (assq-ref data 'inputs))
+                    (input-values
+                     (map
+                      (lambda (label items)
+                        (cons label
+                              (list->vector
+                               (map 
+                                (match-lambda
+                                  ((derivation output)
+                                   `((derivation . ,derivation)
+                                     (output . ,output)
+                                     )))
+                                (or items '())))))
+                      '(base target common)
+                      (list (assq-ref inputs 'base)
+                            (assq-ref inputs 'target))))
+                    
+                    (sources (assq-ref data 'sources))
+                    (source-values
+                     (map
+                      (lambda (label items)
+                        (cons label
+                              (list->vector
+                               (map
+                                (match-lambda
+                                  ((derivation)
+                                   `((derivation . ,derivation)
+                                     )))
+                                (or items '())))))
+                      '(base target common)
+                      (list (assq-ref sources 'base)
+                            (assq-ref sources 'target)
+                            (assq-ref sources 'common))))
+                    
+                    (args    (assq-ref data 'arguments))
+                    (base-args (list->vector (assq-ref args 'base)))
+                    (target-args (list->vector (assq-ref args 'target))))
+               
+               (render-json
+                `((base
+                   . ((derivation . ,base-derivation)))
+                  (target
+                   . ((derivation . ,target-derivation)))
+                  (outputs
+                   . ,output-values)
+                  (inputs
+                   . ,input-values)
+                  (sources                   
+                   . ,source-values)
+                  (system
+                   . ,(assq-ref data 'system))
+                  (builder
+                   . ,(assq-ref data 'builder))
+                  (arguments
+                       . ((base . ,base-args)
+                          (target . ,target-args)))
+                  (environment-variables . ,(assq-ref data 'environment-variables)))
+                #:extra-headers http-headers-for-unchanging-content)))
             (else
              (render-html
               #:sxml (compare/derivation
-- 
2.30.2


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

* Re: Outreachy - Guix Data Service: implementing basic json output for derivation comparison page
  2021-04-18 13:12                           ` Luciana Lima Brito
@ 2021-04-18 13:19                             ` Luciana Lima Brito
  2021-04-18 16:34                             ` Christopher Baines
  1 sibling, 0 replies; 22+ messages in thread
From: Luciana Lima Brito @ 2021-04-18 13:19 UTC (permalink / raw)
  To: Christopher Baines; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 1438 bytes --]

On Sun, 18 Apr 2021 13:12:07 +0000
Luciana Lima Brito <lubrito@posteo.net> wrote:

> Hi,
> 
> On Sat, 17 Apr 2021 18:45:14 +0100
> Christopher Baines <mail@cbaines.net> wrote:
>  
> > Some more things to think about:
> > 
> >  - Variable naming, what does the "matched" in matched outputs mean?
> >    (same goes for the other "matched" things)  
> 
> The name matched would refer to the match function, but I changed to
> *-values. The names I wanted were "outputs", "inputs"
> and "sources", but I already used them. If you have anything in mind,
> please let me know.
> 
> >  - (if (null? ...), I'm unsure if all of those checks are
> > necessary, I believe some fields at least will never be "null?".  
> 
> I revised it, I think now it's better.
> About the "recursive" field, apparently it assumes a string value "t"
> or "f", and I convert this to a boolean. Are there other values
> possible?
> 
> >  - Builder and arguments grouping, I think this makes sense on the
> > HTML page, as they're connected, but does it make sense in the
> > JSON?  
> 
> indeed, I separated them.
> 
> > I think you're getting close to something that's ready to merge
> > though.  
> 
> 
> One last thing, I see that on the html the commom inputs are ommited.
> Does this make sense for the json too?
> 

The last patch had a few misaligned parens, please disregard. This one
is fixed.

-- 
Best Regards,

Luciana Lima Brito
MSc. in Computer Science

[-- Attachment #2: 0001-Implement-basic-json-output-for-the-derivation-compa.patch --]
[-- Type: text/x-patch, Size: 4741 bytes --]

From ff348cb5ce7db9ce9f08a6f0827356faa6465877 Mon Sep 17 00:00:00 2001
From: Luciana Brito <lubrito@posteo.net>
Date: Sun, 11 Apr 2021 11:06:06 -0300
Subject: [PATCH] Implement basic json output for the derivation comparison
 page

---
 guix-data-service/web/compare/controller.scm | 83 +++++++++++++++++++-
 1 file changed, 80 insertions(+), 3 deletions(-)

diff --git a/guix-data-service/web/compare/controller.scm b/guix-data-service/web/compare/controller.scm
index a6aa198..2ff7a40 100644
--- a/guix-data-service/web/compare/controller.scm
+++ b/guix-data-service/web/compare/controller.scm
@@ -588,9 +588,86 @@
                  '(application/json text/html)
                  mime-types)
             ((application/json)
-             (render-json
-              '((error . "unimplemented")) ; TODO
-              #:extra-headers http-headers-for-unchanging-content))
+             (let* ((outputs (assq-ref data 'outputs))
+                    (output-values
+                     (map
+                      (lambda (label items)
+                        (cons label
+                              (list->vector
+                               (map
+                                (match-lambda
+                                  ((name path hash-alg hash recursive)
+                                   `((name . ,name)
+                                     (path . ,path)
+                                     ,@(if (not (string? hash-alg))
+                                           '()
+                                           `((hash-algorithm . ,hash-alg)))
+                                     ,@(if (not (string? hash))
+                                           '()
+                                           `((hash . ,hash)))
+                                     (recursive . ,(string=? recursive "t")))))
+                                (or items '())))))
+                      '(base target common)
+                      (list (assq-ref outputs 'base)
+                            (assq-ref outputs 'target)
+                            (assq-ref outputs 'common))))
+
+                    (inputs  (assq-ref data 'inputs))
+                    (input-values
+                     (map
+                      (lambda (label items)
+                        (cons label
+                              (list->vector
+                               (map 
+                                (match-lambda
+                                  ((derivation output)
+                                   `((derivation . ,derivation)
+                                     (output . ,output))))
+                                (or items '())))))
+                      '(base target common)
+                      (list (assq-ref inputs 'base)
+                            (assq-ref inputs 'target))))
+                    
+                    (sources (assq-ref data 'sources))
+                    (source-values
+                     (map
+                      (lambda (label items)
+                        (cons label
+                              (list->vector
+                               (map
+                                (match-lambda
+                                  ((derivation)
+                                   `((derivation . ,derivation))))
+                                (or items '())))))
+                      '(base target common)
+                      (list (assq-ref sources 'base)
+                            (assq-ref sources 'target)
+                            (assq-ref sources 'common))))
+                    
+                    (args    (assq-ref data 'arguments))
+                    (base-args (list->vector (assq-ref args 'base)))
+                    (target-args (list->vector (assq-ref args 'target))))
+               
+               (render-json
+                `((base
+                   . ((derivation . ,base-derivation)))
+                  (target
+                   . ((derivation . ,target-derivation)))
+                  (outputs
+                   . ,output-values)
+                  (inputs
+                   . ,input-values)
+                  (sources                   
+                   . ,source-values)
+                  (system
+                   . ,(assq-ref data 'system))
+                  (builder
+                   . ,(assq-ref data 'builder))
+                  (arguments
+                   . ((base . ,base-args)
+                      (target . ,target-args)))
+                  (environment-variables . ,(assq-ref data 'environment-variables)))
+                #:extra-headers http-headers-for-unchanging-content)))
             (else
              (render-html
               #:sxml (compare/derivation
-- 
2.30.2


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

* Re: Outreachy - Guix Data Service: implementing basic json output for derivation comparison page
  2021-04-18 13:12                           ` Luciana Lima Brito
  2021-04-18 13:19                             ` Luciana Lima Brito
@ 2021-04-18 16:34                             ` Christopher Baines
  2021-04-18 19:12                               ` Luciana Lima Brito
  1 sibling, 1 reply; 22+ messages in thread
From: Christopher Baines @ 2021-04-18 16:34 UTC (permalink / raw)
  To: Luciana Lima Brito; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 1933 bytes --]


Luciana Lima Brito <lubrito@posteo.net> writes:

> Hi,
>
> On Sat, 17 Apr 2021 18:45:14 +0100
> Christopher Baines <mail@cbaines.net> wrote:
>
>> Some more things to think about:
>>
>>  - Variable naming, what does the "matched" in matched outputs mean?
>>    (same goes for the other "matched" things)
>
> The name matched would refer to the match function, but I changed to
> *-values. The names I wanted were "outputs", "inputs"
> and "sources", but I already used them. If you have anything in mind,
> please let me know.

I think it might be good to do something, just to narrow the scope. The
outputs binding is valid for the whole let*, and all the code in it, but
is only used on three lines, in one single place. Maybe there could be a
let there that just defines outputs (maybe named output-groups so you
can use the outputs binding for the overall thing).

>>  - (if (null? ...), I'm unsure if all of those checks are necessary, I
>>    believe some fields at least will never be "null?".
>
> I revised it, I think now it's better.
> About the "recursive" field, apparently it assumes a string value "t"
> or "f", and I convert this to a boolean. Are there other values
> possible?

That's a good question, I'd look at the database schema, assuming the
type of the field is a boolean, the question is whether the field is
nullable?

>> I think you're getting close to something that's ready to merge
>> though.
>
> One last thing, I see that on the html the commom inputs are ommited.
> Does this make sense for the json too?

Hmm, I'm not sure why that is on the HTML page, but I'd generally try
and keep most bits in the JSON, since it's not as helpful to omit bits
if they're not that important.

One other thing I noticed is that the alist for the args is being picked
apart then reconstructed. Like for the inputs, outputs and sources, I'd
map over the arguments alist and transform it to the way you want it to
be.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

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

* Re: Outreachy - Guix Data Service: implementing basic json output for derivation comparison page
  2021-04-18 16:34                             ` Christopher Baines
@ 2021-04-18 19:12                               ` Luciana Lima Brito
  2021-04-19  8:26                                 ` Christopher Baines
  0 siblings, 1 reply; 22+ messages in thread
From: Luciana Lima Brito @ 2021-04-18 19:12 UTC (permalink / raw)
  To: Christopher Baines; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 1464 bytes --]

Hi,

On Sun, 18 Apr 2021 17:34:13 +0100
Christopher Baines <mail@cbaines.net> wrote:
 
> I think it might be good to do something, just to narrow the scope.
> The outputs binding is valid for the whole let*, and all the code in
> it, but is only used on three lines, in one single place. Maybe there
> could be a let there that just defines outputs (maybe named
> output-groups so you can use the outputs binding for the overall
> thing).

I did it.
 
> That's a good question, I'd look at the database schema, assuming the
> type of the field is a boolean, the question is whether the field is
> nullable?

I looked on the database schema, and the "recursive" field is not
nullable, and it is a boolean, so the test I'm doing is working for
this.
 
> Hmm, I'm not sure why that is on the HTML page, but I'd generally try
> and keep most bits in the JSON, since it's not as helpful to omit bits
> if they're not that important.

I added the "common" field for inputs.
> 
> One other thing I noticed is that the alist for the args is being
> picked apart then reconstructed. Like for the inputs, outputs and
> sources, I'd map over the arguments alist and transform it to the way
> you want it to be.

This part was a bit more complicated for me to understand. You mean I
should build a function similar to outputs, inputs and sources to map
the arguments, wouldn't it be a lot just to show a vector?

-- 
Best Regards,

Luciana Lima Brito
MSc. in Computer Science

[-- Attachment #2: 0001-Implement-basic-json-output-for-the-derivation-compa.patch --]
[-- Type: text/x-patch, Size: 4839 bytes --]

From dc74d1a8f8f5e7527cdb63b66e8e2b937e614f32 Mon Sep 17 00:00:00 2001
From: Luciana Brito <lubrito@posteo.net>
Date: Sun, 11 Apr 2021 11:06:06 -0300
Subject: [PATCH] Implement basic json output for the derivation comparison
 page

---
 guix-data-service/web/compare/controller.scm | 83 +++++++++++++++++++-
 1 file changed, 80 insertions(+), 3 deletions(-)

diff --git a/guix-data-service/web/compare/controller.scm b/guix-data-service/web/compare/controller.scm
index a6aa198..ab5f9c4 100644
--- a/guix-data-service/web/compare/controller.scm
+++ b/guix-data-service/web/compare/controller.scm
@@ -588,9 +588,86 @@
                  '(application/json text/html)
                  mime-types)
             ((application/json)
-             (render-json
-              '((error . "unimplemented")) ; TODO
-              #:extra-headers http-headers-for-unchanging-content))
+             (let* ((outputs
+                     (map
+                      (lambda (label items)
+                        (cons label
+                              (list->vector
+                               (map
+                                (match-lambda
+                                  ((name path hash-alg hash recursive)
+                                   `((name . ,name)
+                                     (path . ,path)
+                                     ,@(if (not (string? hash-alg))
+                                           '()
+                                           `((hash-algorithm . ,hash-alg)))
+                                     ,@(if (not (string? hash))
+                                           '()
+                                           `((hash . ,hash)))
+                                     (recursive . ,(string=? recursive "t")))))
+                                (or items '())))))
+                      '(base target common)
+                      (let ((output-groups (assq-ref data 'outputs)))
+                        (list (assq-ref output-groups 'base)
+                              (assq-ref output-groups 'target)
+                              (assq-ref output-groups 'common)))))
+
+                    (inputs
+                     (map
+                      (lambda (label items)
+                        (cons label
+                              (list->vector
+                               (map 
+                                (match-lambda
+                                  ((derivation output)
+                                   `((derivation . ,derivation)
+                                     (output . ,output))))
+                                (or items '())))))
+                      '(base target common)
+                      (let ((input-groups (assq-ref data 'inputs)))
+                        (list (assq-ref input-groups 'base)
+                              (assq-ref input-groups 'target)
+                              (assq-ref input-groups 'common)))))
+
+                    (sources
+                     (map
+                      (lambda (label items)
+                        (cons label
+                              (list->vector
+                               (map
+                                (match-lambda
+                                  ((derivation)
+                                   `((derivation . ,derivation))))
+                                (or items '())))))
+                      '(base target common)
+                      (let ((source-groups (assq-ref data 'sources)))
+                        (list (assq-ref source-groups 'base)
+                              (assq-ref source-groups 'target)
+                              (assq-ref source-groups 'common)))))
+                    
+                    (args (assq-ref data 'arguments))
+                    (base-args (list->vector (assq-ref args 'base)))
+                    (target-args (list->vector (assq-ref args 'target))))
+               (render-json
+                `((base
+                   . ((derivation . ,base-derivation)))
+                  (target
+                   . ((derivation . ,target-derivation)))
+                  (outputs
+                   . ,outputs)
+                  (inputs
+                   . ,inputs)
+                  (sources                   
+                   . ,sources)
+                  (system
+                   . ,(assq-ref data 'system))
+                  (builder
+                   . ,(assq-ref data 'builder))
+                  (arguments
+                   . ((base . ,base-args)
+                      (target . ,target-args)))
+                  (environment-variables . ,(assq-ref data 'environment-variables)))
+                #:extra-headers http-headers-for-unchanging-content)))
             (else
              (render-html
               #:sxml (compare/derivation
-- 
2.30.2


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

* Re: Outreachy - Guix Data Service: implementing basic json output for derivation comparison page
  2021-04-18 19:12                               ` Luciana Lima Brito
@ 2021-04-19  8:26                                 ` Christopher Baines
  2021-04-19 14:04                                   ` Luciana Lima Brito
  0 siblings, 1 reply; 22+ messages in thread
From: Christopher Baines @ 2021-04-19  8:26 UTC (permalink / raw)
  To: Luciana Lima Brito; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 2125 bytes --]


Luciana Lima Brito <lubrito@posteo.net> writes:

> Hi,
>
> On Sun, 18 Apr 2021 17:34:13 +0100
> Christopher Baines <mail@cbaines.net> wrote:
>
>> I think it might be good to do something, just to narrow the scope.
>> The outputs binding is valid for the whole let*, and all the code in
>> it, but is only used on three lines, in one single place. Maybe there
>> could be a let there that just defines outputs (maybe named
>> output-groups so you can use the outputs binding for the overall
>> thing).
>
> I did it.

Great :)

>> That's a good question, I'd look at the database schema, assuming the
>> type of the field is a boolean, the question is whether the field is
>> nullable?
>
> I looked on the database schema, and the "recursive" field is not
> nullable, and it is a boolean, so the test I'm doing is working for
> this.

Cool.

>> Hmm, I'm not sure why that is on the HTML page, but I'd generally try
>> and keep most bits in the JSON, since it's not as helpful to omit bits
>> if they're not that important.
>
> I added the "common" field for inputs.

Ok.

>> One other thing I noticed is that the alist for the args is being
>> picked apart then reconstructed. Like for the inputs, outputs and
>> sources, I'd map over the arguments alist and transform it to the way
>> you want it to be.
>
> This part was a bit more complicated for me to understand. You mean I
> should build a function similar to outputs, inputs and sources to map
> the arguments, wouldn't it be a lot just to show a vector?

As an example, if you have this structure.

  '((foo . 1)
    (bar . 2))

and you want the numbers to be strings, you can do something like:

  (map (match-lambda
        ((name . number)
         (cons name (number->string number))))
       data)

and I think a similar approach for the transformation you're trying to
perform to the arguments will more cleanly represent what you're trying
to do.

One different thing I noticed:

  ,@(if (not (string? hash-alg))
        '()
        `((hash-algorithm . ,hash-alg)))

I'd suggest simplifying this by flipping the different parts of the if,
and removing the not.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

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

* Re: Outreachy - Guix Data Service: implementing basic json output for derivation comparison page
  2021-04-19  8:26                                 ` Christopher Baines
@ 2021-04-19 14:04                                   ` Luciana Lima Brito
  2021-04-19 20:20                                     ` Christopher Baines
  0 siblings, 1 reply; 22+ messages in thread
From: Luciana Lima Brito @ 2021-04-19 14:04 UTC (permalink / raw)
  To: Christopher Baines; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 829 bytes --]

On Mon, 19 Apr 2021 09:26:13 +0100
Christopher Baines <mail@cbaines.net> wrote:

> As an example, if you have this structure.
> 
>   '((foo . 1)
>     (bar . 2))
> 
> and you want the numbers to be strings, you can do something like:
> 
>   (map (match-lambda
>         ((name . number)
>          (cons name (number->string number))))
>        data)
> 
> and I think a similar approach for the transformation you're trying to
> perform to the arguments will more cleanly represent what you're
> trying to do.

It helped a lot!

> One different thing I noticed:
> 
>   ,@(if (not (string? hash-alg))
>         '()
>         `((hash-algorithm . ,hash-alg)))
> 
> I'd suggest simplifying this by flipping the different parts of the
> if, and removing the not.

Done.

-- 
Best Regards,

Luciana Lima Brito
MSc. in Computer Science

[-- Attachment #2: 0001-Implement-basic-json-output-for-the-derivation-compa.patch --]
[-- Type: text/x-patch, Size: 4739 bytes --]

From ce5d97dd490f6ffb58548e83be78af576404b01f Mon Sep 17 00:00:00 2001
From: Luciana Brito <lubrito@posteo.net>
Date: Sun, 11 Apr 2021 11:06:06 -0300
Subject: [PATCH] Implement basic json output for the derivation comparison
 page

---
 guix-data-service/web/compare/controller.scm | 85 +++++++++++++++++++-
 1 file changed, 82 insertions(+), 3 deletions(-)

diff --git a/guix-data-service/web/compare/controller.scm b/guix-data-service/web/compare/controller.scm
index a6aa198..85fcbe9 100644
--- a/guix-data-service/web/compare/controller.scm
+++ b/guix-data-service/web/compare/controller.scm
@@ -588,9 +588,88 @@
                  '(application/json text/html)
                  mime-types)
             ((application/json)
-             (render-json
-              '((error . "unimplemented")) ; TODO
-              #:extra-headers http-headers-for-unchanging-content))
+             (let ((outputs
+                    (map
+                     (lambda (label items)
+                       (cons label
+                             (list->vector
+                              (map
+                               (match-lambda
+                                 ((name path hash-alg hash recursive)
+                                  `((name . ,name)
+                                    (path . ,path)
+                                    ,@(if (string? hash-alg)
+                                          `((hash-algorithm . ,hash-alg))
+                                          '())
+                                    ,@(if (string? hash)
+                                          `((hash . ,hash))
+                                          '())
+                                    (recursive . ,(string=? recursive "t")))))
+                               (or items '())))))
+                     '(base target common)
+                     (let ((output-groups (assq-ref data 'outputs)))
+                       (list (assq-ref output-groups 'base)
+                             (assq-ref output-groups 'target)
+                             (assq-ref output-groups 'common)))))
+
+                   (inputs
+                    (map
+                     (lambda (label items)
+                       (cons label
+                             (list->vector
+                              (map 
+                               (match-lambda
+                                 ((derivation output)
+                                  `((derivation . ,derivation)
+                                    (output . ,output))))
+                               (or items '())))))
+                     '(base target common)
+                     (let ((input-groups (assq-ref data 'inputs)))
+                       (list (assq-ref input-groups 'base)
+                             (assq-ref input-groups 'target)
+                             (assq-ref input-groups 'common)))))
+
+                   (sources
+                    (map
+                     (lambda (label items)
+                       (cons label
+                             (list->vector
+                              (map
+                               (match-lambda
+                                 ((derivation)
+                                  `((derivation . ,derivation))))
+                               (or items '())))))
+                     '(base target common)
+                     (let ((source-groups (assq-ref data 'sources)))
+                       (list (assq-ref source-groups 'base)
+                             (assq-ref source-groups 'target)
+                             (assq-ref source-groups 'common)))))
+
+                   (arguments
+                    (map
+                     (match-lambda
+                       ((label args ...)
+                        `(,label . ,(list->vector args))))
+                     (assq-ref data 'arguments))))
+               
+               (render-json
+                `((base
+                   . ((derivation . ,base-derivation)))
+                  (target
+                   . ((derivation . ,target-derivation)))
+                  (outputs
+                   . ,outputs)
+                  (inputs
+                   . ,inputs)
+                  (sources                   
+                   . ,sources)
+                  (system
+                   . ,(assq-ref data 'system))
+                  (builder
+                   . ,(assq-ref data 'builder))
+                  (arguments . ,arguments)
+                  (environment-variables . ,(assq-ref data 'environment-variables)))
+                #:extra-headers http-headers-for-unchanging-content)))
             (else
              (render-html
               #:sxml (compare/derivation
-- 
2.30.2


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

* Re: Outreachy - Guix Data Service: implementing basic json output for derivation comparison page
  2021-04-19 14:04                                   ` Luciana Lima Brito
@ 2021-04-19 20:20                                     ` Christopher Baines
  2021-04-19 20:56                                       ` Luciana Lima Brito
  0 siblings, 1 reply; 22+ messages in thread
From: Christopher Baines @ 2021-04-19 20:20 UTC (permalink / raw)
  To: Luciana Lima Brito; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 3653 bytes --]


Luciana Lima Brito <lubrito@posteo.net> writes:

> On Mon, 19 Apr 2021 09:26:13 +0100
> Christopher Baines <mail@cbaines.net> wrote:
>
>> As an example, if you have this structure.
>>
>>   '((foo . 1)
>>     (bar . 2))
>>
>> and you want the numbers to be strings, you can do something like:
>>
>>   (map (match-lambda
>>         ((name . number)
>>          (cons name (number->string number))))
>>        data)
>>
>> and I think a similar approach for the transformation you're trying to
>> perform to the arguments will more cleanly represent what you're
>> trying to do.
>
> It helped a lot!
>
>> One different thing I noticed:
>>
>>   ,@(if (not (string? hash-alg))
>>         '()
>>         `((hash-algorithm . ,hash-alg)))
>>
>> I'd suggest simplifying this by flipping the different parts of the
>> if, and removing the not.
>
> Done.

Great, I've gone ahead and pushed this now.

I tweaked the following things, it's mostly removing trailing
whitespace, and I changed the way the alist was formatted, just so it's
a bit more compact (how it was before is OK as well).

@@ -617,7 +617,7 @@
                      (lambda (label items)
                        (cons label
                              (list->vector
-                              (map 
+                              (map
                                (match-lambda
                                  ((derivation output)
                                   `((derivation . ,derivation)
@@ -651,24 +651,18 @@
                        ((label args ...)
                         `(,label . ,(list->vector args))))
                      (assq-ref data 'arguments))))
-               
+
                (render-json
-                `((base
-                   . ((derivation . ,base-derivation)))
-                  (target
-                   . ((derivation . ,target-derivation)))
-                  (outputs
-                   . ,outputs)
-                  (inputs
-                   . ,inputs)
-                  (sources                   
-                   . ,sources)
-                  (system
-                   . ,(assq-ref data 'system))
-                  (builder
-                   . ,(assq-ref data 'builder))
-                  (arguments . ,arguments)
-                  (environment-variables . ,(assq-ref data 'environment-variables)))
+                `((base                  . ((derivation . ,base-derivation)))
+                  (target                . ((derivation . ,target-derivation)))
+                  (outputs               . ,outputs)
+                  (inputs                . ,inputs)
+                  (sources               . ,sources)
+                  (system                . ,(assq-ref data 'system))
+                  (builder               . ,(assq-ref data 'builder))
+                  (arguments             . ,arguments)
+                  (environment-variables . ,(assq-ref
+                                             data 'environment-variables)))
                 #:extra-headers http-headers-for-unchanging-content)))
             (else
              (render-html



In terms of what to do next, you could continue on this derivation
comparison path. Some of the code you've got here could be used to make
the data better right when the database is queried. Take the recursive
field for outputs for example, it would be better to convert it to a
boolean where the database query is made.

Also, there's some admin to do in terms of recording a contribution, and
starting to think about submitting the final application.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

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

* Re: Outreachy - Guix Data Service: implementing basic json output for derivation comparison page
  2021-04-19 20:20                                     ` Christopher Baines
@ 2021-04-19 20:56                                       ` Luciana Lima Brito
  0 siblings, 0 replies; 22+ messages in thread
From: Luciana Lima Brito @ 2021-04-19 20:56 UTC (permalink / raw)
  To: Christopher Baines; +Cc: guix-devel

Hi

On Mon, 19 Apr 2021 21:20:18 +0100
Christopher Baines <mail@cbaines.net> wrote:


> Great, I've gone ahead and pushed this now.

What a great news!!!

> In terms of what to do next, you could continue on this derivation
> comparison path. Some of the code you've got here could be used to
> make the data better right when the database is queried. Take the
> recursive field for outputs for example, it would be better to
> convert it to a boolean where the database query is made.

Yes, I could do that. I will look into it.

> Also, there's some admin to do in terms of recording a contribution,
> and starting to think about submitting the final application.

I think I have to do something about that too.

-- 
Best Regards,

Luciana Lima Brito
MSc. in Computer Science


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

end of thread, other threads:[~2021-04-19 20:57 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-14 19:48 Outreachy - Guix Data Service: implementing basic json output for derivation comparison page Luciana Lima Brito
2021-04-15  8:46 ` Christopher Baines
2021-04-15 16:09   ` Luciana Lima Brito
2021-04-15 23:19     ` Christopher Baines
2021-04-16 15:07       ` Luciana Lima Brito
2021-04-16 15:47         ` Christopher Baines
2021-04-16 18:46           ` Luciana Lima Brito
2021-04-16 19:17             ` Christopher Baines
2021-04-16 22:47               ` Luciana Lima Brito
2021-04-17  8:40                 ` Christopher Baines
2021-04-17 12:48                   ` Luciana Lima Brito
2021-04-17 13:11                     ` Christopher Baines
2021-04-17 14:08                       ` Luciana Lima Brito
2021-04-17 17:45                         ` Christopher Baines
2021-04-18 13:12                           ` Luciana Lima Brito
2021-04-18 13:19                             ` Luciana Lima Brito
2021-04-18 16:34                             ` Christopher Baines
2021-04-18 19:12                               ` Luciana Lima Brito
2021-04-19  8:26                                 ` Christopher Baines
2021-04-19 14:04                                   ` Luciana Lima Brito
2021-04-19 20:20                                     ` Christopher Baines
2021-04-19 20:56                                       ` Luciana Lima Brito

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.