[bug#60571] [PATCH v2 4/4] gnu: skia: Activate tests.

[Maxim Cournoyer]

Hello,

Nicolas Graves <ngraves@ngraves.fr> writes:

> * gnu/packages/graphics.scm (skia): Activate tests.

Neat!

> ---
>  gnu/packages/graphics.scm | 144 ++++++++++++++++++++++++++++++++++----
>  1 file changed, 132 insertions(+), 12 deletions(-)
>
> diff --git a/gnu/packages/graphics.scm b/gnu/packages/graphics.scm
> index 2a94bd51cc..edb0e82a54 100644
> --- a/gnu/packages/graphics.scm
> +++ b/gnu/packages/graphics.scm
> @@ -83,6 +83,7 @@ (define-module (gnu packages graphics)
>    #:use-module (gnu packages gstreamer)
>    #:use-module (gnu packages gtk)
>    #:use-module (gnu packages haskell-xyz)
> +  #:use-module (gnu packages icu4c)
>    #:use-module (gnu packages image)
>    #:use-module (gnu packages image-processing)
>    #:use-module (gnu packages imagemagick)
> @@ -2017,10 +2018,6 @@ (define-public skia
>        (build-system gnu-build-system)   ;actually GN + Ninja
>        (arguments
>         (list
> -        ;; Running the test suite would require 'dm'; unfortunately the tool
> -        ;; can only be built for debug builds, which require fetching third
> -        ;; party sources.
> -        #:tests? #f
>          #:phases
>          #~(modify-phases %standard-phases
>              (replace 'configure
> @@ -2085,13 +2082,136 @@ (define-public skia
>  URL: https://skia.org/
>  Version: ~a
>  Libs: -L${libdir} -lskia
> -Cflags: -I${includedir}~%" #$output #$version))))))))
> -      (native-inputs (list gn libjpeg-turbo ninja pkg-config python-wrapper))
> -      (inputs (list expat fontconfig freetype harfbuzz mesa libwebp zlib))
> -      (home-page "https://skia.org/";)
> -      (synopsis "2D graphics library")
> -      (description
> -       "Skia is a 2D graphics library for drawing text, geometries, and 
> images.
> +Cflags: -I${includedir}~%" #$output #$version)))))
> +            (replace 'check
> +              (lambda* (#:key inputs native-inputs #:allow-other-keys)
> +                (let ((icu #$(this-package-native-input "icu4c-for-skia")))
> +                  ;; Configure SPIRV-Tools dependency.

I think what this does is to unbundle these from the source?  If that's
the case, the comment should explicit that.

> +                  (substitute* "BUILD.gn"
> +                    (("deps \\+= \\[ 
> \"\\/\\/third_party\\/externals\\/spirv-tools:spvtools_val\" \\]")
> +                      "libs += [ \"SPIRV-Tools\" ]"))
> +                  (substitute* "src/sksl/SkSLCompiler.cpp"
> +                    (("\"spirv-tools/libspirv.hpp\"")
> +                     "<libspirv.hpp>"))
> +                  ;; Configure ICU dependency.
> +                  (substitute* "third_party/icu/BUILD.gn"
> +                    (("data_dir = \"\\.\\.\\/externals\\/icu\\/\"")
> +                     (string-append "data_dir = \"" icu "/share/data/\""))
> +                    (("script = \"\\.\\.\\/externals\\/icu\\/scripts\\/")
> +                     (string-append "script = \"" icu "/share/scripts/"))
> +                    (("\\.\\.\\/externals\\/icu\\/common\\/icudtl.dat")
> +                     (string-append icu "/share/data/icudtl.dat"))
> +                    (("sources = icu_sources")
> +                     "")
> +                    (("sources \\+= \\[ \"\\$data_assembly\" \\]")
> +                     "sources = [ \"$data_assembly\" ]"))

The forward slashes (/) do not need to be escaped for ERE regexps, which
is what Guile uses.  This will help keep your above regexps more
readable.

> +                  ;; Enable system libraries without is_official_build=true.
> +                  ;; Necessary because is_official_build prevents building 
> dm.

Please try to keep your comments full sentences, e.g. "This is necessary
because ...".  It may be a bit more efforts, but this effort pays off for
the next human who'll read these :-).

> +                  (for-each
> +                   (lambda (libname)
> +                     (let ((snake (string-join (string-split libname #\-) 
> "_")))
> +                       (substitute*
> +                           (string-append "third_party/" libname "/BUILD.gn")
> +                         (((string-append "skia_use_system_"
> +                                          snake
> +                                          " = is_official_build.*"))
> +                          (string-append "skia_use_system_" snake " = 
> true")))))
> +                   '("zlib" "libjpeg-turbo" "harfbuzz" "libpng" "libwebp"))
> +                  ;; Configure with gn.
> +                  (invoke "gn" "gen" "out/Debug"
> +                          (string-append
> +                           "--args="
> +                           "cc=\"gcc\" "              ;defaults to 'cc'
> +                           "skia_compile_sksl_tests=false " ; disable some 
> tests
> +                           "skia_use_system_expat=true " ; use system expat 
> library
> +                           ;; Specify where to locate the includes.
> +                           "extra_cflags=["
> +                           (string-join
> +                            (map
> +                             (lambda (lib)
> +                               (string-append
> +                                "\"-I"
> +                                (search-input-directory
> +                                 inputs
> +                                 (string-append "include/" lib)) "\""))
> +                             '("harfbuzz"
> +                               "freetype2"
> +                               "spirv-tools"
> +                               "spirv"
> +                               "unicode"))
> +                            ",")
> +                           "] "
> +                           ;; Otherwise the validate-runpath phase fails.
> +                           "extra_ldflags=["
> +                           "\"-Wl,-rpath=" #$output "/lib\""
> +                           "] "
> +                           ;; Disabled, otherwise the build system attempts 
> to
> +                           ;; download the SDK at build time.
> +                           "skia_use_dng_sdk=false "
> +                           "skia_use_runtime_icu=true "))
> +                  ;; Build dm testing tool.
> +                  (symlink
> +                   (string-append #$(this-package-native-input "gn") 
> "/bin/gn")
> +                   "./bin/gn")
> +                  (invoke "ninja" "-C" "out/Debug" "dm")
> +                  ;; Test require an X server.

"The test suite requires ..."

> +                  (let ((xvfb (search-input-file (or native-inputs inputs)
> +                                                 "bin/Xvfb"))
> +                        (display ":1"))
> +                    (setenv "DISPLAY" display)
> +                    (system (string-append xvfb " " display " &")))
> +                  ;; Run tests.
> +                  (invoke "out/Debug/dm" "-v"
> +                          "-w" "dm_output"
> +                          "--codecWritePath" "dm_output"
> +                          "--simpleCodec"
> +                          "--skip"
> +                          ;; These tests fail with segmentation fault.

Upstream may be interested to know/have a solution.  I'd try submitting
a ticket to them, and link it here for future reference.

> +                          "_" "_" "_" "Codec_trunc"
> +                          "_" "_" "_" "AnimCodecPlayer"
> +                          "_" "_" "_" "Codec_partialAnim"
> +                          "_" "_" "_" "Codec_InvalidImages"
> +                          "_" "_" "_" "Codec_GifInterlacedTruncated"
> +                          "_" "_" "_" "SkText_UnicodeText_Flags"
> +                          "_" "_" "_" "SkParagraph_FontStyle"
> +                          "_" "_" "_" "flight_animated_image"

What are these underscores?  I don't know the 'dm' tool; a comment would
be nice.

> +                          ;; These tests fail because of Codec/Sk failure.
> +                          "_" "_" "_" "AndroidCodec_computeSampleSize"
> +                          "_" "_" "_" "AnimatedImage_invalidCrop"
> +                          "_" "_" "_" "AnimatedImage_scaled"
> +                          "_" "_" "_" "AnimatedImage_copyOnWrite"
> +                          "_" "_" "_" "AnimatedImage"
> +                          "_" "_" "_" "BRD_types"
> +                          "_" "_" "_" "Codec_frames"
> +                          "_" "_" "_" "Codec_partial"
> +                          "_" "_" "_" "Codec_partialWuffs"
> +                          "_" "_" "_" "Codec_requiredFrame"
> +                          "_" "_" "_" "Codec_rewind"
> +                          "_" "_" "_" "Codec_incomplete"
> +                          "_" "_" "_" "Codec_InvalidAnimated"
> +                          "_" "_" "_" "Codec_ossfuzz6274"
> +                          "_" "_" "_" "Codec_gif_out_of_palette"
> +                          "_" "_" "_" "Codec_xOffsetTooBig"
> +                          "_" "_" "_" "Codec_gif"
> +                          "_" "_" "_" "Codec_skipFullParse"
> +                          "_" "_" "_" "AndroidCodec_animated_gif"
> +                          ;; Other failures

"These fail for unknown reasons".  It's typically nice to forward these
failures to upstream with the ticket URL referenced here.  Often
upstream can help explain them or find solutions.

> +                          "_" "_" "_" "Gif"
> +                          "_" "_" "_" "Wuffs_seek_and_decode"
> +                          "_" "_" "_" "Skottie_Shaper_ExplicitFontMgr"
> +                          "8888" "skp" "_" "_"
> +                          "8888" "lottie" "_" "_"
> +                          "gl" "skp" "_" "_"
> +                          "gl" "lottie" "_" "_"
> +                          "_" "_" "_" "ES2BlendWithNoTexture")))))))
> +  (native-inputs (list gn libjpeg-turbo ninja pkg-config python-wrapper
> +                       spirv-tools-for-skia spirv-headers-for-skia
> +                       icu4c-for-skia glu xorg-server-for-tests))
> +  (inputs (list expat fontconfig freetype harfbuzz mesa libwebp zlib))
> +  (home-page "https://skia.org/";)
> +  (synopsis "2D graphics library")
> +  (description
> +   "Skia is a 2D graphics library for drawing text, geometries, and images.
>  It supports:
>  @itemize
>  @item 3x3 matrices with perspective
> @@ -2099,7 +2219,7 @@ (define-public skia
>  @item shaders, xfermodes, maskfilters, patheffects
>  @item subpixel text
>  @end itemize")
> -      (license license:bsd-3))))
> +  (license license:bsd-3))))

Wooh!  That must have taken some time to refine!  Thanks a lot for the
efforts!  I think with the suggestions made above, a v3 would be ready
to roll.

-- 
Thanks,
Maxim