unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
* [bug#60571] activating tests for skia 2D graphics library
@ 2023-01-05 10:02 Nicolas Graves via Guix-patches via
  2023-01-05 12:18 ` [bug#60571] [PATCH 1/4] gnu: Add spirv-headers-for-skia Nicolas Graves via Guix-patches via
                   ` (3 more replies)
  0 siblings, 4 replies; 26+ messages in thread
From: Nicolas Graves via Guix-patches via @ 2023-01-05 10:02 UTC (permalink / raw)
  To: 60571


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




^ permalink raw reply	[flat|nested] 26+ messages in thread

* [bug#60571] [PATCH 1/4] gnu: Add spirv-headers-for-skia.
  2023-01-05 10:02 [bug#60571] activating tests for skia 2D graphics library Nicolas Graves via Guix-patches via
@ 2023-01-05 12:18 ` Nicolas Graves via Guix-patches via
  2023-01-05 12:18   ` [bug#60571] [PATCH 2/4] gnu: Add spirv-tools-for-skia Nicolas Graves via Guix-patches via
                     ` (3 more replies)
  2023-08-27 12:53 ` [bug#60571] [PATCH v2 1/4] gnu: Add spirv-headers-for-skia Nicolas Graves via Guix-patches via
                   ` (2 subsequent siblings)
  3 siblings, 4 replies; 26+ messages in thread
From: Nicolas Graves via Guix-patches via @ 2023-01-05 12:18 UTC (permalink / raw)
  To: 60571; +Cc: ngraves

* 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
+  (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





^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [bug#60571] [PATCH 2/4] gnu: Add spirv-tools-for-skia.
  2023-01-05 12:18 ` [bug#60571] [PATCH 1/4] gnu: Add spirv-headers-for-skia Nicolas Graves via Guix-patches via
@ 2023-01-05 12:18   ` Nicolas Graves via Guix-patches via
  2023-03-24  3:20     ` [bug#60571] activating tests for skia 2D graphics library Maxim Cournoyer
  2023-01-05 12:18   ` [bug#60571] [PATCH 3/4] gnu: Add icu4c-for-skia Nicolas Graves via Guix-patches via
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 26+ messages in thread
From: Nicolas Graves via Guix-patches via @ 2023-01-05 12:18 UTC (permalink / raw)
  To: 60571; +Cc: ngraves

* 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
+  (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





^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [bug#60571] [PATCH 3/4] gnu: Add icu4c-for-skia.
  2023-01-05 12:18 ` [bug#60571] [PATCH 1/4] gnu: Add spirv-headers-for-skia Nicolas Graves via Guix-patches via
  2023-01-05 12:18   ` [bug#60571] [PATCH 2/4] gnu: Add spirv-tools-for-skia Nicolas Graves via Guix-patches via
@ 2023-01-05 12:18   ` Nicolas Graves via Guix-patches via
  2023-01-05 12:18   ` [bug#60571] [PATCH 4/4] gnu: skia: Activate tests Nicolas Graves via Guix-patches via
  2023-03-24  3:15   ` [bug#60571] activating tests for skia 2D graphics library Maxim Cournoyer
  3 siblings, 0 replies; 26+ messages in thread
From: Nicolas Graves via Guix-patches via @ 2023-01-05 12:18 UTC (permalink / raw)
  To: 60571; +Cc: ngraves

* gnu/packages/icu4c.scm (icu4c-for-skia): New variable.
---
 gnu/packages/icu4c.scm | 99 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 99 insertions(+)

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





^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [bug#60571] [PATCH 4/4] gnu: skia: Activate tests.
  2023-01-05 12:18 ` [bug#60571] [PATCH 1/4] gnu: Add spirv-headers-for-skia Nicolas Graves via Guix-patches via
  2023-01-05 12:18   ` [bug#60571] [PATCH 2/4] gnu: Add spirv-tools-for-skia Nicolas Graves via Guix-patches via
  2023-01-05 12:18   ` [bug#60571] [PATCH 3/4] gnu: Add icu4c-for-skia Nicolas Graves via Guix-patches via
@ 2023-01-05 12:18   ` Nicolas Graves via Guix-patches via
  2023-03-24  3:15   ` [bug#60571] activating tests for skia 2D graphics library Maxim Cournoyer
  3 siblings, 0 replies; 26+ messages in thread
From: Nicolas Graves via Guix-patches via @ 2023-01-05 12:18 UTC (permalink / raw)
  To: 60571; +Cc: ngraves

* gnu/packages/graphics.scm (skia): Activate tests.
---
 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 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





^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [bug#60571] activating tests for skia 2D graphics library
  2023-01-05 12:18 ` [bug#60571] [PATCH 1/4] gnu: Add spirv-headers-for-skia Nicolas Graves via Guix-patches via
                     ` (2 preceding siblings ...)
  2023-01-05 12:18   ` [bug#60571] [PATCH 4/4] gnu: skia: Activate tests Nicolas Graves via Guix-patches via
@ 2023-03-24  3:15   ` Maxim Cournoyer
  3 siblings, 0 replies; 26+ messages in thread
From: Maxim Cournoyer @ 2023-03-24  3:15 UTC (permalink / raw)
  To: Nicolas Graves; +Cc: 60571

Hello,

Nicolas Graves <ngraves@ngraves.fr> writes:

> * 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.")

> +  (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




^ permalink raw reply	[flat|nested] 26+ messages in thread

* [bug#60571] activating tests for skia 2D graphics library
  2023-01-05 12:18   ` [bug#60571] [PATCH 2/4] gnu: Add spirv-tools-for-skia Nicolas Graves via Guix-patches via
@ 2023-03-24  3:20     ` Maxim Cournoyer
  0 siblings, 0 replies; 26+ messages in thread
From: Maxim Cournoyer @ 2023-03-24  3:20 UTC (permalink / raw)
  To: Nicolas Graves; +Cc: 60571

Hello,

Nicolas Graves <ngraves@ngraves.fr> writes:

> * 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.

> +  (let ((commit "4b092d2ab81854e61632bdd1e658907f0071c37e")
> +        (revision "0"))
> +    (package
> +      (inherit spirv-tools)
> +      (name "spirv-tools-for-skia")
> +      (version "skia")

Same comment as earlier.

> +      (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




^ permalink raw reply	[flat|nested] 26+ messages in thread

* [bug#60571] [PATCH v2 1/4] gnu: Add spirv-headers-for-skia.
  2023-01-05 10:02 [bug#60571] activating tests for skia 2D graphics library Nicolas Graves via Guix-patches via
  2023-01-05 12:18 ` [bug#60571] [PATCH 1/4] gnu: Add spirv-headers-for-skia Nicolas Graves via Guix-patches via
@ 2023-08-27 12:53 ` Nicolas Graves via Guix-patches via
  2023-08-27 12:53   ` [bug#60571] [PATCH v2 2/4] gnu: Add spirv-tools-for-skia Nicolas Graves via Guix-patches via
                     ` (3 more replies)
  2023-08-29 21:11 ` [bug#60571] [PATCH v3 1/3] gnu: Add icu4c-for-skia Nicolas Graves via Guix-patches via
  2023-09-05 15:07 ` [bug#60571] [PATCH v4 1/3] gnu: skia: Update to 110.0.0f3fb7a Nicolas Graves via Guix-patches via
  3 siblings, 4 replies; 26+ messages in thread
From: Nicolas Graves via Guix-patches via @ 2023-08-27 12:53 UTC (permalink / raw)
  To: 60571; +Cc: ngraves, maxim.cournoyer

* gnu/packages/vulkan.scm (spirv-headers-for-skia): New variable.
(%vulkan-sdk-skia-version): New variable.
---
 gnu/packages/vulkan.scm | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/gnu/packages/vulkan.scm b/gnu/packages/vulkan.scm
index 1d2e58f1d4..da83417dba 100644
--- a/gnu/packages/vulkan.scm
+++ b/gnu/packages/vulkan.scm
@@ -46,6 +46,9 @@ (define-module (gnu packages vulkan)
 
 ;; Note: Remember to change vulkan-loader version when bumping this.
 (define %vulkan-sdk-version "sdk-1.3.231.1")
+;; Note: The current skia version is lagging behind vulkan's.
+;; Use this variable until it has catched up.
+(define %vulkan-sdk-skia-version "sdk-1.2.198.0")
 
 (define-public spirv-headers
   (package
@@ -79,6 +82,21 @@ (define-public spirv-headers
               (string-append "https://github.com/KhronosGroup/SPIRV-Headers/blob/"
                              version "/LICENSE")))))
 
+(define-public spirv-headers-for-skia
+  (package
+    (inherit spirv-headers)
+    (name "spirv-headers-for-skia")
+    (version %vulkan-sdk-skia-version)
+    (source
+     (origin
+       (method git-fetch)
+       (uri (git-reference
+             (url "https://skia.googlesource.com/external/github.com/KhronosGroup/SPIRV-Headers.git")
+             (commit version)))
+       (file-name (git-file-name name version))
+       (sha256
+        (base32 "0v6ycgfxh9d2gzhxrnxgrn5gyg2cshg55767qdg46px8412j5lbi"))))))
+
 (define-public spirv-tools
   (package
     (name "spirv-tools")

base-commit: 5c1be661b988018e3de32a0ce529e1b58209ebb6
-- 
2.41.0





^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [bug#60571] [PATCH v2 2/4] gnu: Add spirv-tools-for-skia.
  2023-08-27 12:53 ` [bug#60571] [PATCH v2 1/4] gnu: Add spirv-headers-for-skia Nicolas Graves via Guix-patches via
@ 2023-08-27 12:53   ` Nicolas Graves via Guix-patches via
  2023-08-28 15:29     ` Maxim Cournoyer
  2023-08-27 12:53   ` [bug#60571] [PATCH v2 3/4] gnu: Add icu4c-for-skia Nicolas Graves via Guix-patches via
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 26+ messages in thread
From: Nicolas Graves via Guix-patches via @ 2023-08-27 12:53 UTC (permalink / raw)
  To: 60571; +Cc: ngraves, maxim.cournoyer

* gnu/packages/vulkan.scm (spirv-tools-for-skia): New variable.
---
 gnu/packages/vulkan.scm | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/gnu/packages/vulkan.scm b/gnu/packages/vulkan.scm
index da83417dba..9f02b1d235 100644
--- a/gnu/packages/vulkan.scm
+++ b/gnu/packages/vulkan.scm
@@ -129,6 +129,29 @@ (define-public spirv-tools
 parser,disassembler, validator, and optimizer for SPIR-V.")
     (license license:asl2.0)))
 
+(define-public spirv-tools-for-skia
+  (package
+    (inherit spirv-tools)
+    (name "spirv-tools-for-skia")
+    (version %vulkan-sdk-skia-version)
+    (source
+     (origin
+       (method git-fetch)
+       (uri (git-reference
+             (url "https://skia.googlesource.com/external/github.com/KhronosGroup/SPIRV-Tools.git")
+             (commit version)))
+       (file-name (git-file-name name version))
+       (sha256
+        (base32 "0cp1amgylflh36nw3sy0cy7pf9bhhzykk3vxqwmgnxiryr65nhph"))))
+    (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.41.0





^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [bug#60571] [PATCH v2 3/4] gnu: Add icu4c-for-skia.
  2023-08-27 12:53 ` [bug#60571] [PATCH v2 1/4] gnu: Add spirv-headers-for-skia Nicolas Graves via Guix-patches via
  2023-08-27 12:53   ` [bug#60571] [PATCH v2 2/4] gnu: Add spirv-tools-for-skia Nicolas Graves via Guix-patches via
@ 2023-08-27 12:53   ` Nicolas Graves via Guix-patches via
  2023-08-28 15:48     ` Maxim Cournoyer
  2023-08-27 12:53   ` [bug#60571] [PATCH v2 4/4] gnu: skia: Activate tests Nicolas Graves via Guix-patches via
  2023-08-28 15:27   ` [bug#60571] [PATCH v2 1/4] gnu: Add spirv-headers-for-skia Maxim Cournoyer
  3 siblings, 1 reply; 26+ messages in thread
From: Nicolas Graves via Guix-patches via @ 2023-08-27 12:53 UTC (permalink / raw)
  To: 60571; +Cc: ngraves, maxim.cournoyer

* gnu/packages/icu4c.scm (icu4c-for-skia): New variable.
---
 gnu/packages/icu4c.scm | 101 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 101 insertions(+)

diff --git a/gnu/packages/icu4c.scm b/gnu/packages/icu4c.scm
index ba8b4915f2..eef8bd2cd3 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))
 
@@ -240,3 +243,101 @@ (define-public java-icu4j
 globalisation support for software applications.  This package contains the
 Java part.")
     (license x11)))
+
+(define-public icu4c-for-skia
+  ;; The current version of skia needs commit-precise icu4c
+  ;; version for its test-dependencies.
+  (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.41.0





^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [bug#60571] [PATCH v2 4/4] gnu: skia: Activate tests.
  2023-08-27 12:53 ` [bug#60571] [PATCH v2 1/4] gnu: Add spirv-headers-for-skia Nicolas Graves via Guix-patches via
  2023-08-27 12:53   ` [bug#60571] [PATCH v2 2/4] gnu: Add spirv-tools-for-skia Nicolas Graves via Guix-patches via
  2023-08-27 12:53   ` [bug#60571] [PATCH v2 3/4] gnu: Add icu4c-for-skia Nicolas Graves via Guix-patches via
@ 2023-08-27 12:53   ` Nicolas Graves via Guix-patches via
  2023-08-28 15:58     ` Maxim Cournoyer
  2023-08-28 15:27   ` [bug#60571] [PATCH v2 1/4] gnu: Add spirv-headers-for-skia Maxim Cournoyer
  3 siblings, 1 reply; 26+ messages in thread
From: Nicolas Graves via Guix-patches via @ 2023-08-27 12:53 UTC (permalink / raw)
  To: 60571; +Cc: ngraves, maxim.cournoyer

* gnu/packages/graphics.scm (skia): Activate tests.
---
 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.
+                  (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
@@ -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))))
 
 (define-public superfamiconv
   (package
-- 
2.41.0





^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [bug#60571] [PATCH v2 1/4] gnu: Add spirv-headers-for-skia.
  2023-08-27 12:53 ` [bug#60571] [PATCH v2 1/4] gnu: Add spirv-headers-for-skia Nicolas Graves via Guix-patches via
                     ` (2 preceding siblings ...)
  2023-08-27 12:53   ` [bug#60571] [PATCH v2 4/4] gnu: skia: Activate tests Nicolas Graves via Guix-patches via
@ 2023-08-28 15:27   ` Maxim Cournoyer
  3 siblings, 0 replies; 26+ messages in thread
From: Maxim Cournoyer @ 2023-08-28 15:27 UTC (permalink / raw)
  To: Nicolas Graves; +Cc: 60571

Hi,

Nicolas Graves <ngraves@ngraves.fr> writes:

> * gnu/packages/vulkan.scm (spirv-headers-for-skia): New variable.
> (%vulkan-sdk-skia-version): New variable.
> ---
>  gnu/packages/vulkan.scm | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>
> diff --git a/gnu/packages/vulkan.scm b/gnu/packages/vulkan.scm
> index 1d2e58f1d4..da83417dba 100644
> --- a/gnu/packages/vulkan.scm
> +++ b/gnu/packages/vulkan.scm
> @@ -46,6 +46,9 @@ (define-module (gnu packages vulkan)
>  
>  ;; Note: Remember to change vulkan-loader version when bumping this.
>  (define %vulkan-sdk-version "sdk-1.3.231.1")
> +;; Note: The current skia version is lagging behind vulkan's.
> +;; Use this variable until it has catched up.

"caught up".

> +(define %vulkan-sdk-skia-version "sdk-1.2.198.0")
>  
>  (define-public spirv-headers
>    (package
> @@ -79,6 +82,21 @@ (define-public spirv-headers
>                (string-append "https://github.com/KhronosGroup/SPIRV-Headers/blob/"
>                               version "/LICENSE")))))
>  
> +(define-public spirv-headers-for-skia
> +  (package
> +    (inherit spirv-headers)
> +    (name "spirv-headers-for-skia")
> +    (version %vulkan-sdk-skia-version)
> +    (source
> +     (origin
> +       (method git-fetch)
> +       (uri (git-reference
> +             (url "https://skia.googlesource.com/external/github.com/KhronosGroup/SPIRV-Headers.git")
> +             (commit version)))

This like busts the 80 characters limit.  You can break it with a
backslash (\) followed by a newline, e.g.

--8<---------------cut here---------------start------------->8---
                (url "https://skia.googlesource.com/external/github.com\
/KhronosGroup/SPIRV-Headers.git")
--8<---------------cut here---------------end--------------->8---

I think 'guix lint' warns about this.

-- 
Thanks,
Maxim




^ permalink raw reply	[flat|nested] 26+ messages in thread

* [bug#60571] [PATCH v2 2/4] gnu: Add spirv-tools-for-skia.
  2023-08-27 12:53   ` [bug#60571] [PATCH v2 2/4] gnu: Add spirv-tools-for-skia Nicolas Graves via Guix-patches via
@ 2023-08-28 15:29     ` Maxim Cournoyer
  0 siblings, 0 replies; 26+ messages in thread
From: Maxim Cournoyer @ 2023-08-28 15:29 UTC (permalink / raw)
  To: Nicolas Graves; +Cc: 60571

Hello,

Nicolas Graves <ngraves@ngraves.fr> writes:

> * gnu/packages/vulkan.scm (spirv-tools-for-skia): New variable.
> ---
>  gnu/packages/vulkan.scm | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>
> diff --git a/gnu/packages/vulkan.scm b/gnu/packages/vulkan.scm
> index da83417dba..9f02b1d235 100644
> --- a/gnu/packages/vulkan.scm
> +++ b/gnu/packages/vulkan.scm
> @@ -129,6 +129,29 @@ (define-public spirv-tools
>  parser,disassembler, validator, and optimizer for SPIR-V.")
>      (license license:asl2.0)))
>  
> +(define-public spirv-tools-for-skia
> +  (package
> +    (inherit spirv-tools)
> +    (name "spirv-tools-for-skia")
> +    (version %vulkan-sdk-skia-version)
> +    (source
> +     (origin
> +       (method git-fetch)
> +       (uri (git-reference
> +             (url "https://skia.googlesource.com/external/github.com/KhronosGroup/SPIRV-Tools.git")

Same line length comment as previously.

> +             (commit version)))
> +       (file-name (git-file-name name version))
> +       (sha256
> +        (base32 "0cp1amgylflh36nw3sy0cy7pf9bhhzykk3vxqwmgnxiryr65nhph"))))
> +    (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")))))))

The configure-flags should use a gexp #~(list ...) in which you can then use
#$(this-package-native-input "spirv-headers-for-skia").

-- 
Thanks,
Maxim




^ permalink raw reply	[flat|nested] 26+ messages in thread

* [bug#60571] [PATCH v2 3/4] gnu: Add icu4c-for-skia.
  2023-08-27 12:53   ` [bug#60571] [PATCH v2 3/4] gnu: Add icu4c-for-skia Nicolas Graves via Guix-patches via
@ 2023-08-28 15:48     ` Maxim Cournoyer
  2023-08-29 13:37       ` Nicolas Graves via Guix-patches via
  0 siblings, 1 reply; 26+ messages in thread
From: Maxim Cournoyer @ 2023-08-28 15:48 UTC (permalink / raw)
  To: Nicolas Graves; +Cc: 60571

Hello!

Nicolas Graves <ngraves@ngraves.fr> writes:

> * gnu/packages/icu4c.scm (icu4c-for-skia): New variable.
> ---
>  gnu/packages/icu4c.scm | 101 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 101 insertions(+)
>
> diff --git a/gnu/packages/icu4c.scm b/gnu/packages/icu4c.scm
> index ba8b4915f2..eef8bd2cd3 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))
>
> @@ -240,3 +243,101 @@ (define-public java-icu4j
>  globalisation support for software applications.  This package contains the
>  Java part.")
>      (license x11)))
> +
> +(define-public icu4c-for-skia
> +  ;; The current version of skia needs commit-precise icu4c
> +  ;; version for its test-dependencies.

I'd reword to use "needs this exact commit".  Also,
s/test-dependencies/test dependencies/ (no hyphen).

> +  (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))

nitpick: Conventionally, it's more common to put the inputs after the
arguments fields.

> +      (arguments
> +       (list
> +        #:make-flags #~`(,(string-append "DESTDIR=" #$output))

It'd be more readable to use the simpler #~(list (string-append ...)).

> +        #:configure-flags #~'(;;"--enable-shared=no" "--enable-static=yes"
> +                              "--prefix=" "--exec-prefix=")

Please drop the dead code (;; ...).

> +        #: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)

Too long of a line; you can but #:allow-other-keys on its own line.

> +                (let ((bash (search-input-file inputs "/bin/sh")))
> +                  ;; Replace bash executable.
> +                  (setenv "CONFIG_SHELL" bash)
> +                  (substitute* "./configure"
> +                    (("`/bin/sh")
> +                     (string-append "`" bash)))

Is just setting CONFIG_SHELL not enough?  Since '/bin/sh' here is used
strictly at build time (in the configure script), you can use simply
(which "sh") in your substitution.  Otherwise you'd have to
(search-input-file (or native-inputs inputs) ...) to not break
cross-compilation.

> +                  ;; Make the static library position-independent.
> +                  ;; (substitute* "./icudefs.mk.in"
> +                    ;; (("^CFLAGS = ")
> +                     ;; "CFLAGS = -fPIC ")
> +                    ;; (("^CXXFLAGS = ")
> +                     ;; "CXXFLAGS = -fPIC "))

All commented out?  Drop unless needed.

> +                  ;; Update runpath.

The comments are a bit terse; I'd expound them to complete sentences
with a tad more context.

> +                  (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.

Ditto.

> +                (with-directory-excursion "data"
> +                  `(,invoke "make" "clean" ,@make-flags))

It'd be cleaner to use 'apply' here, e.g.

                      (apply invoke "make" "clean" make-flags)

> +                ;; Set prefix and exec-prefix.
> +                (substitute* "./runConfigureICU"
> +                  (("^OPTS=")
> +                   (string-append
> +                    "OPTS=\"" (string-join configure-flags " ") "\"")))

Mentioning in passage, you could also use (format #f "OPTS=\"~a\""
(string-join ...)), or even the more advanced (ice-9 format).

> +                ;; Configure for common data.
> +                (setenv "ICU_DATA_FILTER_FILE"
> +                        (string-append (getcwd) "/../filters/common.json"))
> +                (invoke "./runConfigureICU" "Linux/gcc"
> +                        "--disable-layout" "--disable-tests")))

I'm confused here; we re-run the configure script post installation?
How does that work?  If that's really needed it needs a proper
explanatory comment.

> +            (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)

Please use 'apply', as explained above.

> +                  (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")
> +                   ""))))))))))

The rest looks good!  It's seems a tricky package, well done!  Could you
please send a v2 with the above suggestions/comments taken into account?

-- 
Thanks,
Maxim




^ permalink raw reply	[flat|nested] 26+ messages in thread

* [bug#60571] [PATCH v2 4/4] gnu: skia: Activate tests.
  2023-08-27 12:53   ` [bug#60571] [PATCH v2 4/4] gnu: skia: Activate tests Nicolas Graves via Guix-patches via
@ 2023-08-28 15:58     ` Maxim Cournoyer
  0 siblings, 0 replies; 26+ messages in thread
From: Maxim Cournoyer @ 2023-08-28 15:58 UTC (permalink / raw)
  To: Nicolas Graves; +Cc: 60571

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




^ permalink raw reply	[flat|nested] 26+ messages in thread

* [bug#60571] [PATCH v2 3/4] gnu: Add icu4c-for-skia.
  2023-08-28 15:48     ` Maxim Cournoyer
@ 2023-08-29 13:37       ` Nicolas Graves via Guix-patches via
  2023-08-29 14:46         ` Maxim Cournoyer
  0 siblings, 1 reply; 26+ messages in thread
From: Nicolas Graves via Guix-patches via @ 2023-08-29 13:37 UTC (permalink / raw)
  To: Maxim Cournoyer; +Cc: 60571

On 2023-08-28 11:48, Maxim Cournoyer wrote:

> Hello!
>
> Nicolas Graves <ngraves@ngraves.fr> writes:
>
>
> The comments are a bit terse; I'd expound them to complete sentences
> with a tad more context.

Will try to expand a bit, but that will be hard, the commits are very
old and my memory of this mostly faded.

>
> I'm confused here; we re-run the configure script post installation?
> How does that work?  If that's really needed it needs a proper
> explanatory comment.

IIRC, since we use this version of icu4c to run tests, we needed to
also build some "data". It's rather a reconfiguration to build data
rather than a re-run of the configuration script post-installation.

>
> The rest looks good!  It's seems a tricky package, well done!  Could you
> please send a v2 with the above suggestions/comments taken into account?

Doing so 

--
Best regards,
Nicolas Graves




^ permalink raw reply	[flat|nested] 26+ messages in thread

* [bug#60571] [PATCH v2 3/4] gnu: Add icu4c-for-skia.
  2023-08-29 13:37       ` Nicolas Graves via Guix-patches via
@ 2023-08-29 14:46         ` Maxim Cournoyer
  0 siblings, 0 replies; 26+ messages in thread
From: Maxim Cournoyer @ 2023-08-29 14:46 UTC (permalink / raw)
  To: Nicolas Graves; +Cc: 60571

Hi Nicolas,

Nicolas Graves <ngraves@ngraves.fr> writes:

> On 2023-08-28 11:48, Maxim Cournoyer wrote:
>
>> Hello!
>>
>> Nicolas Graves <ngraves@ngraves.fr> writes:
>>
>>
>> The comments are a bit terse; I'd expound them to complete sentences
>> with a tad more context.
>
> Will try to expand a bit, but that will be hard, the commits are very
> old and my memory of this mostly faded.
>
>>
>> I'm confused here; we re-run the configure script post installation?
>> How does that work?  If that's really needed it needs a proper
>> explanatory comment.
>
> IIRC, since we use this version of icu4c to run tests, we needed to
> also build some "data". It's rather a reconfiguration to build data
> rather than a re-run of the configuration script post-installation.

That would perfect in an explanatory comment :-).

>>
>> The rest looks good!  It's seems a tricky package, well done!  Could you
>> please send a v2 with the above suggestions/comments taken into account?
>
> Doing so 

Super!  Thanks for seeing it through.

-- 
Thanks,
Maxim




^ permalink raw reply	[flat|nested] 26+ messages in thread

* [bug#60571] [PATCH v3 1/3] gnu: Add icu4c-for-skia.
  2023-01-05 10:02 [bug#60571] activating tests for skia 2D graphics library Nicolas Graves via Guix-patches via
  2023-01-05 12:18 ` [bug#60571] [PATCH 1/4] gnu: Add spirv-headers-for-skia Nicolas Graves via Guix-patches via
  2023-08-27 12:53 ` [bug#60571] [PATCH v2 1/4] gnu: Add spirv-headers-for-skia Nicolas Graves via Guix-patches via
@ 2023-08-29 21:11 ` Nicolas Graves via Guix-patches via
  2023-08-29 21:11   ` [bug#60571] [PATCH v3 2/3] gnu: skia: Activate tests Nicolas Graves via Guix-patches via
  2023-08-29 21:11   ` [bug#60571] [PATCH v3 3/3] gnu: skia: Update to 110.0.0f3fb7a Nicolas Graves via Guix-patches via
  2023-09-05 15:07 ` [bug#60571] [PATCH v4 1/3] gnu: skia: Update to 110.0.0f3fb7a Nicolas Graves via Guix-patches via
  3 siblings, 2 replies; 26+ messages in thread
From: Nicolas Graves via Guix-patches via @ 2023-08-29 21:11 UTC (permalink / raw)
  To: 60571; +Cc: ngraves, maxim.cournoyer

* gnu/packages/icu4c.scm (icu4c-for-skia): New variable.
---
 gnu/packages/icu4c.scm | 76 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 76 insertions(+)

diff --git a/gnu/packages/icu4c.scm b/gnu/packages/icu4c.scm
index ba8b4915f2..9d1d461d17 100644
--- a/gnu/packages/icu4c.scm
+++ b/gnu/packages/icu4c.scm
@@ -9,6 +9,7 @@
 ;;; Copyright © 2020 Björn Höfling <bjoern.hoefling@bjoernhoefling.de>
 ;;; Copyright © 2020 Julien Lepiller <julien@lepiller.eu>
 ;;; Copyright © 2021 Guillaume Le Vaillant <glv@posteo.net>
+;;; Copyright © 2023 Nicolas Graves <ngraves@ngraves.fr>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -27,14 +28,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))
 
@@ -240,3 +244,75 @@ (define-public java-icu4j
 globalisation support for software applications.  This package contains the
 Java part.")
     (license x11)))
+
+(define-public icu4c-for-skia
+  ;; The current version of skia needs this exact commit
+  ;; for its test dependencies.
+  (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"))))
+      (arguments
+       (list
+        #:make-flags #~(list (string-append "DESTDIR=" #$output))
+        #:configure-flags #~'("--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)
+                (setenv "CONFIG_SHELL" (which "sh"))
+                (setenv "OPTS" (string-join configure-flags))
+                (invoke "./runConfigureICU" "Linux/gcc"
+                        "--disable-layout" "--disable-tests")))
+            (add-after 'install 'install-cleanup
+              (lambda* (#:key make-flags #:allow-other-keys)
+                (with-directory-excursion "data"
+                  (apply invoke "make" "clean" make-flags))))
+            (add-after 'install-cleanup 'configure-filtered-data
+              (lambda* (#:key configure-flags #:allow-other-keys)
+                (setenv "OPTS" (string-join configure-flags))
+                (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")))
+                  (apply 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")
+                   "")))))))
+      (native-inputs (list python cpio pkg-config)))))

base-commit: cf6abf50dbbbd95fef465ab4bb3b608843ac47e1
prerequisite-patch-id: cccdad83975cbf04d7bd618c2c1a4b4de6fa7fd2
prerequisite-patch-id: 6f28833d2efa054d55126980f87ba4d2fdd13c6d
prerequisite-patch-id: afc6cadece838372370f7093f863ce8eaae7bc55
prerequisite-patch-id: b9330c12700355319c104aa3b493eafe03cbb619
prerequisite-patch-id: 46fa9c5a48fcc5b13409049b14b6e7314a6d6956
prerequisite-patch-id: 846f8b50b8de749caaa459b874087d06e15e0a80
prerequisite-patch-id: 023101e5a315951ce9786fb8230955c97001dac9
prerequisite-patch-id: d4b0193f128d8236026e079e746ea0cf6c4c0af0
prerequisite-patch-id: d73442d6d7c88e7375e9de0a9cd655cacb7766f9
prerequisite-patch-id: d8a56dd7bc6c1c3ba3ac0f77b2402b9c6469cfb2
prerequisite-patch-id: 3bc2b2eecd799be8d8b0f96b850ef83a6306dab7
prerequisite-patch-id: 798d4a277eff03a59339af4ebe19406682f361ab
prerequisite-patch-id: 3f65e9cdab64edeacfeb5748cd4fb130839b2b30
prerequisite-patch-id: 6f8225b63a1dd1866b05bb91544e49d7c096601c
prerequisite-patch-id: ff71ec9bdf6337390a720db4535268af271e32df
prerequisite-patch-id: c6b40cc38f5bdfab229ac3d7ed4346c5d9f1b2f7
prerequisite-patch-id: 9ab6dade9e641d5e667ba6a61dbfbb3d32c943a6
prerequisite-patch-id: 4fcae29a8f6dd95716669680141da315acaf6e59
prerequisite-patch-id: 0a5a6da0061188dc9be59bc9829db53288307c58
prerequisite-patch-id: 83c4b7d9d831990e4d37ba89c584d773c872ba6b
prerequisite-patch-id: 1c8dddf99041cf399b8836e3ad6721d7bbcebb7c
prerequisite-patch-id: 773df13cc85606b205d1d914e59525b7a6820a1d
prerequisite-patch-id: 6f8974ce8c0a3a25721a41781f9b0dfd61e96cf4
prerequisite-patch-id: d219948d28923c5ccd34b63f988032df33f2f336
prerequisite-patch-id: 0bd75bb28df9ccf3405caf8217d708afc978047d
prerequisite-patch-id: 26476dd782cf8f5e427d4bd36ac85957538a0aa3
prerequisite-patch-id: b5f87c460fd984c41fbb52e7e0dc305c20c46f22
prerequisite-patch-id: 5502b9c6a64abaca6a9921f25cd324869d26aa1e
prerequisite-patch-id: a7e84bb368349566e9a6fdbe49a371fdb464bb1e
prerequisite-patch-id: a84a766ceef6bedd5da3f9512c87a2c2a11ff33f
prerequisite-patch-id: 0175b2b1cbbc15c1c775147821715bca9e3303a3
prerequisite-patch-id: 9c20b408ac8aa275ecba58383d83be5cef7647df
prerequisite-patch-id: 836749cd3bd3b86f64de637c3c2df48a3608f09f
prerequisite-patch-id: 28ea1ddbef32a1bc9e908f3f9c7466953c60f13d
prerequisite-patch-id: 52a78e387e36e6408d7147950195d552e4e41528
prerequisite-patch-id: 87fee01a70d4b8cbfca44cc0a9c9f54471a92d18
prerequisite-patch-id: 8c8cdb345e815fc3332805ca224103f185d4a568
prerequisite-patch-id: 3434c5caf1eba9e9a64c673681e5911d2c1d9232
prerequisite-patch-id: c45bf303726fb3dacee01a66c0ff75105a81164d
prerequisite-patch-id: d44a1adb0404f23522aac21fa8a7f26be7ddabdc
prerequisite-patch-id: e422fe29bbcef80260b190637faa1a4953c3f1cb
prerequisite-patch-id: 285479a1a1e46e6f0f8aba5429edfa400c81b32b
prerequisite-patch-id: 30fcff6f8c9328c71d3fca609cddde0b56973bb7
prerequisite-patch-id: 70bb47ccac3375de893e4e640ce7c59369a05a39
prerequisite-patch-id: cc3d79386ca4a93146dc195cd5732764f1ac447f
prerequisite-patch-id: 2686866ec4bf08c7faa05b17cb84d9e0c13ec12c
prerequisite-patch-id: 369b61e07e3c90151a5414b784513b9ae3d3e978
prerequisite-patch-id: 065651c3a8cee63b725d7f86c080c274b494627e
prerequisite-patch-id: 1ee71844f0c9a0112a456e5b76079239906a7fb8
prerequisite-patch-id: c7b3d9c5bff04c16576781eff50ce37f7c49131c
prerequisite-patch-id: c8b8fe8dc51fea0b8a2626cf7031f01b6000e023
prerequisite-patch-id: 182e25335d4c357001f4f8bed2b3f89b91d9cae5
prerequisite-patch-id: c0ea00d5f4c6a83642a92c9341f0288fbfe3095a
prerequisite-patch-id: 795d31cc33a24a6a57e67af31b65acd8faa8187c
prerequisite-patch-id: 358af8aa7fa71b5cb8fbe6dde29d141bb7c57f1e
prerequisite-patch-id: b9ab3ee98a9d4ca518a4d99042982d64fbce5d05
prerequisite-patch-id: eb618ab7b10483d917c308a38792af98baa517e2
prerequisite-patch-id: c6bbe6026bfcd2c9ff6e06efc5b8424a6943d1bf
prerequisite-patch-id: 5f664cb2fd995a53765c5ffc19a708ac795cc0c4
prerequisite-patch-id: 8e234d0f4d93d2aad499eec8842be3d28da98707
prerequisite-patch-id: 40b6c9f09f27833367a71ec25d77afae4d2a835e
prerequisite-patch-id: 45e65ea00ece53f3496251401acd464081f8ca7a
prerequisite-patch-id: c3bc83367aa2aab9f121fe3bdbbd9e48dee860ea
prerequisite-patch-id: c12968d02d99c253f858586a86b16fa32d41f1c1
prerequisite-patch-id: 09d995d48139f8e61183d5634cda13a01cdb50f7
prerequisite-patch-id: 86baa45ec2aad977c8c8135f7613aa391155de6d
prerequisite-patch-id: 3425fbbff6a603d60b4e143ea2141aabf4ddc92c
prerequisite-patch-id: c373c01aab5dcba3503a97d51c62a595147a041c
prerequisite-patch-id: cda857c790b88c681c4e713c5f71e40291970daf
-- 
2.41.0





^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [bug#60571] [PATCH v3 2/3] gnu: skia: Activate tests.
  2023-08-29 21:11 ` [bug#60571] [PATCH v3 1/3] gnu: Add icu4c-for-skia Nicolas Graves via Guix-patches via
@ 2023-08-29 21:11   ` Nicolas Graves via Guix-patches via
  2023-08-29 21:11   ` [bug#60571] [PATCH v3 3/3] gnu: skia: Update to 110.0.0f3fb7a Nicolas Graves via Guix-patches via
  1 sibling, 0 replies; 26+ messages in thread
From: Nicolas Graves via Guix-patches via @ 2023-08-29 21:11 UTC (permalink / raw)
  To: 60571; +Cc: ngraves, maxim.cournoyer

* gnu/packages/graphics.scm (skia): Activate tests.
---
 gnu/packages/graphics.scm | 147 ++++++++++++++++++++++++++++++++++----
 1 file changed, 135 insertions(+), 12 deletions(-)

diff --git a/gnu/packages/graphics.scm b/gnu/packages/graphics.scm
index 2a94bd51cc..207da49e05 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,139 @@ (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")))
+                  ;; Unbundle 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.
+                  ;; This is necessary because is_official_build prevents from
+                  ;; 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")
+                  ;; The test suite requires 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"
+                          ;; The underscores are part of the dm syntax for
+                          ;; skipping tests.
+                          ;; 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"
+                          ;; These fail for unknown reasons.
+                          "_" "_" "_" "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 spirv-headers
+                       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 +2222,7 @@ (define-public skia
 @item shaders, xfermodes, maskfilters, patheffects
 @item subpixel text
 @end itemize")
-      (license license:bsd-3))))
+  (license license:bsd-3))))
 
 (define-public superfamiconv
   (package
-- 
2.41.0





^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [bug#60571] [PATCH v3 3/3] gnu: skia: Update to 110.0.0f3fb7a.
  2023-08-29 21:11 ` [bug#60571] [PATCH v3 1/3] gnu: Add icu4c-for-skia Nicolas Graves via Guix-patches via
  2023-08-29 21:11   ` [bug#60571] [PATCH v3 2/3] gnu: skia: Activate tests Nicolas Graves via Guix-patches via
@ 2023-08-29 21:11   ` Nicolas Graves via Guix-patches via
  2023-09-02 13:33     ` [bug#60571] activating tests for skia 2D graphics library Maxim Cournoyer
  2023-09-02 16:17     ` Maxim Cournoyer
  1 sibling, 2 replies; 26+ messages in thread
From: Nicolas Graves via Guix-patches via @ 2023-08-29 21:11 UTC (permalink / raw)
  To: 60571; +Cc: ngraves, maxim.cournoyer

* gnu/packages/graphics.scm (skia): Update to 110.0.0f3fb7a.
[arguments](build-phases): Disable newly introduced libraries wuffs (general
performance) and perfetto (running performance tests).
---
 gnu/packages/graphics.scm | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/gnu/packages/graphics.scm b/gnu/packages/graphics.scm
index 207da49e05..9dfd7dd7ed 100644
--- a/gnu/packages/graphics.scm
+++ b/gnu/packages/graphics.scm
@@ -2000,9 +2000,9 @@ (define-public skia
   ;; https://skia.org/docs/user/release/release_notes/.  The commit used
   ;; should be the last commit, as recommended at
   ;; https://skia.org/docs/user/release/.
-  (let ((version "98")
+  (let ((version "110")
         (revision "0")
-        (commit "55c56abac381e1ae3f0116c410bed81b05e0a38a"))
+        (commit "0f3fb7a005fb357962e8b948ff4ec6b37f11e01b"))
     (package
       (name "skia")
       (version (git-version version revision commit))
@@ -2014,7 +2014,7 @@ (define-public skia
                 (file-name (git-file-name name version))
                 (sha256
                  (base32
-                  "1ldns2j1g2wj2phlxr9zqkdgs5g64pisxhwxcrq9ijn8a3jhafr2"))))
+                  "1aqdnx3817hzhy60mc72j6iwaywvd3b5fj3ffpjxnnras38x2jvp"))))
       (build-system gnu-build-system)   ;actually GN + Ninja
       (arguments
        (list
@@ -2045,7 +2045,10 @@ (define-public skia
                          "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_dng_sdk=false "
+                         ;; Wuffs is a google language that may improve performance
+                         ;; disabled while unpackaged
+                         "skia_use_wuffs=false "))))
             (replace 'build
               (lambda* (#:key parallel-build? #:allow-other-keys)
                 (let ((job-count (if parallel-build?
@@ -2124,6 +2127,8 @@ (define-public skia
                            "--args="
                            "cc=\"gcc\" "              ;defaults to 'cc'
                            "skia_compile_sksl_tests=false " ; disable some tests
+                           "skia_use_perfetto=false " ; disable performance tests
+                           "skia_use_wuffs=false "  ; missing performance tool
                            "skia_use_system_expat=true " ; use system expat library
                            ;; Specify where to locate the includes.
                            "extra_cflags=["
-- 
2.41.0





^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [bug#60571] activating tests for skia 2D graphics library
  2023-08-29 21:11   ` [bug#60571] [PATCH v3 3/3] gnu: skia: Update to 110.0.0f3fb7a Nicolas Graves via Guix-patches via
@ 2023-09-02 13:33     ` Maxim Cournoyer
  2023-09-02 16:17     ` Maxim Cournoyer
  1 sibling, 0 replies; 26+ messages in thread
From: Maxim Cournoyer @ 2023-09-02 13:33 UTC (permalink / raw)
  To: Nicolas Graves; +Cc: 60571

Hi Nicolas,

Nicolas Graves <ngraves@ngraves.fr> writes:

> * gnu/packages/graphics.scm (skia): Update to 110.0.0f3fb7a.
> [arguments](build-phases): Disable newly introduced libraries wuffs (general
> performance) and perfetto (running performance tests).
> ---
>  gnu/packages/graphics.scm | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/gnu/packages/graphics.scm b/gnu/packages/graphics.scm
> index 207da49e05..9dfd7dd7ed 100644
> --- a/gnu/packages/graphics.scm
> +++ b/gnu/packages/graphics.scm
> @@ -2000,9 +2000,9 @@ (define-public skia
>    ;; https://skia.org/docs/user/release/release_notes/.  The commit used
>    ;; should be the last commit, as recommended at
>    ;; https://skia.org/docs/user/release/.
> -  (let ((version "98")
> +  (let ((version "110")
>          (revision "0")
> -        (commit "55c56abac381e1ae3f0116c410bed81b05e0a38a"))
> +        (commit "0f3fb7a005fb357962e8b948ff4ec6b37f11e01b"))
>      (package
>        (name "skia")
>        (version (git-version version revision commit))
> @@ -2014,7 +2014,7 @@ (define-public skia
>                  (file-name (git-file-name name version))
>                  (sha256
>                   (base32
> -                  "1ldns2j1g2wj2phlxr9zqkdgs5g64pisxhwxcrq9ijn8a3jhafr2"))))
> +                  "1aqdnx3817hzhy60mc72j6iwaywvd3b5fj3ffpjxnnras38x2jvp"))))
>        (build-system gnu-build-system)   ;actually GN + Ninja
>        (arguments
>         (list
> @@ -2045,7 +2045,10 @@ (define-public skia
>                           "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_dng_sdk=false "
> +                         ;; Wuffs is a google language that may improve performance
> +                         ;; disabled while unpackaged
> +                         "skia_use_wuffs=false "))))
>              (replace 'build
>                (lambda* (#:key parallel-build? #:allow-other-keys)
>                  (let ((job-count (if parallel-build?
> @@ -2124,6 +2127,8 @@ (define-public skia
>                             "--args="
>                             "cc=\"gcc\" "              ;defaults to 'cc'
>                             "skia_compile_sksl_tests=false " ; disable some tests
> +                           "skia_use_perfetto=false " ; disable performance tests
> +                           "skia_use_wuffs=false "  ; missing performance tool
>                             "skia_use_system_expat=true " ; use system expat library
>                             ;; Specify where to locate the includes.
>                             "extra_cflags=["

If you try to rebuild skia's dependent packages, you'll find that the
upgrade breaks python-skia-pathops, which in turns breaks font-amiri,
... gnome.

We should look at what QA says and fix the breakage before merging.

-- 
Thanks,
Maxim




^ permalink raw reply	[flat|nested] 26+ messages in thread

* [bug#60571] activating tests for skia 2D graphics library
  2023-08-29 21:11   ` [bug#60571] [PATCH v3 3/3] gnu: skia: Update to 110.0.0f3fb7a Nicolas Graves via Guix-patches via
  2023-09-02 13:33     ` [bug#60571] activating tests for skia 2D graphics library Maxim Cournoyer
@ 2023-09-02 16:17     ` Maxim Cournoyer
  1 sibling, 0 replies; 26+ messages in thread
From: Maxim Cournoyer @ 2023-09-02 16:17 UTC (permalink / raw)
  To: Nicolas Graves; +Cc: 60571

Hello,

Nicolas Graves <ngraves@ngraves.fr> writes:

> * gnu/packages/graphics.scm (skia): Update to 110.0.0f3fb7a.
> [arguments](build-phases): Disable newly introduced libraries wuffs (general
> performance) and perfetto (running performance tests).

I've installed the two first patches of this series, but not this one,
as it broke other packages.

-- 
Thanks,
Maxim




^ permalink raw reply	[flat|nested] 26+ messages in thread

* [bug#60571] [PATCH v4 1/3] gnu: skia: Update to 110.0.0f3fb7a.
  2023-01-05 10:02 [bug#60571] activating tests for skia 2D graphics library Nicolas Graves via Guix-patches via
                   ` (2 preceding siblings ...)
  2023-08-29 21:11 ` [bug#60571] [PATCH v3 1/3] gnu: Add icu4c-for-skia Nicolas Graves via Guix-patches via
@ 2023-09-05 15:07 ` Nicolas Graves via Guix-patches via
  2023-09-05 15:07   ` [bug#60571] [PATCH v4 2/3] gnu: skia: Update to 112.0.6d0b938 Nicolas Graves via Guix-patches via
  2023-09-05 15:07   ` [bug#60571] [PATCH v4 3/3] gnu: python-skia-pathops: Update to 0.8.0 Nicolas Graves via Guix-patches via
  3 siblings, 2 replies; 26+ messages in thread
From: Nicolas Graves via Guix-patches via @ 2023-09-05 15:07 UTC (permalink / raw)
  To: 60571; +Cc: ngraves, maxim.cournoyer

* gnu/packages/graphics.scm (skia): Update to 110.0.0f3fb7a.
[arguments](build-phases): Disable newly introduced libraries wuffs (general
performance) and perfetto (running performance tests).
---
 gnu/packages/graphics.scm | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/gnu/packages/graphics.scm b/gnu/packages/graphics.scm
index 346871328c..2182cdeaf2 100644
--- a/gnu/packages/graphics.scm
+++ b/gnu/packages/graphics.scm
@@ -1999,9 +1999,9 @@ (define-public skia
   ;; https://skia.org/docs/user/release/release_notes/.  The commit used
   ;; should be the last commit, as recommended at
   ;; https://skia.org/docs/user/release/.
-  (let ((version "98")
+  (let ((version "110")
         (revision "0")
-        (commit "55c56abac381e1ae3f0116c410bed81b05e0a38a"))
+        (commit "0f3fb7a005fb357962e8b948ff4ec6b37f11e01b"))
     (package
       (name "skia")
       (version (git-version version revision commit))
@@ -2013,7 +2013,7 @@ (define-public skia
                 (file-name (git-file-name name version))
                 (sha256
                  (base32
-                  "1ldns2j1g2wj2phlxr9zqkdgs5g64pisxhwxcrq9ijn8a3jhafr2"))))
+                  "1aqdnx3817hzhy60mc72j6iwaywvd3b5fj3ffpjxnnras38x2jvp"))))
       (build-system gnu-build-system)   ;actually GN + Ninja
       (arguments
        (list
@@ -2044,7 +2044,10 @@ (define-public skia
                          "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_dng_sdk=false "
+                         ;; Wuffs is a google language that may improve performance
+                         ;; disabled while unpackaged
+                         "skia_use_wuffs=false "))))
             (replace 'build
               (lambda* (#:key parallel-build? #:allow-other-keys)
                 (let ((job-count (if parallel-build?
@@ -2123,6 +2126,8 @@ (define-public skia
                            "--args="
                            "cc=\"gcc\" "              ;defaults to 'cc'
                            "skia_compile_sksl_tests=false " ; disable some tests
+                           "skia_use_perfetto=false " ; disable performance tests
+                           "skia_use_wuffs=false "  ; missing performance tool
                            "skia_use_system_expat=true " ; use system expat library
                            ;; Specify where to locate the includes.
                            "extra_cflags=["

base-commit: d010fc6597731a663473b79736766c85d9f7c381
-- 
2.41.0





^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [bug#60571] [PATCH v4 2/3] gnu: skia: Update to 112.0.6d0b938.
  2023-09-05 15:07 ` [bug#60571] [PATCH v4 1/3] gnu: skia: Update to 110.0.0f3fb7a Nicolas Graves via Guix-patches via
@ 2023-09-05 15:07   ` Nicolas Graves via Guix-patches via
  2023-09-05 15:07   ` [bug#60571] [PATCH v4 3/3] gnu: python-skia-pathops: Update to 0.8.0 Nicolas Graves via Guix-patches via
  1 sibling, 0 replies; 26+ messages in thread
From: Nicolas Graves via Guix-patches via @ 2023-09-05 15:07 UTC (permalink / raw)
  To: 60571; +Cc: ngraves, maxim.cournoyer

* gnu/packages/graphics.scm (skia): Update to 112.0.6d0b938.
---
 gnu/packages/graphics.scm | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/gnu/packages/graphics.scm b/gnu/packages/graphics.scm
index 2182cdeaf2..24ed547782 100644
--- a/gnu/packages/graphics.scm
+++ b/gnu/packages/graphics.scm
@@ -1999,9 +1999,9 @@ (define-public skia
   ;; https://skia.org/docs/user/release/release_notes/.  The commit used
   ;; should be the last commit, as recommended at
   ;; https://skia.org/docs/user/release/.
-  (let ((version "110")
+  (let ((version "112")
         (revision "0")
-        (commit "0f3fb7a005fb357962e8b948ff4ec6b37f11e01b"))
+        (commit "6d0b93856303fcf3021a8b40654d7739fda4dfb0"))
     (package
       (name "skia")
       (version (git-version version revision commit))
@@ -2013,7 +2013,7 @@ (define-public skia
                 (file-name (git-file-name name version))
                 (sha256
                  (base32
-                  "1aqdnx3817hzhy60mc72j6iwaywvd3b5fj3ffpjxnnras38x2jvp"))))
+                  "0g07xlvpbbxqmr9igvy5d1hy11z7dz9nzp2fd3ka9y2jqciniyz6"))))
       (build-system gnu-build-system)   ;actually GN + Ninja
       (arguments
        (list
@@ -2033,6 +2033,7 @@ (define-public skia
                          "cc=\"gcc\" "              ;defaults to 'cc'
                          "is_official_build=true "  ;to use system libraries
                          "is_component_build=true " ;build as a shared library
+                         "skia_use_system_zlib=true " ; use system zlib library
                          ;; Specify where locate the harfbuzz and freetype
                          ;; includes.
                          (format #f "extra_cflags=[\"-I~a\",\"-I~a\"] "
@@ -2129,6 +2130,7 @@ (define-public skia
                            "skia_use_perfetto=false " ; disable performance tests
                            "skia_use_wuffs=false "  ; missing performance tool
                            "skia_use_system_expat=true " ; use system expat library
+                           "skia_use_system_zlib=true " ; use system zlib library
                            ;; Specify where to locate the includes.
                            "extra_cflags=["
                            (string-join
-- 
2.41.0





^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [bug#60571] [PATCH v4 3/3] gnu: python-skia-pathops: Update to 0.8.0.
  2023-09-05 15:07 ` [bug#60571] [PATCH v4 1/3] gnu: skia: Update to 110.0.0f3fb7a Nicolas Graves via Guix-patches via
  2023-09-05 15:07   ` [bug#60571] [PATCH v4 2/3] gnu: skia: Update to 112.0.6d0b938 Nicolas Graves via Guix-patches via
@ 2023-09-05 15:07   ` Nicolas Graves via Guix-patches via
  2023-09-06  1:08     ` bug#60571: " Maxim Cournoyer
  1 sibling, 1 reply; 26+ messages in thread
From: Nicolas Graves via Guix-patches via @ 2023-09-05 15:07 UTC (permalink / raw)
  To: 60571; +Cc: ngraves, maxim.cournoyer

* gnu/packages/fontutils.scm (python-skia-pathops): Update to 0.8.0.
---
 gnu/packages/fontutils.scm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gnu/packages/fontutils.scm b/gnu/packages/fontutils.scm
index 153602b4b4..18697cd821 100644
--- a/gnu/packages/fontutils.scm
+++ b/gnu/packages/fontutils.scm
@@ -841,7 +841,7 @@ (define-public python-sfdlib
 (define-public python-skia-pathops
   (package
     (name "python-skia-pathops")
-    (version "0.7.2")
+    (version "0.8.0")
     (source
      (origin
        (method url-fetch)
@@ -849,7 +849,7 @@ (define-public python-skia-pathops
        (modules '((guix build utils)))
        (snippet '(delete-file-recursively "src/cpp")) ;140+ MiB of stuff
        (sha256
-        (base32 "1456rclfn6a01c2cchlgyn166zppcjcqij0k5gwmm8gvzsd5rn0r"))))
+        (base32 "1vlwl1w6sn8c78fsh1w549n3lk9v3v9hcp866vrsdr4byb7g2ani"))))
     (build-system python-build-system)
     (arguments
      (list
-- 
2.41.0





^ permalink raw reply related	[flat|nested] 26+ messages in thread

* bug#60571: [PATCH v4 3/3] gnu: python-skia-pathops: Update to 0.8.0.
  2023-09-05 15:07   ` [bug#60571] [PATCH v4 3/3] gnu: python-skia-pathops: Update to 0.8.0 Nicolas Graves via Guix-patches via
@ 2023-09-06  1:08     ` Maxim Cournoyer
  0 siblings, 0 replies; 26+ messages in thread
From: Maxim Cournoyer @ 2023-09-06  1:08 UTC (permalink / raw)
  To: Nicolas Graves; +Cc: 60571-done

Hi,

Nicolas Graves <ngraves@ngraves.fr> writes:

> * gnu/packages/fontutils.scm (python-skia-pathops): Update to 0.8.0.
> ---
>  gnu/packages/fontutils.scm | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/gnu/packages/fontutils.scm b/gnu/packages/fontutils.scm
> index 153602b4b4..18697cd821 100644
> --- a/gnu/packages/fontutils.scm
> +++ b/gnu/packages/fontutils.scm
> @@ -841,7 +841,7 @@ (define-public python-sfdlib
>  (define-public python-skia-pathops
>    (package
>      (name "python-skia-pathops")
> -    (version "0.7.2")
> +    (version "0.8.0")
>      (source
>       (origin
>         (method url-fetch)
> @@ -849,7 +849,7 @@ (define-public python-skia-pathops
>         (modules '((guix build utils)))
>         (snippet '(delete-file-recursively "src/cpp")) ;140+ MiB of stuff
>         (sha256
> -        (base32 "1456rclfn6a01c2cchlgyn166zppcjcqij0k5gwmm8gvzsd5rn0r"))))
> +        (base32 "1vlwl1w6sn8c78fsh1w549n3lk9v3v9hcp866vrsdr4byb7g2ani"))))
>      (build-system python-build-system)
>      (arguments
>       (list

Applied the series!

-- 
Thanks,
Maxim




^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2023-09-06  1:09 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-05 10:02 [bug#60571] activating tests for skia 2D graphics library Nicolas Graves via Guix-patches via
2023-01-05 12:18 ` [bug#60571] [PATCH 1/4] gnu: Add spirv-headers-for-skia Nicolas Graves via Guix-patches via
2023-01-05 12:18   ` [bug#60571] [PATCH 2/4] gnu: Add spirv-tools-for-skia Nicolas Graves via Guix-patches via
2023-03-24  3:20     ` [bug#60571] activating tests for skia 2D graphics library Maxim Cournoyer
2023-01-05 12:18   ` [bug#60571] [PATCH 3/4] gnu: Add icu4c-for-skia Nicolas Graves via Guix-patches via
2023-01-05 12:18   ` [bug#60571] [PATCH 4/4] gnu: skia: Activate tests Nicolas Graves via Guix-patches via
2023-03-24  3:15   ` [bug#60571] activating tests for skia 2D graphics library Maxim Cournoyer
2023-08-27 12:53 ` [bug#60571] [PATCH v2 1/4] gnu: Add spirv-headers-for-skia Nicolas Graves via Guix-patches via
2023-08-27 12:53   ` [bug#60571] [PATCH v2 2/4] gnu: Add spirv-tools-for-skia Nicolas Graves via Guix-patches via
2023-08-28 15:29     ` Maxim Cournoyer
2023-08-27 12:53   ` [bug#60571] [PATCH v2 3/4] gnu: Add icu4c-for-skia Nicolas Graves via Guix-patches via
2023-08-28 15:48     ` Maxim Cournoyer
2023-08-29 13:37       ` Nicolas Graves via Guix-patches via
2023-08-29 14:46         ` Maxim Cournoyer
2023-08-27 12:53   ` [bug#60571] [PATCH v2 4/4] gnu: skia: Activate tests Nicolas Graves via Guix-patches via
2023-08-28 15:58     ` Maxim Cournoyer
2023-08-28 15:27   ` [bug#60571] [PATCH v2 1/4] gnu: Add spirv-headers-for-skia Maxim Cournoyer
2023-08-29 21:11 ` [bug#60571] [PATCH v3 1/3] gnu: Add icu4c-for-skia Nicolas Graves via Guix-patches via
2023-08-29 21:11   ` [bug#60571] [PATCH v3 2/3] gnu: skia: Activate tests Nicolas Graves via Guix-patches via
2023-08-29 21:11   ` [bug#60571] [PATCH v3 3/3] gnu: skia: Update to 110.0.0f3fb7a Nicolas Graves via Guix-patches via
2023-09-02 13:33     ` [bug#60571] activating tests for skia 2D graphics library Maxim Cournoyer
2023-09-02 16:17     ` Maxim Cournoyer
2023-09-05 15:07 ` [bug#60571] [PATCH v4 1/3] gnu: skia: Update to 110.0.0f3fb7a Nicolas Graves via Guix-patches via
2023-09-05 15:07   ` [bug#60571] [PATCH v4 2/3] gnu: skia: Update to 112.0.6d0b938 Nicolas Graves via Guix-patches via
2023-09-05 15:07   ` [bug#60571] [PATCH v4 3/3] gnu: python-skia-pathops: Update to 0.8.0 Nicolas Graves via Guix-patches via
2023-09-06  1:08     ` bug#60571: " Maxim Cournoyer

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).