[libvirt] [PATCH] maint: ensure src/ directory includes are clean
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@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. cfg.mk | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cfg.mk b/cfg.mk index a4ae978..19537cf 100644 --- a/cfg.mk +++ 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)' \ -- 1.8.5.3
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@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
participants (1)
-
Eric Blake