[PATCH 1/3] eclass/go-module: add support for building based on go.sum

classic Classic list List threaded Threaded
31 messages Options
12
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 3/3] app-admin/kube-bench: convert to go-module go.sum

Sam Jorna (wraeth)
On Thursday, 13 February 2020 5:40:46 AM AEDT Matt Turner wrote:

> On Wed, Feb 12, 2020 at 9:59 AM William Hubbs <[hidden email]> wrote:
> > On Wed, Feb 12, 2020 at 06:54:19PM +1100, Sam Jorna (wraeth) wrote:
> > > On Monday, 10 February 2020 7:55:01 AM AEDT Michał Górny wrote:
> > > > On Sun, 2020-02-09 at 20:38 +0000, Michael 'veremitz' Everitt wrote:
> > > > > Hrm, pardon my ignorance, but do 'we' really need to review 232
> > > > > lines of
> > > > > Manifest?!
> > > >
> > > > Pardon mine but do 'we' really need to read your useless comments
> > > > everywhere, all the time and just get irritated for no benefit to
> > > > Gentoo?
> > >
> > > Perhaps I'm the one being ignorant here, but why are we lambasting
> > > someone for seeking clarification about an unusual inclusion on a
> > > review thread?>
> > I wasn't going to say anything, but I can't let this go by without
> > commenting.
> >
> > Sam is correct. Maybe the tone is a bit off, (and that is debatable),
> > but this definitely can be seen as a legit question, regardless of other
> > things Michael has posted.
>
> Unfortunately it's not about a single issue or email. It's a
> consistent pattern that multiple people have asked him to rein in over
> a long period. :(
Without going into specifics, veremit and I have certainly had our 'differences
of opinion' in the past; but I don't believe this is one of those occasions.

Calling out bad actors (not saying veremit is one, I just mean in the general
sense) is an unfortunate but important task, but call them out on bad
behaviour, not for what appears to be an impassioned but otherwise
unremarkable query.

--
Sam Jorna (wraeth)
GnuPG ID: 0xD6180C26


signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 3/3] app-admin/kube-bench: convert to go-module go.sum

Mike Pagano
On Thu, Feb 13, 2020 at 06:46:43PM +1100, Sam Jorna (wraeth) wrote:

> On Thursday, 13 February 2020 5:40:46 AM AEDT Matt Turner wrote:
> > On Wed, Feb 12, 2020 at 9:59 AM William Hubbs <[hidden email]> wrote:
> > > On Wed, Feb 12, 2020 at 06:54:19PM +1100, Sam Jorna (wraeth) wrote:
> > > > On Monday, 10 February 2020 7:55:01 AM AEDT Michał Górny wrote:
> > > > > On Sun, 2020-02-09 at 20:38 +0000, Michael 'veremitz' Everitt wrote:
> > > > > > Hrm, pardon my ignorance, but do 'we' really need to review 232
> > > > > > lines of
> > > > > > Manifest?!
> > > > >
> > > > > Pardon mine but do 'we' really need to read your useless comments
> > > > > everywhere, all the time and just get irritated for no benefit to
> > > > > Gentoo?
> > > >
> > > > Perhaps I'm the one being ignorant here, but why are we lambasting
> > > > someone for seeking clarification about an unusual inclusion on a
> > > > review thread?>
> > > I wasn't going to say anything, but I can't let this go by without
> > > commenting.
> > >
> > > Sam is correct. Maybe the tone is a bit off, (and that is debatable),
> > > but this definitely can be seen as a legit question, regardless of other
> > > things Michael has posted.
> >
> > Unfortunately it's not about a single issue or email. It's a
> > consistent pattern that multiple people have asked him to rein in over
> > a long period. :(
>
> Without going into specifics, veremit and I have certainly had our 'differences
> of opinion' in the past; but I don't believe this is one of those occasions.
>
> Calling out bad actors (not saying veremit is one, I just mean in the general
> sense) is an unfortunate but important task, but call them out on bad
> behaviour, not for what appears to be an impassioned but otherwise
> unremarkable query.
>

I agree with this 100 percent.  Not judging solely on the content of the
specific email in the thread does not allow people to grow and improve. Are we
all to be judged on our past behavior forever with no chance to overcome past
transgressions ?



> --
> Sam Jorna (wraeth)
> GnuPG ID: 0xD6180C26
>



--
Mike Pagano
Gentoo Developer - Kernel Project
Gentoo Sources - Member
E-Mail     : [hidden email]
GnuPG FP   : EEE2 601D 0763 B60F 848C  9E14 3C33 C650 B576 E4E3
Public Key : http://pgp.mit.edu:11371/pks/lookup?search=0xB576E4E3&op=index

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/3] eclass/go-module: add support for building based on go.sum

Michał Górny-5
In reply to this post by Robin H. Johnson-2
On Sun, 2020-02-09 at 12:31 -0800, Robin H. Johnson wrote:

