All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] [RFC] go.bbclass: install dependencies under $GOPATH instead of $GOROOT
@ 2018-12-19 17:50 Damien Riegel
  2018-12-19 21:40 ` Matt Madison
  0 siblings, 1 reply; 7+ messages in thread
From: Damien Riegel @ 2018-12-19 17:50 UTC (permalink / raw)
  To: openembedded-core

To package a go application in Yocto, one needs to also package its
dependencies to keep the build reproducible and under control. The
default install task of the go class puts the source in ${libdir}/go/src.

${libdir}/go/src is where the standard go packages resides, aka $GOROOT.
When a go package has dependencies on other go packages, they must be
located under $GOPATH, otherwise errors like this one may happen:

  Non-standard package <packagename> in standard package <packagename>

This point of this patch is to trigger a discussion on how this issue
can be tackled in Yocto. Following code is working but should not be
considered for inclusion.

Signed-off-by: Damien Riegel <damien.riegel@gmail.com>
---
 meta/classes/go.bbclass | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/meta/classes/go.bbclass b/meta/classes/go.bbclass
index 167d02e3fa..c0ed3211cd 100644
--- a/meta/classes/go.bbclass
+++ b/meta/classes/go.bbclass
@@ -111,9 +111,9 @@ do_compile_ptest_base() {
 do_compile_ptest_base[dirs] =+ "${GOTMPDIR}"
 
 go_do_install() {
-	install -d ${D}${libdir}/go/src/${GO_IMPORT}
+	install -d ${D}/gopath/src/${GO_IMPORT} ${D}${libdir}/go
 	tar -C ${S}/src/${GO_IMPORT} -cf - --exclude-vcs --exclude '*.test' --exclude 'testdata' . | \
-		tar -C ${D}${libdir}/go/src/${GO_IMPORT} --no-same-owner -xf -
+		tar -C ${D}/gopath/src/${GO_IMPORT} --no-same-owner -xf -
 	tar -C ${B} -cf - pkg | tar -C ${D}${libdir}/go --no-same-owner -xf -
 
 	if [ -n "`ls ${B}/${GO_BUILD_BINDIR}/`" ]; then
@@ -172,9 +172,21 @@ do_install_ptest_base() {
 	chown -R root:root ${D}${PTEST_PATH}
 }
 
+do_fixupdeps() {
+	gopath="${WORKDIR}/recipe-sysroot/gopath"
+	if [ -d ${gopath} ]; then
+		tar -C ${gopath} -cf - src | tar -C ${B} --no-same-owner -k -xf -
+	else
+		echo "no dependencies to fixup"
+	fi
+}
+
 EXPORT_FUNCTIONS do_unpack do_configure do_compile do_install
 
-FILES_${PN}-dev = "${libdir}/go/src"
+addtask do_fixupdeps after do_configure before do_compile
+
+SYSROOT_DIRS_append = "/gopath"
+FILES_${PN}-dev = "/gopath/src"
 FILES_${PN}-staticdev = "${libdir}/go/pkg"
 
 INSANE_SKIP_${PN} += "ldflags"
-- 
2.19.2



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] [RFC] go.bbclass: install dependencies under $GOPATH instead of $GOROOT
  2018-12-19 17:50 [PATCH] [RFC] go.bbclass: install dependencies under $GOPATH instead of $GOROOT Damien Riegel
@ 2018-12-19 21:40 ` Matt Madison
  2018-12-20  1:52   ` Damien Riegel
  0 siblings, 1 reply; 7+ messages in thread
From: Matt Madison @ 2018-12-19 21:40 UTC (permalink / raw)
  To: Damien Riegel; +Cc: Patches and discussions about the oe-core layer

On Wed, Dec 19, 2018 at 9:51 AM Damien Riegel <damien.riegel@gmail.com> wrote:
>
> To package a go application in Yocto, one needs to also package its
> dependencies to keep the build reproducible and under control. The
> default install task of the go class puts the source in ${libdir}/go/src.
>
> ${libdir}/go/src is where the standard go packages resides, aka $GOROOT.
> When a go package has dependencies on other go packages, they must be
> located under $GOPATH, otherwise errors like this one may happen:
>
>   Non-standard package <packagename> in standard package <packagename>
>
> This point of this patch is to trigger a discussion on how this issue
> can be tackled in Yocto. Following code is working but should not be
> considered for inclusion.

Can you describe the context of this a bit more?  The way go.bbclass
is set up, all of your recipe's dependencies (and their dependencies)
should already have been installed in the per-recipe sysroot (where
GOROOT lives for the current recipe's build), and should get resolved
from there. GOPATH gets set to ${B} for the current recipe's build.
Nothing in the GOROOT should be importing packages that reside in
GOPATH, which is what that particular error message is indicating. If
you're getting that error, then there might be something wrong with
recipe dependencies.

-Matt
>
> Signed-off-by: Damien Riegel <damien.riegel@gmail.com>
> ---
>  meta/classes/go.bbclass | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/meta/classes/go.bbclass b/meta/classes/go.bbclass
> index 167d02e3fa..c0ed3211cd 100644
> --- a/meta/classes/go.bbclass
> +++ b/meta/classes/go.bbclass
> @@ -111,9 +111,9 @@ do_compile_ptest_base() {
>  do_compile_ptest_base[dirs] =+ "${GOTMPDIR}"
>
>  go_do_install() {
> -       install -d ${D}${libdir}/go/src/${GO_IMPORT}
> +       install -d ${D}/gopath/src/${GO_IMPORT} ${D}${libdir}/go
>         tar -C ${S}/src/${GO_IMPORT} -cf - --exclude-vcs --exclude '*.test' --exclude 'testdata' . | \
> -               tar -C ${D}${libdir}/go/src/${GO_IMPORT} --no-same-owner -xf -
> +               tar -C ${D}/gopath/src/${GO_IMPORT} --no-same-owner -xf -
>         tar -C ${B} -cf - pkg | tar -C ${D}${libdir}/go --no-same-owner -xf -
>
>         if [ -n "`ls ${B}/${GO_BUILD_BINDIR}/`" ]; then
> @@ -172,9 +172,21 @@ do_install_ptest_base() {
>         chown -R root:root ${D}${PTEST_PATH}
>  }
>
> +do_fixupdeps() {
> +       gopath="${WORKDIR}/recipe-sysroot/gopath"
> +       if [ -d ${gopath} ]; then
> +               tar -C ${gopath} -cf - src | tar -C ${B} --no-same-owner -k -xf -
> +       else
> +               echo "no dependencies to fixup"
> +       fi
> +}
> +
>  EXPORT_FUNCTIONS do_unpack do_configure do_compile do_install
>
> -FILES_${PN}-dev = "${libdir}/go/src"
> +addtask do_fixupdeps after do_configure before do_compile
> +
> +SYSROOT_DIRS_append = "/gopath"
> +FILES_${PN}-dev = "/gopath/src"
>  FILES_${PN}-staticdev = "${libdir}/go/pkg"
>
>  INSANE_SKIP_${PN} += "ldflags"
> --
> 2.19.2
>
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] [RFC] go.bbclass: install dependencies under $GOPATH instead of $GOROOT
  2018-12-19 21:40 ` Matt Madison
