unofficial mirror of bug-guix@gnu.org 
 help / color / mirror / code / Atom feed
* bug#61684: can't compose 'with-patch' with 'with-source'
@ 2023-02-21 18:08 Maxim Cournoyer
  2023-02-21 19:05 ` Simon Tournier
  2023-02-23 14:08 ` Ludovic Courtès
  0 siblings, 2 replies; 17+ messages in thread
From: Maxim Cournoyer @ 2023-02-21 18:08 UTC (permalink / raw)
  To: 61684

Hi,

Given 'with-source' discards any patch from the original source, I thought
I could at least add them back via 'with-patch', but it appears this
does not work:

--8<---------------cut here---------------start------------->8---
scheme@(gnu packages jami)> (options->transformation 
                             `((with-source . "libjami@20230220.0=/home/maxim/src/jami/jami-20230220.0.tar.gz")
                               (with-patch . ,(string-append 
                                               "libjami=" (search-patch 
                                                           "jami-disable-integration-tests.patch")))))
$6 = #<procedure 7f2cd01a97e0 at guix/transformations.scm:1010:2 (obj)>
scheme@(gnu packages jami)> ($6 libjami)
$7 = #<package libjami@20230220.0 guix/transformations.scm:1002 7f2ccc8386e0>
scheme@(gnu packages jami)> (package-source $7)
$8 = #<<downloaded-file> uri: "/home/maxim/src/jami/jami-20230220.0.tar.gz" recursive?: #t>
scheme@(gnu packages jami)>
--8<---------------cut here---------------end--------------->8---

The downloaded-file resulting package source has lost the patch, and no
error got produced, leaving the user to discover this limitation by
themselves.

-- 
Thanks,
Maxim




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

* bug#61684: can't compose 'with-patch' with 'with-source'
  2023-02-21 18:08 bug#61684: can't compose 'with-patch' with 'with-source' Maxim Cournoyer
@ 2023-02-21 19:05 ` Simon Tournier
  2023-02-23 14:08 ` Ludovic Courtès
  1 sibling, 0 replies; 17+ messages in thread
From: Simon Tournier @ 2023-02-21 19:05 UTC (permalink / raw)
  To: Maxim Cournoyer, 61684

Hi Maxim,

On Tue, 21 Feb 2023 at 13:08, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:

> Given 'with-source' discards any patch from the original source, I thought
> I could at least add them back via 'with-patch', but it appears this
> does not work:

I remember some headaches with this thread [1]. :-)

1: <https://yhetil.org/guix/8635qeaegx.fsf@gmail.com>

>
> --8<---------------cut here---------------start------------->8---
> scheme@(gnu packages jami)> (options->transformation 
>                              `((with-source . "libjami@20230220.0=/home/maxim/src/jami/jami-20230220.0.tar.gz")
>                                (with-patch . ,(string-append 
>                                                "libjami=" (search-patch 
>                                                            "jami-disable-integration-tests.patch")))))
> $6 = #<procedure 7f2cd01a97e0 at guix/transformations.scm:1010:2 (obj)>
> scheme@(gnu packages jami)> ($6 libjami)
> $7 = #<package libjami@20230220.0 guix/transformations.scm:1002 7f2ccc8386e0>
> scheme@(gnu packages jami)> (package-source $7)
> $8 = #<<downloaded-file> uri: "/home/maxim/src/jami/jami-20230220.0.tar.gz" recursive?: #t>
> scheme@(gnu packages jami)>
> --8<---------------cut here---------------end--------------->8---
>
> The downloaded-file resulting package source has lost the patch, and no
> error got produced, leaving the user to discover this limitation by
> themselves.

Well, it is probably unrelated because I guess the transformation makes
sense here but indeed you can have bad surprise if the transformation
does not make sense and then silently ignored.

Well, I have never finished my attempt to raise more information about
the transformation because the code about the transformation is hard to
follow, from my point of view.  Anyway!

Thanks for having open this ticket. :-)  Maybe with-source and
with-patch are the first incremental change. ;-)

Cheers,
simon







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

* bug#61684: can't compose 'with-patch' with 'with-source'
  2023-02-21 18:08 bug#61684: can't compose 'with-patch' with 'with-source' Maxim Cournoyer
  2023-02-21 19:05 ` Simon Tournier
@ 2023-02-23 14:08 ` Ludovic Courtès
  2023-02-23 22:27   ` Maxim Cournoyer
  2023-02-24 13:59   ` Maxim Cournoyer
  1 sibling, 2 replies; 17+ messages in thread
From: Ludovic Courtès @ 2023-02-23 14:08 UTC (permalink / raw)
  To: Maxim Cournoyer; +Cc: 61684

