diff mbox series

[bug#48767] gnu: gcc-arm-none-eabi-7-2018-q2-update: Simplify build

Message ID SN6PR05MB4031A60A6B1D19CB3BCFF8CAC5A79@SN6PR05MB4031.namprd05.prod.outlook.com
State New
Headers show
Series [bug#48767] gnu: gcc-arm-none-eabi-7-2018-q2-update: Simplify build | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch success View Laminar job
cbaines/issue success View issue
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch success View Laminar job
cbaines/issue success View issue
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch success View Laminar job
cbaines/issue success View issue

Commit Message

Morgan Smith Sept. 27, 2021, 9:59 p.m. UTC 
From: Morgan Smith <Morgan.J.Smith@outlook.com>

* gnu/packages/embedded.scm
(gcc-arm-none-eabi-4.9)[phases]: Add expand-version-string phase

(gcc-arm-none-eabi-7-2018-q2-update): Inherit from gcc-arm-none-eabi-4.9 and
remove all the redundant bits.
---


Hello Maxime,

Thanks for the review!

Is this the appropriate time to tell you I also have a very eerily similar
patch set read to go for a riscv cross compiler? Maybe trying to make this
stuff reusable is a good thing to do. First step is to clean up the arm stuff I
suppose.

I pretty sure this patch doesn't really change the packages at all in any
meaningful way. Adding the DEV-PHASE version thing means that now when people
type 'arm-none-eabi-gcc --version' they'll see in brackets the guix version
number beside the actual version number. Also
gcc-arm-none-eabi-7-2018-q2-update now has the 'with-multilib-list' configure
flag set twice but it's pretty clear it just uses the latter value as it won't
even compile with the inherited flag value.

There seems to be a weird glitch when you inherit a phase the uses
`this-package' but you also specify arguments. I found a work around though.

To test this patch I compiled gcc-arm-none-eabi-4.9, gcc-arm-none-eabi-6, and
gcc-arm-none-eabi-7-2018-q2-update successfully. Than I installed
arm-none-eabi-toolchain and made sure it could successfully compile the arm
stuff I'm working on. Everything seems to work great.


Morgan



 gnu/packages/embedded.scm | 144 +++++++++++++-------------------------
 1 file changed, 48 insertions(+), 96 deletions(-)

Comments

M Oct. 1, 2021, 8:56 a.m. UTC | #1 
Morgan.J.Smith@outlook.com schreef op ma 27-09-2021 om 17:59 [-0400]:
> From: Morgan Smith <Morgan.J.Smith@outlook.com>
> 
> * gnu/packages/embedded.scm
> (gcc-arm-none-eabi-4.9)[phases]: Add expand-version-string phase
> 
> (gcc-arm-none-eabi-7-2018-q2-update): Inherit from gcc-arm-none-eabi-4.9 and
> remove all the redundant bits.
> ---

I see you moved the definition of gcc-arm-none-eabi-7-2018-q2-update.  This patch
would be easier to review if it wasn't moved, as the diff would then be smaller.

> 
> Hello Maxime,
> 
> Thanks for the review!
> 
> Is this the appropriate time to tell you I also have a very eerily similar
> patch set read to go for a riscv cross compiler? Maybe trying to make this
> stuff reusable is a good thing to do. First step is to clean up the arm stuff I
> suppose.

I'm not really familiar with cross-compilation as done in (gnu packages embedded);
I'm more familiar with cross-compilation with "guix build PACKAGE --target=riscv64-...".
If you're doing the latter, you'll need to look at glibc-dynamic-linker in
(gnu packages bootstrap) and (gnu packages cross-base).

> I pretty sure this patch doesn't really change the packages at all in any
> meaningful way. Adding the DEV-PHASE version thing means that now when people
> type 'arm-none-eabi-gcc --version' they'll see in brackets the guix version
> number beside the actual version number.

> Also gcc-arm-none-eabi-7-2018-q2-update now has the 'with-multilib-list' configure
> flag set twice but it's pretty clear it just uses the latter value as it won't
> even compile with the inherited flag value.
> 
> There seems to be a weird glitch when you inherit a phase the uses
> `this-package' but you also specify arguments. I found a work around though.

Alternatively, you could use

  (receive (name version)
    (package-name->name+version (strip-store-file-name (assoc-ref outputs "out")))
    (write version to gcc/DEV-PHASE))

in the 'expand-version-string build phase of gcc-arm-none-eabi-4.9.
Or set the version in a snippet of the 'origin' records (gcc/DEV-PHASE is source
code, right?).  Possibly using (display ,version) instead of
(display ,(package-version this-package)) avoids the issue (not 100% sure though).

[...]

> +;;; The following definitions are for the "7-2018-q2-update" variant of the
> +;;; ARM cross toolchain as offered on https://developer.arm.com
> +(define-public gcc-arm-none-eabi-7-2018-q2-update
> +  (let ((revision "2")
> +        (svn-revision 261907))
> +    (package (inherit gcc-arm-none-eabi-6)
> +      (version (string-append "7-2018-q2-update-"

The version strings of GCC are things like "11.2.0" or "6.5.0",
not strings like "7.5.0".  As this is a SVN snapshot, and appears to be
based on the 7.?.? series, maybe something like "7.MINOR.PATCH-REVISION-SVNREVISION"
would be appropriate?

I'm wondering why a SVN revision revision is used, as presumably the embedded-7-branch
has been merged and would be available from an appropriately-configured gcc@8.6.0, gcc@9.4.0,
or gcc@10.3.0?

A comment next to gcc-arm-none-eabi-4.9 says

;; We must not use the released GCC sources here, because the cross-compiler
;; does not produce working binaries.  Instead we take the very same SVN
;; revision from the branch that is used for a release of the "GCC ARM
;; embedded" project on launchpad.
;; See https://launchpadlibrarian.net/218827644/release.txt

but maybe these bugs have been solved by now, or is using the branch still required?

> +                              revision "." (number->string svn-revision)))
> +      (source
> +       (origin
> +         (method svn-fetch)
> +         (uri (svn-reference
> +               (url "svn://gcc.gnu.org/svn/gcc/branches/ARM/embedded-7-branch/")
> +               (revision svn-revision)))
> +         (file-name (string-append "gcc-arm-embedded-" version "-checkout"))
> +         (sha256
> +          (base32
> +           "192ggs63bixf3irpijgfkjks73yx1r3a4i6grk1y0i0iny76pmx5"))
> +         (patches
> +          (append
> +           (origin-patches (package-source gcc-7))
> +           (search-patches "gcc-7-cross-environment-variables.patch")))))
> +      (arguments
> +       (substitute-keyword-arguments (package-arguments gcc-arm-none-eabi-4.9)
> +         ((#:configure-flags flags)
> +          ;; I add this flag on the end so it overrides the previously set value
> +          `(append ,flags (list "--with-multilib-list=rmprofile")))
> +         ((#:phases phases)
> +          `(modify-phases ,phases
> +             ;; XXX: I replicated this phase just so that the version is
> +             ;; correct.  This phase automatically gets the correct version if
> +             ;; I don't specify any arguments for a package for some reason
> +             ;; (see gcc-arm-none-eabi-6)
> +             (replace 'expand-version-string
> +               (lambda _
> +                 (make-file-writable "gcc/DEV-PHASE")
> +                 (with-output-to-file "gcc/DEV-PHASE"
> +                   (lambda ()
> +                     (display ,(package-version this-package)))))))))))))
> +

Greetings,
Maxime.
diff mbox series

Patch

diff --git a/gnu/packages/embedded.scm b/gnu/packages/embedded.scm
index f388c11c3d..809708187f 100644
--- a/gnu/packages/embedded.scm
+++ b/gnu/packages/embedded.scm
@@ -115,13 +115,17 @@ 
                             ":"))
                    (format #t
                            "environment variable `CPLUS_INCLUDE_PATH' changed to ~a~%"
-                           (getenv "CPLUS_INCLUDE_PATH"))
-                   #t)))
+                           (getenv "CPLUS_INCLUDE_PATH")))))
              (add-after 'unpack 'fix-genmultilib
                (lambda _
                  (substitute* "gcc/genmultilib"
-                   (("#!/bin/sh") (string-append "#!" (which "sh"))))
-                 #t))))
+                   (("#!/bin/sh") (string-append "#!" (which "sh"))))))
+             (add-after 'unpack 'expand-version-string
+               (lambda _
+                 (make-file-writable "gcc/DEV-PHASE")
+                 (with-output-to-file "gcc/DEV-PHASE"
+                   (lambda ()
+                     (display ,(package-version this-package))))))))
          ((#:configure-flags flags)
           ;; The configure flags are largely identical to the flags used by the
           ;; "GCC ARM embedded" project.
@@ -166,6 +170,46 @@ 
                       (search-patches "gcc-6-cross-environment-variables.patch"
                                       "gcc-6-arm-none-eabi-multilib.patch")))))))
 
