mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* RE: [kbuild-devel] RfC: Don't cd into subdirs during kbuild
@ 2002-10-03 20:33 Grover, Andrew
  0 siblings, 0 replies; 3+ messages in thread
From: Grover, Andrew @ 2002-10-03 20:33 UTC (permalink / raw)
  To: 'Kai Germaschewski', kbuild-devel, linux-kernel

> From: Kai Germaschewski [mailto:kai-germaschewski@uiowa.edu] 
> ChangeSet@1.677, 2002-10-02 14:46:16-05:00, kai@tp1.ruhr-uni-bochum.de
>   kbuild: Standardize ACPI Makefiles
>   
>   ACPI was a bit lazy and just said compile all .c files in 
> this directory,
>   which is different from all other Makefiles and will not work very
>   well e.g. bk, where a .c file may not be checked out yet, 
> or separate
>   obj/src dirs. So just explicitly list the files we want to compile.

Excellent, I was just going to fix this, and now I don't have to ;-)

-- Andy

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [kbuild-devel] RfC: Don't cd into subdirs during kbuild
  2002-10-03  5:18 ` [kbuild-devel] " Peter Samuelson
@ 2002-10-03 14:54   ` Kai Germaschewski
  0 siblings, 0 replies; 3+ messages in thread
From: Kai Germaschewski @ 2002-10-03 14:54 UTC (permalink / raw)
  To: Peter Samuelson; +Cc: kbuild-devel, linux-kernel

On Thu, 3 Oct 2002, Peter Samuelson wrote:

> Which top dir, src or obj?  Most end users will expect obj topdir.
> More on that below.

Yes, I think obj topdir is the way to go - and you're right, it can be 
made work mostly with vpath and does not need much more.

> > So gcc/ld/.. are now called from the topdir, allowing to
> > closer resemble a non-recursive build.
> 
> I still think it's more intuitive to have $(obj) == '.', but you do
> make some good arguments in the cset comment.