> EGO_SUM mode now supplements the existing EGO_VENDOR mode.
>
> EGO_SUM should be populated by the maintainer, directly from the go.sum
> file of the root package. See eclass and conversion example
> (dev-go/go-tour & app-admin/kube-bench) for further details.
>
> The go-module_set_globals function performs validation of
> inputs and does die on fatal errors.
>
> Signed-off-by: Robin H. Johnson <[hidden email]>
> ---
>  eclass/go-module.eclass    | 328 +++++++++++++++++++++++++++++++++++--
>  profiles/thirdpartymirrors |   1 +
>  2 files changed, 311 insertions(+), 18 deletions(-)
>
> diff --git eclass/go-module.eclass eclass/go-module.eclass
> index d5de5f60ccdf..b8a635d52de7 100644
> --- eclass/go-module.eclass
> +++ eclass/go-module.eclass
> @@ -4,22 +4,46 @@
>  # @ECLASS: go-module.eclass
>  # @MAINTAINER:
>  # William Hubbs <[hidden email]>
> +# @AUTHOR:
> +# William Hubbs <[hidden email]>
> +# Robin H. Johnson <[hidden email]>
>  # @SUPPORTED_EAPIS: 7
>  # @BLURB: basic eclass for building software written as go modules
>  # @DESCRIPTION:
> -# This eclass provides basic settings and functions
> -# needed by all software written in the go programming language that uses
> -# go modules.
> +# This eclass provides basic settings and functions needed by all software
> +# written in the go programming language that uses go modules.
> +#
> +# You might know the software you are packaging uses modules because
> +# it has files named go.sum and go.mod in its top-level source directory.
> +# If it does not have these files, try use the golang-* eclasses FIRST!
> +# There ARE legacy Golang packages that use external modules with none of
> +# go.mod, go.sum, vendor/ that can use this eclass regardless.
> +#
> +# Guidelines for usage:
> +# "go.mod" && "go.sum" && "vendor/":
> +# - pre-vendored package. Do NOT set EGO_SUM or EGO_VENDOR.
> +#
> +# "go.mod" && "go.sum":
> +# - Populate EGO_SUM with entries from go.sum
> +# - Do NOT include any lines that contain <version>/go.mod
> +#
> +# "go.mod" only:
> +# - Populate EGO_VENDOR
>  #
> -# You will know the software you are packaging uses modules because
> -# it will have files named go.sum and go.mod in its top-level source
> -# directory. If it does not have these files, use the golang-* eclasses.
> +# None of the above:
> +# - Did you try golang-* eclasses first? Upstream has undeclared dependencies
> +#   (perhaps really old source). You can use either EGO_SUM or EGO_VENDOR.
> +
>  #
> -# If it has these files and a directory named vendor in its top-level
> -# source directory, you only need to inherit the eclass since upstream
> -# is vendoring the dependencies.
> +# If it has these files AND a directory named "vendor" in its top-level source
> +# directory, you only need to inherit the eclass since upstream has already
> +# vendored the dependencies.
> +
> +# If it does not have a vendor directory, you should use the EGO_SUM
> +# variable and the go-module_gosum_uris function as shown in the
> +# example below to handle dependencies.
>  #
> -# 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.
>  #
> @@ -28,6 +52,21 @@
>  # dependencies. So please make sure it is accurate.
>  #
>  # @EXAMPLE:
> +# @CODE
> +#
> +# inherit go-module
> +#
> +# EGO_SUM=(
> +# "github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ="
> +# "github.com/BurntSushi/toml v0.3.1/go.mod h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ="
Is it expected that the two entries would have the same hash?

