go-dep: disable PTEST_ENABLED
PATCH 1/1 go-dep: disable PTEST_ENABLED
From: Mingli Yu <[email protected]>
The run-ptest logic for go-dep actually runs the /usr/lib64/go-dep/ptest/github.com/golang/dep/cmd/dep/dep.test whose source file is https://github.com/golang/dep/blob/master/cmd/dep/dep_test.go.
That dep_test.go starts by rebuilding the dep program from source, then runs the tests using that copy of the program, so it's assuming that we're still in a development environment where we can run a full go build.
Considering it not being designed for a cross-build setup, so disable PTEST_ENABLED.
--- meta/recipes-devtools/go/go-dep_0.5.0.bb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
meta/recipes-devtools/go/go-dep_0.5.0.bb +3 -2
@@ -21,5 +21,6 @@ BBCLASSEXTEND = "native nativesdk" # For compiling ptest on mips and mips64, the current go-dep version fails with the go 1.11 toolchain. # error message: vet config not found -PTEST_ENABLED_mips = "0" -PTEST_ENABLED_mips64 = "0" +# disable PTEST_ENABLED as the run-ptest script for go-dep actually runs the /usr/lib64/go-dep/ptest/github.com/golang/dep/cmd/dep/dep.test whose source file is https://github.com/golang/dep/blob/master/cmd/dep/dep_test.go not being designed for a cross-build setup. +PTEST_ENABLED = "0" +PTEST_ENABLED = "0"
Signature · 1 line
2.21.0
Quoted 37 lines · On Fri, 2019-06-28 at 00:57 -0700, [email protected] wrote:
From: Mingli Yu <[email protected]>
The run-ptest logic for go-dep actually runs the
/usr/lib64/go-dep/ptest/github.com/golang/dep/cmd/dep/dep.testwhose source file is
https://github.com/golang/dep/blob/master/cmd/dep/dep_test.go.That dep_test.go starts by rebuilding the dep program from source, then runs the tests using that copy of the program, so it's assuming that we're still in a development environment where we can run a full go build.
Considering it not being designed for a cross-build setup, so disable
PTEST_ENABLED.Signed-off-by Mingli Yu <[email protected]>--- meta/recipes-devtools/go/go-dep_0.5.0.bb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)meta/recipes- renamed from meta/recipes-devtools/go/go-dep_0.5.0.bb +0
-0devtools/go/go-dep_0.5.0.bb
indexa4d631f8ea..e9fc12fa5a100644meta/recipes-devtools/go/go-dep_0.5.0.bb +0
-0@@ -21,5 +21,6 @@ BBCLASSEXTEND = "native nativesdk" # For compiling ptest on mips and mips64, the current go-dep versionfails with the go 1.11 toolchain.
# error message: vet config not foundmeta/recipes-devtools/go/go-dep_0.5.0.bb +1
-2-PTEST_ENABLED_mips = "0" -PTEST_ENABLED_mips64 = "0" +# disable PTEST_ENABLED as the run-ptest script for go-dep actuallyruns the
/usr/lib64/go-dep/ptest/github.com/golang/dep/cmd/dep/dep.test whose source file is
https://github.com/golang/dep/blob/master/cmd/dep/dep_test.go not being designed for a cross-build setup.
meta/recipes-devtools/go/go-dep_0.5.0.bb BBCLASSEXTEND = "native nativesdk" · line 26
Quoted 2 lines
meta/recipes-devtools/go/go-dep_0.5.0.bb +2
-0+PTEST_ENABLED = "0" +PTEST_ENABLED = "0"
Setting it twice looks wrong.
If we're disabling it, why would we inherit the ptest class at all as its not going to work anywhere?
Upstream not considering cross test usecases isn't a reason to disable a test, we have many tests enabled where upstream haven't considered a cross use case, we just tend to patch as needed and start a discussion with them.
It sounds like its actually a network access problem from the image you're running into anyway?
Cheers,
Richard
Quoted 28 lines · Richard Purdie
Quoted 27 lines · On Fri, 2019-06-28 at 00:57 -0700, [email protected] wrote:
From: Mingli Yu <[email protected]>
The run-ptest logic for go-dep actually runs the
/usr/lib64/go-dep/ptest/github.com/golang/dep/cmd/dep/dep.testwhose source file is
https://github.com/golang/dep/blob/master/cmd/dep/dep_test.go.That dep_test.go starts by rebuilding the dep program from source, then runs the tests using that copy of the program, so it's assuming that we're still in a development environment where we can run a full go build.
Considering it not being designed for a cross-build setup, so disable
PTEST_ENABLED.Signed-off-by Mingli Yu <[email protected]>--- meta/recipes-devtools/go/go-dep_0.5.0.bb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)meta/recipes- renamed from meta/recipes-devtools/go/go-dep_0.5.0.bb +0
-0devtools/go/go-dep_0.5.0.bb
indexa4d631f8ea..e9fc12fa5a100644meta/recipes-devtools/go/go-dep_0.5.0.bb +0
-0@@ -21,5 +21,6 @@ BBCLASSEXTEND = "native nativesdk"
Quoted 14 lines
meta/recipes-devtools/go/go-dep_0.5.0.bb BBCLASSEXTEND = "native nativesdk" · line 26
Quoted 12 lines
meta/recipes-devtools/go/go-dep_0.5.0.bb +0
-0# For compiling ptest on mips and mips64, the current go-dep versionfails with the go 1.11 toolchain.
# error message: vet config not foundmeta/recipes-devtools/go/go-dep_0.5.0.bb +1
-2-PTEST_ENABLED_mips = "0" -PTEST_ENABLED_mips64 = "0" +# disable PTEST_ENABLED as the run-ptest script for go-dep actuallyruns the
/usr/lib64/go-dep/ptest/github.com/golang/dep/cmd/dep/dep.test whose source file is
https://github.com/golang/dep/blob/master/cmd/dep/dep_test.go not being designed for a cross-build setup.meta/recipes-devtools/go/go-dep_0.5.0.bb +2
-0+PTEST_ENABLED = "0" +PTEST_ENABLED = "0"Setting it twice looks wrong.
Sorry, it should be my typo.
Quoted 11 lines
If we're disabling it, why would we inherit the ptest class at all as its not going to work anywhere?
Upstream not considering cross test usecases isn't a reason to disable a test, we have many tests enabled where upstream haven't considered a cross use case, we just tend to patch as needed and start a discussion with them.
It sounds like its actually a network access problem from the image you're running into anyway?
Hi RP,
Have discussed the ptest more with Matt in the maillist and also tried to add the patch under the guide from Matt to make the https://github.com/golang/dep/blob/master/cmd/dep/dep_test.go work with cross-setup env. But seems it still doesn't work.
Hi Matt,
What's your opinion?
Thanks,
Quoted 7 lines
Cheers,
Richard
Quoted 63 lines · Mingli
Quoted 28 lines · Richard Purdie
Quoted 27 lines · On Fri, 2019-06-28 at 00:57 -0700, [email protected] wrote:
From: Mingli Yu <[email protected]>
The run-ptest logic for go-dep actually runs the
/usr/lib64/go-dep/ptest/github.com/golang/dep/cmd/dep/dep.testwhose source file is
https://github.com/golang/dep/blob/master/cmd/dep/dep_test.go.That dep_test.go starts by rebuilding the dep program from source, then runs the tests using that copy of the program, so it's assuming that we're still in a development environment where we can run a full go build.
Considering it not being designed for a cross-build setup, so disable
PTEST_ENABLED.Signed-off-by Mingli Yu <[email protected]>--- meta/recipes-devtools/go/go-dep_0.5.0.bb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)meta/recipes- renamed from meta/recipes-devtools/go/go-dep_0.5.0.bb +0
-0devtools/go/go-dep_0.5.0.bb
indexa4d631f8ea..e9fc12fa5a100644meta/recipes-devtools/go/go-dep_0.5.0.bb +0
-0@@ -21,5 +21,6 @@ BBCLASSEXTEND = "native nativesdk"Quoted 14 lines
meta/recipes-devtools/go/go-dep_0.5.0.bb BBCLASSEXTEND = "native nativesdk" · line 26
Quoted 12 lines
meta/recipes-devtools/go/go-dep_0.5.0.bb +0
-0# For compiling ptest on mips and mips64, the current go-dep versionfails with the go 1.11 toolchain.
# error message: vet config not foundmeta/recipes-devtools/go/go-dep_0.5.0.bb +1
-2-PTEST_ENABLED_mips = "0" -PTEST_ENABLED_mips64 = "0" +# disable PTEST_ENABLED as the run-ptest script for go-dep actuallyruns the
/usr/lib64/go-dep/ptest/github.com/golang/dep/cmd/dep/dep.test whose source file is
https://github.com/golang/dep/blob/master/cmd/dep/dep_test.go not being designed for a cross-build setup.meta/recipes-devtools/go/go-dep_0.5.0.bb +2
-0+PTEST_ENABLED = "0" +PTEST_ENABLED = "0"Setting it twice looks wrong.
Sorry, it should be my typo.
Quoted 11 lines
If we're disabling it, why would we inherit the ptest class at all as its not going to work anywhere?
Upstream not considering cross test usecases isn't a reason to disable a test, we have many tests enabled where upstream haven't considered a cross use case, we just tend to patch as needed and start a discussion with them.
It sounds like its actually a network access problem from the image you're running into anyway?
Hi RP,
Quoted 7 lines
Have discussed the ptest more with Matt in the maillist and also tried to add the patch under the guide from Matt to make the
https://github.com/golang/dep/blob/master/cmd/dep/dep_test.go work with cross-setup env. But seems it still doesn't work.Hi Matt,
What's your opinion?
I was able to get the test to work in my setup, with the addition of that patch and manually setting GOCACHE in my environment. As Richard mentioned, from the failures you were getting, it looked like a network access issue in your setup. I'm not sure why GOCACHE needs to be set, when 'go env' shows the correct (non-'off') default; that might be another issue in dep that needs to be addressed.
It would be good to have at least one non-trivial ptest for a go package, and since OE-Core doesn't have much beyond the toolchain itself, go-dep, and glide, getting this to work would provide some much-needed coverage. Running go-dep or glide - both build tools - on a target seems like a fairly rare use case, though.
As for Richard's initial question, go.bbclass inherits ptest to enable the mapping of go's built-in test builds automatically. In hindsight, maybe this wasn't the best choice, and it was overly optimistic to think that because go supports separate compilation and execution of tests, that packages would generally support that by default.
-Matt
Quoted 9 lines
Thanks,
Quoted 7 lines
Cheers,
Richard
Quoted 5 lines · Matt Madison
As for Richard's initial question, go.bbclass inherits ptest to enable the mapping of go's built-in test builds automatically. In hindsight, maybe this wasn't the best choice, and it was overly optimistic to think that because go supports separate compilation and execution of tests, that packages would generally support that by default.
Maybe the ptest pieces just need to be split out to a separate class go-ptest.bbclass, so that recipes where it *does* work can simply inherit that too.
Ross
Quoted 69 lines · Matt Madison
Quoted 68 lines · Mingli
Quoted 28 lines · Richard Purdie
Quoted 27 lines · On Fri, 2019-06-28 at 00:57 -0700, [email protected] wrote:
From: Mingli Yu <[email protected]>
The run-ptest logic for go-dep actually runs the
/usr/lib64/go-dep/ptest/github.com/golang/dep/cmd/dep/dep.testwhose source file is
https://github.com/golang/dep/blob/master/cmd/dep/dep_test.go.That dep_test.go starts by rebuilding the dep program from source, then runs the tests using that copy of the program, so it's assuming that we're still in a development environment where we can run a full go build.
Considering it not being designed for a cross-build setup, so disable
PTEST_ENABLED.Signed-off-by Mingli Yu <[email protected]>--- meta/recipes-devtools/go/go-dep_0.5.0.bb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)meta/recipes- renamed from meta/recipes-devtools/go/go-dep_0.5.0.bb +0
-0devtools/go/go-dep_0.5.0.bb
indexa4d631f8ea..e9fc12fa5a100644meta/recipes-devtools/go/go-dep_0.5.0.bb +0
-0@@ -21,5 +21,6 @@ BBCLASSEXTEND = "native nativesdk"Quoted 14 lines
meta/recipes-devtools/go/go-dep_0.5.0.bb BBCLASSEXTEND = "native nativesdk" · line 26
Quoted 12 lines
meta/recipes-devtools/go/go-dep_0.5.0.bb +0
-0# For compiling ptest on mips and mips64, the current go-dep versionfails with the go 1.11 toolchain.
# error message: vet config not foundmeta/recipes-devtools/go/go-dep_0.5.0.bb +1
-2-PTEST_ENABLED_mips = "0" -PTEST_ENABLED_mips64 = "0" +# disable PTEST_ENABLED as the run-ptest script for go-dep actuallyruns the
/usr/lib64/go-dep/ptest/github.com/golang/dep/cmd/dep/dep.test whose source file is
https://github.com/golang/dep/blob/master/cmd/dep/dep_test.go not being designed for a cross-build setup.meta/recipes-devtools/go/go-dep_0.5.0.bb +2
-0+PTEST_ENABLED = "0" +PTEST_ENABLED = "0"Setting it twice looks wrong.
Sorry, it should be my typo.
Quoted 11 lines
If we're disabling it, why would we inherit the ptest class at all as its not going to work anywhere?
Upstream not considering cross test usecases isn't a reason to disable a test, we have many tests enabled where upstream haven't considered a cross use case, we just tend to patch as needed and start a discussion with them.
It sounds like its actually a network access problem from the image you're running into anyway?
Hi RP,
Have discussed the ptest more with Matt in the maillist and also tried to add the patch under the guide from Matt to make the
https://github.com/golang/dep/blob/master/cmd/dep/dep_test.go work with cross-setup env. But seems it still doesn't work.
Quoted 5 lines
Quoted 2 lines
Hi Matt,
What's your opinion?I was able to get the test to work in my setup, with the addition of that patch and manually setting GOCACHE in my environment. As Richard
Thanks Matt's information!
If so, could you help to contribute your worked patch to oe-core?
I still got all cases failed even with the patch which to make https://github.com/golang/dep/blob/master/cmd/dep/dep_test.go correspondingly usable for cross-setup.
Quoted 10 lines
mentioned, from the failures you were getting, it looked like a network access issue in your setup. I'm not sure why GOCACHE needs to be set, when 'go env' shows the correct (non-'off') default; that might be another issue in dep that needs to be addressed.
It would be good to have at least one non-trivial ptest for a go package, and since OE-Core doesn't have much beyond the toolchain itself, go-dep, and glide, getting this to work would provide some much-needed coverage. Running go-dep or glide - both build tools - on a target seems like a fairly rare use case, though.
BTW, I noticed even go doesn't have ptest support.
Quoted 6 lines
As for Richard's initial question, go.bbclass inherits ptest to enable the mapping of go's built-in test builds automatically. In hindsight, maybe this wasn't the best choice, and it was overly optimistic to think that because go supports separate compilation and execution of tests, that packages would generally support that by default.
How about to remove inherits ptest logic in go.bbclass and let the inherit ptest logic in each recipe if someone cares about go-dep ptest.
Thanks,
Quoted 13 lines
-Matt
Quoted 9 lines
Thanks,
Quoted 7 lines
Cheers,
Richard
PATCH 1/1 go.bbclass: remove ptest inherit
In reply to Matt MadisonFrom: Mingli Yu <[email protected]>
Remove the ptest bbclass inherit logic to let each recipe inherit ptest individually if the specified recipe wants to support ptest to avoid the recipe which inherits go bblcass inheriting ptest automatically.
--- meta/classes/go.bbclass | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
meta/classes/go.bbclass +1 -1
@@ -1,4 +1,4 @@ -inherit goarch ptest +inherit goarch GO_PARALLEL_BUILD ?= "${@oe.utils.parallel_make_argument(d, '-p %d')}"
Signature · 1 line
2.21.0
Re: [PATCH] go.bbclass: remove ptest inherit
Quoted 8 lines · On Wed, Jul 3, 2019 at 2:25 AM <[email protected]> wrote:
From: Mingli Yu <[email protected]>
Remove the ptest bbclass inherit logic to let each recipe inherit ptest individually if the specified recipe wants to support ptest to avoid the recipe which inherits go bblcass inheriting ptest automatically.
Recipes will have to inherit ptest before inheriting go, which seems counter-intuitive; with ptest after go, the compile_ptest and install_ptest functions will be the null ones from ptest.bbclass. Ross's suggestion of breaking out the ptest-related functions from go.bbclass into a separate bbclass would be better. Perhaps called ptest-go.bbclass, similar to the ones already there for gnome and perl.
Thanks,
-Matt
Quoted 18 lines
Signed-off-by Mingli Yu <[email protected]>--- meta/classes/go.bbclass | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)meta/classes/go.bbclass +1
-2@@ -1,4 +1,4 @@ -inherit goarch ptest +inherit goarch GO_PARALLEL_BUILD ?= "${@oe.utils.parallel_make_argument(d, '-p %d')}" --2.21.0
PATCH v3 1/1 go.bbclass: separate the ptest logic to go-ptest class
From: Mingli Yu <[email protected]>
The current go class includes ptest logic by default and will make the recipe which inherits go class to support ptest automatically though maybe the recipe which inherits go class doesn't plan to support the ptest.
So separate the ptest logic to another specified
class go-ptest to make the recipe which needs to
inherit go class more flexible with regards to
ptest support.
--- meta/classes/go-ptest.bbclass | 54 +++++++++++++++++++++++++++++++++++ meta/classes/go.bbclass | 52 +-------------------------------- 2 files changed, 55 insertions(+), 51 deletions(-) create mode 100644 meta/classes/go-ptest.bbclass
meta/classes/go-ptest.bbclass new file +54 -0
@@ -0,0 +1,54 @@ +inherit go ptest + +do_compile_ptest_base() { + export TMPDIR="${GOTMPDIR}" + rm -f ${B}/.go_compiled_tests.list + go_list_package_tests | while read pkg; do + cd ${B}/src/$pkg + ${GO} test ${GOPTESTBUILDFLAGS} $pkg + find . -mindepth 1 -maxdepth 1 -type f -name '*.test' -exec echo $pkg/{} \; | \ + sed -e's,/\./,/,'>> ${B}/.go_compiled_tests.list + done + do_compile_ptest +} + +do_compile_ptest_base[dirs] =+ "${GOTMPDIR}" + +go_make_ptest_wrapper() { + cat >${D}${PTEST_PATH}/run-ptest <<EOF +#!/bin/sh +RC=0 +run_test() ( + cd "\$1" + ((((./\$2 ${GOPTESTFLAGS}; echo \$? >&3) | sed -r -e"s,^(PASS|SKIP|FAIL)\$,\\1: \$1/\$2," >&4) 3>&1) | (read rc; exit \$rc)) 4>&1 + exit \$?) +EOF + +} + +do_install_ptest_base() { + test -f "${B}/.go_compiled_tests.list" || exit 0 + install -d ${D}${PTEST_PATH} + go_stage_testdata + go_make_ptest_wrapper + havetests="" + while read test; do + testdir=`dirname $test` + testprog=`basename $test` + install -d ${D}${PTEST_PATH}/$testdir + install -m 0755 ${B}/src/$test ${D}${PTEST_PATH}/$test + echo "run_test $testdir $testprog || RC=1" >> ${D}${PTEST_PATH}/run-ptest + havetests="yes" + done < ${B}/.go_compiled_tests.list + if [ -n "$havetests" ]; then + echo "exit \$RC" >> ${D}${PTEST_PATH}/run-ptest + chmod +x ${D}${PTEST_PATH}/run-ptest + else + rm -rf ${D}${PTEST_PATH} + fi + do_install_ptest + chown -R root:root ${D}${PTEST_PATH} +} + +INSANE_SKIP_${PN}-ptest += "ldflags" +
meta/classes/go.bbclass +1 -51
79 lines not on this page · raw message
Signature · 1 line
2.21.0