You are not logged in.

#1 2013-11-24 18:31:24

roepi
Member
From: Germany
Registered: 2012-01-24
Posts: 36

[SOLVED] EurKEY PKGBUILD request for comments

Hi, I just finished a PKGBUILD for EurKEY "European Keyboard Layout". It installs and removes fine on my system, and I took care to respect the packaging standards, but before submitting it to AUR I would like to get any comments/suggestions.

PKGBUILD

# Maintainer: Christoph Roeper <cr (at) roeper (dot) biz>
pkgname=eurkey
pkgver=1.1
pkgrel=1
pkgdesc="The European Keyboard Layout"
arch=('any')
url="http://eurkey.steffen.bruentjen.eu/"
license=('GPL3')
depends=('xkeyboard-config' 'sed' 'gawk')
install="$pkgname.install"
source=(http://eurkey.steffen.bruentjen.eu/download/debian/binary/eurkey.deb)
md5sums=('8da8472f8f8d30baaa6a50145264024b')

package() {
    cd "$srcdir"
    ar x $pkgname.deb
    tar xzf data.tar.gz -C "$pkgdir/"
    cd "${pkgdir}"

    # euro on '5', swap with pound
    awk='\
            BEGIN { fix = 0 } \
            { \
                if ( $2 == "<AE04>" ) { $7="sterling,"; fix = 1 } \
                else if ( $2 == "<AE05>" ) { $7="EuroSign,"; fix = 1 } \
                if ( fix == 1 ) { printf "%11s %s %s %s%16s%19s%22s%21s %s %s\n",$1,$2,$3,$4,$5,$6,$7,$8,$9,$10; fix = 0 } \
                else { print } \
            } \
    '

    file="usr/share/X11/xkb/symbols/eurkey"
    tmpfile="$file".`date +%FT%T`.temp
    awk -- "$awk" $file > $tmpfile
    cat $tmpfile > $file
    rm $tmpfile
}

pkgname.install

post_install() {

  # Installs the EurKEY layout.

  if test "$EUID" = 0; then SUDO=; else SUDO=sudo; fi

  exec $SUDO bash /dev/stdin "$@" <<'ENDSUDO'

  set -e

  str="  <layout>\n\
	<configItem>\n\
	  <name>eurkey</name>\n\
	  <shortDescription>EUR</shortDescription>\n\
	  <description>EurKEY</description>\n\
	  <languageList>\n\
		  <iso639Id>cat</iso639Id>\n\
		  <iso639Id>dan</iso639Id>\n\
		  <iso639Id>eng</iso639Id>\n\
		  <iso639Id>est</iso639Id>\n\
		  <iso639Id>fao</iso639Id>\n\
		  <iso639Id>fin</iso639Id>\n\
		  <iso639Id>ger</iso639Id>\n\
		  <iso639Id>gre</iso639Id>\n\
		  <iso639Id>gsw</iso639Id>\n\
		  <iso639Id>ita</iso639Id>\n\
		  <iso639Id>lav</iso639Id>\n\
		  <iso639Id>lit</iso639Id>\n\
		  <iso639Id>nld</iso639Id>\n\
		  <iso639Id>nor</iso639Id>\n\
		  <iso639Id>por</iso639Id>\n\
		  <iso639Id>spa</iso639Id>\n\
		  <iso639Id>swe</iso639Id>\n\
	  </languageList>\n\
	</configItem>\n\
      </layout>\n\
    </layoutList>"

  for file in /usr/share/X11/xkb/rules/{base,evdev}.xml; do
    if [ ! -f "$file" ]; then
      echo "File $file is not a regular file (skipped)"
    elif [ $(grep -ci eurkey "$file") -ne 0 ]; then
      echo "File $file already constains eurkey (skipped)"
    else
      echo "processing $file"
      sed -i "s~</layoutList>~$str~" "$file"
    fi
  done

ENDSUDO

}

pre_remove() {

  # Removes the EurKEY layout.

  if test "$EUID" = 0; then SUDO=; else SUDO=sudo; fi

  exec $SUDO bash /dev/stdin "$@" <<'ENDSUDO'

  set -e

  for file in {/etc/vconsole.conf,/etc/X11/xorg.conf.d/*keyboard*}; do
      if [ -f $file ]; then
	  [ `grep -ci '^\s*[^#].*eurkey' $file` -ne 0 ] && echo -e "Cannot completely remove EurKEY since it's still configured in $file.\nPlease remove it manually or use the settings manager." && exit 1
      fi
  done

  exit 0

ENDSUDO

}

post_remove() {

  # Removes the EurKEY layout.

  if test "$EUID" = 0; then SUDO=; else SUDO=sudo; fi

  exec $SUDO bash /dev/stdin "$@" <<'ENDSUDO'

  set -e

  awk='
	  BEGIN                     { output = 1 ; buffer = "" }
	  $0~/<layout>/             { output = 0 ; deleteSection = 0 }
	  output == 1               { print $0 }
	  $0~/<\/layout>/           { output = 1 ; buffer = buffer $0 ; if (deleteSection == 0) print buffer ; buffer = "" }
	  $0~/<name>eurkey<\/name>/ { deleteSection = 1 }
	  output == 0               { buffer = buffer $0 "\n" }
  '

  for file in /usr/share/X11/xkb/rules/{base,evdev}.xml; do
    if test -f "$file"; then
      tmpfile="$file".`date +%FT%T`.temp
      echo "processing $file"
      awk -- "$awk" $file > $tmpfile
      cat $tmpfile > $file
      rm $tmpfile
    fi
  done

ENDSUDO

}
  • 'sed' and 'awk' are neither build nor run-time dependencies, both are install dependencies, but I did not find a proper option for that.

  • The (un-)install-scripts are taken from the Debian source, however with deb-packages it is obviously possible to abort  pre/post_remove on failure (here used if the keyboard layout is still in use => no uninstall). I did not find a proper way to accomplish this with PKGBUILD install scripts.

  • Both scripts a rather long, but in other review requests forum veterans preferred to have scripts posted here instead of a link to AUR or pastebin. If it's nevertheless wrong I will relocate the scripts.

Thanks in advance.

Last edited by roepi (2014-03-17 17:42:44)

Offline

#2 2013-11-24 19:02:32

Xyne
Administrator/PM
Registered: 2008-08-03
Posts: 6,963
Website

Re: [SOLVED] EurKEY PKGBUILD request for comments

A few remarks

  • I prefer to use sha256sums instead of md5sums, but it is not important.

  • I also prefer to be consistent in my use of curly brackets around variables. Compare cd "$srcdir" to cd "${pkgdir}". Again, this is not important.

  • You are modifying the upstream package by swapping the euro with the pound. I would consider this a non-trivial modification. Perhaps it should be removed or the package should be renamed. I am not familiar with eurkey, so I cannot comment further.

  • You should use Bash's built-in [[ test operator, e.g. [[ ! -f $file ]] instead of [ ! -f $file ] or test -f "$file". Of the latter 2, the first is subject to word expansion of the variable, and the latter invokes an external binary. The built-in [[ operator does neither and is therefore preferable.

  • The install script is run as root so you do not need to use sudo.

  • I do not understand why you are invoking bash to run a HEREDOC'd subscript within the install function, which is itself already bash. Is this due to the perceived need to use sudo? Run the code directly in the function. I believe that set -e is already set when the functions are invoked, so you may remove that line.

  • My first reaction to the XML editing in the install file is that this is not the right way to do it. It feels kludgy and prone to error, but I do not have any other suggestion right now.

  • I agree that that there is no clear place to add "installdeps" in the PKGBUILD. The best place is the deps array, imo.


My Arch Linux StuffForum EtiquetteCommunity Ethos - Arch is not for everyone

Offline

#3 2013-11-24 19:33:30

Scimmia
Fellow
Registered: 2012-09-01
Posts: 11,463

Re: [SOLVED] EurKEY PKGBUILD request for comments

makepkg will already extract the .deb file for you, you don't need to do that yourself (ar x $pkgname.deb)
You can't modify files owned by another package like you are in the install script. As soon as the package that owns them is upgraded or reinstalled, the modifications are undone. Can the files be overridden in /etc/X11?

Offline

#4 2013-11-24 19:33:37

roepi
Member
From: Germany
Registered: 2012-01-24
Posts: 36

Re: [SOLVED] EurKEY PKGBUILD request for comments

Many thanks for your helpful remarks.

  • sha256sums and curly brackets : will fix this

  • swap euro with pound : US-layout with Euro symbol keyboards have the Euro symbol on 5, UK-layout keyboards on 4 (wiki). My assumption was that most keyboards are US-layout based with Euro on 5 (like mine). I also wanted to contact the author about this for an upstream change. But your are right, currently this is a major change, so I should rename the package to something like "EurKEY (US-layout based)".

  • bash [[ test operator, install as sudo, HEREDOC and XML editing : admitted no real excuse, but this was just copied&pasted from upstreams Debian pre/post install/remove scripts; see my second remark in my initial post. I'll try to fix some parts with regards to your remarks, but I also have no idea for the XML string manipulation (and I agree it feels awkward (pun intended)).

Offline

#5 2013-11-24 19:46:04

Xyne
Administrator/PM
Registered: 2008-08-03
Posts: 6,963
Website

Re: [SOLVED] EurKEY PKGBUILD request for comments

"eurkey-us" might be a good name for the package.


Scimmia wrote:

makepkg will already extract the .deb file for you, you don't need to do that yourself (ar x $pkgname.deb)
You can't modify files owned by another package like you are in the install script. As soon as the package that owns them is upgraded or reinstalled, the modifications are undone. Can the files be overridden in /etc/X11?

I assumed that those files were listed in xkeyboard-config's "backup" array, but they are not. They will indeed be overwritten when xkeyboard-config is updated. If there is no other way to override the target settings then you will have to create a modified xkeyboard-config package that includes the eurkey changes (e.g. xkeyboard-config-eur-us).


My Arch Linux StuffForum EtiquetteCommunity Ethos - Arch is not for everyone

Offline

#6 2013-11-24 19:58:18

roepi
Member
From: Germany
Registered: 2012-01-24
Posts: 36

Re: [SOLVED] EurKEY PKGBUILD request for comments

  • .deb auto-extract : Good to know. This was taken from a forum post and another Debian sourced AUR package (although this package handles different architectures, maybe using ar is needed then).

  • xkeyboard-config : Methinks the question boils down to "is there a better (= correct) way to add a new X keyboard-layout programmatically, so it is available in Desktop Environments Input Sources settings?"

Offline

#7 2013-11-24 20:47:17

Stebalien
Member
Registered: 2010-04-27
Posts: 1,237
Website

Re: [SOLVED] EurKEY PKGBUILD request for comments

If you still want to modify the XML, I would use xmlstarlet instead of awk/sed. I haven't tested the following scripts so they will probably need some debugging but they should give you a general idea of what I would do.

The post remove script would be reduced to:

set -e

# xpath selector to select layouts that are named eurkey
xpath='/xkbConfigRegistry/layoutList/layout/configItem/name[text()="eurkey"]/../..'

for file in /usr/share/X11/xkb/rules/{base,evdev}.xml; do
    [[ -f "$file" ]] && xml ed --inplace -d "$xpath" "$file"
done

And the post install script would become:

set -e

xpath='/xkbConfigRegistry/layoutList/layout/configItem/name[text()="eurkey"]'
for file in /usr/share/X11/xkb/rules/{base,evdev}.xml; do
    if [[ ! -f "$file" ]]; then
        echo "File $file is not a regular file (skipped)"
    elif xml sel -t -c "$xpath" $file >/dev/null; then
        echo "File $file already constains eurkey (skipped)"
    else
        echo "processing $file"
        xml ed -P -L                                                                        \
            -s /xkbConfigRegistry/layoutList        -t elem -n layoutTMP        -v ''       \
            -s //layoutTMP                          -t elem -n configItem       -v ''       \
            -s //layoutTMP/configItem               -t elem -n name             -v 'eurkey' \
            -s //layoutTMP/configItem               -t elem -n shortDescription -v 'EUR'    \
            -s //layoutTMP/configItem               -t elem -n description      -v 'EurKEY' \
            -s //layoutTMP/configItem               -t elem -n languageList     -v ''       \
            -s //layoutTMP/configItem/languageList  -t elem -n iso639Id         -v 'cat'    \
            -s //layoutTMP/configItem/languageList  -t elem -n iso639Id         -v 'dan'    \
            -s //layoutTMP/configItem/languageList  -t elem -n iso639Id         -v 'eng'    \
            -s //layoutTMP/configItem/languageList  -t elem -n iso639Id         -v 'est'    \
            -s //layoutTMP/configItem/languageList  -t elem -n iso639Id         -v 'fao'    \
            -s //layoutTMP/configItem/languageList  -t elem -n iso639Id         -v 'fin'    \
            -s //layoutTMP/configItem/languageList  -t elem -n iso639Id         -v 'ger'    \
            -s //layoutTMP/configItem/languageList  -t elem -n iso639Id         -v 'gre'    \
            -s //layoutTMP/configItem/languageList  -t elem -n iso639Id         -v 'gsw'    \
            -s //layoutTMP/configItem/languageList  -t elem -n iso639Id         -v 'ita'    \
            -s //layoutTMP/configItem/languageList  -t elem -n iso639Id         -v 'lav'    \
            -s //layoutTMP/configItem/languageList  -t elem -n iso639Id         -v 'lit'    \
            -s //layoutTMP/configItem/languageList  -t elem -n iso639Id         -v 'nld'    \
            -s //layoutTMP/configItem/languageList  -t elem -n iso639Id         -v 'nor'    \
            -s //layoutTMP/configItem/languageList  -t elem -n iso639Id         -v 'por'    \
            -s //layoutTMP/configItem/languageList  -t elem -n iso639Id         -v 'spa'    \
            -s //layoutTMP/configItem/languageList  -t elem -n iso639Id         -v 'swe'    \
            -r //layoutTMP                                                      -v layout   \
            "$file"
    fi
done

Steven [ web : git ]
GPG:  327B 20CE 21EA 68CF A7748675 7C92 3221 5899 410C
Do not email: honeypot@stebalien.com

Offline

#8 2013-11-24 21:18:01

roepi
Member
From: Germany
Registered: 2012-01-24
Posts: 36

Re: [SOLVED] EurKEY PKGBUILD request for comments

Thanks, using xmlstarlet really reduces the script dramatically, I should forward this to upstream.
However I also take into consideration to build a replacement "xkeyboard-config" package, as Xyne suggested, after finding a few packages like this in AUR. More research on my site is obviously necessary.

Offline

#9 2014-03-17 08:03:06

stas
Member
Registered: 2013-03-10
Posts: 5

Re: [SOLVED] EurKEY PKGBUILD request for comments

Hi,
can you post your current script here, please?
Id like to try this keyboard layout and dont want to reinvent the wheel.

Thanks  in advance.

Offline

#10 2014-03-17 17:41:49

roepi
Member
From: Germany
Registered: 2012-01-24
Posts: 36

Re: [SOLVED] EurKEY PKGBUILD request for comments

Check the following github-gist xkeyboard-config.PKGBUILD for the PKGBUILD and the eurkey.patch, you also need the Arch patch revert-comma-in-keypad.patch in Extra.
I use this PKGBUILD with ABS and in the end skipped creating a public AUR package, although it is pretty straight forward to create an AUR package from here. The reason is that I managed to get this layout into upstream, so the EurKEY-layout will be available with the next release of xkeyboard-config.
I also mark this thread as solved. Thanks everyone for the valuable input.

Offline

#11 2014-06-04 17:54:18

roepi
Member
From: Germany
Registered: 2012-01-24
Posts: 36

Re: [SOLVED] EurKEY PKGBUILD request for comments

For everyone finding this old thread: the EurKEY-layout is in xkeyboard-config 2.12 (ArchLinux package update 2014-05-28), so no more need for extra AUR or ABS customized package builds.

Offline

Board footer

Powered by FluxBB