[PATCH RFC] tools/xenlight: Create xenlight Makefile

[PATCH RFC] tools/xenlight: Create xenlight Makefile

[Ronald Rojas]

Create a basic Makefile to build and install libxenlight Golang
bindings.  Also add a template.

---

Eventually this patch will contain the actual bindings package; for
now it just includes a stub package.

To Do:
- Have configure detect golang bindings properly

CC: xen-devel <xen-devel@lists.xen.org>
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: George Dunlap <george.dunlap@citrix.com>
CC: George Dunlap <dunlapg@umich.edu>
---
 tools/Makefile                    | 16 ++++++++++++++++
 tools/golang/Makefile             | 33 +++++++++++++++++++++++++++++++++
 tools/golang/xenlight/xenlight.go |  7 +++++++
 3 files changed, 56 insertions(+)
 create mode 100644 tools/golang/Makefile
 create mode 100644 tools/golang/xenlight/xenlight.go

diff --git a/tools/Makefile b/tools/Makefile
index 71515b4..f2198e0 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -11,6 +11,8 @@ SUBDIRS-y += misc
 SUBDIRS-y += examples
 SUBDIRS-y += hotplug
 SUBDIRS-y += xentrace
+SUBDIRS-y += golang
+#FIXME: Have golang controlled by a configure variable
 SUBDIRS-$(CONFIG_XCUTILS) += xcutils
 SUBDIRS-$(CONFIG_X86) += firmware
 SUBDIRS-y += console
@@ -311,6 +313,20 @@ subdir-install-debugger/gdbsx: .phony
 subdir-all-debugger/gdbsx: .phony
 	$(MAKE) -C debugger/gdbsx all
 
+subdir-all-golang/xenlight: .phony
+	$(MAKE) -C golang all
+
+subdir-clean-golang/xenlight: .phony
+	$(MAKE) -C golang clean
+
+subdir-install-golang/xenlight: .phony
+	$(MAKE) -C golang install
+
+subdir-build-golang/xenlight: .phony
+	$(MAKE) -C golang build
+
+subdir-distclean-golang/xenlight: .phony
+	$(MAKE) -C golang distclean
 
 subdir-clean-debugger/kdd subdir-distclean-debugger/kdd: .phony
 	$(MAKE) -C debugger/kdd clean
diff --git a/tools/golang/Makefile b/tools/golang/Makefile
new file mode 100644
index 0000000..eead226
--- /dev/null
+++ b/tools/golang/Makefile
@@ -0,0 +1,33 @@
+XEN_ROOT=$(CURDIR)/../..
+include $(XEN_ROOT)/tools/Rules.mk
+
+BINARY = xenlight.a
+GO ?= go
+
+.PHONY: all
+all: build
+
+.PHONY: build
+build: xenlight/xenlight.a
+
+.PHONY: install
+install: build
+	if [ ! -f $(BINARY) ]; then \
+		mkdir $(GOPATH)/src/xenlight; \
+		$(INSTALL_PROG) xenlight/xenlight.go $(GOPATH)/src/xenlight/xenlight.go; \
+		echo "this"; \
+	fi  
+
+.PHONY: clean
+clean:
+	$(RM) $(BINARY)
+
+.PHONY: distclean
+distclean: clean
+
+
+xenlight/xenlight.a: xenlight/xenlight.go
+	$(GO) build -o $@ $<
+
+
+-include $(DEPS)
diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
new file mode 100644
index 0000000..9136676
--- /dev/null
+++ b/tools/golang/xenlight/xenlight.go
@@ -0,0 +1,7 @@
+package xenlight
+
+import "fmt"
+
+func main() {
+	fmt.Println("go")
+}
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH RFC] tools/xenlight: Create xenlight Makefile

[Doug Goldstein]

[-- Attachment #1.1.1: Type: text/plain, Size: 780 bytes --]

On 12/15/16 5:20 PM, Ronald Rojas wrote:
> Create a basic Makefile to build and install libxenlight Golang
> bindings.  Also add a template.
> 
> ---
> 
> Eventually this patch will contain the actual bindings package; for
> now it just includes a stub package.
> 

So I'm curious why the interest to include Golang bindings in the tree.
Most people are going to expect a Go package to behave like any other go
package and be able to grab it with "go get". But by including this in
the tree I believe that method will not work.

I say this because when I finally get around to making Rust bindings for
Xen, I have no plan to include them in the tree but instead publish them
at crates.io so that they work like every other Rust package.

-- 
Doug Goldstein


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

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH RFC] tools/xenlight: Create xenlight Makefile

[George Dunlap]

On Fri, Dec 16, 2016 at 7:20 AM, Ronald Rojas <ronladred@gmail.com> wrote:
> Create a basic Makefile to build and install libxenlight Golang
> bindings.  Also add a template.

What's this template again?

