Re: [Qemu-devel] [PATCH v5 00/12] Convert over to use keycodemapdb

2017-09-19 Thread Daniel P. Berrange 
On Tue, Sep 19, 2017 at 04:26:22PM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> > > The Makefile checks the output of this script, and if it indicates
> > > that an submodule update is required, it uses an ifeq() to add a
> > > dependancy between "Makefile" and a phony target that re-runs
> > > configure (which in turns updates the submodules). If no update was
> > > required, then no dependancy from Makefile gets added, so build
> > > runs
> > > normally.
> 
> Neat trick.  I think re-running configure should not be needed though. 
> Touching the Makefile should be enough to make make re-evaluating
> things after updating submodules ...

Yeah, re-running configure would only be needed if one of the submodules
had code that needed to be run by configure (eg before we removed pixman
we could re-run pixman/configure).

Since we killed pixman though, we might as well stick with doing it
all in the Makefile and ignore configure.

> diff --git a/Makefile b/Makefile
> index b53fc69a60..a9a0cea6d9 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -31,6 +31,27 @@ CONFIG_ALL=y
>  -include config-all-devices.mak
>  -include config-all-disas.mak
>  
> +ifeq (0,$(MAKELEVEL))
> +  git_module_status := $(shell \
> +cd '$(SRC_PATH)'; \
> +test -d .git || { echo 0; exit; }; \
> +./scripts/git-submodule.sh status; \
> +echo $$?; \
> +  )
> +
> +ifeq (1,$(git_module_status))
> +Makefile: git-submodule-update
> +
> +.PHONY: git-submodule-update
> +
> +git-submodule-update:
> + @echo "GIT submodules out of date, updating."
> + (cd $(SRC_PATH); ./scripts/git-submodule.sh update)
> + @touch Makefile
> +endif
> +endif
> +
> +
>  config-host.mak: $(SRC_PATH)/configure $(SRC_PATH)/pc-bios
>   @echo $@ is out-of-date, running configure
>   @# TODO: The next lines include code which supports a smooth
> diff --git a/.gitignore b/.gitignore
> index cf65316863..0c5fda2fdb 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -111,6 +111,7 @@
>  /docs/version.texi
>  *.tps
>  .stgit-*
> +.git-submodule-status
>  cscope.*
>  tags
>  TAGS
> diff --git a/scripts/git-submodule.sh b/scripts/git-submodule.sh
> new file mode 100755
> index 00..07d36c2b82
> --- /dev/null
> +++ b/scripts/git-submodule.sh
> @@ -0,0 +1,22 @@
> +#!/bin/bash
> +
> +# config
> +modules="dtc"
> +substat=".git-submodule-status"
> +
> +# drop modules not checked out
> +modules="$(git submodule status $modules | awk '/^[^-]/ { print $2
> }')"
> +
> +case "$1" in
> +status)
> + test -f "$substat" || exit 1
> + git submodule status $modules > "${substat}.tmp"
> + trap "rm -f ${substat}.tmp" EXIT
> + diff "${substat}" "${substat}.tmp" >/dev/null
> + exit $?
> + ;;
> +update)
> + git submodule update $modules
> + git submodule status $modules > "${substat}"
> + ;;
> +esac

Yep, this looks reasonable. I'll incorporate this in my keycodemapdb
patch series & repost later this week.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v5 00/12] Convert over to use keycodemapdb

2017-09-19 Thread Gerd Hoffmann 
  Hi,

> > The Makefile checks the output of this script, and if it indicates
> > that an submodule update is required, it uses an ifeq() to add a
> > dependancy between "Makefile" and a phony target that re-runs
> > configure (which in turns updates the submodules). If no update was
> > required, then no dependancy from Makefile gets added, so build
> > runs
> > normally.

Neat trick.  I think re-running configure should not be needed though. 
Touching the Makefile should be enough to make make re-evaluating
things after updating submodules ...

cheers,
  Gerd

diff --git a/Makefile b/Makefile
index b53fc69a60..a9a0cea6d9 100644
--- a/Makefile
+++ b/Makefile
@@ -31,6 +31,27 @@ CONFIG_ALL=y
 -include config-all-devices.mak
 -include config-all-disas.mak
 
+ifeq (0,$(MAKELEVEL))
+  git_module_status := $(shell \
+cd '$(SRC_PATH)'; \
+test -d .git || { echo 0; exit; }; \
+./scripts/git-submodule.sh status; \
+echo $$?; \
+  )
+
+ifeq (1,$(git_module_status))
+Makefile: git-submodule-update
+
+.PHONY: git-submodule-update
+
+git-submodule-update:
+   @echo "GIT submodules out of date, updating."
+   (cd $(SRC_PATH); ./scripts/git-submodule.sh update)
+   @touch Makefile
+endif
+endif
+
+
 config-host.mak: $(SRC_PATH)/configure $(SRC_PATH)/pc-bios
    @echo $@ is out-of-date, running configure
    @# TODO: The next lines include code which supports a smooth
diff --git a/.gitignore b/.gitignore
index cf65316863..0c5fda2fdb 100644
--- a/.gitignore
+++ b/.gitignore
@@ -111,6 +111,7 @@
 /docs/version.texi
 *.tps
 .stgit-*
+.git-submodule-status
 cscope.*
 tags
 TAGS