Hi Maxim,

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

> Given 'with-source' discards any patch from the original source, I thought
> I could at least add them back via 'with-patch', but it appears this
> does not work:
>
> scheme@(gnu packages jami)> (options->transformation 
>                              `((with-source . "libjami@20230220.0=/home/maxim/src/jami/jami-20230220.0.tar.gz")
>                                (with-patch . ,(string-append 
>                                                "libjami=" (search-patch 
>                                                            "jami-disable-integration-tests.patch")))))
> $6 = #<procedure 7f2cd01a97e0 at guix/transformations.scm:1010:2 (obj)>
> scheme@(gnu packages jami)> ($6 libjami)
> $7 = #<package libjami@20230220.0 guix/transformations.scm:1002 7f2ccc8386e0>
> scheme@(gnu packages jami)> (package-source $7)
> $8 = #<<downloaded-file> uri: "/home/maxim/src/jami/jami-20230220.0.tar.gz" recursive?: #t>
> scheme@(gnu packages jami)>
>
> The downloaded-file resulting package source has lost the patch, and no
> error got produced, leaving the user to discover this limitation by
> themselves.

The order of options matters; in this case, you need to do it the other
way around:

--8<---------------cut here---------------start------------->8---
scheme@(guile-user)> (options->transformation '((with-patch . "jami=/tmp/t.patch")
						(with-source . "jami=http://example.org/foo.tar.gz")))
$18 = #<procedure 7f7e6b1fd0c0 at guix/transformations.scm:1010:2 (obj)>
scheme@(guile-user)> ($18 jami)
$19 = #<package jami@20230206.0 guix/transformations.scm:1002 7f7e6b1602c0>
scheme@(guile-user)> (package-source $19)
$20 = #<<computed-file> name: "jami-20230206.0-source" gexp: #<gexp  guix/transformations.scm:688:19 7f7e6b6c8d50> guile: #f options: (#:local-build? #t)>
--8<---------------cut here---------------end--------------->8---

The <computed-file> bit comes from the ‘with-patch’ transformation, to
apply the patch to the <downloaded-file>.

If you do it the other way around, the effect of ‘with-patch’ is
canceled by that of ‘with-source’, which does not preserve patches.  So
strictly speaking, both options had an effect, but they were
contradictory.

Note that ordering is “specified”, notably with the test added in
0f024554e63a49e20c2a7a67e928073c266bf5c5 (this is crucial for our HPC
users, who routinely combine a whole bunch of options; you have no idea
how far they go once you give them the tool :-)).  I don’t think the
manual explicitly states it though.

HTH,
Ludo’.




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

* bug#61684: can't compose 'with-patch' with 'with-source'
  2023-02-23 14:08 ` Ludovic Courtès
@ 2023-02-23 22:27   ` Maxim Cournoyer
  2023-02-24 12:02     ` Simon Tournier
  2023-02-24 13:59   ` Maxim Cournoyer
  1 sibling, 1 reply; 17+ messages in thread
From: Maxim Cournoyer @ 2023-02-23 22:27 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 61684

Hi Ludovic,

Ludovic Courtès <ludo@gnu.org> writes:

> Hi Maxim,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>> Given 'with-source' discards any patch from the original source, I thought
>> I could at least add them back via 'with-patch', but it appears this
>> does not work:
>>
>> scheme@(gnu packages jami)> (options->transformation 
>>                              `((with-source . "libjami@20230220.0=/home/maxim/src/jami/jami-20230220.0.tar.gz")
>>                                (with-patch . ,(string-append 
>>                                                "libjami=" (search-patch 
>>                                                            "jami-disable-integration-tests.patch")))))
>> $6 = #<procedure 7f2cd01a97e0 at guix/transformations.scm:1010:2 (obj)>
>> scheme@(gnu packages jami)> ($6 libjami)
>> $7 = #<package libjami@20230220.0 guix/transformations.scm:1002 7f2ccc8386e0>
>> scheme@(gnu packages jami)> (package-source $7)
>> $8 = #<<downloaded-file> uri: "/home/maxim/src/jami/jami-20230220.0.tar.gz" recursive?: #t>
>> scheme@(gnu packages jami)>
>>
>> The downloaded-file resulting package source has lost the patch, and no
>> error got produced, leaving the user to discover this limitation by
>> themselves.
>
> The order of options matters; in this case, you need to do it the other
> way around:
>
> scheme@(guile-user)> (options->transformation '((with-patch . "jami=/tmp/t.patch")
> 						(with-source . "jami=http://example.org/foo.tar.gz")))
> $18 = #<procedure 7f7e6b1fd0c0 at guix/transformations.scm:1010:2 (obj)>
> scheme@(guile-user)> ($18 jami)
> $19 = #<package jami@20230206.0 guix/transformations.scm:1002 7f7e6b1602c0>
> scheme@(guile-user)> (package-source $19)
> $20 = #<<computed-file> name: "jami-20230206.0-source" gexp: #<gexp  guix/transformations.scm:688:19 7f7e6b6c8d50> guile: #f options: (#:local-build? #t)>
>
> The <computed-file> bit comes from the ‘with-patch’ transformation, to
> apply the patch to the <downloaded-file>.
>
> If you do it the other way around, the effect of ‘with-patch’ is
> canceled by that of ‘with-source’, which does not preserve patches.  So
> strictly speaking, both options had an effect, but they were
> contradictory.

