GNU bug report logs - #54906
[PATCH] build: go-build-system: Add support for #:skip-build? #t.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 54906 in the body.
You can then email your comments to 54906 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Attila Lendvai <attila <at> lendvai.name>
To: guix-patches <at> gnu.org
Cc: Attila Lendvai <attila <at> lendvai.name>
Subject: [PATCH] build: go-build-system: Add support for #:skip-build? #t.
Date: Wed, 13 Apr 2022 14:00:42 +0200

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(-)

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

Message #8 received at 54906 <at> debbugs.gnu.org (full text, mbox):

From: Ricardo Wurmus <rekado <at> elephly.net>
To: 54906 <at> debbugs.gnu.org
Cc: Sarah Morgensen <iskarian <at> mgsn.dev>, Leo Famulari <leo <at> famulari.name>
Subject: [PATCH] build: go-build-system: Add support for #:skip-build? #t.
Date: Thu, 14 Apr 2022 12:38:56 +0200

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
https://issues.guix.gnu.org/54906?

-- 
Ricardo

Message #11 received at 54906 <at> debbugs.gnu.org (full text, mbox):

From: Attila Lendvai <attila <at> lendvai.name>
To: "54906 <at> debbugs.gnu.org" <54906 <at> debbugs.gnu.org>
Subject: ping
Date: Wed, 27 Apr 2022 16:33:15 +0000

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...

https://github.com/attila-lendvai-patches/guix/commits/import

--
• 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.

Message #14 received at 54906 <at> debbugs.gnu.org (full text, mbox):

From: Katherine Cox-Buday <cox.katherine.e <at> gmail.com>
To: Attila Lendvai <attila <at> lendvai.name>
Cc: 54906 <at> debbugs.gnu.org
Subject: Re: [bug#54906] [PATCH] build: go-build-system: Add support for
 #:skip-build? #t.
Date: Wed, 27 Apr 2022 12:42:02 -0500

(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

Message #17 received at 54906 <at> debbugs.gnu.org (full text, mbox):

From: Maxime Devos <maximedevos <at> telenet.be>
To: Attila Lendvai <attila <at> lendvai.name>, 54906 <at> debbugs.gnu.org
Subject: Re: [bug#54906] [PATCH] build: go-build-system: Add support for
 #:skip-build? #t.
Date: Wed, 27 Apr 2022 21:22:08 +0200

[Message part 1 (text/plain, inline)]

Attila Lendvai schreef op wo 13-04-2022 om 14:00 [+0200]:
> 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.,
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.

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

[signature.asc (application/pgp-signature, inline)]

Message #20 received at 54906 <at> debbugs.gnu.org (full text, mbox):

From: Attila Lendvai <attila <at> lendvai.name>
To: Maxime Devos <maximedevos <at> telenet.be>
Cc: 54906 <at> debbugs.gnu.org
Subject: Re: [bug#54906] [PATCH] build: go-build-system: Add support for
 #:skip-build? #t.
Date: Thu, 28 Apr 2022 10:56:20 +0000

> 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)

Message #23 received at 54906 <at> debbugs.gnu.org (full text, mbox):

From: Maxime Devos <maximedevos <at> telenet.be>
To: Attila Lendvai <attila <at> lendvai.name>
Cc: 54906 <at> debbugs.gnu.org
Subject: Re: [bug#54906] [PATCH] build: go-build-system: Add support for
 #:skip-build? #t.
Date: Thu, 28 Apr 2022 13:35:12 +0200

[Message part 1 (text/plain, inline)]

Attila Lendvai schreef op do 28-04-2022 om 10:56 [+0000]:
> 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?

> 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.

[signature.asc (application/pgp-signature, inline)]

Message #26 received at 54906 <at> debbugs.gnu.org (full text, mbox):

From: Attila Lendvai <attila <at> lendvai.name>
To: Maxime Devos <maximedevos <at> telenet.be>
Cc: 54906 <at> debbugs.gnu.org
Subject: Re: [bug#54906] [PATCH] build: go-build-system: Add support for
 #:skip-build? #t.
Date: Thu, 28 Apr 2022 12:14:18 +0000

> > 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.


> 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)

Message #29 received at 54906 <at> debbugs.gnu.org (full text, mbox):

From: Maxime Devos <maximedevos <at> telenet.be>
To: Attila Lendvai <attila <at> lendvai.name>
Cc: 54906 <at> debbugs.gnu.org
Subject: Re: [bug#54906] [PATCH] build: go-build-system: Add support for
 #:skip-build? #t.
Date: Mon, 13 Jun 2022 03:53:16 +0200

[Message part 1 (text/plain, inline)]

Attila Lendvai schreef op do 28-04-2022 om 12:14 [+0000]:
> 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]:
> 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:

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

).

Greetings,
Maxime.

[signature.asc (application/pgp-signature, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.