diff --git a/scripts/git-submodule.sh b/scripts/git-submodule.sh
new file mode 100755
index 00..07d36c2b82
--- /dev/null
+++ b/scripts/git-submodule.sh
@@ -0,0 +1,22 @@
+#!/bin/bash
+
+# config
+modules="dtc"
+substat=".git-submodule-status"
+
+# drop modules not checked out
+modules="$(git submodule status $modules | awk '/^[^-]/ { print $2
}')"
+
+case "$1" in
+status)
+   test -f "$substat" || exit 1
+   git submodule status $modules > "${substat}.tmp"
+   trap "rm -f ${substat}.tmp" EXIT
+   diff "${substat}" "${substat}.tmp" >/dev/null
+   exit $?
+   ;;
+update)
+   git submodule update $modules
+   git submodule status $modules > "${substat}"
+   ;;
+esac

Re: [Qemu-devel] [PATCH v5 00/12] Convert over to use keycodemapdb

2017-09-19 Thread Daniel P. Berrange 
On Tue, Sep 19, 2017 at 12:20:55PM +0100, Daniel P. Berrange wrote:
> On Tue, Sep 19, 2017 at 12:58:20PM +0200, Gerd Hoffmann wrote:
> >   Hi,
> > 
> > > > So I did the keymaps build with a recursive make call too, which
> > > > doesn't look that pretty ...
> > > 
> > > I don't think that's too ugly, but I wonder if there's some way to
> > > avoid
> > > the recursive make call.
> > > 
> > > It feels like this is a similar scenario to 'config-host.mak' being
> > > outdated. I don't entirely understand the logic yet, but we manage to
> > > automatically re-run configure and rebuild config-host.make, when
> > > configure changes, and that in turn affects which dependancies need
> > > rebuild.
> > 
> > Can't spot anything special in the Makefile.  Maybe make is clever
> > enough to figure that a rule updates a include file and starts over
> > then.
> > 
> > > I wonder if we can somehow integrate into that process, so
> > > that configure is responsible for checking out the git submodules,
> > > then make the re-running of configure trigger when .gitmodules
> > > changes content.
> > 
> > .gitmodules only has the repo links, not the checkout hashes.  So it
> > wouldn't be touched on updates.
> > 
> > I can't see an easy way for make to figure a submodule has changed,
> > other than running "git submodule update".
> 
> So I think I figured out the trick libvirt/gnulib uses for this. It
> runs an external script to check the submodule status. This runs
> 'git submodule' to get a list of expected hashes, and compares this
> to a file .git-submodule-status that it previously created (might be
> missing on fresh checkout). If the expected & stores hashes don't
> match this script runs an error.
> 
> The Makefile checks the output of this script, and if it indicates
> that an submodule update is required, it uses an ifeq() to add a
> dependancy between "Makefile" and a phony target that re-runs
> configure (which in turns updates the submodules). If no update was
> required, then no dependancy from Makefile gets added, so build runs
> normally.

Here is an example of integrating this approach with QEMU that seems
like it should work:

diff --git a/.gitmodules b/.gitmodules
index 84c54cdc49..f3bbc01f82 100644
--- a/.gitmodules
+++ b/.gitmodules
@@ -34,3 +34,6 @@
 [submodule "roms/QemuMacDrivers"]
path = roms/QemuMacDrivers
url = git://git.qemu.org/QemuMacDrivers.git
+[submodule "ui/keycodemapdb"]
+   path = ui/keycodemapdb
+   url = https://gitlab.com/keycodemap/keycodemapdb.git
diff --git a/Makefile b/Makefile
index b53fc69a60..62f17d2db5 100644
--- a/Makefile
+++ b/Makefile
@@ -803,6 +803,25 @@ Makefile: $(GENERATED_FILES)
 endif
 endif
 
+ifeq (0,$(MAKELEVEL))
+  git_module_status := $(shell \
+  cd '$(SRC_PATH)'; \
+  test -d .git || test -f .git || { echo 0; exit; }; \
+  ./scripts/git-submodule-status.sh; \
+  echo $$?; \
+  )
+
+ifeq (1,$(git_module_status))
+Makefile: reconfig
+
+.PHONY: reconfig
+
+reconfig:
+   @echo "GIT submodules out of date, re-running configure"
+   ./config.status
+endif
+endif
+
 .SECONDARY: $(TRACE_HEADERS) $(TRACE_HEADERS:%=%-timestamp) \
$(TRACE_SOURCES) $(TRACE_SOURCES:%=%-timestamp) \
$(TRACE_DTRACE) $(TRACE_DTRACE:%=%-timestamp)
diff --git a/configure b/configure
index 94db2d103e..eb29a95ff9 100755
--- a/configure
+++ b/configure
@@ -3583,6 +3583,15 @@ fi
 libs_softmmu="$libs_softmmu $fdt_libs"
 
 ##
+# initialize keycodemapdb module
+
+if test -d "${source_path}/.git"
+then
+git submodule update --init ui/keycodemapdb
+git submodule status ui/keycodemapdb | awk '{print $1 " " $2}' > 
.git-submodule-status
+fi
+
+##
 # opengl probe (for sdl2, gtk, milkymist-tmu2)
 
 if test "$opengl" != "no" ; then
diff --git a/scripts/git-submodule-status.sh b/scripts/git-submodule-status.sh
new file mode 100755
index 00..1a28118ee6
--- /dev/null
+++ b/scripts/git-submodule-status.sh
@@ -0,0 +1,16 @@
+#!/bin/sh
+
+if ! test -f .git-submodule-status
+then
+   exit 1
+fi
+
+git submodule status ui/keycodemapdb | awk '{print $1 " " $2}' > 
.git-submodule-status.tmp
+
+diff .git-submodule-status .git-submodule-status.tmp >/dev/null
+
+ret=$?
+
+rm -f .git-submodule-status.tmp
+
+exit $ret
diff --git a/ui/keycodemapdb b/ui/keycodemapdb
new file mode 16
index 00..56ce5650d2
--- /dev/null
+++ b/ui/keycodemapdb
@@ -0,0 +1 @@
+Subproject commit 56ce5650d2c6ea216b4580df44b9a6dd3bc92c3b
-- 
2.13.5



Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v5 00/12] Convert over to use keycodemapdb

