1. Patchew
  2. Libvirt
  3. View series

[libvirt] [PATCH] keycodemapdb: Update submodule

Andrea Bolognani posted 1 patch 7 years, 2 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20180312132949.24394-1-abologna@redhat.com
Test syntax-check passed
include/libvirt/libvirt-domain.h | 9 ++++++++-
src/keycodemapdb                 | 2 +-
src/qemu/qemu_driver.c           | 8 ++++----
src/util/Makefile.inc.am         | 2 +-
src/util/virkeycode.c            | 8 ++++----
tests/virkeycodetest.c           | 4 ++--
tools/virsh-domain.c             | 5 +++++
tools/virsh.pod                  | 6 +++---
8 files changed, 28 insertions(+), 16 deletions(-)
[libvirt] [PATCH] keycodemapdb: Update submodule
Posted by Andrea Bolognani 7 years, 2 months ago 
This time around it's not enough to just pick the latest commit,
because with aed87bb2aa6ed83b49574eb982e3bdd4c36acf17 keycodemapdb
renamed the 'rfb' keycode to 'qnum' and we need to accept the new
name while maintaining backwards compatibility.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
 include/libvirt/libvirt-domain.h | 9 ++++++++-
 src/keycodemapdb                 | 2 +-
 src/qemu/qemu_driver.c           | 8 ++++----
 src/util/Makefile.inc.am         | 2 +-
 src/util/virkeycode.c            | 8 ++++----
 tests/virkeycodetest.c           | 4 ++--
 tools/virsh-domain.c             | 5 +++++
 tools/virsh.pod                  | 6 +++---
 8 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 4048acf38a..4128d55852 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -2733,7 +2733,7 @@ typedef enum {
     VIR_KEYCODE_SET_XT_KBD         = 6,
     VIR_KEYCODE_SET_USB            = 7,
     VIR_KEYCODE_SET_WIN32          = 8,
-    VIR_KEYCODE_SET_RFB            = 9,
+    VIR_KEYCODE_SET_QNUM           = 9,
 
 # ifdef VIR_ENUM_SENTINELS
     VIR_KEYCODE_SET_LAST
@@ -2745,6 +2745,13 @@ typedef enum {
 # endif
 } virKeycodeSet;
 
+/**
+ * VIR_KEYCODE_SET_RFB:
+ *
+ * Compatibility alias for VIR_KEYCODE_SET_QNUM, which replaced it since 4.2.0.
+ */
+# define VIR_KEYCODE_SET_RFB VIR_KEYCODE_SET_QNUM
+
 /**
  * VIR_DOMAIN_SEND_KEY_MAX_KEYS:
  *
diff --git a/src/keycodemapdb b/src/keycodemapdb
index 267157b96c..16e5b07876 160000
--- a/src/keycodemapdb
+++ b/src/keycodemapdb
@@ -1 +1 @@
-Subproject commit 267157b96c62b5445de9cddd21de42fcd943ffe6
+Subproject commit 16e5b0787687d8904dad2c026107409eb9bfcb95
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e13544f832..8c872c1f08 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2580,17 +2580,17 @@ static int qemuDomainSendKey(virDomainPtr domain,
 
     virCheckFlags(0, -1);
 
-    /* translate the keycode to RFB for qemu driver */
-    if (codeset != VIR_KEYCODE_SET_RFB) {
+    /* translate the keycode to QNUM for qemu driver */
+    if (codeset != VIR_KEYCODE_SET_QNUM) {
         size_t i;
         int keycode;
 
         for (i = 0; i < nkeycodes; i++) {
-            keycode = virKeycodeValueTranslate(codeset, VIR_KEYCODE_SET_RFB,
+            keycode = virKeycodeValueTranslate(codeset, VIR_KEYCODE_SET_QNUM,
                                                keycodes[i]);
             if (keycode < 0) {
                 virReportError(VIR_ERR_INTERNAL_ERROR,
-                               _("cannot translate keycode %u of %s codeset to rfb keycode"),
+                               _("cannot translate keycode %u of %s codeset to qnum keycode"),
                                keycodes[i],
                                virKeycodeSetTypeToString(codeset));
                 return -1;
diff --git a/src/util/Makefile.inc.am b/src/util/Makefile.inc.am
index a91b30dca5..d0e1ec3625 100644
--- a/src/util/Makefile.inc.am
+++ b/src/util/Makefile.inc.am
@@ -214,7 +214,7 @@ EXTRA_DIST += \
 	$(NULL)
 
 
-KEYCODES = linux osx atset1 atset2 atset3 xtkbd usb win32 rfb
+KEYCODES = linux osx atset1 atset2 atset3 xtkbd usb win32 qnum
 KEYNAMES = linux osx win32
 
 KEYTABLES = \
diff --git a/src/util/virkeycode.c b/src/util/virkeycode.c
index eda263218c..8976bbf376 100644
--- a/src/util/virkeycode.c
+++ b/src/util/virkeycode.c
@@ -27,7 +27,7 @@
 #include "virkeycodetable_atset3.h"
 #include "virkeycodetable_linux.h"
 #include "virkeycodetable_osx.h"
-#include "virkeycodetable_rfb.h"
+#include "virkeycodetable_qnum.h"
 #include "virkeycodetable_usb.h"
 #include "virkeycodetable_win32.h"
 #include "virkeycodetable_xtkbd.h"
@@ -52,7 +52,7 @@ static const unsigned short *virKeymapValues[VIR_KEYCODE_SET_LAST] = {
     [VIR_KEYCODE_SET_XT_KBD] = virKeyCodeTable_xtkbd,
     [VIR_KEYCODE_SET_USB] = virKeyCodeTable_usb,
     [VIR_KEYCODE_SET_WIN32] = virKeyCodeTable_win32,
-    [VIR_KEYCODE_SET_RFB] = virKeyCodeTable_rfb,
+    [VIR_KEYCODE_SET_QNUM] = virKeyCodeTable_qnum,
 };
 
 #define VIR_KEYMAP_ENTRY_MAX ARRAY_CARDINALITY(virKeyCodeTable_linux)
@@ -64,7 +64,7 @@ verify(VIR_KEYMAP_ENTRY_MAX == ARRAY_CARDINALITY(virKeyCodeTable_osx));
 verify(VIR_KEYMAP_ENTRY_MAX == ARRAY_CARDINALITY(virKeyCodeTable_xtkbd));
 verify(VIR_KEYMAP_ENTRY_MAX == ARRAY_CARDINALITY(virKeyCodeTable_usb));
 verify(VIR_KEYMAP_ENTRY_MAX == ARRAY_CARDINALITY(virKeyCodeTable_win32));
-verify(VIR_KEYMAP_ENTRY_MAX == ARRAY_CARDINALITY(virKeyCodeTable_rfb));
+verify(VIR_KEYMAP_ENTRY_MAX == ARRAY_CARDINALITY(virKeyCodeTable_qnum));
 verify(VIR_KEYMAP_ENTRY_MAX == ARRAY_CARDINALITY(virKeyNameTable_linux));
 verify(VIR_KEYMAP_ENTRY_MAX == ARRAY_CARDINALITY(virKeyNameTable_osx));
 verify(VIR_KEYMAP_ENTRY_MAX == ARRAY_CARDINALITY(virKeyNameTable_win32));
@@ -79,7 +79,7 @@ VIR_ENUM_IMPL(virKeycodeSet, VIR_KEYCODE_SET_LAST,
     "xt_kbd",
     "usb",
     "win32",
-    "rfb",
+    "qnum",
 );
 
 int virKeycodeValueFromString(virKeycodeSet codeset,
diff --git a/tests/virkeycodetest.c b/tests/virkeycodetest.c
index 24887a36d4..399a13fcae 100644
--- a/tests/virkeycodetest.c
+++ b/tests/virkeycodetest.c
@@ -54,8 +54,8 @@ static int testKeycodeMapping(const void *data ATTRIBUTE_UNUSED)
 
     TRANSLATE(LINUX, LINUX, 111, 111);
     TRANSLATE(LINUX, USB, 111, 76);
-    TRANSLATE(LINUX, RFB, 88, 88);
-    TRANSLATE(LINUX, RFB, 160, 163);
+    TRANSLATE(LINUX, QNUM, 88, 88);
+    TRANSLATE(LINUX, QNUM, 160, 163);
     TRANSLATE(ATSET2, ATSET3, 131, 55);
     TRANSLATE(OSX, WIN32, 90, 131);
     TRANSLATE(OSX, ATSET1, 90, 90);
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index c78cf7f219..2b775fc4cc 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -8688,6 +8688,11 @@ cmdSendKey(vshControl *ctl, const vshCmd *cmd)
     if (vshCommandOptUInt(ctl, cmd, "holdtime", &holdtime) < 0)
         goto cleanup;
 
+    /* The qnum codeset was originally called rfb, so we need to keep
+     * accepting the old name for backwards compatibility reasons */
+    if (STREQ(codeset_option, "rfb"))
+        codeset_option = "qnum";
+
     codeset = virKeycodeSetTypeFromString(codeset_option);
     if (codeset < 0) {
         vshError(ctl, _("unknown codeset: '%s'"), codeset_option);
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 8f0e8d74b0..378e26a20d 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -2249,14 +2249,14 @@ for keyboard input. No symbolic names are provided
 
 See L<virkeycode-usb(7)>
 
-=item B<rfb>
+=item B<qnum>
 
-The numeric values are those defined by the RFB extension for sending
+The numeric values are those defined by the QNUM extension for sending
 raw keycodes. These are a variant on the XT codeset, but extended
 keycodes have the low bit of the second byte set, instead of the high
 bit of the first byte. No symbolic names are provided.
 
-See L<virkeycode-rfb(7)>
+See L<virkeycode-qnum(7)>
 
 =back
 
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] keycodemapdb: Update submodule
Posted by Daniel P. Berrangé 7 years, 2 months ago 
On Mon, Mar 12, 2018 at 02:29:49PM +0100, Andrea Bolognani wrote:
> This time around it's not enough to just pick the latest commit,
> because with aed87bb2aa6ed83b49574eb982e3bdd4c36acf17 keycodemapdb
> renamed the 'rfb' keycode to 'qnum' and we need to accept the new
> name while maintaining backwards compatibility.
> 
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>

When I apply this I get an error during build due to the rename

Traceback (most recent call last):
  File "./keycodemapdb/tools/keymap-gen", line 1054, in <module>
    main()
  File "./keycodemapdb/tools/keymap-gen", line 1049, in main
    args.func(args)
  File "./keycodemapdb/tools/keymap-gen", line 945, in code_table
    SRC_GENERATORS[args.lang].generate_code_table(args.varname, database, args.mapname)
  File "./keycodemapdb/tools/keymap-gen", line 402, in generate_code_table
    mapname, ", ".join(database.mapto.keys())))
Exception: Unknown map rfb, expected one of xorgxwin, osx, qcode, usb, xkb, sun, linux, xkbdxt, adb, win32, html, atset1, xorgkbd, atset3, atset2, xwinxt, x11, qnum, xtkbd, xorgxquartz, xorgevdev


except that it doesn't actually seem to cause an error - it seems
to be ignored. I'm not sure why we see this, nor why it isn't
apparently fatal

We could simply keep libvirt calling it 'rfb' instead of 'qnum',
but 'qnum' does feel like a saner name

> ---
>  include/libvirt/libvirt-domain.h | 9 ++++++++-
>  src/keycodemapdb                 | 2 +-
>  src/qemu/qemu_driver.c           | 8 ++++----
>  src/util/Makefile.inc.am         | 2 +-
>  src/util/virkeycode.c            | 8 ++++----
>  tests/virkeycodetest.c           | 4 ++--
>  tools/virsh-domain.c             | 5 +++++
>  tools/virsh.pod                  | 6 +++---
>  8 files changed, 28 insertions(+), 16 deletions(-)
> 
> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
> index 4048acf38a..4128d55852 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -2733,7 +2733,7 @@ typedef enum {
>      VIR_KEYCODE_SET_XT_KBD         = 6,
>      VIR_KEYCODE_SET_USB            = 7,
>      VIR_KEYCODE_SET_WIN32          = 8,
> -    VIR_KEYCODE_SET_RFB            = 9,
> +    VIR_KEYCODE_SET_QNUM           = 9,
>  
>  # ifdef VIR_ENUM_SENTINELS
>      VIR_KEYCODE_SET_LAST
> @@ -2745,6 +2745,13 @@ typedef enum {
>  # endif
>  } virKeycodeSet;
>  
> +/**
> + * VIR_KEYCODE_SET_RFB:
> + *
> + * Compatibility alias for VIR_KEYCODE_SET_QNUM, which replaced it since 4.2.0.
> + */
> +# define VIR_KEYCODE_SET_RFB VIR_KEYCODE_SET_QNUM
> +
>  /**
>   * VIR_DOMAIN_SEND_KEY_MAX_KEYS:
>   *
> diff --git a/src/keycodemapdb b/src/keycodemapdb
> index 267157b96c..16e5b07876 160000
> --- a/src/keycodemapdb
> +++ b/src/keycodemapdb
> @@ -1 +1 @@
> -Subproject commit 267157b96c62b5445de9cddd21de42fcd943ffe6
> +Subproject commit 16e5b0787687d8904dad2c026107409eb9bfcb95
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index e13544f832..8c872c1f08 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -2580,17 +2580,17 @@ static int qemuDomainSendKey(virDomainPtr domain,
>  
>      virCheckFlags(0, -1);
>  
> -    /* translate the keycode to RFB for qemu driver */
> -    if (codeset != VIR_KEYCODE_SET_RFB) {
> +    /* translate the keycode to QNUM for qemu driver */
> +    if (codeset != VIR_KEYCODE_SET_QNUM) {
>          size_t i;
>          int keycode;
>  
>          for (i = 0; i < nkeycodes; i++) {
> -            keycode = virKeycodeValueTranslate(codeset, VIR_KEYCODE_SET_RFB,
> +            keycode = virKeycodeValueTranslate(codeset, VIR_KEYCODE_SET_QNUM,
>                                                 keycodes[i]);
>              if (keycode < 0) {
>                  virReportError(VIR_ERR_INTERNAL_ERROR,
> -                               _("cannot translate keycode %u of %s codeset to rfb keycode"),
> +                               _("cannot translate keycode %u of %s codeset to qnum keycode"),
>                                 keycodes[i],
>                                 virKeycodeSetTypeToString(codeset));
>                  return -1;
> diff --git a/src/util/Makefile.inc.am b/src/util/Makefile.inc.am
> index a91b30dca5..d0e1ec3625 100644
> --- a/src/util/Makefile.inc.am
> +++ b/src/util/Makefile.inc.am
> @@ -214,7 +214,7 @@ EXTRA_DIST += \
>  	$(NULL)
>  
>  
> -KEYCODES = linux osx atset1 atset2 atset3 xtkbd usb win32 rfb
> +KEYCODES = linux osx atset1 atset2 atset3 xtkbd usb win32 qnum
>  KEYNAMES = linux osx win32
>  
>  KEYTABLES = \
> diff --git a/src/util/virkeycode.c b/src/util/virkeycode.c
> index eda263218c..8976bbf376 100644
> --- a/src/util/virkeycode.c
> +++ b/src/util/virkeycode.c
> @@ -27,7 +27,7 @@
>  #include "virkeycodetable_atset3.h"
>  #include "virkeycodetable_linux.h"
>  #include "virkeycodetable_osx.h"
> -#include "virkeycodetable_rfb.h"
> +#include "virkeycodetable_qnum.h"
>  #include "virkeycodetable_usb.h"
>  #include "virkeycodetable_win32.h"
>  #include "virkeycodetable_xtkbd.h"
> @@ -52,7 +52,7 @@ static const unsigned short *virKeymapValues[VIR_KEYCODE_SET_LAST] = {
>      [VIR_KEYCODE_SET_XT_KBD] = virKeyCodeTable_xtkbd,
>      [VIR_KEYCODE_SET_USB] = virKeyCodeTable_usb,
>      [VIR_KEYCODE_SET_WIN32] = virKeyCodeTable_win32,
> -    [VIR_KEYCODE_SET_RFB] = virKeyCodeTable_rfb,
> +    [VIR_KEYCODE_SET_QNUM] = virKeyCodeTable_qnum,
>  };
>  
>  #define VIR_KEYMAP_ENTRY_MAX ARRAY_CARDINALITY(virKeyCodeTable_linux)
> @@ -64,7 +64,7 @@ verify(VIR_KEYMAP_ENTRY_MAX == ARRAY_CARDINALITY(virKeyCodeTable_osx));
>  verify(VIR_KEYMAP_ENTRY_MAX == ARRAY_CARDINALITY(virKeyCodeTable_xtkbd));
>  verify(VIR_KEYMAP_ENTRY_MAX == ARRAY_CARDINALITY(virKeyCodeTable_usb));
>  verify(VIR_KEYMAP_ENTRY_MAX == ARRAY_CARDINALITY(virKeyCodeTable_win32));
> -verify(VIR_KEYMAP_ENTRY_MAX == ARRAY_CARDINALITY(virKeyCodeTable_rfb));
> +verify(VIR_KEYMAP_ENTRY_MAX == ARRAY_CARDINALITY(virKeyCodeTable_qnum));
>  verify(VIR_KEYMAP_ENTRY_MAX == ARRAY_CARDINALITY(virKeyNameTable_linux));
>  verify(VIR_KEYMAP_ENTRY_MAX == ARRAY_CARDINALITY(virKeyNameTable_osx));
>  verify(VIR_KEYMAP_ENTRY_MAX == ARRAY_CARDINALITY(virKeyNameTable_win32));
> @@ -79,7 +79,7 @@ VIR_ENUM_IMPL(virKeycodeSet, VIR_KEYCODE_SET_LAST,
>      "xt_kbd",
>      "usb",
>      "win32",
> -    "rfb",
> +    "qnum",
>  );
>  
>  int virKeycodeValueFromString(virKeycodeSet codeset,
> diff --git a/tests/virkeycodetest.c b/tests/virkeycodetest.c
> index 24887a36d4..399a13fcae 100644
> --- a/tests/virkeycodetest.c
> +++ b/tests/virkeycodetest.c
> @@ -54,8 +54,8 @@ static int testKeycodeMapping(const void *data ATTRIBUTE_UNUSED)
>  
>      TRANSLATE(LINUX, LINUX, 111, 111);
>      TRANSLATE(LINUX, USB, 111, 76);
> -    TRANSLATE(LINUX, RFB, 88, 88);
> -    TRANSLATE(LINUX, RFB, 160, 163);
> +    TRANSLATE(LINUX, QNUM, 88, 88);
> +    TRANSLATE(LINUX, QNUM, 160, 163);
>      TRANSLATE(ATSET2, ATSET3, 131, 55);
>      TRANSLATE(OSX, WIN32, 90, 131);
>      TRANSLATE(OSX, ATSET1, 90, 90);
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index c78cf7f219..2b775fc4cc 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -8688,6 +8688,11 @@ cmdSendKey(vshControl *ctl, const vshCmd *cmd)
>      if (vshCommandOptUInt(ctl, cmd, "holdtime", &holdtime) < 0)
>          goto cleanup;
>  
> +    /* The qnum codeset was originally called rfb, so we need to keep
> +     * accepting the old name for backwards compatibility reasons */
> +    if (STREQ(codeset_option, "rfb"))
> +        codeset_option = "qnum";
> +
>      codeset = virKeycodeSetTypeFromString(codeset_option);
>      if (codeset < 0) {
>          vshError(ctl, _("unknown codeset: '%s'"), codeset_option);
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index 8f0e8d74b0..378e26a20d 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -2249,14 +2249,14 @@ for keyboard input. No symbolic names are provided
>  
>  See L<virkeycode-usb(7)>
>  
> -=item B<rfb>
> +=item B<qnum>
>  
> -The numeric values are those defined by the RFB extension for sending
> +The numeric values are those defined by the QNUM extension for sending
>  raw keycodes. These are a variant on the XT codeset, but extended
>  keycodes have the low bit of the second byte set, instead of the high
>  bit of the first byte. No symbolic names are provided.
>  
> -See L<virkeycode-rfb(7)>
> +See L<virkeycode-qnum(7)>
>  
>  =back
>  
> -- 
> 2.14.3
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] keycodemapdb: Update submodule
Posted by Andrea Bolognani 7 years, 2 months ago 
On Mon, 2018-03-12 at 14:00 +0000, Daniel P. Berrangé wrote:
> On Mon, Mar 12, 2018 at 02:29:49PM +0100, Andrea Bolognani wrote:
> > This time around it's not enough to just pick the latest commit,
> > because with aed87bb2aa6ed83b49574eb982e3bdd4c36acf17 keycodemapdb
> > renamed the 'rfb' keycode to 'qnum' and we need to accept the new
> > name while maintaining backwards compatibility.
> > 
> > Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> 
> When I apply this I get an error during build due to the rename
> 
> Traceback (most recent call last):
>   File "./keycodemapdb/tools/keymap-gen", line 1054, in <module>
>     main()
>   File "./keycodemapdb/tools/keymap-gen", line 1049, in main
>     args.func(args)
>   File "./keycodemapdb/tools/keymap-gen", line 945, in code_table
>     SRC_GENERATORS[args.lang].generate_code_table(args.varname, database, args.mapname)
>   File "./keycodemapdb/tools/keymap-gen", line 402, in generate_code_table
>     mapname, ", ".join(database.mapto.keys())))
> Exception: Unknown map rfb, expected one of xorgxwin, osx, qcode, usb, xkb, sun, linux, xkbdxt, adb, win32, html, atset1, xorgkbd, atset3, atset2, xwinxt,
x11, qnum, xtkbd, xorgxquartz, xorgevdev
> 
> 
> except that it doesn't actually seem to cause an error - it seems
> to be ignored. I'm not sure why we see this, nor why it isn't
> apparently fatal

The build system doesn't get refreshed automatically after this
change, so it's still trying to generate virkeycodetable_rfb.h
and failing because the current version of keymap-gen doesn't know
about it.

It should work properly again after running 'git clean -xdf'; there
might be a way to trigger automatic build system refresh but it
didn't look like something worth spending too much time on.

> We could simply keep libvirt calling it 'rfb' instead of 'qnum',
> but 'qnum' does feel like a saner name

Yeah, I think we should try to avoid diverging from whatever
naming convention keycodemapdb has adopted. The couple of tweaks
that I've made should be enough to ensure existing users keep on
working even after the rename.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] keycodemapdb: Update submodule
Posted by Daniel P. Berrangé 7 years, 2 months ago 
On Mon, Mar 12, 2018 at 03:30:33PM +0100, Andrea Bolognani wrote:
> On Mon, 2018-03-12 at 14:00 +0000, Daniel P. Berrangé wrote:
> > On Mon, Mar 12, 2018 at 02:29:49PM +0100, Andrea Bolognani wrote:
> > > This time around it's not enough to just pick the latest commit,
> > > because with aed87bb2aa6ed83b49574eb982e3bdd4c36acf17 keycodemapdb
> > > renamed the 'rfb' keycode to 'qnum' and we need to accept the new
> > > name while maintaining backwards compatibility.
> > > 
> > > Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> > 
> > When I apply this I get an error during build due to the rename
> > 
> > Traceback (most recent call last):
> >   File "./keycodemapdb/tools/keymap-gen", line 1054, in <module>
> >     main()
> >   File "./keycodemapdb/tools/keymap-gen", line 1049, in main
> >     args.func(args)
> >   File "./keycodemapdb/tools/keymap-gen", line 945, in code_table
> >     SRC_GENERATORS[args.lang].generate_code_table(args.varname, database, args.mapname)
> >   File "./keycodemapdb/tools/keymap-gen", line 402, in generate_code_table
> >     mapname, ", ".join(database.mapto.keys())))
> > Exception: Unknown map rfb, expected one of xorgxwin, osx, qcode, usb, xkb, sun, linux, xkbdxt, adb, win32, html, atset1, xorgkbd, atset3, atset2, xwinxt,
> x11, qnum, xtkbd, xorgxquartz, xorgevdev
> > 
> > 
> > except that it doesn't actually seem to cause an error - it seems
> > to be ignored. I'm not sure why we see this, nor why it isn't
> > apparently fatal
> 
> The build system doesn't get refreshed automatically after this
> change, so it's still trying to generate virkeycodetable_rfb.h
> and failing because the current version of keymap-gen doesn't know
> about it.

Perhaps it would be better if I just made 'keymap-gen' continue to
accept 'rfb' as a deprecated alias for 'qnum'

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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] keycodemapdb: Update submodule
Posted by Andrea Bolognani 7 years, 2 months ago 
On Mon, 2018-03-12 at 14:46 +0000, Daniel P. Berrangé wrote:
> > The build system doesn't get refreshed automatically after this
> > change, so it's still trying to generate virkeycodetable_rfb.h
> > and failing because the current version of keymap-gen doesn't know
> > about it.
> 
> Perhaps it would be better if I just made 'keymap-gen' continue to
> accept 'rfb' as a deprecated alias for 'qnum'

That would probably be nice regardless, but it wouldn't really help
here - unless we decided to stick with the old name in libvirt too.
Are you advocating for doing just that?

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] keycodemapdb: Update submodule
Posted by Daniel P. Berrangé 7 years, 2 months ago 
On Mon, Mar 12, 2018 at 03:53:09PM +0100, Andrea Bolognani wrote:
> On Mon, 2018-03-12 at 14:46 +0000, Daniel P. Berrangé wrote:
> > > The build system doesn't get refreshed automatically after this
> > > change, so it's still trying to generate virkeycodetable_rfb.h
> > > and failing because the current version of keymap-gen doesn't know
> > > about it.
> > 
> > Perhaps it would be better if I just made 'keymap-gen' continue to
> > accept 'rfb' as a deprecated alias for 'qnum'
> 
> That would probably be nice regardless, but it wouldn't really help
> here - unless we decided to stick with the old name in libvirt too.
> Are you advocating for doing just that?

No, just take your patch as is. Just fix keymap-gen, so we make the
(harmless) error message disappear for people with existing checkouts
that have built the old filename.

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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] keycodemapdb: Update submodule
Posted by Andrea Bolognani 7 years, 2 months ago 
On Mon, 2018-03-12 at 14:58 +0000, Daniel P. Berrangé wrote:
> On Mon, Mar 12, 2018 at 03:53:09PM +0100, Andrea Bolognani wrote:
> > On Mon, 2018-03-12 at 14:46 +0000, Daniel P. Berrangé wrote:
> > > > The build system doesn't get refreshed automatically after this
> > > > change, so it's still trying to generate virkeycodetable_rfb.h
> > > > and failing because the current version of keymap-gen doesn't know
> > > > about it.
> > > 
> > > Perhaps it would be better if I just made 'keymap-gen' continue to
> > > accept 'rfb' as a deprecated alias for 'qnum'
> > 
> > That would probably be nice regardless, but it wouldn't really help
> > here - unless we decided to stick with the old name in libvirt too.
> > Are you advocating for doing just that?
> 
> No, just take your patch as is. Just fix keymap-gen, so we make the
> (harmless) error message disappear for people with existing checkouts
> that have built the old filename.

But that still wouldn't work seamlessly, would it? The existing
build system would try to generate virkeycodetable_rfb.h, and
succeed, but the code won't compile because it's looking for
virkeycodetable_qnum.h and related symbols now.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] keycodemapdb: Update submodule
Posted by Daniel P. Berrangé 7 years, 2 months ago 
On Mon, Mar 12, 2018 at 04:03:59PM +0100, Andrea Bolognani wrote:
> On Mon, 2018-03-12 at 14:58 +0000, Daniel P. Berrangé wrote:
> > On Mon, Mar 12, 2018 at 03:53:09PM +0100, Andrea Bolognani wrote:
> > > On Mon, 2018-03-12 at 14:46 +0000, Daniel P. Berrangé wrote:
> > > > > The build system doesn't get refreshed automatically after this
> > > > > change, so it's still trying to generate virkeycodetable_rfb.h
> > > > > and failing because the current version of keymap-gen doesn't know
> > > > > about it.
> > > > 
> > > > Perhaps it would be better if I just made 'keymap-gen' continue to
> > > > accept 'rfb' as a deprecated alias for 'qnum'
> > > 
> > > That would probably be nice regardless, but it wouldn't really help
> > > here - unless we decided to stick with the old name in libvirt too.
> > > Are you advocating for doing just that?
> > 
> > No, just take your patch as is. Just fix keymap-gen, so we make the
> > (harmless) error message disappear for people with existing checkouts
> > that have built the old filename.
> 
> But that still wouldn't work seamlessly, would it? The existing
> build system would try to generate virkeycodetable_rfb.h, and
> succeed, but the code won't compile because it's looking for
> virkeycodetable_qnum.h and related symbols now.

The resulting virkeycodetable_rfb.h would never be used - it would only
be generated because the automagic .deps files automake creates still
list the old filename. In fact on that basis I think we can just ignore
the failure - it is self-correcting - automake triggers re-generation of
the old filename, and that's non-fatal because we actally use the new
filename. So just push your current patch as is

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

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 :|

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

Patchew 2.1-devel-8c64711

© 2016 - 2025 Red Hat, Inc.