[PATCH] extras/keymap: fix build (GEN keys.txt)

[PATCH] extras/keymap: fix build (GEN keys.txt)

[Alan Jenkins]

Commit 7d701b0 "Merge keymap building in the top-level Makefile.am"
caused a build failure when extras are enabled.

Specifically, it changed the command used to generate keys.txt in several
ways.  The most obvious change was that the output was completely empty :).
This was probably missed due to stale Makefiles.  Hint: to reproduce the
build failure, first run "make distclean".

Fix the generation of keys.txt by reverting to the original command.

Also add some dependencies on the Makefile.  This means keys.txt will be
rebuilt automatically if the command is changed in future.

Build tested only.  I don't use udev-keymap, so it would a good idea for
someone who does to test it (Diego?) :).

Signed-off-by: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
CC: Diego Elio 'Flameeyes' Pettenò <flameeyes@gmail.com>
---
 Makefile.am |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index cf7d364..a439157 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -530,10 +530,10 @@ dist_udevkeymap_DATA = \
 	extras/keymap/keymaps/toshiba-satellite_a110 \
 	extras/keymap/keymaps/zepto-znote
 
-extras/keymap/keys.txt: /usr/include/linux/input.h
-	$(AM_V_GEN)$(AWK) '/^#define.*KEY_[^ ]+[[:space:]]+[0-9]/ { if ($$2 != "KEY_MAX") { print $$2 } }' < $< | sed 's/^KEY_COFFEE$$/KEY_SCREENLOCK/' > $@
+extras/keymap/keys.txt: /usr/include/linux/input.h Makefile
+	$(AM_V_GEN)$(AWK) '/^#define.*KEY_/ { if ($$2 != "KEY_MAX" && $$2 != "KEY_CNT") { print $$2 } }' < $< > $@
 
-extras/keymap/keys-from-name.gperf: extras/keymap/keys.txt
+extras/keymap/keys-from-name.gperf: extras/keymap/keys.txt Makefile
 	$(AM_V_GEN)$(AWK) 'BEGIN{ print "struct key { const char* name; unsigned short id; };"; print "%null-strings"; print "%%";} { print $$1 ", " $$1 }' < $< > $@
 
 extras/keymap/keys-from-name.h: extras/keymap/keys-from-name.gperf Makefile
-- 
1.5.6.5

Re: [PATCH] extras/keymap: fix build (GEN keys.txt)

[Lennart Poettering]

On Thu, 29.10.09 14:27, Alan Jenkins (alan-jenkins@tuffmail.co.uk) wrote:

> Commit 7d701b0 "Merge keymap building in the top-level Makefile.am"
> caused a build failure when extras are enabled.
> 
> Fix the generation of keys.txt by reverting to the original command.
> -extras/keymap/keys.txt: /usr/include/linux/input.h
> -	$(AM_V_GEN)$(AWK) '/^#define.*KEY_[^ ]+[[:space:]]+[0-9]/ { if ($$2 != "KEY_MAX") { print $$2 } }' < $< | sed 's/^KEY_COFFEE$$/KEY_SCREENLOCK/' > $@
> +extras/keymap/keys.txt: /usr/include/linux/input.h Makefile
> +	$(AM_V_GEN)$(AWK) '/^#define.*KEY_/ { if ($$2 != "KEY_MAX" &&
> $$2 != "KEY_CNT") { print $$2 } }' < $< > $@

The s/^KEY_COFFEE$$/KEY_SCREENLOCK/ you dropped here actually
matters. It needs to be kept.

Lennart

-- 
Lennart Poettering                        Red Hat, Inc.
lennart [at] poettering [dot] net
http://0pointer.net/lennart/           GnuPG 0x1A015CC4

Re: [PATCH] extras/keymap: fix build (GEN keys.txt)

[Alan Jenkins]

Lennart Poettering wrote:
> On Thu, 29.10.09 14:27, Alan Jenkins (alan-jenkins@tuffmail.co.uk) wrote:
>
>   
>> Commit 7d701b0 "Merge keymap building in the top-level Makefile.am"
>> caused a build failure when extras are enabled.
>>
>> Fix the generation of keys.txt by reverting to the original command.
>> -extras/keymap/keys.txt: /usr/include/linux/input.h
>> -	$(AM_V_GEN)$(AWK) '/^#define.*KEY_[^ ]+[[:space:]]+[0-9]/ { if ($$2 != "KEY_MAX") { print $$2 } }' < $< | sed 's/^KEY_COFFEE$$/KEY_SCREENLOCK/' > $@
>> +extras/keymap/keys.txt: /usr/include/linux/input.h Makefile
>> +	$(AM_V_GEN)$(AWK) '/^#define.*KEY_/ { if ($$2 != "KEY_MAX" &&
>> $$2 != "KEY_CNT") { print $$2 } }' < $< > $@
>>     
>
> The s/^KEY_COFFEE$$/KEY_SCREENLOCK/ you dropped here actually
> matters. It needs to be kept.
>   