2017-09-19 Thread Daniel P. Berrange 
On Tue, Sep 19, 2017 at 12:58:20PM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> > > So I did the keymaps build with a recursive make call too, which
> > > doesn't look that pretty ...
> > 
> > I don't think that's too ugly, but I wonder if there's some way to
> > avoid
> > the recursive make call.
> > 
> > It feels like this is a similar scenario to 'config-host.mak' being
> > outdated. I don't entirely understand the logic yet, but we manage to
> > automatically re-run configure and rebuild config-host.make, when
> > configure changes, and that in turn affects which dependancies need
> > rebuild.
> 
> Can't spot anything special in the Makefile.  Maybe make is clever
> enough to figure that a rule updates a include file and starts over
> then.
> 
> > I wonder if we can somehow integrate into that process, so
> > that configure is responsible for checking out the git submodules,
> > then make the re-running of configure trigger when .gitmodules
> > changes content.
> 
> .gitmodules only has the repo links, not the checkout hashes.  So it
> wouldn't be touched on updates.
> 
> I can't see an easy way for make to figure a submodule has changed,
> other than running "git submodule update".

So I think I figured out the trick libvirt/gnulib uses for this. It
runs an external script to check the submodule status. This runs
'git submodule' to get a list of expected hashes, and compares this
to a file .git-submodule-status that it previously created (might be
missing on fresh checkout). If the expected & stores hashes don't
match this script runs an error.

The Makefile checks the output of this script, and if it indicates
that an submodule update is required, it uses an ifeq() to add a
dependancy between "Makefile" and a phony target that re-runs
configure (which in turns updates the submodules). If no update was
required, then no dependancy from Makefile gets added, so build runs
normally.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v5 00/12] Convert over to use keycodemapdb

2017-09-19 Thread Peter Maydell 
On 19 September 2017 at 11:58, Gerd Hoffmann  wrote:

>> It feels like this is a similar scenario to 'config-host.mak' being
>> outdated. I don't entirely understand the logic yet, but we manage to
>> automatically re-run configure and rebuild config-host.make, when
>> configure changes, and that in turn affects which dependancies need
>> rebuild.
>
> Can't spot anything special in the Makefile.  Maybe make is clever
> enough to figure that a rule updates a include file and starts over
> then.

Yes, Make will notice if it has a dependency rule for a file
included via 'include foo' (or for the makefile itself). In
this case we 'include config-host.mak', and config-host.mak
has a rule which says it depends on configure (and on pc-bios ???)
which triggers a rerun of configure.

I also notice that we have a "TODO: this code can be removed
after QEMU 1.7" in the run-configure rune :-)

thanks
-- PMM

Re: [Qemu-devel] [PATCH v5 00/12] Convert over to use keycodemapdb

2017-09-19 Thread Gerd Hoffmann 
  Hi,

> > So I did the keymaps build with a recursive make call too, which
> > doesn't look that pretty ...
> 
> I don't think that's too ugly, but I wonder if there's some way to
> avoid
> the recursive make call.
> 
> It feels like this is a similar scenario to 'config-host.mak' being
> outdated. I don't entirely understand the logic yet, but we manage to
> automatically re-run configure and rebuild config-host.make, when
> configure changes, and that in turn affects which dependancies need
> rebuild.

Can't spot anything special in the Makefile.  Maybe make is clever
enough to figure that a rule updates a include file and starts over
then.

> I wonder if we can somehow integrate into that process, so
> that configure is responsible for checking out the git submodules,
> then make the re-running of configure trigger when .gitmodules
> changes content.

.gitmodules only has the repo links, not the checkout hashes.  So it
wouldn't be touched on updates.

I can't see an easy way for make to figure a submodule has changed,
other than running "git submodule update".

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v5 00/12] Convert over to use keycodemapdb

2017-09-19 Thread Daniel P. Berrange 
On Mon, Sep 18, 2017 at 05:12:55PM +0200, Gerd Hoffmann wrote:
> On Thu, 2017-09-14 at 13:40 +0100, Daniel P. Berrange wrote:
> > On Thu, Sep 14, 2017 at 12:58:26PM +0100, Peter Maydell wrote:
> > > On 14 September 2017 at 12:55, Gerd Hoffmann 
> > > wrote:
> > > >   Hi,
> > > > 
> > > > > I think a better approach is to have something in rules.mak
> > > > > that ensures the submodule is checked out correctly (only
> > > > > when building from GIT, not dist), and then have the rules
> > > > > which generate the keymap files depend on this.
> > > > 
> > > > Care sending a patch doing that for dtc?
> > > 
> > > It sounds awfully fiddly. Maybe it is the best we can do
> > > given the mess that is git submodules, but is it really
> > > the common approach?
> > 
> > I'll do a prototype so we can see something concrete working and
> > evaluate how pleasant (or not) it is
> 
> Tried to brew something:
> 
> https://www.kraxel.org/cgit/qemu/log/?h=work/submodule
> 
> dtc was pretty simple due to the recursive make call.
> 
> Hooking the submodule update into a non-recursive make looks
> complicated, especially because the submodule update might change the
> timestamps and therefore the target set which needs a rebuild ...
> 
> So I did the keymaps build with a recursive make call too, which
> doesn't look that pretty ...

