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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to