[PATCH] build: go-build-system: Add support for #:skip-build? #t.

DoneSubmitted by Attila Lendvai.
Details
4 participants
  • Attila Lendvai
  • Katherine Cox-Buday
  • Maxime Devos
  • Ricardo Wurmus
Owner
unassigned
Severity
normal
A
A
Attila Lendvai wrote on 13 Apr 14:00 +0200
(address . guix-patches@gnu.org)(name . Attila Lendvai)(address . attila@lendvai.name)
20220413120052.25602-1-attila@lendvai.name
This mimics the same feature of the cargo-build-system.

* guix/build-system/go.scm (go-build): Add skip-build? keyword param and
propagate it.
* guix/build/go-build-system.scm (build): Add skip-build? keyword param.
---
guix/build-system/go.scm | 4 +++-
guix/build/go-build-system.scm | 31 ++++++++++++++++---------------
2 files changed, 19 insertions(+), 16 deletions(-)

Toggle diff (67 lines)
diff --git a/guix/build-system/go.scm b/guix/build-system/go.scm
index 5e0e5bbad3..6bcb3656be 100644
--- a/guix/build-system/go.scm
+++ b/guix/build-system/go.scm
@@ -175,6 +175,7 @@ (define* (go-build name inputs
                    (import-path "")
                    (unpack-path "")
                    (build-flags ''())
+                   (skip-build? #f)
                    (tests? #t)
                    (allow-go-reference? #f)
                    (system (%current-system))
@@ -205,7 +206,8 @@ (define builder
                     #:import-path #$import-path
                     #:unpack-path #$unpack-path
                     #:build-flags #$build-flags
-                    #:tests? #$tests?
+                    #:skip-build? #$skip-build?
+                    #:tests? #$(and tests? (not skip-build?))
                     #:allow-go-reference? #$allow-go-reference?
                     #:inputs #$(input-tuples->gexp inputs)))))
 
diff --git a/guix/build/go-build-system.scm b/guix/build/go-build-system.scm
index 7f25e05d0d..637d66a6f1 100644
--- a/guix/build/go-build-system.scm
+++ b/guix/build/go-build-system.scm
@@ -254,22 +254,23 @@ (define (go-inputs inputs)
                 (_ #f))
               inputs))))
 
-(define* (build #:key import-path build-flags #:allow-other-keys)
+(define* (build #:key skip-build? import-path build-flags #:allow-other-keys)
   "Build the package named by IMPORT-PATH."
-  (with-throw-handler
-    #t
-    (lambda _
-      (apply invoke "go" "install"
-              "-v" ; print the name of packages as they are compiled
-              "-x" ; print each command as it is invoked
-              ;; Respectively, strip the symbol table and debug
-              ;; information, and the DWARF symbol table.
-              "-ldflags=-s -w"
-              `(,@build-flags ,import-path)))
-    (lambda (key . args)
-      (display (string-append "Building '" import-path "' failed.\n"
-                              "Here are the results of `go env`:\n"))
-      (invoke "go" "env"))))
+  (or skip-build?
+      (with-throw-handler
+        #t
+        (lambda _
+          (apply invoke "go" "install"
+                 "-v"        ; print the name of packages as they are compiled
+                 "-x"        ; print each command as it is invoked
+                 ;; Respectively, strip the symbol table and debug
+                 ;; information, and the DWARF symbol table.
+                 "-ldflags=-s -w"
+                 `(,@build-flags ,import-path)))
+        (lambda (key . args)
+          (display (string-append "Building '" import-path "' failed.\n"
+                                  "Here are the results of `go env`:\n"))
+          (invoke "go" "env")))))
 
 ;; Can this also install commands???
 (define* (check #:key tests? import-path #:allow-other-keys)
-- 
2.35.1
R
R
Ricardo Wurmus wrote on 14 Apr 12:38 +0200
(address . 54906@debbugs.gnu.org)
87mtgovtea.fsf@elephly.net
Thanks for the patch!

I’m not qualified to evaluate this, so I Cc’d Sarah and Leo who have
previously worked on the go-build-system.

@Sarah and @Leo: Could you please comment on the issue at

--
Ricardo
A
A
Attila Lendvai wrote on 27 Apr 18:33 +0200
ping
(name . 54906@debbugs.gnu.org)(address . 54906@debbugs.gnu.org)
_mgJPB2NBxhL0YK6je7jMzFddB7iJ6qTT9wscY74QlG6Nip6PT8hHyEclU_omg6XWsx-opE5fCSeAzKQr71iTynqFguY6f2GsUWYmCD8p-s=@lendvai.name
kindly pinging the involved parties, because i have some extensive
work on the golang importer(*) that depends on this, or at least on
the decision whether this will be merged or not.

the longer those commits are sitting on my side, the higher the chance
of a commit to master that will lead to a painful merge session...


--
• attila lendvai
• PGP: 963F 5D5F 45C7 DFCD 0A39
--
Every task involves constraint,
Solve the thing without complaint;
There are magic links and chains
Forged to loose our rigid brains.
Structures, strictures, though they bind,
Strangely liberate the mind.
K
K
Katherine Cox-Buday wrote on 27 Apr 19:42 +0200
Re: [bug#54906] [PATCH] build: go-build-system: Add support for #:skip-build? #t.
(name . Attila Lendvai)(address . attila@lendvai.name)(address . 54906@debbugs.gnu.org)
87wnfamnit.fsf@gmail.com
(Please forgive me for using your patch to try out reviewing some Guix
patches! This is also my first time reviewing code over email, so feedback welcome!)

I don't have the context for why such a change is needed, but purely from a code perspective this LGTM.

Unfortunately I have no power to commit this.

--
Katherine
M
M
Maxime Devos wrote on 27 Apr 21:22 +0200
e375a336cf2622663357ea3e8c37e236133eb846.camel@telenet.be
Attila Lendvai schreef op wo 13-04-2022 om 14:00 [+0200]:
Toggle quote (6 lines)
> This mimics the same feature of the cargo-build-system.
>
> * guix/build-system/go.scm (go-build): Add skip-build? keyword param and
> propagate it.
> * guix/build/go-build-system.scm (build): Add skip-build? keyword param.

The new WIP antioxidant-build-system (intended to replace cargo-build-
system) will not have #:skip-build?, because the new build system
actually reuses the build results off the dependents.

Likewise, maybe long-term someone will figure out how to do something
similar for go -- e.g.,
build cache’.

So it seems more of a work-around than a feature to me.

Maybe after building, the new cache entries could be copied to an
output, and before building, the cache could be populated by old cache
entries from dependents? That would allow for only having to compile
the dependencies only once, reusing them for all dependents.

Greetings,
Maxime
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYmmX4BccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7vrGAP9GhZwie4hn1tgJsdAM7VtDWsRi
Ar9dQTNqTlSWxSLN6gD/a+Y2p3/KhN1DXZqfRfyDr63wFSQGVEZYai8/WqoXoQU=
=Smdh
-----END PGP SIGNATURE-----


A
A
Attila Lendvai wrote on 28 Apr 12:56 +0200
(name . Maxime Devos)(address . maximedevos@telenet.be)(address . 54906@debbugs.gnu.org)
gbg-UILYI0h6xq8as2NN44n3kwhNDtu1kzG-UhEYKIYSQgUgy8ZcIqGIDLbd4-qGbota7T5SO8AH_uLIcLyX5VQ6h6URaTRhfTcuycVTpeo=@lendvai.name
Toggle quote (12 lines)
> The new WIP antioxidant-build-system (intended to replace cargo-build-
> system) will not have #:skip-build?, because the new build system
> actually reuses the build results off the dependents.
>
> Likewise, maybe long-term someone will figure out how to do something
> similar for go -- e.g.,
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=32919#5 mentions a ‘go
> build cache’.
>
> So it seems more of a work-around than a feature to me.


i lack here the necessary resolution from the bird's eye view
perspective, so let me describe the actual ache that i'm trying to
resolve with this:

currently, the GO-BUILD-SYSTEM does not reuse build artifacts of the
dependencies, only includes them as source.

in the current setup, i.e. without SKIP-BUILD?, if i want to import an
app with 100+ dependencies, then i need to make sure that all those
100+ dependencies build fine by themselves. this is substantially more
work.

if there is SKIP-BUILD?, then i can just set it to false in the
importer for all the dependencies, and only flip it to true for the
leaf packages that i'm actualy trying to build.

it seems to me that i should just remove the SKIP-BUILD? assumption
from the go importer for now, and file my commits against vanilla
master.

i'll proceed with that.

thanks for the feedback!

--
• attila lendvai
• PGP: 963F 5D5F 45C7 DFCD 0A39
--
“People do not seem to realize that their opinion of the world is also a confession of character.”
— Ralph Waldo Emerson (1803–1882)
M
M
Maxime Devos wrote on 28 Apr 13:35 +0200
(name . Attila Lendvai)(address . attila@lendvai.name)(address . 54906@debbugs.gnu.org)
2011a56256b561e89fcecc3febdc8dbc2bf7beae.camel@telenet.be
Attila Lendvai schreef op do 28-04-2022 om 10:56 [+0000]:
Toggle quote (9 lines)
> in the current setup, i.e. without SKIP-BUILD?, if i want to import an
> app with 100+ dependencies, then i need to make sure that all those
> 100+ dependencies build fine by themselves. this is substantially more
> work.
>
> if there is SKIP-BUILD?, then i can just set it to false in the
> importer for all the dependencies, and only flip it to true for the
> leaf packages that i'm actualy trying to build.

Except for long build times due to not reusing build results, I don't
follow: if dependency X doesn't build, doesn't that imply that
dependent Y won't build either? Conversely, if dependent Y builds,
doesn't that imply that the dependents can also be built by
theirselves?

Toggle quote (6 lines)
> it seems to me that i should just remove the SKIP-BUILD? assumption
> from the go importer for now, and file my commits against vanilla
> master.
>
> i'll proceed with that.

To be clear, my comment was more about the wording (feature / work-
around / ...) than about the addition of #:skip-build?.

Greetings,
Maxime.
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYmp78BccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7omsAQDqW5pj5KoxAY7vKUdk5CTowYUG
ccneEGUlG5JkiwCIHAD/d2oTZbWKFQdntvdpszJz5gFRwnF6rnRHVBb9k8o9qA8=
=nhug
-----END PGP SIGNATURE-----


A
A
Attila Lendvai wrote on 28 Apr 14:14 +0200
(name . Maxime Devos)(address . maximedevos@telenet.be)(address . 54906@debbugs.gnu.org)
3D-9L8EDNEWZf3igXJF7tggNU1dw8wj5nUKGHbiHtcWciiKmwAP5QdazadE4OYLDYgzvPeOOehdi7fnc4ktx7f3KmR3N76ukjSopOc-hkS4=@lendvai.name
Toggle quote (12 lines)
> > if there is SKIP-BUILD?, then i can just set it to false in the
> > importer for all the dependencies, and only flip it to true for the
> > leaf packages that i'm actualy trying to build.
>
>
> Except for long build times due to not reusing build results, I don't
> follow: if dependency X doesn't build, doesn't that imply that
> dependent Y won't build either? Conversely, if dependent Y builds,
> doesn't that imply that the dependents can also be built by
> theirselves?


i'm afraid i'm stepping beyond my level of knowledge here... but i
think this may not be true for golang.

and AFAIU, the current GO-BUILD-SYSTEM doesn't reuse any build
artifacts. it only arranges the sources of the dependencies in a way
that the invoked `go build ...` can find them.


Toggle quote (4 lines)
> To be clear, my comment was more about the wording (feature / work-
> around / ...) than about the addition of #:skip-build?.


thanks for clarifying that!

--
• attila lendvai
• PGP: 963F 5D5F 45C7 DFCD 0A39
--
“Most economic fallacies derive… from the tendency to assume that there is a fixed pie, that one party can gain only at the expense of another.”
— Milton Friedman (1912–2006)
M
M
Maxime Devos wrote on 13 Jun 03:53 +0200
(name . Attila Lendvai)(address . attila@lendvai.name)(address . 54906@debbugs.gnu.org)
ab44af36599ef3fa6ea5e1bfbe057b8814e5b6f1.camel@telenet.be
Attila Lendvai schreef op do 28-04-2022 om 12:14 [+0000]:
Toggle quote (3 lines)
> i'm afraid i'm stepping beyond my level of knowledge here... but i
> think this may not be true for golang.

Barring any evidence to the contrary, I'm going to assume this is not
the case, because this tends to be false for other languages. And even
if the dependent does build while the dependency doesn't, then either
that would cause problems at runtime due to the dependency being broken
or the dependency is unused, i.e., it wasn't an actual dependency after
all.

Attila Lendvai schreef op do 28-04-2022 om 12:14 [+0000]:
Toggle quote (3 lines)
> and AFAIU, the current GO-BUILD-SYSTEM doesn't reuse any build
> artifacts.

Currently, it doesn't reuse them, but in the past it did, and maybe in
the future it can do again.

More generally, the use of #:skip-build? and ‘let's only actually build
all the things in the leaf package’ has lead to several problems in
Rust:

* things that weren't actually dependencies were packaged
E.g.: all crates that require an unstable rust compiler.

* things that are only required on platforms that Guix doesn't
support anyways were packaged (e.g.: winapi, redox, cocoa and
foundation crates
(e.g.: crates using ‘unstable’ features which cannot be compiled,
or crates

* cycles (doesn't apply to Go though)

* packages with incorrect dependency information, that only happen
to work because of how #:skip-build? implies propagation and
because of Cargo's dependency resolving algorithms smoothing over
them (don't know if this applies to Go).

* impossibility of grafting (not relevant to Go, I think Go is too
static-library-specific for that?)

While maybe not all are 100% caused by #:skip-build? or apply to Go
100%, I don't think these issues should be spread to Go as well, so
TBC, ¬LGTM.

(I guess this invalid my previous remark:

Toggle quote (3 lines)
> To be clear, my comment was more about the wording (feature / work-
> around / ...) than about the addition of #:skip-build?.

).

Greetings,
Maxime.
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYqaYjBccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7oH0AQD+X5NB8o4tjBbKi3iKDKR9pD8e
7Ir5foDHhICT+CVcxAD+O2CoNHE7iQ/hrwKVW2ZRr1czYRB3feTorjZ+kx20Jgw=
=l5To
-----END PGP SIGNATURE-----


A
A
Attila Lendvai wrote on 24 Aug 15:07 +0200
Re: control message for bug #54906
(name . control@debbugs.gnu.org)(address . control@debbugs.gnu.org)
h3U_C3YgP6aZex4WHqxfNagqK--jqoFUDUKqvdIsSjM00iMRE6ciSMbw3WTecebCHxHPz7OiAwJwlopOkcT7Bh2vRuERGqXxxdGwVUqs020=@lendvai.name
close 54906
?
Your comment

This issue is archived.

To comment on this conversation send email to 54906@debbugs.gnu.org