I don't think that's too ugly, but I wonder if there's some way to avoid
the recursive make call.

It feels like this is a similar scenario to 'config-host.mak' being
outdated. I don't entirely understand the logic yet, but we manage to
automatically re-run configure and rebuild config-host.make, when
configure changes, and that in turn affects which dependancies need
rebuild. I wonder if we can somehow integrate into that process, so
that configure is responsible for checking out the git submodules,
then make the re-running of configure trigger when .gitmodules
changes content.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v5 00/12] Convert over to use keycodemapdb

2017-09-18 Thread Gerd Hoffmann 
On Thu, 2017-09-14 at 13:40 +0100, Daniel P. Berrange wrote:
> On Thu, Sep 14, 2017 at 12:58:26PM +0100, Peter Maydell wrote:
> > On 14 September 2017 at 12:55, Gerd Hoffmann 
> > wrote:
> > >   Hi,
> > > 
> > > > I think a better approach is to have something in rules.mak
> > > > that ensures the submodule is checked out correctly (only
> > > > when building from GIT, not dist), and then have the rules
> > > > which generate the keymap files depend on this.
> > > 
> > > Care sending a patch doing that for dtc?
> > 
> > It sounds awfully fiddly. Maybe it is the best we can do
> > given the mess that is git submodules, but is it really
> > the common approach?
> 
> I'll do a prototype so we can see something concrete working and
> evaluate how pleasant (or not) it is

Tried to brew something:

https://www.kraxel.org/cgit/qemu/log/?h=work/submodule

dtc was pretty simple due to the recursive make call.

Hooking the submodule update into a non-recursive make looks
complicated, especially because the submodule update might change the
timestamps and therefore the target set which needs a rebuild ...

So I did the keymaps build with a recursive make call too, which
doesn't look that pretty ...

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v5 00/12] Convert over to use keycodemapdb

2017-09-14 Thread Daniel P. Berrange 
On Thu, Sep 14, 2017 at 12:58:26PM +0100, Peter Maydell wrote:
> On 14 September 2017 at 12:55, Gerd Hoffmann  wrote:
> >   Hi,
> >
> >> I think a better approach is to have something in rules.mak
> >> that ensures the submodule is checked out correctly (only
> >> when building from GIT, not dist), and then have the rules
> >> which generate the keymap files depend on this.
> >
> > Care sending a patch doing that for dtc?
> 
> It sounds awfully fiddly. Maybe it is the best we can do
> given the mess that is git submodules, but is it really
> the common approach?

I'll do a prototype so we can see something concrete working and
evaluate how pleasant (or not) it is

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v5 00/12] Convert over to use keycodemapdb

2017-09-14 Thread Peter Maydell 
On 14 September 2017 at 12:55, Gerd Hoffmann  wrote:
>   Hi,
>
>> I think a better approach is to have something in rules.mak
>> that ensures the submodule is checked out correctly (only
>> when building from GIT, not dist), and then have the rules
>> which generate the keymap files depend on this.
>
> Care sending a patch doing that for dtc?

It sounds awfully fiddly. Maybe it is the best we can do
given the mess that is git submodules, but is it really
the common approach?

thanks
-- PMM

Re: [Qemu-devel] [PATCH v5 00/12] Convert over to use keycodemapdb

2017-09-14 Thread Gerd Hoffmann 
  Hi,

> I think a better approach is to have something in rules.mak
> that ensures the submodule is checked out correctly (only
> when building from GIT, not dist), and then have the rules
> which generate the keymap files depend on this.

Care sending a patch doing that for dtc?

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v5 00/12] Convert over to use keycodemapdb

2017-09-12 Thread Daniel P. Berrange 
On Tue, Sep 12, 2017 at 03:24:22PM +0100, Peter Maydell wrote:
> On 12 September 2017 at 15:19, Daniel P. Berrange  wrote:
> > My POV is that we should a) never check generated files into GIT, and
> > we should use the submodule in such a way that it is indistinguish
> > from the content of the submodule being part of the main GIT repo.
> 
> AFAICT this is impossible in that git submodules are broken
> (checking out a different commit in the top level repo doesn't
> check out the correct version of the submodule to go with it).
> We put up with it for our current uses of submodules because
> the submodule contents don't change very often...

Yep, you need to have something in your build system which does
a checkout of the git submodule at the "right" time.

In autoconf world you can set things up so that 'autoreconf'
gets retriggered when submodule hash changes, but that doesn't
apply to qemu.

I tried to put some magic in configure in v4 but that was
flawed because I didn't take acount of people building
from dist.

I think a better approach is to have something in rules.mak
that ensures the submodule is checked out correctly (only
when building from GIT, not dist), and then have the rules
which generate the keymap files depend on this.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v5 00/12] Convert over to use keycodemapdb

2017-09-12 Thread Peter Maydell 
On 12 September 2017 at 15:19, Daniel P. Berrange  wrote:
> My POV is that we should a) never check generated files into GIT, and
> we should use the submodule in such a way that it is indistinguish
> from the content of the submodule being part of the main GIT repo.

AFAICT this is impossible in that git submodules are broken
(checking out a different commit in the top level repo doesn't
check out the correct version of the submodule to go with it).
We put up with it for our current uses of submodules because
the submodule contents don't change very often...

I agree that checking generated files into git is a bad idea
if we can avoid it.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v5 00/12] Convert over to use keycodemapdb

