diff mbox series

[XEN,24/57] tools/debugger/gdbsx: Fix and cleanup makefiles

Message ID 20211206170241.13165-25-anthony.perard@citrix.com (mailing list archive)
State New, archived
Headers show
Series Toolstack build system improvement, toward non-recursive makefiles | expand

Commit Message

Anthony PERARD Dec. 6, 2021, 5:02 p.m. UTC
gdbsx/:
  - Make use of subdir facility for the "clean" target.
  - No need to remove the *.a, they aren't in this dir.
  - Avoid calling "distclean" in subdirs as "distclean" targets do only
    call "clean", and the "clean" also runs "clean" in subdirs.
  - Avoid the need to make "gx_all.a" and "xg_all.a" in the "all"
    recipe by forcing make to check for update of "xg/xg_all.a" and
    "gx/gx_all.a" by having "FORCE" as prerequisite. Now, when making
    "gdbsx", make will recurse even when both *.a already exist.
  - List target in $(TARGETS).

gdbsx/*/:
  - Fix dependency on *.h.
  - Remove some dead code.
  - List targets in $(TARGETS).
  - Remove "build" target.
  - Cleanup "clean" targets.
  - remove comments about the choice of "ar" instead of "ld"

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/debugger/gdbsx/Makefile    | 20 ++++++++++----------
 tools/debugger/gdbsx/gx/Makefile | 15 +++++++--------
 tools/debugger/gdbsx/xg/Makefile | 25 +++++++------------------
 3 files changed, 24 insertions(+), 36 deletions(-)

Comments

Andrew Cooper Dec. 16, 2021, 5:55 p.m. UTC | #1
On 06/12/2021 17:02, Anthony PERARD wrote:
> diff --git a/tools/debugger/gdbsx/Makefile b/tools/debugger/gdbsx/Makefile
> index 8d7cd94a31..4aaf427c45 100644
> --- a/tools/debugger/gdbsx/Makefile
> +++ b/tools/debugger/gdbsx/Makefile
> @@ -28,7 +28,7 @@ uninstall:
>  gdbsx: gx/gx_all.a xg/xg_all.a 
>  	$(CC) $(LDFLAGS) -o $@ $^
>  
> -xg/xg_all.a:
> +xg/xg_all.a: FORCE
>  	$(MAKE) -C xg
> -gx/gx_all.a:
> +gx/gx_all.a: FORCE
>  	$(MAKE) -C gx

Shouldn't these be in the sub Make's ?

> diff --git a/tools/debugger/gdbsx/gx/Makefile b/tools/debugger/gdbsx/gx/Makefile
> index 3b8467f799..ff5c8e9e6e 100644
> --- a/tools/debugger/gdbsx/gx/Makefile
> +++ b/tools/debugger/gdbsx/gx/Makefile
> @@ -2,21 +2,20 @@ XEN_ROOT = $(CURDIR)/../../../..
>  include ../Rules.mk
>  
>  GX_OBJS := gx_comm.o gx_main.o gx_utils.o gx_local.o
> -GX_HDRS := $(wildcard *.h)
> +
> +TARGETS := gx_all.a
>  
>  .PHONY: all
> -all: gx_all.a
> +all: $(TARGETS)
>  
>  .PHONY: clean
>  clean:
> -	rm -rf gx_all.a *.o .*.d
> +	rm -f *.o $(TARGETS) $(DEPS_RM)

$(RM)

>  
>  .PHONY: distclean
>  distclean: clean
>  
> -#%.o: %.c $(GX_HDRS) Makefile
> -#	$(CC) -c $(CFLAGS) -o $@ $<
> -
> -gx_all.a: $(GX_OBJS) Makefile $(GX_HDRS)
> -	ar cr $@ $(GX_OBJS)        # problem with ld using -m32 
> +gx_all.a: $(GX_OBJS) Makefile
> +	ar cr $@ $(GX_OBJS)

There's probably an $(AR) we ought to be using.

~Andrew
Anthony PERARD Dec. 21, 2021, 5:30 p.m. UTC | #2
On Thu, Dec 16, 2021 at 05:55:29PM +0000, Andrew Cooper wrote:
> On 06/12/2021 17:02, Anthony PERARD wrote:
> > diff --git a/tools/debugger/gdbsx/Makefile b/tools/debugger/gdbsx/Makefile
> > index 8d7cd94a31..4aaf427c45 100644
> > --- a/tools/debugger/gdbsx/Makefile
> > +++ b/tools/debugger/gdbsx/Makefile
> > @@ -28,7 +28,7 @@ uninstall:
> >  gdbsx: gx/gx_all.a xg/xg_all.a 
> >  	$(CC) $(LDFLAGS) -o $@ $^
> >  
> > -xg/xg_all.a:
> > +xg/xg_all.a: FORCE
> >  	$(MAKE) -C xg
> > -gx/gx_all.a:
> > +gx/gx_all.a: FORCE
> >  	$(MAKE) -C gx
> 
> Shouldn't these be in the sub Make's ?

No, this is how we tell make how to build some of the prerequisite
needed to build "gdbsx", we tell make that they are build in
sub-directory.

> > diff --git a/tools/debugger/gdbsx/gx/Makefile b/tools/debugger/gdbsx/gx/Makefile
> > -#%.o: %.c $(GX_HDRS) Makefile
> > -#	$(CC) -c $(CFLAGS) -o $@ $<
> > -
> > -gx_all.a: $(GX_OBJS) Makefile $(GX_HDRS)
> > -	ar cr $@ $(GX_OBJS)        # problem with ld using -m32 
> > +gx_all.a: $(GX_OBJS) Makefile
> > +	ar cr $@ $(GX_OBJS)
> 
> There's probably an $(AR) we ought to be using.

Yes, I'll look at that.

Thanks,
diff mbox series

Patch

diff --git a/tools/debugger/gdbsx/Makefile b/tools/debugger/gdbsx/Makefile
index 8d7cd94a31..4aaf427c45 100644
--- a/tools/debugger/gdbsx/Makefile
+++ b/tools/debugger/gdbsx/Makefile
@@ -1,20 +1,20 @@ 
 XEN_ROOT = $(CURDIR)/../../..
 include ./Rules.mk
 
+SUBDIRS-y += gx
+SUBDIRS-y += xg
+
+TARGETS := gdbsx
+
 .PHONY: all
-all:
-	$(MAKE) -C gx
-	$(MAKE) -C xg
-	$(MAKE) gdbsx
+all: $(TARGETS)
 
 .PHONY: clean
-clean:
-	rm -f xg_all.a gx_all.a gdbsx
-	set -e; for d in xg gx; do $(MAKE) -C $$d clean; done
+clean: subdirs-clean
+	rm -f $(TARGETS)
 
 .PHONY: distclean
 distclean: clean
-	set -e; for d in xg gx; do $(MAKE) -c $$d distclean; done
 
 .PHONY: install
 install: all
@@ -28,7 +28,7 @@  uninstall:
 gdbsx: gx/gx_all.a xg/xg_all.a 
 	$(CC) $(LDFLAGS) -o $@ $^
 
-xg/xg_all.a:
+xg/xg_all.a: FORCE
 	$(MAKE) -C xg
-gx/gx_all.a:
+gx/gx_all.a: FORCE
 	$(MAKE) -C gx
diff --git a/tools/debugger/gdbsx/gx/Makefile b/tools/debugger/gdbsx/gx/Makefile
index 3b8467f799..ff5c8e9e6e 100644
--- a/tools/debugger/gdbsx/gx/Makefile
+++ b/tools/debugger/gdbsx/gx/Makefile
@@ -2,21 +2,20 @@  XEN_ROOT = $(CURDIR)/../../../..
 include ../Rules.mk
 
 GX_OBJS := gx_comm.o gx_main.o gx_utils.o gx_local.o
-GX_HDRS := $(wildcard *.h)
+
+TARGETS := gx_all.a
 
 .PHONY: all
-all: gx_all.a
+all: $(TARGETS)
 
 .PHONY: clean
 clean:
-	rm -rf gx_all.a *.o .*.d
+	rm -f *.o $(TARGETS) $(DEPS_RM)
 
 .PHONY: distclean
 distclean: clean
 
-#%.o: %.c $(GX_HDRS) Makefile
-#	$(CC) -c $(CFLAGS) -o $@ $<
-
-gx_all.a: $(GX_OBJS) Makefile $(GX_HDRS)
-	ar cr $@ $(GX_OBJS)        # problem with ld using -m32 
+gx_all.a: $(GX_OBJS) Makefile
+	ar cr $@ $(GX_OBJS)
 
+-include $(DEPS_INCLUDE)
diff --git a/tools/debugger/gdbsx/xg/Makefile b/tools/debugger/gdbsx/xg/Makefile
index acdcddf0d5..a02c7649cf 100644
--- a/tools/debugger/gdbsx/xg/Makefile
+++ b/tools/debugger/gdbsx/xg/Makefile
@@ -1,35 +1,24 @@ 
 XEN_ROOT = $(CURDIR)/../../../..
 include ../Rules.mk
 
-XG_HDRS := xg_public.h 
 XG_OBJS := xg_main.o 
 
 CFLAGS += -D__XEN_TOOLS__
 CFLAGS += $(CFLAGS_xeninclude)
 
+TARGETS := xg_all.a
 
 .PHONY: all
-all: build
+all: $(TARGETS)
 
-.PHONY: build
-build: xg_all.a $(XG_HDRS) $(XG_OBJS) Makefile
-# build: mk-symlinks xg_all.a $(XG_HDRS) $(XG_OBJS) Makefile
-# build: mk-symlinks xg_all.a
-
-xg_all.a: $(XG_OBJS) Makefile $(XG_HDRS)
-	ar cr $@ $(XG_OBJS)    # problems using -m32 in ld 
-#	$(LD) -b elf32-i386 $(LDFLAGS) -r -o $@ $^
-#	$(CC) -m32 -c -o $@ $^
-
-# xg_main.o: xg_main.c Makefile $(XG_HDRS)
-#$(CC) -c $(CFLAGS) -o $@ $<
-
-# %.o: %.c $(XG_HDRS) Makefile  -- doesn't work as it won't overwrite Rules.mk
-#%.o: %.c       -- doesn't recompile when .c changed
+xg_all.a: $(XG_OBJS) Makefile
+	ar cr $@ $(XG_OBJS)
 
 .PHONY: clean
 clean:
-	rm -rf xen xg_all.a $(XG_OBJS)  .*.d
+	rm -f $(TARGETS) $(XG_OBJS) $(DEPS_RM)
 
 .PHONY: distclean
 distclean: clean
+
+-include $(DEPS_INCLUDE)