The most important argument is that a none-recursive make of course cannot 
change dirs, so there it is definitely needed to have the right paths 
relative to the top dir. However, that's not really relevant, since we're 
not doing a none-recursive make (though it's a step into that direction).

> > -	@$(MAKE) -C $(patsubst _sfdep_%, %, $@) fastdep
> > +	@$(call descend,$(patsubst _sfdep_%,%,$@),fastdep)
> >  
> >  else # !CONFIG_MODVERSIONS
> >  
> > @@ -533,7 +526,7 @@
> >  
> >  .PHONY: $(patsubst %, _modinst_%, $(SUBDIRS))
> >  $(patsubst %, _modinst_%, $(SUBDIRS)) :
> > -	@$(MAKE) -C $(patsubst _modinst_%, %, $@) modules_install
> > +	$(descend,$(patsubst _modinst_%,%,$@),modules_install)
> 
> 	$(call descend,
> for consistency.

Duh, I found that during testing. - Looks like I forgot checking in my 
last cset.

> 
> > -fastdep: sub_dirs
> > -	@echo -n
> > +fastdep: $(subdir-ym)
> > +	@/bin/true
> 
> 	@:
> Seriously.  You can assume ':' is built in to your /bin/sh.  If not,
> you've got bigger problems, like perhaps a /bin/sh -> csh link.

Alright, if you're sure about this, send me a patch (later).

> Yeah, I always felt this was "too clever for its own good".  Same
> category as fs/smbfs/proto.h...
> 
> >  $(obj)/devlist.h: $(src)/pci.ids $(obj)/gen-devlist
> > -	$(obj)/gen-devlist < $<
> > +	( cd $(obj); ./gen-devlist ) < $<
> 
> 	cd $(obj); ./gen-devlist < $<
> 
> No need for parentheses - $(src) is an absolute path.

No, $(src) is a relative path - I really don't want to use an absolute 
path anywhere anymore.

> Really gen-devlist should output to stdout instead of a fixed file.

That's what I was thinking, but actually it generates two files at a time, 
so that wouldn't quite work.

> >  lxdialog:
> > -	$(MAKE) -C lxdialog all
> > +	$(call descend,lxdialog,)
> 
> 	$(call descend,scripts/lxdialog,)
> I didn't actually test this but isn't $(descend) relative to
> $(TOPDIR)?  At least in your later changeset.

You're right. Fixed.

> > diff -Nru a/drivers/isdn/i4l/isdn_ppp.h b/drivers/isdn/i4l/isdn_ppp.h
> > --- a/drivers/isdn/i4l/isdn_ppp.h	Wed Oct  2 21:52:06 2002
> > +++ b/drivers/isdn/i4l/isdn_ppp.h	Wed Oct  2 21:52:06 2002
> > @@ -27,7 +27,7 @@
> >  #else
> >  
> >  static inline int
> > -isdn_ppp_xmit(struct sk_buff *, struct net_device *);
> > +isdn_ppp_xmit(struct sk_buff *, struct net_device *)
> 
> Obviously correct, but was that supposed to be part of your changeset?
> I suppose it relates to "kbuild" in that "I can't compile my
> kernel". (:

Grrh. That happens to me all the time...

> > +# Add subdir path
> > +
> > +EXTRA_TARGETS	:= $(addprefix $(obj)/,$(EXTRA_TARGETS))
> > +obj-y		:= $(addprefix $(obj)/,$(obj-y))
> > +obj-m		:= $(addprefix $(obj)/,$(obj-m))
> > +export-objs	:= $(addprefix $(obj)/,$(export-objs))
> > +subdir-obj-y	:= $(addprefix $(obj)/,$(subdir-obj-y))
> > +real-objs-y	:= $(addprefix $(obj)/,$(real-objs-y))
> > +real-objs-m	:= $(addprefix $(obj)/,$(real-objs-m))
> > +multi-used-y	:= $(addprefix $(obj)/,$(multi-used-y))
> > +multi-used-m	:= $(addprefix $(obj)/,$(multi-used-m))
> > +multi-objs-y	:= $(addprefix $(obj)/,$(multi-objs-y))
> > +multi-objs-m	:= $(addprefix $(obj)/,$(multi-objs-m))
> > +subdir-ym	:= $(addprefix $(obj)/,$(subdir-ym))
> 
> add_obj_prefix = $1 := $(addprefix $(obj)/,$($1))
> $(call add_obj_prefix EXTRA_TARGETS)

I don't think that works - make is not always pretty, that's life.

> Hey, why not make life exciting.  Don't ifndef the O_TARGET.  Just
> break the existing O_TARGET users.

I added a $(warning ) for this case somewhere above. I'll break it 
eventually ;)

>    ACPI_CFLAGS	:= -D_LINUX -I$(src)/include
> 
> or do you require '.' to be source TOPDIR?  If so, this is the
> opposite of what most projects do.  Normally the user expects to build
> from the top of $(obj).  Why not do the same here, even if it's a bit
> more work?

I'm planning on doing the same. I think for $(src) != $(obj), I'll do
a $(patsubst -I%,-I% -I$(src)/%) on the [AC]FLAGS. (BTW, in this case, of
course $(src) would probably be an absolute path). We also have generated 
files which need to be included, so we basically need both $(src) and 
$(obj) in the include path.

> > -EXTRA_CFLAGS += -I. ${MPT_CFLAGS}
> > +EXTRA_CFLAGS += ${MPT_CFLAGS}
> 
> So can we assume -I$(src) is always part of the cflags?

I think (need to check), gcc automatically seaches the path of the source 
for includes. If not we'll have to add it.

Anyway, the case above worked fine without the -I.

> > +++ b/drivers/net/sk98lin/Makefile	Wed Oct  2 21:52:10 2002
> > @@ -55,7 +55,7 @@
> >  # SK_DBGCAT_DRV_INT_SRC         0x04000000      interrupts sources
> >  # SK_DBGCAT_DRV_EVENT           0x08000000      driver events
> >  
> > -EXTRA_CFLAGS += -I. -DSK_USE_CSUM $(DBGDEF)
> > +EXTRA_CFLAGS += -Idrivers/net/sk98lin -DSK_USE_CSUM $(DBGDEF)
> 
> ...because if so, this isn't needed.

In this case someones including "h/header.h", and this did not work 
without the -I.

Well, you see, some issues still need tackling for $(src) != $(obj), but
that's not what I was aiming for with this patch, anyway.

--Kai


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [kbuild-devel] RfC: Don't cd into subdirs during kbuild
  2002-10-03  2:59 Kai Germaschewski
@ 2002-10-03  5:18 ` Peter Samuelson
  2002-10-03 14:54   ` Kai Germaschewski
  0 siblings, 1 reply; 3+ messages in thread
From: Peter Samuelson @ 2002-10-03  5:18 UTC (permalink / raw)
  To: Kai Germaschewski; +Cc: kbuild-devel, linux-kernel


Random thoughts.  I haven't actually tested the patch yet, so feel
free to ignore me. (:


[Kai Germaschewski]
> i.e. the current working directory remains the top dir for all
> times.

Which top dir, src or obj?  Most end users will expect obj topdir.
More on that below.

> So gcc/ld/.. are now called from the topdir, allowing to
> closer resemble a non-recursive build.

I still think it's more intuitive to have $(obj) == '.', but you do
make some good arguments in the cset comment.

> -	@$(MAKE) -C $(patsubst _sfdep_%, %, $@) fastdep
> +	@$(call descend,$(patsubst _sfdep_%,%,$@),fastdep)
>  
>  else # !CONFIG_MODVERSIONS
>  
> @@ -533,7 +526,7 @@
>  
>  .PHONY: $(patsubst %, _modinst_%, $(SUBDIRS))
>  $(patsubst %, _modinst_%, $(SUBDIRS)) :
> -	@$(MAKE) -C $(patsubst _modinst_%, %, $@) modules_install
> +	$(descend,$(patsubst _modinst_%,%,$@),modules_install)

	$(call descend,
for consistency.

> -fastdep: sub_dirs
> -	@echo -n
> +fastdep: $(subdir-ym)
> +	@/bin/true

	@:
Seriously.  You can assume ':' is built in to your /bin/sh.  If not,
you've got bigger problems, like perhaps a /bin/sh -> csh link.

> -# we also have source in the subdirectories..
> -vpath %.c =  . linux pagebuf support

Good riddance!  This is worse than $(MAKEFILES) abuse. (:

>  # Objects in pagebuf/
> -xfs-objs			+= page_buf.o \
> -				   page_buf_locking.o
> +xfs-objs			+= $(addprefix pagebuf/, \
> +				   page_buf.o \
> +				   page_buf_locking.o)
>  # Objects in linux/
>  # Objects in support/

Go ahead and delete those comments as well.

>   ACPI was a bit lazy and just said compile all .c files in this directory,

Yeah, I always felt this was "too clever for its own good".  Same
category as fs/smbfs/proto.h...

>  $(obj)/devlist.h: $(src)/pci.ids $(obj)/gen-devlist
> -	$(obj)/gen-devlist < $<
> +	( cd $(obj); ./gen-devlist ) < $<

	cd $(obj); ./gen-devlist < $<

No need for parentheses - $(src) is an absolute path.

Really gen-devlist should output to stdout instead of a fixed file.

>  $(obj)/devlist.h: $(src)/zorro.ids $(obj)/gen-devlist
> -	$(obj)/gen-devlist < $<
> +	( cd $(obj); ./gen-devlist ) < $<

ditto

>  lxdialog:
> -	$(MAKE) -C lxdialog all
> +	$(call descend,lxdialog,)

	$(call descend,scripts/lxdialog,)
I didn't actually test this but isn't $(descend) relative to
$(TOPDIR)?  At least in your later changeset.

> diff -Nru a/drivers/isdn/i4l/isdn_ppp.h b/drivers/isdn/i4l/isdn_ppp.h
> --- a/drivers/isdn/i4l/isdn_ppp.h	Wed Oct  2 21:52:06 2002
> +++ b/drivers/isdn/i4l/isdn_ppp.h	Wed Oct  2 21:52:06 2002
> @@ -27,7 +27,7 @@
>  #else
>  
>  static inline int
> -isdn_ppp_xmit(struct sk_buff *, struct net_device *);
> +isdn_ppp_xmit(struct sk_buff *, struct net_device *)

Obviously correct, but was that supposed to be part of your changeset?
I suppose it relates to "kbuild" in that "I can't compile my
kernel". (:

> +# Add subdir path
> +
> +EXTRA_TARGETS	:= $(addprefix $(obj)/,$(EXTRA_TARGETS))
> +obj-y		:= $(addprefix $(obj)/,$(obj-y))
> +obj-m		:= $(addprefix $(obj)/,$(obj-m))
> +export-objs	:= $(addprefix $(obj)/,$(export-objs))
> +subdir-obj-y	:= $(addprefix $(obj)/,$(subdir-obj-y))
> +real-objs-y	:= $(addprefix $(obj)/,$(real-objs-y))
> +real-objs-m	:= $(addprefix $(obj)/,$(real-objs-m))
> +multi-used-y	:= $(addprefix $(obj)/,$(multi-used-y))
> +multi-used-m	:= $(addprefix $(obj)/,$(multi-used-m))
> +multi-objs-y	:= $(addprefix $(obj)/,$(multi-objs-y))
> +multi-objs-m	:= $(addprefix $(obj)/,$(multi-objs-m))
> +subdir-ym	:= $(addprefix $(obj)/,$(subdir-ym))

add_obj_prefix = $1 := $(addprefix $(obj)/,$($1))
$(call add_obj_prefix EXTRA_TARGETS)

or something like that?

>  ifndef O_TARGET
>  ifndef L_TARGET
> -O_TARGET := built-in.o
> +O_TARGET := $(obj)/built-in.o
> +endif
>  endif
> +
> +ifdef L_TARGET
> +L_TARGET := $(obj)/$(L_TARGET)
>  endif

Hey, why not make life exciting.  Don't ifndef the O_TARGET.  Just
break the existing O_TARGET users.

> +host-progs-single     := $(addprefix $(obj)/,$(host-progs-single))
> +host-progs-multi      := $(addprefix $(obj)/,$(host-progs-multi))
> +host-progs-multi-objs := $(addprefix $(obj)/,$(host-progs-multi-objs))

$(call add_obj_prefix host-progs-single)

> +ifeq ($(KBUILD_VERBOSE),1)
> +descend = echo '$(MAKE) -f $(1)/Makefile $(2)';
> +endif
> +descend += $(MAKE) -f $(1)/Makefile obj=$(1) $(2)

Heh, clever.

> -ACPI_CFLAGS	:= -D_LINUX -I$(CURDIR)/include
> +ACPI_CFLAGS	:= -D_LINUX -Idrivers/acpi/include

   ACPI_CFLAGS	:= -D_LINUX -I$(src)/include

or do you require '.' to be source TOPDIR?  If so, this is the
opposite of what most projects do.  Normally the user expects to build
from the top of $(obj).  Why not do the same here, even if it's a bit
more work?

> +++ b/drivers/ide/arm/Makefile	Wed Oct  2 21:52:10 2002
> @@ -2,6 +2,6 @@
>  obj-$(CONFIG_BLK_DEV_IDE_ICSIDE)	+= icside.o
>  obj-$(CONFIG_BLK_DEV_IDE_RAPIDE)	+= rapide.o
>  
> -EXTRA_CFLAGS	:= -I../
> +EXTRA_CFLAGS	:= -Idrivers/ide

again (and 3 more cases below)..

> +++ b/drivers/message/fusion/Makefile	Wed Oct  2 21:52:10 2002
> @@ -13,7 +13,7 @@
>  #			# sparc64
>  #EXTRA_CFLAGS += -gstabs+
>  
> -EXTRA_CFLAGS += -I. ${MPT_CFLAGS}
> +EXTRA_CFLAGS += ${MPT_CFLAGS}

So can we assume -I$(src) is always part of the cflags?

> +++ b/drivers/net/sk98lin/Makefile	Wed Oct  2 21:52:10 2002
> @@ -55,7 +55,7 @@
>  # SK_DBGCAT_DRV_INT_SRC         0x04000000      interrupts sources
>  # SK_DBGCAT_DRV_EVENT           0x08000000      driver events
>  
> -EXTRA_CFLAGS += -I. -DSK_USE_CSUM $(DBGDEF)
> +EXTRA_CFLAGS += -Idrivers/net/sk98lin -DSK_USE_CSUM $(DBGDEF)

...because if so, this isn't needed.

> +++ b/drivers/net/skfp/Makefile	Wed Oct  2 21:52:10 2002
> @@ -17,7 +17,7 @@
>  #   projects. To keep the source common for all those drivers (and
>  #   thus simplify fixes to it), please do not clean it up!
>  
> -EXTRA_CFLAGS += -I. -DPCI -DMEM_MAPPED_IO -Wno-strict-prototypes 
> +EXTRA_CFLAGS += -Idrivers/net/skfp -DPCI -DMEM_MAPPED_IO -Wno-strict-prototypes 

...or this.

> +++ b/drivers/usb/storage/Makefile	Wed Oct  2 21:52:10 2002
> @@ -5,7 +5,7 @@
>  # Rewritten to use lists instead of if-statements.
>  #
>  
> -EXTRA_CFLAGS	:= -I../../scsi/
> +EXTRA_CFLAGS	:= -Idrivers/scsi

once again is '.' always directly below $(src) or below $(obj)?

> -# This needs -I. because everything does #include <xfs.h> instead of "xfs.h".
> +# This needs -I because everything does #include <xfs.h> instead of "xfs.h".

This implies that -I$(src) is *not* implicit - right?

I hope this is intended to be temporary - xfs should just be changed
to #include "" like everybody else.

Peter

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2002-10-03 20:27 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2002-10-03 20:33 [kbuild-devel] RfC: Don't cd into subdirs during kbuild Grover, Andrew
  -- strict thread matches above, loose matches on Subject: below --
2002-10-03  2:59 Kai Germaschewski
2002-10-03  5:18 ` [kbuild-devel] " Peter Samuelson
2002-10-03 14:54   ` Kai Germaschewski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®