diff mbox series

[bug#41562] use consistent arguments.

Message ID 3098426.mvXUDI8C0e@cherry
State Accepted
Headers show
Series [bug#41562] use consistent arguments. | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch fail View Laminar job

Commit Message

guix--- via Guix-patches via May 29, 2020, 1:27 p.m. UTC
As suggested on IRC

Comments

Marius Bakke May 29, 2020, 8:56 p.m. UTC | #1
Tom Zander via Guix-patches via <guix-patches@gnu.org> writes:

> As suggested on IRC
> -- 
> Tom Zander
>>From bc9a3faa045579f48804cf42f958409e51cdfbf6 Mon Sep 17 00:00:00 2001
> From: TomZ <tomz@freedommail.ch>
> Date: Wed, 27 May 2020 13:21:58 +0200
> Subject: [PATCH] gnu: flowee: Disable unpredictable test.
>
> * gnu/packages/finance.scm (flowee): The test 'api' has some timeouts
>   which fail half the time in a build, so disable the test.

Can you move this piece of information to a comment in the code?

Also, this should say (flowee)[arguments]: ...

> diff --git a/gnu/packages/finance.scm b/gnu/packages/finance.scm
> index f182057bb4..d176ac0660 100644
> --- a/gnu/packages/finance.scm
> +++ b/gnu/packages/finance.scm
> @@ -1531,7 +1531,7 @@ like Flowee the Hub, which Fulcrum connects to over RPC.")
>           (base32 "1vwvaxm3b71pfx8l4rrv06wqks6xdf2333w856b36s1bzvj53rhc"))))
>      (build-system cmake-build-system)
>      (arguments
> -     `(#:configure-flags '("-Dbuild_tests=ON" "-Denable_gui=OFF")
> +     `(#:configure-flags '("-Dbuild_tests=ON" "-Denable_gui=OFF" "-Denable_gui=OFF")

This should probably not be here?

>         #:phases
>          (modify-phases %standard-phases
>            (add-before 'configure 'make-qt-deterministic
> @@ -1546,6 +1546,8 @@ like Flowee the Hub, which Fulcrum connects to over RPC.")
>              (lambda _
>                (substitute* "testing/CMakeLists.txt"
>                  (("test_api") ""))

Can this be removed?  It looks like it tries to do the same as your new
substitution below.

> +              (substitute* "testing/CMakeLists.txt"
> +                (("add_subdirectory\\(api\\)") ""))
>                #t))
>            (add-after 'configure 'set-build-info
>              ;; Their genbuild.sh to generate a build.h fails in guix (no .git dir) .
> -- 
> 2.25.1
guix--- via Guix-patches via May 30, 2020, 7:40 a.m. UTC | #2
On vrijdag 29 mei 2020 22:56:18 CEST Marius Bakke wrote:
> Tom Zander via Guix-patches via <guix-patches@gnu.org> writes:
> > As suggested on IRC
> > 
> >>From bc9a3faa045579f48804cf42f958409e51cdfbf6 Mon Sep 17 00:00:00 2001
> >>
> > From: TomZ <tomz@freedommail.ch>
> > Date: Wed, 27 May 2020 13:21:58 +0200
> > Subject: [PATCH] gnu: flowee: Disable unpredictable test.
> > 
> > * gnu/packages/finance.scm (flowee): The test 'api' has some timeouts
> > 
> >   which fail half the time in a build, so disable the test.
> 
> Can you move this piece of information to a comment in the code?

Can you point to the file I find the code in, to add this to?

In case you mean the package description, its already there.

> Also, this should say (flowee)[arguments]: ...
> 
> > diff --git a/gnu/packages/finance.scm b/gnu/packages/finance.scm
> > index f182057bb4..d176ac0660 100644
> > --- a/gnu/packages/finance.scm
> > +++ b/gnu/packages/finance.scm
> > @@ -1531,7 +1531,7 @@ like Flowee the Hub, which Fulcrum connects to over
> > RPC.")> 
> >           (base32
> >           "1vwvaxm3b71pfx8l4rrv06wqks6xdf2333w856b36s1bzvj53rhc"))))
> >      
> >      (build-system cmake-build-system)
> >      (arguments
> > 
> > -     `(#:configure-flags '("-Dbuild_tests=ON" "-Denable_gui=OFF")
> > +     `(#:configure-flags '("-Dbuild_tests=ON" "-Denable_gui=OFF"
> > "-Denable_gui=OFF")
> This should probably not be here?

Why not?

 
> >         #:phases
> >         
> >          (modify-phases %standard-phases
> >          
> >            (add-before 'configure 'make-qt-deterministic
> > 
> > @@ -1546,6 +1546,8 @@ like Flowee the Hub, which Fulcrum connects to over
> > RPC.")> 
> >              (lambda _
> >              
> >                (substitute* "testing/CMakeLists.txt"
> >                
> >                  (("test_api") ""))
> 
> Can this be removed?  It looks like it tries to do the same as your new
> substitution below.

Same file, two substitutes because there are two lines that need changing. 
CMake blocks for 1) the actual subdir and 2) a build-dependency for 'make-
check'
 
> > +              (substitute* "testing/CMakeLists.txt"
> > +                (("add_subdirectory\\(api\\)") ""))
> > 
> >                #t))
> >            
> >            (add-after 'configure 'set-build-info
> >            
> >              ;; Their genbuild.sh to generate a build.h fails in guix (no
> >              .git dir) .
Marius Bakke May 30, 2020, 8:07 a.m. UTC | #3
Tom Zander <tomz@freedommail.ch> writes:

> On vrijdag 29 mei 2020 22:56:18 CEST Marius Bakke wrote:
>> Tom Zander via Guix-patches via <guix-patches@gnu.org> writes:
>> > As suggested on IRC
>> > 
>> >>From bc9a3faa045579f48804cf42f958409e51cdfbf6 Mon Sep 17 00:00:00 2001
>> >>
>> > From: TomZ <tomz@freedommail.ch>
>> > Date: Wed, 27 May 2020 13:21:58 +0200
>> > Subject: [PATCH] gnu: flowee: Disable unpredictable test.
>> > 
>> > * gnu/packages/finance.scm (flowee): The test 'api' has some timeouts
>> > 
>> >   which fail half the time in a build, so disable the test.
>> 
>> Can you move this piece of information to a comment in the code?
>
> Can you point to the file I find the code in, to add this to?
>
> In case you mean the package description, its already there.

Oh you're right.  In that case it would be clearer to change the commit
message to something like "really disable the black box tests".

>> Also, this should say (flowee)[arguments]: ...
>> 
>> > diff --git a/gnu/packages/finance.scm b/gnu/packages/finance.scm
>> > index f182057bb4..d176ac0660 100644
>> > --- a/gnu/packages/finance.scm
>> > +++ b/gnu/packages/finance.scm
>> > @@ -1531,7 +1531,7 @@ like Flowee the Hub, which Fulcrum connects to over
>> > RPC.")> 
>> >           (base32
>> >           "1vwvaxm3b71pfx8l4rrv06wqks6xdf2333w856b36s1bzvj53rhc"))))
>> >      
>> >      (build-system cmake-build-system)
>> >      (arguments
>> > 
>> > -     `(#:configure-flags '("-Dbuild_tests=ON" "-Denable_gui=OFF")
>> > +     `(#:configure-flags '("-Dbuild_tests=ON" "-Denable_gui=OFF"
>> > "-Denable_gui=OFF")
>> This should probably not be here?
>
> Why not?

I don't think passing -Denable_gui=OFF twice makes a difference.  Does
it?

>> >         #:phases
>> >         
>> >          (modify-phases %standard-phases
>> >          
>> >            (add-before 'configure 'make-qt-deterministic
>> > 
>> > @@ -1546,6 +1546,8 @@ like Flowee the Hub, which Fulcrum connects to over
>> > RPC.")> 
>> >              (lambda _
>> >              
>> >                (substitute* "testing/CMakeLists.txt"
>> >                
>> >                  (("test_api") ""))
>> 
>> Can this be removed?  It looks like it tries to do the same as your new
>> substitution below.
>
> Same file, two substitutes because there are two lines that need changing. 
> CMake blocks for 1) the actual subdir and 2) a build-dependency for 'make-
> check'

Makes sense, thanks for the explanation.
diff mbox series

Patch

From bc9a3faa045579f48804cf42f958409e51cdfbf6 Mon Sep 17 00:00:00 2001
From: TomZ <tomz@freedommail.ch>
Date: Wed, 27 May 2020 13:21:58 +0200
Subject: [PATCH] gnu: flowee: Disable unpredictable test.

* gnu/packages/finance.scm (flowee): The test 'api' has some timeouts
  which fail half the time in a build, so disable the test.
---
 gnu/packages/finance.scm | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/finance.scm b/gnu/packages/finance.scm
index f182057bb4..d176ac0660 100644
--- a/gnu/packages/finance.scm
+++ b/gnu/packages/finance.scm
@@ -1531,7 +1531,7 @@  like Flowee the Hub, which Fulcrum connects to over RPC.")
          (base32 "1vwvaxm3b71pfx8l4rrv06wqks6xdf2333w856b36s1bzvj53rhc"))))
     (build-system cmake-build-system)
     (arguments
-     `(#:configure-flags '("-Dbuild_tests=ON" "-Denable_gui=OFF")
+     `(#:configure-flags '("-Dbuild_tests=ON" "-Denable_gui=OFF" "-Denable_gui=OFF")
        #:phases
         (modify-phases %standard-phases
           (add-before 'configure 'make-qt-deterministic
@@ -1546,6 +1546,8 @@  like Flowee the Hub, which Fulcrum connects to over RPC.")
             (lambda _
               (substitute* "testing/CMakeLists.txt"
                 (("test_api") ""))
+              (substitute* "testing/CMakeLists.txt"
+                (("add_subdirectory\\(api\\)") ""))
               #t))
           (add-after 'configure 'set-build-info
             ;; Their genbuild.sh to generate a build.h fails in guix (no .git dir) .
-- 
2.25.1