Hm.  That seems sub-optimal; it seems to me that ideally, the
transformations would be additive, so that users would not need to care
about the ordering.  Or perhaps, alternatively, we could enforce such
ordering at the implementation level (sorting the transformations in the
order that is required).

> Note that ordering is “specified”, notably with the test added in
> 0f024554e63a49e20c2a7a67e928073c266bf5c5 (this is crucial for our HPC
> users, who routinely combine a whole bunch of options; you have no idea
> how far they go once you give them the tool :-)).  I don’t think the
> manual explicitly states it though.

It doesn't seem to be, indeed.  But I wish we wouldn't need to (see my
above suggestions/ideas).

-- 
Thanks,
Maxim




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

* bug#61684: can't compose 'with-patch' with 'with-source'
  2023-02-23 22:27   ` Maxim Cournoyer
@ 2023-02-24 12:02     ` Simon Tournier
  2023-02-24 13:21       ` Maxim Cournoyer
  2023-02-27 14:09       ` Ludovic Courtès
  0 siblings, 2 replies; 17+ messages in thread
From: Simon Tournier @ 2023-02-24 12:02 UTC (permalink / raw)
  To: Maxim Cournoyer, Ludovic Courtès; +Cc: 61684

Hi,

On jeu., 23 févr. 2023 at 17:27, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:

> Hm.  That seems sub-optimal; it seems to me that ideally, the
> transformations would be additive, so that users would not need to care
> about the ordering.  Or perhaps, alternatively, we could enforce such
> ordering at the implementation level (sorting the transformations in the
> order that is required).

From my point of view (and what I tried stopping in the middle :-)) is
to report if the transformation makes sense or not.  For instance,

    with-patch
    with-source

makes sense contrary to

    with-source
    with-patch

and it would already be an improvement to report that the latter
transformation does not make sense instead of silently does nothing or
raises some weird errors.

Well, I am not convinced that enforce the ordering is a good thing
because as Ludo said, some HPC user exploits this control of ordering to
generate complex transformations.

To me, the fix is:

 1. document the ordering bits
 2. check if the ordering “makes sense“ and raises if not.


Cheers,
simon




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

* bug#61684: can't compose 'with-patch' with 'with-source'
  2023-02-24 12:02     ` Simon Tournier
@ 2023-02-24 13:21       ` Maxim Cournoyer
  2023-02-24 13:43         ` Simon Tournier
  2023-02-27 14:09       ` Ludovic Courtès
  1 sibling, 1 reply; 17+ messages in thread
From: Maxim Cournoyer @ 2023-02-24 13:21 UTC (permalink / raw)
  To: Simon Tournier; +Cc: Ludovic Courtès, 61684

Hello,

Simon Tournier <zimon.toutoune@gmail.com> writes:

> Hi,
>
> On jeu., 23 févr. 2023 at 17:27, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
>
>> Hm.  That seems sub-optimal; it seems to me that ideally, the
>> transformations would be additive, so that users would not need to care
>> about the ordering.  Or perhaps, alternatively, we could enforce such
>> ordering at the implementation level (sorting the transformations in the
>> order that is required).
>
> From my point of view (and what I tried stopping in the middle :-)) is
> to report if the transformation makes sense or not.  For instance,
>
>     with-patch
>     with-source
>
> makes sense contrary to
>
>     with-source
>     with-patch
>
> and it would already be an improvement to report that the latter
> transformation does not make sense instead of silently does nothing or
> raises some weird errors.
>
> Well, I am not convinced that enforce the ordering is a good thing
> because as Ludo said, some HPC user exploits this control of ordering to
> generate complex transformations.

Could we gather more information about that use case?  It needs to be
clear if we are to constrain the design (solution) by it.

> To me, the fix is:
>
>  1. document the ordering bits
>  2. check if the ordering “makes sense“ and raises if not.

