From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lists.gentoo.org (pigeon.gentoo.org [208.92.234.80]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by finch.gentoo.org (Postfix) with ESMTPS id 9B79B138350 for ; Mon, 17 Feb 2020 05:48:53 +0000 (UTC) Received: from pigeon.gentoo.org (localhost [127.0.0.1]) by pigeon.gentoo.org (Postfix) with SMTP id 3BA41E08AC; Mon, 17 Feb 2020 05:48:49 +0000 (UTC) Received: from smtp.gentoo.org (smtp.gentoo.org [140.211.166.183]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by pigeon.gentoo.org (Postfix) with ESMTPS id BF119E086B for ; Mon, 17 Feb 2020 05:48:48 +0000 (UTC) Received: from grubbs.orbis-terrarum.net (localhost [127.0.0.1]) (using TLSv1.2 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by smtp.gentoo.org (Postfix) with ESMTPS id A9E2B34EC85 for ; Mon, 17 Feb 2020 05:48:46 +0000 (UTC) Received: (qmail 23322 invoked by uid 10000); 17 Feb 2020 05:48:46 -0000 Date: Mon, 17 Feb 2020 05:48:46 +0000 From: "Robin H. Johnson" To: gentoo-dev@lists.gentoo.org Subject: Re: [gentoo-dev] [PATCH 1/3] eclass/go-module: add support for building based on go.sum Message-ID: References: <20200209203121.4646-1-robbat2@gentoo.org> <092ae60dfccaac8975d224b048fc1daf87a77a7a.camel@gentoo.org> Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-Id: Gentoo Linux mail X-BeenThere: gentoo-dev@lists.gentoo.org Reply-to: gentoo-dev@lists.gentoo.org X-Auto-Response-Suppress: DR, RN, NRN, OOF, AutoReply MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="Jz4B0QKJ3xxfGEtb" Content-Disposition: inline In-Reply-To: <092ae60dfccaac8975d224b048fc1daf87a77a7a.camel@gentoo.org> X-Archives-Salt: 8160f1dc-d2b0-42cd-9c3a-91ac727e1666 X-Archives-Hash: 1dac8551f878cd54c646398d3dfcce92 --Jz4B0QKJ3xxfGEtb Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Stay tuned for v2 with major improvements based on talking to upstream. - Dropped an entire distfile per golang module (down to 1-2 files per module in go.sum) - Better Go 1.13 support (some semantics changed slightly from 1.12) - Easier way to track & include licenses of all the modules On Thu, Feb 13, 2020 at 05:57:57PM +0100, Micha=C5=82 G=C3=B3rny wrote: > > +# EGO_SUM=3D( > > +# "github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimB= fB0XUf9Vl28OQ=3D" > > +# "github.com/BurntSushi/toml v0.3.1/go.mod h1:WXkYYl6Yr3qBf1K79EBnL4m= ak0OimBfB0XUf9Vl28OQ=3D" > Is it expected that the two entries would have the same hash? In this case, they SHOULD have been different, but it does happen in reality for the /go.mod entries, here's an example: github.com/stretchr/testify v1.2.1/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0P= P7pXfy6kDkUVs=3D github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0P= P7pXfy6kDkUVs=3D github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtf= Hm1UVUgZn+9EI=3D github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8Wfu= Z+81PSLYec5m4=3D 1.2.1->1.2.2 there was no change in the dependencies, so the file didn't ch= ange. > > -EXPORT_FUNCTIONS src_unpack pkg_postinst > > +EXPORT_FUNCTIONS src_unpack src_prepare pkg_postinst > Exporting a new phase looks potentially dangerous. Are you sure no > ebuilds are broken by this? I'm not sure, so I've rolled it into src_unpack for now. >=20 > > + > > +# @ECLASS-VARIABLE: EGO_SUM > > +# @DESCRIPTION: > > +# This variable duplicates the go.sum content from inside the target p= ackage. > > +# Entries of the form /go.mod should be excluded. > ...but you've included one of them in the example on top of the eclass. There was an ongoing discussion with upstream, where I was trying to trim down the number of distfiles involved. Sadly generating the .mod files turns out to have some non-trivial corner cases I did manage to drop the .info files at least, so it's 1-2 distfiles per dependency now. >=20 > > +# > > +# >=20 > Now I'm confused. Unless my eyes betray me, PATCH 2 has entries without > hash. >=20 > Also, the description fails to mention that you're supposed to quote > each line. Improved in documentation, and covered why the hash is optional right now. > > +# The format is described upstream here: > > +# https://tip.golang.org/cmd/go/#hdr-Module_authentication_using_go_sum =2E.. > I think it would be valuable to include an example here as well. Done. > > +# proxy generally verifies modules via the Hash1 code. > > +# > > +# Note: Users in China may find some mirrors in the list blocked, and = may wish > > +# to an explicit entry to /etc/portage/mirrors pointing mirror://gopro= xy/ to > > +# https://goproxy.cn/, or change this variable. > > +# See https://arslan.io/2019/08/02/why-you-should-use-a-go-module-prox= y/ for further details > > +: "${GOMODULE_GOPROXY_BASEURI:=3Dmirror://goproxy/}" > 'Changing this variable' sounds like violating metadata immutability > rule and running in trouble with the caches. Covered who & why this should be set, esp wrt to immutability. > > + > > +# @FUNCTION: go-module_set_globals > > +# @DESCRIPTION: > > +# Convert the information in EGO_SUM for other usage in the ebuild. > > +# - Populates EGO_SUM_SRC_URI that can be added to SRC_URI > > +# - Exports _EGO_SUM_MAPPING which provides reverse mapping from distf= ile back > > +# to the relative part of SRC_URI, as needed for GOPROXY=3Dfile:///.= =2E. > > +go-module_set_globals() { > > + local line error_in_gosum errorlines errormsg exts > > + local newline=3D$'\n' > > + error_in_gosum=3D0 > > + errorlines=3D( ) > > + for line in "${EGO_SUM[@]}"; do > > + local module version modfile version_modfile hash1 x > > + read -r module version_modfile hash1 x <<< "${line}" > > + # Validate input > > + if [[ -n $hash1 ]] && [[ ${hash1:0:3} !=3D "h1:" ]] ; then >=20 > Please use ${foo} everywhere consistently, and put && inside [[ ]].=20 > Also, I dare say wildcard match is more readable than hardcoding string > length, i.e.: >=20 > [[ -n ${hash1} && ${hash1} !=3D h1:* ]] =2E.. > > + # Split 'v0.3.0/go.mod' into 'v0.3.0' and '/go.mod' > > + version=3D${version_modfile%%/*} > > + modfile=3D${version_modfile#*/} > > + [[ "$modfile" =3D=3D "${version_modfile}" ]] && modfile=3D > Check the initial string, not the result of arbitrary manipulations > on it. This would wrongly evaluate true for 'v0.3.0/v0.3.0'. Reworked this > > +go-module_src_unpack() { > > + if [[ "${#EGO_VENDOR[@]}" -gt 0 ]]; then > > + _go-module_src_unpack_vendor > > + elif [[ "${#EGO_SUM[@]}" -gt 0 ]]; then > > + _go-module_src_unpack_gosum > Does that mean those two are mutually exclusive? Yes. > > +# @FUNCTION: go-module_src_prepare =2E.. > Wouldn't it be better to append this to src_unpack? Overriding > src_prepare is generally problematic, and as I've said above, you're > already risking by adding a new export. Moved it. > > +# @ECLASS-VARIABLE: GOMODULE_GOSUM_PATH > > +# @DESCRIPTION: > > +# Path to root go.sum of package. If your ebuild modifies S after inhe= riting > > +# the eclass, you may need to update this variable. > > +: "${GO_MODULE_GOSUM_PATH:=3D${S}/go.sum}" > Wouldn't it be cleaner to have the path relative to ${S} by default? Variable is no longer needed. > > +# @FUNCTION: _go-module_src_unpack_gosum =2E.. > > + declare -A _EGO_SUM_MAPPING_ASSOC > Why not make it local? Done. > > + # go.sum entries ending in /go.mod aren't strictly needed at this pha= se =2E.. > Why not create and restore a copy? Or does go-get make other changes? The minimize is dropped entirely per upstream discussions. --=20 Robin Hugh Johnson Gentoo Linux: Dev, Infra Lead, Foundation Treasurer E-Mail : robbat2@gentoo.org GnuPG FP : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85 GnuPG FP : 7D0B3CEB E9B85B1F 825BCECF EE05E6F6 A48F6136 --Jz4B0QKJ3xxfGEtb Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Robbat2 @ Orbis-Terrarum Networks - The text below is a digital signature. If it doesn't make any sense to you, ignore it. iQKTBAABCgB9FiEEveu2pS8Vb98xaNkRGTlfI8WIJsQFAl5KKTxfFIAAAAAALgAo aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldEJE RUJCNkE1MkYxNTZGREYzMTY4RDkxMTE5Mzk1RjIzQzU4ODI2QzQACgkQGTlfI8WI JsQGzg/+J01BCgwcDAYuzK0jGP9wql5q8Hmj7ziByAZ3tJZ3+6ahYk8ttm2opdNs qUGROKhggLddqcfX+DnIyS1iioT6ux5SNmiGbkciEB1G1c7Rl+j10jKsFZT+jdxa JpPcwd8isZH5RQqA5T6RUKcgRbAH8YwFVHvuXWfGB/Z9jFP1EQar9/F2nNF7wXJr cvi7yQexAC+i750Vgl94qXwfyge7UyVcZ45s/mw5YeWmC725r2jQb5UfPzK64r3p cuuDT3UlLGKNshvUJHXDw+mTbO0p5j5WKzhMRP9BRr6Ej3Vd03C4wj65GDQWU7E4 mwLteTKAt5AgED1wb1Sv2TveS5s0GAaEoz8PPG2via1qKZDUyUL9EHXgVUo3tiFW o5ThSxO8levilKFyYLov6ASb/Yy3civFd68ZjnYsQEbce8+n8Vrz0iCXsy5KZB2F h9XYdIk7Zjnr9K4YPmvREbq8MlnY/UQ+NSWf8+CjRPCum1LOzVAnGX83Yyt622BM IwUK/cH/zJmVCNe3abg0xPS34W+9gN7atObyJVZgkS1jmhxR9fABuhBlKm6JxKOY Rl2IMrEFtshLRY4zDeQvhB32a3BFTT/gTaPnsvTDauJ3rq4X0Ta7d3Vsr8NkBcgs Ti6f93xxROxtdqM9CAK386OMH5+SF81HhjTij7IV44GhHZJ0y0M= =H7o9 -----END PGP SIGNATURE----- --Jz4B0QKJ3xxfGEtb--