I'm going to add my comments while reading through instead of reading
through, digesting, and then commenting.

On Thu, Dec 15, 2022 at 12:12:59PM +0000, Ekaitz Zarraga wrote:
> Hi all,
> 
> I'm working on a zig-build-system and I have something working.
> 
> I'd like to discuss some of the problems I found because I don't
> know what's the best solution:
> 
> - I don't know how to configure the cross-compilation

cross-compilation can come later. It's not uncommon when writing new
build systems.

> - The `zig` command uses a compiler to find the libc it needs to use
>   we should be able to remove the `gcc-toolchain` from the
>   dependencies if we just add the libc to the build system properly
>   (this point is mixed with the point above)

Are you sure about removing gcc-toolchain? I don't have a problem with
it if it is possible, especially since zig does provide a 'zig cc'
command. We'd still likely need to use something like the
make-gcc-toolchain to properly use ld-wrapper, unless we somehow
wouldn't need that either.

> I need suggestions on these...
> 
> I attach two patches, one for the current build system I made and
> a second one for an example package that actually works but has
> some weird needs you can find in the comments.
> 
> Cheers,
> Ekaitz

> From c19c7161cf0864be5ffee12e8baac248d2d9874a Mon Sep 17 00:00:00 2001
> From: Ekaitz Zarraga <ekaitz@elenq.tech>
> Date: Thu, 15 Dec 2022 13:02:25 +0100
> Subject: [PATCH 2/2] Add a sample package for testing the zig build system
> 
> ---
>  gnu/packages/zig.scm | 43 ++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 42 insertions(+), 1 deletion(-)
> 
> diff --git a/gnu/packages/zig.scm b/gnu/packages/zig.scm
> index cda93bed2e..7bce76ed51 100644
> --- a/gnu/packages/zig.scm
> +++ b/gnu/packages/zig.scm
> @@ -25,7 +25,9 @@ (define-module (gnu packages zig)
>    #:use-module ((guix licenses) #:prefix license:)
>    #:use-module (guix build-system cmake)
>    #:use-module (gnu packages)
> -  #:use-module (gnu packages llvm))
> +  #:use-module (gnu packages llvm)
> +  #:use-module (guix build-system zig)
> +  #:use-module (gnu packages commencement))
>  
>  (define-public zig
>    (package
> @@ -104,3 +106,42 @@ (define-public zig
>      ;; https://github.com/ziglang/zig/issues/6485
>      (supported-systems %64bit-supported-systems)
>      (license license:expat)))
> +

I'm not looking at the actual package definition itself, that can be
left to another time as needed.

> +(define-public tigerbeetle
> +  (let ((commit-id "2022-12-12-weekly")
> +        (revision "0"))
> +    (package
> +      (name "tigerbeetle")
> +      (version (string-append revision "-" commit-id))
> +      (source
> +        (origin
> +          (method git-fetch)
> +          (uri (git-reference
> +                 (url "https://github.com/tigerbeetledb/tigerbeetle.git")
> +                 (commit commit-id)))
> +          (file-name (git-file-name name version))
> +          (sha256
> +            (base32 "18rawl8rhyplw8hpa3fzbq9fqg088x0calz688c7zdff6y6f6mcr"))))
> +      (build-system zig-build-system)
> +      (inputs (list gcc-toolchain))

Isn't gcc-toolchain part of %standard-packages in a build-system? Did I
miss it not being there or does it also need to be added here for the
reason you listed below?

> +      (arguments
> +         `(#:phases
> +           (modify-phases %standard-phases
> +          ;; TODO: Zig needs the gcc-toolchain in order to find the libc.
> +          ;;       we need to think about how to solve this in the build system
> +          ;;       directly: --libc

by default glibc is named 'libc' in the build system, and when
cross-compiling 'libc' is generally the correct libc that you'd be
looking for. Also I'd be curious about reusing the libc vendored in zig
itself or if that's not actually possible/only for embedded uses.

Also if you just need libc you could import glibc-final for the
non-cross-build as a standard native-input (input?) and then you'd have
it available.

> +                          (add-before 'build 'set-env
> +                            (lambda _
> +                              (setenv "CC" "gcc"))))))

If you really don't need gcc by default you could consider something
like what we have in the cargo-build-system:

(when (assoc-ref inputs "openssl")
  (setenv "OPENSSL_DIR" (assoc-ref inputs "openssl")))

could be:

 (cond
  ((assoc-ref inputs "gcc") (setenv "CC" "gcc"))
  ((assoc-ref inputs "clang") (setenv "CC" "llvm"))
  (_ (setenv "CC" "zig cc")))

> +      (synopsis "Distributed financial accounting database designed for mission
> +criti cal safety and performance")
> +
> +      (description "Financial accounting database designed for mission critical
> +safet y and performance to power the future of financial
> +servi ces.")
> +      (home-page "https://github.com/tigerbeetledb/tigerbeetle.git")
> +      (supported-systems %64bit-supported-systems)
> +      (license license:asl2.0))))
> -- 
> 2.38.0
> 

> From 4dd02f0ea421bc1f4081380e847f048998131873 Mon Sep 17 00:00:00 2001
> From: Ekaitz Zarraga <ekaitz@elenq.tech>
> Date: Thu, 15 Dec 2022 13:01:23 +0100
> Subject: [PATCH 1/2] WIP: Add a zig-build-system
> 
> ---
>  Makefile.am                               |   2 +
>  etc/snippets/yas/scheme-mode/guix-package |   5 +-
>  guix/build-system/zig.scm                 | 119 ++++++++++++++++++++++
>  guix/build/zig-build-system.scm           |  82 +++++++++++++++
>  4 files changed, 206 insertions(+), 2 deletions(-)
>  create mode 100644 guix/build-system/zig.scm
>  create mode 100644 guix/build/zig-build-system.scm
> 
> diff --git a/Makefile.am b/Makefile.am
> index b54288c0fc..4cc7c2e5cf 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -179,6 +179,7 @@ MODULES =					\
>    guix/build-system/scons.scm			\
>    guix/build-system/texlive.scm			\
>    guix/build-system/trivial.scm			\
> +  guix/build-system/zig.scm			\
>    guix/ftp-client.scm				\
>    guix/http-client.scm				\
>    guix/gnupg.scm				\
> @@ -263,6 +264,7 @@ MODULES =					\
>    guix/build/graft.scm				\
>    guix/build/bournish.scm			\
>    guix/build/qt-utils.scm			\
> +  guix/build/zig-build-system.scm		\
>    guix/build/make-bootstrap.scm			\
>    guix/search-paths.scm				\
>    guix/packages.scm				\
> diff --git a/etc/snippets/yas/scheme-mode/guix-package b/etc/snippets/yas/scheme-mode/guix-package
> index 724a392f81..3bb6307659 100644
> --- a/etc/snippets/yas/scheme-mode/guix-package
> +++ b/etc/snippets/yas/scheme-mode/guix-package
> @@ -43,8 +43,9 @@
>                                            "scons-build-system"
>                                            "texlive-build-system"
>                                            "trivial-build-system"
> -                                          "waf-build-system")})
> +                                          "waf-build-system"
> +                                          "zig-build-system")})
>      (home-page "$4")
>      (synopsis "$5")
>      (description "$6")
> -    (license $7)))
> \ No newline at end of file
> +    (license $7)))
> diff --git a/guix/build-system/zig.scm b/guix/build-system/zig.scm
> new file mode 100644
> index 0000000000..77010f1bed
> --- /dev/null
> +++ b/guix/build-system/zig.scm
> @@ -0,0 +1,119 @@
> +;;; GNU Guix --- Functional package management for GNU
> +;;; Copyright © 2022 Ekaitz Zarraga <ekaitz@elenq.tech>
> +;;;
> +;;; This file is part of GNU Guix.
> +;;;
> +;;; GNU Guix is free software; you can redistribute it and/or modify it
> +;;; under the terms of the GNU General Public License as published by
> +;;; the Free Software Foundation; either version 3 of the License, or (at
> +;;; your option) any later version.
> +;;;
> +;;; GNU Guix is distributed in the hope that it will be useful, but
> +;;; WITHOUT ANY WARRANTY; without even the implied warranty of
> +;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +;;; GNU General Public License for more details.
> +;;;
> +;;; You should have received a copy of the GNU General Public License
> +;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
> +
> +(define-module (guix build-system zig)
> +  #:use-module (guix search-paths)
> +  #:use-module (guix store)
> +  #:use-module (guix utils)
> +  #:use-module (guix gexp)
> +  #:use-module (guix monads)
> +  #:use-module (guix packages)
> +  #:use-module (guix build-system)
> +  #:use-module (guix build-system gnu)
> +  #:use-module (ice-9 match)
> +  #:use-module (srfi srfi-26)
> +  #:export (zig-build-system))
> +
> +
> +(define (default-zig)
> +  "Return the default zig package."
> +  ;; Lazily resolve the binding to avoid a circular dependency.
> +  (let ((zig (resolve-interface '(gnu packages zig))))
> +    (module-ref zig 'zig)))
> +
> +(define %zig-build-system-modules
> +  ;; Build-side modules imported by default.
> +  `((guix build zig-build-system)
> +    (guix build syscalls)
> +    ,@%gnu-build-system-modules))
> +
> +(define* (zig-build name inputs
> +                    #:key
> +                    source
> +                    (tests? #t)
> +                    (test-target #f)
> +                    (zig-build-flags ''())

Is it possible to pass flags during the test phase also? If it is then
having a zig-test-flags keyword would also be good.

I would also suggest a 'zig' flag, to allow choosing a different zig
binary to be used for a build.

> +                    (phases '%standard-phases)
> +                    (outputs '("out"))
> +                    (search-paths '())
> +                    (system (%current-system))
> +                    (guile #f)
> +                    (imported-modules %zig-build-system-modules)
> +                    (modules '((guix build zig-build-system)
> +                               (guix build utils))))
> +  "Build SOURCE using Zig, and with INPUTS."
> +  (define builder
> +    (with-imported-modules imported-modules
> +      #~(begin
> +          (use-modules #$@(sexp->gexp modules))
> +          (zig-build #:name #$name
> +                     #:source #+source
> +                     #:system #$system
> +                     #:test-target #$test-target
> +                     #:zig-build-flags #$zig-build-flags
> +                     #:tests? #$tests?
> +                     #:phases #$phases
> +                     #:outputs #$(outputs->gexp outputs)
> +                     #:search-paths '#$(sexp->gexp
> +                                        (map search-path-specification->sexp
> +                                             search-paths))
> +                     #:inputs #$(input-tuples->gexp inputs)))))
> +
> +  (mlet %store-monad ((guile (package->derivation (or guile (default-guile))
> +                                                  system #:graft? #f)))
> +    (gexp->derivation name builder
> +                      #:system system
> +                      #:guile-for-build guile)))
> +
> +(define* (lower name
> +                #:key source inputs native-inputs outputs system target
> +                (zig (default-zig))
> +                #:allow-other-keys
> +                #:rest arguments)
> +  "Return a bag for NAME."
> +
> +  (define private-keywords
> +    '(#:target #:zig #:inputs #:native-inputs #:outputs))
> +
> +  ;; TODO: support cross-compilation
> +  ;; It's as simple as adding some build flags to `zig-build-flags`
> +  (and (not target)
> +       (bag
> +         (name name)
> +         (system system)
> +         (target target)
> +         (host-inputs `(,@(if source
> +                              `(("source" ,source))
> +                              '())
> +                        ,@inputs
> +
> +                        ;; Keep the standard inputs of 'gnu-build-system'
> +                        ;; TODO: do we need this?
> +                        ,@(standard-packages)))
> +         (build-inputs `(("zig" ,zig)
> +                         ,@native-inputs))
> +         (outputs outputs)
> +         (build zig-build)
> +         (arguments (strip-keyword-arguments private-keywords arguments)))))
> +
> +(define zig-build-system
> +  (build-system
> +    (name 'zig)
> +    (description
> +     "Zig build system, to build Zig packages")
> +    (lower lower)))
> diff --git a/guix/build/zig-build-system.scm b/guix/build/zig-build-system.scm
> new file mode 100644
> index 0000000000..de006770ee
> --- /dev/null
> +++ b/guix/build/zig-build-system.scm
> @@ -0,0 +1,82 @@
> +;;; GNU Guix --- Functional package management for GNU
> +;;; Copyright © 2022 Ekaitz Zarraga <ekaitz@elenq.tech>
> +;;;
> +;;; This file is part of GNU Guix.
> +;;;
> +;;; GNU Guix is free software; you can redistribute it and/or modify it
> +;;; under the terms of the GNU General Public License as published by
> +;;; the Free Software Foundation; either version 3 of the License, or (at
> +;;; your option) any later version.
> +;;;
> +;;; GNU Guix is distributed in the hope that it will be useful, but
> +;;; WITHOUT ANY WARRANTY; without even the implied warranty of
> +;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +;;; GNU General Public License for more details.
> +;;;
> +;;; You should have received a copy of the GNU General Public License
> +;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
> +
> +(define-module (guix build zig-build-system)
> +  #:use-module ((guix build gnu-build-system) #:prefix gnu:)
> +  #:use-module (guix build utils)
> +  #:use-module (ice-9 popen)
> +  #:use-module (ice-9 rdelim)
> +  #:use-module (ice-9 ftw)
> +  #:use-module (ice-9 format)
> +  #:use-module (ice-9 match)
> +  #:use-module (rnrs io ports)
> +  #:use-module (srfi srfi-1)
> +  #:use-module (srfi srfi-26)
> +  #:export (%standard-phases
> +            zig-build))
> +
> +;; Commentary:
> +;;
> +;; Builder side code of the Zig build system
> +;;
> +;; Code:
> +
> +;; Interesting guide here:
> +;; https://github.com/riverwm/river/blob/master/PACKAGING.md

I would define a global-cache-dir and use ZIG_GLOBAL_CACHE_DIR to refer
to it and not pass it around to all the phases.

> +(define* (build #:key (zig-build-flags '())
> +                #:allow-other-keys)
> +  "Build a given Zig package."
> +  (setenv "DESTDIR" "out")
> +  (apply invoke
> +    `("zig" "build"
> +      "--global-cache-dir" "zig-cache"
> +      "-p" ""  ;; Don't add /usr

If '-p' is the short form of '--prefix' or something then use the long
form.

> +      "--prefix-lib-dir" "lib"
> +      "--prefix-exe-dir" "bin"
> +      "--prefix-include-dir" "include"
> +      ,@zig-build-flags)))
> +  ;; Let the build flags decide if it needs `-Drelease-safe` or
> +  ;; `-Drelease-fast` or any other.

cmake-build-system has an option to determine Release or Debug (or to
leave it as RelWithDebInfo) named 'build-type'. I think that should be
added here.

> +
> +(define* (check #:key tests? #:allow-other-keys)
> +  "Run all the tests"
> +  (when tests?
> +    (setenv "DESTDIR" "test-out")

I assume this is to prevent overwriting the actual binary with the tests
enabled binary? I would consider unsetting it at the end of the phase.
Or storing its previous value and setting it back to that, we have seen
packages which use DESTDIR and I wouldn't want any surprises.

> +    (apply invoke
> +           `("zig" "build" "test"
> +             "--global-cache-dir" "zig-cache"))))

What's the difference between 'zig test build.zig' and 'zig build test'?

> +(define* (install #:key inputs outputs #:allow-other-keys)
> +  "Install a given Zig package."
> +  (let* ((out    (assoc-ref outputs "out")))
> +    (copy-recursively "out" out)))

Beware of copy-pasting :). You don't use inputs, don't need let* vs let,
and could probably directly (copy-recursively #$output out). Or rely on
'make install'. 'make install' was available for ncdu2, not sure about
other zig projects.

> +(define %standard-phases
> +  (modify-phases gnu:%standard-phases
> +    (delete 'bootstrap)
> +    (delete 'configure)
> +    (replace 'build build)
> +    (replace 'check check)
> +    (replace 'install install)))
> +
> +
> +(define* (zig-build #:key inputs (phases %standard-phases)
> +                      #:allow-other-keys #:rest args)
> +  "Build the given Zig package, applying all of PHASES in order."
> +  (apply gnu:gnu-build #:inputs inputs #:phases phases args))
> -- 
> 2.38.0
> 


-- 
Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted