diff mbox series

[bug#67505] build/go: Don't use set!

Message ID 92508cd851b013f54a799350082f49b157c7aebf.1701167049.git.efraim@flashner.co.il
State New
Headers show
Series [bug#67505] build/go: Don't use set! | expand

Commit Message

Efraim Flashner Nov. 28, 2023, 10:24 a.m. UTC 
This causes build failures on powerpc-linux.

* guix/build/go-build-system.scm (unpack): When the unpack-path is unset
use the import-path but don't redefine the unpack-path.

Change-Id: I2b5a36eb738abb14307941d388038139dbaf2bdf
---

I checked the rest of the build code in (guix build go-build-system) and
I didn't see anywhere that didn't also check to make sure the
unpack-path wasn't empty.  I have yet to create a minimal reproducer for
the set! issue on powerpc-linux but this is the only change preventing
building go packages on powerpc-linux (the 32-bit ones, not
powerpc64le-linux).

 guix/build/go-build-system.scm | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)


base-commit: 62376e3eb67644454bc655bed56be4be965bd13e

Comments

Ludovic Courtès Dec. 3, 2023, 10:49 p.m. UTC | #1 
Efraim Flashner <efraim@flashner.co.il> skribis:

> This causes build failures on powerpc-linux.
>
> * guix/build/go-build-system.scm (unpack): When the unpack-path is unset
> use the import-path but don't redefine the unpack-path.
>
> Change-Id: I2b5a36eb738abb14307941d388038139dbaf2bdf

[...]

> -  (when (string-null? unpack-path)
> -    (set! unpack-path import-path))
> -  (let ((dest (string-append (getenv "GOPATH") "/src/" unpack-path)))
> +  (let ((dest (string-append (getenv "GOPATH") "/src/"
> +                             (if (string-null? unpack-path)
> +                               import-path
> +                               unpack-path))))

Could you adjust indentation of the ‘if’ arms?

Otherwise LGTM, thanks!

Ludo’.
Efraim Flashner Dec. 4, 2023, 9:53 a.m. UTC | #2 
On Sun, Dec 03, 2023 at 11:49:03PM +0100, Ludovic Courtès wrote:
> Efraim Flashner <efraim@flashner.co.il> skribis:
> 
> > This causes build failures on powerpc-linux.
> >
> > * guix/build/go-build-system.scm (unpack): When the unpack-path is unset
> > use the import-path but don't redefine the unpack-path.
> >
> > Change-Id: I2b5a36eb738abb14307941d388038139dbaf2bdf
> 
> [...]
> 
> > -  (when (string-null? unpack-path)
> > -    (set! unpack-path import-path))
> > -  (let ((dest (string-append (getenv "GOPATH") "/src/" unpack-path)))
> > +  (let ((dest (string-append (getenv "GOPATH") "/src/"
> > +                             (if (string-null? unpack-path)
> > +                               import-path
> > +                               unpack-path))))
> 
> Could you adjust indentation of the ‘if’ arms?
> 
> Otherwise LGTM, thanks!

Done. Patch pushed!
Efraim Flashner Dec. 6, 2023, 12:27 p.m. UTC | #3 
On Mon, Dec 04, 2023 at 11:53:58AM +0200, Efraim Flashner wrote:
> On Sun, Dec 03, 2023 at 11:49:03PM +0100, Ludovic Courtès wrote:
> > Efraim Flashner <efraim@flashner.co.il> skribis:
> > 
> > > This causes build failures on powerpc-linux.
> > >
> > > * guix/build/go-build-system.scm (unpack): When the unpack-path is unset
> > > use the import-path but don't redefine the unpack-path.
> > >
> > > Change-Id: I2b5a36eb738abb14307941d388038139dbaf2bdf
> > 
> > [...]
> > 
> > > -  (when (string-null? unpack-path)
> > > -    (set! unpack-path import-path))
> > > -  (let ((dest (string-append (getenv "GOPATH") "/src/" unpack-path)))
> > > +  (let ((dest (string-append (getenv "GOPATH") "/src/"
> > > +                             (if (string-null? unpack-path)
> > > +                               import-path
> > > +                               unpack-path))))
> > 
> > Could you adjust indentation of the ‘if’ arms?
> > 
> > Otherwise LGTM, thanks!
> 
> Done. Patch pushed!

Patch re-opened. I reverted it since it caused ~4800 package builds on
Berlin.  I'll probably carry it locally for now (I'm almost certainly
the only one affected) and we can apply it another time something
touches go.
diff mbox series

Patch

diff --git a/guix/build/go-build-system.scm b/guix/build/go-build-system.scm
index 7f25e05d0d..d32235bf5a 100644
--- a/guix/build/go-build-system.scm
+++ b/guix/build/go-build-system.scm
@@ -4,7 +4,7 @@ 
 ;;; Copyright © 2019 Maxim Cournoyer <maxim.cournoyer@gmail.com>
 ;;; Copyright © 2020 Jack Hill <jackhill@jackhill.us>
 ;;; Copyright © 2020 Jakub Kądziołka <kuba@kadziolka.net>
-;;; Copyright © 2020, 2021 Efraim Flashner <efraim@flashner.co.il>
+;;; Copyright © 2020, 2021, 2023 Efraim Flashner <efraim@flashner.co.il>
 ;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev>
 ;;;
 ;;; This file is part of GNU Guix.
@@ -227,9 +227,10 @@  (define* (unpack #:key source import-path unpack-path #:allow-other-keys)
 
   (when (string-null? import-path)
     (display "WARNING: The Go import path is unset.\n"))
-  (when (string-null? unpack-path)
-    (set! unpack-path import-path))
-  (let ((dest (string-append (getenv "GOPATH") "/src/" unpack-path)))
+  (let ((dest (string-append (getenv "GOPATH") "/src/"
+                             (if (string-null? unpack-path)
+                               import-path
+                               unpack-path))))
     (mkdir-p dest)
     (if (file-is-directory? source)
         (copy-recursively source dest #:keep-mtime? #t)