Subject: Re: bug#75856: [PATCH] gnu: eureka: Update to 2.0.2.

From: Maxim Cournoyer <[email-masked]>
Date: Thu, 13 Feb 2025 11:46:31 +0900

Hello,

Andrew Wong <[email-masked]> writes:

> * gnu/packages/emulators.scm (flycast): New variable.
>
> Change-Id: I358235492a3493c3126f7ca9f738ed2706e36105
> ---
> Flycast has a libretro core too, but I haven't gotten it working
> yet. I am also working directly with the developer to try to un-bundle
> some of the deps.
>  gnu/packages/emulators.scm | 56 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 56 insertions(+)
>
> diff --git a/gnu/packages/emulators.scm b/gnu/packages/emulators.scm
> index f20b6da79b..c6ff685948 100644
> --- a/gnu/packages/emulators.scm
> +++ b/gnu/packages/emulators.scm
> @@ -4072,6 +4072,62 @@ (define-public exomizer
>      ;; zlib license with an (non-)advertising clause.
>      (license license:zlib)))
>  
> +(define-public flycast
> +  (package
> +    (name "flycast")
> +    (version "2.4")
> +    (source
> +     (origin
> +       (method git-fetch)
> +       (uri (git-reference
> +             (url "https://github.com/flyinghead/flycast")
> +             (commit (string-append "v" version))

Please add a comment as to why recursive? is needed; typically, that's
to pull bundled sources that should be pruned as much as possible,
e.g. at least those being used from the system should be removed in the
origin snippet below.  I've used a local 'delete-all-but' definition in
the past, for example in the turbovnc package, that may be useful here.

> +             (recursive? #t)))
> +       (file-name (git-file-name name version))
> +       (sha256
> +        (base32 "0ainy75gkrvilcm89hq6wq9md41w0mxgp6l27q5fzrxxykpjh6ym"))
> +       (modules '((guix build utils)
> +                  (guile)))

You don't ever need to import the (guile) module; it's always loaded
(except pehaps in pure R7RS mode, but that's not used in Guix) :-).

> +       (snippet
> +        #~(begin
> +            (substitute* "CMakeLists.txt"
> +              (("add_subdirectory\\(core/deps/Vulkan-Headers\\)")
> +               "find_package(VulkanHeaders)"))))))
> +    (build-system cmake-build-system)
> +    (inputs (list pkg-config
> +                  sdl2
> +                  vulkan-headers
> +                  glslang
> +                  libzip
> +                  curl
> +                  alsa-lib
> +                  lua
> +                  ao
> +                  pulseaudio
> +                  miniupnpc
> +                  ;; vv needed for vulkan
> +                  spirv-tools))
> +    (arguments
> +     (list #:tests? #f                   ;no target 'test'

I'd research a bit more and write ';no test suite', if that's truly the
case.  The test target is sometimes named differently.

> +           #:configure-flags
> +           #~(list "-DUSE_HOST_GLSLANG=OFF"
> +                   ;; https://github.com/flyinghead/flycast/issues/1843

I'd write a full fledge comment briefly mentioning what's the error we
get otherwise, with a (see: $url) reference at the end.  Use complete
(including punctuation) sentences for line comments.  Margin comments
can be incomplete and unpunctuate sentences.

> +                   "-DUSE_HOST_LIBZIP=ON"
> +                   "-DUSE_HOST_SDL=ON"
> +                   "-DUSE_ALSA=ON"
> +                   "-DUSE_LUA=ON"
> +                   "-DUSE_LIBAO=ON"
> +                   "-DUSE_PULSEAUDIO=ON"
> +                   "-DUSE_BREAKPAD=OFF"
> +                   "-DUSE_VULKAN=ON"
> +                   "-DUSE_DX9=OFF"
> +                   "-DUSE_DX11=OFF")))
> +    (home-page "https://github.com/flyinghead/flycast")
> +    (synopsis "Sega Dreamcast, Naomi, Naomi 2, and Atomiswave emulator")
> +    (description "Flycast is a multi-platform Sega Dreamcast, Naomi, Naomi 2,
> +and Atomiswave emulator derived from reicast.")
> +    (license license:gpl2+)))
> +

With the above addressed, it LGTM.  Could you please send a new revision?

-- 
Thanks,
Maxim