activating tests for skia 2D graphics library

  • Open
  • quality assurance status badge
Details
2 participants
  • Maxim Cournoyer
  • Nicolas Graves
Owner
unassigned
Submitted by
Nicolas Graves
Severity
normal
N
N
Nicolas Graves wrote on 5 Jan 11:02 +0100
(address . guix-patches@gnu.org)
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
N
N
Nicolas Graves wrote on 5 Jan 13:18 +0100
[PATCH 1/4] gnu: Add spirv-headers-for-skia.
(address . 60571@debbugs.gnu.org)(address . ngraves@ngraves.fr)
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
N
N
Nicolas Graves wrote on 5 Jan 13:18 +0100
[PATCH 2/4] gnu: Add spirv-tools-for-skia.
(address . 60571@debbugs.gnu.org)(address . ngraves@ngraves.fr)
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
N
N
Nicolas Graves wrote on 5 Jan 13:18 +0100
[PATCH 3/4] gnu: Add icu4c-for-skia.
(address . 60571@debbugs.gnu.org)(address . ngraves@ngraves.fr)
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
N
N
Nicolas Graves wrote on 5 Jan 13:18 +0100
[PATCH 4/4] gnu: skia: Activate tests.
(address . 60571@debbugs.gnu.org)(address . ngraves@ngraves.fr)
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
M
M
Maxim Cournoyer wrote on 24 Mar 04:15 +0100
Re: bug#60571: activating tests for skia 2D graphics library
(name . Nicolas Graves)(address . ngraves@ngraves.fr)(address . 60571@debbugs.gnu.org)
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
M
M
Maxim Cournoyer wrote on 24 Mar 04:20 +0100
(name . Nicolas Graves)(address . ngraves@ngraves.fr)(address . 60571@debbugs.gnu.org)
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
?