@ 2018-12-20  1:52   ` Damien Riegel
  2018-12-20 12:53     ` Matt Madison
  0 siblings, 1 reply; 7+ messages in thread
From: Damien Riegel @ 2018-12-20  1:52 UTC (permalink / raw)
  To: Matt Madison; +Cc: Patches and discussions about the oe-core layer

On Wed, 19 Dec 2018 at 16:41, Matt Madison <matt@madison.systems> wrote:
>
> On Wed, Dec 19, 2018 at 9:51 AM Damien Riegel <damien.riegel@gmail.com> wrote:
> >
> > To package a go application in Yocto, one needs to also package its
> > dependencies to keep the build reproducible and under control. The
> > default install task of the go class puts the source in ${libdir}/go/src.
> >
> > ${libdir}/go/src is where the standard go packages resides, aka $GOROOT.
> > When a go package has dependencies on other go packages, they must be
> > located under $GOPATH, otherwise errors like this one may happen:
> >
> >   Non-standard package <packagename> in standard package <packagename>
> >
> > This point of this patch is to trigger a discussion on how this issue
> > can be tackled in Yocto. Following code is working but should not be
> > considered for inclusion.
>
> Can you describe the context of this a bit more?  The way go.bbclass
> is set up, all of your recipe's dependencies (and their dependencies)
> should already have been installed in the per-recipe sysroot (where
> GOROOT lives for the current recipe's build), and should get resolved
> from there. GOPATH gets set to ${B} for the current recipe's build.
> Nothing in the GOROOT should be importing packages that reside in
> GOPATH, which is what that particular error message is indicating. If
> you're getting that error, then there might be something wrong with
> recipe dependencies.

Sure. I think the issue is because I have the following chain of
dependencies:

A --> B --> C
  `-> C

Building C is fine, building B is fine, but building A triggers the
error I mentioned, because the Go compiler complains about B using a
non-standard package C. It thinks B is a standard package because it's
located in GOROOT.

Regarding dependencies, I created recipes named foocorp.com-{A,B,C}.bb
and used DEPENDS_append = " foocorp.com-A" for instance.

I understand it's convenient to install dependencies in the GOROOT
directory of recipes' sysroot, but we should keep in mind that when one
calls `go get`, dependencies are installed in GOPATH, not GOROOT. I
think that if we want to avoid (a bit obscure) build failures and cover
all use cases, we should try to mimic that behaviour as close as we can.

>
> -Matt
> >
> > Signed-off-by: Damien Riegel <damien.riegel@gmail.com>
> > ---
> >  meta/classes/go.bbclass | 18 +++++++++++++++---
> >  1 file changed, 15 insertions(+), 3 deletions(-)
> >
> > diff --git a/meta/classes/go.bbclass b/meta/classes/go.bbclass
> > index 167d02e3fa..c0ed3211cd 100644
> > --- a/meta/classes/go.bbclass
> > +++ b/meta/classes/go.bbclass
> > @@ -111,9 +111,9 @@ do_compile_ptest_base() {
> >  do_compile_ptest_base[dirs] =+ "${GOTMPDIR}"
> >
> >  go_do_install() {
> > -       install -d ${D}${libdir}/go/src/${GO_IMPORT}
> > +       install -d ${D}/gopath/src/${GO_IMPORT} ${D}${libdir}/go
> >         tar -C ${S}/src/${GO_IMPORT} -cf - --exclude-vcs --exclude '*.test' --exclude 'testdata' . | \
> > -               tar -C ${D}${libdir}/go/src/${GO_IMPORT} --no-same-owner -xf -
> > +               tar -C ${D}/gopath/src/${GO_IMPORT} --no-same-owner -xf -
> >         tar -C ${B} -cf - pkg | tar -C ${D}${libdir}/go --no-same-owner -xf -
> >
> >         if [ -n "`ls ${B}/${GO_BUILD_BINDIR}/`" ]; then
> > @@ -172,9 +172,21 @@ do_install_ptest_base() {
> >         chown -R root:root ${D}${PTEST_PATH}
> >  }
> >
> > +do_fixupdeps() {
> > +       gopath="${WORKDIR}/recipe-sysroot/gopath"
> > +       if [ -d ${gopath} ]; then
> > +               tar -C ${gopath} -cf - src | tar -C ${B} --no-same-owner -k -xf -
> > +       else
> > +               echo "no dependencies to fixup"
> > +       fi
> > +}
> > +
> >  EXPORT_FUNCTIONS do_unpack do_configure do_compile do_install
> >
> > -FILES_${PN}-dev = "${libdir}/go/src"
> > +addtask do_fixupdeps after do_configure before do_compile
> > +
> > +SYSROOT_DIRS_append = "/gopath"
> > +FILES_${PN}-dev = "/gopath/src"
> >  FILES_${PN}-staticdev = "${libdir}/go/pkg"
> >
> >  INSANE_SKIP_${PN} += "ldflags"
> > --
> > 2.19.2
> >
> > --
> > _______________________________________________
> > Openembedded-core mailing list
> > Openembedded-core@lists.openembedded.org
> > http://lists.openembedded.org/mailman/listinfo/openembedded-core


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] [RFC] go.bbclass: install dependencies under $GOPATH instead of $GOROOT
  2018-12-20  1:52   ` Damien Riegel
@ 2018-12-20 12:53     ` Matt Madison
  2018-12-20 14:02       ` Damien Riegel
  0 siblings, 1 reply; 7+ messages in thread
From: Matt Madison @ 2018-12-20 12:53 UTC (permalink / raw)
  To: Damien Riegel; +Cc: Patches and discussions about the oe-core layer

On Wed, Dec 19, 2018 at 5:52 PM Damien Riegel <damien.riegel@gmail.com> wrote:
>
> On Wed, 19 Dec 2018 at 16:41, Matt Madison <matt@madison.systems> wrote:
> >
> > On Wed, Dec 19, 2018 at 9:51 AM Damien Riegel <damien.riegel@gmail.com> wrote:
> > >
> > > To package a go application in Yocto, one needs to also package its
> > > dependencies to keep the build reproducible and under control. The
> > > default install task of the go class puts the source in ${libdir}/go/src.
> > >
> > > ${libdir}/go/src is where the standard go packages resides, aka $GOROOT.
> > > When a go package has dependencies on other go packages, they must be
> > > located under $GOPATH, otherwise errors like this one may happen:
> > >
> > >   Non-standard package <packagename> in standard package <packagename>
> > >
> > > This point of this patch is to trigger a discussion on how this issue
> > > can be tackled in Yocto. Following code is working but should not be
> > > considered for inclusion.
> >
> > Can you describe the context of this a bit more?  The way go.bbclass
> > is set up, all of your recipe's dependencies (and their dependencies)
> > should already have been installed in the per-recipe sysroot (where
> > GOROOT lives for the current recipe's build), and should get resolved
> > from there. GOPATH gets set to ${B} for the current recipe's build.
> > Nothing in the GOROOT should be importing packages that reside in
> > GOPATH, which is what that particular error message is indicating. If
> > you're getting that error, then there might be something wrong with
> > recipe dependencies.
>
> Sure. I think the issue is because I have the following chain of
> dependencies:
>
> A --> B --> C
>   `-> C
>
> Building C is fine, building B is fine, but building A triggers the
> error I mentioned, because the Go compiler complains about B using a
> non-standard package C. It thinks B is a standard package because it's
> located in GOROOT.

Are the three packages in separate source repositories? If so, the go
tool should not be finding any C files in GOPATH.

If A and C are part of the same source repository, but B is separate,
that would make the situation a bit trickier, but still solvable.

> Regarding dependencies, I created recipes named foocorp.com-{A,B,C}.bb
> and used DEPENDS_append = " foocorp.com-A" for instance.

If A imports from B and C, you should have DEPENDS = "foocorp.com-B
foocorp.com-C" in the recipe for A, and
DEPENDS = "foocorp.com-C" in the recipe for B.

> I understand it's convenient to install dependencies in the GOROOT
> directory of recipes' sysroot, but we should keep in mind that when one
> calls `go get`, dependencies are installed in GOPATH, not GOROOT. I
> think that if we want to avoid (a bit obscure) build failures and cover
> all use cases, we should try to mimic that behaviour as close as we can.

go.bbclass does not use `go get`.  It only uses `go install`.

-Matt

>
> >
> > -Matt
> > >
> > > Signed-off-by: Damien Riegel <damien.riegel@gmail.com>
> > > ---
> > >  meta/classes/go.bbclass | 18 +++++++++++++++---
> > >  1 file changed, 15 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/meta/classes/go.bbclass b/meta/classes/go.bbclass
> > > index 167d02e3fa..c0ed3211cd 100644
> > > --- a/meta/classes/go.bbclass
> > > +++ b/meta/classes/go.bbclass
> > > @@ -111,9 +111,9 @@ do_compile_ptest_base() {
> > >  do_compile_ptest_base[dirs] =+ "${GOTMPDIR}"
> > >
> > >  go_do_install() {
> > > -       install -d ${D}${libdir}/go/src/${GO_IMPORT}
> > > +       install -d ${D}/gopath/src/${GO_IMPORT} ${D}${libdir}/go
> > >         tar -C ${S}/src/${GO_IMPORT} -cf - --exclude-vcs --exclude '*.test' --exclude 'testdata' . | \
> > > -               tar -C ${D}${libdir}/go/src/${GO_IMPORT} --no-same-owner -xf -
> > > +               tar -C ${D}/gopath/src/${GO_IMPORT} --no-same-owner -xf -
> > >         tar -C ${B} -cf - pkg | tar -C ${D}${libdir}/go --no-same-owner -xf -
> > >
> > >         if [ -n "`ls ${B}/${GO_BUILD_BINDIR}/`" ]; then
> > > @@ -172,9 +172,21 @@ do_install_ptest_base() {
> > >         chown -R root:root ${D}${PTEST_PATH}
> > >  }
> > >
> > > +do_fixupdeps() {
> > > +       gopath="${WORKDIR}/recipe-sysroot/gopath"
> > > +       if [ -d ${gopath} ]; then
> > > +               tar -C ${gopath} -cf - src | tar -C ${B} --no-same-owner -k -xf -
> > > +       else
> > > +               echo "no dependencies to fixup"
> > > +       fi
> > > +}
> > > +
> > >  EXPORT_FUNCTIONS do_unpack do_configure do_compile do_install
> > >
> > > -FILES_${PN}-dev = "${libdir}/go/src"
> > > +addtask do_fixupdeps after do_configure before do_compile
> > > +
> > > +SYSROOT_DIRS_append = "/gopath"
> > > +FILES_${PN}-dev = "/gopath/src"
> > >  FILES_${PN}-staticdev = "${libdir}/go/pkg"
> > >
> > >  INSANE_SKIP_${PN} += "ldflags"
> > > --
> > > 2.19.2
> > >
> > > --
> > > _______________________________________________
> > > Openembedded-core mailing list
> > > Openembedded-core@lists.openembedded.org
> > > http://lists.openembedded.org/mailman/listinfo/openembedded-core


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] [RFC] go.bbclass: install dependencies under $GOPATH instead of $GOROOT
  2018-12-20 12:53     ` Matt Madison
@ 2018-12-20 14:02       ` Damien Riegel
  2018-12-20 16:00         ` Matt Madison
  0 siblings, 1 reply; 7+ messages in thread
From: Damien Riegel @ 2018-12-20 14:02 UTC (permalink / raw)
  To: Matt Madison; +Cc: Patches and discussions about the oe-core layer

On Thu, 20 Dec 2018 at 07:53, Matt Madison <matt@madison.systems> wrote:
>
> On Wed, Dec 19, 2018 at 5:52 PM Damien Riegel <damien.riegel@gmail.com> wrote:
> >
> > On Wed, 19 Dec 2018 at 16:41, Matt Madison <matt@madison.systems> wrote:
> > >
> > > On Wed, Dec 19, 2018 at 9:51 AM Damien Riegel <damien.riegel@gmail.com> wrote:
> > > >
> > > > To package a go application in Yocto, one needs to also package its
> > > > dependencies to keep the build reproducible and under control. The
> > > > default install task of the go class puts the source in ${libdir}/go/src.
> > > >
> > > > ${libdir}/go/src is where the standard go packages resides, aka $GOROOT.
> > > > When a go package has dependencies on other go packages, they must be
> > > > located under $GOPATH, otherwise errors like this one may happen:
> > > >
> > > >   Non-standard package <packagename> in standard package <packagename>
> > > >
> > > > This point of this patch is to trigger a discussion on how this issue
> > > > can be tackled in Yocto. Following code is working but should not be
> > > > considered for inclusion.
> > >
> > > Can you describe the context of this a bit more?  The way go.bbclass
> > > is set up, all of your recipe's dependencies (and their dependencies)
> > > should already have been installed in the per-recipe sysroot (where
> > > GOROOT lives for the current recipe's build), and should get resolved
> > > from there. GOPATH gets set to ${B} for the current recipe's build.
> > > Nothing in the GOROOT should be importing packages that reside in
> > > GOPATH, which is what that particular error message is indicating. If
> > > you're getting that error, then there might be something wrong with
> > > recipe dependencies.
> >
> > Sure. I think the issue is because I have the following chain of
> > dependencies:
> >
> > A --> B --> C
> >   `-> C
> >
> > Building C is fine, building B is fine, but building A triggers the
> > error I mentioned, because the Go compiler complains about B using a
> > non-standard package C. It thinks B is a standard package because it's
> > located in GOROOT.
>
> Are the three packages in separate source repositories? If so, the go
> tool should not be finding any C files in GOPATH.
>
> If A and C are part of the same source repository, but B is separate,
> that would make the situation a bit trickier, but still solvable.

They are three separate repositories. The issue is precisely that C
files are not in GOPATH but in GOROOT. C files _should be_ in GOPATH or
that will break Go tools.

> > Regarding dependencies, I created recipes named foocorp.com-{A,B,C}.bb
> > and used DEPENDS_append = " foocorp.com-A" for instance.
>
> If A imports from B and C, you should have DEPENDS = "foocorp.com-B
> foocorp.com-C" in the recipe for A, and
> DEPENDS = "foocorp.com-C" in the recipe for B.

Okay, that part I got right then!

> > I understand it's convenient to install dependencies in the GOROOT
> > directory of recipes' sysroot, but we should keep in mind that when one
> > calls `go get`, dependencies are installed in GOPATH, not GOROOT. I
> > think that if we want to avoid (a bit obscure) build failures and cover
> > all use cases, we should try to mimic that behaviour as close as we can.
>
> go.bbclass does not use `go get`.  It only uses `go install`.

Sorry, that was not clear, I was talking about Go development outside of
Yocto. So outside of Yocto, I would use `go fetch` to get the
dependencies and they would end up somewhere in GOPATH.  When packaging
these dependencies in Yocto, they will end up in the GOROOT of recipes'
sysroots, and that breaks builds.

My point was that if we want to be able to write recipe for any valid Go
package, dependencies should be found under GOPATH, not GOROOT.

> -Matt
>
> >
> > >
> > > -Matt
> > > >
> > > > Signed-off-by: Damien Riegel <damien.riegel@gmail.com>
> > > > ---
> > > >  meta/classes/go.bbclass | 18 +++++++++++++++---
> > > >  1 file changed, 15 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/meta/classes/go.bbclass b/meta/classes/go.bbclass
> > > > index 167d02e3fa..c0ed3211cd 100644
> > > > --- a/meta/classes/go.bbclass
> > > > +++ b/meta/classes/go.bbclass
> > > > @@ -111,9 +111,9 @@ do_compile_ptest_base() {
> > > >  do_compile_ptest_base[dirs] =+ "${GOTMPDIR}"
> > > >
> > > >  go_do_install() {
> > > > -       install -d ${D}${libdir}/go/src/${GO_IMPORT}
> > > > +       install -d ${D}/gopath/src/${GO_IMPORT} ${D}${libdir}/go
> > > >         tar -C ${S}/src/${GO_IMPORT} -cf - --exclude-vcs --exclude '*.test' --exclude 'testdata' . | \
> > > > -               tar -C ${D}${libdir}/go/src/${GO_IMPORT} --no-same-owner -xf -
> > > > +               tar -C ${D}/gopath/src/${GO_IMPORT} --no-same-owner -xf -
> > > >         tar -C ${B} -cf - pkg | tar -C ${D}${libdir}/go --no-same-owner -xf -
> > > >
> > > >         if [ -n "`ls ${B}/${GO_BUILD_BINDIR}/`" ]; then
> > > > @@ -172,9 +172,21 @@ do_install_ptest_base() {
> > > >         chown -R root:root ${D}${PTEST_PATH}
> > > >  }
> > > >
> > > > +do_fixupdeps() {
> > > > +       gopath="${WORKDIR}/recipe-sysroot/gopath"
> > > > +       if [ -d ${gopath} ]; then
> > > > +               tar -C ${gopath} -cf - src | tar -C ${B} --no-same-owner -k -xf -
> > > > +       else
> > > > +               echo "no dependencies to fixup"
> > > > +       fi
> > > > +}
> > > > +
> > > >  EXPORT_FUNCTIONS do_unpack do_configure do_compile do_install
> > > >
> > > > -FILES_${PN}-dev = "${libdir}/go/src"
> > > > +addtask do_fixupdeps after do_configure before do_compile
> > > > +
> > > > +SYSROOT_DIRS_append = "/gopath"
> > > > +FILES_${PN}-dev = "/gopath/src"
> > > >  FILES_${PN}-staticdev = "${libdir}/go/pkg"
> > > >
> > > >  INSANE_SKIP_${PN} += "ldflags"
> > > > --
> > > > 2.19.2
> > > >
> > > > --
> > > > _______________________________________________
> > > > Openembedded-core mailing list
> > > > Openembedded-core@lists.openembedded.org
> > > > http://lists.openembedded.org/mailman/listinfo/openembedded-core


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] [RFC] go.bbclass: install dependencies under $GOPATH instead of $GOROOT
  2018-12-20 14:02       ` Damien Riegel
@ 2018-12-20 16:00         ` Matt Madison
  2018-12-20 19:17           ` Damien Riegel
  0 siblings, 1 reply; 7+ messages in thread
From: Matt Madison @ 2018-12-20 16:00 UTC (permalink / raw)
  To: Damien Riegel; +Cc: Patches and discussions about the oe-core layer

On Thu, Dec 20, 2018 at 6:02 AM Damien Riegel <damien.riegel@gmail.com> wrote:
>
> On Thu, 20 Dec 2018 at 07:53, Matt Madison <matt@madison.systems> wrote:
> >
> > On Wed, Dec 19, 2018 at 5:52 PM Damien Riegel <damien.riegel@gmail.com> wrote:
> > >
> > > On Wed, 19 Dec 2018 at 16:41, Matt Madison <matt@madison.systems> wrote:
> > > >
> > > > On Wed, Dec 19, 2018 at 9:51 AM Damien Riegel <damien.riegel@gmail.com> wrote:
> > > > >
> > > > > To package a go application in Yocto, one needs to also package its
> > > > > dependencies to keep the build reproducible and under control. The
> > > > > default install task of the go class puts the source in ${libdir}/go/src.
> > > > >
> > > > > ${libdir}/go/src is where the standard go packages resides, aka $GOROOT.
> > > > > When a go package has dependencies on other go packages, they must be
> > > > > located under $GOPATH, otherwise errors like this one may happen:
> > > > >
> > > > >   Non-standard package <packagename> in standard package <packagename>
> > > > >
> > > > > This point of this patch is to trigger a discussion on how this issue
> > > > > can be tackled in Yocto. Following code is working but should not be
> > > > > considered for inclusion.
> > > >
> > > > Can you describe the context of this a bit more?  The way go.bbclass
> > > > is set up, all of your recipe's dependencies (and their dependencies)
> > > > should already have been installed in the per-recipe sysroot (where
> > > > GOROOT lives for the current recipe's build), and should get resolved
> > > > from there. GOPATH gets set to ${B} for the current recipe's build.
> > > > Nothing in the GOROOT should be importing packages that reside in
> > > > GOPATH, which is what that particular error message is indicating. If
> > > > you're getting that error, then there might be something wrong with
> > > > recipe dependencies.
> > >
> > > Sure. I think the issue is because I have the following chain of
> > > dependencies:
> > >
> > > A --> B --> C
> > >   `-> C
> > >
> > > Building C is fine, building B is fine, but building A triggers the
> > > error I mentioned, because the Go compiler complains about B using a
> > > non-standard package C. It thinks B is a standard package because it's
> > > located in GOROOT.
> >
> > Are the three packages in separate source repositories? If so, the go
> > tool should not be finding any C files in GOPATH.
> >
> > If A and C are part of the same source repository, but B is separate,
> > that would make the situation a bit trickier, but still solvable.
>
> They are three separate repositories. The issue is precisely that C
> files are not in GOPATH but in GOROOT. C files _should be_ in GOPATH or
> that will break Go tools.

Go does allow for non-"standard" packages to be in GOROOT as well.
The way the go tool marks a package as "standard" is if it resides in
GOROOT _and_ starts with a prefix that does not look like a domain
name (specifically, does not contain a dot '.').  The Go convention,
though, is to use a domain name as the first element of the import
path for anything that isn't part of the base runtime (std, cmd).

If you do have a situation where you have multiple interdependent
packages that for some reason have import paths that do not start with
a domain name, you could build them all using a single recipe, rather
than splitting them up into separate recipes.  I've had to do that for
cases where there were circular dependencies between two (or more)
packages; the same technique could work this, though, too.

-Matt

> > > Regarding dependencies, I created recipes named foocorp.com-{A,B,C}.bb
> > > and used DEPENDS_append = " foocorp.com-A" for instance.
> >
> > If A imports from B and C, you should have DEPENDS = "foocorp.com-B
> > foocorp.com-C" in the recipe for A, and
> > DEPENDS = "foocorp.com-C" in the recipe for B.
>
> Okay, that part I got right then!
>
> > > I understand it's convenient to install dependencies in the GOROOT
> > > directory of recipes' sysroot, but we should keep in mind that when one
> > > calls `go get`, dependencies are installed in GOPATH, not GOROOT. I
> > > think that if we want to avoid (a bit obscure) build failures and cover
> > > all use cases, we should try to mimic that behaviour as close as we can.
> >
> > go.bbclass does not use `go get`.  It only uses `go install`.
>
> Sorry, that was not clear, I was talking about Go development outside of
> Yocto. So outside of Yocto, I would use `go fetch` to get the
> dependencies and they would end up somewhere in GOPATH.  When packaging
> these dependencies in Yocto, they will end up in the GOROOT of recipes'
> sysroots, and that breaks builds.
>
> My point was that if we want to be able to write recipe for any valid Go
> package, dependencies should be found under GOPATH, not GOROOT.
>
> > -Matt
> >
> > >
> > > >
> > > > -Matt
> > > > >
> > > > > Signed-off-by: Damien Riegel <damien.riegel@gmail.com>
> > > > > ---
> > > > >  meta/classes/go.bbclass | 18 +++++++++++++++---
> > > > >  1 file changed, 15 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/meta/classes/go.bbclass b/meta/classes/go.bbclass
> > > > > index 167d02e3fa..c0ed3211cd 100644
> > > > > --- a/meta/classes/go.bbclass
> > > > > +++ b/meta/classes/go.bbclass
> > > > > @@ -111,9 +111,9 @@ do_compile_ptest_base() {
> > > > >  do_compile_ptest_base[dirs] =+ "${GOTMPDIR}"
> > > > >
> > > > >  go_do_install() {
> > > > > -       install -d ${D}${libdir}/go/src/${GO_IMPORT}
> > > > > +       install -d ${D}/gopath/src/${GO_IMPORT} ${D}${libdir}/go
> > > > >         tar -C ${S}/src/${GO_IMPORT} -cf - --exclude-vcs --exclude '*.test' --exclude 'testdata' . | \
> > > > > -               tar -C ${D}${libdir}/go/src/${GO_IMPORT} --no-same-owner -xf -
> > > > > +               tar -C ${D}/gopath/src/${GO_IMPORT} --no-same-owner -xf -
> > > > >         tar -C ${B} -cf - pkg | tar -C ${D}${libdir}/go --no-same-owner -xf -
> > > > >
> > > > >         if [ -n "`ls ${B}/${GO_BUILD_BINDIR}/`" ]; then
> > > > > @@ -172,9 +172,21 @@ do_install_ptest_base() {
> > > > >         chown -R root:root ${D}${PTEST_PATH}
> > > > >  }
> > > > >
> > > > > +do_fixupdeps() {
> > > > > +       gopath="${WORKDIR}/recipe-sysroot/gopath"
> > > > > +       if [ -d ${gopath} ]; then
> > > > > +               tar -C ${gopath} -cf - src | tar -C ${B} --no-same-owner -k -xf -
> > > > > +       else
> > > > > +               echo "no dependencies to fixup"
> > > > > +       fi
> > > > > +}
> > > > > +
> > > > >  EXPORT_FUNCTIONS do_unpack do_configure do_compile do_install
> > > > >
> > > > > -FILES_${PN}-dev = "${libdir}/go/src"
> > > > > +addtask do_fixupdeps after do_configure before do_compile
> > > > > +
> > > > > +SYSROOT_DIRS_append = "/gopath"
> > > > > +FILES_${PN}-dev = "/gopath/src"
> > > > >  FILES_${PN}-staticdev = "${libdir}/go/pkg"
> > > > >
> > > > >  INSANE_SKIP_${PN} += "ldflags"
> > > > > --
> > > > > 2.19.2
> > > > >
> > > > > --
> > > > > _______________________________________________
> > > > > Openembedded-core mailing list
> > > > > Openembedded-core@lists.openembedded.org
> > > > > http://lists.openembedded.org/mailman/listinfo/openembedded-core


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] [RFC] go.bbclass: install dependencies under $GOPATH instead of $GOROOT
  2018-12-20 16:00         ` Matt Madison
@ 2018-12-20 19:17           ` Damien Riegel
  0 siblings, 0 replies; 7+ messages in thread
From: Damien Riegel @ 2018-12-20 19:17 UTC (permalink / raw)
  To: Matt Madison; +Cc: Patches and discussions about the oe-core layer

On Thu, 20 Dec 2018 at 11:00, Matt Madison <matt@madison.systems> wrote:
>
> On Thu, Dec 20, 2018 at 6:02 AM Damien Riegel <damien.riegel@gmail.com> wrote:
> >
> > On Thu, 20 Dec 2018 at 07:53, Matt Madison <matt@madison.systems> wrote:
> > >
> > > On Wed, Dec 19, 2018 at 5:52 PM Damien Riegel <damien.riegel@gmail.com> wrote:
> > > >
> > > > On Wed, 19 Dec 2018 at 16:41, Matt Madison <matt@madison.systems> wrote:
> > > > >
> > > > > On Wed, Dec 19, 2018 at 9:51 AM Damien Riegel <damien.riegel@gmail.com> wrote:
> > > > > >
> > > > > > To package a go application in Yocto, one needs to also package its
> > > > > > dependencies to keep the build reproducible and under control. The
> > > > > > default install task of the go class puts the source in ${libdir}/go/src.
> > > > > >
> > > > > > ${libdir}/go/src is where the standard go packages resides, aka $GOROOT.
> > > > > > When a go package has dependencies on other go packages, they must be
> > > > > > located under $GOPATH, otherwise errors like this one may happen:
> > > > > >
> > > > > >   Non-standard package <packagename> in standard package <packagename>
> > > > > >
> > > > > > This point of this patch is to trigger a discussion on how this issue
> > > > > > can be tackled in Yocto. Following code is working but should not be
> > > > > > considered for inclusion.
> > > > >
> > > > > Can you describe the context of this a bit more?  The way go.bbclass
> > > > > is set up, all of your recipe's dependencies (and their dependencies)
> > > > > should already have been installed in the per-recipe sysroot (where
> > > > > GOROOT lives for the current recipe's build), and should get resolved
> > > > > from there. GOPATH gets set to ${B} for the current recipe's build.
> > > > > Nothing in the GOROOT should be importing packages that reside in
> > > > > GOPATH, which is what that particular error message is indicating. If
> > > > > you're getting that error, then there might be something wrong with
> > > > > recipe dependencies.
> > > >
> > > > Sure. I think the issue is because I have the following chain of
> > > > dependencies:
> > > >
> > > > A --> B --> C
> > > >   `-> C
> > > >
> > > > Building C is fine, building B is fine, but building A triggers the
> > > > error I mentioned, because the Go compiler complains about B using a
> > > > non-standard package C. It thinks B is a standard package because it's
> > > > located in GOROOT.
> > >
> > > Are the three packages in separate source repositories? If so, the go
> > > tool should not be finding any C files in GOPATH.
> > >
> > > If A and C are part of the same source repository, but B is separate,
> > > that would make the situation a bit trickier, but still solvable.
> >
> > They are three separate repositories. The issue is precisely that C
> > files are not in GOPATH but in GOROOT. C files _should be_ in GOPATH or
> > that will break Go tools.
>
> Go does allow for non-"standard" packages to be in GOROOT as well.
> The way the go tool marks a package as "standard" is if it resides in
> GOROOT _and_ starts with a prefix that does not look like a domain
> name (specifically, does not contain a dot '.').  The Go convention,
> though, is to use a domain name as the first element of the import
> path for anything that isn't part of the base runtime (std, cmd).

Damn, I think that's the issue, some packages don't use a domain name
as the first element of the import path.

> If you do have a situation where you have multiple interdependent
> packages that for some reason have import paths that do not start with
> a domain name, you could build them all using a single recipe, rather
> than splitting them up into separate recipes.  I've had to do that for
> cases where there were circular dependencies between two (or more)
> packages; the same technique could work this, though, too.

Okay, got it. There are no circular dependencies here, it's really a
straightforward situation "A needs B", so fixing the package names
should solve the issue.

On a side note, it would be nice to have documentation somewhere on
how to use the go class, especially for GO_IMPORT and GO_INSTALL.
Does that belong in the class file itself? Or should that be put in the doc
repository?


Thank your very much for your help!
 --
Damien


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-12-20 19:17 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-19 17:50 [PATCH] [RFC] go.bbclass: install dependencies under $GOPATH instead of $GOROOT Damien Riegel
2018-12-19 21:40 ` Matt Madison
2018-12-20  1:52   ` Damien Riegel
2018-12-20 12:53     ` Matt Madison
2018-12-20 14:02       ` Damien Riegel
2018-12-20 16:00         ` Matt Madison
2018-12-20 19:17           ` Damien Riegel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.