[gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in eclass: ChangeLog golang-build.eclass

[gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in eclass: ChangeLog golang-build.eclass

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 2249 bytes --]

Sorry for replying to the commit but it's different than last full
snippet on the list, so easier to review here.

Dnia 2015-06-24, o godz. 15:38:34
"William Hubbs (williamh)" <williamh@gentoo.org> napisał(a):

> Index: golang-build.eclass
> ===================================================================
> # Copyright 1999-2015 Gentoo Foundation
> # Distributed under the terms of the GNU General Public License v2
> # $Header: /var/cvsroot/gentoo-x86/eclass/golang-build.eclass,v 1.1 2015/06/24 15:38:33 williamh Exp $
> 
> # @ECLASS: golang-build.eclass
> # @MAINTAINER:
> # William Hubbs <williamh@gentoo.org>
> # @BLURB: Eclass for compiling go packages.
> # @DESCRIPTION:
> # This eclass provides default  src_compile, src_test and src_install
> # functions for software written in the Go programming language.
> 
> case "${EAPI:-0}" in
> 	5)
> 		;;
> 	*)
> 		die "${ECLASS}: Unsupported eapi (EAPI=${EAPI})"
> 		;;
> esac
> 
> EXPORT_FUNCTIONS src_compile src_install src_test
> 
> if [[ -z ${_GOLANG_BUILD} ]]; then
> 
> _GOLANG_BUILD=1
> 
> DEPEND=">=dev-lang/go-1.4.2:="
> STRIP_MASK="*.a"
> 
> # @ECLASS-VARIABLE: EGO_PN
> # @REQUIRED
> # @DESCRIPTION:
> # This is the import path for the go package(s) to build. Please emerge
> # dev-lang/go and read "go help importpath" for syntax.
> #
> # Example:
> # @CODE
> # EGO_PN=github.com/user/package
> # @CODE
> 
> # @FUNCTION: _golang-build_setup
> # @INTERNAL
> # @DESCRIPTION:
> # Make sure EGO_PN has a value.
> _golang-build_setup() {
> 	[ -z "${EGO_PN}" ] &&

Please don't use 'unsafe' single brackets. Use bash's double brackets,
i.e. [[ -z ${EGO_PN} ]].

> 		die "${ECLASS}.eclass: EGO_PN is not set"
> 	return 0
> }
> 
> golang-build_src_compile() {
> 	debug-print-function ${FUNCNAME} "$@"
> 
> 	_golang-build_setup
> 	set -- env GOPATH="${WORKDIR}/${P}:${EPREFIX}/usr/lib/go-gentoo" \
> 		go build -v -work -x "${EGO_PN}"
> 	echo "$@"

I suggest to push this to stderr, >&2.

> 	"$@" || die

And anyway, this looks to have a common, repeating part that sounds
like a good candidate for separate emake-like function... ego? :D

-- 
Best regards,
Michał Górny
<http://dev.gentoo.org/~mgorny/>

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 949 bytes --]

Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in eclass: ChangeLog golang-build.eclass

[William Hubbs]

