On Wed, Nov 30, 2016 at 12:30:04PM +1100, George Dunlap wrote:
> On Tue, Nov 29, 2016 at 4:18 AM, Ronald Rojas <ronlad...@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:
> tools: Introduce xenlight golang bindings
> Create tools/golang/xenlight and hook it into the main tools Makefile.
> Signed-off-by: John Smith <jsm...@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
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
> 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?
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.
Xen-devel mailing list