[bug#66199] gnu: librepcb: Update to 1.0.0.

* gnu/packages/engineering.scm (librepcb): Update to 1.0.0.
---
 gnu/packages/engineering.scm | 29 +++++++++++++----------------
 1 file changed, 13 insertions(+), 16 deletions(-)


base-commit: 445a0359083388b5ee686e6e855f94a3aac5f79c
  

Hello,

Em 25/09/2023 17:27, pinoaffe escreveu:
> * gnu/packages/engineering.scm (librepcb): Update to 1.0.0.

Thank you.

The commit message should also mention the switch to cmake-build-system, 
the "not overriding" of the configure phase, the new inputs and the test 
skip. I have a few more comments below.

> ---
>   gnu/packages/engineering.scm | 29 +++++++++++++----------------
>   1 file changed, 13 insertions(+), 16 deletions(-)
>
> diff --git a/gnu/packages/engineering.scm b/gnu/packages/engineering.scm
> index c2846f0bda..f12f4ad59d 100644
> --- a/gnu/packages/engineering.scm
> +++ b/gnu/packages/engineering.scm
> @@ -2260,33 +2260,30 @@ (define-public freehdl
>   (define-public librepcb
>     (package
>       (name "librepcb")
> -    (version "0.1.5")
> +    (version "1.0.0")
>       (source
>        (origin
>          (method url-fetch)
>          (uri (string-append "https://download.librepcb.org/releases/"
>                              version "/librepcb-" version "-source.zip"))
>          (sha256
> -        (base32 "0smp1p7wnrj0vh4rmz1cr2krfawc2lzx0pbzmgyay7xdp6jxympr"))))
> -    (build-system gnu-build-system)
> +        (base32 "02qfwyhdq1pklb5gkwn3rbsdhwvcgiksd21swaphz3kw6s4p9i8v"))))
> +    (build-system cmake-build-system)
>       (inputs
> -     (list qtbase-5 qtsvg-5 zlib))
> +     (list qtbase-5
> +           qtsvg-5
> +           qtdeclarative-5
> +           qtquickcontrols2-5
> +           zlib
> +           opencascade-occt
> +           glu
> +           fontconfig))

Inputs should be sorted alphabetically.

>       (native-inputs
>        (list qttools-5 ; for lrelease
>              unzip))

The comment about lrelease can be removed as lrelease is not being 
manually invoked anymore, and the native inputs list can be squashed in 
one line.

