[libvirt] [PATCH v2 1/2] util: switch over to use keycodemapdb GIT submodule

[Andrea Bolognani]

On Fri, 2017-03-03 at 16:54 +0000, Daniel P. Berrange wrote:
[...]
> @@ -1,3 +1,6 @@
>  [submodule "gnulib"]
>  	path = .gnulib
>  	url = git://git.sv.gnu.org/gnulib.git
> +[submodule "src/keycodemapdb"]
> +	path = src/keycodemapdb
> +	url = https://gitlab.com/keycodemap/keycodemapdb.git

I think you'll need to update bootstrap_hash() in autogen.sh
to ignore this new submodule, so that it won't run gnulib's
bootstrap every time we update keycodemapdb.

I'm also wondering whether we can avoid having all developers
run 'git submodule init && git submodule update' after these
changes have been pushed...

[...]
> +EXTRA_DIST += $(srcdir)/keycodemapdb/data/keymaps.csv \
> +		$(srcdir)/keycodemapdb/tools/keymap-gen

Please change this into

EXTRA_DIST += \
    $(srcdir)/keycodemapdb/data/keymaps.csv \
    $(srcdir)/keycodemapdb/tools/keymap-gen \
    $(NULL)

Same for other multi-line lists you define later, like
$(KEYTABLES).

[...]
> +util/virkeycodetable_%.h: $(srcdir)/keycodemapdb/data/keymaps.csv \
> +			$(srcdir)/keycodemapdb/tools/keymap-gen Makefile.am
> +	$(AM_V_GEN)export NAME=`echo $@ | sed -e 's,util/virkeycodetable_,,' \
> +					      -e 's,\.h,,'` && \
> +		$(MKDIR_P) util/ && \
> +		$(PYTHON) $(srcdir)/keycodemapdb/tools/keymap-gen \
> +		--lang stdc --varname virKeyCodeTable_$$NAME code-table \
> +		$(srcdir)/keycodemapdb/data/keymaps.csv $$NAME > \
> +			$@-tmp && mv $@-tmp $@ || rm $@-tmp
> +
> +util/virkeynametable_%.h: $(srcdir)/keycodemapdb/data/keymaps.csv \
> +			$(srcdir)/keycodemapdb/tools/keymap-gen Makefile.am
> +	$(AM_V_GEN)export NAME=`echo $@ | sed -e 's,util/virkeynametable_,,' \
> +					      -e 's,\.h,,'` && \
> +		$(MKDIR_P) util/ && \
> +		$(PYTHON) $(srcdir)/keycodemapdb/tools/keymap-gen \
> +		--lang stdc --varname virKeyNameTable_$$NAME name-table \
> +		$(srcdir)/keycodemapdb/data/keymaps.csv $$NAME > \
> +			$@-tmp && mv $@-tmp $@ || rm $@-tmp

Do you really need the dependency on Makefile.am there?

You could also use 'rm -f' instead of plain rm and reformat
the shell snippets a bit so that they look better.

[...]
> +#define VIR_KEYMAP_ENTRY_MAX ARRAY_CARDINALITY(virKeyCodeTable_linux)
> +
> +verify(ARRAY_CARDINALITY(virKeyCodeTable_linux) == ARRAY_CARDINALITY(virKeyCodeTable_xt));
> +verify(ARRAY_CARDINALITY(virKeyCodeTable_linux) == ARRAY_CARDINALITY(virKeyCodeTable_atset1));
> +verify(ARRAY_CARDINALITY(virKeyCodeTable_linux) == ARRAY_CARDINALITY(virKeyCodeTable_atset2));
> +verify(ARRAY_CARDINALITY(virKeyCodeTable_linux) == ARRAY_CARDINALITY(virKeyCodeTable_atset3));
> +verify(ARRAY_CARDINALITY(virKeyCodeTable_linux) == ARRAY_CARDINALITY(virKeyCodeTable_osx));
> +verify(ARRAY_CARDINALITY(virKeyCodeTable_linux) == ARRAY_CARDINALITY(virKeyCodeTable_xtkbd));
> +verify(ARRAY_CARDINALITY(virKeyCodeTable_linux) == ARRAY_CARDINALITY(virKeyCodeTable_usb));
> +verify(ARRAY_CARDINALITY(virKeyCodeTable_linux) == ARRAY_CARDINALITY(virKeyCodeTable_win32));
> +verify(ARRAY_CARDINALITY(virKeyCodeTable_linux) == ARRAY_CARDINALITY(virKeyCodeTable_rfb));
> +verify(ARRAY_CARDINALITY(virKeyCodeTable_linux) == ARRAY_CARDINALITY(virKeyNameTable_linux));
> +verify(ARRAY_CARDINALITY(virKeyCodeTable_linux) == ARRAY_CARDINALITY(virKeyNameTable_osx));
> +verify(ARRAY_CARDINALITY(virKeyCodeTable_linux) == ARRAY_CARDINALITY(virKeyNameTable_win32));

You can use VIR_KEYMAP_ENTRY_MAX, which you've just defined,
instead of calling ARRAY_CARDINALITY() over and over.

-- 
Andrea Bolognani / Red Hat / Virtualization