If the use case above (the one where transformation order matters and
are useful) is expounded and appears reasonable, then yes, that seems
like a good solution.  Such a use case could appear in a complex example
next to the documentation that explains the ordering "rules".

-- 
Thanks,
Maxim




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

* bug#61684: can't compose 'with-patch' with 'with-source'
  2023-02-24 13:21       ` Maxim Cournoyer
@ 2023-02-24 13:43         ` Simon Tournier
  2023-02-24 13:52           ` Maxim Cournoyer
  0 siblings, 1 reply; 17+ messages in thread
From: Simon Tournier @ 2023-02-24 13:43 UTC (permalink / raw)
  To: Maxim Cournoyer; +Cc: Ludovic Courtès, 61684

Hi Maxim,

On ven., 24 févr. 2023 at 08:21, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:

>> Well, I am not convinced that enforce the ordering is a good thing
>> because as Ludo said, some HPC user exploits this control of ordering to
>> generate complex transformations.
>
> Could we gather more information about that use case?  It needs to be
> clear if we are to constrain the design (solution) by it.

Well, I do not have the details.  Just to mention two presentations
[1,2] exposing how transformations help for them.

1: <https://10years.guix.gnu.org/static/slides/07-swartvagher.pdf>
2: <https://hpc.guix.info/static/doc/atelier-reproductibilit%C3%A9-2021/marek-fel%C5%A1%C3%B6ci-org-guix.pdf>

There they intensively uses transformations.  For instance, p.4 of [2]
it reads,

        guix environment --pure --with-input=pastix-5=pastix-5-mkl \
        --with-input=mumps-scotch-openmpi=mumps-mkl-scotch-openmpi \
        --with-input=openblas=mkl --with-git-url=gcvb=$HOME/src/gcvb \
        --with-commit=gcvb=40d88ba241db4c71ac3e1fe8024fba4d906f45b1 \
        --preserve=^SLURM --ad-hoc bash coreutils inetutils findutils \
        grep sed bc openssh python python-psutil gcvb scab slurm@19 openmpi

For this specific example, the order may or not matter.  The point is
that HPC folks are intensively using transformations and, since the
order currently matters, enforcing one specific order could break their
workflow, and even could make impossible what is currently possible.

Quoting Ludo,

                                                 (this is crucial for our HPC
        users, who routinely combine a whole bunch of options; you have no idea
        how far they go once you give them the tool :-))

        <https://issues.guix.gnu.org/msgid/871qmg79u7.fsf@gnu.org>

and I agree with « you have no idea how far they go once you give them
the tool :-)) ».

For what my opinion is worth. :-)

Cheers,
simon




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

* bug#61684: can't compose 'with-patch' with 'with-source'
  2023-02-24 13:43         ` Simon Tournier
