all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* [bug#60847] [PATCH 0/5] Enable cross-compilation for the pyproject-build-system
@ 2023-01-16  4:04 Maxim Cournoyer
  2023-01-16  5:01 ` [bug#60847] [PATCH 0/1] " Maxim Cournoyer
  2023-01-23 13:32 ` [bug#60847] [PATCH v2 0/1] Enable cross-compilation for the pyproject-build-system Maxim Cournoyer
  0 siblings, 2 replies; 19+ messages in thread
From: Maxim Cournoyer @ 2023-01-16  4:04 UTC (permalink / raw)
  To: 60847; +Cc: Maxim Cournoyer

This change enables cross-compilation for the pyproject-build-system, at the
same time paving the way to fix the longstanding bug #25235 (Wrapped python
programs get native-inputs in PYTHONPATH), by keeping the distinction between
native and host (regular) inputs even when not cross-compiling.  A unified
builder is used, which departs from the other build systems (which use one
builder for the native build and another one for the cross build, where
implemented).

If this change is accepted, there would be adjustments needed to some of the
~150 Python packages using pyproject (looking for a native inputs now needs to
be done using the 'native-inputs' instead of 'inputs' argument even in the
native compilation mode).  Over time I think it'd make sense to migrate more
build systems to use this scheme, since it gives more control as to what
inputs go into the wrapper.

A demo package, python-pycryptodome, that contains C extensions is adjusted so
that it can be cross-compiled using this new build system.


Maxim Cournoyer (5):
  gnu: libtommath: Update to 1.2.0-0.03de03d.
  gnu: libtomcrypt: Update to 1.18.2-0.29986d0.
  build: Enable cross-compilation for pyproject-build-system.
  gnu: python-pycryptodome: Fix build and enable cross-compilation.
  gnu: python-coverage: Switch to pyproject-build-system.

 gnu/packages/check.scm                |   2 +-
 gnu/packages/multiprecision.scm       | 179 +++++++++-----------------
 gnu/packages/python-crypto.scm        |  20 ++-
 guix/build-system/pyproject.scm       | 115 ++++++++++-------
 guix/build/pyproject-build-system.scm | 126 ++++++++++++++++--
 guix/packages.scm                     |  46 +++----
 6 files changed, 288 insertions(+), 200 deletions(-)


base-commit: 5c921977179489caef4a9e54ada6696fc86d2f0b
-- 
2.38.1





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

* [bug#60847] [PATCH 0/1] Enable cross-compilation for the pyproject-build-system
  2023-01-16  4:04 [bug#60847] [PATCH 0/5] Enable cross-compilation for the pyproject-build-system Maxim Cournoyer
@ 2023-01-16  5:01 ` Maxim Cournoyer
  2023-01-16  5:01   ` [bug#60847] [PATCH 1/1] build: Enable cross-compilation for pyproject-build-system Maxim Cournoyer
  2023-01-23 13:32 ` [bug#60847] [PATCH v2 0/1] Enable cross-compilation for the pyproject-build-system Maxim Cournoyer
  1 sibling, 1 reply; 19+ messages in thread
From: Maxim Cournoyer @ 2023-01-16  5:01 UTC (permalink / raw)
  To: 60847
  Cc: Josselin Poiret, Tobias Geerinckx-Rice, Maxim Cournoyer,
	Simon Tournier, Mathieu Othacehe, Ludovic Courtès,
	Christopher Baines, Lars-Dominik Braun, Ricardo Wurmus, jgart

This change enables cross-compilation for the pyproject-build-system, at the
same time paving the way to fix the longstanding bug #25235 (Wrapped python
programs get native-inputs in PYTHONPATH), by keeping the distinction between
native and host (regular) inputs even when not cross-compiling.  A unified
builder is used, which departs from the other build systems (which use one
builder for the native build and another one for the cross build, where
implemented).

If this change is accepted, there would be adjustments needed to some of the
~120 Python packages using pyproject (looking for a native inputs now needs to
be done using the 'native-inputs' instead of 'inputs' argument even in the
native compilation mode).  Over time I think it'd make sense to migrate more
build systems to use this scheme, since it gives more control as to what
inputs go into the wrapper.

A demo package, python-pycryptodome, that contains C extensions is adjusted so
that it can be cross-compiled using this new build system.  It is provided
separately as it would need to go to core-updates.


Maxim Cournoyer (1):
  build: Enable cross-compilation for pyproject-build-system.

 guix/build-system/pyproject.scm       | 115 ++++++++++++++---------
 guix/build/pyproject-build-system.scm | 126 +++++++++++++++++++++++---
 guix/packages.scm                     |  46 +++++-----
 3 files changed, 210 insertions(+), 77 deletions(-)


base-commit: 5c921977179489caef4a9e54ada6696fc86d2f0b
-- 
2.38.1





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

* [bug#60847] [PATCH 1/1] build: Enable cross-compilation for pyproject-build-system.
  2023-01-16  5:01 ` [bug#60847] [PATCH 0/1] " Maxim Cournoyer
@ 2023-01-16  5:01   ` Maxim Cournoyer
  0 siblings, 0 replies; 19+ messages in thread
From: Maxim Cournoyer @ 2023-01-16  5:01 UTC (permalink / raw)
  To: 60847
  Cc: Josselin Poiret, Tobias Geerinckx-Rice, Maxim Cournoyer,
	Simon Tournier, Mathieu Othacehe, Ludovic Courtès,
	Christopher Baines, Lars-Dominik Braun, Ricardo Wurmus, jgart

* guix/build-system/pyproject.scm (lower): Do not return #f when TARGET is
set.  Add the target field.  Separate build-inputs from target-inputs, and
extend these with the standard-packages or standard-cross-packages procedures.
(pyproject-build): Add the target, build-inputs, target-inputs, host-inputs
and native-search-paths arguments.  Remove the inputs argument.  Adjust doc.
Adjust the call to the build-side pyproject-build procedure.  Always pass the
target to the gexp->derivation call.
* guix/packages.scm (bag->derivation): Special case the pyproject-build
builder to always use cross-derivation bags
* guix/build/pyproject-build-system.scm (set-paths): New procedure, overriding
that provided by gnu-build-system.
(check) [TARGET]: New argument.  Disable the phase when it is set.
(install): Adjust to look for Python in the native-inputs instead of inputs.
(compile-bytecode): Likewise.
(create-entrypoints): Likewise.
(set-setuptools-env): New procedure.
(sanity-check): Adjust to look for Python in the native-inputs instead of
inputs.
(add-install-to-pythonpath, wrap, rename-pth-file): Likewise.
(%standard-phases): Override set-paths, add-install-to-pythonpath, wrap,
sanity-check and rename-pth-file.  Register set-setuptools-env.

---

 guix/build-system/pyproject.scm       | 115 ++++++++++++++---------
 guix/build/pyproject-build-system.scm | 126 +++++++++++++++++++++++---
 guix/packages.scm                     |  46 +++++-----
 3 files changed, 210 insertions(+), 77 deletions(-)

diff --git a/guix/build-system/pyproject.scm b/guix/build-system/pyproject.scm
index 8f3b562ca3..74d739023f 100644
--- a/guix/build-system/pyproject.scm
+++ b/guix/build-system/pyproject.scm
@@ -1,6 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2021 Lars-Dominik Braun <lars@6xq.net>
 ;;; Copyright © 2022 Marius Bakke <marius@gnu.org>
+;;; Copyright © 2023 Maxim Cournoyer <maxim.cournoyer@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -69,28 +70,37 @@ (define* (lower name
                 #:rest arguments)
   "Return a bag for NAME."
   (define private-keywords
-    '(#:target #:python #:inputs #:native-inputs))
-
-  (and (not target)                               ;XXX: no cross-compilation
-       (bag
-         (name name)
-         (system system)
-         (host-inputs `(,@(if source
-                              `(("source" ,source))
-                              '())
-                        ,@inputs
-
-                        ;; Keep the standard inputs of 'gnu-build-system'.
-                        ,@(standard-packages)))
-         (build-inputs `(("python" ,python)
-                         ("sanity-check.py" ,(local-file sanity-check.py))
-                         ,@native-inputs))
-         (outputs (append outputs '(wheel)))
-         (build pyproject-build)
-         (arguments (strip-keyword-arguments private-keywords arguments)))))
-
-(define* (pyproject-build name inputs
-                          #:key source
+    `(#:python #:inputs #:native-inputs
+      ,@(if target '() '(#:target))))
+
+  (bag
+    (name name)
+    (system system)
+    (target target)
+    (build-inputs `(,@(if source
+                          `(("source" ,source))
+                          '())
+                    ("python" ,python)
+                    ("sanity-check.py" ,(local-file sanity-check.py))
+                    ,@native-inputs
+                    ,@(if target
+                          (standard-cross-packages target 'host)
+                          '())
+                    ;; Keep the standard inputs of 'gnu-build-system'.
+                    ,@(standard-packages)))
+    (host-inputs inputs)
+    (target-inputs (if target
+                       (standard-cross-packages target 'target)
+                       '()))
+    (outputs (append outputs '(wheel)))
+    (build pyproject-build)
+    (arguments (strip-keyword-arguments private-keywords arguments))))
+
+(define* (pyproject-build name
+                          #:key
+                          target
+                          build-inputs target-inputs host-inputs
+                          source
                           (tests? #t)
                           (configure-flags ''())
                           (build-backend #f)
@@ -98,44 +108,63 @@ (define* (pyproject-build name inputs
                           (test-flags ''())
                           (phases '%standard-phases)
                           (outputs '("out" "wheel"))
+                          (native-search-paths '())
                           (search-paths '())
                           (system (%current-system))
                           (guile #f)
                           (imported-modules %pyproject-build-system-modules)
                           (modules '((guix build pyproject-build-system)
                                      (guix build utils))))
-  "Build SOURCE using PYTHON, and with INPUTS."
+  "Build SOURCE using PYTHON, and with BUILD-INPUTS, HOST-INPUTS and
+TARGET-INPUTS (if available)."
   (define build
     (with-imported-modules imported-modules
       #~(begin
           (use-modules #$@(sexp->gexp modules))
 
-          #$(with-build-variables inputs outputs
-              #~(pyproject-build
-                 #:name #$name
-                 #:source #+source
-                 #:configure-flags #$configure-flags
-                 #:system #$system
-                 #:build-backend #$build-backend
-                 #:test-backend #$test-backend
-                 #:test-flags #$test-flags
-                 #:tests? #$tests?
-                 #:phases #$(if (pair? phases)
-                                (sexp->gexp phases)
-                                phases)
-                 #:outputs %outputs
-                 #:search-paths '#$(sexp->gexp
-                                    (map search-path-specification->sexp
-                                         search-paths))
-                 #:inputs %build-inputs)))))
+          (define %build-host-inputs
+            #+(input-tuples->gexp build-inputs))
+
+          (define %build-target-inputs
+            (append #$(input-tuples->gexp host-inputs)
+                    #+(input-tuples->gexp target-inputs)))
+
+          (define %build-inputs
+            (append %build-host-inputs %build-target-inputs))
+
+          (define %outputs
+            #$(outputs->gexp outputs))
 
+          (pyproject-build #:name #$name
+                           #:source #+source
+                           #:configure-flags #$configure-flags
+                           #:system #$system
+                           #:target #$target
+                           #:build-backend #$build-backend
+                           #:test-backend #$test-backend
+                           #:test-flags #$test-flags
+                           #:tests? #$tests?
+                           #:phases #$(if (pair? phases)
+                                          (sexp->gexp phases)
+                                          phases)
+                           #:outputs %outputs
+                           #:search-paths
+                           '#$(sexp->gexp
+                               (map search-path-specification->sexp
+                                    search-paths))
+                           #:native-search-paths
+                           '#$(sexp->gexp
+                               (map search-path-specification->sexp
+                                    native-search-paths))
+                           #:native-inputs %build-host-inputs
+                           #:inputs %build-target-inputs))))
 
   (mlet %store-monad ((guile (package->derivation (or guile (default-guile))
                                                   system #:graft? #f)))
     (gexp->derivation name build
                       #:system system
-                      #:graft? #f                 ;consistent with 'gnu-build'
-                      #:target #f
+                      #:target target
+                      #:graft? #f       ;consistent with 'gnu-build'
                       #:guile-for-build guile)))
 
 (define pyproject-build-system
diff --git a/guix/build/pyproject-build-system.scm b/guix/build/pyproject-build-system.scm
index c69ccc9d64..e51b5cfc43 100644
--- a/guix/build/pyproject-build-system.scm
+++ b/guix/build/pyproject-build-system.scm
@@ -86,6 +86,58 @@ (define-condition-type &cannot-extract-multiple-wheels &python-build-error
 ;; Raised, when no wheel has been built by the build system.
 (define-condition-type &no-wheels-built &python-build-error no-wheels-built?)
 
+;;; XXX: This is the same as in (guix build gnu-build-system), except adjusted
+;;; for the fact that native-inputs always exist now, whether cross-compiling
+;;; or not.  When not cross-compiling, input-directories are appended to
+;;; native-input-directories to so that native-search-paths are computed for
+;;; all inputs.
+(define* (set-paths #:key target inputs native-inputs
+                    search-paths native-search-paths
+                    #:allow-other-keys)
+  (define input-directories
+    ;; The "source" input can be a directory, but we don't want it for search
+    ;; paths.  See <https://issues.guix.gnu.org/44924>.
+    (match (alist-delete "source" inputs)
+      (((_ . dir) ...)
+       dir)))
+
+  (define native-input-directories
+    (match (alist-delete "source" native-inputs)
+      (((_ . dir) ...)
+       dir)))
+
+  ;; Tell 'ld-wrapper' to disallow non-store libraries.
+  (setenv "GUIX_LD_WRAPPER_ALLOW_IMPURITIES" "no")
+
+  ;; When cross building, $PATH must refer only to native (host) inputs since
+  ;; target inputs are not executable.
+  (set-path-environment-variable "PATH" '("bin" "sbin")
+                                 (append native-input-directories
+                                         (if target
+                                             '()
+                                             input-directories)))
+
+  (for-each (match-lambda
+              ((env-var (files ...) separator type pattern)
+               (set-path-environment-variable env-var files
+                                              input-directories
+                                              #:separator separator
+                                              #:type type
+                                              #:pattern pattern)))
+            search-paths)
+
+  (for-each (match-lambda
+              ((env-var (files ...) separator type pattern)
+               (set-path-environment-variable env-var files
+                                              (append native-input-directories
+                                                      (if target
+                                                          '()
+                                                          input-directories))
+                                              #:separator separator
+                                              #:type type
+                                              #:pattern pattern)))
+            native-search-paths))
+
 (define* (build #:key outputs build-backend configure-flags #:allow-other-keys)
   "Build a given Python package."
 
@@ -135,9 +187,9 @@ (define (pyproject.toml->build-backend file)
      wheel-dir
      config-settings)))
 
-(define* (check #:key tests? test-backend test-flags #:allow-other-keys)
+(define* (check #:key target tests? test-backend test-flags #:allow-other-keys)
   "Run the test suite of a given Python package."
-  (if tests?
+  (if (and tests? (not target))
       ;; Unfortunately with PEP 517 there is no common method to specify test
       ;; systems.  Guess test system based on inputs instead.
       (let* ((pytest (which "pytest"))
@@ -172,11 +224,11 @@ (define* (check #:key tests? test-backend test-flags #:allow-other-keys)
           (else (raise (condition (&test-system-not-found))))))
       (format #t "test suite not run~%")))
 
-(define* (install #:key inputs outputs #:allow-other-keys)
+(define* (install #:key native-inputs outputs #:allow-other-keys)
   "Install a wheel file according to PEP 427"
   ;; See https://www.python.org/dev/peps/pep-0427/#installing-a-wheel-distribution-1-0-py32-none-any-whl
-  (let ((site-dir (site-packages inputs outputs))
-        (python (assoc-ref inputs "python"))
+  (let ((site-dir (site-packages native-inputs outputs))
+        (python (assoc-ref native-inputs "python"))
         (out (assoc-ref outputs "out")))
     (define (extract file)
       "Extract wheel (ZIP file) into site-packages directory"
@@ -262,10 +314,10 @@ (define (list-directories base predicate)
                   (expand-data-directory directory)
                   (rmdir directory)) datadirs))))
 
-(define* (compile-bytecode #:key inputs outputs #:allow-other-keys)
+(define* (compile-bytecode #:key native-inputs outputs #:allow-other-keys)
   "Compile installed byte-code in site-packages."
-  (let* ((site-dir (site-packages inputs outputs))
-         (python (assoc-ref inputs "python"))
+  (let* ((site-dir (site-packages native-inputs outputs))
+         (python (assoc-ref native-inputs "python"))
          (major-minor (map string->number
                            (take (string-split (python-version python) #\.) 2)))
          (<3.7? (match major-minor
@@ -281,7 +333,7 @@ (define* (compile-bytecode #:key inputs outputs #:allow-other-keys)
         (invoke "python" "-m" "compileall"
                 "--invalidation-mode=unchecked-hash" site-dir))))
 
-(define* (create-entrypoints #:key inputs outputs #:allow-other-keys)
+(define* (create-entrypoints #:key native-inputs outputs #:allow-other-keys)
   "Implement Entry Points Specification
 (https://packaging.python.org/specifications/entry-points/) by PyPa,
 which creates runnable scripts in bin/ from entry point specification
@@ -337,8 +389,7 @@ (define (create-script path name module function)
 import ~a as mod
 sys.exit (mod.~a ())~%" interpreter module function)))
         (chmod file-path #o755)))
-
-  (let* ((site-dir (site-packages inputs outputs))
+  (let* ((site-dir (site-packages native-inputs outputs))
          (out (assoc-ref outputs "out"))
          (bin-dir (string-append out "/bin"))
          (entry-point-files (find-files site-dir "^entry_points.txt$")))
@@ -358,6 +409,51 @@ (define* (set-SOURCE-DATE-EPOCH* #:rest _)
   ;; not support timestamps before 1980.
   (setenv "SOURCE_DATE_EPOCH" "315619200"))
 
+(define* (set-setuptools-env #:key target #:allow-other-keys)
+  "Set environment variables such as LDSHARED, LDXXSHARED, etc. used by
+setuptools when building native extensions.  This is particularly useful for
+cross-compilation."
+  (define cc-for-target (if target
+                            (string-append target "-gcc")
+                            "gcc"))
+  (define cxx-for-target (if target
+                             (string-append target "-g++")
+                             "g++"))
+  ;; The variables defined here are taken from CPython's configure.ac file.
+  ;; The explanations are those of the Poky (from Yocto) project.
+  (setenv "CC" cc-for-target)
+  (setenv "CXX" cxx-for-target)
+  ;; LDSHARED is the ld *command* used to create shared library.
+  (setenv "LDSHARED" (string-append cc-for-target " -shared"))
+  ;; LDXXSHARED is the ld *command* used to create shared library of C++
+  ;; objects.
+  (setenv "LDCXXSHARED" (string-append cxx-for-target " -shared"))
+  ;; CCSHARED are the C *flags* used to create objects to go into a shared
+  ;; library (module).
+  (setenv "CCSHARED" "-fPIC")
+  ;; LINKFORSHARED are the flags passed to the $(CC) command that links the
+  ;; python executable.
+  (setenv "LINKFORSHARED" "-Xlinker -export-dynamic"))
+
+(define* (sanity-check #:key tests? native-inputs outputs #:allow-other-keys
+                       #:rest args)
+  (apply (assoc-ref python:%standard-phases 'sanity-check)
+         (append args (list #:inputs native-inputs))))
+
+(define* (add-install-to-pythonpath #:key native-inputs outputs
+                                    #:allow-other-keys)
+  "A phase that just wraps the 'add-installed-pythonpath' procedure."
+  (add-installed-pythonpath native-inputs outputs))
+
+(define* (wrap #:key native-inputs outputs #:allow-other-keys #:rest args)
+  (apply (assoc-ref python:%standard-phases 'wrap)
+         (append args (list #:inputs native-inputs))))
+
+(define* (rename-pth-file #:key name native-inputs outputs #:allow-other-keys
+                          #:rest args)
+  (apply (assoc-ref python:%standard-phases 'rename-pth-file)
+         (append args (list #:inputs native-inputs))))
+
 (define %standard-phases
   ;; The build phase only builds C extensions and copies the Python sources,
   ;; while the install phase copies then byte-compiles the sources to the
@@ -365,13 +461,19 @@ (define %standard-phases
   ;; to ease testing the built package.
   (modify-phases python:%standard-phases
     (replace 'set-SOURCE-DATE-EPOCH set-SOURCE-DATE-EPOCH*)
+    (replace 'set-paths set-paths)
+    (add-after 'set-paths 'set-setuptools-env set-setuptools-env)
     (replace 'build build)
     (replace 'install install)
+    (replace 'add-install-to-pythonpath add-install-to-pythonpath)
+    (replace 'wrap wrap)
     (delete 'check)
     ;; Must be before tests, so they can use installed packages’ entry points.
     (add-before 'wrap 'create-entrypoints create-entrypoints)
     (add-after 'wrap 'check check)
-    (add-before 'check 'compile-bytecode compile-bytecode)))
+    (add-before 'check 'compile-bytecode compile-bytecode)
+    (replace 'sanity-check sanity-check)
+    (replace 'rename-pth-file rename-pth-file)))
 
 (define* (pyproject-build #:key inputs (phases %standard-phases)
                           #:allow-other-keys #:rest args)
diff --git a/guix/packages.scm b/guix/packages.scm
index 041a872f9d..6d7df17fc3 100644
--- a/guix/packages.scm
+++ b/guix/packages.scm
@@ -1864,28 +1864,30 @@ (define* (bag->derivation bag #:optional context)
   "Return the derivation to build BAG for SYSTEM.  Optionally, CONTEXT can be
 a package object describing the context in which the call occurs, for improved
 error reporting."
-  (if (bag-target bag)
-      (bag->cross-derivation bag)
-      (mlet* %store-monad ((system ->  (bag-system bag))
-                           (inputs ->  (bag-transitive-inputs bag))
-                           (input-drvs (mapm %store-monad
-                                             (cut expand-input context <> system)
-                                             inputs))
-                           (paths ->   (delete-duplicates
-                                        (append-map (match-lambda
-                                                      ((_ (? package? p) _ ...)
-                                                       (package-native-search-paths
-                                                        p))
-                                                      (_ '()))
-                                                    inputs))))
-        ;; It's possible that INPUTS contains packages that are not 'eq?' but
-        ;; that lead to the same derivation.  Delete those duplicates to avoid
-        ;; issues down the road, such as duplicate entries in '%build-inputs'.
-        (apply (bag-build bag) (bag-name bag)
-               (delete-duplicates input-drvs input=?)
-               #:search-paths paths
-               #:outputs (bag-outputs bag) #:system system
-               (bag-arguments bag)))))
+  (let ((builder-name (procedure-name (bag-build bag))))
+    (if (or (bag-target bag)
+            (eq? 'pyproject-build builder-name))
+        (bag->cross-derivation bag)
+        (mlet* %store-monad ((system ->  (bag-system bag))
+                             (inputs ->  (bag-transitive-inputs bag))
+                             (input-drvs (mapm %store-monad
+                                               (cut expand-input context <> system)
+                                               inputs))
+                             (paths ->   (delete-duplicates
+                                          (append-map (match-lambda
+                                                        ((_ (? package? p) _ ...)
+                                                         (package-native-search-paths
+                                                          p))
+                                                        (_ '()))
+                                                      inputs))))
+          ;; It's possible that INPUTS contains packages that are not 'eq?' but
+          ;; that lead to the same derivation.  Delete those duplicates to avoid
+          ;; issues down the road, such as duplicate entries in '%build-inputs'.
+          (apply (bag-build bag) (bag-name bag)
+                 (delete-duplicates input-drvs input=?)
+                 #:search-paths paths
+                 #:outputs (bag-outputs bag) #:system system
+                 (bag-arguments bag))))))
 
 (define* (bag->cross-derivation bag #:optional context)
   "Return the derivation to build BAG, which is actually a cross build.
-- 
2.38.1





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

* [bug#60847] [PATCH v2 0/1] Enable cross-compilation for the pyproject-build-system
  2023-01-16  4:04 [bug#60847] [PATCH 0/5] Enable cross-compilation for the pyproject-build-system Maxim Cournoyer
  2023-01-16  5:01 ` [bug#60847] [PATCH 0/1] " Maxim Cournoyer
@ 2023-01-23 13:32 ` Maxim Cournoyer
  2023-01-23 13:32   ` [bug#60847] [PATCH v2 1/1] build: Enable cross-compilation for pyproject-build-system Maxim Cournoyer
  2023-01-24  2:05   ` [bug#60847] [PATCH v2 1/1] build: Enable cross-compilation for pyproject-build-system jgart via Guix-patches via
  1 sibling, 2 replies; 19+ messages in thread
From: Maxim Cournoyer @ 2023-01-23 13:32 UTC (permalink / raw)
  To: 60847
  Cc: Josselin Poiret, Tobias Geerinckx-Rice, Maxim Cournoyer,
	Simon Tournier, Mathieu Othacehe, ludo, Christopher Baines,
	Lars-Dominik Braun, Ricardo Wurmus, jgart

This change enables cross-compilation for the pyproject-build-system, at the
same time paving the way to fix the longstanding bug #25235 (Wrapped python
programs get native-inputs in PYTHONPATH), by keeping the distinction between
native and host (regular) inputs even when not cross-compiling.  A unified
builder is used, which departs from the other build systems (which use one
builder for the native build and another one for the cross build, where
implemented).

If this change is accepted, there would be adjustments needed to some of the
~120 Python packages using pyproject (looking for a native inputs now needs to
be done using the 'native-inputs' instead of 'inputs' argument even in the
native compilation mode).  Over time I think it'd make sense to migrate more
build systems to use this scheme, since it gives more control as to what
inputs go into the wrapper.

A demo package, python-pycryptodome, that contains C extensions is adjusted so
that it can be cross-compiled using this new build system.  It is provided
separately as it would need to go to core-updates.

Changes in v2:
- Rebase

Maxim Cournoyer (1):
  build: Enable cross-compilation for pyproject-build-system.

 guix/build-system/pyproject.scm       | 115 ++++++++++++++---------
 guix/build/pyproject-build-system.scm | 126 +++++++++++++++++++++++---
 guix/packages.scm                     |  46 +++++-----
 3 files changed, 210 insertions(+), 77 deletions(-)


base-commit: 3a1b18aa4540e6f96ded0a98dd907a8033262582
-- 
2.39.1





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

* [bug#60847] [PATCH v2 1/1] build: Enable cross-compilation for pyproject-build-system.
  2023-01-23 13:32 ` [bug#60847] [PATCH v2 0/1] Enable cross-compilation for the pyproject-build-system Maxim Cournoyer
@ 2023-01-23 13:32   ` Maxim Cournoyer
  2023-03-06 17:04     ` [bug#60847] [PATCH] Enable cross-compilation for the pyproject-build-system Ludovic Courtès
                       ` (2 more replies)
  2023-01-24  2:05   ` [bug#60847] [PATCH v2 1/1] build: Enable cross-compilation for pyproject-build-system jgart via Guix-patches via
  1 sibling, 3 replies; 19+ messages in thread
From: Maxim Cournoyer @ 2023-01-23 13:32 UTC (permalink / raw)
  To: 60847
  Cc: Josselin Poiret, Tobias Geerinckx-Rice, Maxim Cournoyer,
	Simon Tournier, Mathieu Othacehe, ludo, Christopher Baines,
	Lars-Dominik Braun, Ricardo Wurmus, jgart

* guix/build-system/pyproject.scm (lower): Do not return #f when TARGET is
set.  Add the target field.  Separate build-inputs from target-inputs, and
extend these with the standard-packages or standard-cross-packages procedures.
(pyproject-build): Add the target, build-inputs, target-inputs, host-inputs
and native-search-paths arguments.  Remove the inputs argument.  Adjust doc.
Adjust the call to the build-side pyproject-build procedure.  Always pass the
target to the gexp->derivation call.
* guix/packages.scm (bag->derivation): Special case the pyproject-build
builder to always use cross-derivation bags
* guix/build/pyproject-build-system.scm (set-paths): New procedure, overriding
that provided by gnu-build-system.
(check) [TARGET]: New argument.  Disable the phase when it is set.
(install): Adjust to look for Python in the native-inputs instead of inputs.
(compile-bytecode): Likewise.
(create-entrypoints): Likewise.
(set-setuptools-env): New procedure.
(sanity-check): Adjust to look for Python in the native-inputs instead of
inputs.
(add-install-to-pythonpath, wrap, rename-pth-file): Likewise.
(%standard-phases): Override set-paths, add-install-to-pythonpath, wrap,
sanity-check and rename-pth-file.  Register set-setuptools-env.

---

Changes in v2:
- Rebase

 guix/build-system/pyproject.scm       | 115 ++++++++++++++---------
 guix/build/pyproject-build-system.scm | 126 +++++++++++++++++++++++---
 guix/packages.scm                     |  46 +++++-----
 3 files changed, 210 insertions(+), 77 deletions(-)

diff --git a/guix/build-system/pyproject.scm b/guix/build-system/pyproject.scm
index 8f3b562ca3..74d739023f 100644
--- a/guix/build-system/pyproject.scm
+++ b/guix/build-system/pyproject.scm
@@ -1,6 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2021 Lars-Dominik Braun <lars@6xq.net>
 ;;; Copyright © 2022 Marius Bakke <marius@gnu.org>
+;;; Copyright © 2023 Maxim Cournoyer <maxim.cournoyer@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -69,28 +70,37 @@ (define* (lower name
                 #:rest arguments)
   "Return a bag for NAME."
   (define private-keywords
-    '(#:target #:python #:inputs #:native-inputs))
-
-  (and (not target)                               ;XXX: no cross-compilation
-       (bag
-         (name name)
-         (system system)
-         (host-inputs `(,@(if source
-                              `(("source" ,source))
-                              '())
-                        ,@inputs
-
-                        ;; Keep the standard inputs of 'gnu-build-system'.
-                        ,@(standard-packages)))
-         (build-inputs `(("python" ,python)
-                         ("sanity-check.py" ,(local-file sanity-check.py))
-                         ,@native-inputs))
-         (outputs (append outputs '(wheel)))
-         (build pyproject-build)
-         (arguments (strip-keyword-arguments private-keywords arguments)))))
-
-(define* (pyproject-build name inputs
-                          #:key source
+    `(#:python #:inputs #:native-inputs
+      ,@(if target '() '(#:target))))
+
+  (bag
+    (name name)
+    (system system)
+    (target target)
+    (build-inputs `(,@(if source
+                          `(("source" ,source))
+                          '())
+                    ("python" ,python)
+                    ("sanity-check.py" ,(local-file sanity-check.py))
+                    ,@native-inputs
+                    ,@(if target
+                          (standard-cross-packages target 'host)
+                          '())
+                    ;; Keep the standard inputs of 'gnu-build-system'.
+                    ,@(standard-packages)))
+    (host-inputs inputs)
+    (target-inputs (if target
+                       (standard-cross-packages target 'target)
+                       '()))
+    (outputs (append outputs '(wheel)))
+    (build pyproject-build)
+    (arguments (strip-keyword-arguments private-keywords arguments))))
+
+(define* (pyproject-build name
+                          #:key
+                          target
+                          build-inputs target-inputs host-inputs
+                          source
                           (tests? #t)
                           (configure-flags ''())
                           (build-backend #f)
@@ -98,44 +108,63 @@ (define* (pyproject-build name inputs
                           (test-flags ''())
                           (phases '%standard-phases)
                           (outputs '("out" "wheel"))
+                          (native-search-paths '())
                           (search-paths '())
                           (system (%current-system))
                           (guile #f)
                           (imported-modules %pyproject-build-system-modules)
                           (modules '((guix build pyproject-build-system)
                                      (guix build utils))))
-  "Build SOURCE using PYTHON, and with INPUTS."
+  "Build SOURCE using PYTHON, and with BUILD-INPUTS, HOST-INPUTS and
+TARGET-INPUTS (if available)."
   (define build
     (with-imported-modules imported-modules
       #~(begin
           (use-modules #$@(sexp->gexp modules))
 
-          #$(with-build-variables inputs outputs
-              #~(pyproject-build
-                 #:name #$name
-                 #:source #+source
-                 #:configure-flags #$configure-flags
-                 #:system #$system
-                 #:build-backend #$build-backend
-                 #:test-backend #$test-backend
-                 #:test-flags #$test-flags
-                 #:tests? #$tests?
-                 #:phases #$(if (pair? phases)
-                                (sexp->gexp phases)
-                                phases)
-                 #:outputs %outputs
-                 #:search-paths '#$(sexp->gexp
-                                    (map search-path-specification->sexp
-                                         search-paths))
-                 #:inputs %build-inputs)))))
+          (define %build-host-inputs
+            #+(input-tuples->gexp build-inputs))
+
+          (define %build-target-inputs
+            (append #$(input-tuples->gexp host-inputs)
+                    #+(input-tuples->gexp target-inputs)))
+
+          (define %build-inputs
+            (append %build-host-inputs %build-target-inputs))
+
+          (define %outputs
+            #$(outputs->gexp outputs))
 
+          (pyproject-build #:name #$name
+                           #:source #+source
+                           #:configure-flags #$configure-flags
+                           #:system #$system
+                           #:target #$target
+                           #:build-backend #$build-backend
+                           #:test-backend #$test-backend
+                           #:test-flags #$test-flags
+                           #:tests? #$tests?
+                           #:phases #$(if (pair? phases)
+                                          (sexp->gexp phases)
+                                          phases)
+                           #:outputs %outputs
+                           #:search-paths
+                           '#$(sexp->gexp
+                               (map search-path-specification->sexp
+                                    search-paths))
+                           #:native-search-paths
+                           '#$(sexp->gexp
+                               (map search-path-specification->sexp
+                                    native-search-paths))
+                           #:native-inputs %build-host-inputs
+                           #:inputs %build-target-inputs))))
 
   (mlet %store-monad ((guile (package->derivation (or guile (default-guile))
                                                   system #:graft? #f)))
     (gexp->derivation name build
                       #:system system
-                      #:graft? #f                 ;consistent with 'gnu-build'
-                      #:target #f
+                      #:target target
+                      #:graft? #f       ;consistent with 'gnu-build'
                       #:guile-for-build guile)))
 
 (define pyproject-build-system
diff --git a/guix/build/pyproject-build-system.scm b/guix/build/pyproject-build-system.scm
index c69ccc9d64..e51b5cfc43 100644
--- a/guix/build/pyproject-build-system.scm
+++ b/guix/build/pyproject-build-system.scm
@@ -86,6 +86,58 @@ (define-condition-type &cannot-extract-multiple-wheels &python-build-error
 ;; Raised, when no wheel has been built by the build system.
 (define-condition-type &no-wheels-built &python-build-error no-wheels-built?)
 
+;;; XXX: This is the same as in (guix build gnu-build-system), except adjusted
+;;; for the fact that native-inputs always exist now, whether cross-compiling
+;;; or not.  When not cross-compiling, input-directories are appended to
+;;; native-input-directories to so that native-search-paths are computed for
+;;; all inputs.
+(define* (set-paths #:key target inputs native-inputs
+                    search-paths native-search-paths
+                    #:allow-other-keys)
+  (define input-directories
+    ;; The "source" input can be a directory, but we don't want it for search
+    ;; paths.  See <https://issues.guix.gnu.org/44924>.
+    (match (alist-delete "source" inputs)
+      (((_ . dir) ...)
+       dir)))
+
+  (define native-input-directories
+    (match (alist-delete "source" native-inputs)
+      (((_ . dir) ...)
+       dir)))
+
+  ;; Tell 'ld-wrapper' to disallow non-store libraries.
+  (setenv "GUIX_LD_WRAPPER_ALLOW_IMPURITIES" "no")
+
+  ;; When cross building, $PATH must refer only to native (host) inputs since
+  ;; target inputs are not executable.
+  (set-path-environment-variable "PATH" '("bin" "sbin")
+                                 (append native-input-directories
+                                         (if target
+                                             '()
+                                             input-directories)))
+
+  (for-each (match-lambda
+              ((env-var (files ...) separator type pattern)
+               (set-path-environment-variable env-var files
+                                              input-directories
+                                              #:separator separator
+                                              #:type type
+                                              #:pattern pattern)))
+            search-paths)
+
+  (for-each (match-lambda
+              ((env-var (files ...) separator type pattern)
+               (set-path-environment-variable env-var files
+                                              (append native-input-directories
+                                                      (if target
+                                                          '()
+                                                          input-directories))
+                                              #:separator separator
+                                              #:type type
+                                              #:pattern pattern)))
+            native-search-paths))
+
 (define* (build #:key outputs build-backend configure-flags #:allow-other-keys)
   "Build a given Python package."
 
@@ -135,9 +187,9 @@ (define (pyproject.toml->build-backend file)
      wheel-dir
      config-settings)))
 
-(define* (check #:key tests? test-backend test-flags #:allow-other-keys)
+(define* (check #:key target tests? test-backend test-flags #:allow-other-keys)
   "Run the test suite of a given Python package."
-  (if tests?
+  (if (and tests? (not target))
       ;; Unfortunately with PEP 517 there is no common method to specify test
       ;; systems.  Guess test system based on inputs instead.
       (let* ((pytest (which "pytest"))
@@ -172,11 +224,11 @@ (define* (check #:key tests? test-backend test-flags #:allow-other-keys)
           (else (raise (condition (&test-system-not-found))))))
       (format #t "test suite not run~%")))
 
-(define* (install #:key inputs outputs #:allow-other-keys)
+(define* (install #:key native-inputs outputs #:allow-other-keys)
   "Install a wheel file according to PEP 427"
   ;; See https://www.python.org/dev/peps/pep-0427/#installing-a-wheel-distribution-1-0-py32-none-any-whl
-  (let ((site-dir (site-packages inputs outputs))
-        (python (assoc-ref inputs "python"))
+  (let ((site-dir (site-packages native-inputs outputs))
+        (python (assoc-ref native-inputs "python"))
         (out (assoc-ref outputs "out")))
     (define (extract file)
       "Extract wheel (ZIP file) into site-packages directory"
@@ -262,10 +314,10 @@ (define (list-directories base predicate)
                   (expand-data-directory directory)
                   (rmdir directory)) datadirs))))
 
-(define* (compile-bytecode #:key inputs outputs #:allow-other-keys)
+(define* (compile-bytecode #:key native-inputs outputs #:allow-other-keys)
   "Compile installed byte-code in site-packages."
-  (let* ((site-dir (site-packages inputs outputs))
-         (python (assoc-ref inputs "python"))
+  (let* ((site-dir (site-packages native-inputs outputs))
+         (python (assoc-ref native-inputs "python"))
          (major-minor (map string->number
                            (take (string-split (python-version python) #\.) 2)))
          (<3.7? (match major-minor
@@ -281,7 +333,7 @@ (define* (compile-bytecode #:key inputs outputs #:allow-other-keys)
         (invoke "python" "-m" "compileall"
                 "--invalidation-mode=unchecked-hash" site-dir))))
 
-(define* (create-entrypoints #:key inputs outputs #:allow-other-keys)
+(define* (create-entrypoints #:key native-inputs outputs #:allow-other-keys)
   "Implement Entry Points Specification
 (https://packaging.python.org/specifications/entry-points/) by PyPa,
 which creates runnable scripts in bin/ from entry point specification
@@ -337,8 +389,7 @@ (define (create-script path name module function)
 import ~a as mod
 sys.exit (mod.~a ())~%" interpreter module function)))
         (chmod file-path #o755)))
-
-  (let* ((site-dir (site-packages inputs outputs))
+  (let* ((site-dir (site-packages native-inputs outputs))
          (out (assoc-ref outputs "out"))
          (bin-dir (string-append out "/bin"))
          (entry-point-files (find-files site-dir "^entry_points.txt$")))
@@ -358,6 +409,51 @@ (define* (set-SOURCE-DATE-EPOCH* #:rest _)
   ;; not support timestamps before 1980.
   (setenv "SOURCE_DATE_EPOCH" "315619200"))
 
+(define* (set-setuptools-env #:key target #:allow-other-keys)
+  "Set environment variables such as LDSHARED, LDXXSHARED, etc. used by
+setuptools when building native extensions.  This is particularly useful for
+cross-compilation."
+  (define cc-for-target (if target
+                            (string-append target "-gcc")
+                            "gcc"))
+  (define cxx-for-target (if target
+                             (string-append target "-g++")
+                             "g++"))
+  ;; The variables defined here are taken from CPython's configure.ac file.
+  ;; The explanations are those of the Poky (from Yocto) project.
+  (setenv "CC" cc-for-target)
+  (setenv "CXX" cxx-for-target)
+  ;; LDSHARED is the ld *command* used to create shared library.
+  (setenv "LDSHARED" (string-append cc-for-target " -shared"))
+  ;; LDXXSHARED is the ld *command* used to create shared library of C++
+  ;; objects.
+  (setenv "LDCXXSHARED" (string-append cxx-for-target " -shared"))
+  ;; CCSHARED are the C *flags* used to create objects to go into a shared
+  ;; library (module).
+  (setenv "CCSHARED" "-fPIC")
+  ;; LINKFORSHARED are the flags passed to the $(CC) command that links the
+  ;; python executable.
+  (setenv "LINKFORSHARED" "-Xlinker -export-dynamic"))
+
+(define* (sanity-check #:key tests? native-inputs outputs #:allow-other-keys
+                       #:rest args)
+  (apply (assoc-ref python:%standard-phases 'sanity-check)
+         (append args (list #:inputs native-inputs))))
+
+(define* (add-install-to-pythonpath #:key native-inputs outputs
+                                    #:allow-other-keys)
+  "A phase that just wraps the 'add-installed-pythonpath' procedure."
+  (add-installed-pythonpath native-inputs outputs))
+
+(define* (wrap #:key native-inputs outputs #:allow-other-keys #:rest args)
+  (apply (assoc-ref python:%standard-phases 'wrap)
+         (append args (list #:inputs native-inputs))))
+
+(define* (rename-pth-file #:key name native-inputs outputs #:allow-other-keys
+                          #:rest args)
+  (apply (assoc-ref python:%standard-phases 'rename-pth-file)
+         (append args (list #:inputs native-inputs))))
+
 (define %standard-phases
   ;; The build phase only builds C extensions and copies the Python sources,
   ;; while the install phase copies then byte-compiles the sources to the
@@ -365,13 +461,19 @@ (define %standard-phases
   ;; to ease testing the built package.
   (modify-phases python:%standard-phases
     (replace 'set-SOURCE-DATE-EPOCH set-SOURCE-DATE-EPOCH*)
+    (replace 'set-paths set-paths)
+    (add-after 'set-paths 'set-setuptools-env set-setuptools-env)
     (replace 'build build)
     (replace 'install install)
+    (replace 'add-install-to-pythonpath add-install-to-pythonpath)
+    (replace 'wrap wrap)
     (delete 'check)
     ;; Must be before tests, so they can use installed packages’ entry points.
     (add-before 'wrap 'create-entrypoints create-entrypoints)
     (add-after 'wrap 'check check)
-    (add-before 'check 'compile-bytecode compile-bytecode)))
+    (add-before 'check 'compile-bytecode compile-bytecode)
+    (replace 'sanity-check sanity-check)
+    (replace 'rename-pth-file rename-pth-file)))
 
 (define* (pyproject-build #:key inputs (phases %standard-phases)
                           #:allow-other-keys #:rest args)
diff --git a/guix/packages.scm b/guix/packages.scm
index 041a872f9d..6d7df17fc3 100644
--- a/guix/packages.scm
+++ b/guix/packages.scm
@@ -1864,28 +1864,30 @@ (define* (bag->derivation bag #:optional context)
   "Return the derivation to build BAG for SYSTEM.  Optionally, CONTEXT can be
 a package object describing the context in which the call occurs, for improved
 error reporting."
-  (if (bag-target bag)
-      (bag->cross-derivation bag)
-      (mlet* %store-monad ((system ->  (bag-system bag))
-                           (inputs ->  (bag-transitive-inputs bag))
-                           (input-drvs (mapm %store-monad
-                                             (cut expand-input context <> system)
-                                             inputs))
-                           (paths ->   (delete-duplicates
-                                        (append-map (match-lambda
-                                                      ((_ (? package? p) _ ...)
-                                                       (package-native-search-paths
-                                                        p))
-                                                      (_ '()))
-                                                    inputs))))
-        ;; It's possible that INPUTS contains packages that are not 'eq?' but
-        ;; that lead to the same derivation.  Delete those duplicates to avoid
-        ;; issues down the road, such as duplicate entries in '%build-inputs'.
-        (apply (bag-build bag) (bag-name bag)
-               (delete-duplicates input-drvs input=?)
-               #:search-paths paths
-               #:outputs (bag-outputs bag) #:system system
-               (bag-arguments bag)))))
+  (let ((builder-name (procedure-name (bag-build bag))))
+    (if (or (bag-target bag)
+            (eq? 'pyproject-build builder-name))
+        (bag->cross-derivation bag)
+        (mlet* %store-monad ((system ->  (bag-system bag))
+                             (inputs ->  (bag-transitive-inputs bag))
+                             (input-drvs (mapm %store-monad
+                                               (cut expand-input context <> system)
+                                               inputs))
+                             (paths ->   (delete-duplicates
+                                          (append-map (match-lambda
+                                                        ((_ (? package? p) _ ...)
+                                                         (package-native-search-paths
+                                                          p))
+                                                        (_ '()))
+                                                      inputs))))
+          ;; It's possible that INPUTS contains packages that are not 'eq?' but
+          ;; that lead to the same derivation.  Delete those duplicates to avoid
+          ;; issues down the road, such as duplicate entries in '%build-inputs'.
+          (apply (bag-build bag) (bag-name bag)
+                 (delete-duplicates input-drvs input=?)
+                 #:search-paths paths
+                 #:outputs (bag-outputs bag) #:system system
+                 (bag-arguments bag))))))
 
 (define* (bag->cross-derivation bag #:optional context)
   "Return the derivation to build BAG, which is actually a cross build.
-- 
2.39.1





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

* [bug#60847] [PATCH v2 1/1] build: Enable cross-compilation for pyproject-build-system.
  2023-01-23 13:32 ` [bug#60847] [PATCH v2 0/1] Enable cross-compilation for the pyproject-build-system Maxim Cournoyer
  2023-01-23 13:32   ` [bug#60847] [PATCH v2 1/1] build: Enable cross-compilation for pyproject-build-system Maxim Cournoyer
@ 2023-01-24  2:05   ` jgart via Guix-patches via
  1 sibling, 0 replies; 19+ messages in thread
From: jgart via Guix-patches via @ 2023-01-24  2:05 UTC (permalink / raw)
  To: Maxim Cournoyer, 60847
  Cc: Josselin Poiret, Tobias Geerinckx-Rice, Simon Tournier,
	Mathieu Othacehe, ludo, Christopher Baines, Lars-Dominik Braun,
	Ricardo Wurmus

Hi Maxim,

Sorry, This week is still too busy for me with work to review this. Looking forward to reviewing future patches or these if still needed.

all best,

jgart




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

* [bug#60847] [PATCH] Enable cross-compilation for the pyproject-build-system.
  2023-01-23 13:32   ` [bug#60847] [PATCH v2 1/1] build: Enable cross-compilation for pyproject-build-system Maxim Cournoyer
@ 2023-03-06 17:04     ` Ludovic Courtès
  2023-03-07 14:08       ` Maxim Cournoyer
  2023-03-06 22:56     ` jgart via Guix-patches via
  2023-03-07  0:25     ` jgart via Guix-patches via
  2 siblings, 1 reply; 19+ messages in thread
From: Ludovic Courtès @ 2023-03-06 17:04 UTC (permalink / raw)
  To: Maxim Cournoyer
  Cc: Josselin Poiret, Christopher Baines, Simon Tournier,
	Mathieu Othacehe, 60847, Tobias Geerinckx-Rice,
	Lars-Dominik Braun, Ricardo Wurmus, jgart

Hello,

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

> +++ b/guix/packages.scm
> @@ -1864,28 +1864,30 @@ (define* (bag->derivation bag #:optional context)

[…]

> +  (let ((builder-name (procedure-name (bag-build bag))))
> +    (if (or (bag-target bag)
> +            (eq? 'pyproject-build builder-name))
> +        (bag->cross-derivation bag)

This one part is a showstopper to me, for two reasons:

  1. We cannot rely on ‘procedure-name’ (it’s a debugging aid and it’s
     not guaranteed to return something useful).

  2. Special-casing build systems here is not okay: the bag and build
     system abstractions exist to maintain separation of concerns.

I understand there’s an actual bug to fix and the desire to fix a more
common issue, but I think this one approach is not the way forward.

I hope that makes sense!

Ludo’.




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

* [bug#60847] [PATCH] Enable cross-compilation for the pyproject-build-system.
  2023-01-23 13:32   ` [bug#60847] [PATCH v2 1/1] build: Enable cross-compilation for pyproject-build-system Maxim Cournoyer
  2023-03-06 17:04     ` [bug#60847] [PATCH] Enable cross-compilation for the pyproject-build-system Ludovic Courtès
@ 2023-03-06 22:56     ` jgart via Guix-patches via
  2023-03-07  0:25     ` jgart via Guix-patches via
  2 siblings, 0 replies; 19+ messages in thread
From: jgart via Guix-patches via @ 2023-03-06 22:56 UTC (permalink / raw)
  To: Ludovic Courtès, Maxim Cournoyer
  Cc: Josselin Poiret, Christopher Baines, Simon Tournier,
	Mathieu Othacehe, 60847, Tobias Geerinckx-Rice,
	Lars-Dominik Braun, Ricardo Wurmus

Hi Ludo,

> I understand there’s an actual bug to fix ...

Is the bug you are referencing exist as an open ticket somewhere on our tracker? 

Just trying to get more context on that if it does.

all best,

jgart




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

* [bug#60847] [PATCH] Enable cross-compilation for the pyproject-build-system.
  2023-01-23 13:32   ` [bug#60847] [PATCH v2 1/1] build: Enable cross-compilation for pyproject-build-system Maxim Cournoyer
  2023-03-06 17:04     ` [bug#60847] [PATCH] Enable cross-compilation for the pyproject-build-system Ludovic Courtès
  2023-03-06 22:56     ` jgart via Guix-patches via
@ 2023-03-07  0:25     ` jgart via Guix-patches via
  2 siblings, 0 replies; 19+ messages in thread
From: jgart via Guix-patches via @ 2023-03-07  0:25 UTC (permalink / raw)
  To: Ludovic Courtès, Maxim  Cournoyer
  Cc: Josselin Poiret, Christopher Baines, Simon Tournier,
	Mathieu Othacehe, 60847, Tobias Geerinckx-Rice,
	Lars-Dominik Braun, Ricardo Wurmus

> Is the bug you are referencing exist as an open ticket somewhere on our tracker?

Oooops, Ignore, found it: https://issues.guix.gnu.org/issue/25235

Reread Maxim's original message now ;()




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

* [bug#60847] [PATCH] Enable cross-compilation for the pyproject-build-system.
  2023-03-06 17:04     ` [bug#60847] [PATCH] Enable cross-compilation for the pyproject-build-system Ludovic Courtès
@ 2023-03-07 14:08       ` Maxim Cournoyer
  2023-03-07 15:05         ` Christopher Baines
  2023-03-10  8:57         ` Ludovic Courtès
  0 siblings, 2 replies; 19+ messages in thread
From: Maxim Cournoyer @ 2023-03-07 14:08 UTC (permalink / raw)
  To: Ludovic Courtès
  Cc: Josselin Poiret, Christopher Baines, Simon Tournier,
	Mathieu Othacehe, 60847, Tobias Geerinckx-Rice,
	Lars-Dominik Braun, Ricardo Wurmus, jgart

Hi Ludo!

Ludovic Courtès <ludo@gnu.org> writes:

> Hello,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>> +++ b/guix/packages.scm
>> @@ -1864,28 +1864,30 @@ (define* (bag->derivation bag #:optional context)
>
> […]
>
>> +  (let ((builder-name (procedure-name (bag-build bag))))
>> +    (if (or (bag-target bag)
>> +            (eq? 'pyproject-build builder-name))
>> +        (bag->cross-derivation bag)
>
> This one part is a showstopper to me, for two reasons:
>
>   1. We cannot rely on ‘procedure-name’ (it’s a debugging aid and it’s
>      not guaranteed to return something useful).
>
>   2. Special-casing build systems here is not okay: the bag and build
>      system abstractions exist to maintain separation of concerns.
>
> I understand there’s an actual bug to fix and the desire to fix a more
> common issue, but I think this one approach is not the way forward.
>
> I hope that makes sense!

I agree this is not "pretty", but it would be a "temporary" kludge until
all the build systems can be migrated (and the package adjusted for) the
"new" way, which is: native-inputs and inputs always co-exist, whether
the build is a native one or a cross one.

In light of this, it seems OK to test the water with a not so
significant build system (only a handful of package relies on
pyproject-build-system thus far).  When all the build systems will have
been migrated to the new way (a too big undertaking to be done in one
shot), this kludge can be removed.

Otherwise, could you offer a concrete suggestion as the way forward?  I
appreciate the "that's not the way", but stopping short of suggesting a
better alternative leaves me wanting more :-).

-- 
Thanks,
Maxim




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

* [bug#60847] [PATCH] Enable cross-compilation for the pyproject-build-system.
  2023-03-07 14:08       ` Maxim Cournoyer
@ 2023-03-07 15:05         ` Christopher Baines
  2023-03-07 19:03           ` Maxim Cournoyer
  2023-03-10  8:57         ` Ludovic Courtès
  1 sibling, 1 reply; 19+ messages in thread
From: Christopher Baines @ 2023-03-07 15:05 UTC (permalink / raw)
  To: Maxim Cournoyer; +Cc: 60847

[-- Attachment #1: Type: text/plain, Size: 2408 bytes --]


Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

> Hi Ludo!
>
> Ludovic Courtès <ludo@gnu.org> writes:
>
>> Hello,
>>
>> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>>
>>> +++ b/guix/packages.scm
>>> @@ -1864,28 +1864,30 @@ (define* (bag->derivation bag #:optional context)
>>
>> […]
>>
>>> +  (let ((builder-name (procedure-name (bag-build bag))))
>>> +    (if (or (bag-target bag)
>>> +            (eq? 'pyproject-build builder-name))
>>> +        (bag->cross-derivation bag)
>>
>> This one part is a showstopper to me, for two reasons:
>>
>>   1. We cannot rely on ‘procedure-name’ (it’s a debugging aid and it’s
>>      not guaranteed to return something useful).
>>
>>   2. Special-casing build systems here is not okay: the bag and build
>>      system abstractions exist to maintain separation of concerns.
>>
>> I understand there’s an actual bug to fix and the desire to fix a more
>> common issue, but I think this one approach is not the way forward.
>>
>> I hope that makes sense!
>
> I agree this is not "pretty", but it would be a "temporary" kludge until
> all the build systems can be migrated (and the package adjusted for) the
> "new" way, which is: native-inputs and inputs always co-exist, whether
> the build is a native one or a cross one.
>
> In light of this, it seems OK to test the water with a not so
> significant build system (only a handful of package relies on
> pyproject-build-system thus far).  When all the build systems will have
> been migrated to the new way (a too big undertaking to be done in one
> shot), this kludge can be removed.
>
> Otherwise, could you offer a concrete suggestion as the way forward?  I
> appreciate the "that's not the way", but stopping short of suggesting a
> better alternative leaves me wanting more :-).

I think it would be clearer to see other potential ways forward if the
end goal was more clearly articulated.

Guessing at that here from the changes proposed to guix/packages.scm,
once all build systems are adjusted, cross derivations will be produced
for all bags, regardless of whether there's a target.

That doesn't make much sense to me. One explaination is that the current
naming is confusing when thinking about this goal, so maybe
bag->cross-derivation happens to do what you want it to do in all
circumstances, even when target is #f?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

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

* [bug#60847] [PATCH] Enable cross-compilation for the pyproject-build-system.
  2023-03-07 15:05         ` Christopher Baines
@ 2023-03-07 19:03           ` Maxim Cournoyer
  2023-03-07 19:26             ` Christopher Baines
  0 siblings, 1 reply; 19+ messages in thread
From: Maxim Cournoyer @ 2023-03-07 19:03 UTC (permalink / raw)
  To: Christopher Baines; +Cc: 60847

Hi Christopher,

Christopher Baines <mail@cbaines.net> writes:

> Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
>
>> Hi Ludo!
>>
>> Ludovic Courtès <ludo@gnu.org> writes:
>>
>>> Hello,
>>>
>>> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>>>
>>>> +++ b/guix/packages.scm
>>>> @@ -1864,28 +1864,30 @@ (define* (bag->derivation bag #:optional context)
>>>
>>> […]
>>>
>>>> +  (let ((builder-name (procedure-name (bag-build bag))))
>>>> +    (if (or (bag-target bag)
>>>> +            (eq? 'pyproject-build builder-name))
>>>> +        (bag->cross-derivation bag)
>>>
>>> This one part is a showstopper to me, for two reasons:
>>>
>>>   1. We cannot rely on ‘procedure-name’ (it’s a debugging aid and it’s
>>>      not guaranteed to return something useful).
>>>
>>>   2. Special-casing build systems here is not okay: the bag and build
>>>      system abstractions exist to maintain separation of concerns.
>>>
>>> I understand there’s an actual bug to fix and the desire to fix a more
>>> common issue, but I think this one approach is not the way forward.
>>>
>>> I hope that makes sense!
>>
>> I agree this is not "pretty", but it would be a "temporary" kludge until
>> all the build systems can be migrated (and the package adjusted for) the
>> "new" way, which is: native-inputs and inputs always co-exist, whether
>> the build is a native one or a cross one.
>>
>> In light of this, it seems OK to test the water with a not so
>> significant build system (only a handful of package relies on
>> pyproject-build-system thus far).  When all the build systems will have
>> been migrated to the new way (a too big undertaking to be done in one
>> shot), this kludge can be removed.
>>
>> Otherwise, could you offer a concrete suggestion as the way forward?  I
>> appreciate the "that's not the way", but stopping short of suggesting a
>> better alternative leaves me wanting more :-).
>
> I think it would be clearer to see other potential ways forward if the
> end goal was more clearly articulated.
>
> Guessing at that here from the changes proposed to guix/packages.scm,
> once all build systems are adjusted, cross derivations will be produced
> for all bags, regardless of whether there's a target.
>
> That doesn't make much sense to me. One explaination is that the current
> naming is confusing when thinking about this goal, so maybe
> bag->cross-derivation happens to do what you want it to do in all
> circumstances, even when target is #f?

Thanks for tipping in.  The end goal is to avoid loosing the information
of which inputs are native (build inputs) vs regular in the bag, and yes
bag->cross-derivation allows that.  It appears to me the distinction in
the bag representations (native vs cross) was originally perceived
useful as some kind of optimization (there's less variables to worry
about, and we can squash the inputs/search-paths together, since they're
all native anyway), but this information (currently discarded) ends up
being very useful even on the build side (to wrap only the target
inputs, say, and not all the native/build inputs).

So yes, the change long term would be to integrate the
bag->cross-derivation logic into bag->derivation, at which point it
would be unified for any type of build (the bag representation would be
shared between native and cross builds).

When authoring packages, we'd no longer see the odd idiom '(or
native-inputs inputs)' which results from this implementation detail;
instead we'd always have access to both 'native-inputs' and 'inputs'.

I hope that helps understanding the rationale.

-- 
Thanks,
Maxim




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

* [bug#60847] [PATCH] Enable cross-compilation for the pyproject-build-system.
  2023-03-07 19:03           ` Maxim Cournoyer
@ 2023-03-07 19:26             ` Christopher Baines
  2023-03-10 14:13               ` Maxim Cournoyer
  0 siblings, 1 reply; 19+ messages in thread
From: Christopher Baines @ 2023-03-07 19:26 UTC (permalink / raw)
  To: Maxim Cournoyer; +Cc: 60847

[-- Attachment #1: Type: text/plain, Size: 4284 bytes --]


Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

> Hi Christopher,
>
> Christopher Baines <mail@cbaines.net> writes:
>
>> Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
>>
>>> Hi Ludo!
>>>
>>> Ludovic Courtès <ludo@gnu.org> writes:
>>>
>>>> Hello,
>>>>
>>>> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>>>>
>>>>> +++ b/guix/packages.scm
>>>>> @@ -1864,28 +1864,30 @@ (define* (bag->derivation bag #:optional context)
>>>>
>>>> […]
>>>>
>>>>> +  (let ((builder-name (procedure-name (bag-build bag))))
>>>>> +    (if (or (bag-target bag)
>>>>> +            (eq? 'pyproject-build builder-name))
>>>>> +        (bag->cross-derivation bag)
>>>>
>>>> This one part is a showstopper to me, for two reasons:
>>>>
>>>>   1. We cannot rely on ‘procedure-name’ (it’s a debugging aid and it’s
>>>>      not guaranteed to return something useful).
>>>>
>>>>   2. Special-casing build systems here is not okay: the bag and build
>>>>      system abstractions exist to maintain separation of concerns.
>>>>
>>>> I understand there’s an actual bug to fix and the desire to fix a more
>>>> common issue, but I think this one approach is not the way forward.
>>>>
>>>> I hope that makes sense!
>>>
>>> I agree this is not "pretty", but it would be a "temporary" kludge until
>>> all the build systems can be migrated (and the package adjusted for) the
>>> "new" way, which is: native-inputs and inputs always co-exist, whether
>>> the build is a native one or a cross one.
>>>
>>> In light of this, it seems OK to test the water with a not so
>>> significant build system (only a handful of package relies on
>>> pyproject-build-system thus far).  When all the build systems will have
>>> been migrated to the new way (a too big undertaking to be done in one
>>> shot), this kludge can be removed.
>>>
>>> Otherwise, could you offer a concrete suggestion as the way forward?  I
>>> appreciate the "that's not the way", but stopping short of suggesting a
>>> better alternative leaves me wanting more :-).
>>
>> I think it would be clearer to see other potential ways forward if the
>> end goal was more clearly articulated.
>>
>> Guessing at that here from the changes proposed to guix/packages.scm,
>> once all build systems are adjusted, cross derivations will be produced
>> for all bags, regardless of whether there's a target.
>>
>> That doesn't make much sense to me. One explaination is that the current
>> naming is confusing when thinking about this goal, so maybe
>> bag->cross-derivation happens to do what you want it to do in all
>> circumstances, even when target is #f?
>
> Thanks for tipping in.  The end goal is to avoid loosing the information
> of which inputs are native (build inputs) vs regular in the bag, and yes
> bag->cross-derivation allows that.  It appears to me the distinction in
> the bag representations (native vs cross) was originally perceived
> useful as some kind of optimization (there's less variables to worry
> about, and we can squash the inputs/search-paths together, since they're
> all native anyway), but this information (currently discarded) ends up
> being very useful even on the build side (to wrap only the target
> inputs, say, and not all the native/build inputs).
>
> So yes, the change long term would be to integrate the
> bag->cross-derivation logic into bag->derivation, at which point it
> would be unified for any type of build (the bag representation would be
> shared between native and cross builds).

Thanks for the explanation, so maybe an alternative to trying to get
bag->derivation to function differently for different build systems
would be to push combining the inputs down in to each build system.

Take the gnu-build-system as an example, gnu-build in (guix build-system
gnu) would be changed to take multiple lists of inputs, rather than a
single list. It can then combine the lists of inputs as is done in
bag->derivation, to avoid affecting any packages.

While this does require changing all the build systems, I think it's a
bit more forward thinking compared to trying to add a kludge in to
bag->derivation, since hopefully the change there can be the longer term
one.

Does that make sense?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

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

* [bug#60847] [PATCH] Enable cross-compilation for the pyproject-build-system.
  2023-03-07 14:08       ` Maxim Cournoyer
  2023-03-07 15:05         ` Christopher Baines
@ 2023-03-10  8:57         ` Ludovic Courtès
  2023-03-10 14:21           ` Maxim Cournoyer
  1 sibling, 1 reply; 19+ messages in thread
From: Ludovic Courtès @ 2023-03-10  8:57 UTC (permalink / raw)
  To: Maxim Cournoyer
  Cc: Josselin Poiret, Christopher Baines, Simon Tournier,
	Mathieu Othacehe, 60847, Tobias Geerinckx-Rice,
	Lars-Dominik Braun, Ricardo Wurmus, jgart

Hi Maxim,

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:
>
>> Hello,
>>
>> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>>
>>> +++ b/guix/packages.scm
>>> @@ -1864,28 +1864,30 @@ (define* (bag->derivation bag #:optional context)
>>
>> […]
>>
>>> +  (let ((builder-name (procedure-name (bag-build bag))))
>>> +    (if (or (bag-target bag)
>>> +            (eq? 'pyproject-build builder-name))
>>> +        (bag->cross-derivation bag)
>>
>> This one part is a showstopper to me, for two reasons:
>>
>>   1. We cannot rely on ‘procedure-name’ (it’s a debugging aid and it’s
>>      not guaranteed to return something useful).
>>
>>   2. Special-casing build systems here is not okay: the bag and build
>>      system abstractions exist to maintain separation of concerns.
>>
>> I understand there’s an actual bug to fix and the desire to fix a more
>> common issue, but I think this one approach is not the way forward.
>>
>> I hope that makes sense!
>
> I agree this is not "pretty", but it would be a "temporary" kludge until

To make sure there’s no misunderstanding, I think that’s not okay even
as a “temporary kludge” (I’m not against temporary kludges in general,
I’ve done my share of that ;-), but this would be way too brittle.)

> all the build systems can be migrated (and the package adjusted for) the
> "new" way, which is: native-inputs and inputs always co-exist, whether
> the build is a native one or a cross one.

Again, I’m not sure about the “new way”.  I’m sorry I lack the bandwidth
to review this quickly, especially a wide-ranging change like
inputs/native-inputs.

(Just yesterday I was fixing cross-compilation issues on ‘core-updates’;
that gives a lot of insight as to how native-inputs/inputs play out in
practice.)

[...]

> Otherwise, could you offer a concrete suggestion as the way forward?  I
> appreciate the "that's not the way", but stopping short of suggesting a
> better alternative leaves me wanting more :-).

Yup, I’m sorry I don’t have any suggestions!  Perhaps one suggestion
would be: start the native-inputs/inputs change on a new branch, for all
the build systems (or at least the main ones), and then try to build the
‘core’ subset (which includes cross-compilation) to get an idea of the
kind of code simplification opportunities as well as issues that arise.
I feel like it’s hard to anticipate the impact of such a change without
actually trying.

Thanks,
Ludo’.




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

* [bug#60847] [PATCH] Enable cross-compilation for the pyproject-build-system.
  2023-03-07 19:26             ` Christopher Baines
@ 2023-03-10 14:13               ` Maxim Cournoyer
  0 siblings, 0 replies; 19+ messages in thread
From: Maxim Cournoyer @ 2023-03-10 14:13 UTC (permalink / raw)
  To: Christopher Baines; +Cc: 60847

Hello Christopher,

Christopher Baines <mail@cbaines.net> writes:

[...]

>> Thanks for tipping in.  The end goal is to avoid loosing the information
>> of which inputs are native (build inputs) vs regular in the bag, and yes
>> bag->cross-derivation allows that.  It appears to me the distinction in
>> the bag representations (native vs cross) was originally perceived
>> useful as some kind of optimization (there's less variables to worry
>> about, and we can squash the inputs/search-paths together, since they're
>> all native anyway), but this information (currently discarded) ends up
>> being very useful even on the build side (to wrap only the target
>> inputs, say, and not all the native/build inputs).
>>
>> So yes, the change long term would be to integrate the
>> bag->cross-derivation logic into bag->derivation, at which point it
>> would be unified for any type of build (the bag representation would be
>> shared between native and cross builds).
>
> Thanks for the explanation, so maybe an alternative to trying to get
> bag->derivation to function differently for different build systems
> would be to push combining the inputs down in to each build system.
>
> Take the gnu-build-system as an example, gnu-build in (guix build-system
> gnu) would be changed to take multiple lists of inputs, rather than a
> single list. It can then combine the lists of inputs as is done in
> bag->derivation, to avoid affecting any packages.
>
> While this does require changing all the build systems, I think it's a
> bit more forward thinking compared to trying to add a kludge in to
> bag->derivation, since hopefully the change there can be the longer term
> one.

I'll see if I have a good enough understanding of the code to make sense
of your suggestion.  I'll definitely try it!

Thanks for suggesting.

-- 
Thanks,
Maxim




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

* [bug#60847] [PATCH] Enable cross-compilation for the pyproject-build-system.
  2023-03-10  8:57         ` Ludovic Courtès
@ 2023-03-10 14:21           ` Maxim Cournoyer
  2023-03-10 17:00             ` Ludovic Courtès
  0 siblings, 1 reply; 19+ messages in thread
From: Maxim Cournoyer @ 2023-03-10 14:21 UTC (permalink / raw)
  To: Ludovic Courtès
  Cc: Josselin Poiret, Christopher Baines, Simon Tournier,
	Mathieu Othacehe, 60847, Tobias Geerinckx-Rice,
	Lars-Dominik Braun, Ricardo Wurmus, jgart

Hi Ludovic,

Ludovic Courtès <ludo@gnu.org> writes:

> Hi Maxim,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>> Ludovic Courtès <ludo@gnu.org> writes:
>>
>>> Hello,
>>>
>>> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>>>
>>>> +++ b/guix/packages.scm
>>>> @@ -1864,28 +1864,30 @@ (define* (bag->derivation bag #:optional context)
>>>
>>> […]
>>>
>>>> +  (let ((builder-name (procedure-name (bag-build bag))))
>>>> +    (if (or (bag-target bag)
>>>> +            (eq? 'pyproject-build builder-name))
>>>> +        (bag->cross-derivation bag)
>>>
>>> This one part is a showstopper to me, for two reasons:
>>>
>>>   1. We cannot rely on ‘procedure-name’ (it’s a debugging aid and it’s
>>>      not guaranteed to return something useful).
>>>
>>>   2. Special-casing build systems here is not okay: the bag and build
>>>      system abstractions exist to maintain separation of concerns.
>>>
>>> I understand there’s an actual bug to fix and the desire to fix a more
>>> common issue, but I think this one approach is not the way forward.
>>>
>>> I hope that makes sense!
>>
>> I agree this is not "pretty", but it would be a "temporary" kludge until
>
> To make sure there’s no misunderstanding, I think that’s not okay even
> as a “temporary kludge” (I’m not against temporary kludges in general,
> I’ve done my share of that ;-), but this would be way too brittle.)

"way too brittle" is a strong warning :-).  I don't understand what is
so brittle about it?  It's not like we change the build system names
often.

>> all the build systems can be migrated (and the package adjusted for) the
>> "new" way, which is: native-inputs and inputs always co-exist, whether
>> the build is a native one or a cross one.
>
> Again, I’m not sure about the “new way”.  I’m sorry I lack the bandwidth
> to review this quickly, especially a wide-ranging change like
> inputs/native-inputs.
>
> (Just yesterday I was fixing cross-compilation issues on ‘core-updates’;
> that gives a lot of insight as to how native-inputs/inputs play out in
> practice.)
>
> [...]
>
>> Otherwise, could you offer a concrete suggestion as the way forward?  I
>> appreciate the "that's not the way", but stopping short of suggesting a
>> better alternative leaves me wanting more :-).
>
> Yup, I’m sorry I don’t have any suggestions!

Thanks, it's useful to state that up front.  Otherwise the other side of
the line goes like "... so?" :-).

> Perhaps one suggestion would be: start the native-inputs/inputs change
> on a new branch, for all the build systems (or at least the main
> ones), and then try to build the ‘core’ subset (which includes
> cross-compilation) to get an idea of the kind of code simplification
> opportunities as well as issues that arise.  I feel like it’s hard to
> anticipate the impact of such a change without actually trying.

I can do this; the exercise would be similar to what I've done for
pyproject-build-system, where I was able to build complex packages (both
natively and cross-compiled) with the new same-bag representation
change, but to a larger scale.  It's more work, which is why I would
have preferred to contain its scope to get started, but the changes were
easy, IIRC.

Basically, it forces us to review all the (assoc-ref inputs
"package-name") or (search-input-file inputs "file") procedure calls and
remove the assumption that native-inputs are also inputs for native
builds (that'll fix multiple cross-compilation bugs at the same time, so
it's a good thing to do anyway).

To be continued!

-- 
Thanks,
Maxim




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

* [bug#60847] [PATCH] Enable cross-compilation for the pyproject-build-system.
  2023-03-10 14:21           ` Maxim Cournoyer
@ 2023-03-10 17:00             ` Ludovic Courtès
  2023-03-12  4:05               ` Maxim Cournoyer
  2023-03-12  4:05               ` Maxim Cournoyer
  0 siblings, 2 replies; 19+ messages in thread
From: Ludovic Courtès @ 2023-03-10 17:00 UTC (permalink / raw)
  To: Maxim Cournoyer
  Cc: Josselin Poiret, Christopher Baines, Simon Tournier,
	Mathieu Othacehe, 60847, Tobias Geerinckx-Rice,
	Lars-Dominik Braun, Ricardo Wurmus, jgart

Hi Maxim,

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

[...]

>>>>> +  (let ((builder-name (procedure-name (bag-build bag))))
>>>>> +    (if (or (bag-target bag)
>>>>> +            (eq? 'pyproject-build builder-name))
>>>>> +        (bag->cross-derivation bag)
>>>>
>>>> This one part is a showstopper to me, for two reasons:
>>>>
>>>>   1. We cannot rely on ‘procedure-name’ (it’s a debugging aid and it’s
>>>>      not guaranteed to return something useful).
>>>>
>>>>   2. Special-casing build systems here is not okay: the bag and build
>>>>      system abstractions exist to maintain separation of concerns.
>>>>
>>>> I understand there’s an actual bug to fix and the desire to fix a more
>>>> common issue, but I think this one approach is not the way forward.
>>>>
>>>> I hope that makes sense!
>>>
>>> I agree this is not "pretty", but it would be a "temporary" kludge until
>>
>> To make sure there’s no misunderstanding, I think that’s not okay even
>> as a “temporary kludge” (I’m not against temporary kludges in general,
>> I’ve done my share of that ;-), but this would be way too brittle.)
>
> "way too brittle" is a strong warning :-).

It is; I’m objecting to this change.

> I don't understand what is
> so brittle about it?  It's not like we change the build system names
> often.

I have little to add to what I wrote above: procedure names don’t mean
anything, especially with higher-order functions, and separation of
concerns is an important design principle.

I’m speaking with 15+ years of experience with Guile; I don’t mind being
challenged, it’s healthy, but I think we also need to recognize the
expertise of each other and encounter each other halfway.

[...]

>> Perhaps one suggestion would be: start the native-inputs/inputs change
>> on a new branch, for all the build systems (or at least the main
>> ones), and then try to build the ‘core’ subset (which includes
>> cross-compilation) to get an idea of the kind of code simplification
>> opportunities as well as issues that arise.  I feel like it’s hard to
>> anticipate the impact of such a change without actually trying.
>
> I can do this; the exercise would be similar to what I've done for
> pyproject-build-system, where I was able to build complex packages (both
> natively and cross-compiled) with the new same-bag representation
> change, but to a larger scale.  It's more work, which is why I would
> have preferred to contain its scope to get started, but the changes were
> easy, IIRC.

Yes, I agree it’s more work, but it’s dissimilar in that you’ll be
confronted with tons of weird cases, some of which we can already
anticipate, but probably others we’ll probably be surprised to discover.

> Basically, it forces us to review all the (assoc-ref inputs
> "package-name") or (search-input-file inputs "file") procedure calls and
> remove the assumption that native-inputs are also inputs for native
> builds (that'll fix multiple cross-compilation bugs at the same time, so
> it's a good thing to do anyway).

I do hope it’ll be this simple, but I just can’t tell.

Thank you,
Ludo’.




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

* [bug#60847] [PATCH] Enable cross-compilation for the pyproject-build-system.
  2023-03-10 17:00             ` Ludovic Courtès
@ 2023-03-12  4:05               ` Maxim Cournoyer
  2023-03-12  4:05               ` Maxim Cournoyer
  1 sibling, 0 replies; 19+ messages in thread
From: Maxim Cournoyer @ 2023-03-12  4:05 UTC (permalink / raw)
  To: Ludovic Courtès
  Cc: Josselin Poiret, Christopher Baines, Simon Tournier,
	Mathieu Othacehe, 60847, Tobias Geerinckx-Rice,
	Lars-Dominik Braun, Ricardo Wurmus, jgart

Hi,

Ludovic Courtès <ludo@gnu.org> writes:

> Hi Maxim,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
> [...]
>
>>>>>> +  (let ((builder-name (procedure-name (bag-build bag))))
>>>>>> +    (if (or (bag-target bag)
>>>>>> +            (eq? 'pyproject-build builder-name))
>>>>>> +        (bag->cross-derivation bag)
>>>>>
>>>>> This one part is a showstopper to me, for two reasons:
>>>>>
>>>>>   1. We cannot rely on ‘procedure-name’ (it’s a debugging aid and it’s
>>>>>      not guaranteed to return something useful).
>>>>>
>>>>>   2. Special-casing build systems here is not okay: the bag and build
>>>>>      system abstractions exist to maintain separation of concerns.
>>>>>
>>>>> I understand there’s an actual bug to fix and the desire to fix a more
>>>>> common issue, but I think this one approach is not the way forward.
>>>>>
>>>>> I hope that makes sense!
>>>>
>>>> I agree this is not "pretty", but it would be a "temporary" kludge until
>>>
>>> To make sure there’s no misunderstanding, I think that’s not okay even
>>> as a “temporary kludge” (I’m not against temporary kludges in general,
>>> I’ve done my share of that ;-), but this would be way too brittle.)
>>
>> "way too brittle" is a strong warning :-).
>
> It is; I’m objecting to this change.

OK.

>> I don't understand what is
>> so brittle about it?  It's not like we change the build system names
>> often.
>
> I have little to add to what I wrote above: procedure names don’t mean
> anything, especially with higher-order functions, and separation of
> concerns is an important design principle.
>
> I’m speaking with 15+ years of experience with Guile; I don’t mind being
> challenged, it’s healthy, but I think we also need to recognize the
> expertise of each other and encounter each other halfway.

When I don't understand something, I ask for an explanation.  I'm happy
(and greatly value) that my correspondent has 15 years of experience
with Guile, but I'll still ask if something is unclear.  I hope you
don't mind.

> [...]
>
>>> Perhaps one suggestion would be: start the native-inputs/inputs change
>>> on a new branch, for all the build systems (or at least the main
>>> ones), and then try to build the ‘core’ subset (which includes
>>> cross-compilation) to get an idea of the kind of code simplification
>>> opportunities as well as issues that arise.  I feel like it’s hard to
>>> anticipate the impact of such a change without actually trying.
>>
>> I can do this; the exercise would be similar to what I've done for
>> pyproject-build-system, where I was able to build complex packages (both
>> natively and cross-compiled) with the new same-bag representation
>> change, but to a larger scale.  It's more work, which is why I would
>> have preferred to contain its scope to get started, but the changes were
>> easy, IIRC.
>
> Yes, I agree it’s more work, but it’s dissimilar in that you’ll be
> confronted with tons of weird cases, some of which we can already
> anticipate, but probably others we’ll probably be surprised to discover.

OK.  I'll put this to the test, when I'm done with the current Ruby
rabbit-hole I've been pursuing.  Thanks for the feedback!

-- 
Thanks,
Maxim




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

* [bug#60847] [PATCH] Enable cross-compilation for the pyproject-build-system.
  2023-03-10 17:00             ` Ludovic Courtès
  2023-03-12  4:05               ` Maxim Cournoyer
@ 2023-03-12  4:05               ` Maxim Cournoyer
  1 sibling, 0 replies; 19+ messages in thread
From: Maxim Cournoyer @ 2023-03-12  4:05 UTC (permalink / raw)
  To: Ludovic Courtès
  Cc: Josselin Poiret, Christopher Baines, Simon Tournier,
	Mathieu Othacehe, 60847, Tobias Geerinckx-Rice,
	Lars-Dominik Braun, Ricardo Wurmus, jgart

Hi,

Ludovic Courtès <ludo@gnu.org> writes:

> Hi Maxim,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
> [...]
>
>>>>>> +  (let ((builder-name (procedure-name (bag-build bag))))
>>>>>> +    (if (or (bag-target bag)
>>>>>> +            (eq? 'pyproject-build builder-name))
>>>>>> +        (bag->cross-derivation bag)
>>>>>
>>>>> This one part is a showstopper to me, for two reasons:
>>>>>
>>>>>   1. We cannot rely on ‘procedure-name’ (it’s a debugging aid and it’s
>>>>>      not guaranteed to return something useful).
>>>>>
>>>>>   2. Special-casing build systems here is not okay: the bag and build
>>>>>      system abstractions exist to maintain separation of concerns.
>>>>>
>>>>> I understand there’s an actual bug to fix and the desire to fix a more
>>>>> common issue, but I think this one approach is not the way forward.
>>>>>
>>>>> I hope that makes sense!
>>>>
>>>> I agree this is not "pretty", but it would be a "temporary" kludge until
>>>
>>> To make sure there’s no misunderstanding, I think that’s not okay even
>>> as a “temporary kludge” (I’m not against temporary kludges in general,
>>> I’ve done my share of that ;-), but this would be way too brittle.)
>>
>> "way too brittle" is a strong warning :-).
>
> It is; I’m objecting to this change.

OK.

>> I don't understand what is
>> so brittle about it?  It's not like we change the build system names
>> often.
>
> I have little to add to what I wrote above: procedure names don’t mean
> anything, especially with higher-order functions, and separation of
> concerns is an important design principle.
>
> I’m speaking with 15+ years of experience with Guile; I don’t mind being
> challenged, it’s healthy, but I think we also need to recognize the
> expertise of each other and encounter each other halfway.

When I don't understand something, I ask for an explanation.  I'm happy
(and greatly value) that my correspondent has 15 years of experience
with Guile, but I'll still ask if something is unclear.  I hope you
don't mind.

> [...]
>
>>> Perhaps one suggestion would be: start the native-inputs/inputs change
>>> on a new branch, for all the build systems (or at least the main
>>> ones), and then try to build the ‘core’ subset (which includes
>>> cross-compilation) to get an idea of the kind of code simplification
>>> opportunities as well as issues that arise.  I feel like it’s hard to
>>> anticipate the impact of such a change without actually trying.
>>
>> I can do this; the exercise would be similar to what I've done for
>> pyproject-build-system, where I was able to build complex packages (both
>> natively and cross-compiled) with the new same-bag representation
>> change, but to a larger scale.  It's more work, which is why I would
>> have preferred to contain its scope to get started, but the changes were
>> easy, IIRC.
>
> Yes, I agree it’s more work, but it’s dissimilar in that you’ll be
> confronted with tons of weird cases, some of which we can already
> anticipate, but probably others we’ll probably be surprised to discover.

OK.  I'll put this to the test, when I'm done with the current Ruby
rabbit-hole I've been pursuing.  Thanks for the feedback!

-- 
Thanks,
Maxim




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

end of thread, other threads:[~2023-03-12  4:06 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-16  4:04 [bug#60847] [PATCH 0/5] Enable cross-compilation for the pyproject-build-system Maxim Cournoyer
2023-01-16  5:01 ` [bug#60847] [PATCH 0/1] " Maxim Cournoyer
2023-01-16  5:01   ` [bug#60847] [PATCH 1/1] build: Enable cross-compilation for pyproject-build-system Maxim Cournoyer
2023-01-23 13:32 ` [bug#60847] [PATCH v2 0/1] Enable cross-compilation for the pyproject-build-system Maxim Cournoyer
2023-01-23 13:32   ` [bug#60847] [PATCH v2 1/1] build: Enable cross-compilation for pyproject-build-system Maxim Cournoyer
2023-03-06 17:04     ` [bug#60847] [PATCH] Enable cross-compilation for the pyproject-build-system Ludovic Courtès
2023-03-07 14:08       ` Maxim Cournoyer
2023-03-07 15:05         ` Christopher Baines
2023-03-07 19:03           ` Maxim Cournoyer
2023-03-07 19:26             ` Christopher Baines
2023-03-10 14:13               ` Maxim Cournoyer
2023-03-10  8:57         ` Ludovic Courtès
2023-03-10 14:21           ` Maxim Cournoyer
2023-03-10 17:00             ` Ludovic Courtès
2023-03-12  4:05               ` Maxim Cournoyer
2023-03-12  4:05               ` Maxim Cournoyer
2023-03-06 22:56     ` jgart via Guix-patches via
2023-03-07  0:25     ` jgart via Guix-patches via
2023-01-24  2:05   ` [bug#60847] [PATCH v2 1/1] build: Enable cross-compilation for pyproject-build-system jgart via Guix-patches via

Code repositories for project(s) associated with this external index

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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.