> +# )
> +# S="${WORKDIR}/${MY_P}"
> +# go-module_set_globals
> +#
> +# SRC_URI="https://github.com/example/${PN}/archive/v${PV}.tar.gz -> ${P}.tar.gz
> +# ${EGO_SUM_SRC_URI}"
> +#
> +# @CODE
>  #
>  # @CODE
>  #
> @@ -35,7 +74,7 @@
>  #
>  # EGO_VENDOR=(
>  # "github.com/xenolf/lego 6cac0ea7d8b28c889f709ec7fa92e92b82f490dd"
> -# "golang.org/x/crypto 453249f01cfeb54c3d549ddb75ff152ca243f9d8 github.com/golang/crypto"
> +# "golang.org/x/crypto 453249f01cfeb54c3d549ddb75ff152ca243f9d8 github.com/golang/crypto"
>  # )
>  #
>  # SRC_URI="https://github.com/example/${PN}/archive/v${PV}.tar.gz -> ${P}.tar.gz
> @@ -64,10 +103,12 @@ export GO111MODULE=on
>  export GOCACHE="${T}/go-build"
>  
>  # The following go flags should be used for all builds.
> -# -mod=vendor stopps downloading of dependencies from the internet.
>  # -v prints the names of packages as they are compiled
>  # -x prints commands as they are executed
> -export GOFLAGS="-mod=vendor -v -x"
> +# -mod=vendor use the vendor directory instead of downloading dependencies
> +# -mod=readonly do not update go.mod/go.sum but fail if updates are needed
> +export GOFLAGS="-v -x -mod=readonly"
> +[[ ${#EGO_VENDOR[@]} -gt 0 ]] && GOFLAGS+=" -mod=vendor"
>  
>  # Do not complain about CFLAGS etc since go projects do not use them.
>  QA_FLAGS_IGNORED='.*'
> @@ -75,7 +116,23 @@ QA_FLAGS_IGNORED='.*'
>  # Go packages should not be stripped with strip(1).
>  RESTRICT="strip"
>  
> -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?

> +
> +# @ECLASS-VARIABLE: EGO_SUM
> +# @DESCRIPTION:
> +# This variable duplicates the go.sum content from inside the target package.
> +# Entries of the form <version>/go.mod should be excluded.

...but you've included one of them in the example on top of the eclass.

> +#
> +# <module> <version> <hash>

Now I'm confused.  Unless my eyes betray me, PATCH 2 has entries without
hash.

Also, the description fails to mention that you're supposed to quote
each line.

> +#
> +# The format is described upstream here:
> +# https://tip.golang.org/cmd/go/#hdr-Module_authentication_using_go_sum
> +#
> +# <hash> is the Hash1 structure used by upstream Go
> +# Note that 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. E.g. it does NOT capture mtime changes in
> +# files within a zipfile.

I think it would be valuable to include an example here as well.

>  
>  # @ECLASS-VARIABLE: EGO_VENDOR
>  # @DESCRIPTION:
> @@ -106,13 +163,202 @@ go-module_vendor_uris() {
>   done
>  }
>  
> +# @ECLASS-VARIABLE: GOMODULE_GOPROXY_BASEURI
> +# @DESCRIPTION:
> +# Golagg module proxy service to fetch module files from. Note that the module
Typo: golagg -> golang.

> +# 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://goproxy/ to
> +# https://goproxy.cn/, or change this variable.
> +# See https://arslan.io/2019/08/02/why-you-should-use-a-go-module-proxy/ for further details
> +: "${GOMODULE_GOPROXY_BASEURI:=mirror://goproxy/}"

'Changing this variable' sounds like violating metadata immutability
rule and running in trouble with the caches.

> +
> +# @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 distfile back
> +#   to the relative part of SRC_URI, as needed for GOPROXY=file:///...
> +go-module_set_globals() {
> + local line error_in_gosum errorlines errormsg exts
> + local newline=$'\n'
> + error_in_gosum=0
> + errorlines=( )
> + 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} != "h1:" ]] ; then
Please use ${foo} everywhere consistently, and put && inside [[ ]].
Also, I dare say wildcard match is more readable than hardcoding string
length, i.e.:

  [[ -n ${hash1} && ${hash1} != h1:* ]]

> + error_in_gosum=1
> + errorlines+=( "Unknown hash: ${line}" )
> + elif [[ -n $x ]]; then
> + error_in_gosum=1
> + errorlines+=( "Trailing data: ${line}" )
> + fi
> +
> + # Split 'v0.3.0/go.mod' into 'v0.3.0' and '/go.mod'
> + version=${version_modfile%%/*}
> + modfile=${version_modfile#*/}
> + [[ "$modfile" == "${version_modfile}" ]] && modfile=
Check the initial string, not the result of arbitrary manipulations
on it.  This would wrongly evaluate true for 'v0.3.0/v0.3.0'.

> +
> + # The trailing part should be either empty or '/go.mod'
> + # There is a chance that upstream Go might add something else here in
> + # future, and we should be prepared to capture it.
> + exts=()
> + errormsg=''
> + case "$modfile" in
> + '') exts=( mod info zip ) ;;
> + 'go.mod'|'/go.mod') exts=( mod info ) ;;
> + #'go.mod'|'/go.mod') errormsg="Prohibited file: You must exclude /go.mod lines from EGO_SUM! " ;;
Why is it commented out?

