[PATCH] make it possible to keep Config/Kbuild snippets in *.c files

Sun May 9 17:06:45 UTC 2010

On Sunday 09 May 2010 09:19:58 Denys Vlasenko wrote:
> On Sunday 09 May 2010 07:31, Rob Landley wrote:
> > I note that CONFIG_APPNAME += appname.o is the common case, which could
> > presumably be generated automatically for 90% of the commands.  (You only
> > really need to specify something like this when there's more than one .o
> > file, or if the .o file is a different name...)
>
> Yes, de do have those 10%
>
> > Again, this looks like a default case.  If you're going to do this for
> > more than one directory, possibly this should be emitted by the script
> > rather than appended to, and only non-default stuff should go in here.
> >
> > The Config.src file had a reason to exist because it was naming the menu,
> > but this one isn't actually doing anything.
>
> applets/ and archival/ have non-trivial Kbuilds

Oh sure.  There would need to be a way to override the default case.  (Perhaps 
if there are no kbuild lines provide the default, if there are any don't.)

> > Also, since these files are generated you don't really need one per
> > directory. You could just make one big one.  (Which brings us back to the
> > generated directory, mentioned earlier.)
> >
> > Still, that sort of thing could easily be done in stages.  (Cleanup on
> > top of cleanup, this is fine for now...)  The important thing is getting
> > the syntax that goes into the .c files right, so that conversion doesn't
> > have to be redone.
>
> Right.
>
> > This rebuilds every time.  Wouldn't it be better to do dependencies on
> > the .c files, ala:
> >
> >   gen_build_files: findutils/*.c
>
> Tried that:
>
> gen_build_files: $(wildcard */*.c)
>
> It attempted to rebuild a host tool:
>
> $ make
>   HOSTCC  scripts/basic/fixdep
>   HOSTCC  scripts/basic/split-include
>   HOSTCC  scripts/basic/docproc
>   HOSTCC  applets/usage
> applets/usage.c:11:22: error: autoconf.h: No such file or directory
> In file included from applets/usage.c:27:
> include/applets.h:70: error: expected ',' or ';' before 'IF_TEST'
> make[2]: *** [applets/usage] Error 1
> make[1]: *** [applets_dir] Error 2
> make: *** [include/autoconf.h] Error 2

That's weird.

A) Why is it doing that?  The *.c file exists.  Presumably some other makefile 
is indicating a strange prereqisite here that's getting triggered.