>
> ---
>
> Eventually this patch will contain the actual bindings package; for
> now it just includes a stub package.
>
> To Do:
> - Have configure detect golang bindings properly
>
> CC: xen-devel <xen-devel@lists.xen.org>
> CC: Ian Jackson <ian.jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: George Dunlap <george.dunlap@citrix.com>
> CC: George Dunlap <dunlapg@umich.edu>
> ---
>  tools/Makefile                    | 16 ++++++++++++++++
>  tools/golang/Makefile             | 33 +++++++++++++++++++++++++++++++++
>  tools/golang/xenlight/xenlight.go |  7 +++++++
>  3 files changed, 56 insertions(+)
>  create mode 100644 tools/golang/Makefile
>  create mode 100644 tools/golang/xenlight/xenlight.go
>
> diff --git a/tools/Makefile b/tools/Makefile
> index 71515b4..f2198e0 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -11,6 +11,8 @@ SUBDIRS-y += misc
>  SUBDIRS-y += examples
>  SUBDIRS-y += hotplug
>  SUBDIRS-y += xentrace
> +SUBDIRS-y += golang
> +#FIXME: Have golang controlled by a configure variable
>  SUBDIRS-$(CONFIG_XCUTILS) += xcutils
>  SUBDIRS-$(CONFIG_X86) += firmware
>  SUBDIRS-y += console
> @@ -311,6 +313,20 @@ subdir-install-debugger/gdbsx: .phony
>  subdir-all-debugger/gdbsx: .phony
>         $(MAKE) -C debugger/gdbsx all
>
> +subdir-all-golang/xenlight: .phony
> +       $(MAKE) -C golang all
> +
> +subdir-clean-golang/xenlight: .phony
> +       $(MAKE) -C golang clean
> +
> +subdir-install-golang/xenlight: .phony
> +       $(MAKE) -C golang install
> +
> +subdir-build-golang/xenlight: .phony
> +       $(MAKE) -C golang build
> +
> +subdir-distclean-golang/xenlight: .phony
> +       $(MAKE) -C golang distclean
>
>  subdir-clean-debugger/kdd subdir-distclean-debugger/kdd: .phony
>         $(MAKE) -C debugger/kdd clean
> diff --git a/tools/golang/Makefile b/tools/golang/Makefile
> new file mode 100644
> index 0000000..eead226
> --- /dev/null
> +++ b/tools/golang/Makefile
> @@ -0,0 +1,33 @@
> +XEN_ROOT=$(CURDIR)/../..
> +include $(XEN_ROOT)/tools/Rules.mk
> +
> +BINARY = xenlight.a
> +GO ?= go
> +
> +.PHONY: all
> +all: build
> +
> +.PHONY: build
> +build: xenlight/xenlight.a

I think eventually we'll want to have a makefile in tools/golang which
calls the makefiles in tools/golang/xenlight and other directories;
but since we have only one at the moment, I think this is probably
fine.

> +.PHONY: install
> +install: build
> +       if [ ! -f $(BINARY) ]; then \
> +               mkdir $(GOPATH)/src/xenlight; \
> +               $(INSTALL_PROG) xenlight/xenlight.go $(GOPATH)/src/xenlight/xenlight.go; \
> +               echo "this"; \
> +       fi

A couple of things.

First, as we discussed on IRC, the if[] clause is not necessary --
make will automatically stop if one of the prerequisites isn't
available.

Secondly -- this is a bit confusing, but the "install" section of the
parts of the tree that actually build things don't install files
directly onto the host filesystem.  Instead, they install into
$(DESTDIR) (which is xen.git/dist/install), and then the final step of
"make install" will copy everything from dist/install into the host
filesystem.

Also, you shouldn't do a plain mkdir, but should use $(INSTALL_DIR).
And since the file you're installing is not going to be executed, you
should use $(INSTALL_DATA).  (I didn't realize that distinction
existed until I looked at tools/libxl/Makefile).

So this should look more like:

install: build
  $(INSTALL_DIR) $(DESTDIR)$(GOLANG_SRC)
  $(INSTALL_DATA) xenlight/xenlight.go $(DESTDIR)$(GOLANG_SRC)

And we want the package name to be "xenproject.org/xenlight", so that
we can put up a repo at git://xenproject.org/xenlight and have people
download it automatically.  So GOLANG_SRC should be set by default to
$(GOPATH)/src/xenproject.org/xenlight

With that done I think we might be ready to add some golang code. :-)

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH RFC] tools/xenlight: Create xenlight Makefile

[George Dunlap]

> On Dec 16, 2016, at 11:20 AM, Doug Goldstein <cardoe@cardoe.com> wrote:
> 
> On 12/15/16 5:20 PM, Ronald Rojas wrote:
>> Create a basic Makefile to build and install libxenlight Golang
>> bindings.  Also add a template.
>> 
>> ---
>> 
>> Eventually this patch will contain the actual bindings package; for
>> now it just includes a stub package.
>> 
> 
> So I'm curious why the interest to include Golang bindings in the tree.
> Most people are going to expect a Go package to behave like any other go
> package and be able to grab it with "go get". But by including this in
> the tree I believe that method will not work.

My plan was to name the package such that we could have an automatically-mirrored git repo in the expected place; i.e.,

git://xenproject.org/xenlight

The “fetch the tree automatically based on the package name” distribution mechanism might be a decent fit for web-based technologies, but I think it’s a terrible fit for mature software projects like Xen; and I expect that there will be a shift in the next few years as people (re-)discover this.  I want to make it work, but I think that should be secondary to actually installing the library as a part of installing a Xen package / “make install”.

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH RFC] tools/xenlight: Create xenlight Makefile

[Ronald Rojas]