> + *) errormsg="Unknown modfile: line='${line}', modfile='${modfile}'" ;;
> + esac
> +
> + # If it was a bad entry, restart the loop
> + if [[ -n $errormsg ]]; then
> + error_in_gosum=1
> + errorlines+=( "${errormsg} line='${line}', modfile='${modfile}'" )
> + continue
> + fi
> +
> + # Directory structure for Go proxy hosts:
> + # - def encode(s):
> + #     return re.sub('([A-Z]{1})', r'!\1', s).lower()
> + #
> + # Sed variant:
> + # This uses GNU Sed extension \l to downcase the match
> + #_dir=$(echo "${module}" |sed 's,[A-Z],!\l&,g')
> + #
> + # Bash variant:
> + re='(.*)([A-Z])(.*)'
> + input=${module}
> + while [[ $input =~ $re ]]; do
> + lower='!'"${BASH_REMATCH[2],}"
> + input="${BASH_REMATCH[1]}${lower}${BASH_REMATCH[3]}"
> + done
> + _dir=$input
> + unset lower input re
> +
> + for _ext in "${exts[@]}" ; do
> + # Relative URI within a GOPROXY for a file
> + _reluri="${_dir}/@v/${version}.${_ext}"
> + # SRC_URI: LHS entry
> + _uri="${GOMODULE_GOPROXY_BASEURI}/${_reluri}"
> + # SRC_URI: RHS entry, encode any slash in the path as %2F in the filename
> + _distfile="${_reluri//\//%2F}"
> +
> + EGO_SUM_SRC_URI+=" ${_uri} -> ${_distfile}${newline}"
> + _EGO_SUM_MAPPING+=" ${_distfile}:${_reluri}${newline}"
> + done
> + done
> +
> + if [[ $error_in_gosum != 0 ]]; then
> + eerror "Trailing information in EGO_SUM in ${P}.ebuild"
> + for line in "${errorlines[@]}" ; do
> + eerror "${line}"
> + done
> + die "Invalid EGO_SUM format"
> + fi
> +
> + # Ensure these variables not not changed past this point
> + readonly EGO_SUM
> + readonly EGO_SUM_SRC_URI
> + readonly _EGO_SUM_MAPPING
> +
> + # Set the guard that we are safe
> + _GO_MODULE_SET_GLOBALS_CALLED=1
> +}
> +
> +
>  # @FUNCTION: go-module_src_unpack
>  # @DESCRIPTION:
> +# Extract & configure Go modules for consumpations.
> +# - Modules listed in EGO_SUM are configured as a local GOPROXY via symlinks (fast!)
> +# - Modules listed in EGO_VENDOR are extracted to "${S}/vendor" (slow)
> +#
> +# This function does NOT unpack the base distfile of a Go-based package.
> +# While the entries in EGO_SUM will be listed in ${A}, they should NOT be
> +# unpacked, Go will directly consume the files, including zips.
> +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?

> + else
> + die "Neither EGO_SUM nor EGO_VENDOR are set!"
> + fi
> +}
> +
> +# @FUNCTION: go-module_src_prepare
> +# @DESCRIPTION:
> +# Prepare for building. Presently only needed for EGO_SUM variant.
> +go-module_src_prepare() {
> + # shellcheck disable=SC2120
> + debug-print-function "${FUNCNAME}" "$@"
> +
> + if [[ "${#EGO_SUM[@]}" -gt 0 ]]; then
> + _go-module_src_prepare_gosum
> + fi
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.

> +
> + default
> +}
> +
> +# @ECLASS-VARIABLE: GOMODULE_GOSUM_PATH
> +# @DESCRIPTION:
> +# Path to root go.sum of package. If your ebuild modifies S after inheriting
> +# the eclass, you may need to update this variable.
> +: "${GO_MODULE_GOSUM_PATH:=${S}/go.sum}"

Wouldn't it be cleaner to have the path relative to ${S} by default?

