| Message ID | 20210426183235.3856-1-efraim@flashner.co.il |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [bug#48044] build/go: Support cross compiling. | expand |
| Context | Check | Description |
|---|---|---|
| cbaines/comparison | success | View comparision |
| cbaines/git branch | success | View Git branch |
| cbaines/applying patch | success | View Laminar job |
| cbaines/issue | success | View issue |
> + ;; We really only support targeting Linux and mingw. > + (setenv "GOOS" (if (string-contains target "mingw") > + "windows" > + "linux")) If only Linux and mingw is supported, perhaps some kind of error could be thrown if 'target' does not contain either "mingw" or "linux", to avoid silently compiling for the wrong operating system? There is the Hurd as well for example. I would recommend throwing the error (or returning #f?) from lower, to notify the user cross-compilation is not possible as early as possible. > + (setenv "GOARM" "7") ; Default is 6, target our armhf-linux architecture. Would it be possible to only set that variable when targetting armhf-linux? Setting this when targetting other architectures seems unlikely to cause issues, but still seems subtly wrong to me, for some value of subtle. Note: I do not know much about go and did not test this. Greetings, Maxime.
Hi Efraim, Thanks for doing this work! I'm excited to see it in action. Efraim Flashner <efraim@flashner.co.il> writes: > * guix/build-system/go.scm (lower): Only add target to private-keywords > when not cross compiling. Adjust bag depending if doing a native or > cross compile. > (%go-build-system-modules): Use source-module-closure, add (guix utils). > (go-cross-build): New procedure. > * guix/build/go-build-system.scm (setup-go-environment): Accept target > keyword. Add logic to choose correct target architecture when cross > compiling. > --- > > Third version of this patch. I think I'm ready to push it. I don't love > using source-module-closure to include (guix utils), but I need it for > gnu-triplet->nix-system in setup-go-environment instead of the custom > parsing I was doing before. Can you do the parsing host-side and pass e.g. TARGET-GOOS/TARGET-GOARCH keyword arguments to the build-side? > -(define* (setup-go-environment #:key inputs outputs #:allow-other-keys) > +(define* (setup-go-environment #:key inputs outputs target #:allow-other-keys) > "Prepare a Go build environment for INPUTS and OUTPUTS. Build a file system > union of INPUTS. Export GOPATH, which helps the compiler find the source code > of the package being built and its dependencies, and GOBIN, which determines > @@ -149,6 +150,38 @@ dependencies, so it should be self-contained." > ;; GOPATH behavior. > (setenv "GO111MODULE" "off") > (setenv "GOBIN" (string-append (assoc-ref outputs "out") "/bin")) > + > + ;; Cross-build > + (when target > + ;; Parse the nix-system equivalent of the target and set the > + ;; target for compilation accordingly. > + (let* ((system (gnu-triplet->nix-system target)) > + (dash (string-index system #\-)) > + (arch (substring system 0 dash)) > + (os (substring system (+ 1 dash)))) And then, if this parsing is host-side, you can probably just do something like --8<---------------cut here---------------start------------->8--- (match (string-split (gnu-triplet->nix-system target) #\-) ((arch os) [...] --8<---------------cut here---------------end--------------->8--- > + (match arch > + ((or "arm" "armhf") > + (setenv "GOARM" "7")) > + ((or "mips" "mipsel") > + (setenv "GOMIPS" "hardfloat")) > + ((or "mips64" "mips64el") > + (setenv "GOMIPS64" "hardfloat")) > + ((or "powerpc64" "powerpc64le") > + (setenv "GOPPC64" "power8")) > + (_ #t)))) Are these choices obvious for those compiling for those architectures? If not, this could probably do with some documentation on why these were chosen. (I note that these are all Go's defaults with the exception of GOARM). -- Sarah
On Sun, Aug 22, 2021 at 11:52:29AM -0700, Sarah Morgensen wrote: > Hi Efraim, > > Thanks for doing this work! I'm excited to see it in action. > > Efraim Flashner <efraim@flashner.co.il> writes: > > > * guix/build-system/go.scm (lower): Only add target to private-keywords > > when not cross compiling. Adjust bag depending if doing a native or > > cross compile. > > (%go-build-system-modules): Use source-module-closure, add (guix utils). > > (go-cross-build): New procedure. > > * guix/build/go-build-system.scm (setup-go-environment): Accept target > > keyword. Add logic to choose correct target architecture when cross > > compiling. > > --- > > > > Third version of this patch. I think I'm ready to push it. I don't love > > using source-module-closure to include (guix utils), but I need it for > > gnu-triplet->nix-system in setup-go-environment instead of the custom > > parsing I was doing before. > > Can you do the parsing host-side and pass e.g. TARGET-GOOS/TARGET-GOARCH > keyword arguments to the build-side? I'll have to see if there's somewhere I can slot that in. We already have target, but that's what we're parsing now. I guess if I add them as a keywords then we can build for targets that go supports but which Guix doesn't. > > -(define* (setup-go-environment #:key inputs outputs #:allow-other-keys) > > +(define* (setup-go-environment #:key inputs outputs target #:allow-other-keys) > > "Prepare a Go build environment for INPUTS and OUTPUTS. Build a file system > > union of INPUTS. Export GOPATH, which helps the compiler find the source code > > of the package being built and its dependencies, and GOBIN, which determines > > @@ -149,6 +150,38 @@ dependencies, so it should be self-contained." > > ;; GOPATH behavior. > > (setenv "GO111MODULE" "off") > > (setenv "GOBIN" (string-append (assoc-ref outputs "out") "/bin")) > > + > > + ;; Cross-build > > + (when target > > + ;; Parse the nix-system equivalent of the target and set the > > + ;; target for compilation accordingly. > > + (let* ((system (gnu-triplet->nix-system target)) > > + (dash (string-index system #\-)) > > + (arch (substring system 0 dash)) > > + (os (substring system (+ 1 dash)))) > > And then, if this parsing is host-side, you can probably just do > something like > > --8<---------------cut here---------------start------------->8--- > (match (string-split (gnu-triplet->nix-system target) #\-) > ((arch os) > [...] > --8<---------------cut here---------------end--------------->8--- I like the way this looks much better. Even if we did just parse the gnu-triplet we'd have our special case for arm-linux-gnueabihf (unless we ignored it, as I did below). > > + (match arch > > + ((or "arm" "armhf") > > + (setenv "GOARM" "7")) > > + ((or "mips" "mipsel") > > + (setenv "GOMIPS" "hardfloat")) > > + ((or "mips64" "mips64el") > > + (setenv "GOMIPS64" "hardfloat")) > > + ((or "powerpc64" "powerpc64le") > > + (setenv "GOPPC64" "power8")) > > + (_ #t)))) > > Are these choices obvious for those compiling for those architectures? > If not, this could probably do with some documentation on why these were > chosen. (I note that these are all Go's defaults with the exception of > GOARM). For the mips I did just copy from the Go documentation, but for arm and ppc64 I wanted to make sure we targeted the same systems that Guix already supports. > > -- > Sarah Thanks for taking a look at it.
diff --git a/guix/build-system/go.scm b/guix/build-system/go.scm index 8f55796e86..c9c8f5ba15 100644 --- a/guix/build-system/go.scm +++ b/guix/build-system/go.scm @@ -2,6 +2,7 @@ ;;; Copyright © 2016 Petter <petter@mykolab.ch> ;;; Copyright © 2017 Leo Famulari <leo@famulari.name> ;;; Copyright © 2020 Jakub Kądziołka <kuba@kadziolka.net> +;;; Copyright © 2021 Efraim Flashner <efraim@flashner.co.il> ;;; ;;; This file is part of GNU Guix. ;;; @@ -96,24 +97,40 @@ commit hash and its date rather than a proper release tag." #:rest arguments) "Return a bag for NAME." (define private-keywords - '(#:source #:target #:go #:inputs #:native-inputs)) + `(#:source #:go #:inputs #:native-inputs + ,@(if target '() '(#:target)))) - (and (not target) ;XXX: no cross-compilation - (bag - (name name) - (system system) - (host-inputs `(,@(if source - `(("source" ,source)) - '()) - ,@inputs + (bag + (name name) + (system system) + (target target) + (build-inputs `(,@(if source + `(("source" ,source)) + '()) + ,@`(("go" ,go)) + ,@native-inputs + ,@(if target '() inputs) + ,@(if target + ;; Use the standard cross inputs of + ;; 'gnu-build-system'. + (standard-cross-packages target 'host) + '()) + ;; Keep the standard inputs of 'gnu-build-system'. + ,@(standard-packages))) + (host-inputs (if target inputs '())) - ;; Keep the standard inputs of 'gnu-build-system'. - ,@(standard-packages))) - (build-inputs `(("go" ,go) - ,@native-inputs)) - (outputs outputs) - (build go-build) - (arguments (strip-keyword-arguments private-keywords arguments))))) + ;; The cross-libc is really a target package, but for bootstrapping + ;; reasons, we can't put it in 'host-inputs'. Namely, 'cross-gcc' is a + ;; native package, so it would end up using a "native" variant of + ;; 'cross-libc' (built with 'gnu-build'), whereas all the other packages + ;; would use a target variant (built with 'gnu-cross-build'.) + (target-inputs (if target + (standard-cross-packages target 'target) + '())) + + (outputs outputs) + (build (if target go-cross-build go-build)) + (arguments (strip-keyword-arguments private-keywords arguments)))) (define* (go-build store name inputs #:key @@ -174,6 +191,94 @@ commit hash and its date rather than a proper release tag." #:outputs outputs #:guile-for-build guile-for-build)) +(define* (go-cross-build store name + #:key + target native-drvs target-drvs + (phases '(@ (guix build go-build-system) + %standard-phases)) + (outputs '("out")) + (search-paths '()) + (native-search-paths '()) + (install-source? #t) + (import-path "") + (unpack-path "") + (build-flags ''()) + (tests? #f) ; nothing can be done + (allow-go-reference? #f) + (system (%current-system)) + (guile #f) + (imported-modules %go-build-system-modules) + (modules '((guix build go-build-system) + (guix build union) + (guix build utils)))) + "Cross-build NAME using GO, where TARGET is a GNU triplet and with INPUTS." + (define builder + `(begin + (use-modules ,@modules) + (let () + (define %build-host-inputs + ',(map (match-lambda + ((name (? derivation? drv) sub ...) + `(,name . ,(apply derivation->output-path drv sub))) + ((name path) + `(,name . ,path))) + native-drvs)) + + (define %build-target-inputs + ',(map (match-lambda + ((name (? derivation? drv) sub ...) + `(,name . ,(apply derivation->output-path drv sub))) + ((name (? package? pkg) sub ...) + (let ((drv (package-cross-derivation store pkg + target system))) + `(,name . ,(apply derivation->output-path drv sub)))) + ((name path) + `(,name . ,path))) + target-drvs)) + + (go-build #:name ,name + #:source ,(match (assoc-ref native-drvs "source") + (((? derivation? source)) + (derivation->output-path source)) + ((source) + source) + (source + source)) + #:system ,system + #:phases ,phases + #:outputs %outputs + #:target ,target + #:inputs %build-target-inputs + #:native-inputs %build-host-inputs + #:search-paths ',(map search-path-specification->sexp + search-paths) + #:native-search-paths ',(map + search-path-specification->sexp + native-search-paths) + #:install-source? ,install-source? + #:import-path ,import-path + #:unpack-path ,unpack-path + #:build-flags ,build-flags + #:tests? ,tests? + #:allow-go-reference? ,allow-go-reference? + #:inputs %build-inputs)))) + + (define guile-for-build + (match guile + ((? package?) + (package-derivation store guile system #:graft? #f)) + (#f ; the default + (let* ((distro (resolve-interface '(gnu packages commencement))) + (guile (module-ref distro 'guile-final))) + (package-derivation store guile system #:graft? #f))))) + + (build-expression->derivation store name builder + #:system system + #:inputs (append native-drvs target-drvs) + #:outputs outputs + #:modules imported-modules + #:guile-for-build guile-for-build)) + (define go-build-system (build-system (name 'go) diff --git a/guix/build/go-build-system.scm b/guix/build/go-build-system.scm index 227df820db..5436e19854 100644 --- a/guix/build/go-build-system.scm +++ b/guix/build/go-build-system.scm @@ -4,7 +4,7 @@ ;;; Copyright © 2019 Maxim Cournoyer <maxim.cournoyer@gmail.com> ;;; Copyright © 2020 Jack Hill <jackhill@jackhill.us> ;;; Copyright © 2020 Jakub Kądziołka <kuba@kadziolka.net> -;;; Copyright © 2020 Efraim Flashner <efraim@flashner.co.il> +;;; Copyright © 2020, 2021 Efraim Flashner <efraim@flashner.co.il> ;;; ;;; This file is part of GNU Guix. ;;; @@ -131,7 +131,7 @@ ;; ;; Code: -(define* (setup-go-environment #:key inputs outputs #:allow-other-keys) +(define* (setup-go-environment #:key inputs outputs target #:allow-other-keys) "Prepare a Go build environment for INPUTS and OUTPUTS. Build a file system union of INPUTS. Export GOPATH, which helps the compiler find the source code of the package being built and its dependencies, and GOBIN, which determines @@ -149,6 +149,31 @@ dependencies, so it should be self-contained." ;; GOPATH behavior. (setenv "GO111MODULE" "off") (setenv "GOBIN" (string-append (assoc-ref outputs "out") "/bin")) + + ;; Cross-build + (when target + ;; Separate the 3 parts of the target-triplet + (let* ((first-dash (string-index target #\-)) + (second-dash (string-index target #\- (1+ first-dash))) + (machine (string-take target first-dash))) + (setenv "GOARCH" (match machine + ("aarch64" "arm64") + ("powerpc64le" "ppc64le") + ("powerpc64" "ppc64") + ("i686" "386") + ("x86_64" "amd64") + ("mips64el" "mips64le") + (_ machine))) + ;; We really only support targeting Linux and mingw. + (setenv "GOOS" (if (string-contains target "mingw") + "windows" + "linux")) + (setenv "GOARM" "7") ; Default is 6, target our armhf-linux architecture. + ;; (setenv "GOMIPS" "hardfloat") ; The default. + ;; (setenv "GOMIPS64" "hardfloat") ; The default. + ;; (setenv "GOPPC64" "power8") ; The default. + )) + (let ((tmpdir (tmpnam))) (match (go-inputs inputs) (((names . directories) ...)