activating tests for skia 2D graphics library

Open

Details

2 participants

Maxim Cournoyer
Nicolas Graves

Owner: unassigned

Submitted by: Nicolas Graves

Severity: normal

Nicolas Graves wrote on 5 Jan 11:02 +0100

Recipients:(address . guix-patches@gnu.org)

Message-ID:87pmbtfge5.fsf@ngraves.fr

Hi guix!

I have a series of patches to activate tests on the SKIA graphics 2D

library.

Originally my goal was to add skia as a dependency to libreoffice and

remove the --without-skia flag, but I didn't want to do that before

being able to test the library change.

It took me quite a while since google uses its own tools in a way that

is hard to package in guix.

There are still some disabled tests that I'm not able to fix myself, but

at least it's testable for most tests. Help and review welcome, at least

one function fails with segmentation fault, but I'm not able to debug

it.

Best regards,

Nicolas Graves

Nicolas Graves wrote on 5 Jan 13:18 +0100

[PATCH 1/4] gnu: Add spirv-headers-for-skia.

Recipients:(address . 60571@debbugs.gnu.org)(address . ngraves@ngraves.fr)

Message-ID:20230105121842.18662-1-ngraves@ngraves.fr

* gnu/packages/vulkan.scm (spirv-headers-for-skia): New variable.

---

gnu/packages/vulkan.scm | 17 +++++++++++++++++

1 file changed, 17 insertions(+)

Toggle diff (30 lines)diff --git a/gnu/packages/vulkan.scm b/gnu/packages/vulkan.scm
index a2db5511d5..8895d8a5cf 100644
--- a/gnu/packages/vulkan.scm
+++ b/gnu/packages/vulkan.scm
@@ -74,6 +74,23 @@ (define-public spirv-headers
               (string-append "https://github.com/KhronosGroup/SPIRV-Headers/blob/"
                              version "/LICENSE")))))
 
+(define-public spirv-headers-for-skia
+  (let ((commit "814e728b30ddd0f4509233099a3ad96fd4318c07")
+        (revision "0"))
+    (package
+      (inherit spirv-headers)
+      (name "spirv-headers-for-skia")
+      (version "skia")
+      (source
+       (origin
+         (method git-fetch)
+         (uri (git-reference
+               (url "https://skia.googlesource.com/external/github.com/KhronosGroup/SPIRV-Headers.git")
+               (commit commit)))
+         (file-name (git-file-name name version))
+         (sha256
+          (base32 "0v6ycgfxh9d2gzhxrnxgrn5gyg2cshg55767qdg46px8412j5lbi")))))))
+
 (define-public spirv-tools
   (package
     (name "spirv-tools")
-- 
2.38.1

Nicolas Graves wrote on 5 Jan 13:18 +0100

[PATCH 2/4] gnu: Add spirv-tools-for-skia.

Recipients:(address . 60571@debbugs.gnu.org)(address . ngraves@ngraves.fr)

Message-ID:20230105121842.18662-2-ngraves@ngraves.fr

* gnu/packages/vulkan.scm (spirv-tools-for-skia): New variable.

---

gnu/packages/vulkan.scm | 25 +++++++++++++++++++++++++

1 file changed, 25 insertions(+)

Toggle diff (38 lines)diff --git a/gnu/packages/vulkan.scm b/gnu/packages/vulkan.scm
index 8895d8a5cf..231fa2e281 100644
--- a/gnu/packages/vulkan.scm
+++ b/gnu/packages/vulkan.scm
@@ -120,6 +120,31 @@ (define-public spirv-tools
 parser,disassembler, validator, and optimizer for SPIR-V.")
     (license license:asl2.0)))
 
+(define-public spirv-tools-for-skia
+  (let ((commit "4b092d2ab81854e61632bdd1e658907f0071c37e")
+        (revision "0"))
+    (package
+      (inherit spirv-tools)
+      (name "spirv-tools-for-skia")
+      (version "skia")
+      (source
+       (origin
+         (method git-fetch)
+         (uri (git-reference
+               (url "https://skia.googlesource.com/external/github.com/KhronosGroup/SPIRV-Tools.git")
+               (commit commit)))
+         (file-name (git-file-name name version))
+         (sha256
+          (base32 "0dqdybkxkwjiwwhxf0p3p1408wrqvwysymq7c9rzd1a2220y6lry"))))
+      (inputs (list spirv-headers-for-skia))
+      (native-inputs (list pkg-config python-wrapper))
+      (arguments
+       (list
+        #:configure-flags `(list "-DBUILD_SHARED_LIBS=ON"
+                                 (string-append
+                                  "-DSPIRV-Headers_SOURCE_DIR="
+                                  (assoc-ref %build-inputs "spirv-headers-for-skia"))))))))
+
 (define-public spirv-cross
   (package
     (name "spirv-cross")
-- 
2.38.1

Nicolas Graves wrote on 5 Jan 13:18 +0100

[PATCH 3/4] gnu: Add icu4c-for-skia.

Recipients:(address . 60571@debbugs.gnu.org)(address . ngraves@ngraves.fr)

Message-ID:20230105121842.18662-3-ngraves@ngraves.fr

* gnu/packages/icu4c.scm (icu4c-for-skia): New variable.

---

gnu/packages/icu4c.scm | 99 ++++++++++++++++++++++++++++++++++++++++++

1 file changed, 99 insertions(+)

Toggle diff (124 lines)diff --git a/gnu/packages/icu4c.scm b/gnu/packages/icu4c.scm
index 1e4f66d956..05911addcd 100644
--- a/gnu/packages/icu4c.scm
+++ b/gnu/packages/icu4c.scm
@@ -27,14 +27,17 @@
 
 (define-module (gnu packages icu4c)
   #:use-module (gnu packages)
+  #:use-module (gnu packages cpio)
   #:use-module (gnu packages java)
   #:use-module (gnu packages perl)
+  #:use-module (gnu packages pkg-config)
   #:use-module (gnu packages python)
   #:use-module (guix gexp)
   #:use-module (guix licenses)
   #:use-module (guix packages)
   #:use-module (guix utils)
   #:use-module (guix download)
+  #:use-module (guix git-download)
   #:use-module (guix build-system ant)
   #:use-module (guix build-system gnu))
 
@@ -243,3 +246,99 @@ (define-public java-icu4j
 globalisation support for software applications.  This package contains the
 Java part.")
     (license x11)))
+
+(define-public icu4c-for-skia
+  (let ((commit "a0718d4f121727e30b8d52c7a189ebf5ab52421f")
+        (revision "0"))
+    (package
+      (inherit icu4c)
+      (name "icu4c-for-skia")
+      (version "skia")
+      (source
+       (origin
+         (method git-fetch)
+         (uri (git-reference
+               (url "https://chromium.googlesource.com/chromium/deps/icu.git")
+               (commit commit)))
+         (file-name (git-file-name name version))
+         (sha256
+          (base32 "1qxws2p91f6dmhy7d3967r5ygz06r88pkmpm97px067x0zzdz384"))))
+      (native-inputs (list python cpio pkg-config))
+      (arguments
+       (list
+        #:make-flags #~`(,(string-append "DESTDIR=" #$output))
+        #:configure-flags #~'(;;"--enable-shared=no" "--enable-static=yes"
+                              "--prefix=" "--exec-prefix=")
+        #:phases
+        #~(modify-phases %standard-phases
+            (add-after 'unpack 'chdir-to-source
+              (lambda _ (chdir "source")))
+            (replace 'configure
+              (lambda* (#:key inputs parallel-build? configure-flags #:allow-other-keys)
+                (let ((bash (search-input-file inputs "/bin/sh")))
+                  ;; Replace bash executable.
+                  (setenv "CONFIG_SHELL" bash)
+                  (substitute* "./configure"
+                    (("`/bin/sh")
+                     (string-append "`" bash)))
+                  ;; Make the static library position-independent.
+                  ;; (substitute* "./icudefs.mk.in"
+                    ;; (("^CFLAGS = ")
+                     ;; "CFLAGS = -fPIC ")
+                    ;; (("^CXXFLAGS = ")
+                     ;; "CXXFLAGS = -fPIC "))
+                  ;; Update runpath.
+                  (substitute* "./icudefs.mk.in"
+                    (("= -L\\$\\(LIBDIR\\)")
+                     (string-append "= -L$(LIBDIR)"
+                                    " -Wl,-rpath=\"" #$output "/lib\"")))
+                  ;; Set prefix and exec-prefix.
+                  (substitute* "./runConfigureICU"
+                    (("^OPTS=")
+                     (string-append "OPTS=\""
+                                    (string-join configure-flags " ")
+                                    "\"")))
+                  ;; Configure.
+                  (invoke "./runConfigureICU" "Linux/gcc"
+                          "--disable-layout" "--disable-tests"))))
+            (add-after 'install 'configure-filtered-data
+              (lambda* (#:key make-flags configure-flags #:allow-other-keys)
+                ;; Cleanup.
+                (with-directory-excursion "data"
+                  `(,invoke "make" "clean" ,@make-flags))
+                ;; Set prefix and exec-prefix.
+                (substitute* "./runConfigureICU"
+                  (("^OPTS=")
+                   (string-append
+                    "OPTS=\"" (string-join configure-flags " ") "\"")))
+                ;; Configure for common data.
+                (setenv "ICU_DATA_FILTER_FILE"
+                        (string-append (getcwd) "/../filters/common.json"))
+                (invoke "./runConfigureICU" "Linux/gcc"
+                        "--disable-layout" "--disable-tests")))
+            (add-after 'configure-filtered-data 'build-filtered-data
+              (lambda* (#:key parallel-build? make-flags #:allow-other-keys)
+                (let ((job-count (if parallel-build?
+                                     (number->string (parallel-job-count))
+                                     "1")))
+                  `(,invoke "make" "-j" ,job-count ,@make-flags)
+                  (setenv "DESTDIR" #$output)
+                  (invoke "bash" "../scripts/copy_data.sh" "common"))))
+            (add-after 'build-filtered-data 'install-scripts-and-data
+              (lambda _
+                (let* ((share (string-append #$output "/share"))
+                       (scripts (string-append share "/scripts"))
+                       (data (string-append share "/data/common")))
+                  ;; Install scripts.
+                  (mkdir-p scripts)
+                  (copy-recursively "../scripts/" scripts)
+                  ;; Install data.
+                  (mkdir-p data)
+                  (copy-recursively "./dataout/common/data/out/tmp" data)
+                  (symlink (string-append data "/icudt69l.dat")
+                           (string-append data "/icudtl.dat")))))
+            (add-before 'check 'disable-failing-uconv-test
+              (lambda _
+                (substitute* "extra/uconv/Makefile.in"
+                  (("check: check-local")
+                   ""))))))))))
-- 
2.38.1

Nicolas Graves wrote on 5 Jan 13:18 +0100

[PATCH 4/4] gnu: skia: Activate tests.

Recipients:(address . 60571@debbugs.gnu.org)(address . ngraves@ngraves.fr)

Message-ID:20230105121842.18662-4-ngraves@ngraves.fr

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

---

gnu/packages/graphics.scm | 144 ++++++++++++++++++++++++++++++++++----

1 file changed, 132 insertions(+), 12 deletions(-)

Toggle diff (178 lines)diff --git a/gnu/packages/graphics.scm b/gnu/packages/graphics.scm
index f23fad7c50..2392a63d33 100644
--- a/gnu/packages/graphics.scm
+++ b/gnu/packages/graphics.scm
@@ -80,6 +80,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)
@@ -1876,10 +1877,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
@@ -1944,13 +1941,136 @@ (define skia.pc (string-append #$output
 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.
+                  (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\" ]"))
+                  ;; Enable system libraries without is_official_build=true.
+                  ;; Necessary because is_official_build prevents building dm.
+                  (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.
+                  (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.
+                          "_" "_" "_" "Codec_trunc"
+                          "_" "_" "_" "AnimCodecPlayer"
+                          "_" "_" "_" "Codec_partialAnim"
+                          "_" "_" "_" "Codec_InvalidImages"
+                          "_" "_" "_" "Codec_GifInterlacedTruncated"
+                          "_" "_" "_" "SkText_UnicodeText_Flags"
+                          "_" "_" "_" "SkParagraph_FontStyle"
+                          "_" "_" "_" "flight_animated_image"
+                          ;; 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
+                          "_" "_" "_" "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
@@ -1958,7 +2078,7 @@ (define skia.pc (string-append #$output
 @item shaders, xfermodes, maskfilters, patheffects
 @item subpixel text
 @end itemize")
-      (license license:bsd-3))))
+  (license license:bsd-3))))
 
 (define-public superfamiconv
   (package
-- 
2.38.1

Maxim Cournoyer wrote on 24 Mar 04:15 +0100

Re: bug#60571: activating tests for skia 2D graphics library

Recipients:(name . Nicolas Graves)(address . ngraves@ngraves.fr)(address . 60571@debbugs.gnu.org)

Message-ID:87fs9u3kne.fsf_-_@gmail.com

Hello,

Nicolas Graves <ngraves@ngraves.fr> writes:

Toggle quote (15 lines)> * gnu/packages/vulkan.scm (spirv-headers-for-skia): New variable.
> ---
>  gnu/packages/vulkan.scm | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/gnu/packages/vulkan.scm b/gnu/packages/vulkan.scm
> index a2db5511d5..8895d8a5cf 100644
> --- a/gnu/packages/vulkan.scm
> +++ b/gnu/packages/vulkan.scm
> @@ -74,6 +74,23 @@ (define-public spirv-headers
>                (string-append "https://github.com/KhronosGroup/SPIRV-Headers/blob/"
>                               version "/LICENSE")))))
>  
> +(define-public spirv-headers-for-skia

Could you please add a comment here about why we use this commit, and

not a release, e.g. ("There is no release; use the latest commit.")

Toggle quote (7 lines)

> + (let ((commit "814e728b30ddd0f4509233099a3ad96fd4318c07")

> + (revision "0"))

> + (package

> + (inherit spirv-headers)

> + (name "spirv-headers-for-skia")

> + (version "skia")

That's not OK as a version. See if there is a version string in the

source, and use that else 0.0.0, combined wih revision and commit via

the 'git-version' procedure.

Thanks for working on it!

Thanks,

Maxim

Maxim Cournoyer wrote on 24 Mar 04:20 +0100

Recipients:(name . Nicolas Graves)(address . ngraves@ngraves.fr)(address . 60571@debbugs.gnu.org)

Message-ID:87bkki3ked.fsf_-_@gmail.com

Hello,

Nicolas Graves <ngraves@ngraves.fr> writes:

Toggle quote (15 lines)> * gnu/packages/vulkan.scm (spirv-tools-for-skia): New variable.
> ---
>  gnu/packages/vulkan.scm | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>
> diff --git a/gnu/packages/vulkan.scm b/gnu/packages/vulkan.scm
> index 8895d8a5cf..231fa2e281 100644
> --- a/gnu/packages/vulkan.scm
> +++ b/gnu/packages/vulkan.scm
> @@ -120,6 +120,31 @@ (define-public spirv-tools
>  parser,disassembler, validator, and optimizer for SPIR-V.")
>      (license license:asl2.0)))
>  
> +(define-public spirv-tools-for-skia

Same comment as earlier.

Toggle quote (7 lines)

> + (let ((commit "4b092d2ab81854e61632bdd1e658907f0071c37e")

> + (revision "0"))

> + (package

> + (inherit spirv-tools)

> + (name "spirv-tools-for-skia")

> + (version "skia")

Same comment as earlier.

Toggle quote (18 lines)> +      (source
> +       (origin
> +         (method git-fetch)
> +         (uri (git-reference
> +               (url "https://skia.googlesource.com/external/github.com/KhronosGroup/SPIRV-Tools.git")
> +               (commit commit)))
> +         (file-name (git-file-name name version))
> +         (sha256
> +          (base32 "0dqdybkxkwjiwwhxf0p3p1408wrqvwysymq7c9rzd1a2220y6lry"))))
> +      (inputs (list spirv-headers-for-skia))
> +      (native-inputs (list pkg-config python-wrapper))
> +      (arguments
> +       (list
> +        #:configure-flags `(list "-DBUILD_SHARED_LIBS=ON"
> +                                 (string-append
> +                                  "-DSPIRV-Headers_SOURCE_DIR="
> +                                  (assoc-ref %build-inputs "spirv-headers-for-skia"))))))))

Hm.  If the base package used somethig like search-input-directory this
could be avoided.

-- 
Thanks,
Maxim

Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 60571@debbugs.gnu.org

is:open	open issues
is:done	closed issues
submitter:<who>	search issue submitter
author:<who>	search by message author
date:yesterday..now	search by issue date
mdate:3m..2d	search by message date