You are not logged in.

#1 2014-06-25 17:48:03

brix
Member
Registered: 2014-05-26
Posts: 69

Request criticism of new 'xword' package

I packaged this elegant little Across Lite crossword-solving app for my own use with some text-display code patched for use on a 3200x1800 screen, and thought a package of the vanilla version (without HiDPI edits) submitted to the AUR might be more widely useful.

I'm new to Arch and packaging, not to mention patching sources, so I'm looking for criticism of my proposed aurball contents:

PKGBUILD

# Maintainer: [i]myname <email .. domain . tld>[/i]

pkgname=xword
pkgver=2.0.0_rc2
pkgrel=1
pkgdesc="Do crossword puzzles in the Across Lite format"
arch=('any')
url="https://alioth.debian.org/projects/xword/"
license=('BSD')
depends=('pygtk' 'libwnck')
source=(http://ftp.de.debian.org/debian/pool/main/x/xword/xword_2.0.0~rc2.orig.tar.gz xword.patch xword.desktop)
sha1sums=('0409f93f9cf65a5e450b89c81f358545ea268dcf'
 'f7cec09fe259bad5fab76ef939ce8623fefe1762' '9cd828c07f9522493ee1f8b7d9bc7763aaa0663d')

package() {
  mv xword-2.0.0~rc2 xword-2.0.0_rc2
  cd "${srcdir}/xword-${pkgver}"
  patch -p1 < ../xword.patch
  mkdir -p $pkgdir/usr/share/licenses/$pkgname
  install -D -m644 LICENSE "$pkgdir/usr/share/licenses/$pkgname/"
  mkdir -p $pkgdir/usr/share/doc/$pkgname
  install -D -m644 README PKG-INFO "$pkgdir/usr/share/doc/$pkgname/"
  mkdir -p $pkgdir/usr/share/applications
  install -D -m644 ../xword.desktop "$pkgdir/usr/share/applications/"
  python2 setup.py install --root="${pkgdir}"
}

xword.patch

diff -Naur xword-2.0.0~rc2/scripts/xword xword-2.0.0_rc2/scripts/xword
--- xword-2.0.0~rc2/scripts/xword	2011-07-24 15:05:38.000000000 -0700
+++ xword-2.0.0_rc2/scripts/xword	2014-06-24 15:14:13.232116926 -0700
@@ -1,4 +1,4 @@
-#!/usr/bin/env python
+#!/usr/bin/env python2
 
 from xword.main import MainWindow
 from xword.organizer import OrganizerWindow
diff -Naur xword-2.0.0~rc2/xword/main.py xword-2.0.0_rc2/xword/main.py
--- xword-2.0.0~rc2/xword/main.py	2011-07-24 21:29:01.000000000 -0700
+++ xword-2.0.0_rc2/xword/main.py	2014-06-24 15:20:45.081624109 -0700
@@ -479,10 +479,12 @@
              'Bill McCloskey <bill.mccloskey@gmail.com>\n' +
              'Maemo Port: Bradley Bell <bradleyb@u.washington.edu>\n' +
              'and Terrence Fleury <terrencegf@gmail.com>'])
-        dialog.set_website('http://x-word.org')
-        dialog.set_website_label('x-word.org')
-
-        dialog.connect('response', lambda *args: dialog.destroy())
+# remove website dialog (site appears to have been abandoned)
+#        dialog.set_website('http://x-word.org')
+#        dialog.set_website_label('x-word.org')
+# close 'About' dialog without hanging program
+#        dialog.connect('response', lambda *args: dialog.destroy())
+        dialog.connect("response", lambda dlg, resp: dlg.destroy())
         dialog.show()
 
     def create_widgets(self):

xword.desktop

[Desktop Entry]
Encoding=UTF-8
Name=Xword
Comment=Do crossword puzzles in the Across Lite file format
Exec=xword
Terminal=false
Type=Application
StartupNotify=true
Icon=text-editor.png
Categories=GTK;Application;Game

It builds, installs and runs without problem (or at least the edited-for-HiDPI version does) and namcap seems to have no serious objections, but it's my first package so I'm pretty confident I've overlooked something.


Enough is more.

Offline

#2 2014-06-25 18:18:24

ids1024
Member
From: California
Registered: 2013-08-16
Posts: 243
Website

Re: Request criticism of new 'xword' package

Patching should be done in the prepare function.

https://wiki.archlinux.org/index.php/Cr … 9_function


"Only wimps use tape backup: _real_ men just upload their important stuff on ftp, and let the rest of the world mirror it."—Linus Torvalds
s/ftp/git/

https://iandouglasscott.com | https://github.org/ids1024 | https://keybase.io/ids1024

Offline

#3 2014-06-25 19:11:51

Slithery
Administrator
From: Norfolk, UK
Registered: 2013-12-01
Posts: 5,776

Re: Request criticism of new 'xword' package

All of the mkdir commands in the PKGBUILD are unnecessary, using install with the D switch automatically creates the directories for you.


No, it didn't "fix" anything. It just shifted the brokeness one space to the right. - jasonwryan
Closing -- for deletion; Banning -- for muppetry. - jasonwryan

aur - dotfiles

Offline

#4 2014-06-25 19:25:02

brix
Member
Registered: 2014-05-26
Posts: 69

Re: Request criticism of new 'xword' package

ids1024 wrote:

Patching should be done in the prepare function.

https://wiki.archlinux.org/index.php/Cr … 9_function

Aha, thank you.

This

prepare() {
  mv xword-2.0.0~rc2 xword-2.0.0_rc2
  patch -p0 < xword.patch
}

package() {
  cd "${srcdir}/xword-${pkgver}"
  mkdir -p $pkgdir/usr/share/licenses/$pkgname
  install -D -m644 LICENSE "$pkgdir/usr/share/licenses/$pkgname/"
[...]

besides conforming, simply makes more sense.


Enough is more.

Offline

#5 2014-06-25 20:17:44

brix
Member
Registered: 2014-05-26
Posts: 69

Re: Request criticism of new 'xword' package

slithery wrote:

All of the mkdir commands in the PKGBUILD are unnecessary, using install with the D switch automatically creates the directories for you

...but only (you've helped me discover) when you include a filename at the end of the destination path:

install -D -m644 LICENSE "$pkgdir/usr/share/licenses/$pkgname/LICENSE"

I'd expected

 cp ./LICENSE $pkgdir/usr/share/licenses/$pkgname/"

but with 'mkdir -p' tacked on the front. When it wasn't there, of course it had to be added. smile


Enough is more.

Offline

#6 2014-06-25 20:22:45

stevenhoneyman
Member
From: England
Registered: 2014-05-25
Posts: 241

Re: Request criticism of new 'xword' package

Your pkgver shouldn't have the underscore:

Version tags may not include hyphens! Letters, numbers, and periods only.

https://wiki.archlinux.org/index.php/Ar … age_naming

Also it should include python2 in "depends"
(and python2-setuptools in makedepends I assume, but haven't checked)

also, personal preference I'm sure.. but:

patch -p0 < xword.patch

I would have had as:

patch -i xword.patch

or

cd "${srcdir}/xword-${pkgver}"
patch -p1 -i ../xword.patch

or actually looking at the patch, I'd have just used sed, which would mean you don't need to bundle the patch file.
for example:

sed -i '1s/env python/&2/' scripts/xword
sed -i '/set_website/d' xword/main.py
sed -i 's/lambda .args/lambda dlg,resp/' xword/main.py

Last edited by stevenhoneyman (2014-06-25 20:52:10)

Offline

#7 2014-06-26 00:13:36

brix
Member
Registered: 2014-05-26
Posts: 69

Re: Request criticism of new 'xword' package

stevenhoneyman wrote:

Your pkgver shouldn't have the underscore:

Version tags may not include hyphens! Letters, numbers, and periods only.

https://wiki.archlinux.org/index.php/Ar … age_naming

Thanks for the detailed reply.

Also it should include python2 in "depends"
(and python2-setuptools in makedepends I assume, but haven't checked)

I wondered about that, but

namcap xword-2.0.0rc2-1-any.pkg.tar.xz
xword W: Dependency python2 included but already satisfied

which I take to mean pygtk would (ultimately) require python2 in any case. And Arch's python2 seems to include distutils/setup stuff.


I'd have just used sed, which would mean you don't need to bundle the patch file.
for example:

sed -i '1s/env python/&2/' scripts/xword
sed -i '/set_website/d' xword/main.py
sed -i 's/lambda .args/lambda dlg,resp/' xword/main.py

Less informative but more elegant. I'll do it, and keep my commented patch for personal reference. I've now got

PGKBUILD

# Maintainer: myname <email .. domain . tld>

pkgname=xword
pkgver=2.0.0rc2
pkgrel=1
pkgdesc="Do crossword puzzles in Across Lite format"
arch=('any')
url="https://alioth.debian.org/projects/xword/"
license=('BSD')
depends=('python2' 'pygtk' 'libwnck')
source=(http://ftp.de.debian.org/debian/pool/main/x/xword/xword_2.0.0~rc2.orig.tar.gz xword.desktop)
sha1sums=('0409f93f9cf65a5e450b89c81f358545ea268dcf' '06d4b27c61091f0d152e575078ee9d1ed89e1648')

prepare() {
  cd xword-2.0.0~rc2
  sed -i '1s/env python/&2/' scripts/xword
  sed -i '/set_website/d' xword/main.py
  sed -i 's/lambda .args: dialog/lambda dlg,resp: dlg/' xword/main.py
  sed -i '/set_website/d' xword/organizer.py
  sed -i 's/lambda .args: dialog/lambda dlg,resp: dlg/' xword/organizer.py
}

package() {
  cd xword-2.0.0~rc2
  install -D -m644 LICENSE "$pkgdir/usr/share/licenses/$pkgname/LICENSE"
  install -D -m644 README "$pkgdir/usr/share/doc/$pkgname/README"
  install -D -m644 PKG-INFO "$pkgdir/usr/share/doc/$pkgname/PKG-INFO"
  install -D -m644 ../xword.desktop "$pkgdir/usr/share/applications/xword.desktop"
  python2 setup.py install --root="${pkgdir}"
}

You probably shouldn't have offered those lines of sed: "Give a man a fish and he may acquire a taste for fish."

altered as per ids1024 & stevenhoneyman

Last edited by brix (2014-06-26 01:23:34)


Enough is more.

Offline

#8 2014-06-26 00:23:52

ids1024
Member
From: California
Registered: 2013-08-16
Posts: 243
Website

Re: Request criticism of new 'xword' package

brix wrote:

  mv xword-2.0.0~rc2 ${pkgname}-${pkgver}

Couldn't you just cd into xword-2.0.0~rc2?

brix wrote:

package() {
  cd "${srcdir}/${pkgname}-${pkgver}"

Isn't ${srcdir} unnecessary here?


"Only wimps use tape backup: _real_ men just upload their important stuff on ftp, and let the rest of the world mirror it."—Linus Torvalds
s/ftp/git/

https://iandouglasscott.com | https://github.org/ids1024 | https://keybase.io/ids1024

Offline

#9 2014-06-26 00:29:45

stevenhoneyman
Member
From: England
Registered: 2014-05-25
Posts: 241

Re: Request criticism of new 'xword' package

brix wrote:

I wondered about that, but

namcap xword-2.0.0rc2-1-any.pkg.tar.xz
xword W: Dependency python2 included but already satisfied

which I take to mean pygtk would (ultimately) require python2 in any case

Technically, yes you're right. Maybe it's just a personal preference... but it drives me mad having to hunt around for deps! In this package, it's "fairly" obvious that PyGTK will need python - but to explain why it bugs me, I'll use the example of libreoffice: the Arch package for "libreoffice-impress" depends on libreoffice-common, which depends on libcups, which depends on avahi. Yes, the dependency is technically met... but if you have an alternative version of part of the chain (I had a custom libcups), then the link fails and the effects might not be immediately visible.

The only things I can spot now, are $srcdir isn't really necessary (its not harming anything though).
Also, there'll be people saying you should quote variables always (you've missed a couple in prepare() function)

Last edited by stevenhoneyman (2014-06-26 00:30:39)

Offline

#10 2014-06-26 00:56:06

brix
Member
Registered: 2014-05-26
Posts: 69

Re: Request criticism of new 'xword' package

Thanks, people. You could get the impression I have very little idea what I'm doing, apart from cribbing from /var/abs/community.

Must say I'm impressed by and grateful for the patient responses here.


Enough is more.

Offline

#11 2014-06-26 01:03:07

ids1024
Member
From: California
Registered: 2013-08-16
Posts: 243
Website

Re: Request criticism of new 'xword' package

brix wrote:

Must say I'm impressed by and grateful for the patient responses here.

It's in everyone's best interest for more users to know how to create PKGBUILDs.  The more slaves volunteers to maintain the AUR the better!


"Only wimps use tape backup: _real_ men just upload their important stuff on ftp, and let the rest of the world mirror it."—Linus Torvalds
s/ftp/git/

https://iandouglasscott.com | https://github.org/ids1024 | https://keybase.io/ids1024

Offline

#12 2014-06-26 01:20:48

brix
Member
Registered: 2014-05-26
Posts: 69

Re: Request criticism of new 'xword' package

ids1024 wrote:

Couldn't you just cd into xword-2.0.0~rc2?

Yikes, didn't notice that remark -- I maybe couldn't, but makepgk can. (Seems I only understand my sig right to left.)


Enough is more.

Offline

#13 2014-06-26 02:42:10

progandy
Member
Registered: 2012-05-17
Posts: 5,321

Re: Request criticism of new 'xword' package

You can improve the integration with makepkg if you split the setup process in build and install like this:
Edit: This way you can build() as the current user and package() in a fakeroot.

build() {
  cd xword-2.0.0~rc2
  python2 setup.py build
}
package() {
  cd xword-2.0.0~rc2
...
  python2 setup.py install --root="${pkgdir}"
...
}

Last edited by progandy (2014-06-26 02:49:33)


| alias CUTF='LANG=en_XX.UTF-8@POSIX ' | alias ENGLISH='LANG=C.UTF-8 ' |

Offline

#14 2014-06-26 13:45:54

brix
Member
Registered: 2014-05-26
Posts: 69

Re: Request criticism of new 'xword' package

progandy wrote:

you can build() as the current user and package() in a fakeroot

Hmm, now that you mention it, there's a heap of "setup.py build" in /var/abs -- thanks. I'll go away now and swot up the makepkg process more attentively before finalizing and submitting.


Enough is more.

Offline

Board footer

Powered by FluxBB