On Fri, Dec 16, 2016 at 12:27:31PM +0800, George Dunlap wrote:
> On Fri, Dec 16, 2016 at 7:20 AM, Ronald Rojas <ronladred@gmail.com> wrote:
> > Create a basic Makefile to build and install libxenlight Golang
> > bindings.  Also add a template.
> 
> What's this template again?

I use 'template' to mean the same thing as 'stub package'. I can change the 
wording to say 'stub package' instead if that's more clear.
> 
> >
> > ---
> >
> > Eventually this patch will contain the actual bindings package; for
> > now it just includes a stub package.
> >
> > To Do:
> > - Have configure detect golang bindings properly
> >
> > CC: xen-devel <xen-devel@lists.xen.org>
> > CC: Ian Jackson <ian.jackson@eu.citrix.com>
> > CC: Wei Liu <wei.liu2@citrix.com>
> > CC: George Dunlap <george.dunlap@citrix.com>
> > CC: George Dunlap <dunlapg@umich.edu>
> > ---
> >  tools/Makefile                    | 16 ++++++++++++++++
> >  tools/golang/Makefile             | 33 +++++++++++++++++++++++++++++++++
> >  tools/golang/xenlight/xenlight.go |  7 +++++++
> >  3 files changed, 56 insertions(+)
> >  create mode 100644 tools/golang/Makefile
> >  create mode 100644 tools/golang/xenlight/xenlight.go
> >
> > diff --git a/tools/Makefile b/tools/Makefile
> > index 71515b4..f2198e0 100644
> > --- a/tools/Makefile
> > +++ b/tools/Makefile
> > @@ -11,6 +11,8 @@ SUBDIRS-y += misc
> >  SUBDIRS-y += examples
> >  SUBDIRS-y += hotplug
> >  SUBDIRS-y += xentrace
> > +SUBDIRS-y += golang
> > +#FIXME: Have golang controlled by a configure variable
> >  SUBDIRS-$(CONFIG_XCUTILS) += xcutils
> >  SUBDIRS-$(CONFIG_X86) += firmware
> >  SUBDIRS-y += console
> > @@ -311,6 +313,20 @@ subdir-install-debugger/gdbsx: .phony
> >  subdir-all-debugger/gdbsx: .phony
> >         $(MAKE) -C debugger/gdbsx all
> >
> > +subdir-all-golang/xenlight: .phony
> > +       $(MAKE) -C golang all
> > +
> > +subdir-clean-golang/xenlight: .phony
> > +       $(MAKE) -C golang clean
> > +
> > +subdir-install-golang/xenlight: .phony
> > +       $(MAKE) -C golang install
> > +
> > +subdir-build-golang/xenlight: .phony
> > +       $(MAKE) -C golang build
> > +
> > +subdir-distclean-golang/xenlight: .phony
> > +       $(MAKE) -C golang distclean
> >
> >  subdir-clean-debugger/kdd subdir-distclean-debugger/kdd: .phony
> >         $(MAKE) -C debugger/kdd clean
> > diff --git a/tools/golang/Makefile b/tools/golang/Makefile
> > new file mode 100644
> > index 0000000..eead226
> > --- /dev/null
> > +++ b/tools/golang/Makefile
> > @@ -0,0 +1,33 @@
> > +XEN_ROOT=$(CURDIR)/../..
> > +include $(XEN_ROOT)/tools/Rules.mk
> > +
> > +BINARY = xenlight.a
> > +GO ?= go
> > +
> > +.PHONY: all
> > +all: build
> > +
> > +.PHONY: build
> > +build: xenlight/xenlight.a
> 
> I think eventually we'll want to have a makefile in tools/golang which
> calls the makefiles in tools/golang/xenlight and other directories;
> but since we have only one at the moment, I think this is probably
> fine.
I'll make a list so that if I have time late down the line, I'll remember 
to do this. 
> 
> > +.PHONY: install
> > +install: build
> > +       if [ ! -f $(BINARY) ]; then \
> > +               mkdir $(GOPATH)/src/xenlight; \
> > +               $(INSTALL_PROG) xenlight/xenlight.go $(GOPATH)/src/xenlight/xenlight.go; \
> > +               echo "this"; \
> > +       fi
> 
> A couple of things.
> 
> First, as we discussed on IRC, the if[] clause is not necessary --
> make will automatically stop if one of the prerequisites isn't
> available.
ok. I will delete the 'if' clause.
> 
> Secondly -- this is a bit confusing, but the "install" section of the
> parts of the tree that actually build things don't install files
> directly onto the host filesystem.  Instead, they install into
> $(DESTDIR) (which is xen.git/dist/install), and then the final step of
> "make install" will copy everything from dist/install into the host
> filesystem.
> 
> Also, you shouldn't do a plain mkdir, but should use $(INSTALL_DIR).
> And since the file you're installing is not going to be executed, you
> should use $(INSTALL_DATA).  (I didn't realize that distinction
> existed until I looked at tools/libxl/Makefile).
> 
> So this should look more like:
> 
> install: build
>   $(INSTALL_DIR) $(DESTDIR)$(GOLANG_SRC)
>   $(INSTALL_DATA) xenlight/xenlight.go $(DESTDIR)$(GOLANG_SRC)
Ok, I will change the the install rule with the appropriate changes
> 
> And we want the package name to be "xenproject.org/xenlight", so that
> we can put up a repo at git://xenproject.org/xenlight and have people
> download it automatically.  So GOLANG_SRC should be set by default to
> $(GOPATH)/src/xenproject.org/xenlight
Fixed
> 
> With that done I think we might be ready to add some golang code. :-)
> 
>  -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH RFC] tools/xenlight: Create xenlight Makefile

