guix-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Ludovic Courtès
Subject: [bug#33059] [PATCH 08/10] gnu: Add fenics-dolfin.
Date: Thu, 25 Oct 2018 00:12:19 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux)

Paul Garlick <address@hidden> 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!





reply via email to

[Prev in Thread] Current Thread [Next in Thread]