@ 2023-02-24 13:52           ` Maxim Cournoyer
  2023-02-25 12:29             ` Simon Tournier
  2023-02-27 14:12             ` Ludovic Courtès
  0 siblings, 2 replies; 17+ messages in thread
From: Maxim Cournoyer @ 2023-02-24 13:52 UTC (permalink / raw)
  To: Simon Tournier; +Cc: Ludovic Courtès, 61684

Hi Simon,

Simon Tournier <zimon.toutoune@gmail.com> writes:

> Hi Maxim,
>
> On ven., 24 févr. 2023 at 08:21, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
>
>>> Well, I am not convinced that enforce the ordering is a good thing
>>> because as Ludo said, some HPC user exploits this control of ordering to
>>> generate complex transformations.
>>
>> Could we gather more information about that use case?  It needs to be
>> clear if we are to constrain the design (solution) by it.
>
> Well, I do not have the details.  Just to mention two presentations
> [1,2] exposing how transformations help for them.
>
> 1: <https://10years.guix.gnu.org/static/slides/07-swartvagher.pdf>
> 2: <https://hpc.guix.info/static/doc/atelier-reproductibilit%C3%A9-2021/marek-fel%C5%A1%C3%B6ci-org-guix.pdf>
>
> There they intensively uses transformations.  For instance, p.4 of [2]
> it reads,
>
>         guix environment --pure --with-input=pastix-5=pastix-5-mkl \
>         --with-input=mumps-scotch-openmpi=mumps-mkl-scotch-openmpi \
>         --with-input=openblas=mkl --with-git-url=gcvb=$HOME/src/gcvb \
>         --with-commit=gcvb=40d88ba241db4c71ac3e1fe8024fba4d906f45b1 \
>         --preserve=^SLURM --ad-hoc bash coreutils inetutils findutils \
>         grep sed bc openssh python python-psutil gcvb scab slurm@19 openmpi
>
> For this specific example, the order may or not matter.  The point is
> that HPC folks are intensively using transformations and, since the
> order currently matters, enforcing one specific order could break their
> workflow, and even could make impossible what is currently possible.

I'd argue that in the above case, the order doesn't or shouldn't matter.
The user clearly intended for them to be additive.

> Quoting Ludo,
>
>                                                  (this is crucial for our HPC
>         users, who routinely combine a whole bunch of options; you have no idea
>         how far they go once you give them the tool :-))
>
>         <https://issues.guix.gnu.org/msgid/871qmg79u7.fsf@gnu.org>
>
> and I agree with « you have no idea how far they go once you give them
> the tool :-)) ».

It seems a bit hypothetical at this point, so I wouldn't want to cripple
the design by it.  Sure, users will find by experimentation how to
accomplish something, perhaps twisting it in odd ways to, but in my
opinion if the API was consistent it'd be much easier for everyone to
accomplish what they are after.  *If* a valid use case of the current
quirky behavior is discovered, we could consider adding a
'--order-sensitive' option or similar to restore that behavior, I just
don't think it should be the default.

> For what my opinion is worth. :-)

Thanks for sharing it!

-- 
Thanks,
Maxim




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

* bug#61684: can't compose 'with-patch' with 'with-source'
  2023-02-23 14:08 ` Ludovic Courtès
  2023-02-23 22:27   ` Maxim Cournoyer
@ 2023-02-24 13:59   ` Maxim Cournoyer
  2023-03-03  9:43     ` Ludovic Courtès
  1 sibling, 1 reply; 17+ messages in thread
From: Maxim Cournoyer @ 2023-02-24 13:59 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 61684

Hi Ludovic,

Ludovic Courtès <ludo@gnu.org> writes:

> Hi Maxim,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>> Given 'with-source' discards any patch from the original source, I thought
>> I could at least add them back via 'with-patch', but it appears this
>> does not work:
>>
>> scheme@(gnu packages jami)> (options->transformation
>>                              `((with-source . "libjami@20230220.0=/home/maxim/src/jami/jami-20230220.0.tar.gz")
>>                                (with-patch . ,(string-append
>>                                                "libjami=" (search-patch
>>                                                            "jami-disable-integration-tests.patch")))))
>> $6 = #<procedure 7f2cd01a97e0 at guix/transformations.scm:1010:2 (obj)>
>> scheme@(gnu packages jami)> ($6 libjami)
>> $7 = #<package libjami@20230220.0 guix/transformations.scm:1002 7f2ccc8386e0>
>> scheme@(gnu packages jami)> (package-source $7)
>> $8 = #<<downloaded-file> uri: "/home/maxim/src/jami/jami-20230220.0.tar.gz" recursive?: #t>
>> scheme@(gnu packages jami)>
>>
>> The downloaded-file resulting package source has lost the patch, and no
>> error got produced, leaving the user to discover this limitation by
>> themselves.
>
> The order of options matters; in this case, you need to do it the other
> way around:
>
> scheme@(guile-user)> (options->transformation '((with-patch . "jami=/tmp/t.patch")
> 						(with-source . "jami=http://example.org/foo.tar.gz")))
> $18 = #<procedure 7f7e6b1fd0c0 at guix/transformations.scm:1010:2 (obj)>
> scheme@(guile-user)> ($18 jami)
> $19 = #<package jami@20230206.0 guix/transformations.scm:1002 7f7e6b1602c0>
> scheme@(guile-user)> (package-source $19)
> $20 = #<<computed-file> name: "jami-20230206.0-source" gexp: #<gexp  guix/transformations.scm:688:19 7f7e6b6c8d50> guile: #f options: (#:local-build? #t)>
>
> The <computed-file> bit comes from the ‘with-patch’ transformation, to
> apply the patch to the <downloaded-file>.

I tried to do that, like so:

--8<---------------cut here---------------start------------->8---
(define (with-latest-sources name)
  (options->transformation
   ;; XXX: The ordering is important; with-patch must appear before
   ;; with-source, otherwise it is silently discarded.
   `(,@(if (string=? name "libjami")
           `((with-patch . ,(string-append
                             name "="
                             (search-patch
                              "jami-disable-integration-tests.patch")))
             (with-patch . ,(string-append
                             name "="
                             (search-patch
                              "jami-libjami-headers-search.patch"))))
           '())
     (with-source . ,(format #f "~a@~a=~a" name
                             %release-version %release-file-name)))))

(define libjami/latest
  ((with-latest-sources "libjami") libjami))