2017-09-12 Thread Daniel P. Berrange 
On Tue, Sep 12, 2017 at 03:46:34PM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> > The keycodemapdb code is designed to be used as a git sub-module, it
> > is
> > not an external dependancy you need installed before use. In this
> > version,
> > however, the sub-module is not directly use. Instead all the
> > generated
> > files are checked into GIT. The downside to this is that we get an
> > enourmous pile of errors from checkpatch.pl. The tool that generates
> > the
> > mapping files doesn't care about QEMU's coding style rules, because
> > it is
> > a general purpose tool intended for use by many different projects.
> > 
> > As such I'm not very happy about the idea of checking the generated
> > files into
> > GIT, and would prefer to go back to generating the files from the
> > submodule
> > on every build, as was done in v4.
> 
> The submodule approach has its share of issues too, just see how many
> patch versions you've needed until patchew builds actually worked.  And
> I guess lots of people have qemu build scripts which likewise need
> adjustments.  Handling of release tarballs must be considered too.
> 
> I still think we should not require a checked out keycodemapdb
> submodule.  But maybe it'll work better if we check in a copy of the
> script and database instead of checking in the generated files.

This feels like it all just defeats the point of submodules if you manually
copy stuff out of the submodule, or checkin generated files.

My POV is that we should a) never check generated files into GIT, and
we should use the submodule in such a way that it is indistinguish
from the content of the submodule being part of the main GIT repo.
IOW, when building from git always initialize the submodule, but
its content gets add to tar.gz dist, so people building from dist
don't rely on the submodule.

This is the way most projects deal with submodules - its been tricky
getting that working because QEMU has not used submodules in the
way other projects normally do, so there's no current framework in
the configure/makefiles todo the right thing. IMHO it is beneficial
to fix this so we can avoid pointless copying of files and/or storing
generated sources

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v5 00/12] Convert over to use keycodemapdb

2017-09-12 Thread Gerd Hoffmann 
  Hi,

> The keycodemapdb code is designed to be used as a git sub-module, it
> is
> not an external dependancy you need installed before use. In this
> version,
> however, the sub-module is not directly use. Instead all the
> generated
> files are checked into GIT. The downside to this is that we get an
> enourmous pile of errors from checkpatch.pl. The tool that generates
> the
> mapping files doesn't care about QEMU's coding style rules, because
> it is
> a general purpose tool intended for use by many different projects.
> 
> As such I'm not very happy about the idea of checking the generated
> files into
> GIT, and would prefer to go back to generating the files from the
> submodule
> on every build, as was done in v4.

The submodule approach has its share of issues too, just see how many
patch versions you've needed until patchew builds actually worked.  And
I guess lots of people have qemu build scripts which likewise need
adjustments.  Handling of release tarballs must be considered too.

I still think we should not require a checked out keycodemapdb
submodule.  But maybe it'll work better if we check in a copy of the
script and database instead of checking in the generated files.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v5 00/12] Convert over to use keycodemapdb

2017-09-12 Thread no-reply 
Hi,

This series failed build test on s390x host. Please find the details below.

Subject: [Qemu-devel] [PATCH v5 00/12] Convert over to use keycodemapdb
Type: series
Message-id: 20170912123744.14730-1-berra...@redhat.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash
# Testing script will be invoked under the git checkout with
# HEAD pointing to a commit that has the patches applied on top of "base"
# branch
set -e
echo "=== ENV ==="
env
echo "=== PACKAGES ==="
rpm -qa
echo "=== TEST BEGIN ==="
CC=$HOME/bin/cc
INSTALL=$PWD/install
BUILD=$PWD/build
echo -n "Using CC: "
realpath $CC
mkdir -p $BUILD $INSTALL
SRC=$PWD
cd $BUILD
$SRC/configure --cc=$CC --prefix=$INSTALL
make -j4
# XXX: we need reliable clean up
# make check -j4 V=1
make install
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]  patchew/20170823162004.27337-1-marcandre.lur...@redhat.com 
-> patchew/20170823162004.27337-1-marcandre.lur...@redhat.com
 - [tag update]  patchew/20170911201610.15204-1-...@gmx.com -> 
patchew/20170911201610.15204-1-...@gmx.com
 * [new tag] patchew/20170912123744.14730-1-berra...@redhat.com -> 
patchew/20170912123744.14730-1-berra...@redhat.com
Switched to a new branch 'test'
fae652e display: convert XenInput keyboard to keycodemapdb
c8d9524 ui: convert GTK and SDL1 frontends to keycodemapdb
4175200 ui: convert the SDL2 frontend to keycodemapdb
d54c4f7 ui: convert cocoa frontend to keycodemapdb
60a4cdb char: convert the escc device to keycodemapdb
df24350 input: convert the adb device to keycodemapdb
85343f3 input: convert ps2 device to keycodemapdb
00dbdc1 input: convert virtio-input-hid device to keycodemapdb
475272e ui: don't export qemu_input_event_new_key
0d88a11 ui: convert key events to QKeyCodes immediately
33c690e ui: convert common input code to keycodemapdb
1a87122 ui: add keycodemapdb repository as a GIT submodule

