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

[George Dunlap <dunlapg@umich.edu>]

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