[libvirt] [PATCH] maint: ensure src/ directory includes are clean

[Eric Blake]

On 03/25/2014 02:14 PM, Eric Blake wrote:
> In 'make syntax-check', we have a rule that prevents layering
> violations between the various files in src.  However, we
> forgot to treat conf/ and the more recently-added access/ as
> lower-level directories, and were not detecting cases where
> they might have used a driver file.  Also, it's not nice that
> qemu can use storage/ but none of the other drivers could do so.
> 
> * cfg.mk (sc_prohibit_cross_inclusion): Tighten rules for conf/
> and access/, let all other drivers use storage/.
> 
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
> 
> I noticed this because of my work on domain_conf.h: I want to share
> a struct between util/virstoragefile and conf/domain_conf, and ran
> into a syntax check when I tried to make util/ depend on conf/.  I
> fixed things to obey layering with conf/ depending on util/, but in
> the process noticed that some layering violations went undetected.

Looks like I pushed this without a review, as part of my virstoragefile
patches.  I also noticed that the rule isn't catching all violations; it
catches cases of #include "conf/domain_conf.h" but not "#include
"domain_conf.h".  So to make it even more useful, it's probably worth an
audit of the code base to remove things like '-I util' and '-I conf'
from Makefile.am, as well as rewrite all includes to explicitly mention
which directory they are expecting to pull headers from, so that
cross-directory includes are more obvious.  Looks like a bigger cleanup,
so more patches on the way, although not currently my highest priority.

> +++ b/cfg.mk
> @@ -760,17 +760,17 @@ sc_prohibit_gettext_markup:
>  # lower-level code must not include higher-level headers.
>  cross_dirs=$(patsubst $(srcdir)/src/%.,%,$(wildcard $(srcdir)/src/*/.))
>  cross_dirs_re=($(subst / ,/|,$(cross_dirs)))
> +mid_dirs=access|conf|cpu|locking|network|node_device|rpc|security|storage
>  sc_prohibit_cross_inclusion:
>  	@for dir in $(cross_dirs); do					\
>  	  case $$dir in							\
>  	    util/) safe="util";;					\
> -	    locking/)							\
> -	      safe="($$dir|util|conf|rpc)";;				\
> -	    cpu/ | locking/ | network/ | rpc/ | security/)		\
> +	    access/ | conf/) safe="($$dir|conf|util)";;			\
> +	    locking/) safe="($$dir|util|conf|rpc)";;			\
> +	    cpu/| network/| node_device/| rpc/| security/| storage/)	\
>  	      safe="($$dir|util|conf)";;				\
>  	    xenapi/ | xenxs/ ) safe="($$dir|util|conf|xen)";;		\
> -	    qemu/ ) safe="($$dir|util|conf|cpu|network|locking|rpc|security|storage)";; \
> -	    *) safe="($$dir|util|conf|cpu|network|locking|rpc|security)";; \
> +	    *) safe="($$dir|$(mid_dirs)|util)";;			\
>  	  esac;								\
>  	  in_vc_files="^src/$$dir"					\
>  	  prohibit='^# *include .$(cross_dirs_re)'			\
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 604 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140401/f0bc9d16/attachment-0001.sig>