[Ronald Rojas]

Create a basic Makefile to build and install libxenlight Golang
bindings.  Also add a stub package.

---

Eventually this patch will contain the actual bindings package; for
now it just includes a stub package.

To Do:
- Have configure detect golang bindings properly

CC: xen-devel <xen-devel@lists.xen.org>
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: George Dunlap <george.dunlap@citrix.com>
CC: George Dunlap <dunlapg@umich.edu>
---
 tools/Makefile                    | 16 ++++++++++++++++
 tools/golang/Makefile             | 29 +++++++++++++++++++++++++++++
 tools/golang/xenlight/xenlight.go |  7 +++++++
 3 files changed, 52 insertions(+)
 create mode 100644 tools/golang/Makefile
 create mode 100644 tools/golang/xenlight/xenlight.go

diff --git a/tools/Makefile b/tools/Makefile
index 71515b4..f2198e0 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -11,6 +11,8 @@ SUBDIRS-y += misc
 SUBDIRS-y += examples
 SUBDIRS-y += hotplug
 SUBDIRS-y += xentrace
+SUBDIRS-y += golang
+#FIXME: Have golang controlled by a configure variable
 SUBDIRS-$(CONFIG_XCUTILS) += xcutils
 SUBDIRS-$(CONFIG_X86) += firmware
 SUBDIRS-y += console
@@ -311,6 +313,20 @@ subdir-install-debugger/gdbsx: .phony
 subdir-all-debugger/gdbsx: .phony
 	$(MAKE) -C debugger/gdbsx all
 
+subdir-all-golang/xenlight: .phony
+	$(MAKE) -C golang all
+
+subdir-clean-golang/xenlight: .phony
+	$(MAKE) -C golang clean
+
+subdir-install-golang/xenlight: .phony
+	$(MAKE) -C golang install
+
+subdir-build-golang/xenlight: .phony
+	$(MAKE) -C golang build
+
+subdir-distclean-golang/xenlight: .phony
+	$(MAKE) -C golang distclean
 
 subdir-clean-debugger/kdd subdir-distclean-debugger/kdd: .phony
 	$(MAKE) -C debugger/kdd clean
diff --git a/tools/golang/Makefile b/tools/golang/Makefile
new file mode 100644
index 0000000..96589c8
--- /dev/null
+++ b/tools/golang/Makefile
@@ -0,0 +1,29 @@
+XEN_ROOT=$(CURDIR)/../..
+GOLANG_SRC=$(GOPATH)/src/xenproject.org/xenlight
+include $(XEN_ROOT)/tools/Rules.mk
+
+BINARY = xenlight.a
+GO ?= go
+
+.PHONY: all
+all: build
+
+.PHONY: build
+build: xenlight/xenlight.a
+
+.PHONY: install
+install: build
+	$(INSTALL_DIR) $(DESTDIR)$(GOLANG_SRC)
+	$(INSTALL_DATA) xenlight/xenlight.go $(DESTDIR)$(GOLANG_SRC)
+
+.PHONY: clean
+clean:
+	$(RM) xenlight/$(BINARY)
+
+.PHONY: distclean
+distclean: clean
+
+xenlight/xenlight.a: xenlight/xenlight.go
+	$(GO) build -o $@ $<
+
+-include $(DEPS)
diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
new file mode 100644
index 0000000..9136676
--- /dev/null
+++ b/tools/golang/xenlight/xenlight.go
@@ -0,0 +1,7 @@
+package xenlight
+
+import "fmt"
+
+func main() {
+	fmt.Println("go")
+}
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH RFC] tools/xenlight: Create xenlight Makefile

[Ronald Rojas]

On Wed, Nov 30, 2016 at 12:30:04PM +1100, George Dunlap wrote:
> On Tue, Nov 29, 2016 at 4:18 AM, Ronald Rojas <ronladred@gmail.com> wrote:
> > Created basic Makfele for the Golang xenlight
> > project so that the project is built and
> > installed. A template template is alo added.
> 
> Thanks Ronald!  Not a bad first submission, but a lot of things that
> need tightening up.
> 
> First, the normal style of most commit messages is more direct;
> usually in the form "Do X" (like a recipe), or in the form "We do Y".
> 
> So to translate the comment above, I'd probably say something like this:
> 
> "Create a basic Makefile to build and install libxenlight Golang
> bindings.  Also add a template."  (Or, as an example of the second
> form, "We also add a template so that we have something to build.")

Alright, I can change the commit message. I'm still getting used to
how to write patches but I'll keep it in mind of next time. 

> 
> But in the final patch, we'll probably be introducing all the libxl
> golang bindings in one go (not first the makefile, then the bindings,
> &c), so it probably makes sense to have your mock-up start with that
> assumption, then explain that it's still a work in progress.
> 
> The best way to do this is to put comments for the reviewers under a
> line with three dashes ("---") in the commit message.
> 
> So something like this:
> 
> 8<--------
> 
> tools: Introduce xenlight golang bindings
> 
> Create tools/golang/xenlight and hook it into the main tools Makefile.
> 
> Signed-off-by: John Smith <jsmith@google.com>
> 
> ---
> 
> Eventually this patch will contain the actual bindings package; for
> now just include a stub package.
> 
> To do:
> - Make a real package
> - Have configure detect golang bindings properly
> 
> -------------->8