--8<---------------cut here---------------end--------------->8---

Unfortunately the source derivation fails because it attempts to apply a
patch (a single one?) to a tarball:

--8<---------------cut here---------------start------------->8---
(begin
  (use-modules
   (guix build utils))
  (setenv "PATH" "/gnu/store/mp0syh29rjknflaiv0hkpdlb2mjk0rlx-patch-2.7.6/bin")
  (copy-recursively "/gnu/store/ig8awlxbzrasp9p4f9vq8fqcidrma5qj-jami-20230224.0.tar.gz"
                    ((@
                      (guile)
                      getenv)
                     "out"))
  (chdir
   ((@
     (guile)
     getenv)
    "out"))
  (for-each
   (lambda
       (patch)
     (invoke "patch" "-p1" "--batch" "-i" patch))
   (quote
    ("/gnu/store/iq7hd3f9kr2kvz7lvnygq51yxp85gxbn-jami-libjami-headers-search.patch"))))
--8<---------------cut here---------------end--------------->8---

My expectation of the source rewriting options is that it should end up
with something like what I'd write manually, e.g. an origin with an
appropriate fetch method and its patches field populated, etc., so that
it'd work the same.

-- 
Thanks,
Maxim




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

* bug#61684: can't compose 'with-patch' with 'with-source'
  2023-02-24 13:52           ` Maxim Cournoyer
@ 2023-02-25 12:29             ` Simon Tournier
  2023-02-25 19:43               ` Maxim Cournoyer
  2023-02-27 14:12             ` Ludovic Courtès
  1 sibling, 1 reply; 17+ messages in thread
From: Simon Tournier @ 2023-02-25 12:29 UTC (permalink / raw)
  To: Maxim Cournoyer; +Cc: Ludovic Courtès, 61684

Hi Maxim,

On Fri, 24 Feb 2023 at 08:52, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:

>         if the API was consistent it'd be much easier for everyone

Indeed.  However, when it is currently not, the implicit rule is to not
break backward compatibility.  That’s the whole point. :-)

We need to be very cautious when we change the API; even when it is for
fixing an inconsistency.

Cheers,
simon




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

* bug#61684: can't compose 'with-patch' with 'with-source'
  2023-02-25 12:29             ` Simon Tournier
@ 2023-02-25 19:43               ` Maxim Cournoyer
  0 siblings, 0 replies; 17+ messages in thread
From: Maxim Cournoyer @ 2023-02-25 19:43 UTC (permalink / raw)
  To: Simon Tournier; +Cc: Ludovic Courtès, 61684

Hi,

Simon Tournier <zimon.toutoune@gmail.com> writes:

> Hi Maxim,
>
> On Fri, 24 Feb 2023 at 08:52, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
>
>>         if the API was consistent it'd be much easier for everyone
>
> Indeed.  However, when it is currently not, the implicit rule is to not
> break backward compatibility.  That’s the whole point. :-)
>
> We need to be very cautious when we change the API; even when it is for
> fixing an inconsistency.

Agreed; if the current behavior really provides well outlined benefits,
we could always introduce a "--order-sensitive" option for its users.

-- 
Thanks,
Maxim




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

* bug#61684: can't compose 'with-patch' with 'with-source'
  2023-02-24 12:02     ` Simon Tournier
  2023-02-24 13:21       ` Maxim Cournoyer
@ 2023-02-27 14:09       ` Ludovic Courtès
  2023-02-28 10:00         ` Simon Tournier
  1 sibling, 1 reply; 17+ messages in thread
From: Ludovic Courtès @ 2023-02-27 14:09 UTC (permalink / raw)
  To: Simon Tournier; +Cc: 61684, Maxim Cournoyer

Hi!

Simon Tournier <zimon.toutoune@gmail.com> skribis:

>>From my point of view (and what I tried stopping in the middle :-)) is
> to report if the transformation makes sense or not.  For instance,

You stated that multiple times and there’s general consensus that
reporting the issue would be great.

However, as I explained before, there’s no clear way to do that for two
reasons:

  1. Transformations apply to bags, not packages, so we cannot tell
     whether a transformation has an effect until after the transformed
     package has been lowered.  Even then, it’s tricky.

  2. In this case, this has to do with the semantics of transformations
     themselves: by definition, ‘with-source’ dismisses patches.

[...]

> Well, I am not convinced that enforce the ordering is a good thing
> because as Ludo said, some HPC user exploits this control of ordering to
> generate complex transformations.

My point is that there are folks who have been using package
transformation options for years; any claim has to be evaluated in that
light, and any change would have to be considered very carefully.

Thanks,
Ludo’.




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

