Re: [gentoo-dev] [PATCH v2 1/4] eclass/go-module: add support for building based on go.sum

[William Hubbs]

On Wed, Feb 19, 2020 at 09:20:13AM -0600, William Hubbs wrote:
> On Wed, Feb 19, 2020 at 07:36:27AM +0000, Robin H. Johnson wrote:
> > On Tue, Feb 18, 2020 at 11:46:45PM -0600, William Hubbs wrote:
> > > > -# If it does not have a vendor directory, you should use the EGO_VENDOR
> > > > +# Alternatively, older versions of this eclass used the EGO_VENDOR
> > > >  # variable and the go-module_vendor_uris function as shown in the
> > > >  # example below to handle dependencies.
> > > I think we can remove the example with EGO_VENDOR and
> > > go-module_vendor_uris; we really don't want people to continue following
> > > that example.
> > I tried to handle more cases here, but now I'm wondering if it would be
> > cleaner just to put all of new way into a distinct eclass, and sunset
> > the old eclass entirely. I found unforeseen interactions, see below.
> >
> > > > +# S="${WORKDIR}/${MY_P}"
> > > The default setting of S should be fine for most ebuilds, so I don't
> > > think we need this in the example.
> > I'd copied it, but yes in this case.
> >
> > >
> > > > +# go-module_set_globals
> > > > +#
> > > > +# SRC_URI="https://github.com/example/${PN}/archive/v${PV}.tar.gz -> ${P}.tar.gz
> > > > +#		   ${EGO_SUM_SRC_URI}"
> > > > +#
> > > > +# LICENSE="some-license ${EGO_SUM_LICENSES}"
> > > > +#
> > > > +# src_unpack() {
> > > > +# 	unpack ${P}.tar.gz
> > > > +#	go-module_src_unpack
> > > > +# }
> > >  I don't think I would put an src_unpack() in the example.
> > This is one of the unforeseen interactions.
> > The go.sum unpack only applies special handling to distfiles that it
> > knows about. It does NOT process any other distfiles at all.
> >
> > EAPI8 or future Portage improvements might have annotations to disable
> > any automatic unpacking for specific distfiles, which would resolve this
> > issue.
> >
> > Hence, you need to explicitly unpack any distfiles that are NOT part of
> > the go.sum dependencies. There are some ebuilds that do unpack & rename
> > in src_unpack already, so they need extra care as well.
> >
> > The EGO_VENDOR src_unpack unpacked EVERYTHING, so it didn't have this
> > issue.

It used filtering to decide what to unpack where, so I think we can use
the same idea.  Look at this patch to what is in the tree currently.

Look at this patch. Part of it is just comments, but I think this could
work if module_files is populated correctly.

diff --git a/eclass/go-module.eclass b/eclass/go-module.eclass
index 80ff2902b3a..3e0091a0d25 100644
--- a/eclass/go-module.eclass
+++ b/eclass/go-module.eclass
@@ -113,7 +113,8 @@ go-module_vendor_uris() {
 # ${EGO_VENDOR} to ${S}/vendor.
 go-module_src_unpack() {
 	debug-print-function ${FUNCNAME} "$@"
-	local f hash import line repo tarball vendor_tarballs x
+	local f hash import line repo tarball module_files vendor_tarballs x
+	module_files=()
 	vendor_tarballs=()
 	for line in "${EGO_VENDOR[@]}"; do
 		read -r import hash repo x <<< "${line}"
@@ -125,9 +126,15 @@ go-module_src_unpack() {
 		: "${repo:=${import}}"
 		vendor_tarballs+=("${repo//\//-}-${hash}.tar.gz")
 	done
+	# populate module_files with the files we do not want to unpack
+	# based on EGO_SUM
+
+	# unpack the appropriate files from $A
 	for f in $A; do
 		[[ -n ${vendor_tarballs[*]} ]] && has "$f" "${vendor_tarballs[@]}" &&
 			continue
+		[[ -n ${module_files[*]} ]] && has "$f" "${module_files[@]}" &&
+			continue
 		unpack "$f"
 	done