diff mbox

[for-4.9,2/2] Makefile: Regularise subdir targets and their dependencies

Message ID 1495642470-1079-2-git-send-email-ian.jackson@eu.citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ian Jackson May 24, 2017, 4:14 p.m. UTC
Recent changes to this Makefile have broken some build targets, and
some parallel builds.

Looking at it, I think I have identified the undocumented design
intent in the top-level Makefile.  So in this patch I document it, and
also make it true.

In detail:

 * Add a comment with the new design intent
 * Get rid of the ad-hoc rules for recursing into tools/include,
   and replace them with a pattern rule
 * Add an appropriate dependency on TARGET-tools-public-headers from
   TARGET-tools and TARGET-stubdom (but not dist-*).
 * Get rid of all the separate invocations of $(MAKE) -C tools/include
   which are now obsolete
 * Un-deprecate the simple `tools' etc. targets (aliases for `dist-tools')
   which we seem not to be making any effort to get rid of

I have verified with the following shell script that after my change,
the tree producese the same results for various build targets as
3fafdc28eb98 (before the Makefile-hacking started).

My tests failed as expected for make -C tools, both before and after.

Separately, there is a bug in the Makefiles that `make distclean-tools'
fails.  I have not investigated that bug in detail.

    #!/bin/bash

    set -e
    set -o pipefail

    listings=../listings

    rm -rf $listings
    mkdir $listings

    chks () {
         reskey="C$subdir $*"
         reskey="${reskey// /_}"
         reskey="${reskey//\//:}"
         lk=$listings/$reskey
         for suffix in '' -xen -tools -stubdom -docs; do
             case "$subdir:$suffix" in
             .:*) ;;
             *:) ;;
             *) continue;;
             esac
             git clean -qxdff
             rm -rf $output
             printf '%s' "running -C$subdir suffix=$suffix "
             case "$subdir $suffix" in
             *xen*) ;;
             *) printf 'configure '; ./configure >$lk.cfg 2>&1 ;;
             esac
             fail=''
             for targ in $*; do
                 realtarg=$targ$suffix
                 printf '%s ' "$realtarg"
                 if ! make -C $subdir -j10 $realtarg >${lk}_${realtarg}.log 2>&1
                 then
                    fail=$realtarg
                    break
                 fi
             done
             if [ "$fail" ]; then
               echo fail!
               echo "$fail failed" >$lk.list
             else
               echo ok.
               (test ! -e "$output" || find $output) |sort >$lk.list
             fi
        done
    }

    subdirs='. xen docs tools'

    output=$PWD/dist
    for subdir in $subdirs; do
        chks build clean distclean
    done

    output=$PWD/dist
    subdir=.
    chks dist

    export DESTDIR=$PWD/destdir
    output=$PWD/destdir
    for subdir in $subdirs; do
        chks install
    done

And the output:

    (64)iwj@mariner:~/work/xen.git$ ~/junk/chks
    running -C. suffix= configure build clean distclean ok.
    running -C. suffix=-xen build-xen clean-xen distclean-xen ok.
    running -C. suffix=-tools configure build-tools clean-tools distclean-tools fail!
    running -C. suffix=-stubdom configure build-stubdom clean-stubdom distclean-stubdom ok.
    running -C. suffix=-docs configure build-docs clean-docs distclean-docs ok.
    running -Cxen suffix= build clean distclean ok.
    running -Cdocs suffix= configure build clean distclean ok.
    running -Ctools suffix= configure build fail!
    running -C. suffix= configure dist ok.
    running -C. suffix=-xen dist-xen ok.
    running -C. suffix=-tools configure dist-tools ok.
    running -C. suffix=-stubdom configure dist-stubdom ok.
    running -C. suffix=-docs configure dist-docs ok.
    running -C. suffix= configure install ok.
    running -C. suffix=-xen install-xen ok.
    running -C. suffix=-tools configure install-tools ok.
    running -C. suffix=-stubdom configure install-stubdom ok.
    running -C. suffix=-docs configure install-docs ok.
    running -Cxen suffix= install ok.
    running -Cdocs suffix= configure install ok.
    running -Ctools suffix= configure install fail!
    (64)iwj@mariner:~/work/xen.git$

CC: Julien Grall <julien.grall@arm.com>
CC: M A Young <m.a.young@durham.ac.uk>
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 Makefile | 39 +++++++++++++++++++++++----------------
 1 file changed, 23 insertions(+), 16 deletions(-)

Comments

Andrew Cooper May 24, 2017, 5:14 p.m. UTC | #1
On 24/05/17 17:14, Ian Jackson wrote:
> CC: Julien Grall <julien.grall@arm.com>
> CC: M A Young <m.a.young@durham.ac.uk>
> CC: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

This resolves all the XenServer build issues I encountered.

Tested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Michael Young May 24, 2017, 9:42 p.m. UTC | #2
On Wed, 24 May 2017, Andrew Cooper wrote:

> On 24/05/17 17:14, Ian Jackson wrote:
>> CC: Julien Grall <julien.grall@arm.com>
>> CC: M A Young <m.a.young@durham.ac.uk>
>> CC: Andrew Cooper <andrew.cooper3@citrix.com>
>> CC: Wei Liu <wei.liu2@citrix.com>
>> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
>
> This resolves all the XenServer build issues I encountered.
>
> Tested-by: Andrew Cooper <andrew.cooper3@citrix.com>

I tried building with both patches and it works for me as well.

 	Michael Young
Julien Grall May 25, 2017, 12:15 p.m. UTC | #3
On 24/05/17 18:14, Andrew Cooper wrote:
> On 24/05/17 17:14, Ian Jackson wrote:
>> CC: Julien Grall <julien.grall@arm.com>
>> CC: M A Young <m.a.young@durham.ac.uk>
>> CC: Andrew Cooper <andrew.cooper3@citrix.com>
>> CC: Wei Liu <wei.liu2@citrix.com>
>> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
>
> This resolves all the XenServer build issues I encountered.
>
> Tested-by: Andrew Cooper <andrew.cooper3@citrix.com>

Release-acked-by: Julien Grall <julien.grall@arm.com>

>
Roger Pau Monne May 25, 2017, 2:05 p.m. UTC | #4
On Wed, May 24, 2017 at 05:14:30PM +0100, Ian Jackson wrote:
> Recent changes to this Makefile have broken some build targets, and
> some parallel builds.
> 
> Looking at it, I think I have identified the undocumented design
> intent in the top-level Makefile.  So in this patch I document it, and
> also make it true.
> 
> In detail:
> 
>  * Add a comment with the new design intent
>  * Get rid of the ad-hoc rules for recursing into tools/include,
>    and replace them with a pattern rule
>  * Add an appropriate dependency on TARGET-tools-public-headers from
>    TARGET-tools and TARGET-stubdom (but not dist-*).
>  * Get rid of all the separate invocations of $(MAKE) -C tools/include
>    which are now obsolete
>  * Un-deprecate the simple `tools' etc. targets (aliases for `dist-tools')
>    which we seem not to be making any effort to get rid of
> 
> I have verified with the following shell script that after my change,
> the tree producese the same results for various build targets as
> 3fafdc28eb98 (before the Makefile-hacking started).
> 
> My tests failed as expected for make -C tools, both before and after.
> 
> Separately, there is a bug in the Makefiles that `make distclean-tools'
> fails.  I have not investigated that bug in detail.
> 
>     #!/bin/bash
> 
>     set -e
>     set -o pipefail
> 
>     listings=../listings
> 
>     rm -rf $listings
>     mkdir $listings
> 
>     chks () {
>          reskey="C$subdir $*"
>          reskey="${reskey// /_}"
>          reskey="${reskey//\//:}"
>          lk=$listings/$reskey
>          for suffix in '' -xen -tools -stubdom -docs; do
>              case "$subdir:$suffix" in
>              .:*) ;;
>              *:) ;;
>              *) continue;;
>              esac
>              git clean -qxdff
>              rm -rf $output
>              printf '%s' "running -C$subdir suffix=$suffix "
>              case "$subdir $suffix" in
>              *xen*) ;;
>              *) printf 'configure '; ./configure >$lk.cfg 2>&1 ;;
>              esac
>              fail=''
>              for targ in $*; do
>                  realtarg=$targ$suffix
>                  printf '%s ' "$realtarg"
>                  if ! make -C $subdir -j10 $realtarg >${lk}_${realtarg}.log 2>&1
>                  then
>                     fail=$realtarg
>                     break
>                  fi
>              done
>              if [ "$fail" ]; then
>                echo fail!
>                echo "$fail failed" >$lk.list
>              else
>                echo ok.
>                (test ! -e "$output" || find $output) |sort >$lk.list
>              fi
>         done
>     }
> 
>     subdirs='. xen docs tools'
> 
>     output=$PWD/dist
>     for subdir in $subdirs; do
>         chks build clean distclean
>     done
> 
>     output=$PWD/dist
>     subdir=.
>     chks dist
> 
>     export DESTDIR=$PWD/destdir
>     output=$PWD/destdir
>     for subdir in $subdirs; do
>         chks install
>     done
> 
> And the output:
> 
>     (64)iwj@mariner:~/work/xen.git$ ~/junk/chks
>     running -C. suffix= configure build clean distclean ok.
>     running -C. suffix=-xen build-xen clean-xen distclean-xen ok.
>     running -C. suffix=-tools configure build-tools clean-tools distclean-tools fail!
>     running -C. suffix=-stubdom configure build-stubdom clean-stubdom distclean-stubdom ok.
>     running -C. suffix=-docs configure build-docs clean-docs distclean-docs ok.
>     running -Cxen suffix= build clean distclean ok.
>     running -Cdocs suffix= configure build clean distclean ok.
>     running -Ctools suffix= configure build fail!
>     running -C. suffix= configure dist ok.
>     running -C. suffix=-xen dist-xen ok.
>     running -C. suffix=-tools configure dist-tools ok.
>     running -C. suffix=-stubdom configure dist-stubdom ok.
>     running -C. suffix=-docs configure dist-docs ok.
>     running -C. suffix= configure install ok.
>     running -C. suffix=-xen install-xen ok.
>     running -C. suffix=-tools configure install-tools ok.
>     running -C. suffix=-stubdom configure install-stubdom ok.
>     running -C. suffix=-docs configure install-docs ok.
>     running -Cxen suffix= install ok.
>     running -Cdocs suffix= configure install ok.
>     running -Ctools suffix= configure install fail!
>     (64)iwj@mariner:~/work/xen.git$
> 
> CC: Julien Grall <julien.grall@arm.com>
> CC: M A Young <m.a.young@durham.ac.uk>
> CC: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

Acked-by: Roger Pau Monné <roger.pau@citrix.com>

Just a minor nit/question.

> ---
>  Makefile | 39 +++++++++++++++++++++++----------------
>  1 file changed, 23 insertions(+), 16 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index fc30b3c..51905eb 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -38,18 +38,13 @@ mini-os-dir-force-update: mini-os-dir
>  export XEN_TARGET_ARCH
>  export DESTDIR

Maybe it would be good to add a note like:

"All the Makefiles invoked with -C from the toplevel should have the
following targets: all, build, install, clean, distclean"

?

Roger.
diff mbox

Patch

diff --git a/Makefile b/Makefile
index fc30b3c..51905eb 100644
--- a/Makefile
+++ b/Makefile
@@ -38,18 +38,13 @@  mini-os-dir-force-update: mini-os-dir
 export XEN_TARGET_ARCH
 export DESTDIR
 
-.PHONY: build-tools-public-headers
-build-tools-public-headers:
-	$(MAKE) -C tools/include
-
-.PHONY: dist-tools-public-headers
-dist-tools-public-headers: build-tools-public-headers
-	$(MAKE) -C tools/include dist
+.PHONY: %-tools-public-headers
+%-tools-public-headers:
+	$(MAKE) -C tools/include $*
 
 # build and install everything into the standard system directories
 .PHONY: install
 install: $(TARGS_INSTALL)
-	$(MAKE) -C tools/include install
 
 .PHONY: build
 build: $(TARGS_BUILD)
@@ -80,7 +75,22 @@  build-docs:
 test:
 	$(MAKE) -C tools/python test
 
-# build and install everything into local dist directory
+# For most targets here,
+#   make COMPONENT-TARGET
+# is implemented, more or less, by
+#   make -C COMPONENT TARGET
+#
+# Each rule that does this needs to have dependencies on any
+# other COMPONENTs that have to be processed first.  See
+# The install-tools target here for an example.
+#
+# dist* targets are special: these do not occur in lower-level
+# Makefiles.  Instead, these are all implemented only here.
+# They run the appropriate install targets with DESTDIR set.
+#
+# Also, we have a number of targets COMPONENT which run
+# dist-COMPONENT, for convenience.
+
 .PHONY: dist
 dist: DESTDIR=$(DISTDIR)/install
 dist: $(TARGS_DIST) dist-misc
@@ -92,12 +102,10 @@  dist-misc:
 	$(INSTALL_PROG) ./install.sh $(DISTDIR)
 
 
-dist-tools: dist-tools-public-headers
 dist-%: DESTDIR=$(DISTDIR)/install
 dist-%: install-%
 	@: # do nothing
 
-# Legacy dist targets
 .PHONY: xen tools stubdom docs
 xen: dist-xen
 tools: dist-tools
@@ -109,11 +117,11 @@  install-xen:
 	$(MAKE) -C xen install
 
 .PHONY: install-tools
-install-tools: build-tools-public-headers
+install-tools: install-tools-public-headers
 	$(MAKE) -C tools install
 
 .PHONY: install-stubdom
-install-stubdom: mini-os-dir build-tools-public-headers
+install-stubdom: mini-os-dir install-tools-public-headers
 	$(MAKE) -C stubdom install
 ifeq (x86_64,$(XEN_TARGET_ARCH))
 	XEN_TARGET_ARCH=x86_32 $(MAKE) -C stubdom install-grub
@@ -180,18 +188,17 @@  src-tarball: subtree-force-update-all
 
 .PHONY: clean
 clean: $(TARGS_CLEAN)
-	$(MAKE) -C tools/include clean
 
 .PHONY: clean-xen
 clean-xen:
 	$(MAKE) -C xen clean
 
 .PHONY: clean-tools
-clean-tools:
+clean-tools: clean-tools-public-headers
 	$(MAKE) -C tools clean
 
 .PHONY: clean-stubdom
-clean-stubdom:
+clean-stubdom: clean-tools-public-headers
 	$(MAKE) -C stubdom crossclean
 ifeq (x86_64,$(XEN_TARGET_ARCH))
 	XEN_TARGET_ARCH=x86_32 $(MAKE) -C stubdom crossclean