Re: [gentoo-dev] [PATCH] go-module.eclass: deprecate EGO_SUM and call ego instead of go

[Michał Górny]

On Fri, 2022-02-25 at 20:50 -0600, William Hubbs wrote:
> EGO_SUM can be thousands of lines long in ebuilds, and it leads to
> creating Manifest files that are thousands of lines long.
> It has been determined that vendor tarballs are a better solution if
> upstream doesn't vendor their dependencies.
>
> Also, call the ego helper function instead of calling go directly.
>
> Signed-off-by: William Hubbs <williamh@g.o>
> ---
>  eclass/go-module.eclass | 140 ++++++++++------------------------------
>  1 file changed, 34 insertions(+), 106 deletions(-)
>
> diff --git a/eclass/go-module.eclass b/eclass/go-module.eclass
> index 66fe52c9ad7..7f0d766f444 100644
> --- a/eclass/go-module.eclass
> +++ b/eclass/go-module.eclass
> @@ -25,8 +25,18 @@
>  #
>  # If the software has a directory named vendor in its
>  # top level directory, the only thing you need to do is inherit the
> -# eclass. Otherwise, you need to also populate
> -# EGO_SUM and call go-module_set_globals as discussed below.
> +# eclass.  If it doesn't, you need to also create a vendor tarball and
> +# host it somewhere, for example in your dev space.
> +#
> +# Here are the commands to create a vendor tarball.
> +#
> +# @CODE
> +#
> +# $ cd /path/to/project
> +# $ go mod vendor
> +# $ tar -acf project-1.0.tar.xz vendor

Why not 'cd ..' first and create a tarball that doesn't require custom
src_unpack()?  Also, shouldn't the name be 'project-1.0-vendor.tar'? 
Also, you're mising -J.