B) The applets directory hasn't actually got any applets in it, so the 
dependency should exclude it.  Using */*.c is fiddly because libbb and the 
testsite and docs and examples and so on don't have applets in them.  The 
build logic for those other directories is who knows what, and those are most 
likely to cause trouble.

Probably there should be some kind of APPLET_DIRS.  I note that the top level 
Makefile is setting libs-y to more or less the directory list we want, probably 
what needs to happen is that list should first be saved in a variable 
(containing all those dirs except libbb), and then libs-y should be set to 
that variable (plus libbb).

Does that sound reasonable?

> > >  # To make sure we do not include .config for any of the *config
> > > targets # catch them early, and hand them over to
> > > scripts/kconfig/Makefile # It is allowed to specify more targets when
> > > calling make, including @@ -428,7 +433,7 @@ ifeq ($(config-targets),1)
> > >  -include $(srctree)/arch/$(ARCH)/Makefile
> > >  export KBUILD_DEFCONFIG
> > >
> > > -config %config: scripts_basic outputmakefile FORCE
> > > +config %config: scripts_basic outputmakefile gen_build_files FORCE
> > >         $(Q)mkdir -p include
> > >         $(Q)$(MAKE) $(build)=scripts/kconfig $@
> > >         $(Q)$(MAKE) -C $(srctree) KBUILD_SRC= .kernelrelease
> > > @@ -443,7 +448,7 @@ ifeq ($(KBUILD_EXTMOD),)
> > >  # Carefully list dependencies so we do not try to build scripts twice
> > >  # in parrallel
> > >  PHONY += scripts
> > > -scripts: scripts_basic include/config/MARKER
> > > +scripts: gen_build_files scripts_basic include/config/MARKER
> > >         $(Q)$(MAKE) $(build)=$(@)
> > >
> > >  scripts_basic: include/autoconf.h
> >
> > It seems like that first hunk would have been sufficient, but the busybox
> > makefiles have always been a bit overcomplicated, haven't they?
>
> (1) I don't know.
> (2) Yes.

I hate makefiles.  That's why I never got around to doing a serious crapectomy 
on the busybox ones.  (For my own projects, I tend to just have a shell script 
drive the build...)

Still, it's a lot better than it was.  The "30 seconds to figure out there's 
nothing to do" days are gone, as is Vladimir's amazing segfaulting dependency 
generator...

> > > diff -ad -urpN busybox.0/scripts/gen_build_files.sh
> > > busybox.1/scripts/gen_build_files.sh ---
> > > busybox.0/scripts/gen_build_files.sh        1970-01-01
> > > 01:00:00.000000000 +0100 +++ busybox.1/scripts/gen_build_files.sh      
> > >  2010-05-09 03:58:09.000000000 +0200 @@ -0,0 +1,51 @@
> > > +#!/bin/sh
> > > +
> > > +test $# -ge 2 || exit 1
> > > +
> >
> > Might want to emit a usage message there.  Just a thought.
>
> Ok. New version of the script:
>
>
> #!/bin/sh
>
> test $# -ge 2 || { echo "Syntax: $0 SRCTREE OBJTREE"; exit 1; }
>
> # cd to objtree
> cd "$2" || { echo "Syntax: $0 SRCTREE OBJTREE"; exit 1; }
>
> srctree="$1"
>
> find -type d | while read; do
>     d="$REPLY"
>
>     src="$srctree/$d/Kbuild.src"
>     dst="$d/Kbuild"
>     if test -f "$src"; then
> 	echo "  CHK     $dst"

Mixing spaces and tabs in the indent.

> 	s=`sed -n 's@^//kbuild:@@p' "$srctree/$d"/*.c`
> 	echo "# DO NOT EDIT. This file is generated from Kbuild.src"
> >"$dst.$$.tmp" while read; do
> 	    test x"$REPLY" = x"INSERT" && REPLY="$s"
> 	    printf "%s\n" "$REPLY"
> 	done <"$src" >>"$dst.$$.tmp"
>
> 	if test -f "$dst" && cmp -s "$dst.$$.tmp" "$dst"; then
> 	    rm "$dst.$$.tmp"
> 	else
> 	    echo "  GEN     $dst"
> 	    mv "$dst.$$.tmp" "$dst"
> 	fi
>     fi
>
>     src="$srctree/$d/Config.src"
>     dst="$d/Config.in"
>     if test -f "$src"; then
> 	echo "  CHK     $dst"
>
> 	s=`sed -n 's@^//config:@@p' "$srctree/$d"/*.c`
> 	echo "# DO NOT EDIT. This file is generated from Config.src"
> >"$dst.$$.tmp" while read; do
> 	    test x"$REPLY" = x"INSERT" && REPLY="$s"
> 	    printf "%s\n" "$REPLY"
> 	done <"$src" >>"$dst.$$.tmp"
>
> 	if test -f "$dst" && cmp -s "$dst.$$.tmp" "$dst"; then
> 	    rm "$dst.$$.tmp"
> 	else
> 	    echo "  GEN     $dst"
> 	    mv "$dst.$$.tmp" "$dst"
> 	fi
>     fi
>
> done
>
> # Last read failed. This is normal. Don't exit with its error code:
> exit 0

Looks reasonable.  More cleanup passes can always occur later...

Rob
-- 
Latency is more important than throughput. It's that simple. - Linus Torvalds