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

[William Hubbs]

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 <will...@gentoo.org>
---
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
+#
+# @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()
--
2.34.1

[Michał Górny]

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.

Best regards,
Michał Górny

[William Hubbs]

Using -a and -J together doesn't make sense since -a uses the suffix on
the file name to determine which compressor to use.

If I change the instructions as you suggest to re-package upstream's
tarball the name wouldn't need the -vendor suffix since the tarball would be a
fork, but I'm not asking people to fork and repackage upstream's tarballs, just
the vendor folder. Fortunately, there is no reason to fork since it is very
easy to package the vendor folder with Go's tools.

To move the unpacked vendor folder to the right place, I have two
choices, src_unpack and src_prepare. Since we already have a custom
src_unpack, it is less invasive to do this there. If I use src_prepare,
it would force me to bump the eclass, which I'm not interested in doing.

William

[Ionen Wolkens]

On Sat, Feb 26, 2022 at 11:55:10AM -0600, William Hubbs wrote:
> If I change the instructions as you suggest to re-package upstream's
> tarball the name wouldn't need the -vendor suffix since the tarball would be a
> fork, but I'm not asking people to fork and repackage upstream's tarballs, just
> the vendor folder. Fortunately, there is no reason to fork since it is very
> easy to package the vendor folder with Go's tools.

Doing:
tar ... project-1.0-vendor.tar.xz project-1.0/vendor

would only package the "vendor" directory and not the whole
project directory. Then when auto-unpacked by src_unpack, the
vendor directory is added under project-1.0/

So this isn't repacking upstream's or a fork.

--
ionen

[William Hubbs]

Ok, I get what you are saying, but normally the directory you have the
package in doesn't have a version number if you are working from a git
repository, so I don't know how to get around this:

cd path/to/project
# note, no version number in "project"
go mod vendor
cd ..
tar -acf project/vendor project-1.0-vendor.tar.xz

This will put everything in the tarball under "project/" instead of
"project-1.0/".

Does tar have an option to change that top level path that I don't know
about?

Thanks,

William

[Michał Górny]

If you're working from a git repo, you should use 'git archive' and it
has --prefix= option that does what you need.

git archive --prefix=project-1.0/ vendor |
xz -9 > project-1.0-vendor.tar.xz

[William Hubbs]

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 <will...@gentoo.org>
---

eclass/go-module.eclass | 144 +++++++++++-----------------------------
1 file changed, 38 insertions(+), 106 deletions(-)

diff --git a/eclass/go-module.eclass b/eclass/go-module.eclass
index 66fe52c9ad7..b9686ad555a 100644
--- a/eclass/go-module.eclass
+++ b/eclass/go-module.eclass
@@ -25,9 +25,26 @@

#
# 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-vendor.tar.xz vendor --xform 's:^":project-1.0/:'
+#
+# @CODE
+#
+# Note that 'go mod vendor' drops files from the dependency tree, and
+# this causes errors with some builds. If your build crashes with errors
+# like the ones mentioned in the below issue, you will need to emerge
+# dev-go/vend and replace 'go mod vendor' above with 'vend'.
+#
+# https://github.com/golang/go/issues/26366
+#

# Since Go programs are statically linked, it is important that your ebuild's
# LICENSE= setting includes the licenses of all statically linked

# dependencies. So please make sure it is accurate.
@@ -40,15 +57,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 +111,6 @@ QA_FLAGS_IGNORED='.*'

@@ -181,7 +126,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 +144,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 +197,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 +230,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,11 +270,15 @@ 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"

@@ -347,11 +290,7 @@ go-module_src_unpack() {

# @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 +331,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 +355,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 +370,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 +396,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()
--

2.34.1

[Ionen Wolkens]

On Sat, Feb 26, 2022 at 10:38:33PM -0600, William Hubbs wrote:
> diff --git a/eclass/go-module.eclass b/eclass/go-module.eclass
> index 66fe52c9ad7..b9686ad555a 100644
> --- a/eclass/go-module.eclass
> +++ b/eclass/go-module.eclass
> @@ -25,9 +25,26 @@
> #
> # 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

Unnecessary double space.

> +# 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-vendor.tar.xz vendor --xform 's:^":project-1.0/:'

Typo, that " doesn't exist at start of paths and it'd do nothing.
Should just be 's:^:project-1.0/:'

> -# 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"

Since example use .xz, should use it over .gz for vendor tarball.

> @@ -256,7 +197,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}"

Unnecessary comment that duplicates the now-used above line.

--
ionen

[Ulrich Mueller]

>>>>> On Sun, 27 Feb 2022, Ionen Wolkens wrote:

