tools/makefile: Add build target

tools/makefile: Add build target

[Andrew Cooper]

[-- Attachment #1: Type: text/plain, Size: 228 bytes --]

The alternative is to change the root makefile to call "$(MAKE) -C
tools/ all" instead, but this way feels neater to me.

-- 
Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer
T: +44 (0)1223 225 900, http://www.citrix.com


[-- Attachment #2: tools-build-target.patch --]
[-- Type: text/x-patch, Size: 688 bytes --]

# HG changeset patch
# Parent f8ea2e12ed5ab33f160e65f22c9ce8f379863053
tools/makefile: Add build target

The root Makefile has a build target which calls "$(MAKE) build" in each of xen/
tools/ stubdom/ and docs/, which fails because of the tools/ Makefile.

This patch adds a phony build target, allowing a call to "make build" in the
repository root directory still work.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

diff -r f8ea2e12ed5a tools/Makefile
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -64,6 +64,9 @@ endif
 .PHONY: all
 all: subdirs-all
 
+.PHONY: build
+build: subdirs-all
+
 .PHONY: install
 install: subdirs-install
 	$(INSTALL_DIR) $(DESTDIR)/var/xen/dump

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

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

Re: tools/makefile: Add build target

[Ian Jackson]

Andrew Cooper writes ("[Xen-devel] tools/makefile: Add build target"):
> The alternative is to change the root makefile to call "$(MAKE) -C
> tools/ all" instead, but this way feels neater to me.

Yes.

> tools/makefile: Add build target
> 
> The root Makefile has a build target which calls "$(MAKE) build" in each of xen/
> tools/ stubdom/ and docs/, which fails because of the tools/ Makefile.

That this didn't work is clearly a bug.

But when I tried it my build did this:

  CC    i386-stubdom/eeprom93xx.o
/u/iwj/work/xen-unstable-tools.hg/stubdom/mini-os-x86_32-c/test.o: In function `app_main':
/u/iwj/work/xen-unstable-tools.hg/extras/mini-os/test.c:441: multiple definition of `app_main'
/u/iwj/work/xen-unstable-tools.hg/stubdom/mini-os-x86_32-c/main.o:/u/iwj/work/xen-unstable-tools.hg/extras/mini-os/main.c:187: first defined here
  CC    i386-stubdom/eepro100.o
make[2]: *** [/u/iwj/work/xen-unstable-tools.hg/stubdom/mini-os-x86_32-c/mini-os] Error 1
make[2]: Leaving directory `/u/iwj/work/xen-unstable-tools.hg/extras/mini-os'
make[1]: *** [c-stubdom] Error 2
make[1]: *** Waiting for unfinished jobs....

That's the result of:

 (make -j4 build && echo ok.) 2>&1 | tee ../log

Ian.

Re: tools/makefile: Add build target

[Andrew Cooper]

On 07/08/12 19:27, Ian Jackson wrote:
> Andrew Cooper writes ("[Xen-devel] tools/makefile: Add build target"):
>> The alternative is to change the root makefile to call "$(MAKE) -C
>> tools/ all" instead, but this way feels neater to me.
> Yes.
>
>> tools/makefile: Add build target
>>
>> The root Makefile has a build target which calls "$(MAKE) build" in each of xen/
>> tools/ stubdom/ and docs/, which fails because of the tools/ Makefile.
> That this didn't work is clearly a bug.
>
> But when I tried it my build did this:
>
>   CC    i386-stubdom/eeprom93xx.o
> /u/iwj/work/xen-unstable-tools.hg/stubdom/mini-os-x86_32-c/test.o: In function `app_main':
> /u/iwj/work/xen-unstable-tools.hg/extras/mini-os/test.c:441: multiple definition of `app_main'
> /u/iwj/work/xen-unstable-tools.hg/stubdom/mini-os-x86_32-c/main.o:/u/iwj/work/xen-unstable-tools.hg/extras/mini-os/main.c:187: first defined here
>   CC    i386-stubdom/eepro100.o
> make[2]: *** [/u/iwj/work/xen-unstable-tools.hg/stubdom/mini-os-x86_32-c/mini-os] Error 1
> make[2]: Leaving directory `/u/iwj/work/xen-unstable-tools.hg/extras/mini-os'
> make[1]: *** [c-stubdom] Error 2
> make[1]: *** Waiting for unfinished jobs....
>
> That's the result of:
>
>  (make -j4 build && echo ok.) 2>&1 | tee ../log
>
> Ian.

The issue there is that app_main() is defined as __attribute__((weak)),
and has two definitions in the code.

I have just tested and confirmed that minios (and therefore stubdom) is
not -j safe.  As a result, I would argue that this build failure is not
nesseserally a barrier of entry to the patch itself;  There are more
issues which need fixing as well.

stubdom itself has further build issues regarding relative paths to
configure scripts, which I was going to around to fixing after some of
my more important tasks.

-- 
Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer
T: +44 (0)1223 225 900, http://www.citrix.com

Re: tools/makefile: Add build target

[Ian Jackson]

Andrew Cooper writes ("Re: [Xen-devel] tools/makefile: Add build target"):
> The issue there is that app_main() is defined as __attribute__((weak)),
> and has two definitions in the code.
> 
> I have just tested and confirmed that minios (and therefore stubdom) is
> not -j safe.  As a result, I would argue that this build failure is not
> nesseserally a barrier of entry to the patch itself;  There are more
> issues which need fixing as well.

After struggling with testing a bit, -j appears to be a red herring.

The real bug is that if you say "make -C stubdom build" you end up
trying to run the "c-stubdom" target.  However that target (some kind
of test application?) is what breaks.  It's not built or used by
"make -C stubdom install" which is what you get if you say "make" at
the toplevel.

See patch below.

With that and your patch "make build" (and "make build -j4") works for
me.  I don't understand how it can have worked for you with just your
original patch; without mine, "make build" (without -j) fails in the
same way as I quote above.

Can you go into a bit more detail about the -j bug(s) in minios ?  I'd
be happy to take patches to add ".NOTPARALLEL:" in appropriate places.

> stubdom itself has further build issues regarding relative paths to
> configure scripts, which I was going to around to fixing after some of
> my more important tasks.

Fixes welcome of course.

Ian.

# HG changeset patch
# Parent bcacd62f0460bd461f0df477e41f45144e19f2e8
stubdom: do not build "c-stubdom"

This does not build:

  /u/iwj/work/xen-unstable-tools.hg/stubdom/mini-os-x86_32-c/test.o: In function `app_main':
  /u/iwj/work/xen-unstable-tools.hg/extras/mini-os/test.c:441: multiple definition of `app_main'
  /u/iwj/work/xen-unstable-tools.hg/stubdom/mini-os-x86_32-c/main.o:/u/iwj/work/xen-unstable-tools.hg/extras/mini-os/main.c:187: first defined here

It's not built by the default toplevel "make all", which is how this
has gone unnoticed.  c-stubdom appears to be some kind of test
application; in any case it is not currently used.

Fixing this means that the contents of the stubdom build: target are
aligned with the contents of the install: target.  Now
"make -C stubdom build" only builds the things that would be built
(for installation) by "make -C stubdom install".

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

diff -r bcacd62f0460 stubdom/Makefile
--- a/stubdom/Makefile	Tue Aug 07 19:14:31 2012 +0100
+++ b/stubdom/Makefile	Thu Aug 09 17:42:23 2012 +0100
@@ -81,7 +81,7 @@ CROSS_MAKE := $(MAKE) DESTDIR=
 .PHONY: all
 all: build
 ifeq ($(STUBDOM_SUPPORTED),1)
-build: genpath ioemu-stubdom c-stubdom pv-grub xenstore-stubdom
+build: genpath ioemu-stubdom pv-grub xenstore-stubdom
 else
 build: genpath
 endif