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

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

[Attila Lendvai]

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

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

[Ricardo Wurmus]

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

[bug#54906] ping

[Attila Lendvai]

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.

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

[Katherine Cox-Buday]

(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

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

[Maxime Devos]

[-- Attachment #1: Type: text/plain, Size: 1029 bytes --]

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

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

[Attila Lendvai]

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

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

[Maxime Devos]

[-- Attachment #1: Type: text/plain, Size: 1096 bytes --]

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.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

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

[Attila Lendvai]

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

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

[Maxime Devos]

[-- Attachment #1: Type: text/plain, Size: 2083 bytes --]

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.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]