* bug#61684: can't compose 'with-patch' with 'with-source'
  2023-02-24 13:52           ` Maxim Cournoyer
  2023-02-25 12:29             ` Simon Tournier
@ 2023-02-27 14:12             ` Ludovic Courtès
  2023-02-27 16:29               ` Maxim Cournoyer
  1 sibling, 1 reply; 17+ messages in thread
From: Ludovic Courtès @ 2023-02-27 14:12 UTC (permalink / raw)
  To: Maxim Cournoyer; +Cc: 61684, Simon Tournier

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

>> Quoting Ludo,
>>
>>                                                  (this is crucial for our HPC
>>         users, who routinely combine a whole bunch of options; you have no idea
>>         how far they go once you give them the tool :-))
>>
>>         <https://issues.guix.gnu.org/msgid/871qmg79u7.fsf@gnu.org>
>>
>> and I agree with « you have no idea how far they go once you give them
>> the tool :-)) ».
>
> It seems a bit hypothetical at this point, so I wouldn't want to cripple
> the design by it.

It baffles me that you would consider other people’s experience as
“hypothetical”.  It’s not, really.

The choice of words after the comma is also unfortunate in several ways.

Ludo’.




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

* bug#61684: can't compose 'with-patch' with 'with-source'
  2023-02-27 14:12             ` Ludovic Courtès
@ 2023-02-27 16:29               ` Maxim Cournoyer
  0 siblings, 0 replies; 17+ messages in thread
From: Maxim Cournoyer @ 2023-02-27 16:29 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 61684, Simon Tournier

Hi Ludovic,

Ludovic Courtès <ludo@gnu.org> writes:

> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>>> Quoting Ludo,
>>>
>>>                                                  (this is crucial for our HPC
>>>         users, who routinely combine a whole bunch of options; you have no idea
>>>         how far they go once you give them the tool :-))
>>>
>>>         <https://issues.guix.gnu.org/msgid/871qmg79u7.fsf@gnu.org>
>>>
>>> and I agree with « you have no idea how far they go once you give them
>>> the tool :-)) ».
>>
>> It seems a bit hypothetical at this point, so I wouldn't want to cripple
>> the design by it.
>
> It baffles me that you would consider other people’s experience as
> “hypothetical”.  It’s not, really.

I didn't mean to dismiss other people experiences.  The reason I'd like
to orient this discussion toward use cases, is that these are easier to
work with (more concrete/well defined) than user experiences.  I'm sure
people use it in many ways I can't think of, and what I'd like us to do
is be able to put a finger on what these ways are, and how the current
API could be improved in a way that satisfies them all (if possible).

> The choice of words after the comma is also unfortunate in several ways.

Noted.  For the record I used in for its "flawed or imperfect"
definition.

-- 
Thanks,
Maxim




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

* bug#61684: can't compose 'with-patch' with 'with-source'
  2023-02-27 14:09       ` Ludovic Courtès
@ 2023-02-28 10:00         ` Simon Tournier
  2023-03-01 15:49           ` Ludovic Courtès
  0 siblings, 1 reply; 17+ messages in thread
From: Simon Tournier @ 2023-02-28 10:00 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 61684, Maxim Cournoyer

Hi Ludo,

On lun., 27 févr. 2023 at 15:09, Ludovic Courtès <ludo@gnu.org> wrote:

> However, as I explained before, there’s no clear way to do that for two
> reasons:
>
>   1. Transformations apply to bags, not packages, so we cannot tell
>      whether a transformation has an effect until after the transformed
>      package has been lowered.  Even then, it’s tricky.
>
>   2. In this case, this has to do with the semantics of transformations
>      themselves: by definition, ‘with-source’ dismisses patches.

I probably miss many details and that’s why I do not understand
correctly your words.  Or maybe we are not using the same meaning behind
“report if the transformation makes sense or not”.


 1. From my point of view, the transformations are functions that you
    compose.  The composition rule is not commutative maybe neither
    associative.  Writing down how each function (transformation)
    composes with the others allows to specify the composition rules.

 2. All the code in (guix transformations) acts at the package level, so
    I am still missing why it would not be possible to detect some
    issues there.


For instance,

--8<---------------cut here---------------start------------->8---
  (define applicable
    ;; List of applicable transformations as symbol/procedure pairs in the
    ;; order in which they appear on the command line.
    (filter-map (match-lambda
                  ((key . value)
                   (match (transformation-procedure key)
                     (#f
                      #f)
                     (transform
                      ;; XXX: We used to pass TRANSFORM a list of several
                      ;; arguments, but we now pass only one, assuming that
                      ;; transform composes well.
                      (list key value (transform (list value)))))))
                (reverse opts)))
--8<---------------cut here---------------end--------------->8---

and I miss why,

 1. it would not be possible to check if the transforms compose well;
    somehow verify the assumption.

For instance, ’package-with-upstream-version’ raises many warnings
depending on various cases, and I miss why,

 2. it would not be possible to have similar warnings for other
    transformations.


Cheers,
simon




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

* bug#61684: can't compose 'with-patch' with 'with-source'
  2023-02-28 10:00         ` Simon Tournier