Understood :). I will do this in the next iteration of the patch

> 
> >  tools/Makefile                    | 15 +++++++++++++++
> >  tools/golang/xenlight/Makefile    | 29 +++++++++++++++++++++++++++++
> >  tools/golang/xenlight/xenlight.go |  7 +++++++
> >  3 files changed, 51 insertions(+)
> >  create mode 100644 tools/golang/xenlight/Makefile
> >  create mode 100644 tools/golang/xenlight/xenlight.go
> >
> > diff --git a/tools/Makefile b/tools/Makefile
> > index 71515b4..b89f06b 100644
> > --- a/tools/Makefile
> > +++ b/tools/Makefile
> > @@ -11,6 +11,7 @@ SUBDIRS-y += misc
> >  SUBDIRS-y += examples
> >  SUBDIRS-y += hotplug
> >  SUBDIRS-y += xentrace
> > +SUBDIRS-y += golang/xenlight
> 
> As Wei said, you should follow the python model, and put another
> Makefile in tools/golang.

Sorry but I don't really I don't really understand how the python
Makefile works. In particular I don't really understand how the 
build rule and setup.py works( maybe because I haven't written 
any python). Would you mind giving me some intuition on how it 
works?

> 
> And you should add a comment here saying that you plan to use
> autotools to do this eventually:
> 
> # FIXME: Have this controlled by a configure variable

Understood.

> 
> >  SUBDIRS-$(CONFIG_XCUTILS) += xcutils
> >  SUBDIRS-$(CONFIG_X86) += firmware
> >  SUBDIRS-y += console
> > @@ -311,6 +312,20 @@ subdir-install-debugger/gdbsx: .phony
> >  subdir-all-debugger/gdbsx: .phony
> >         $(MAKE) -C debugger/gdbsx all
> >
> > +subdir-all-golang/xenlight: .phony
> > +       $(MAKE) -C golang/xenlight all
> > +
> > +subdir-clean-golang/xenlight: .phony
> > +       $(MAKE) -C golang/xenlight clean
> > +
> > +subdir-install-golang/xenlight: .phony
> > +       $(MAKE) -C golang/xenlight install
> > +
> > +subdir-build-golang/xenlight: .phony
> > +       $(MAKE) -C golang/xenlight build
> > +
> > +subdir-distclean-golang/xenlight: .phony
> > +       $(MAKE) -C golang/xenlight distclean
> >
> >  subdir-clean-debugger/kdd subdir-distclean-debugger/kdd: .phony
> >         $(MAKE) -C debugger/kdd clean
> > diff --git a/tools/golang/xenlight/Makefile b/tools/golang/xenlight/Makefile
> > new file mode 100644
> > index 0000000..c723c1d
> > --- /dev/null
> > +++ b/tools/golang/xenlight/Makefile
> > @@ -0,0 +1,29 @@
> > +XEN_ROOT=$(CURDIR)/../../..
> > +include $(XEN_ROOT)/tools/Rules.mk
> > +
> > +BINARY = xenlightgo
> > +GO = go
> > +
> > +.PHONY: all
> > +all: build
> > +
> > +.PHONY: build
> > +build: xenlight
> > +
> > +.PHONY: install
> > +install: build
> > +       [ -f $(BINARY) ] || $(INSTALL_PROG) xenlight.go $(DESTDIR)$(bindir)
> 
> Is there a reason you decided to make this a binary (and to install it
> as a binary), rather than making a stub package?
I'm not sure what you mean here. Doesn't this install the go file xenlight.go, 
not a binary file? 
Or do you want me to create a package and install that instead package? 
Sorry, I'm a little lost here.
> 
> Thanks,
>  -George
Thanks, 
Ronald 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH RFC] tools/xenlight: Create xenlight Makefile

[George Dunlap]

On Tue, Nov 29, 2016 at 4:18 AM, Ronald Rojas <ronladred@gmail.com> wrote:
> Created basic Makfele for the Golang xenlight
> project so that the project is built and
> installed. A template template is alo added.

Thanks Ronald!  Not a bad first submission, but a lot of things that
need tightening up.

First, the normal style of most commit messages is more direct;
usually in the form "Do X" (like a recipe), or in the form "We do Y".

So to translate the comment above, I'd probably say something like this:

"Create a basic Makefile to build and install libxenlight Golang
bindings.  Also add a template."  (Or, as an example of the second
form, "We also add a template so that we have something to build.")

But in the final patch, we'll probably be introducing all the libxl
golang bindings in one go (not first the makefile, then the bindings,
&c), so it probably makes sense to have your mock-up start with that
assumption, then explain that it's still a work in progress.

The best way to do this is to put comments for the reviewers under a
line with three dashes ("---") in the commit message.

So something like this:

8<--------

tools: Introduce xenlight golang bindings

Create tools/golang/xenlight and hook it into the main tools Makefile.

Signed-off-by: John Smith <jsmith@google.com>

---