>       (arguments
> -     `(#:phases
> -       (modify-phases %standard-phases
> -         (replace 'configure
> -           (lambda* (#:key inputs outputs #:allow-other-keys)
> -             (mkdir-p "build")
> -             (chdir "build")
> -             (let ((lrelease (search-input-file inputs "/bin/lrelease"))
> -                   (out (assoc-ref outputs "out")))
> -               (invoke "qmake"
> -                       (string-append "QMAKE_LRELEASE=" lrelease)
> -                       (string-append "PREFIX=" out)
> -                       "../librepcb.pro")))))))
> +     ;; There is no cmake test target
> +     `(#:tests? #f))
>       (home-page "https://librepcb.org/")
>       (synopsis "Electronic Design Automation tool")
>       (description "LibrePCB is @dfn{Electronic Design Automation} (EDA)
>
> base-commit: 445a0359083388b5ee686e6e855f94a3aac5f79c

There are options in LibrePCB's CMakeLists.txt to unvendor some 
dependencies: 
https://github.com/LibrePCB/LibrePCB/blob/9edb6ede393e5b48785f95252f81a027db4b718a/CMakeLists.txt#L51

The only dependencies we do not have is dxflib and fontobene-qt5. It 
failed to find muparser and polyclipping ('clipper' package in Guix), I 
don't know why.

It would be best to unvendor as many dependencies as possible. Could you 
send an updated patch? Optionally, you can try to package fontobene-qt5 
and have cmake find muparser and polyclipping.

Vinicius
  

Hi,

thank you for your review!

Vinicius Monego <monego@posteo.net> writes:
> The commit message should also mention the switch to
> cmake-build-system, the "not overriding" of the configure phase, the
> new inputs and the test skip..
I wasn't sure how to phrase / format this, is what I arrived at OK?

> Inputs should be sorted alphabetically.
Done

> The comment about lrelease can be removed as lrelease is not being
> manually invoked anymore, and the native inputs list can be squashed
> in one line.
Done

> There are options in LibrePCB's CMakeLists.txt to unvendor some
> dependencies:
> https://github.com/LibrePCB/LibrePCB/blob/9edb6ede393e5b48785f95252f81a027db4b718a/CMakeLists.txt#L51
I unvendored all of the mentioned dependencies apart from dxflib (I
couldn't get it to build).

> The only dependencies we do not have is dxflib and fontobene-qt5. It
> failed to find muparser and polyclipping ('clipper' package in Guix),
> I don't know why.
Adding pkg-config as a native dependency seems to have done the trick

> It would be best to unvendor as many dependencies as possible. Could
> you send an updated patch? Optionally, you can try to package
> fontobene-qt5 and have cmake find muparser and polyclipping.

I'll send some updated patches in a minute

Kind regards,
pinoaffe
  

Em 26/09/2023 09:08, pinoaffe escreveu:
> Hi,

Hi!

> thank you for your review!
>
> Vinicius Monego <monego@posteo.net> writes:
>> The commit message should also mention the switch to
>> cmake-build-system, the "not overriding" of the configure phase, the
>> new inputs and the test skip..
> I wasn't sure how to phrase / format this, is what I arrived at OK?

You can check the logs of e.g. commit 
19617735df2b1af3b169d8153ae543ad3e0fc1a1 for reference.

>> Inputs should be sorted alphabetically.
> Done
>
'googletest' should be a native input, it's a test requirement only.

[...]

> I'll send some updated patches in a minute
>
> Kind regards,
> pinoaffe

[Comment on the LibrePCB update patch specifically]: This v2 is much 
better, thanks. While investigating the build I also found a few more 
things that could be improved:

- The bundles that were unvendored can be deleted in a source snippet 
(see e.g. the mixxx package).

- LibrePCB builds a test executable at 
tests/unittests/librepcb-unittests. Instead of 'make test', that file 
could be run manually when overriding the check phase.

- The license list should be updated as some of the bundles listed there 
are not being provided anymore, or even better let it be only gpl3+ 
because that's the license of the final product.

- I got two merge conflicts on 06dc36ffb7cde821a4762b299d1c95b3788ba110, 
please rebase it to the latest commit.

If the tests are too problematic that's fine, I can merge this patch 
with the other changes later this week.

Vinicius
  

Thanks again!

Vinicius Monego <monego@posteo.net> writes:
> Em 26/09/2023 09:08, pinoaffe escreveu:
>> Vinicius Monego <monego@posteo.net> writes:
>>> The commit message should also mention the switch to
>>> cmake-build-system, the "not overriding" of the configure phase, the
>>> new inputs and the test skip..
>> I wasn't sure how to phrase / format this, is what I arrived at OK?
> You can check the logs of e.g. commit
> 19617735df2b1af3b169d8153ae543ad3e0fc1a1 for reference.
Ah yes, I'll try to replicate that


> 'googletest' should be a native input, it's a test requirement only.
oh yes, of course, thanks!

> [Comment on the LibrePCB update patch specifically]: This v2 is much
> better, thanks. While investigating the build I also found a few more
> things that could be improved:
>
> - The bundles that were unvendored can be deleted in a source snippet
>   (see e.g. the mixxx package).
Done

> - LibrePCB builds a test executable at
>   tests/unittests/librepcb-unittests. Instead of 'make test', that
>   file could be run manually when overriding the check phase.
Done

> - The license list should be updated as some of the bundles listed
>   there are not being provided anymore, or even better let it be only
>   gpl3+ because that's the license of the final product.
I removed all of the licenses and license comments that were no longer
relevant, but there's still some non-GPL3+ code involved, so I figured
I'd keep those licenses around.

> - I got two merge conflicts on
>   06dc36ffb7cde821a4762b299d1c95b3788ba110, please rebase it to the
>  latest commit.
Done

> If the tests are too problematic that's fine, I can merge this patch
> with the other changes later this week.
>
> Vinicius

Kind regards, pinoaffe