+;;; The following definitions are for the "7-2018-q2-update" variant of the
+;;; ARM cross toolchain as offered on https://developer.arm.com
+(define-public gcc-arm-none-eabi-7-2018-q2-update
+  (let ((revision "2")
+        (svn-revision 261907))
+    (package (inherit gcc-arm-none-eabi-6)
+      (version (string-append "7-2018-q2-update-"
+                              revision "." (number->string svn-revision)))
+      (source
+       (origin
+         (method svn-fetch)
+         (uri (svn-reference
+               (url "svn://gcc.gnu.org/svn/gcc/branches/ARM/embedded-7-branch/")
+               (revision svn-revision)))
+         (file-name (string-append "gcc-arm-embedded-" version "-checkout"))
+         (sha256
+          (base32
+           "192ggs63bixf3irpijgfkjks73yx1r3a4i6grk1y0i0iny76pmx5"))
+         (patches
+          (append
+           (origin-patches (package-source gcc-7))
+           (search-patches "gcc-7-cross-environment-variables.patch")))))
+      (arguments
+       (substitute-keyword-arguments (package-arguments gcc-arm-none-eabi-4.9)
+         ((#:configure-flags flags)
+          ;; I add this flag on the end so it overrides the previously set value
+          `(append ,flags (list "--with-multilib-list=rmprofile")))
+         ((#:phases phases)
+          `(modify-phases ,phases
+             ;; XXX: I replicated this phase just so that the version is
+             ;; correct.  This phase automatically gets the correct version if
+             ;; I don't specify any arguments for a package for some reason
+             ;; (see gcc-arm-none-eabi-6)
+             (replace 'expand-version-string
+               (lambda _
+                 (make-file-writable "gcc/DEV-PHASE")
+                 (with-output-to-file "gcc/DEV-PHASE"
+                   (lambda ()
+                     (display ,(package-version this-package)))))))))))))
+
 (define-public newlib-arm-none-eabi
   (package
     (name "newlib")
@@ -268,98 +312,6 @@  usable on embedded products.")
     (synopsis "Newlib variant for small systems with limited memory")))
 
 
-;;; The following definitions are for the "7-2018-q2-update" variant of the
-;;; ARM cross toolchain as offered on https://developer.arm.com
-(define-public gcc-arm-none-eabi-7-2018-q2-update
-  (let ((xgcc (cross-gcc "arm-none-eabi"
-                         #:xgcc gcc-7
-                         #:xbinutils (cross-binutils "arm-none-eabi")))
-        (revision "1")
-        (svn-revision 261907))
-    (package (inherit xgcc)
-      (version (string-append "7-2018-q2-update-"
-                              revision "." (number->string svn-revision)))
-      (source
-       (origin
-         (method svn-fetch)
-         (uri (svn-reference
-               (url "svn://gcc.gnu.org/svn/gcc/branches/ARM/embedded-7-branch/")
-               (revision svn-revision)))
-         (file-name (string-append "gcc-arm-embedded-" version "-checkout"))
-         (sha256
-          (base32
-           "192ggs63bixf3irpijgfkjks73yx1r3a4i6grk1y0i0iny76pmx5"))
-         (patches
-          (append
-           (origin-patches (package-source gcc-7))
-           (search-patches "gcc-7-cross-environment-variables.patch")))))
-      (native-inputs
-       `(("flex" ,flex)
-         ("isl" ,isl-0.18)
-         ,@(alist-delete "isl" (package-native-inputs xgcc))))
-      (arguments
-       (substitute-keyword-arguments (package-arguments xgcc)
-         ((#:phases phases)
-          `(modify-phases ,phases
-             (add-after 'unpack 'expand-version-string
-               (lambda _
-                 (make-file-writable "gcc/DEV-PHASE")
-                 (with-output-to-file "gcc/DEV-PHASE"
-                   (lambda ()
-                     (display "7-2018-q2-update")))
-                 #t))
-             (add-after 'unpack 'fix-genmultilib
-               (lambda _
-                 (substitute* "gcc/genmultilib"
-                   (("#!/bin/sh") (string-append "#!" (which "sh"))))
-                 #t))
-             (add-after 'set-paths 'augment-CPLUS_INCLUDE_PATH
-               (lambda* (#:key inputs #:allow-other-keys)
-                 (let ((gcc (assoc-ref inputs  "gcc")))
-                   ;; Remove the default compiler from CPLUS_INCLUDE_PATH to
-                   ;; prevent header conflict with the GCC from native-inputs.
-                   (setenv "CPLUS_INCLUDE_PATH"
-                           (string-join
-                            (delete (string-append gcc "/include/c++")
-                                    (string-split (getenv "CPLUS_INCLUDE_PATH")
-                                                  #\:))
-                            ":"))
-                   (format #t
-                           "environment variable `CPLUS_INCLUDE_PATH' changed to ~a~%"
-                           (getenv "CPLUS_INCLUDE_PATH"))
-                   #t)))))
-         ((#:configure-flags flags)
-          ;; The configure flags are largely identical to the flags used by the
-          ;; "GCC ARM embedded" project.
-          `(append (list "--enable-multilib"
-                         "--with-newlib"
-                         "--with-multilib-list=rmprofile"
-                         "--with-host-libstdcxx=-static-libgcc -Wl,-Bstatic,-lstdc++,-Bdynamic -lm"
-                         "--enable-plugins"
-                         "--disable-decimal-float"
-                         "--disable-libffi"
-                         "--disable-libgomp"
-                         "--disable-libmudflap"
-                         "--disable-libquadmath"
-                         "--disable-libssp"
-                         "--disable-libstdcxx-pch"
-                         "--disable-nls"
-                         "--disable-shared"
-                         "--disable-threads"
-                         "--disable-tls")
-                   (delete "--disable-multilib" ,flags)))))
-      (native-search-paths
-       (list (search-path-specification
-              (variable "CROSS_C_INCLUDE_PATH")
-              (files '("arm-none-eabi/include")))
-             (search-path-specification
-              (variable "CROSS_CPLUS_INCLUDE_PATH")
-              (files '("arm-none-eabi/include"
-                       "arm-none-eabi/include/c++"
-                       "arm-none-eabi/include/c++/arm-none-eabi")))
-             (search-path-specification
-              (variable "CROSS_LIBRARY_PATH")
-              (files '("arm-none-eabi/lib"))))))))
 
 (define-public newlib-arm-none-eabi-7-2018-q2-update
   ;; This is the same commit as used for the 7-2018-q2-update release