Eventually this patch will contain the actual bindings package; for
now just include a stub package.

To do:
- Make a real package
- Have configure detect golang bindings properly

-------------->8

>  tools/Makefile                    | 15 +++++++++++++++
>  tools/golang/xenlight/Makefile    | 29 +++++++++++++++++++++++++++++
>  tools/golang/xenlight/xenlight.go |  7 +++++++
>  3 files changed, 51 insertions(+)
>  create mode 100644 tools/golang/xenlight/Makefile
>  create mode 100644 tools/golang/xenlight/xenlight.go
>
> diff --git a/tools/Makefile b/tools/Makefile
> index 71515b4..b89f06b 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -11,6 +11,7 @@ SUBDIRS-y += misc
>  SUBDIRS-y += examples
>  SUBDIRS-y += hotplug
>  SUBDIRS-y += xentrace
> +SUBDIRS-y += golang/xenlight

As Wei said, you should follow the python model, and put another
Makefile in tools/golang.

And you should add a comment here saying that you plan to use
autotools to do this eventually:

# FIXME: Have this controlled by a configure variable

>  SUBDIRS-$(CONFIG_XCUTILS) += xcutils
>  SUBDIRS-$(CONFIG_X86) += firmware
>  SUBDIRS-y += console
> @@ -311,6 +312,20 @@ subdir-install-debugger/gdbsx: .phony
>  subdir-all-debugger/gdbsx: .phony
>         $(MAKE) -C debugger/gdbsx all
>
> +subdir-all-golang/xenlight: .phony
> +       $(MAKE) -C golang/xenlight all
> +
> +subdir-clean-golang/xenlight: .phony
> +       $(MAKE) -C golang/xenlight clean
> +
> +subdir-install-golang/xenlight: .phony
> +       $(MAKE) -C golang/xenlight install
> +
> +subdir-build-golang/xenlight: .phony
> +       $(MAKE) -C golang/xenlight build
> +
> +subdir-distclean-golang/xenlight: .phony
> +       $(MAKE) -C golang/xenlight distclean
>
>  subdir-clean-debugger/kdd subdir-distclean-debugger/kdd: .phony
>         $(MAKE) -C debugger/kdd clean
> diff --git a/tools/golang/xenlight/Makefile b/tools/golang/xenlight/Makefile
> new file mode 100644
> index 0000000..c723c1d
> --- /dev/null
> +++ b/tools/golang/xenlight/Makefile
> @@ -0,0 +1,29 @@
> +XEN_ROOT=$(CURDIR)/../../..
> +include $(XEN_ROOT)/tools/Rules.mk
> +
> +BINARY = xenlightgo
> +GO = go
> +
> +.PHONY: all
> +all: build
> +
> +.PHONY: build
> +build: xenlight
> +
> +.PHONY: install
> +install: build
> +       [ -f $(BINARY) ] || $(INSTALL_PROG) xenlight.go $(DESTDIR)$(bindir)

Is there a reason you decided to make this a binary (and to install it
as a binary), rather than making a stub package?

Thanks,
 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH RFC] tools/xenlight: Create xenlight Makefile

[George Dunlap]

On Tue, Nov 29, 2016 at 6:19 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> On Mon, Nov 28, 2016 at 12:18:03PM -0500, Ronald Rojas wrote:
>> diff --git a/tools/Makefile b/tools/Makefile
>> index 71515b4..b89f06b 100644
>> --- a/tools/Makefile
>> +++ b/tools/Makefile
>> @@ -11,6 +11,7 @@ SUBDIRS-y += misc
>>  SUBDIRS-y += examples
>>  SUBDIRS-y += hotplug
>>  SUBDIRS-y += xentrace
>> +SUBDIRS-y += golang/xenlight
>
> This shouldn't be built unconditionally.
>
> Please use configure to probe for go compiler first.
>
> "GNU autotools" is the term to google for. But I guess it wouldn't be
> too hard to follow existing examples.
>
> Don't hesitate to ask questions.

BTW I suggested that Ronald start with enabling it unconditionally, so
that he could get something straightforward related to his project
(golang bindings) accomplished before having to dive right into
something complicated and unrelated (autoconf).

But it's probably worth communicating that here with a comment.  I'll
respond directly to his mail.

>>  SUBDIRS-$(CONFIG_XCUTILS) += xcutils
>>  SUBDIRS-$(CONFIG_X86) += firmware
>>  SUBDIRS-y += console
>> @@ -311,6 +312,20 @@ subdir-install-debugger/gdbsx: .phony
>>  subdir-all-debugger/gdbsx: .phony
>>       $(MAKE) -C debugger/gdbsx all
>>
>> +subdir-all-golang/xenlight: .phony
>> +     $(MAKE) -C golang/xenlight all
>> +
>> +subdir-clean-golang/xenlight: .phony
>> +     $(MAKE) -C golang/xenlight clean
>> +
>> +subdir-install-golang/xenlight: .phony
>> +     $(MAKE) -C golang/xenlight install
>> +
>> +subdir-build-golang/xenlight: .phony
>> +     $(MAKE) -C golang/xenlight build
>> +
>> +subdir-distclean-golang/xenlight: .phony
>> +     $(MAKE) -C golang/xenlight distclean
>>
>
> I think I would prefer to organise this like python bindings. That is,
> to have a dedicated Makefile under $LANG (python or golang) directory.
>
> What do you think?