=== OUTPUT BEGIN ===
=== ENV ===
XDG_SESSION_ID=5
SHELL=/bin/sh
USER=fam
PATCHEW=/home/fam/patchew/patchew-cli -s http://patchew.org --nodebug
PATH=/usr/bin:/bin
PWD=/var/tmp/patchew-tester-tmp-4iydjbd7/src
LANG=en_US.UTF-8
HOME=/home/fam
SHLVL=2
LOGNAME=fam
DBUS_SESSION_BUS_ADDRESS=unix:path=/run/user/1012/bus
XDG_RUNTIME_DIR=/run/user/1012
_=/usr/bin/env
=== PACKAGES ===
gpg-pubkey-873529b8-54e386ff
xz-libs-5.2.2-2.fc24.s390x
libxshmfence-1.2-3.fc24.s390x
giflib-4.1.6-15.fc24.s390x
trousers-lib-0.3.13-6.fc24.s390x
ncurses-base-6.0-6.20160709.fc25.noarch
gmp-6.1.1-1.fc25.s390x
libidn-1.33-1.fc25.s390x
slang-2.3.0-7.fc25.s390x
pkgconfig-0.29.1-1.fc25.s390x
alsa-lib-1.1.1-2.fc25.s390x
yum-metadata-parser-1.1.4-17.fc25.s390x
python3-slip-dbus-0.6.4-4.fc25.noarch
python2-cssselect-0.9.2-1.fc25.noarch
createrepo_c-libs-0.10.0-6.fc25.s390x
initscripts-9.69-1.fc25.s390x
parted-3.2-21.fc25.s390x
flex-2.6.0-3.fc25.s390x
colord-libs-1.3.4-1.fc25.s390x
python-osbs-client-0.33-3.fc25.noarch
perl-Pod-Simple-3.35-1.fc25.noarch
python2-simplejson-3.10.0-1.fc25.s390x
brltty-5.4-2.fc25.s390x
librados2-10.2.4-2.fc25.s390x
tcp_wrappers-7.6-83.fc25.s390x
libcephfs_jni1-10.2.4-2.fc25.s390x
nettle-devel-3.3-1.fc25.s390x
bzip2-devel-1.0.6-21.fc25.s390x
libuuid-2.28.2-2.fc25.s390x
python3-dnf-1.1.10-6.fc25.noarch
texlive-kpathsea-doc-svn41139-33.fc25.1.noarch
openssh-7.4p1-4.fc25.s390x
texlive-kpathsea-bin-svn40473-33.20160520.fc25.1.s390x
texlive-graphics-svn41015-33.fc25.1.noarch
texlive-dvipdfmx-def-svn40328-33.fc25.1.noarch
texlive-mfware-svn40768-33.fc25.1.noarch
texlive-texlive-scripts-svn41433-33.fc25.1.noarch
texlive-euro-svn22191.1.1-33.fc25.1.noarch
texlive-etex-svn37057.0-33.fc25.1.noarch
texlive-iftex-svn29654.0.2-33.fc25.1.noarch
texlive-palatino-svn31835.0-33.fc25.1.noarch
texlive-texlive-docindex-svn41430-33.fc25.1.noarch
texlive-xunicode-svn30466.0.981-33.fc25.1.noarch
texlive-koma-script-svn41508-33.fc25.1.noarch
texlive-pst-grad-svn15878.1.06-33.fc25.1.noarch
texlive-pst-blur-svn15878.2.0-33.fc25.1.noarch
texlive-jknapltx-svn19440.0-33.fc25.1.noarch
texinfo-6.1-4.fc25.s390x
openssl-devel-1.0.2k-1.fc25.s390x
jansson-2.10-2.fc25.s390x
fedora-repos-25-4.noarch
python3-libs-3.5.3-6.fc25.s390x
perl-Errno-1.25-387.fc25.s390x
acl-2.2.52-13.fc25.s390x
systemd-pam-231-17.fc25.s390x
NetworkManager-libnm-1.4.4-5.fc25.s390x
poppler-0.45.0-5.fc25.s390x
ccache-3.3.4-1.fc25.s390x
valgrind-3.12.0-9.fc25.s390x
perl-open-1.10-387.fc25.noarch
libgcc-6.4.1-1.fc25.s390x
libsoup-2.56.1-1.fc25.s390x
libstdc++-devel-6.4.1-1.fc25.s390x
nss-softokn-devel-3.31.0-1.0.fc25.s390x
libobjc-6.4.1-1.fc25.s390x
python2-rpm-4.13.0.1-2.fc25.s390x
python2-gluster-3.10.5-1.fc25.s390x
rpm-build-4.13.0.1-2.fc25.s390x
glibc-static-2.24-10.fc25.s390x
lz4-1.8.0-1.fc25.s390x
xapian-core-libs-1.2.24-1.fc25.s390x
elfutils-libelf-devel-0.169-1.fc25.s390x
libaio-0.3.110-6.fc24.s390x
libfontenc-1.1.3-3.fc24.s390x
lzo-2.08-8.fc24.s390x
isl-0.14-5.fc24.s390x
libXau-1.0.8-6.fc24.s390x
linux-atm-lib

Re: [Qemu-devel] [PATCH v5 00/12] Convert over to use keycodemapdb

2017-09-12 Thread no-reply 
Hi,

This series failed automatic build test. Please find the testing commands and
their output below. If you have docker installed, you can probably reproduce it
locally.

