Closed kloczek closed 2 years ago
legionus
commented
2 years ago This testsuite passed: https://github.com/legionus/kbd/runs/6709846783?check_suite_focus=true#step%3A7%3A155=
This testcase is architecture specific. On what architecture are you running your tests?
kloczek
commented
2 years ago x86/64 So what could be possible cause that thisd unit is failing? FYI: I'm building and testing kbd inside LXC zone.
legionus
commented
2 years ago Here is a log:
27. libkeymap.at:170: testing binary keymap (cz.map) ...
./libkeymap.at:176: $abs_top_builddir/src/loadkeys -b "$abs_top_srcdir/data/keymaps/i386/qwerty/cz.map"
--- /dev/null 2022-05-22 20:47:22.365061056 +0000
+++ /home/tkloczko/rpmbuild/BUILD/kbd-2.5.0/tests/testsuite.dir/at-groups/27/stderr 2022-06-02 15:28:35.878405356 +0000
@@ -0,0 +1 @@
+loadkeys: Unable to open file: /home/tkloczko/rpmbuild/BUILD/kbd-2.5.0/data/keymaps/i386/qwerty/cz.map: No such file or directory
Binary files expout and /home/tkloczko/rpmbuild/BUILD/kbd-2.5.0/tests/testsuite.dir/at-groups/27/stdout differ
./libkeymap.at:176: exit code was 1, expected 0
27. libkeymap.at:170: 27. binary keymap (cz.map) (libkeymap.at:170): FAILED (libkeymap.at:176)Hm. For some reason, test did not find the data/keymaps/i386/qwerty/cz.map
legionus
commented
2 years ago @kloczek Is there a possibility that something in the rpm spec-file removed or moved it?
thesamesam
commented
2 years ago Yeah, I hit the same thing on Gentoo just now, and it looks like both Fedora (and us in Gentoo) are renaming some of them.
Fedora's spec file:
[...]
# Rename conflicting keymaps
mv fgGIod/trf.map fgGIod/trf-fgGIod.map
mv olpc/es.map olpc/es-olpc.map
mv olpc/pt.map olpc/pt-olpc.map
mv qwerty/cz.map qwerty/cz-qwerty.map
popd
</pre>And in our ebuild in Gentoo:
# Rename conflicting keymaps to have unique names, bug #293228
cd "${S}"/data/keymaps/i386 || die
mv fgGIod/trf.map fgGIod/trf-fgGIod.map || die
mv olpc/es.map olpc/es-olpc.map || die
mv olpc/pt.map olpc/pt-olpc.map || die
mv qwerty/cz.map qwerty/cz-qwerty.map || dieMy own spec file initially based on Fedora one. In my case %prep looks like below
VCS: https://github.com/legionus/kbd/
Source0: %{VCS}/archive/v%{version}/%{name}-%{version}.tar.gz
Source1: %{name}-latsun-fonts.tar.bz2
Source2: %{name}-latarcyrheb-32.tar.bz2
Source3: %{name}-xml2lst.pl
Source4: %{name}-vlock.pamd
Source5: %{name}info.1
Source6: %{name}-cz-map.patch
Patch00: %{name}-keycodes-man.patch
Patch01: %{name}-sparc.patch
Patch02: %{name}-unicode_start.patch
Patch03: %{name}-dumpkeys-man.patch
Patch04: %{name}-sg-decimal-separator.patch
Patch05: %{name}-loadkeys-search-path.patch
Patch06: %{name}-unicode-start-font.patch
Patch07: %{name}-remove_using_compiler_warning_flags.patch
Patch08: %{name}-POTFILES.in.patch
Patch09: %{VCS}/commit/3cd7aa69.patch#/%{name}-Added-braces-to-IT-keyboard-map.patch
Patch10: %{VCS}/commit/1be18aa6.patch#/%{name}-src-libkeymap-better-error-message-on-unsupported-un.patch
Patch11: %{VCS}/commit/df07510a.patch#/%{name}-setfont-propagate-error-exit-code.patch
Patch12: %{VCS}/commit/f0af5384.patch#/%{name}-libkeymap-check-get_sym-return-value.patch
Patch13: %{VCS}/commit/ef2893f5.patch#/%{name}-libkbdfile-fix-memory-leak.patch
Patch14: %{VCS}/commit/8da81cc6.patch#/%{name}-libkbdfile-make-sure-there-is-space-for-terminating-.patch
Patch15: %{VCS}/commit/28afe8b8.patch#/%{name}-libkeymap-fix-memory-leak.patch
Patch16: %{VCS}/commit/42bd43c7.patch#/%{name}-libkfont-fix-memory-leak.patch
Patch17: %{VCS}/commit/24256f90.patch#/%{name}-data-keymaps-i386-neo-use-Delete-instead-of-Backspac.patch
Patch18: %{name}-fa.map-drop-high-codepoint-character-that-chokes-loa.patch
[..]
%prep
%setup -q -a1 -a2
%autopatch -p1
%__cp -fp %{S:3} xml2lst.pl
%__cp -fp %{S:6} .
cd data/keymaps/i386
%__cp qwerty/pt-latin9.map qwerty/pt.map
%__cp qwerty/sv-latin1.map qwerty/se-latin1.map
%__mv azerty/fr.map azerty/fr-old.map
%__cp azerty/fr-latin9.map azerty/fr.map
%__cp azerty/fr-latin9.map azerty/fr-latin0.map # legacy alias
%__mv fgGIod/trf.map fgGIod/trf-fgGIod.map
%__mv olpc/es.map olpc/es-olpc.map
%__mv olpc/pt.map olpc/pt-olpc.map
%__mv qwerty/cz.map qwerty/cz-qwerty.map
cd -
%build
autoreconf -fiv
%configure \
%{nil}
%make_build -C po update-po
%make_build
%install
%make_install
%__install %{S:4} -Dm644 %{buildroot}/etc/pam.d/vlock
%__install %{S:5} %{buildroot}/%{_mandir}/man1/kbdinfo.1
%__ln_s sr-cy.map.gz %{buildroot}%{_datadir}/keymaps/i386/qwerty/sr-latin.map.gz
%__ln_s us.map.gz %{buildroot}%{_datadir}/keymaps/i386/qwerty/ko.map.gz
%__ln_s fa.map.gz %{buildroot}%{_datadir}/keymaps/i386/qwerty/ara.map.gz
# Some microoptimization
%__sed -i 's,\<kbd_mode\>,%{_bindir}/kbd_mode,g;s,\<setfont\>,%{_bindir}/setfont,g' \
%{buildroot}%{_bindir}/unicode_start
%__mkdir_p %{buildroot}%{_datadir}/keymaps/legacy
%__mv %{buildroot}%{_datadir}/keymaps/{amiga,atari,i386,include,mac,ppc,sun} %{buildroot}%{_datadir}/keymaps/legacy
# Convert X keyboard layouts to console keymaps
%__mkdir_p %{buildroot}%{_datadir}/keymaps/xkb
%__perl xml2lst.pl < %{_datadir}/X11/xkb/rules/base.xml > layouts-variants.lst
while read line; do
XKBLAYOUT=`echo "$line" | cut -d " " -f 1`
echo "$XKBLAYOUT" >> layouts-list.lst
XKBVARIANT=`echo "$line" | cut -d " " -f 2`
ckbcomp "$XKBLAYOUT" "$XKBVARIANT" | gzip > %{buildroot}%{_datadir}/keymaps/xkb/"$XKBLAYOUT"-"$XKBVARIANT".map.gz
done < layouts-variants.lst
# Convert X keyboard layouts (plain, no variant)
cat layouts-list.lst | sort -u >> layouts-list-uniq.lst
while read line; do
ckbcomp "$line" | gzip > %{buildroot}%{_datadir}/keymaps/xkb/"$line".map.gz
done < layouts-list-uniq.lst
# wipe converted layouts which cannot input ASCII (#1031848)
zgrep -L "U+0041" %{buildroot}%{_datadir}/keymaps/xkb/* | xargs %__rm -fv
%__gzip -d %{buildroot}%{_datadir}/keymaps/xkb/cz.map.gz
%__patch %{buildroot}%{_datadir}/keymaps/xkb/cz.map < %{S:6}
%__gzip %{buildroot}%{_datadir}/keymaps/xkb/cz.map
%find_lang %{name}
%check
%make_build checkThe best situation would be to be able completely skip almost everything to have only something like
%prep
%autosetup -p1
%build
autoreconf -fiv
%configure \
%{nil}
%make_build -C po update-po
%make_build
%install
%make_installFedora has as well some patches which I'm using as well. It would be good to have situation in which none of those patches or additional steps on source tree preparation, build and install will be necessary.
kloczek
commented
2 years ago Ups I've copied my kbd.spec for 2.4. Here is updated part for 2.5
VCS: https://github.com/legionus/kbd/
Source0: %{VCS}/archive/v%{version}/%{name}-%{version}.tar.gz
Source1: %{name}-latsun-fonts.tar.bz2
Source2: %{name}-latarcyrheb-32.tar.bz2
Source3: %{name}-xml2lst.pl
Source4: %{name}-vlock.pamd
Source5: %{name}info.1
Source6: %{name}-cz-map.patch
Patch: %{name}-keycodes-man.patch
Patch: %{name}-sparc.patch
Patch: %{name}-unicode_start.patch
Patch: %{name}-sg-decimal-separator.patch
Patch: %{name}-loadkeys-search-path.patch
Patch: %{name}-unicode-start-font.patch
Patch: %{name}-remove_using_compiler_warning_flags.patch
Patch: %{name}-POTFILES.in.patch
[..]
%prep
%setup -q -a1 -a2
%autopatch -p1
%__cp -fp %{S:3} xml2lst.pl
%__cp -fp %{S:6} .
cd data/keymaps/i386
%__cp qwerty/pt-latin9.map qwerty/pt.map
%__cp qwerty/sv-latin1.map qwerty/se-latin1.map
%__mv azerty/fr.map azerty/fr-old.map
%__cp azerty/fr-latin9.map azerty/fr.map
%__cp azerty/fr-latin9.map azerty/fr-latin0.map # legacy alias
%__mv fgGIod/trf.map fgGIod/trf-fgGIod.map
%__mv olpc/es.map olpc/es-olpc.map
%__mv olpc/pt.map olpc/pt-olpc.map
%__mv qwerty/cz.map qwerty/cz-qwerty.map
cd -
%build
autoreconf -fiv
%configure \
%{nil}
%make_build -C po update-po
%make_build
%install
%make_install
%__install %{S:4} -Dm644 %{buildroot}/etc/pam.d/vlock
%__install %{S:5} %{buildroot}/%{_mandir}/man1/kbdinfo.1
%__ln_s sr-cy.map.gz %{buildroot}%{_datadir}/keymaps/i386/qwerty/sr-latin.map.gz
%__ln_s us.map.gz %{buildroot}%{_datadir}/keymaps/i386/qwerty/ko.map.gz
%__ln_s fa.map.gz %{buildroot}%{_datadir}/keymaps/i386/qwerty/ara.map.gz
# Some microoptimization
%__sed -i 's,\<kbd_mode\>,%{_bindir}/kbd_mode,g;s,\<setfont\>,%{_bindir}/setfont,g' \
%{buildroot}%{_bindir}/unicode_start
%__mkdir_p %{buildroot}%{_datadir}/keymaps/legacy
%__mv %{buildroot}%{_datadir}/keymaps/{amiga,atari,i386,include,mac,ppc,sun} %{buildroot}%{_datadir}/keymaps/legacy
# Convert X keyboard layouts to console keymaps
%__mkdir_p %{buildroot}%{_datadir}/keymaps/xkb
%__perl xml2lst.pl < %{_datadir}/X11/xkb/rules/base.xml > layouts-variants.lst
while read line; do
XKBLAYOUT=`echo "$line" | cut -d " " -f 1`
echo "$XKBLAYOUT" >> layouts-list.lst
XKBVARIANT=`echo "$line" | cut -d " " -f 2`
ckbcomp "$XKBLAYOUT" "$XKBVARIANT" | gzip > %{buildroot}%{_datadir}/keymaps/xkb/"$XKBLAYOUT"-"$XKBVARIANT".map.gz
done < layouts-variants.lst
# Convert X keyboard layouts (plain, no variant)
cat layouts-list.lst | sort -u >> layouts-list-uniq.lst
while read line; do
ckbcomp "$line" | gzip > %{buildroot}%{_datadir}/keymaps/xkb/"$line".map.gz
done < layouts-list-uniq.lst
# wipe converted layouts which cannot input ASCII (#1031848)
zgrep -L "U+0041" %{buildroot}%{_datadir}/keymaps/xkb/* | xargs %__rm -fv
%__gzip -d %{buildroot}%{_datadir}/keymaps/xkb/cz.map.gz
%__patch %{buildroot}%{_datadir}/keymaps/xkb/cz.map < %{S:6}
%__gzip %{buildroot}%{_datadir}/keymaps/xkb/cz.map
%find_lang %{name}
%check
%{!?with_check:exit 0}
%make_build checkSo my understanding is that line
%__mv qwerty/cz.map qwerty/cz-qwerty.mapcan be skipped. Am I right? If yes what more can be removed from above procedure?
kloczek
commented
2 years ago Just tested and after remove that one line from my %prep test suite is OK now. Nevertheless it would be good to have some advices about other parts of the prep, build and install 😋
thesamesam
commented
2 years ago FWIW I've just moved them before/after tests as a cheesy hack (so in %check for you). But I'd welcome advice too!
Also, thanks @kloczek for always filing these test failure bugs. Often I go to do it and you've beaten me! 💯
legionus
commented
2 years ago Since you are all renaming keymaps inplace, I think it's best for me not to use data/keymaps in tests. I will copy what I use in the tests.
Coming back to the spec file question, it's very strange that you rename keymaps to %prep and %install to %buildroot. My advice to you is to move all renames to %install. All renames will be in one place and will not change the source tree.
kloczek
commented
2 years ago Question is a bit deeper. Since those renames have some roots which currently is hard to evaluate are they still valid I would more likely today would rely on your expertise. Do those renames still make any sense? 🤔
Coming back to the spec file question, it's very strange that you rename keymaps to %prep and %install to %buildroot. My advice to you is to move all renames to %install. All renames will be in one place and will not change the source tree.
That is what we inherited from older version of packaging procedures. As for quite long time kbd had no proper maintenance some pieces of strings between causes and some (looks like) some JFDI solutions have been broken. As now kbd looks like is EnoughGood™️ maintained and those old procedures are causing some issues it is IMO the best time to rethink what was in the past used as packaging procedures in absence proper maintenance in original source tree please provide some expertise which would allow maximise simplicity of the packaging procedures used in different distributions. As I wrote I would be way more happy to have in spesc file something like only:
%prep
%%autosetup -p1
%build
autoreconf -fiv
%configure \
%{nil}
%make_build -C po update-po
%make_build
%install
%make_install
%find_lang %{name}
%check
%make_build checkWe (you and all packagers) don't need to move to that state in single step so please take your time to review:
IMO more topics will be discussed now related to packaging procedure than maintenance of the kbd will be easier and less costly (man/hour) in future). So .. please do not try to be to much flexible/polite 😋
I'm really not able to answer on some questions which are bouncing in my head so I would prefer to listen some people opinions with better expertise on kbd area 😝
kloczek
commented
2 years ago BTW what is still possible to review https://github.com/OpenMandrivaAssociation/kbd
kloczek
commented
2 years ago So here is my first patch which I would like to see review. It is %{name}-POTFILES.in.patch which contains:
--- a/po/POTFILES.in~ 2020-12-15 14:32:21.000000000 +0000
+++ b/po/POTFILES.in 2021-01-16 04:02:40.015662396 +0000
@@ -18,7 +18,7 @@
src/libcommon/version.c
src/libkbdfile/init.c
src/libkbdfile/kbdfile.c
-src/libkeymap/analyze.c
+src/libkeymap/analyze.l
src/libkeymap/array.c
src/libkeymap/common.c
src/libkeymap/diacr.c
@@ -29,7 +29,7 @@
src/libkeymap/ksyms.c
src/libkeymap/loadkeys.c
src/libkeymap/modifiers.c
-src/libkeymap/parser.c
+src/libkeymap/parser.y
src/libkeymap/summary.c
src/libkfont/context.c
src/libkfont/kdfontop.cIt solves precessing by gettext files just after only execute configure.
Gettext can process bison/flex files and without that fix when .c files are still not generated make -C po uptede-po fails.
Please let me know do you want that fix as PR.
kloczek
commented
2 years ago Another though .. as currently IIRC Linux console in any currently used language should use unicode key mapping I have suspicion that most (if not all) latin mappings files can be dropped in kbd source tree.
legionus
commented
2 years ago Question is a bit deeper. Since those renames have some roots which currently is hard to evaluate are they still valid I would more likely today would rely on your expertise. Do those renames still make any sense? thinking
I don't think it makes sense other than backwards compatibility. See below.
That is what we inherited from older version of packaging procedures. As for quite long time kbd had no proper maintenance some pieces of strings between causes and some (looks like) some JFDI solutions have been broken. As now kbd looks like is EnoughGoodtm maintained and those old procedures are causing some issues it is IMO the best time to rethink what was in the past used as packaging procedures in absence proper maintenance in original source tree please provide some expertise which would allow maximise simplicity of the packaging procedures used in different distributions. As I wrote I would be way more happy to have in spesc file something like only:
To be honest, I don’t know why the maintainers do these renames. I am not the original author of kbd. Created by Andries E. Brouwer. He also wrote the part in the linux kernel that is responsible for keymaps. After he gave me the project and when I started to understand the utilities, I realized that almost everyone does not understand how to use loadkeys correctly.
Let me explain. Many maintainers rename keymaps because there are duplicate names in keymaps:
$ find data/keymaps -name 'cz.*'
data/keymaps/i386/qwertz/cz.map
data/keymaps/i386/qwerty/cz.mapIf you execute the loadkeys cz, one of them will be loaded and not always the one that the user thinks about.
So for example you as a maintainer do something like this:
mv qwerty/cz.map qwerty/cz-qwerty.mapThis is done so that the user can do loadkeys cz-qwerty and get a proper keymap with qwerty layout.
But according to the idea of the original author, loadkeys takes not just a file name, but a subpath. So the user can always specify loadkeys qwerty/cz or loadkeys i386/qwerty/cz. It turns out essentially the same notation but with / instead of -. This behavior was a very long time ago in this utility. So names duplication is not an issue.
When I refactored the code I wrote few tests to not break this behavior.
https://github.com/legionus/kbd/blob/master/tests/libkbdfile/libkbdfile-test11.c https://github.com/legionus/kbd/blob/master/tests/libkbdfile/libkbdfile-test12.c
%prep %%autosetup -p1
There are no renames in the distribution where I maintain a kbd rpm package. I only have dances around backwards compatibility.
We (you and all packagers) don't need to move to that state in single step so please take your time to review:
* Why fedora uses three source tar balls and is it possible to merge some used bits from outside to kbd tree?
As far as I know they add some additional fonts. Periodically, redhat's maintainer updates the same(?) font in kbd. That's all I know about it.
* Does it make any sense today still rename some files or add some symlinks?
This issue deserves a separate discussion. I had an idea to rename all keymaps and bring them all to data/keymaps/<arch>/<layout>/<main-lang>[-<variant>]. Something like that. But then there are issues of backward compatibility. Such renaming will break configurations for users. Therefore, it can be made as a separate target in the makefile. Cleaning up the naming of keymaps is my old dream.
On the other hand, I've started work on adding support for xkb keymaps to loadkeys[1][2]. Not a replacement, but an addition. This could be a step towards unifying keymaps. We have hand made keymaps and we have xkb generated ones with less creative freedom. Unfortunately, there are problems here too. The xkbcommon does not provide an API to iterate over the compose table. I talked to the developers of xkbcommon about extending the API and they agreed to do it[3]. But work somehow stalled. But in any case, this functionality is in my plans for kbd-2.6.0.
[1] https://lore.altlinux.org/kbd/20220214124918.r7vjp7epspvw35jd@example.org/T/#u [2] https://github.com/legionus/kbd/tree/xkbcommon [3] https://github.com/bluetech/libxkbcommon/commits/compose-traverse
* There is vlock pam file. I'm going to review what is in fedora and kbd version and if it will be any difference which cannot be kept in kbd tree I'm going to change (for good reason) that to patch src/vlock/vlock.pamd instead blindly overwrite that file .. only to have rejection when in kbd version would be changed to keep them in kbd source tree. For me the best would be just use source tree version of that file.
pam configuration differs from distribution to distribution. It would be better to consult Dmitry Levin (@ldv-alt) here.
* I see that kbdinfo.1 is now part of the source tree so looks like I drop that part * In my procedure kbd-cz-map.patch is applied in 5prep and rolled back in %install. I have no idea why and is that really still needed and if yes doe it can be integrated into kbd repo?
Are you talking about https://src.fedoraproject.org/rpms/kbd/blob/rawhide/f/kbd.spec#_19 or are we talking about some other distribution?
If we are talking about fedora, then this patch is superimposed on the result of converting xkb keymaps. These are not keymaps from kbd:
https://src.fedoraproject.org/rpms/kbd/blob/rawhide/f/kbd.spec#_167 https://src.fedoraproject.org/rpms/kbd/blob/rawhide/f/kbd.spec#_176
* There are few other steps in my %install which I've inherited and which looks a bit not-so-well-smelly ..IMO more topics will be discussed now related to packaging procedure than maintenance of the kbd will be easier and less costly (man/hour) in future). So .. please do not try to be to much flexible/polite yum
I'm really not able to answer on some questions which are bouncing in my head so I would prefer to listen some people opinions with better expertise on kbd area stuck_out_tongue_closed_eyes
We can of course continue discussing these issues here on github, but consider subscribing to the mailing list.
legionus
commented
2 years ago It solves precessing by gettext files just after only execute configure. Gettext can process bison/flex files and without that fix when .c files are still not generated
make -C po uptede-pofails. Please let me know do you want that fix as PR.
Yes. Please.
kloczek
commented
2 years ago We can of course continue discussing these issues here on github, but consider subscribing to the mailing list.
github offers "discussion" tab. To enable it you need to go to repository settings and enable it. IMO github discussions are way better tan mailing list.
legionus
commented
2 years ago github offers "discussion" tab. To enable it you need to go to repository settings and enable it. IMO github discussions are way better tan mailing list.
OK. Let's do it here. Discussions enabled.
legionus
commented
2 years ago I'm an oldfag. I do not consider github as a convenient forum for discussions. But I understand that many will not agree with this. I've never tried github discussions and if they aren't much better than the issues, I'll turn them back off.
kloczek
commented
2 years ago It solves precessing by gettext files just after only execute configure. Gettext can process bison/flex files and without that fix when .c files are still not generated
make -C po uptede-pofails. Please let me know do you want that fix as PR.Yes. Please.
Done https://github.com/legionus/kbd/pull/77
Just added kbd discussions to my watch list 😋 I'm going to close this ticket as I'm going to open discussion (probably today evening or tomorrow) topic about improve kbd packaging procedures. After that I'm going to send email to fedora-devel to gather here as many people as possible (there are many languages keyboards mappings in kbd so you may need some advices/hints/reports from pole using all possible languages). If after that someone could spread that to Debian and other distros IMO that would be great.
IMO it wold be good to open another dummy topic like "What do you want to see in next kbd version X.X.X?" In that topic should be IMO:
Thank you 😃
legionus
commented
2 years ago If after that someone could spread that to Debian and other distros IMO that would be great.
@kloczek FYI I know that the guys from suse, redhat, altlinux, frugalware and gentoo are subscribed to the mailing list :)
Just stested new 2.5.0 and looks like test suite is failing in one unit.
Content of the tests/testsuite.log in attachment testsuite.log.txt