That's what I had in mind as well.

>
>>  subdir-clean-debugger/kdd subdir-distclean-debugger/kdd: .phony
>>       $(MAKE) -C debugger/kdd clean
>> diff --git a/tools/golang/xenlight/Makefile b/tools/golang/xenlight/Makefile
>> new file mode 100644
>> index 0000000..c723c1d
>> --- /dev/null
>> +++ b/tools/golang/xenlight/Makefile
>> @@ -0,0 +1,29 @@
>> +XEN_ROOT=$(CURDIR)/../../..
>> +include $(XEN_ROOT)/tools/Rules.mk
>> +
>> +BINARY = xenlightgo
>> +GO = go
>
> This should probably be:
>
>   GO ?= go

With an additional comment above it:

# FIXME Use autoconf to find this?

>> +
>> +.PHONY: all
>> +all: build
>> +
>> +.PHONY: build
>> +build: xenlight
>> +
>> +.PHONY: install
>> +install: build
>> +     [ -f $(BINARY) ] || $(INSTALL_PROG) xenlight.go $(DESTDIR)$(bindir)
>> +
>> +.PHONY: clean
>> +clean:
>> +     $(RM) $(BINARY)
>
> If there are other intermediate files generated, they should be deleted,
> too.

At the moment there won't be any intermediate files -- "go build"
creates a temporary directory for those and deletes them after it's
done automatically.

>> +.PHONY: distclean
>> +distclean: clean
>> +
>> +
>> +xenlight: xenlight.go
>
> xenlightgo: xenlight.go and delete BINARY
>
> or
>
> $(BINARY): xenlight.go
>
>> +     $(GO) build -o $(BINARY) xenlight.go
>
> Use $@ instead of $(BINARY).  Use $< instead of xenlight.go.
>
> But before we spend too much time on details, let's sort out some higher
> level issues first. My golang knowledge is rather rusted, please bear
> with me. :-)
>
> I have a question: how does golang build a package that has multiple
> files? Presumably it has some sort of file that acts like Makefile. If
> so, you should probably introduce that now and use that to build this
> package.

Not really -- you either hand it all the files it needs (like you
would a C compiler), or you hand it some of the files it needs and
also give it a directory to the source code for your packages, or you
do things entirely its way and just type "go build" and it figures out
what to do.  If you want Makefile-like functionality you probably want
make. :-)

In our case we'll probably end up doing the first for building the
xenlight package, and the second for building the test tool.

But there's no point in getting into the nitty-gritty now because what
Ronald has here isn't a package (meant to be linked against), but a
program (meant to be run).  So we'll have to wait until we have basic
package stuff to figure out the best way to link it.

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH RFC] tools/xenlight: Create xenlight Makefile

[Wei Liu]

On Mon, Nov 28, 2016 at 12:18:03PM -0500, Ronald Rojas wrote:
> Created basic Makfele for the Golang xenlight

Makefile.

> project so that the project is built and
> installed. A template template is alo added.

Duplicated "template".

> ---
>  tools/Makefile                    | 15 +++++++++++++++
>  tools/golang/xenlight/Makefile    | 29 +++++++++++++++++++++++++++++
>  tools/golang/xenlight/xenlight.go |  7 +++++++
>  3 files changed, 51 insertions(+)
>  create mode 100644 tools/golang/xenlight/Makefile
>  create mode 100644 tools/golang/xenlight/xenlight.go
> 
> diff --git a/tools/Makefile b/tools/Makefile
> index 71515b4..b89f06b 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -11,6 +11,7 @@ SUBDIRS-y += misc
>  SUBDIRS-y += examples
>  SUBDIRS-y += hotplug
>  SUBDIRS-y += xentrace
> +SUBDIRS-y += golang/xenlight

This shouldn't be built unconditionally.

Please use configure to probe for go compiler first.

"GNU autotools" is the term to google for. But I guess it wouldn't be
too hard to follow existing examples.

Don't hesitate to ask questions.

>  SUBDIRS-$(CONFIG_XCUTILS) += xcutils
>  SUBDIRS-$(CONFIG_X86) += firmware
>  SUBDIRS-y += console
> @@ -311,6 +312,20 @@ subdir-install-debugger/gdbsx: .phony
>  subdir-all-debugger/gdbsx: .phony
>  	$(MAKE) -C debugger/gdbsx all
>  
> +subdir-all-golang/xenlight: .phony
> +	$(MAKE) -C golang/xenlight all
> +
> +subdir-clean-golang/xenlight: .phony
> +	$(MAKE) -C golang/xenlight clean
> +
> +subdir-install-golang/xenlight: .phony
> +	$(MAKE) -C golang/xenlight install
> +
> +subdir-build-golang/xenlight: .phony
> +	$(MAKE) -C golang/xenlight build
> +
> +subdir-distclean-golang/xenlight: .phony
> +	$(MAKE) -C golang/xenlight distclean
>  

I think I would prefer to organise this like python bindings. That is,
to have a dedicated Makefile under $LANG (python or golang) directory.

What do you think?