Subject: [Qemu-devel] [PATCH v5 00/12] Convert over to use keycodemapdb
Message-id: 20170912123744.14730-1-berra...@redhat.com
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
# Let docker tests dump environment info
export SHOW_ENV=1
export J=8
time make docker-test-quick@centos6
time make docker-test-build@min-glib
time make docker-test-mingw@fedora
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
fae652ea62 display: convert XenInput keyboard to keycodemapdb
c8d95241f3 ui: convert GTK and SDL1 frontends to keycodemapdb
4175200b40 ui: convert the SDL2 frontend to keycodemapdb
d54c4f7d1b ui: convert cocoa frontend to keycodemapdb
60a4cdb7cc char: convert the escc device to keycodemapdb
df243503e5 input: convert the adb device to keycodemapdb
85343f342f input: convert ps2 device to keycodemapdb
00dbdc189b input: convert virtio-input-hid device to keycodemapdb
475272eb06 ui: don't export qemu_input_event_new_key
0d88a11efd ui: convert key events to QKeyCodes immediately
33c690e7a1 ui: convert common input code to keycodemapdb
1a87122cbb ui: add keycodemapdb repository as a GIT submodule

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into '/var/tmp/patchew-tester-tmp-86s7dwy8/src/dtc'...
Submodule path 'dtc': checked out '558cd81bdd432769b59bff01240c44f82cfb1a9d'
  BUILD   centos6
make[1]: Entering directory '/var/tmp/patchew-tester-tmp-86s7dwy8/src'
  ARCHIVE qemu.tgz
  ARCHIVE dtc.tgz
  COPYRUNNER
RUN test-quick in qemu:centos6 
Packages installed:
SDL-devel-1.2.14-7.el6_7.1.x86_64
bison-2.4.1-5.el6.x86_64
bzip2-devel-1.0.5-7.el6_0.x86_64
ccache-3.1.6-2.el6.x86_64
csnappy-devel-0-6.20150729gitd7bc683.el6.x86_64
flex-2.5.35-9.el6.x86_64
gcc-4.4.7-18.el6.x86_64
git-1.7.1-8.el6.x86_64
glib2-devel-2.28.8-9.el6.x86_64
libepoxy-devel-1.2-3.el6.x86_64
libfdt-devel-1.4.0-1.el6.x86_64
librdmacm-devel-1.0.21-0.el6.x86_64
lzo-devel-2.03-3.1.el6_5.1.x86_64
make-3.81-23.el6.x86_64
mesa-libEGL-devel-11.0.7-4.el6.x86_64
mesa-libgbm-devel-11.0.7-4.el6.x86_64
package g++ is not installed
pixman-devel-0.32.8-1.el6.x86_64
spice-glib-devel-0.26-8.el6.x86_64
spice-server-devel-0.12.4-16.el6.x86_64
tar-1.23-15.el6_8.x86_64
vte-devel-0.25.1-9.el6.x86_64
xen-devel-4.6.3-15.el6.x86_64
zlib-devel-1.2.3-29.el6.x86_64

Environment variables:
PACKAGES=bison bzip2-devel ccache csnappy-devel flex g++
 gcc git glib2-devel libepoxy-devel libfdt-devel 
librdmacm-devel lzo-devel make mesa-libEGL-devel 
mesa-libgbm-devel pixman-devel SDL-devel spice-glib-devel 
spice-server-devel tar vte-devel xen-devel zlib-devel
HOSTNAME=73f5b8f10f48
TERM=xterm
MAKEFLAGS= -j8
HISTSIZE=1000
J=8
USER=root
LS_COLORS=rs=0:di=01;34:ln=01;36:mh=00:pi=40;33:so=01;35:do=01;35:bd=40;33;01:cd=40;33;01:or=40;31;01:mi=01;05;37;41:su=37;41:sg=30;43:ca=30;41:tw=30;42:ow=34;42:st=37;44:ex=01;32:*.tar=01;31:*.tgz=01;31:*.arj=01;31:*.taz=01;31:*.lzh=01;31:*.lzma=01;31:*.tlz=01;31:*.txz=01;31:*.zip=01;31:*.z=01;31:*.Z=01;31:*.dz=01;31:*.gz=01;31:*.lz=01;31:*.xz=01;31:*.bz2=01;31:*.tbz=01;31:*.tbz2=01;31:*.bz=01;31:*.tz=01;31:*.deb=01;31:*.rpm=01;31:*.jar=01;31:*.rar=01;31:*.ace=01;31:*.zoo=01;31:*.cpio=01;31:*.7z=01;31:*.rz=01;31:*.jpg=01;35:*.jpeg=01;35:*.gif=01;35:*.bmp=01;35:*.pbm=01;35:*.pgm=01;35:*.ppm=01;35:*.tga=01;35:*.xbm=01;35:*.xpm=01;35:*.tif=01;35:*.tiff=01;35:*.png=01;35:*.svg=01;35:*.svgz=01;35:*.mng=01;35:*.pcx=01;35:*.mov=01;35:*.mpg=01;35:*.mpeg=01;35:*.m2v=01;35:*.mkv=01;35:*.ogm=01;35:*.mp4=01;35:*.m4v=01;35:*.mp4v=01;35:*.vob=01;35:*.qt=01;35:*.nuv=01;35:*.wmv=01;35:*.asf=01;35:*.rm=01;35:*.rmvb=01;35:*.flc=01;35:*.avi=01;35:*.fli=01;35:*.flv=01;35:*.gl=01;35:*.dl=01;35:*.xcf=01;35:*.xwd=01;35:*.yuv=01;35:*.cgm=01;35:*.emf=01;35:*.axv=01;35:*.anx=01;35:*.ogv=01;35:*.ogx=01;35:*.aac=01;36:*.au=01;36:*.flac=01;36:*.mid=01;36:*.midi=01;36:*.mka=01;36:*.mp3=01;36:*.mpc=01;36:*.ogg=01;36:*.ra=01;36:*.wav=01;36:*.axa=01;36:*.oga=01;36:*.spx=01;36:*.xspf=01;36:
CCACHE_DIR=/var/tmp/ccache
EXTRA_CONFIGURE_OPTS=
V=
SHOW_ENV=1
MAIL=/var/spool/mail/root
PATH=/usr/lib/ccache:/usr/lib64/ccache:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
PWD=/
LANG=en_US.UTF-8
TARGET_LIST=
HISTCONTROL=ignoredups
SHLVL=1
HOME=/root
TEST_DIR=/tmp/qemu-test
LOGNAME=root
LESSOPEN=||/usr/bin/lesspipe.sh %s
FEATURES= dtc
DEBUG=
G_BROKEN_FILENAMES=1
CCACHE_HASHDIR=
_=/usr/bin/env

