GNU bug report logs - #40495
[PATCH] Add taisei, and spirv-cross

Previous Next

Package: guix-patches;

Reported by: Vitaliy Shatrov <D0dyBo0D0dyBo0 <at> protonmail.com>

Date: Tue, 7 Apr 2020 19:53:01 UTC

Severity: normal

Tags: patch

Done: Nicolas Goaziou <mail <at> nicolasgoaziou.fr>

Bug is archived. No further changes may be made.

Full log


View this message in rfc822 format

From: Nicolas Goaziou <mail <at> nicolasgoaziou.fr>
To: 40495 <at> debbugs.gnu.org
Cc: D0dyBo0D0dyBo0 <at> protonmail.com
Subject: [bug#40495] v3-- fixed indentation
Date: Sat, 11 Apr 2020 22:23:30 +0200
Hello,

Vitaliy Shatrov via Guix-patches via <guix-patches <at> gnu.org> writes:

> Subject: [PATCH] gnu: Add taisei, and spirv-cross
>
> * gnu/packages/games.scm  (taisei):      new variable
> * gnu/packages/vulkan.scm (spirv-cross): new variable

It works nicely, thank you.

> +     `(;;configure option developer=true enables the diagnostics
> +       ;;needed for bug reports.  it is 'true' if not "release"
> +       ;;#:build-type "release"

So, what do you suggest here? Use release or developer build?

> +    (synopsis "Fangame and libre clone of Touhou Project")

I would suggest something like

  Shoot'em up game set in a world full of Japanese folklore

> +    (description
> +     "Taisei is a shoot-em-up game: The player controls a character (one of
> +three: Good, Bad, and Dead), dodges the missiles (lots of it cover the screen,
> +but the character's hitbox is very small), and shoot at the adversaries that
> +keep appear on the screen.")


> +    (license (list license:expat      ;game
> +                   license:cc-by4.0   ;resources/00-taisei.pkgdir/bgm/
> +                                        ;atlas/portraits/
> +                   ;;miscellaneous
> +                   license:cc0
> +                   license:public-domain))))

It would be clearer to explain in a comment above the license field what
is subject to what terms.

> +    (source
> +     (origin
> +       (method git-fetch)
> +       (uri (git-reference
> +             (url "https://github.com/KhronosGroup/SPIRV-Cross")
> +             (commit (string-append version))))

You can remove the `string-append' here.

> +       (sha256
> +        (base32
> +         "0489s29kqgq20clxqg22y299yxz23p0yjh87yhka705hm9skx4sa"))
> +       (file-name (git-file-name name version))))
> +    (build-system cmake-build-system)
> +    (arguments
> +     `(#:tests? #f  ;FIXME: Tests fail.

IMO, this is not a blocker. However, if you have more information than
"Tests fail", it would be nice to add it in a comment.

> +       #:configure-flags
> +       (list "-DSPIRV_CROSS_SHARED=YES")
> +       #:phases
> +       (modify-phases %standard-phases
> +         (add-after 'unpack 'fix-tests-to-find-deps
> +           (lambda* (#:key inputs #:allow-other-keys)
> +             (substitute* "CMakeLists.txt"
> +               (((string-append "PATHS "
> +                                "\\$\\{CMAKE_CURRENT_SOURCE_DIR\\}"
> +                                "/external/glslang-build/output/bin"))

Why do you need this? What about simply writing the full string without
`string-append'?

> +             (substitute* "CMakeLists.txt"
> +               (((string-append "PATHS "
> +                                "\\$\\{CMAKE_CURRENT_SOURCE_DIR\\}"
> +                                "/external/spirv-tools-build/output/bin"))

Ditto.

> +    (native-inputs `(("glslang" ,glslang)
> +                     ("python" ,python)
> +                     ("spirv-headers" ,spirv-headers)
> +                     ("spirv-tools" ,spirv-tools)))

Nitpick: I would move the inputs below the `native-inputs' line.

> +    (description
> +     "SPIRV-Cross tries hard to emit readable and clean output from the
> +SPIR-V.  The goal is to emit GLSL or MSL that looks like it was written by a
> +human and not awkward IR/assembly-like code.  NOTE: Individual features are

You can drop the "NOTE:" prefix. 

Actually, I think you can drop everything after "NOTE:". Is it useful
information for someone looking at the package?

> +expected to be mostly complete, but it is possible that certain obscure GLSL
> +features are not yet supported.  However, most missing features are expected
> +to be \"trivial\" improvements at this stage.")

If you disagree with my suggestion, you need to remove these double
quotes. Texinfo uses ``trivial'', but you could also write
@emph{trivial}.

Regards,

-- 
Nicolas Goaziou




This bug report was last modified 5 years and 21 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.