>  subdir-clean-debugger/kdd subdir-distclean-debugger/kdd: .phony
>  	$(MAKE) -C debugger/kdd clean
> diff --git a/tools/golang/xenlight/Makefile b/tools/golang/xenlight/Makefile
> new file mode 100644
> index 0000000..c723c1d
> --- /dev/null
> +++ b/tools/golang/xenlight/Makefile
> @@ -0,0 +1,29 @@
> +XEN_ROOT=$(CURDIR)/../../..
> +include $(XEN_ROOT)/tools/Rules.mk
> +
> +BINARY = xenlightgo
> +GO = go

This should probably be:

  GO ?= go

> +
> +.PHONY: all
> +all: build
> +
> +.PHONY: build
> +build: xenlight
> +
> +.PHONY: install
> +install: build
> +	[ -f $(BINARY) ] || $(INSTALL_PROG) xenlight.go $(DESTDIR)$(bindir)
> +
> +.PHONY: clean
> +clean:
> +	$(RM) $(BINARY)

If there are other intermediate files generated, they should be deleted,
too.

> +
> +.PHONY: distclean
> +distclean: clean
> +
> +
> +xenlight: xenlight.go

xenlightgo: xenlight.go and delete BINARY

or

$(BINARY): xenlight.go

> +	$(GO) build -o $(BINARY) xenlight.go

Use $@ instead of $(BINARY).  Use $< instead of xenlight.go.

But before we spend too much time on details, let's sort out some higher
level issues first. My golang knowledge is rather rusted, please bear
with me. :-)

I have a question: how does golang build a package that has multiple
files? Presumably it has some sort of file that acts like Makefile. If
so, you should probably introduce that now and use that to build this
package.

Otherwise in order to avoid repetition you probably need something like
the %.o: %.c rule in tools/Rules.mk.

If you need more information about GNU Make, please use `info make` on
your system or ask questions here.

> +
> +
> +-include $(DEPS)
> diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
> new file mode 100644
> index 0000000..50e8d8d
> --- /dev/null
> +++ b/tools/golang/xenlight/xenlight.go
> @@ -0,0 +1,7 @@
> +package main
> +
> +import "fmt"
> +
> +func main() {
> +	fmt.Println("vim-go")

vim-go ?

> +}
> -- 
> 2.7.4
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH RFC] tools/xenlight: Create xenlight Makefile

[Ronald Rojas]

Created basic Makfele for the Golang xenlight
project so that the project is built and
installed. A template template is alo added.
---
 tools/Makefile                    | 15 +++++++++++++++
 tools/golang/xenlight/Makefile    | 29 +++++++++++++++++++++++++++++
 tools/golang/xenlight/xenlight.go |  7 +++++++
 3 files changed, 51 insertions(+)
 create mode 100644 tools/golang/xenlight/Makefile
 create mode 100644 tools/golang/xenlight/xenlight.go

diff --git a/tools/Makefile b/tools/Makefile
index 71515b4..b89f06b 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -11,6 +11,7 @@ SUBDIRS-y += misc
 SUBDIRS-y += examples
 SUBDIRS-y += hotplug
 SUBDIRS-y += xentrace
+SUBDIRS-y += golang/xenlight
 SUBDIRS-$(CONFIG_XCUTILS) += xcutils
 SUBDIRS-$(CONFIG_X86) += firmware
 SUBDIRS-y += console
@@ -311,6 +312,20 @@ subdir-install-debugger/gdbsx: .phony
 subdir-all-debugger/gdbsx: .phony
 	$(MAKE) -C debugger/gdbsx all
 
+subdir-all-golang/xenlight: .phony
+	$(MAKE) -C golang/xenlight all
+
+subdir-clean-golang/xenlight: .phony
+	$(MAKE) -C golang/xenlight clean
+
+subdir-install-golang/xenlight: .phony
+	$(MAKE) -C golang/xenlight install
+
+subdir-build-golang/xenlight: .phony
+	$(MAKE) -C golang/xenlight build
+
+subdir-distclean-golang/xenlight: .phony
+	$(MAKE) -C golang/xenlight distclean
 
 subdir-clean-debugger/kdd subdir-distclean-debugger/kdd: .phony
 	$(MAKE) -C debugger/kdd clean
diff --git a/tools/golang/xenlight/Makefile b/tools/golang/xenlight/Makefile
new file mode 100644
index 0000000..c723c1d
--- /dev/null
+++ b/tools/golang/xenlight/Makefile
@@ -0,0 +1,29 @@
+XEN_ROOT=$(CURDIR)/../../..
+include $(XEN_ROOT)/tools/Rules.mk
+
+BINARY = xenlightgo
+GO = go
+
+.PHONY: all
+all: build
+
+.PHONY: build
+build: xenlight
+
+.PHONY: install
+install: build
+	[ -f $(BINARY) ] || $(INSTALL_PROG) xenlight.go $(DESTDIR)$(bindir)
+
+.PHONY: clean
+clean:
+	$(RM) $(BINARY)
+
+.PHONY: distclean
+distclean: clean
+
+
+xenlight: xenlight.go
+	$(GO) build -o $(BINARY) xenlight.go
+
+
+-include $(DEPS)
diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
new file mode 100644
index 0000000..50e8d8d
--- /dev/null
+++ b/tools/golang/xenlight/xenlight.go
@@ -0,0 +1,7 @@
+package main
+
+import "fmt"
+
+func main() {
+	fmt.Println("vim-go")
+}
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel