You are not logged in.

Pages: 1

#1 2022-06-29 20:29:56

RandomArcher
Member
Registered: 2021-08-28
Posts: 5

PKGBUILD Review Request: mueller-dict

Hi!

I made a PKGBUILD for this dictionary: http://mueller-dict.sourceforge.net
This is the first PKGBUILD i've made from scratch and I'd appreciate if someone
could review it for mistakes/improvement opportunities.

PKGBUILD:

# Maintainer: Isho Antar <0xisho@proton.me>

pkgname=mueller-dict
pkgver=3.1.1
pkgrel=1
pkgdesc="English -> Russian dictionary for dictd et al."
arch=('any')
url="http://mueller-dict.sourceforge.net"
license=('GPL2')
optdepends=('dictd: dict client and server')
install=${pkgname}.install
source=("http://downloads.sourceforge.net/$pkgname/$pkgname-$pkgver.tar.gz")
sha1sums=('6aebd3fdbe8f921e0011630d35137caf523fd40a')

prepare()
{
    cd "$pkgname-$pkgver"
    ./configure --prefix=/usr
}


build()
{
    make -C "$pkgname-$pkgver" -j$(( $(nproc) + 2 ))
}


package()
{
    DESTDIR="$pkgdir" make -C "$pkgname-$pkgver" install
    mv "$pkgdir/usr/share/dict"{,d}
    
    install -m 0755 -d "$pkgdir/usr/share/doc/$pkgname"
    install -m 0644 -t "$pkgdir/usr/share/doc/$pkgname/" \
        "$pkgname-$pkgver/"{AUTHORS,ChangeLog,COPYING,INSTALL,NEWS,README,README.koi8r}

    install -m 0755 -d "$pkgdir/usr/share/licenses/$pkgname"
    ln -s "/usr/share/doc/$pkgname/COPYING" "${pkgdir}/usr/share/licenses/$pkgname"
}

mueller-dict.install:

basename=mueller
pkgname="${basename}-dict"
dictd_conf=/etc/dict/dictd.conf
datadir=/usr/share/dictd
conf="database ${basename}-abbrev {
        data ${datadir}/${basename}-abbrev.dict.dz
        index ${datadir}/${basename}-abbrev.index
}
database ${basename}-base {
        data ${datadir}/${basename}-base.dict.dz
        index ${datadir}/${basename}-base.index
}
database ${basename}-dict {
        data ${datadir}/${basename}-dict.dict.dz
        index ${datadir}/${basename}-dict.index
}
database ${basename}-geo {
        data ${datadir}/${basename}-geo.dict.dz
        index ${datadir}/${basename}-geo.index
}
database ${basename}-names {
        data  ${datadir}/${basename}-names.dict.dz
        index ${datadir}/${basename}-names.index
}"

post_install()
{
        echo
        if pacman -Qq dictd > /dev/null 2>&1
        then
                if grep -q "^database *${basename}" "${dictd_conf}"
                then
                        echo "${pkgname} already configured in ${dictd_conf}"
                else
                        echo "Adding configuration for ${pkgname} to ${dictd_conf}"
                        echo "${conf}" >> "${dictd_conf}"
                fi

                if systemctl -q is-active dictd.service
                then
                        echo "Restarting dictd service in order to" \
                                 "use the new dictionary database"
                        systemctl restart dictd.service
                else
                        echo "Starting dictd service in order to" \
                                 "use the new dictionary database"
                        systemctl start dictd.service
                fi
        else
                echo "dictd does not appear to be installed."
                echo "In order to use this database you should either" \
                     "install dictd or alternatively" \
                     "another dict server and configure it on your own."
        fi
        echo
}

post_upgrade()
{
        if pacman -Qq dictd > /dev/null 2>&1    && \
           systemctl -q is-active dictd.service
        then
                echo -e "\nRestarting dictd service in order to" \
                        "use the updated dictionary database"
                systemctl restart dictd.service
        fi
}

post_remove()
{
        if pacman -Qq dictd > /dev/null 2>&1
        then
                current_conf="$(grep -A 3 "^database *${basename}" "${dictd_conf}")"
                if test -n "${current_conf}"
                then
                        echo
                        if test "${current_conf}" = "${conf}"
                        then
                                echo "Removing configuration for ${pkgname} from ${dictd_conf}"
                                sed -i "/database ${basename}-.* {/,/}/d" "${dictd_conf}"
                        else
                                echo "User created / modified configuration" \
                                         "for ${pkgname} in ${dictd_conf} is left untouched."
                        fi
                fi

                if systemctl -q is-active dictd.service
                then
                        echo "Restarting dictd service in order to" \
                             "stop using the removed dictionary database"
                        systemctl restart dictd.service
                fi
        fi
}

One thing that might stand out is the 'mv' in the package() function. I've checked
other similar packages and all of them seem to install dictd dictionaries under
/usr/share/dictd whereas this package installs it's stuff under /usr/share/dict/.
'sed'ing the makefiles might be a better idea? Not sure, but yeah, that's why it's there.

Offline

#2 2022-06-29 23:24:50

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

Re: PKGBUILD Review Request: mueller-dict

Please don't use -j$(( $(nproc) + 2 )) when calling make. The user sets their own MAKEFLAGS in makepkg.conf. Don't override them.

Since you're calling `install` with -t when installing the docs, you can add -D to automatically create the dir. No need to do it in the previous command.

GPL2 is a common license, so there's no need to install another copy and make that symlink. I didn't check it, is it actually GPL2 only and not GPL2 or any later version?

Automatically editing configs installed by a different package is a very bad idea. Personally, I'm against automatically restarting the services automatically as well. That should all be up to the user.

Offline

#3 2022-06-30 05:26:30

RandomArcher
Member
Registered: 2021-08-28
Posts: 5

Re: PKGBUILD Review Request: mueller-dict

Scimmia wrote:

Please don't use -j$(( $(nproc) + 2 )) when calling make. The user sets their own MAKEFLAGS in makepkg.conf. Don't override them.

Since you're calling `install` with -t when installing the docs, you can add -D to automatically create the dir. No need to do it in the previous command.

GPL2 is a common license, so there's no need to install another copy and make that symlink.

Done.

Scimmia wrote:

I didn't check it, is it actually GPL2 only and not GPL2 or any later version?

Here it says GPLv2. I couldn't find the "or any later version" (or similar notice) anywhere, so i think it's GPL2 only.

Scimmia wrote:

Automatically editing configs installed by a different package is a very bad idea. Personally, I'm against automatically restarting the services automatically as well. That should all be up to the user.

I did that because that's how the other dict packages seem to be doing it. Should I just echo instructions
on what to do and let the user handle the rest?

Offline

#4 2022-06-30 07:55:15

schard
Forum Moderator
From: Hannover
Registered: 2016-05-06
Posts: 1,962
Website

Re: PKGBUILD Review Request: mueller-dict

Another question would be, whether it is wise to package 9 years old abandonware when other, more up-to-date dictionaries are out there.

Also, your complex install file should be replaced with appropriate ALPM hooks.

Last edited by schard (2022-06-30 07:58:11)

Offline

#5 2022-06-30 15:20:33

RandomArcher
Member
Registered: 2021-08-28
Posts: 5

Re: PKGBUILD Review Request: mueller-dict

schard wrote:

Another question would be, whether it is wise to package 9 years old abandonware when other, more up-to-date dictionaries are out there.

I couldn't find any English -> Russian (or the other way around) dictd dictionaries in the repositories and
a DDG query for "English Russian dictionary in DICT format" (without the quotes) or similar queries,
this package's homepage is one of the first to pop up. I was surprised it wasn't already in the AUR, tbh.

schard wrote:

Also, your complex install file should be replaced with appropriate ALPM hooks.

I'll check the wiki page on the hooks and get rid of the install file.

Edit: nvm, the ALPM hook's 'Exec' action seems to only allow one-liners which will result in an overly-long line when printing out the configuration.

Last edited by RandomArcher (2022-06-30 16:47:01)

Offline

#6 2022-06-30 16:46:04

RandomArcher
Member
Registered: 2021-08-28
Posts: 5

Re: PKGBUILD Review Request: mueller-dict

AIright, so incorporating the feedback I got so far, this is what i ended up with:

PKGBUILD:

# Maintainer: Isho Antar <0xisho@proton.me>

pkgname=mueller-dict
pkgver=3.1.1
pkgrel=1
pkgdesc="English -> Russian dictionary for dictd et al."
arch=('any')
url="http://mueller-dict.sourceforge.net"
license=('GPL2')
optdepends=('dictd: dict client and server')
install="$pkgname.install"
source=("http://downloads.sourceforge.net/$pkgname/$pkgname-$pkgver.tar.gz")
sha1sums=('6aebd3fdbe8f921e0011630d35137caf523fd40a')

prepare()
{
    cd "$pkgname-$pkgver"
    ./configure --prefix=/usr
}


build()
{
    make -C "$pkgname-$pkgver"
}


package()
{
    DESTDIR="$pkgdir" make -C "$pkgname-$pkgver" install
    mv "$pkgdir/usr/share/dict"{,d}
    
    install -m 0644 -Dt "$pkgdir/usr/share/doc/$pkgname/" \
        "$pkgname-$pkgver/"{AUTHORS,ChangeLog,INSTALL,NEWS,README,README.koi8r}
}

mueller-dict.install:

basename=mueller
dictd_conf=/etc/dict/dictd.conf
datadir=/usr/share/dictd
conf="database ${basename}-abbrev {
      data ${datadir}/${basename}-abbrev.dict.dz
      index ${datadir}/${basename}-abbrev.index
}
database ${basename}-base {
      data ${datadir}/${basename}-base.dict.dz
      index ${datadir}/${basename}-base.index
}
database ${basename}-dict {
      data ${datadir}/${basename}-dict.dict.dz
      index ${datadir}/${basename}-dict.index
}
database ${basename}-geo {
      data ${datadir}/${basename}-geo.dict.dz
      index ${datadir}/${basename}-geo.index
}
database ${basename}-names {
      data  ${datadir}/${basename}-names.dict.dz
      index ${datadir}/${basename}-names.index
}"

post_install()
{
      echo
      echo "Add the following database entries to $dictd_conf and" \
              "restart dictd.service for the changes to take effect:"
      echo
      echo "$conf"
      echo
}

post_remove()
{
      echo
      echo "Don't forget to remove the database entries from $dictd_conf" \
              "and restart dictd.service for the changes to take effect."
      echo
}

The PKGBUILD no longer installs unneeded stuff, simplified the install commands and the .install
script no longer does any funny business.

Further feedback is appreciated!

Offline

#7 2022-07-01 10:13:12

Lone_Wolf
Member
From: Netherlands, Europe
Registered: 2005-10-04
Posts: 11,910

Re: PKGBUILD Review Request: mueller-dict

RandomArcher wrote:

the ALPM hook's 'Exec' action seems to only allow one-liners which will result in an overly-long line when printing out the configuration.

Correct, and the solution is to include a script with the hook and call that from the exec line .

an example is the mkinitcpio package

run 

$ pacman -Ql mkinitcpio | grep libalpm

and look at the files it lists.

Disliking systemd intensely, but not satisfied with alternatives so focusing on taming systemd.


(A works at time B)  && (time C > time B ) ≠  (A works at time C)

Offline

#8 2022-07-01 18:19:46

RandomArcher
Member
Registered: 2021-08-28
Posts: 5

Re: PKGBUILD Review Request: mueller-dict

Update including ALPM hooks:

PKGBUILD:

# Maintainer: Isho Antar <0xisho@proton.me>

pkgname=mueller-dict
pkgver=3.1.1
pkgrel=1
pkgdesc="English -> Russian dictionary for dictd et al."
arch=('any')
url="http://mueller-dict.sourceforge.net"
license=('GPL2')
optdepends=('dictd: dict client and server')
# install="$pkgname.install"
source=(
    "http://downloads.sourceforge.net/$pkgname/$pkgname-$pkgver.tar.gz"
    "mueller-dict-"{install,remove}{,.hook}
)
sha1sums=(
    '6aebd3fdbe8f921e0011630d35137caf523fd40a'
    'e4464f3d3d39f50dda1320b61dabf3b175383dda' # install
    '11df4a93d604553edb1136317391af519ef8837c' # install hook
    '742adff8597df5372ca130ad6665841f94fb97a7' # remove
    '10a018ad44990a5ca7fd4fb56ec639866d248d23' # remove hook
)

prepare()
{
    cd "$pkgname-$pkgver"
    ./configure --prefix=/usr
}


build()
{
    make -C "$pkgname-$pkgver"
}


package()
{
    DESTDIR="$pkgdir" make -C "$pkgname-$pkgver" install
    mv "$pkgdir/usr/share/dict"{,d}

    install -m 0644 -Dt "$pkgdir/usr/share/doc/$pkgname/" \
        "$pkgname-$pkgver/"{AUTHORS,ChangeLog,INSTALL,NEWS,README,README.koi8r}

    install -m 0644 -Dt "$pkgdir/usr/share/libalpm/hooks" \
        "mueller-dict-"{install,remove}.hook
    install -m 0755 -Dt "$pkgdir/usr/share/libalpm/scripts" \
        "mueller-dict-"{install,remove}
}

mueller-dict-install.hook:

[Trigger]
Type = Package
Operation = Install
Target = mueller-dict

[Action]
Description = Setup Instructions
When = PostTransaction
Exec = /usr/share/libalpm/scripts/mueller-dict-install

mueller-dict-install:

#!/bin/bash -e

dictd_conf=/etc/dict/dictd.conf
datadir=/usr/share/dictd
dicts=(
	mueller-abbrev
	mueller-base
	mueller-dict
	mueller-geo
	mueller-names
)

echo
echo "Add the following database entries to $dictd_conf and"
echo "restart dictd.service for the changes to take effect:"
echo
for dict in "${dicts[@]}"; do
	echo "database ${dict} {"
	echo "  data  ${datadir}/${dict}.dict.dz"
	echo "  index ${datadir}/${dict}.index"
	echo "}"
done
echo

mueller-dict-remove.hook:

[Trigger]
Type = Package
Operation = Remove
Target = mueller-dict

[Action]
Description = Configuration Update Reminder
When = PreTransaction
Exec = /usr/share/libalpm/scripts/mueller-dict-remove

mueller-dict-remove:

#!/bin/bash -e

echo
echo "Don't forget to remove the database entries from /etc/dict/dictd.conf"
echo "and restart dictd.service for the changes to take effect."
echo

Offline

Pages: 1

Board footer

Powered by FluxBB