I'm not dropping anything that was ever used, or which ever worked, at 
least within the main udev tree :-P.  This patch just reverts to the 
last known working version of the command.

I would suggest applying any necessary changes as separate commits.  
Sorry for not being more explicit about this in the first place :).

Note that this specific sed command doesn't seem to work for my system:

      GEN    extras/keymap/keys-from-name.gperf
      GEN    extras/keymap/keys-from-name.h
    Key link: "KEY_SCREENLOCK" = "KEY_SCREENLOCK", with key set "cekss".
    1 input keys have identical hash values,
    use option -D.
    make: *** [extras/keymap/keys-from-name.h] Error 1

This is with /usr/include/linux/input.h as provided by debian 
"linux-libc-dev-2.6.30-8".

Thanks
Alan

Re: [PATCH] extras/keymap: fix build (GEN keys.txt)

[Martin Pitt]

Hello Alan,

Alan Jenkins [2009-10-29 14:27 +0000]:
> Specifically, it changed the command used to generate keys.txt in several
> ways.  The most obvious change was that the output was completely empty :).
> This was probably missed due to stale Makefiles.  Hint: to reproduce the
> build failure, first run "make distclean".

Hm, I'm autoreconfing and/or rebuilding the udev tree all the time,
without problems. We also have this in our packages in Ubuntu, and it
produces valid and working files.
Can you please give some precise steps how it fails?

> -     $(AM_V_GEN)$(AWK) '/^#define.*KEY_[^ ]+[[:space:]]+[0-9]/ { if ($$2 != "KEY_MAX") { print $$2 } }' < $< | sed 's/^KEY_COFFEE$$/KEY_SCREENLOCK/' > $@
> +	$(AM_V_GEN)$(AWK) '/^#define.*KEY_/ { if ($$2 != "KEY_MAX" && $$2 != "KEY_CNT") { print $$2 } }' < $< > $@

As Lennart already pointed out, the COFFEE->SCREENLOCK sed is
important to not break existing keymaps (commit 7f06ec2e19). Also,
your change drops the check that it's a real key definition instead of
an alias definition, and thus generates duplicate hash values. That
wasn't changed in the move to single top-level Makefile.am, but was an
explicit bug fix (commit 6983c0d0f).

> -extras/keymap/keys.txt: /usr/include/linux/input.h
> +extras/keymap/keys.txt: /usr/include/linux/input.h Makefile

> -extras/keymap/keys-from-name.gperf: extras/keymap/keys.txt
> +extras/keymap/keys-from-name.gperf: extras/keymap/keys.txt Makefile

Why is that necessary? You shouldn't change the Makefile manually, it
should at most depend on Makefile.am changes (but even that is not
necessary since it just calls gperf, which doesn't care about any
other file in the tree).

Thanks,

Martin

-- 
Martin Pitt                        | http://www.piware.de
Ubuntu Developer (www.ubuntu.com)  | Debian Developer  (www.debian.org)

Re: [PATCH] extras/keymap: fix build (GEN keys.txt)

[Alan Jenkins]

Martin Pitt wrote:
> Hello Alan,
>
> Alan Jenkins [2009-10-29 14:27 +0000]:
>   
>> Specifically, it changed the command used to generate keys.txt in several
>> ways.  The most obvious change was that the output was completely empty :).
>> This was probably missed due to stale Makefiles.  Hint: to reproduce the
>> build failure, first run "make distclean".
>>     
>
> Hm, I'm autoreconfing and/or rebuilding the udev tree all the time,
> without problems. We also have this in our packages in Ubuntu, and it
> produces valid and working files.
> Can you please give some precise steps how it fails?
>   

Yup.  I tried cloning and building a fresh checkout (./autogen.sh; 
make), and it failed with the same problem.

>   
>> -     $(AM_V_GEN)$(AWK) '/^#define.*KEY_[^ ]+[[:space:]]+[0-9]/ { if ($$2 != "KEY_MAX") { print $$2 } }' < $< | sed 's/^KEY_COFFEE$$/KEY_SCREENLOCK/' > $@
>> +	$(AM_V_GEN)$(AWK) '/^#define.*KEY_/ { if ($$2 != "KEY_MAX" && $$2 != "KEY_CNT") { print $$2 } }' < $< > $@
>>     
>
> As Lennart already pointed out, the COFFEE->SCREENLOCK sed is
> important to not break existing keymaps (commit 7f06ec2e19). Also,
> your change drops the check that it's a real key definition instead of
> an alias definition, and thus generates duplicate hash values. That
> wasn't changed in the move to single top-level Makefile.am, but was an
> explicit bug fix (commit 6983c0d0f).
>
>   
>> -extras/keymap/keys.txt: /usr/include/linux/input.h
>> +extras/keymap/keys.txt: /usr/include/linux/input.h Makefile
>>     
>
>   
>> -extras/keymap/keys-from-name.gperf: extras/keymap/keys.txt
>> +extras/keymap/keys-from-name.gperf: extras/keymap/keys.txt Makefile
>>     
>
> Why is that necessary? You shouldn't change the Makefile manually, it
> should at most depend on Makefile.am changes (but even that is not
> necessary since it just calls gperf, which doesn't care about any
> other file in the tree).
>
> Thanks,
>
> Martin
>   