> +#
> +# @CODE
>  #
>  # Since Go programs are statically linked, it is important that your ebuild's
>  # LICENSE= setting includes the licenses of all statically linked
> @@ -40,15 +50,9 @@
>  #
>  # inherit go-module
>  #
> -# EGO_SUM=(
> -#   "github.com/aybabtme/rgbterm v0.0.0-20170906152045-cc83f3b3ce59/go.mod"
> -#   "github.com/aybabtme/rgbterm v0.0.0-20170906152045-cc83f3b3ce59"
> -# )
> -#
> -# go-module_set_globals
> -#
> -# SRC_URI="https://github.com/example/${PN}/archive/v${PV}.tar.gz -> ${P}.tar.gz
> -#		   ${EGO_SUM_SRC_URI}"
> +# SRC_URI="https://github.com/example/${PN}/archive/v${PV}.tar.gz -> ${P}.tar.gz"
> +# Do not add the next line if you don't have a vendor tarball
> +# SRC_URI+="${P}-vendor.tar.gz"
>  #
>  # @CODE
>
> @@ -100,72 +104,6 @@ QA_FLAGS_IGNORED='.*'
>  # Go packages should not be stripped with strip(1).
>  RESTRICT+=" strip"
>
> -# @ECLASS-VARIABLE: EGO_SUM
> -# @DESCRIPTION:
> -# This array is based on the contents of the go.sum file from the top
> -# level directory of the software you are packaging. Each entry must be
> -# quoted and contain the first two fields of a line from go.sum.
> -#
> -# You can use some combination of sed/awk/cut to extract the
> -# contents of EGO_SUM or use the dev-go/get-ego-vendor tool.
> -#
> -# One manual way to do this is the following:
> -#
> -# @CODE
> -#
> -# cat go.sum | cut -d" " -f1,2 | awk '{print "\t\"" $0 "\""}'
> -#
> -# @CODE
> -#
> -# The format of go.sum is described upstream here:
> -# https://go.dev/ref/mod#go-sum-files
> -#
> -# For inclusion in EGO_SUM, the h1: value and other future extensions SHOULD be
> -# omitted at this time. The EGO_SUM parser will accept them for ease of ebuild
> -# creation.
> -#
> -# h1:<hash> is the Hash1 structure used by upstream Go
> -# The Hash1 is MORE stable than Gentoo distfile hashing, and upstream warns
> -# that it's conceptually possible for the Hash1 value to remain stable while
> -# the upstream zipfiles change. Here are examples that do NOT change the h1:
> -# hash, but do change a regular checksum over all bytes of the file:
> -# - Differing mtimes within zipfile
> -# - Differing filename ordering with the zipfile
> -# - Differing zipfile compression parameters
> -# - Differing zipfile extra fields
> -#
> -# For Gentoo usage, the authors of this eclass feel that the h1: hash should
> -# NOT be included in the EGO_SUM at this time in order to reduce size of the
> -# ebuilds. This position will be reconsidered in future when a Go module
> -# distfile collision comes to light, where the Hash1 value of two distfiles is
> -# the same, but checksums over the file as a byte stream consider the files to
> -# be different.
> -#
> -# This decision  does NOT weaken Go module security, as Go will verify the
> -# go.sum copy of the Hash1 values during building of the package.
> -
> -# @ECLASS-VARIABLE: _GOMODULE_GOPROXY_BASEURI
> -# @DESCRIPTION:
> -# Golang module proxy service to fetch module files from. Note that the module
> -# proxy generally verifies modules via the Hash1 code.
> -#
> -# Users in China may find some mirrors in the default list blocked, and should
> -# explicitly set an entry in /etc/portage/mirrors for goproxy to
> -# https://goproxy.cn/ or another mirror that is not blocked in China.
> -# See https://arslan.io/2019/08/02/why-you-should-use-a-go-module-proxy/ for
> -# further details
> -#
> -# This variable is NOT intended for user-level configuration of mirrors, but
> -# rather to cover go modules that might exist only on specific Goproxy
> -# servers for non-technical reasons.
> -#
> -# This variable should NOT be present in user-level configuration e.g.
> -# /etc/portage/make.conf, as it will violate metadata immutability!
> -#
> -# I am considering removing this and just hard coding mirror://goproxy
> -# below, so please do not rely on it.
> -: "${_GOMODULE_GOPROXY_BASEURI:=mirror://goproxy/}"
> -
>  # @ECLASS-VARIABLE: _GOMODULE_GOSUM_REVERSE_MAP
>  # @DESCRIPTION:
>  # Mapping back from Gentoo distfile name to upstream distfile path.
> @@ -181,7 +119,7 @@ declare -A -g _GOMODULE_GOSUM_REVERSE_MAP
>  # ebuild will be considered optional. No dependencies will be added and
>  # no phase functions will be exported.
>  #
> -# If you enable GO_OPTIONAL, you have to set BDEPEND on >=dev-lang/go-1.12
> +# If you enable GO_OPTIONAL, you have to set BDEPEND on >=dev-lang/go-1.16
>  # for your package and call go-module_src_unpack manually.
>
>  # @FUNCTION: ego
> @@ -199,11 +137,7 @@ ego() {
>
>  # @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 _GOMODULE_GOSUM_REVERSE_MAP which provides reverse mapping from
> -#   distfile back to the relative part of SRC_URI, as needed for
> -#   GOPROXY=file:///...
> +# This function is deprecated.
>  go-module_set_globals() {
>  	local line exts
>  	# for tracking go.sum errors
> @@ -256,7 +190,7 @@ go-module_set_globals() {
>  			# Relative URI within a GOPROXY for a file
>  			_reluri="${_dir}/@v/${version}.${_ext}"
>  			# SRC_URI: LHS entry
> -			_uri="${_GOMODULE_GOPROXY_BASEURI}/${_reluri}"
> +			_uri="mirror://goproxy/${_reluri}"
>  #			_uri="mirror://goproxy/${_reluri}"
>  			# SRC_URI: RHS entry, encode any slash in the path as
>  			# %2F in the filename
> @@ -289,9 +223,7 @@ go-module_set_globals() {
>
>  # @FUNCTION: go-module_setup_proxy
>  # @DESCRIPTION:
> -# If your ebuild redefines src_unpack and uses EGO_SUM you need to call
> -# this function in src_unpack.
> -# It sets up the go module proxy in the appropriate location.
> +# This function is deprecated.
>  go-module_setup_proxy() {
>  	# shellcheck disable=SC2120
>  	debug-print-function "${FUNCNAME}" "$@"
> @@ -331,27 +263,30 @@ go-module_setup_proxy() {
>  # @FUNCTION: go-module_src_unpack
>  # @DESCRIPTION:
>  # If EGO_SUM is set, unpack the base tarball(s) and set up the
> -#   local go proxy.
> +#   local go proxy. Also warn that this usage is deprecated.
>  # - Otherwise, if EGO_VENDOR is set, bail out.
> -# - Otherwise do a normal unpack.
> +# - Otherwise do a normal unpack and move the vendor directory into
> +# place if appropriate.
>  go-module_src_unpack() {
>  	if [[ "${#EGO_SUM[@]}" -gt 0 ]]; then
> +		eqawarn "This ebuild uses EGO_SUM which is deprecated"
> +		eqawarn "Please migrate to a vendor tarball"
> +		eqawarn "This will become a fatal error in the future"
>  		_go-module_src_unpack_gosum
>  	elif [[ "${#EGO_VENDOR[@]}" -gt 0 ]]; then
>  		eerror "${EBUILD} is using EGO_VENDOR which is no longer supported"
>  		die "Please update this ebuild"
>  	else
>  		default
> +		[[ -d "${S}"/vendor ]] && return
> +		[[ ! -d vendor ]] && return
> +		mv vendor "${S}" || die
>  	fi
>  }
>
>  # @FUNCTION: _go-module_src_unpack_gosum
>  # @DESCRIPTION:
> -# Populate a GOPROXY directory hierarchy with distfiles from EGO_SUM and
> -# unpack the base distfiles.
> -#
> -# Exports GOPROXY environment variable so that Go calls will source the
> -# directory correctly.
> +# This function is deprecated.
>  _go-module_src_unpack_gosum() {
>  	# shellcheck disable=SC2120
>  	debug-print-function "${FUNCNAME}" "$@"
> @@ -392,10 +327,7 @@ _go-module_src_unpack_gosum() {
>
>  # @FUNCTION: _go-module_gosum_synthesize_files
>  # @DESCRIPTION:
> -# Given a path &  version, populate all Goproxy metadata files which aren't
> -# needed to be downloaded directly.
> -# - .../@v/${version}.info
> -# - .../@v/list
> +# This function is deprecated.
>  _go-module_gosum_synthesize_files() {
>  	local target=$1
>  	local version=$2
> @@ -419,8 +351,7 @@ _go-module_gosum_synthesize_files() {
>
>  # @FUNCTION: _go-module_src_unpack_verify_gosum
>  # @DESCRIPTION:
> -# Validate the Go modules declared by EGO_SUM are sufficient to cover building
> -# the package, without actually building it yet.
> +# This function is deprecated.
>  _go-module_src_unpack_verify_gosum() {
>  	# shellcheck disable=SC2120
>  	debug-print-function "${FUNCNAME}" "$@"
> @@ -435,7 +366,7 @@ _go-module_src_unpack_verify_gosum() {
>  	# This will print 'downloading' messages, but it's accessing content from
>  	# the $GOPROXY file:/// URL!
>  	einfo "Tidying go.mod/go.sum"
> -	go mod tidy >/dev/null
> +	ego mod tidy >/dev/null
>
>  	# This used to call 'go get' to verify by fetching everything from the main
>  	# go.mod. However 'go get' also turns out to recursively try to fetch
> @@ -461,16 +392,13 @@ go-module_live_vendor() {
>  		die "${FUNCNAME} only allowed when upstream isn't vendoring"
>
>  	pushd "${S}" >& /dev/null || die
> -	go mod vendor || die
> +	ego mod vendor
>  	popd >& /dev/null || die
>  }
>
>  # @FUNCTION: _go-module_gomod_encode
>  # @DESCRIPTION:
> -# Encode the name(path) of a Golang module in the format expected by Goproxy.
> -#
> -# Upper letters are replaced by their lowercase version with a '!' prefix.
> -#
> +# This function is deprecated.
>  _go-module_gomod_encode() {
>  	## Python:
>  	# return re.sub('([A-Z]{1})', r'!\1', s).lower()

-- 
Best regards,
Michał Górny