> +
> +# @FUNCTION: _go-module_src_unpack_gosum
> +# @DESCRIPTION:
> +# Populate a GOPROXY directory hierarchy with distfiles from EGO_SUM
> +#
> +# Exports GOPROXY environment variable so that Go calls will source the
> +# directory correctly.
> +_go-module_src_unpack_gosum() {
> + # shellcheck disable=SC2120
> + debug-print-function "${FUNCNAME}" "$@"
> +
> + if [[ ! ${_GO_MODULE_SET_GLOBALS_CALLED} ]]; then
> + die "go-module_set_globals must be called in global scope"
> + fi
> +
> + local goproxy_dir="${T}/goproxy"
> + local goproxy_mod_dir
> + mkdir -p "${goproxy_dir}"
> + # Convert the list format to an associative array to avoid O(N*M)
> + # performance when populating the GOPROXY directory structure.
> + declare -A _EGO_SUM_MAPPING_ASSOC
Why not make it local?

> + for s in ${_EGO_SUM_MAPPING}; do
> + a=${s//:*}
> + b=${s//*:}
> + _EGO_SUM_MAPPING_ASSOC["$a"]=$b
> + done
> +
> + # For each Golang module distfile, look up where it's supposed to go, and
> + # symlink into place.
> + for _A in ${A}; do

This one looks like local candidate as well.

> + goproxy_mod_path="${_EGO_SUM_MAPPING_ASSOC["${_A}"]}"
> + if [[ -n "${goproxy_mod_path}" ]]; then
> + einfo "Populating goproxy for $goproxy_mod_path"
> + # Build symlink hierarchy
> + goproxy_mod_dir=$( dirname "${goproxy_dir}"/"${goproxy_mod_path}" )
> + mkdir -p "${goproxy_mod_dir}"

|| die

> + ln -sf "${DISTDIR}"/"${_A}" "${goproxy_dir}/${goproxy_mod_path}" || die "Failed to ln"
> + fi
> + done
> + export GOPROXY="file://${goproxy_dir}"
> + unset _EGO_SUM_MAPPING_ASSOC
> +}
> +
> +# @FUNCTION: _go-module_src_unpack_vendor
> +# @DESCRIPTION:
>  # Extract all archives in ${a} which are not nentioned in ${EGO_VENDOR}
>  # to their usual locations then extract all archives mentioned in
>  # ${EGO_VENDOR} to ${S}/vendor.
> -go-module_src_unpack() {
> - debug-print-function ${FUNCNAME} "$@"
> +_go-module_src_unpack_vendor() {
> + # shellcheck disable=SC2120
> + debug-print-function "${FUNCNAME}" "$@"
>   local f hash import line repo tarball vendor_tarballs x
>   vendor_tarballs=()
>   for line in "${EGO_VENDOR[@]}"; do
> @@ -145,13 +391,59 @@ go-module_src_unpack() {
>   done
>  }
>  
> +# @FUNCTION: _go-module_src_prepare_gosum
> +# @DESCRIPTION:
> +# Validate the Go modules declared by EGO_SUM are sufficent to cover building
> +# the package, without actually building it yet.
> +_go-module_src_prepare_gosum() {
> + # shellcheck disable=SC2120
> + debug-print-function "${FUNCNAME}" "$@"
> +
> + if [[ ! ${_GO_MODULE_SET_GLOBALS_CALLED} ]]; then
> + die "go-module_set_globals must be called in global scope"
> + fi
> +
> + # go.sum entries ending in /go.mod aren't strictly needed at this phase
> + if [[ ! -e "${GO_MODULE_GOSUM_PATH}" ]]; then
> + die "Could not find package root go.sum, please update GO_MODULE_GOSUM_PATH"
> + fi
> + go-module_minimize_gosum "${GO_MODULE_GOSUM_PATH}"
> +
> + # Verify that all needed modules are present.
> + GO111MODULE=on \
> + go get -v -d -mod readonly || die "Some module is missing, update EGO_SUM"
> +
> + # Need to re-minimize because go-get expands it again
Why not create and restore a copy?  Or does go-get make other changes?

> + go-module_minimize_gosum "${GO_MODULE_GOSUM_PATH}"
> +}
> +
> +# @FUNCTION: go-module_minimize_gosum
> +# @DESCRIPTION:
> +# Remove all /go.mod entries from go.sum files
> +# In most cases, if go.sum only has a /go.mod entry without a corresponding
> +# direct entry, this is a sign of a weak dependency that is NOT required for
> +# building the package.
> +go-module_minimize_gosum() {
> + local gosumfile=${1}
> + if test ! -e "${gosumfile}".orig; then
Use [[ ... ]].  Be consistent.  This is an eclass, not a throwaway
script.

> + cp -f "${gosumfile}"{,.orig} || die
> + fi
> + awk -e '$2 ~ /\/go.mod$/{next} {print}' \
> + <"${gosumfile}".orig \
> + >"${gosumfile}" || die
> + if grep -sq /go.mod "${gosumfile}"; then
> + die "sed failed to remove all module go.mod entries from go.sum"

Err, but the rule for grep is inconsistent with the rule for awk.  It's
going to fail when 'go.mod' (i.e. go<ANYCHAR>mod) happens anywhere
on the line.

> + fi
> +}
> +
>  # @FUNCTION: go-module_live_vendor
>  # @DESCRIPTION:
>  # This function is used in live ebuilds to vendor the dependencies when
>  # upstream doesn't vendor them.
>  go-module_live_vendor() {
> - debug-print-function ${FUNCNAME} "$@"
> + debug-print-function "${FUNCNAME}" "$@"
>  
> + # shellcheck disable=SC2086
>   has live ${PROPERTIES} ||
>   die "${FUNCNAME} only allowed in live ebuilds"
>   [[ "${EBUILD_PHASE}" == unpack ]] ||
> @@ -168,7 +460,7 @@ go-module_live_vendor() {
>  # @DESCRIPTION:
>  # Display a warning about security updates for Go programs.
>  go-module_pkg_postinst() {
> - debug-print-function ${FUNCNAME} "$@"
> + debug-print-function "${FUNCNAME}" "$@"
>   [[ -n ${REPLACING_VERSIONS} ]] && return 0
>   ewarn "${PN} is written in the Go programming language."
>   ewarn "Since this language is statically linked, security"
> diff --git profiles/thirdpartymirrors profiles/thirdpartymirrors
> index ad4c4b972146..d60f166e07c9 100644
> --- profiles/thirdpartymirrors
> +++ profiles/thirdpartymirrors
> @@ -25,3 +25,4 @@ sourceforge https://download.sourceforge.net
>  sourceforge.jp http://iij.dl.sourceforge.jp https://osdn.dl.sourceforge.jp https://jaist.dl.sourceforge.jp
>  ubuntu http://mirror.internode.on.net/pub/ubuntu/ubuntu/ https://mirror.tcc.wa.edu.au/ubuntu/ http://ubuntu.uni-klu.ac.at/ubuntu/ http://mirror.dhakacom.com/ubuntu-archive/ http://ubuntu.c3sl.ufpr.br/ubuntu/ http://ubuntu.uni-sofia.bg/ubuntu/ http://hr.archive.ubuntu.com/ubuntu/ http://cz.archive.ubuntu.com/ubuntu/ https://mirror.dkm.cz/ubuntu http://ftp.cvut.cz/ubuntu/ http://ftp.stw-bonn.de/ubuntu/ https://ftp-stud.hs-esslingen.de/ubuntu/ https://mirror.netcologne.de/ubuntu/ https://mirror.unej.ac.id/ubuntu/ http://kr.archive.ubuntu.com/ubuntu/ https://mirror.nforce.com/pub/linux/ubuntu/ http://mirror.amsiohosting.net/archive.ubuntu.com/ http://nl3.archive.ubuntu.com/ubuntu/ https://mirror.timeweb.ru/ubuntu/ http://ubuntu.mirror.su.se/ubuntu/ https://ftp.yzu.edu.tw/ubuntu/ https://mirror.aptus.co.tz/pub/ubuntuarchive/ https://ubuntu.volia.net/ubuntu-archive/ https://mirror.sax.uk.as61049.net/ubuntu/ https://mirror.pnl.gov/ubuntu/ http://mirror.cc.columbia.edu/pub/linux/ubuntu/archive/ https://mirrors.namecheap.com/ubuntu/
>  vdr-developerorg http://projects.vdr-developer.org/attachments/download
> +goproxy https://proxy.golang.org/ https://goproxy.io/ https://gocenter.io/
--
Best regards,
Michał Górny


signature.asc (631 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 3/3] app-admin/kube-bench: convert to go-module go.sum

Matt Turner-5
In reply to this post by Mike Pagano
On Thu, Feb 13, 2020 at 4:12 AM Mike Pagano <[hidden email]> wrote:

>
> On Thu, Feb 13, 2020 at 06:46:43PM +1100, Sam Jorna (wraeth) wrote:
> > On Thursday, 13 February 2020 5:40:46 AM AEDT Matt Turner wrote:
> > > On Wed, Feb 12, 2020 at 9:59 AM William Hubbs <[hidden email]> wrote:
> > > > On Wed, Feb 12, 2020 at 06:54:19PM +1100, Sam Jorna (wraeth) wrote:
> > > > > On Monday, 10 February 2020 7:55:01 AM AEDT Michał Górny wrote:
> > > > > > On Sun, 2020-02-09 at 20:38 +0000, Michael 'veremitz' Everitt wrote:
> > > > > > > Hrm, pardon my ignorance, but do 'we' really need to review 232
> > > > > > > lines of
> > > > > > > Manifest?!
> > > > > >
> > > > > > Pardon mine but do 'we' really need to read your useless comments
> > > > > > everywhere, all the time and just get irritated for no benefit to
> > > > > > Gentoo?
> > > > >
> > > > > Perhaps I'm the one being ignorant here, but why are we lambasting
> > > > > someone for seeking clarification about an unusual inclusion on a
> > > > > review thread?>
> > > > I wasn't going to say anything, but I can't let this go by without
> > > > commenting.
> > > >
> > > > Sam is correct. Maybe the tone is a bit off, (and that is debatable),
> > > > but this definitely can be seen as a legit question, regardless of other
> > > > things Michael has posted.
> > >
> > > Unfortunately it's not about a single issue or email. It's a
> > > consistent pattern that multiple people have asked him to rein in over
> > > a long period. :(
> >
> > Without going into specifics, veremit and I have certainly had our 'differences
> > of opinion' in the past; but I don't believe this is one of those occasions.
> >
> > Calling out bad actors (not saying veremit is one, I just mean in the general
> > sense) is an unfortunate but important task, but call them out on bad
> > behaviour, not for what appears to be an impassioned but otherwise
> > unremarkable query.
> >
>
> I agree with this 100 percent.  Not judging solely on the content of the
> specific email in the thread does not allow people to grow and improve. Are we
> all to be judged on our past behavior forever with no chance to overcome past
> transgressions ?

That's not what's going on.

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 3/3] app-admin/kube-bench: convert to go-module go.sum

Sam Jorna (wraeth)
On Friday, 14 February 2020 2:21:32 PM AEDT Matt Turner wrote:
> On Thu, Feb 13, 2020 at 4:12 AM Mike Pagano <[hidden email]> wrote:
> > On Thu, Feb 13, 2020 at 06:46:43PM +1100, Sam Jorna (wraeth) wrote:
> > > On Thursday, 13 February 2020 5:40:46 AM AEDT Matt Turner wrote:
> > > > On Wed, Feb 12, 2020 at 9:59 AM William Hubbs <[hidden email]>
wrote:
> > > > > On Wed, Feb 12, 2020 at 06:54:19PM +1100, Sam Jorna (wraeth) wrote:
> > > > > > On Monday, 10 February 2020 7:55:01 AM AEDT Michał Górny wrote:
> > > > > > > On Sun, 2020-02-09 at 20:38 +0000, Michael 'veremitz' Everitt
wrote:

> > > > > > > > Hrm, pardon my ignorance, but do 'we' really need to review
> > > > > > > > 232
> > > > > > > > lines of
> > > > > > > > Manifest?!
> > > > > > >
> > > > > > > Pardon mine but do 'we' really need to read your useless
> > > > > > > comments
> > > > > > > everywhere, all the time and just get irritated for no benefit
> > > > > > > to
> > > > > > > Gentoo?
> > > > > >
> > > > > > Perhaps I'm the one being ignorant here, but why are we lambasting
> > > > > > someone for seeking clarification about an unusual inclusion on a
> > > > > > review thread?>
> > > > >
> > > > > I wasn't going to say anything, but I can't let this go by without
> > > > > commenting.
> > > > >
> > > > > Sam is correct. Maybe the tone is a bit off, (and that is
> > > > > debatable),
> > > > > but this definitely can be seen as a legit question, regardless of
> > > > > other
> > > > > things Michael has posted.
> > > >
> > > > Unfortunately it's not about a single issue or email. It's a
> > > > consistent pattern that multiple people have asked him to rein in over
> > > > a long period. :(
> > >
> > > Without going into specifics, veremit and I have certainly had our
> > > 'differences of opinion' in the past; but I don't believe this is one
> > > of those occasions.
> > >
> > > Calling out bad actors (not saying veremit is one, I just mean in the
> > > general sense) is an unfortunate but important task, but call them out
> > > on bad behaviour, not for what appears to be an impassioned but
> > > otherwise unremarkable query.
> >
> > I agree with this 100 percent.  Not judging solely on the content of the
> > specific email in the thread does not allow people to grow and improve.
> > Are we all to be judged on our past behavior forever with no chance to
> > overcome past transgressions ?
>
> That's not what's going on.
Maybe not; but that's what appears is going on.

mpagano is absolutely correct that people need an opportunity to engage
positively if they're expected to change their behaviour in a positive way. At
the same time, however, chastising someone without an apparent transgression
both reinforces negative behaviour (such as the subsequent mails on that sub-
thread) *and* gives anyone not intimately familiar with that particular case
the impression that people are ridiculed because they asked a relevant
question (since there's no other context).

In this instance, at least two people (myself included) have drawn an
impression that led them to voice their concern in some way (I'm unsure if
mpagano was voicing concern or just agreeing with the general concept). Maybe
we're the only ones. Maybe not.

I understand that you or others may have had a history of issues dealing with
someone, perhaps to the point where even seeing their name puts a damper on
your day. I'm sure there are people who feel the same about me. But let bad
actors dig their own hole to fall in, even if you're certain beyond the shadow
of a doubt that they're sitting in a darkened lair, twirling their moustache
and saying things like "my evil plan is coming to fruition!"; because lashing
out at an unrelated list post just looks like you're being an asshole.

--
Sam Jorna (wraeth)
GnuPG ID: 0xD6180C26


signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 3/3] app-admin/kube-bench: convert to go-module go.sum

Matt Turner-5
On Fri, Feb 14, 2020 at 12:31 AM Sam Jorna (wraeth) <[hidden email]> wrote:
> In this instance, at least two people (myself included) have drawn an
> impression that led them to voice their concern in some way (I'm unsure if
> mpagano was voicing concern or just agreeing with the general concept). Maybe
> we're the only ones. Maybe not.

What do you think the threshold should be? If one person objects,
should ComRel cease and desist? Two? Should we have a Gentoo-wide
vote?

I don't have the highest opinion of ComRel and I'm a member, but maybe
you could let us do our jobs?

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 3/3] app-admin/kube-bench: convert to go-module go.sum

Sam Jorna (wraeth)
On Saturday, 15 February 2020 3:14:55 AM AEDT Matt Turner wrote:
> On Fri, Feb 14, 2020 at 12:31 AM Sam Jorna (wraeth) <[hidden email]>
wrote:

> > In this instance, at least two people (myself included) have drawn an
> > impression that led them to voice their concern in some way (I'm unsure if
> > mpagano was voicing concern or just agreeing with the general concept).
> > Maybe we're the only ones. Maybe not.
>
> What do you think the threshold should be? If one person objects,
> should ComRel cease and desist? Two? Should we have a Gentoo-wide
> vote?
>
> I don't have the highest opinion of ComRel and I'm a member, but maybe
> you could let us do our jobs?
I didn't say ComRel shouldn't do their job, nor offered any opinion on ComRel
whatsoever. I said people shouldn't be called out on what looks like a
legitimate question, and that quote was illustrating one of the reasons why.
The threshold I'm talking about would be "is this question, however it's
worded, relevant to the subject."

Having said that, if this is you wearing a ComRel hat telling me to mind my
own business, so be it.

--
Sam Jorna (wraeth)
GnuPG ID: 0xD6180C26


signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/3] eclass/go-module: add support for building based on go.sum

Robin H. Johnson-2
In reply to this post by Michał Górny-5
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ł Górny wrote:
> > +# EGO_SUM=(
> > +# "github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ="
> > +# "github.com/BurntSushi/toml v0.3.1/go.mod h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ="
> 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+Yts87kKdq0PP7pXfy6kDkUVs=
github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=

1.2.1->1.2.2 there was no change in the dependencies, so the file didn't change.

> > -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.
>
> > +
> > +# @ECLASS-VARIABLE: EGO_SUM
> > +# @DESCRIPTION:
> > +# This variable duplicates the go.sum content from inside the target package.
> > +# Entries of the form <version>/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.

>
> > +#
> > +# <module> <version> <hash>
>
> Now I'm confused.  Unless my eyes betray me, PATCH 2 has entries without
> hash.
>
> 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
...
> 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://goproxy/ to
> > +# https://goproxy.cn/, or change this variable.
> > +# See https://arslan.io/2019/08/02/why-you-should-use-a-go-module-proxy/ for further details
> > +: "${GOMODULE_GOPROXY_BASEURI:=mirror://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 distfile back
> > +#   to the relative part of SRC_URI, as needed for GOPROXY=file:///...
> > +go-module_set_globals() {
> > + local line error_in_gosum errorlines errormsg exts
> > + local newline=$'\n'
> > + error_in_gosum=0
> > + errorlines=( )
> > + 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} != "h1:" ]] ; then
>
> Please use ${foo} everywhere consistently, and put && inside [[ ]].
> Also, I dare say wildcard match is more readable than hardcoding string
> length, i.e.:
>
>   [[ -n ${hash1} && ${hash1} != h1:* ]]
...

> > + # Split 'v0.3.0/go.mod' into 'v0.3.0' and '/go.mod'
> > + version=${version_modfile%%/*}
> > + modfile=${version_modfile#*/}
> > + [[ "$modfile" == "${version_modfile}" ]] && modfile=
> 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
...
> 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 inheriting
> > +# the eclass, you may need to update this variable.
> > +: "${GO_MODULE_GOSUM_PATH:=${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
...
> > + 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 phase
...
> Why not create and restore a copy?  Or does go-get make other changes?
The minimize is dropped entirely per upstream discussions.

--
Robin Hugh Johnson
Gentoo Linux: Dev, Infra Lead, Foundation Treasurer
E-Mail   : [hidden email]
GnuPG FP : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85
GnuPG FP : 7D0B3CEB E9B85B1F 825BCECF EE05E6F6 A48F6136

signature.asc (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 3/3] app-admin/kube-bench: convert to go-module go.sum

desultory
In reply to this post by Matt Turner-5
On 02/14/20 11:14, Matt Turner wrote:

> On Fri, Feb 14, 2020 at 12:31 AM Sam Jorna (wraeth) <[hidden email]> wrote:
>> In this instance, at least two people (myself included) have drawn an
>> impression that led them to voice their concern in some way (I'm unsure if
>> mpagano was voicing concern or just agreeing with the general concept). Maybe
>> we're the only ones. Maybe not.
>
> What do you think the threshold should be? If one person objects,
> should ComRel cease and desist? Two? Should we have a Gentoo-wide
> vote?
>
How many people objecting to your handling of a situation would it take
for you to consider that you might have handled it in a less than ideal
manner? Two? Three? Do we need unanimous declaration by all holders of
@gentoo.org e-mail addresses, including yourself, before you even
consider it?

> I don't have the highest opinion of ComRel and I'm a member, but maybe
> you could let us do our jobs?
>
>
Given that I am not your therapist, I am going to consider this comment
from an objective perspective not en emotional one. Given that you
"don't have the highest opinion of ComRel", that implies rather strongly
that you do not consider ComRel to be competent. Given that you are
still a member, that implies that either (1) you consider yourself to
not be the least competent member of ComRel (presumably of basic
competence), or (2) you are a member specifically to attempt to gain
such competence. In the former case, perhaps consider undertaking
training of those less competent than you (thereby improving your
opinion of ComRel as a whole), in the latter do kindly avoid undertaking
actions that you are not competent in.

As for the "maybe you could let us do our jobs?" part of that comment,
this appears to be a distinctly worrying trend among "special" projects
in Gentoo. Proctors now openly refuse to actually undertake their
mandate because they face the existential horror of negative feedback
when they make outlandishly perverse claims. ComRel now insists, by
implication, that while it is by the description of at least one member
openly incompetent, feedback is unwelcome at best. Even QA has made
similar sorts of empty appeals to their own authority, while also
refusing to actually argue their case. None of these should be at all
acceptable, yet somehow this nonsense is largely left uncontested for
reasons that escape me entirely, If you cannot adequately "do [y]our
job", consider that perhaps you should not be doing it in the first place.

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 3/3] app-admin/kube-bench: convert to go-module go.sum

Matt Turner-5
On Mon, Feb 17, 2020 at 11:47 PM desultory <[hidden email]> wrote:
>

You've got a particular knack for this kind of argumentative nonsense.

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 3/3] app-admin/kube-bench: convert to go-module go.sum

desultory
On 02/18/20 02:36, Matt Turner wrote:
> On Mon, Feb 17, 2020 at 11:47 PM desultory <[hidden email]> wrote:
>>
>
> You've got a particular knack for this kind of argumentative nonsense.
>
>
While I will gladly accept that post being described as "argumentative",
as after all I am very much interested in reading actual arguments
sufficient to convince me that my impressions and the opinions derived
therefrom are incorrect; I do take issue with it being described as
"nonsense".

So, I put to you the simple question: how, exactly, is it nonsense? I
extrapolated from your own activities and your own statements about your
own activities. If you don't like the impressions thus derived it might
do you well to address the sources of those impressions instead of
dismissing them as nonsense. Or, are you telling me that your own
statements are nonsense and your actions nonsensical and thus
impressions derived from them are nonsense? That in itself produces a
rather strong impression.

12