[bug#33059] [PATCH 08/10] gnu: Add fenics-dolfin.

[ludo@gnu.org (Ludovic Courtès)]

Paul Garlick <pgarlick@tourbillion-technology.com> skribis:

> * gnu/packages/simulation.scm (fenics-dolfin): New variable.

[...]

> +         (add-before 'configure 'pre-configure
> +           (lambda _
> +             (use-modules (ice-9 regex)
> +                          (ice-9 rdelim)
> +                          (guix build utils)
> +                          (rnrs io ports))

Please use #:modules instead of an inner ‘use-modules’ form (which may
or may not work in future Guile versions.)

> +             ;; Add extra include directories required by the unit tests.
> +             (with-atomic-file-replacement "test/unit/cpp/CMakeLists.txt"
> +               (let ((rx (make-regexp "target_link_libraries")))
> +                 (lambda (in out)
> +                   (let loop ()
> +                     (let ((line (read-line in 'concat)))
> +                       (if (eof-object? line)
> +                           #t
> +                           (begin
> +                             (display line out)
> +                             (when (regexp-exec rx line)
> +                               (display
> +                                 (string-append
> +                                  "target_include_directories("
> +                                  "unittests PRIVATE "
> +                                  "${DOLFIN_SOURCE_DIR} "
> +                                  "${DOLFIN_SOURCE_DIR}/dolfin "
> +                                  "${DOLFIN_BINARY_DIR})\n") out))
> +                             (loop))))))))

Could this be achieved with a single ‘substitute*’?  It looks like that
would be more compact.

Also, perhaps this should be done in a ‘snippet’?

> +             ;; Add extra include directories required by the demo tests.
> +             (with-atomic-file-replacement "demo/CMakeLists.txt"
> +               (let ((rx (make-regexp "find_package")))
> +                 (lambda (in out)
> +                   (let loop ()
> +                     (let ((line (read-line in 'concat)))
> +                       (if (eof-object? line)
> +                           #t
> +                           (begin
> +                             (display line out)
> +                             (when (regexp-exec rx line)
> +                               (display
> +                                 (string-append
> +                                  "include_directories("
> +                                  "${DOLFIN_SOURCE_DIR} "
> +                                  "${DOLFIN_SOURCE_DIR}/dolfin "
> +                                  "${DOLFIN_BINARY_DIR})\n") out))
> +                             (loop))))))))))

Same question here.

> +             (call-with-output-file "CTestCustom.cmake"
> +               (lambda (port)
> +                 (display
> +                   (string-append
> +                    "set(CTEST_CUSTOM_TESTS_IGNORE "
> +                    "demo_bcs_serial "
> +                    "demo_bcs_mpi "
> +                    "demo_eigenvalue_serial "
> +                    "demo_eigenvalue_mpi "
> +                    "demo_navier-stokes_serial "

Could we avoid listing all the files here?  I’m thinking about something
like ‘scandir’ + ‘delete’.

> +    ;; The source code files for the DOLFIN C++ library are licensed under the
> +    ;; GNU Lesser General Public License, version 3 or later, with the
> +    ;; following exceptions:
> +    ;;
> +    ;; bsd-2:         cmake/modules/FindAMD.cmake
> +    ;;                cmake/modules/FindBLASHeader.cmake
> +    ;;                cmake/modules/FindCHOLMOD.cmake
> +    ;;                cmake/modules/FindEigen3.cmake
> +    ;;                cmake/modules/FindMPFR.cmake
> +    ;;                cmake/modules/FindNumPy.cmake
> +    ;;                cmake/modules/FindPETSc.cmake
> +    ;;                cmake/modules/FindPETSc4py.cmake
> +    ;;                cmake/modules/FindParMETIS.cmake
> +    ;;                cmake/modules/FindSCOTCH.cmake
> +    ;;                cmake/modules/FindSLEPc.cmake
> +    ;;                cmake/modules/FindSLEPc4py.cmake
> +    ;;                cmake/modules/FindSphinx.cmake
> +    ;;                cmake/modules/FindSUNDIALS.cmake
> +    ;;                cmake/modules/FindUFC.cmake
> +    ;;
> +    ;; bsd-3:         cmake/modules/FindBLAS.cmake
> +    ;;                cmake/modules/FindLAPACK.cmake
> +    ;;                cmake/modules/FindMPI.cmake
> +    ;;
> +    ;; public-domain: dolfin/geometry/predicates.cpp
> +    ;;                dolfin/geometry/predicates.h
> +    ;;
> +    ;; zlib:          dolfin/io/base64.cpp
> +    ;;                dolfin/io/base64.h
> +    ;;
> +    ;; expat:         dolfin/io/pugiconfig.hpp
> +    ;;                dolfin/io/pugixml.cpp
> +    ;;                dolfin/io/pugixml.hpp
> +    ;;
> +    ;; boost1.0:      test/unit/cpp/catch/catch.hpp

Thanks for the detailed licensing review!

IMO we don’t need to list the license of the .cmake files, which are
just build files (likewise, we usually ignore M4 files and shell scripts
found in Autotools-based projects.)

I wonder if we could use our ‘catch’ package and remove ‘catch.hpp’.

That’s it!