Configure options:
--enable-werror --target-list=x86_64-softmmu,aarch64-softmmu 
--prefix=/var/tmp/qemu-build/install
No C++ compiler available; disabling C++ specific optional code
Install prefix/var/tmp/qemu-build/install
BIOS directory/var/tmp/qemu-build

[Qemu-devel] [PATCH v5 00/12] Convert over to use keycodemapdb

2017-09-12 Thread Daniel P. Berrange 
An update of:

  v1: https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg02047.html
  v2: https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg02471.html
  v3: https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg02517.html
  v4: https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg02708.html

The keycodemap project[1] provides a database mapping between many different
keysym/keycode/scancode sets, along with a tool to generate mapping/lookup
tables in various programming languages. It is already used by GTK-VNC,
SPICE-GTK and libvirt.

This series enables its use in QEMU, thus fixing a great many bugs/ommissions
in the 15+ key mapping tables people have manually written for QEMU.

The keycodemapdb code is designed to be used as a git sub-module, it is
not an external dependancy you need installed before use. In this version,
however, the sub-module is not directly use. Instead all the generated
files are checked into GIT. The downside to this is that we get an
enourmous pile of errors from checkpatch.pl. The tool that generates the
mapping files doesn't care about QEMU's coding style rules, because it is
a general purpose tool intended for use by many different projects.

As such I'm not very happy about the idea of checking the generated files into
GIT, and would prefer to go back to generating the files from the submodule
on every build, as was done in v4. I did this this in v5 way to illustrate the
approach, but on balance I think v4 approach was better. Using the v4 approach
would also means when updating the submodule later on, we don't have to post
largely meaningless patches of diffs to all the generated files.

This series converts all the front ends and all the input devices which are
using the new InputEvent framework. A handful of devices still use the
legacy kbd handler

  $ git grep -l add_kbd_event_handler hw
  hw/arm/musicpal.c
  hw/arm/nseries.c
  hw/arm/palm.c
  hw/arm/spitz.c
  hw/input/pxa2xx_keypad.c
  hw/input/stellaris_input.c

and could be usefully converted too.

I've not done much realworld testing of this yet. I did however write code
that compared the mapping tables before and after conversion to identify what
mapping changes have resulted in each frontend/backend.  What I still need to
go back and validate is the Print/Sysrq handling, because that is special
everywhere and I'm not entirely sure I've done that correctly yet. The GTK
frontend should now work correctly when run on X11 servers on Win32 and OS-X,
as well as when run on native Win32/OS-X display backends.

[1] https://gitlab.com/keycodemap/keycodemapdb/

Changed in v5:

 - Don't try to initialize git submodule at all
 - Store generate keymap files in GIT

Changed in v4:

 - Run submodule update in source_dir for vpath builds (patchew)
 - Force submodule update in docker rules in case they
   are run without configure (patchew)

Changed in v3:

 - Ensure docker builds pull in keycodemapdb submodule (patchew)
 - Add compat with py26 for RHEL-6 in keycodemapdb tools (patchew)
 - Initialize submodule in configure script (patchew)

Changed in v2:

 - Change filename pattern to 'ui/input-keymap-$SRC-to-$DST.c'
   and map names 'qemu_input_map_$SRC_to_$DST'  (Eric)
 - Fix typos (Eric)
 - Drop changes to InputKeyEvent struct (Eric)
 - Fix VPATH build (patchew)
 - Fix code style errors (patchew)


Daniel P. Berrange (12):
  ui: add keycodemapdb repository as a GIT submodule
  ui: convert common input code to keycodemapdb
  ui: convert key events to QKeyCodes immediately
  ui: don't export qemu_input_event_new_key
  input: convert virtio-input-hid device to keycodemapdb
  input: convert ps2 device to keycodemapdb
  input: convert the adb device to keycodemapdb
  char: convert the escc device to keycodemapdb
  ui: convert cocoa frontend to keycodemapdb
  ui: convert the SDL2 frontend to keycodemapdb
  ui: convert GTK and SDL1 frontends to keycodemapdb
  display: convert XenInput keyboard to keycodemapdb

 .gitmodules|   3 +
 hw/char/escc.c | 126 +
 hw/display/xenfb.c | 133 +++---
 hw/input/adb.c | 124 +
 hw/input/ps2.c | 406 +
 hw/input/virtio-input-hid.c| 136 +-
 include/hw/input/adb-keys.h| 141 --
 include/ui/input.h |  57 +++-
 ui/Makefile.objs   |  43 ++-
 ui/cocoa.m | 129 +
 ui/gtk.c   | 205 +--
 ui/input-keymap-atset1-to-qcode.c  | 245 +
 ui/input-keymap-linux-to-qcode.c   | 463 +
 ui/input-keymap-osx-to-qcode.c | 128 +
 ui/input-keymap-qcode-to-adb.c | 156 +++
 ui/input-keymap-qcode-to-atset1.c  | 154 +++
 ui/input-keymap-qcode-to-atset2.c  | 142 ++