From: Sarah Morgensen <iskarian@mgsn.dev>
To: Marius Bakke <marius@gnu.org>
Cc: 50227@debbugs.gnu.org
Subject: [bug#50227] [PATCH 3/3] gnu: go-gotest-tools-assert: Provide internal inputs with the source.
Date: Sat, 28 Aug 2021 23:17:21 -0700 [thread overview]
Message-ID: <8635qs3hce.fsf@mgsn.dev> (raw)
In-Reply-To: Marius Bakke's message of "Fri, 27 Aug 2021 17:13:30 +0200 (1 day, 8 hours, 16 minutes ago)"
Hello,
Marius Bakke <marius@gnu.org> writes:
> * gnu/packages/golang.scm (go-gotest-tools-assert)[inputs]: Add
> GO-GOTEST-TOOLS-INTERNAL-FORMAT, GO-GOTEST-TOOLS-INTERNAL-DIFFLIB, and
> GO-GOTEST-TOOLS-INTERNAL-SOURCE.
> [arguments]: Add phase to install a union of the above inputs.
> * gnu/packages/golang.scm (gotestsum)[native-inputs]: Don't add the above
> mentioned inputs.
> ---
> gnu/packages/golang.scm | 45 +++++++++++++++++++++++++++--------------
> 1 file changed, 30 insertions(+), 15 deletions(-)
>
> diff --git a/gnu/packages/golang.scm b/gnu/packages/golang.scm
> index 3a5c6ddc3f..295b442a2a 100644
> --- a/gnu/packages/golang.scm
> +++ b/gnu/packages/golang.scm
> @@ -20,7 +20,7 @@
> ;;; Copyright © 2020 Jakub Kądziołka <kuba@kadziolka.net>
> ;;; Copyright © 2020 Nicolas Goaziou <mail@nicolasgoaziou.com>
> ;;; Copyright © 2020 Ryan Prior <rprior@protonmail.com>
> -;;; Copyright © 2020 Marius Bakke <marius@gnu.org>
> +;;; Copyright © 2020, 2021 Marius Bakke <marius@gnu.org>
> ;;; Copyright © 2020 raingloom <raingloom@riseup.net>
> ;;; Copyright © 2020 Martin Becze <mjbecze@riseup.net>
> ;;; Copyright © 2021 Ricardo Wurmus <rekado@elephly.net>
> @@ -5945,9 +5945,35 @@ gotest-tools.")))
> (arguments
> `(#:tests? #f ; Test failure concerning message formatting (FIXME)
> #:import-path "gotest.tools/assert"
> - #:unpack-path "gotest.tools"))
> - ;(propagated-inputs
> - ; `(("go-gotest-tools-internal-format" ,go-gotest-tools-internal-format)))
> + #:unpack-path "gotest.tools"
> + #:modules ((ice-9 match)
> + (srfi srfi-26)
> + ,@%go-build-system-modules)
> + #:phases
> + (modify-phases (@ (guix build go-build-system) %standard-phases)
> + (add-before 'install 'install-internal-inputs
> + (lambda* (#:key inputs outputs #:allow-other-keys)
> + (let ((out (assoc-ref outputs "out")))
> + ;; The Go compiler does not permit importing libraries with
> + ;; "internal" in the path from anywhere except below the
> + ;; package that uses them. Thus, install these inputs
> + ;; alongside this package.
> + (union-build
> + out
> + (match (filter (lambda (input)
> + (string-prefix? "go-gotest-tools-internal"
> + (car input)))
> + inputs)
> + (((names . directories) ...) directories))
> + #:create-all-directories? #t
> + #:log-port (%make-void-port "w"))))))))
> + (inputs
> + `(("go-gotest-tools-internal-format"
> + ,go-gotest-tools-internal-format)
> + ("go-gotest-tools-internal-difflib"
> + ,go-gotest-tools-internal-difflib)
> + ("go-gotest-tools-internal-source"
> + ,go-gotest-tools-internal-source)))
> (native-inputs
> `(("go-github-com-pkg-errors" ,go-github-com-pkg-errors)
> ("go-github-com-google-go-cmp-cmp"
> @@ -5985,17 +6011,6 @@ test when a comparison fails.")
> ,go-github-com-jonboulle-clockwork)
> ("go-golang-org-x-crypto" ,go-golang-org-x-crypto)
> ("go-gotest-tools-assert" ,go-gotest-tools-assert)
> - ("go-github-com-google-go-cmp-cmp"
> - ,go-github-com-google-go-cmp-cmp)
> - ;; TODO: This would be better as a propagated-input of
> - ;; go-gotest-tools-assert, but that does not work for
> - ;; some reason.
> - ("go-gotest-tools-internal-format"
> - ,go-gotest-tools-internal-format)
> - ("go-gotest-tools-internal-difflib"
> - ,go-gotest-tools-internal-difflib)
> - ("go-gotest-tools-internal-source"
> - ,go-gotest-tools-internal-source)
> ("go-github-com-google-go-cmp-cmp"
> ,go-github-com-google-go-cmp-cmp)))
> (synopsis "Go test runner with output optimized for humans")
Just piggybacking off this to add that I believe the "correct" way
forward to handle packages like these is to put all of them in a single
go-gotest-tools package, and modify the build system to build them all.
I've tested a proof-of-concept of this, based off of what Debian does
[0]. Essentially:
1. Add two arguments to the build system, GO-PACKAGES-INCLUDE and
GO-PACKAGES-EXCLUDE. GO-PACKAGES-INCLUDE defaults to something like
'("IMPORT-PATH/...") and GO-PACKAGES-EXCLUDE defaults to the empty list.
2. Run "go list GO-PACKAGES-INCLUDE", which lists all packages matching
GO-PACKAGES-INCLUDE.
3. Remove any packages matching GO-PACKAGES-EXCLUDE (should this be a
regex? I'm not sure), leaving us with GO-PACKAGES.
4. Run "go install ... GO-PACKAGES"
From my testing, this causes a LOT of packages to need edits, because it
surfaces a lot of hidden bugs. For example, suppose we have a Guix
package "go-B-tool" with import path "B/tool" and another Guix package
"go-use-B" which imports "B/tool/extra". If "B/tool/extra" is not
imported by "B/tool", we will not have actually built or tested
"B/tool/extra" so we will only encounter issues when building
"go-use-B", even those the actual issue should be addressed in
"go-B-tool".
In the case of the above package, we would merge all go-gotest-tools
packages into a go-gotest-tools-v3 package, with the import path
"gotest.tools/v3", which is what its go.mod states. (Note that none of
the sub-packages have their own go.mod, so it would cause issues down
the road with the module system to have each of those sub-packages be a
Guix package.) With the above build-system changes, all of the
previously-separate packages would be built and tested together. (If we
wanted to exclude a problematic package which we didn't need, we could
remove the directory in a snippet.)
Depending on how many packages are affected, perhaps this part will
warrant a wip-go-build-system branch?
[0] https://manpages.debian.org/testing/dh-golang/Debian::Debhelper::Buildsystem::golang.3pm.en.html
--
Sarah
next prev parent reply other threads:[~2021-08-29 6:18 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-27 15:10 [bug#50227] [PATCH 0/3] go-build-system and GOPATH improvements Marius Bakke
2021-08-27 15:13 ` [bug#50227] [PATCH 1/3] build-system/go: Use a native-search-path for GOPATH Marius Bakke
2021-08-27 15:13 ` [bug#50227] [PATCH 2/3] gnu: hyperledger-fabric: Do not assume GOPATH contains a single entry Marius Bakke
2021-08-27 15:13 ` [bug#50227] [PATCH 3/3] gnu: go-gotest-tools-assert: Provide internal inputs with the source Marius Bakke
2021-08-27 16:44 ` [bug#50227] [PATCH] build-system/go: Trim store references using the native compiler option Marius Bakke
2021-08-27 17:47 ` Marius Bakke
2021-08-27 19:38 ` Marius Bakke
2021-08-28 2:16 ` [bug#50227] [PATCH 0/3] go-build-system and GOPATH improvements Sarah Morgensen
2021-08-28 14:52 ` Marius Bakke
2022-01-14 3:13 ` Maxim Cournoyer
2021-08-29 6:17 ` Sarah Morgensen [this message]
2021-09-03 22:55 ` Sarah Morgensen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://guix.gnu.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8635qs3hce.fsf@mgsn.dev \
--to=iskarian@mgsn.dev \
--cc=50227@debbugs.gnu.org \
--cc=marius@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/guix.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).