Thanks for the corrections Martin.  Looks like the real problem is my 
awk doesn't grok "[[:space:]]".

$ awk '/^#define.*KEY_[^ ]+[[:space:]]+[0-9]/ { if ($2 != "KEY_MAX") { 
print $2 } }' < /usr/include/linux/input.h
$


$ awk '/^#define.*KEY_[^ ]+[[:space:]]/ { if ($2 != "KEY_MAX") { print 
$2 } }' < /usr/include/linux/input.h
$


$ awk '/^#define.*KEY_[^ ]+/ { if ($2 != "KEY_MAX") { print $2 } }' < 
/usr/include/linux/input.h
KEY_RESERVED
KEY_ESC
KEY_1
...

My awk package is mawk-1.3.3-11.1.

Thanks again
Alan

Re: [PATCH] extras/keymap: fix build (GEN keys.txt)

[@ 2009-10-29 20:17 UTC (permalink / raw)]

Il giorno Thu, 29/10/2009 alle 20.10 +0000, Alan Jenkins ha scritto:
> 
> Thanks for the corrections Martin.  Looks like the real problem is my 
> awk doesn't grok "[[:space:]]".

> My awk package is mawk-1.3.3-11.1.
> 
> 
Heh, no wonder then, [[:space:]] is a GNU regexp extension, which is
thus only available on GNU awk (gawk); who's going to choose between
checking for gawk explicitly or make it use a generic regexp?

-- 
Diego Elio Pettenò — “Flameeyes”
http://blog.flameeyes.eu/

If you found a .asc file in this mail and know not what it is,
it's a GnuPG digital signature: http://www.gnupg.org/

Re: [PATCH] extras/keymap: fix build (GEN keys.txt)

[Dan Nicholson]

On Thu, Oct 29, 2009 at 1:17 PM, Diego Elio “Flameeyes”
<flameeyes@gmail.com> wrote:
> Il giorno Thu, 29/10/2009 alle 20.10 +0000, Alan Jenkins ha scritto:
>>
>> Thanks for the corrections Martin.  Looks like the real problem is my
>> awk doesn't grok "[[:space:]]".
>
>> My awk package is mawk-1.3.3-11.1.
>>
>>
> Heh, no wonder then, [[:space:]] is a GNU regexp extension, which is
> thus only available on GNU awk (gawk); who's going to choose between
> checking for gawk explicitly or make it use a generic regexp?

gawk(1) says that the character classes are a POSIX extension, but
whatever. Isn't [ \t] equivalent for this case? That works on my
system running gawk with --traditional, but I don't have mawk.

--
Dan

Re: [PATCH] extras/keymap: fix build (GEN keys.txt)

[@ 2009-10-29 20:46 UTC (permalink / raw)]

Il giorno Thu, 29/10/2009 alle 13.40 -0700, Dan Nicholson ha scritto:
> 
> gawk(1) says that the character classes are a POSIX extension, but
> whatever. Isn't [ \t] equivalent for this case? That works on my
> system running gawk with --traditional, but I don't have mawk. 

Ehm yes I noticed now, I thought it was GNU-specific, my bad sorry!

-- 
Diego Elio Pettenò — “Flameeyes”
http://blog.flameeyes.eu/

If you found a .asc file in this mail and know not what it is,
it's a GnuPG digital signature: http://www.gnupg.org/

Re: [PATCH] extras/keymap: fix build (GEN keys.txt)

[Alan Jenkins]

Diego Elio “Flameeyes” Pettenò wrote:
> Il giorno Thu, 29/10/2009 alle 13.40 -0700, Dan Nicholson ha scritto:
>   
>> gawk(1) says that the character classes are a POSIX extension, but
>> whatever. Isn't [ \t] equivalent for this case? That works on my
>> system running gawk with --traditional, but I don't have mawk. 
>>     
Yes, that works fine for mawk.

Thanks
Alan

Re: [PATCH] extras/keymap: fix build (GEN keys.txt)

[Martin Pitt]

[-- Attachment #1: Type: text/plain, Size: 429 bytes --]

Hello Alan,

Alan Jenkins [2009-10-29 20:10 +0000]:
> Looks like the real problem is my awk doesn't grok "[[:space:]]".

Ah, that would be it then.

Fixed in

  http://git.kernel.org/?p=linux/hotplug/udev.git;a=commit;h=82c6558e0109f66091e3a8e2c4ceef0372b0b331

Thanks!

Martin
-- 
Martin Pitt                        | http://www.piware.de
Ubuntu Developer (www.ubuntu.com)  | Debian Developer  (www.debian.org)

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]