> On Sat, Feb 26, 2022 at 10:38:33PM -0600, William Hubbs wrote:
>> +# eclass. If it doesn't, you need to also create a vendor tarball and

> Unnecessary double space.

No. It is a sentence end, so the double space is mandatory.

(The reason is that we generate manpages from eclass documentation,
so we should follow groff conventions for sentence ends.)

Ulrich

[William Hubbs]

This is fixed.

>
> > -# 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"
>
> Since example use .xz, should use it over .gz for vendor tarball.

Fixed, but note that it doesn't really matter. you can use .z, .gz,
.zip, or whatever you want to use.

>
> > @@ -256,7 +197,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}"
>
> Unnecessary comment that duplicates the now-used above line.

This part of the eclass is out of scope for what I was changing, and
since most of this code will go away eventually, I'm not really worried
about this extra comment.

William

[William Hubbs]

I've never heard of this convention, and there many places in eclasses,
including this one, that have a single space as a sentence end.
that have a single space at a sentence end, so I will go ahead and fix
this.

William

[Sam James]

> On 27 Feb 2022, at 04:38, William Hubbs <will...@gentoo.org> 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 <will...@gentoo.org>
> ---

Per comments on IRC, I suggest not removing the EGO_SUM
examples for now until we've got good documentation and tooling
for vendor stuff, and a transition period is completed.

We can do it in a little bit but let's not add the new thing and remove
the old one in the same swoop.

Best,
sam

[William Hubbs]

On Tue, Mar 01, 2022 at 09:03:52PM +0000, Sam James wrote:
>
>
> > On 27 Feb 2022, at 04:38, William Hubbs <will...@gentoo.org> 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 <will...@gentoo.org>
> > ---
>
> Per comments on IRC, I suggest not removing the EGO_SUM
> examples for now until we've got good documentation and tooling
> for vendor stuff, and a transition period is completed.

I would rather not keep the EGO_SUM documentation. I do not want any
new ebuilds coming into the tree using it.

I am willing to flag EGO_SUM as deprecated if a variable can be flagged
as deprecated; that is what I'm looking up now.

William

[Anna Vyalkova]

On 2022-03-01 15:55, William Hubbs wrote:
> I am willing to flag EGO_SUM as deprecated if a variable can be flagged
> as deprecated; that is what I'm looking up now.

EGO_SUM is often the only choice for overlays, so consider not
deprecating it.

[William Hubbs]

EGO_SUM does not work for large SRC_URI; that is the reason it is
being deprecated.

Also, my understanding is we do not normally keep code around
if that code's only purpose is to support overlays.

Thanks,

William

[Sam James]

I don't think there's a need to rip it out given we know it can be quite
useful there though, right?

It's not actively harming anything to just keep the small amount of code there.

I think it's quite a nice convenience option (in fact, for ::gentoo too), where
EGO_SUM isn't huge. But I admit this isn't that common.

As for the series: as per commits on IRC: fine to compromise on you
removing it now, but please do in two separate commits so it's more
obvious in the git history. But that's notwithstanding possibly keeping it
for overlays.

[William Hubbs]

On Fri, Mar 04, 2022 at 12:01:42AM +0000, Sam James wrote:
>
>
> > On 4 Mar 2022, at 00:00, William Hubbs <will...@gentoo.org> wrote:
> >
> > On Wed, Mar 02, 2022 at 09:32:14PM +0500, Anna Vyalkova wrote:
> >> On 2022-03-01 15:55, William Hubbs wrote:
> >>> I am willing to flag EGO_SUM as deprecated if a variable can be flagged
> >>> as deprecated; that is what I'm looking up now.
> >>
> >> EGO_SUM is often the only choice for overlays, so consider not
> >> deprecating it.
> >
> > EGO_SUM does not work for large SRC_URI; that is the reason it is
> > being deprecated.
> >
> > Also, my understanding is we do not normally keep code around
> > if that code's only purpose is to support overlays.
>
> I don't think there's a need to rip it out given we know it can be quite
> useful there though, right?
>
> It's not actively harming anything to just keep the small amount of code there.

Sure, it may not be harmful to keep it there, but using it in the main
tree isn't going to be a thing.

> I think it's quite a nice convenience option (in fact, for ::gentoo too), where
> EGO_SUM isn't huge. But I admit this isn't that common.

Another thing to consider is that the bulk of the code in the eclass is
for handling EGO_SUM, so if I can remove it, ultimately, it will mean less code to maintain.

I will post a separate patch which I will not merge at this point to
show what I'm talking about.

Thanks,

William