[-- Attachment #1: Type: text/plain, Size: 1107 bytes --]

On Wed, Jun 24, 2015 at 05:54:31PM +0200, Michał Górny wrote:
> Dnia 2015-06-24, o godz. 15:38:34
> "William Hubbs (williamh)" <williamh@gentoo.org> napisał(a):

*snip*

> > # @FUNCTION: _golang-build_setup
> > # @INTERNAL
> > # @DESCRIPTION:
> > # Make sure EGO_PN has a value.
> > _golang-build_setup() {
> > 	[ -z "${EGO_PN}" ] &&
> 
> Please don't use 'unsafe' single brackets. Use bash's double brackets,
> i.e. [[ -z ${EGO_PN} ]].

Ok, this will be fixed.

> > golang-build_src_compile() {
> > 	debug-print-function ${FUNCNAME} "$@"
> > 
> > 	_golang-build_setup
> > 	set -- env GOPATH="${WORKDIR}/${P}:${EPREFIX}/usr/lib/go-gentoo" \
> > 		go build -v -work -x "${EGO_PN}"
> > 	echo "$@"
> 
> I suggest to push this to stderr, >&2.

Why? this is just printing the command before we run it. Do Make, etc
print this output to stderr?

> 
> > 	"$@" || die
> 
> And anyway, this looks to have a common, repeating part that sounds
> like a good candidate for separate emake-like function... ego? :D

Do you mean these lines:

echo "$@"
"$@" || die

William


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in eclass: ChangeLog golang-build.eclass

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 1643 bytes --]

Dnia 2015-06-24, o godz. 11:51:44
William Hubbs <williamh@gentoo.org> napisał(a):

> On Wed, Jun 24, 2015 at 05:54:31PM +0200, Michał Górny wrote:
> > Dnia 2015-06-24, o godz. 15:38:34
> > "William Hubbs (williamh)" <williamh@gentoo.org> napisał(a):
> 
> *snip*
> 
> > > # @FUNCTION: _golang-build_setup
> > > # @INTERNAL
> > > # @DESCRIPTION:
> > > # Make sure EGO_PN has a value.
> > > _golang-build_setup() {
> > > 	[ -z "${EGO_PN}" ] &&
> > 
> > Please don't use 'unsafe' single brackets. Use bash's double brackets,
> > i.e. [[ -z ${EGO_PN} ]].
> 
> Ok, this will be fixed.
> 
> > > golang-build_src_compile() {
> > > 	debug-print-function ${FUNCNAME} "$@"
> > > 
> > > 	_golang-build_setup
> > > 	set -- env GOPATH="${WORKDIR}/${P}:${EPREFIX}/usr/lib/go-gentoo" \
> > > 		go build -v -work -x "${EGO_PN}"
> > > 	echo "$@"
> > 
> > I suggest to push this to stderr, >&2.
> 
> Why? this is just printing the command before we run it. Do Make, etc
> print this output to stderr?

This is pretty much a debugging output which doesn't belong besides
regular output. Make doesn't, bash does.

> > > 	"$@" || die
> > 
> > And anyway, this looks to have a common, repeating part that sounds
> > like a good candidate for separate emake-like function... ego? :D
> 
> Do you mean these lines:
> 
> echo "$@"
> "$@" || die

Also the common settings for go.

i.e.:

set -- env GOPATH="${WORKDIR}/${P}:${EPREFIX}/usr/lib/go-gentoo" \
       go "${@}"
echo "${@}"
"${@}" || die...

Possibly even more arguments to go.

-- 
Best regards,
Michał Górny
<http://dev.gentoo.org/~mgorny/>

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 949 bytes --]

Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in eclass: ChangeLog golang-build.eclass

[William Hubbs]

[-- Attachment #1: Type: text/plain, Size: 2100 bytes --]

On Wed, Jun 24, 2015 at 07:02:49PM +0200, Michał Górny wrote:
> Dnia 2015-06-24, o godz. 11:51:44
> William Hubbs <williamh@gentoo.org> napisał(a):
> 
> > On Wed, Jun 24, 2015 at 05:54:31PM +0200, Michał Górny wrote:
> > > Dnia 2015-06-24, o godz. 15:38:34
> > > "William Hubbs (williamh)" <williamh@gentoo.org> napisał(a):
> > 
> > *snip*
> > 
> > > > # @FUNCTION: _golang-build_setup
> > > > # @INTERNAL
> > > > # @DESCRIPTION:
> > > > # Make sure EGO_PN has a value.
> > > > _golang-build_setup() {
> > > > 	[ -z "${EGO_PN}" ] &&
> > > 
> > > Please don't use 'unsafe' single brackets. Use bash's double brackets,
> > > i.e. [[ -z ${EGO_PN} ]].
> > 
> > Ok, this will be fixed.
> > 
> > > > golang-build_src_compile() {
> > > > 	debug-print-function ${FUNCNAME} "$@"
> > > > 
> > > > 	_golang-build_setup
> > > > 	set -- env GOPATH="${WORKDIR}/${P}:${EPREFIX}/usr/lib/go-gentoo" \
> > > > 		go build -v -work -x "${EGO_PN}"
> > > > 	echo "$@"
> > > 
> > > I suggest to push this to stderr, >&2.
> > 
> > Why? this is just printing the command before we run it. Do Make, etc
> > print this output to stderr?
> 
> This is pretty much a debugging output which doesn't belong besides
> regular output. Make doesn't, bash does.

My concern is, if the go tools themselves put some output on stdout and
some on stderr and I redirect everything to stderr, that could break
upstream behaviour, and I would rather not do that.

> 
> > > > 	"$@" || die
> > > 
> > > And anyway, this looks to have a common, repeating part that sounds
> > > like a good candidate for separate emake-like function... ego? :D
> > 
> > Do you mean these lines:
> > 
> > echo "$@"
> > "$@" || die
> 
> Also the common settings for go.
> 
> i.e.:
> 
> set -- env GOPATH="${WORKDIR}/${P}:${EPREFIX}/usr/lib/go-gentoo" \
>        go "${@}"
> echo "${@}"
> "${@}" || die...

I would have to think about that, because you are looking at different
subcommands (build, get, install), so I would need to see which flags
are really common to all of them.

William


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [gentoo-dev] Re: [gentoo-commits] gentoo-x86 commit in eclass: ChangeLog golang-build.eclass

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 1757 bytes --]

Dnia 2015-06-24, o godz. 16:04:39
William Hubbs <williamh@gentoo.org> napisał(a):

> On Wed, Jun 24, 2015 at 07:02:49PM +0200, Michał Górny wrote:
> > Dnia 2015-06-24, o godz. 11:51:44
> > William Hubbs <williamh@gentoo.org> napisał(a):
> > 
> > > On Wed, Jun 24, 2015 at 05:54:31PM +0200, Michał Górny wrote:
> > > > Dnia 2015-06-24, o godz. 15:38:34
> > > > "William Hubbs (williamh)" <williamh@gentoo.org> napisał(a):
> > > 
> > > *snip*
> > > 
> > > > > # @FUNCTION: _golang-build_setup
> > > > > # @INTERNAL
> > > > > # @DESCRIPTION:
> > > > > # Make sure EGO_PN has a value.
> > > > > _golang-build_setup() {
> > > > > 	[ -z "${EGO_PN}" ] &&
> > > > 
> > > > Please don't use 'unsafe' single brackets. Use bash's double brackets,
> > > > i.e. [[ -z ${EGO_PN} ]].
> > > 
> > > Ok, this will be fixed.
> > > 
> > > > > golang-build_src_compile() {
> > > > > 	debug-print-function ${FUNCNAME} "$@"
> > > > > 
> > > > > 	_golang-build_setup
> > > > > 	set -- env GOPATH="${WORKDIR}/${P}:${EPREFIX}/usr/lib/go-gentoo" \
> > > > > 		go build -v -work -x "${EGO_PN}"
> > > > > 	echo "$@"
> > > > 
> > > > I suggest to push this to stderr, >&2.
> > > 
> > > Why? this is just printing the command before we run it. Do Make, etc
> > > print this output to stderr?
> > 
> > This is pretty much a debugging output which doesn't belong besides
> > regular output. Make doesn't, bash does.
> 
> My concern is, if the go tools themselves put some output on stdout and
> some on stderr and I redirect everything to stderr, that could break
> upstream behaviour, and I would rather not do that.

I mean only the echo, not the command itself.

-- 
Best regards,
Michał Górny
<http://dev.gentoo.org/~mgorny/>

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 949 bytes --]