@ 2023-03-01 15:49           ` Ludovic Courtès
  0 siblings, 0 replies; 17+ messages in thread
From: Ludovic Courtès @ 2023-03-01 15:49 UTC (permalink / raw)
  To: Simon Tournier; +Cc: 61684, Maxim Cournoyer

Hi,

Simon Tournier <zimon.toutoune@gmail.com> skribis:

>  1. From my point of view, the transformations are functions that you
>     compose.  The composition rule is not commutative maybe neither
>     associative.  Writing down how each function (transformation)
>     composes with the others allows to specify the composition rules.
>
>  2. All the code in (guix transformations) acts at the package level, so
>     I am still missing why it would not be possible to detect some
>     issues there.

Please check out ‘package-input-rewriting’ and its behavior
with #:deep? #t.  Design discussion: <https://issues.guix.gnu.org/43578>.

>  1. it would not be possible to check if the transforms compose well;
>     somehow verify the assumption.

Try it.  :-)  I tried to explain it multiple times, I really did, but I
guess there’s no substitute to first-hand experience.

> For instance, ’package-with-upstream-version’ raises many warnings
> depending on various cases, and I miss why,
>
>  2. it would not be possible to have similar warnings for other
>     transformations.

‘package-with-upstream-version’ raises warnings that depend exclusively
on local knowledge.

Here we’re talking about warnings related to the composition to two
different options which, like you wrote, are independent functions.
It’s not similar.

HTH!

Ludo’.




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

* bug#61684: can't compose 'with-patch' with 'with-source'
  2023-02-24 13:59   ` Maxim Cournoyer
@ 2023-03-03  9:43     ` Ludovic Courtès
  0 siblings, 0 replies; 17+ messages in thread
From: Ludovic Courtès @ 2023-03-03  9:43 UTC (permalink / raw)
  To: Maxim Cournoyer; +Cc: 61684

Hi Maxim,

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

> Unfortunately the source derivation fails because it attempts to apply a
> patch (a single one?) to a tarball:
>
> (begin
>   (use-modules
>    (guix build utils))
>   (setenv "PATH" "/gnu/store/mp0syh29rjknflaiv0hkpdlb2mjk0rlx-patch-2.7.6/bin")
>   (copy-recursively "/gnu/store/ig8awlxbzrasp9p4f9vq8fqcidrma5qj-jami-20230224.0.tar.gz"
>                     ((@
>                       (guile)
>                       getenv)
>                      "out"))
>   (chdir
>    ((@
>      (guile)
>      getenv)
>     "out"))
>   (for-each
>    (lambda
>        (patch)
>      (invoke "patch" "-p1" "--batch" "-i" patch))
>    (quote
>     ("/gnu/store/iq7hd3f9kr2kvz7lvnygq51yxp85gxbn-jami-libjami-headers-search.patch"))))

That’s a bug we should fix (and indeed, ‘patched-source’ has an XXX
comment about it…).

Ludo’.




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

end of thread, other threads:[~2023-03-03  9:44 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-21 18:08 bug#61684: can't compose 'with-patch' with 'with-source' Maxim Cournoyer
2023-02-21 19:05 ` Simon Tournier
2023-02-23 14:08 ` Ludovic Courtès
2023-02-23 22:27   ` Maxim Cournoyer
2023-02-24 12:02     ` Simon Tournier
2023-02-24 13:21       ` Maxim Cournoyer
2023-02-24 13:43         ` Simon Tournier
2023-02-24 13:52           ` Maxim Cournoyer
2023-02-25 12:29             ` Simon Tournier
2023-02-25 19:43               ` Maxim Cournoyer
2023-02-27 14:12             ` Ludovic Courtès
2023-02-27 16:29               ` Maxim Cournoyer
2023-02-27 14:09       ` Ludovic Courtès
2023-02-28 10:00         ` Simon Tournier
2023-03-01 15:49           ` Ludovic Courtès
2023-02-24 13:59   ` Maxim Cournoyer
2023-03-03  9:43     ` Ludovic Courtès

Code repositories for project(s) associated with this public inbox

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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).