[PATCH] build/go: Don't use set!

  • Done
  • quality assurance status badge
Details
2 participants
  • Efraim Flashner
  • Ludovic Courtès
Owner
unassigned
Submitted by
Efraim Flashner
Severity
normal
E
E
Efraim Flashner wrote on 28 Nov 2023 11:24
(address . guix-patches@gnu.org)(name . Efraim Flashner)(address . efraim@flashner.co.il)
92508cd851b013f54a799350082f49b157c7aebf.1701167049.git.efraim@flashner.co.il
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(-)

Toggle diff (33 lines)
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)

base-commit: 62376e3eb67644454bc655bed56be4be965bd13e
--
Efraim Flashner <efraim@flashner.co.il> ????? ?????
GPG key = A28B F40C 3E55 1372 662D 14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted
L
L
Ludovic Courtès wrote on 3 Dec 2023 23:49
(name . Efraim Flashner)(address . efraim@flashner.co.il)
87jzpuewbk.fsf@gnu.org
Efraim Flashner <efraim@flashner.co.il> skribis:

Toggle quote (7 lines)
> 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

[...]

Toggle quote (8 lines)
> - (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’.
E
E
Efraim Flashner wrote on 4 Dec 2023 10:53
(name . Ludovic Courtès)(address . ludo@gnu.org)
ZW2htr6wC4MVr21Q@3900XT
On Sun, Dec 03, 2023 at 11:49:03PM +0100, Ludovic Courtès wrote:
Toggle quote (23 lines)
> 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 <efraim@flashner.co.il> ????? ?????
GPG key = A28B F40C 3E55 1372 662D 14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted
-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEoov0DD5VE3JmLRT3Qarn3Mo9g1EFAmVtobYACgkQQarn3Mo9
g1E25xAAlEKqSolqlZkPM3XELAwj4J66kCCAdaT7O7gUs2Tv+AOfRQws9eHCHu1v
POOFNF+MY4I/lYMkhFMvbMp/71MxwJiHU8nFdpUEK7pRWzx+Ia4f+kTkTf2deyDz
gyFTIZ0F9RRKLqYv4TsahW4WHgd/iDWPQzAt2ndUHnbYVufgMP1Jn8mwQcGzQhT0
fvqpSjohDsPvpZT5eHeUMQ4ajA6CySLm6OSWecWt7hYu4d8/A/Y/yY0I905zxfzi
c8f8HhEdFJMjqxlVkOR7ZBttl1zj9Lj/Y8fIDnjMrgcT1adhAmhqYCVRri8nW2cP
5+0n8apBJs6S9GX7z5wDY4QJcGPulEuY57KELesDSYX614sAR/I6QoY+pW0/8QIU
bbmjzwFLpNeog4+OAkYx6+KGDUq9jrh/FA+SeVFU69PA7SWWXCeO0cuJOkUan0Td
3stqHN4hdeqtBed+2zw6eSbjG0V4gf/brY7BXgwUoCzAYLy+NrQi3KGJvpLOHKUS
bjf3n2xayhwIKLYtlrfj0AfURUMuP13ngycWbOT46zq5E4bLxy+X5IatQvGhWCIJ
LWg8ZSlD29keBqAqn6gpq01SVLUvmki14l/kXrYOsJPR/ca44bNf9GW2uWz864cJ
J50z0AL1V3g3V3gCzG/ju4FtDL6wjUj1FnsFyERWiia294hGJp0=
=PuSU
-----END PGP SIGNATURE-----


Closed
E
E
Efraim Flashner wrote on 6 Dec 2023 13:27
Re: bug#67505: [PATCH] build/go: Don't use set!
ZXBoqKBWZCgwkjXH@3900XT
On Mon, Dec 04, 2023 at 11:53:58AM +0200, Efraim Flashner wrote:
Toggle quote (26 lines)
> 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.


--
Efraim Flashner <efraim@flashner.co.il> ????? ?????
GPG key = A28B F40C 3E55 1372 662D 14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted
-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEoov0DD5VE3JmLRT3Qarn3Mo9g1EFAmVwaKgACgkQQarn3Mo9
g1FL0RAAnZOeJIhtF3EHus3PNv7fwqnY5ItE0L0XBGzf4cpnMegnjQ4n3K6hje5Y
j5PfnTnUNHC7zKbqXrum/lpQEAI7WfG4vow2i2ejEh95vrxLBujbC0ccqd+8rZBp
TZtd3C+Vltyz+fPoBe0xz9DP1JRtsCQtJIocHJssGc9n1JQOZerMMyJG9R+kxXkl
gadsF3BHb5+wjB91dMRKGGhto0iJ0xISh9/U/62qaY74tvERcTRB3CHgeS/2HyrH
i0TYmaieFw3GX3BUH7dCa6TYE1jH7t6lxcYX/4Y692XoWr/+6/rHLvufKSbWTrw2
0puiM1GKYwT4E5fsfY8VUkE0YrolploE3kksw1VOJ+8VsNOdSOmmutdkAQxKfHWo
/gluuWVv33L3XPR4XWrasLg5kRPah7+ILm0xNs1VrXC2Xacat0nYiWYvfsuMbixi
AwcF0hOMtIv4H/iCO8vHgH/NF7mKhzjIn3UQqEkpo2W5S95H2Qg5orffyM8BLlJ/
jzcptpAhEXLckvOMlJNZI4b95+5uBVAsSIFPK69lN4XYbeLBi+OJAI2ALwBEpWpG
XQM24yBbp5IN3EQsuAN/P8b4pcM8OWeks/QOQYVmG4ALB7qgYomEixr/Jhq99uoH
63GmA0YLds20PeHAG3CwT4zghlHYw/Qde4DdHgPFCojBG0yoDzM=
=U2NR
-----END PGP SIGNATURE-----


?