| Message ID | c38afab85d9fc005edade229896008a4ad5a1929.1588282027.git.rosbrookn@ainfosec.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | initialize xenlight go module | expand |
> On Apr 30, 2020, at 10:39 PM, Nick Rosbrook <rosbrookn@gmail.com> wrote: > > Initialize the xenlight Go module using the xenbits git-http URL, > xenbits.xen.org/git-http/xen.git/tools/golang/xenlight, and update the > XEN_GOCODE_URL variable in tools/Rules.mk accordingly. > > Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com> > --- > tools/Rules.mk | 2 +- > tools/golang/xenlight/go.mod | 1 + > 2 files changed, 2 insertions(+), 1 deletion(-) > create mode 100644 tools/golang/xenlight/go.mod > > diff --git a/tools/Rules.mk b/tools/Rules.mk > index 5b8cf748ad..ca33cc7b31 100644 > --- a/tools/Rules.mk > +++ b/tools/Rules.mk > @@ -36,7 +36,7 @@ debug ?= y > debug_symbols ?= $(debug) > > XEN_GOPATH = $(XEN_ROOT)/tools/golang > -XEN_GOCODE_URL = golang.xenproject.org > +XEN_GOCODE_URL = xenbits.xen.org/git-http/xen.git/tools/golang The primary effect of this will be to install the code in $PREFIX/share/gocode/xenbits.xen.org/git-http/xen.git/tools/golang/xenlight when making debballs or doing `make install`. I don’t immediately see the advantage of that, particularly if we’re still thinking about having a “prettier” path at some point in the future. What was your thinking here? > ifeq ($(debug_symbols),y) > CFLAGS += -g3 > diff --git a/tools/golang/xenlight/go.mod b/tools/golang/xenlight/go.mod > new file mode 100644 > index 0000000000..232d102153 > --- /dev/null > +++ b/tools/golang/xenlight/go.mod > @@ -0,0 +1 @@ > +module xenbits.xen.org/git-http/xen.git/tools/golang/xenlight This should probably be s/xen/xenproject/; If you want I could check in a version of this patch with just the go.mod, with that change. -George
On Tue, May 12, 2020 at 10:36 AM George Dunlap <George.Dunlap@citrix.com> wrote: > > > > > On Apr 30, 2020, at 10:39 PM, Nick Rosbrook <rosbrookn@gmail.com> wrote: > > > > Initialize the xenlight Go module using the xenbits git-http URL, > > xenbits.xen.org/git-http/xen.git/tools/golang/xenlight, and update the > > XEN_GOCODE_URL variable in tools/Rules.mk accordingly. > > > > Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com> > > --- > > tools/Rules.mk | 2 +- > > tools/golang/xenlight/go.mod | 1 + > > 2 files changed, 2 insertions(+), 1 deletion(-) > > create mode 100644 tools/golang/xenlight/go.mod > > > > diff --git a/tools/Rules.mk b/tools/Rules.mk > > index 5b8cf748ad..ca33cc7b31 100644 > > --- a/tools/Rules.mk > > +++ b/tools/Rules.mk > > @@ -36,7 +36,7 @@ debug ?= y > > debug_symbols ?= $(debug) > > > > XEN_GOPATH = $(XEN_ROOT)/tools/golang > > -XEN_GOCODE_URL = golang.xenproject.org > > +XEN_GOCODE_URL = xenbits.xen.org/git-http/xen.git/tools/golang > > The primary effect of this will be to install the code in $PREFIX/share/gocode/xenbits.xen.org/git-http/xen.git/tools/golang/xenlight when making debballs or doing `make install`. > > I don’t immediately see the advantage of that, particularly if we’re still thinking about having a “prettier” path at some point in the future. What was your thinking here? With the module being defined as `xenbits.xen.org/...`, the `build` Make target will fail as-is for a module-aware version of go (because it cannot find a module named `golang.xenproject.org/xenlight`). So, the reason for this change is to preserve the existing functionality of that Make target. Changing XEN_GOCODE_URL seemed like the correct change, but I'm open to suggestions. > > > ifeq ($(debug_symbols),y) > > CFLAGS += -g3 > > diff --git a/tools/golang/xenlight/go.mod b/tools/golang/xenlight/go.mod > > new file mode 100644 > > index 0000000000..232d102153 > > --- /dev/null > > +++ b/tools/golang/xenlight/go.mod > > @@ -0,0 +1 @@ > > +module xenbits.xen.org/git-http/xen.git/tools/golang/xenlight > > This should probably be s/xen/xenproject/; AFAICT, that's the correct URL, e.g. [1] and [2]. Am I missing something? Thanks, -NR [1] https://pkg.go.dev/mod/xenbits.xen.org/git-http/xen.git [2] https://xenbits.xen.org/gitweb/?p=xen.git;a=summary
> On May 12, 2020, at 4:06 PM, Nick Rosbrook <rosbrookn@gmail.com> wrote: > > [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments unless you have verified the sender and know the content is safe. > > On Tue, May 12, 2020 at 10:36 AM George Dunlap <George.Dunlap@citrix.com> wrote: >> >> >> >>> On Apr 30, 2020, at 10:39 PM, Nick Rosbrook <rosbrookn@gmail.com> wrote: >>> >>> +module xenbits.xen.org/git-http/xen.git/tools/golang/xenlight >> >> This should probably be s/xen/xenproject/; > > AFAICT, that's the correct URL, e.g. [1] and [2]. Am I missing something? For trademark reasons, when we joined the Linux Foundation, we did a big rebranding from “Xen” to “XenProject”. (See [1] for more details.) The xen.org domains are just for backwards compatibility. :-) -George [1] https://xenproject.org/2013/04/17/upcoming-changes-to-the-xen-websites/
> For trademark reasons, when we joined the Linux Foundation, we did a big rebranding from “Xen” to “XenProject”. (See [1] for more details.) The xen.org domains are just for backwards compatibility. :-)
Ah, thanks! I'll fix that.
-NR
> On May 12, 2020, at 4:06 PM, Nick Rosbrook <rosbrookn@gmail.com> wrote: > > [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments unless you have verified the sender and know the content is safe. > > On Tue, May 12, 2020 at 10:36 AM George Dunlap <George.Dunlap@citrix.com> wrote: >> >> >> >>> On Apr 30, 2020, at 10:39 PM, Nick Rosbrook <rosbrookn@gmail.com> wrote: >>> >>> Initialize the xenlight Go module using the xenbits git-http URL, >>> xenbits.xen.org/git-http/xen.git/tools/golang/xenlight, and update the >>> XEN_GOCODE_URL variable in tools/Rules.mk accordingly. >>> >>> Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com> >>> --- >>> tools/Rules.mk | 2 +- >>> tools/golang/xenlight/go.mod | 1 + >>> 2 files changed, 2 insertions(+), 1 deletion(-) >>> create mode 100644 tools/golang/xenlight/go.mod >>> >>> diff --git a/tools/Rules.mk b/tools/Rules.mk >>> index 5b8cf748ad..ca33cc7b31 100644 >>> --- a/tools/Rules.mk >>> +++ b/tools/Rules.mk >>> @@ -36,7 +36,7 @@ debug ?= y >>> debug_symbols ?= $(debug) >>> >>> XEN_GOPATH = $(XEN_ROOT)/tools/golang >>> -XEN_GOCODE_URL = golang.xenproject.org >>> +XEN_GOCODE_URL = xenbits.xen.org/git-http/xen.git/tools/golang >> >> The primary effect of this will be to install the code in $PREFIX/share/gocode/xenbits.xen.org/git-http/xen.git/tools/golang/xenlight when making debballs or doing `make install`. >> >> I don’t immediately see the advantage of that, particularly if we’re still thinking about having a “prettier” path at some point in the future. What was your thinking here? > > With the module being defined as `xenbits.xen.org/...`, the `build` > Make target will fail as-is for a module-aware version of go (because > it cannot find a module named `golang.xenproject.org/xenlight`). So, > the reason for this change is to preserve the existing functionality > of that Make target. Changing XEN_GOCODE_URL seemed like the correct > change, but I'm open to suggestions. Oh. But no, that’s not at all what we want. The whole point of running ‘go build’ is to make sure that *the code we just copied* — the code right now in our own local tree, perhaps which was just generated — actually compiles. It looks like when we add the `go.mod` further up the tree, it makes `go build` ignore the GOPATH environment variable we’re giving it, which causes the build failure. But your “fix” doesn’t make it use the in-tree go code again; instead it looks like it causes `go build` command to go and fetch the most recent `master` version from xenbits, ignoring the go code in the tree completely. :-) -George
> On May 12, 2020, at 6:30 PM, George Dunlap <george.dunlap@citrix.com> wrote: > >> >> On May 12, 2020, at 4:06 PM, Nick Rosbrook <rosbrookn@gmail.com> wrote: >> >> [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments unless you have verified the sender and know the content is safe. >> >> On Tue, May 12, 2020 at 10:36 AM George Dunlap <George.Dunlap@citrix.com> wrote: >>> >>> >>> >>>> On Apr 30, 2020, at 10:39 PM, Nick Rosbrook <rosbrookn@gmail.com> wrote: >>>> >>>> Initialize the xenlight Go module using the xenbits git-http URL, >>>> xenbits.xen.org/git-http/xen.git/tools/golang/xenlight, and update the >>>> XEN_GOCODE_URL variable in tools/Rules.mk accordingly. >>>> >>>> Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com> >>>> --- >>>> tools/Rules.mk | 2 +- >>>> tools/golang/xenlight/go.mod | 1 + >>>> 2 files changed, 2 insertions(+), 1 deletion(-) >>>> create mode 100644 tools/golang/xenlight/go.mod >>>> >>>> diff --git a/tools/Rules.mk b/tools/Rules.mk >>>> index 5b8cf748ad..ca33cc7b31 100644 >>>> --- a/tools/Rules.mk >>>> +++ b/tools/Rules.mk >>>> @@ -36,7 +36,7 @@ debug ?= y >>>> debug_symbols ?= $(debug) >>>> >>>> XEN_GOPATH = $(XEN_ROOT)/tools/golang >>>> -XEN_GOCODE_URL = golang.xenproject.org >>>> +XEN_GOCODE_URL = xenbits.xen.org/git-http/xen.git/tools/golang >>> >>> The primary effect of this will be to install the code in $PREFIX/share/gocode/xenbits.xen.org/git-http/xen.git/tools/golang/xenlight when making debballs or doing `make install`. >>> >>> I don’t immediately see the advantage of that, particularly if we’re still thinking about having a “prettier” path at some point in the future. What was your thinking here? >> >> With the module being defined as `xenbits.xen.org/...`, the `build` >> Make target will fail as-is for a module-aware version of go (because >> it cannot find a module named `golang.xenproject.org/xenlight`). So, >> the reason for this change is to preserve the existing functionality >> of that Make target. Changing XEN_GOCODE_URL seemed like the correct >> change, but I'm open to suggestions. > > Oh. But no, that’s not at all what we want. > > The whole point of running ‘go build’ is to make sure that *the code we just copied* — the code right now in our own local tree, perhaps which was just generated — actually compiles. > > It looks like when we add the `go.mod` further up the tree, it makes `go build` ignore the GOPATH environment variable we’re giving it, which causes the build failure. But your “fix” doesn’t make it use the in-tree go code again; instead it looks like it causes `go build` command to go and fetch the most recent `master` version from xenbits, ignoring the go code in the tree completely. :-) OK, so actually what you want to do is replace the `go install -x $OTHER_PLACE_WE_JUST_COPIED_THE_FILES` with `go build -x` in the Makefile. -George
> The whole point of running ‘go build’ is to make sure that *the code we just copied* — the code right now in our own local tree, perhaps which was just generated — actually compiles. I understand that, and in my tests that's the outcome. > It looks like when we add the `go.mod` further up the tree, it makes `go build` ignore the GOPATH environment variable we’re giving it, which causes the build failure. But your “fix” doesn’t make it use the in-tree go code again; instead it looks like it causes `go build` command to go and fetch the most recent `master` version from xenbits, ignoring the go code in the tree completely. :-) The GOPATH is "ignored" because it is obsolete in module-aware go (this is one of the primary motivations of modules). The build fails (without changing XEN_GOCODE_URL) because xenproject.golang.org is *not* the module in the local tree, and the subsequent fetch fails. However, when the correct import path (after updating XEN_GOCODE_URL) is used, go is smart enough to realize we're trying to build our local module and will not do a fetch. However, I'm more than happy to just use `go build` instead of `go install` in that make target. Thanks -NR
diff --git a/tools/Rules.mk b/tools/Rules.mk index 5b8cf748ad..ca33cc7b31 100644 --- a/tools/Rules.mk +++ b/tools/Rules.mk @@ -36,7 +36,7 @@ debug ?= y debug_symbols ?= $(debug) XEN_GOPATH = $(XEN_ROOT)/tools/golang -XEN_GOCODE_URL = golang.xenproject.org +XEN_GOCODE_URL = xenbits.xen.org/git-http/xen.git/tools/golang ifeq ($(debug_symbols),y) CFLAGS += -g3 diff --git a/tools/golang/xenlight/go.mod b/tools/golang/xenlight/go.mod new file mode 100644 index 0000000000..232d102153 --- /dev/null +++ b/tools/golang/xenlight/go.mod @@ -0,0 +1 @@ +module xenbits.xen.org/git-http/xen.git/tools/golang/xenlight
Initialize the xenlight Go module using the xenbits git-http URL, xenbits.xen.org/git-http/xen.git/tools/golang/xenlight, and update the XEN_GOCODE_URL variable in tools/Rules.mk accordingly. Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com> --- tools/Rules.mk | 2 +- tools/golang/xenlight/go.mod | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 tools/golang/xenlight/go.mod