You are not logged in.

#1 2023-05-24 19:59:47

benjaminhottell
Member
Registered: 2023-05-24
Posts: 4

PKGBUILD Review: csvrewrite

Hello, I am a new user.

I have recently created a Python script available at GitHub here: https://github.com/benjaminhottell/pycsvrewrite

It consists of a single executable script named 'csvrewrite'. It can be installed via setup.py (but depends on python-setuptools)

I looked through the AUR and found similar scripts and tried to assemble a PKGBUILD file.

Please let me know if you found any mistakes or have any improvements.

# Maintainer: Benjamin Hottell <benjaminhottell@gmail.com>

pkgname=pycsvrewrite-git
pkgver=v1.0.0.r0.0b0f997
pkgrel=1
arch=('any')
depends=('python')
makedepends=('python-setuptools')
license=('AGPL')
pkgdesc="Python script to convert between CSV, TSV, and similar formats"
url='https://github.com/benjaminhottell/pycsvrewrite'
source=('git://github.com/benjaminhottell/pycsvrewrite')
md5sums=('SKIP')

pkgver() {
	cd "$srcdir/${pkgname%-git}" || return
	printf "%s" "$(git describe --long --tags | sed 's/\([^-]*-\)g/r\1/;s/-/./g')"
}

build() {
	cd "$srcdir/${pkgname%-git}" || return
	python setup.py build
}

package() {
	cd "$srcdir/${pkgname%-git}" || return
	python setup.py install \
		--skip-build \
		--optimize=1 \
		--prefix=/usr \
		--root="${pkgdir}"
}

Offline

#2 2023-05-24 20:20:15

Trilby
Inspector Parrot
Registered: 2011-11-29
Posts: 29,523
Website

Re: PKGBUILD Review: csvrewrite

The PKGBUILD looks good, but what does this do that wouldn't be done easier with `tr`? Does this skip quoted or otherwise enclosed delimeters?  I don't see anything in the code to do so.

EDIT: thanks for the clarification.  Handling the quoting or other escaping of delimeters is a good justification for a tool.

Last edited by Trilby (2023-05-25 01:13:37)


"UNIX is simple and coherent..." - Dennis Ritchie, "GNU's Not UNIX" -  Richard Stallman

Offline

#3 2023-05-24 20:29:36

benjaminhottell
Member
Registered: 2023-05-24
Posts: 4

Re: PKGBUILD Review: csvrewrite

Hi Trilby,

Thank you for looking over the PKGBUILD.

Consider the following CSV file which I created on the fly using LibreOffice Calc:

"weird,data,",123
"red",124
"orange",125

Note in the first row that the first column has

weird,data,

as its value, which LibreOffice has quoted so as to escape the commas. LibreOffice has not quoted the second column. So, we have a mixture of columns that have quotes and no quotes. (And, we have values with commas in them!)

The script itself leverages Python's csv module which supports these quirks in LibreOffice's output format.

Upon rewriting with a new delimiter:

$ csvrewrite --od ';' -i dummy.csv
weird,data,;123
red;124
orange;125

So, I intend for it to handle situations that are not obviously handled by (e.g.) `tr`.

Basically, I'm just trying to get Python's flexibility in handling files like these onto the command line.

I realize now I should put this explanation on the GitHub page too. Thank you for pointing this out.

Offline

#4 2023-05-24 20:55:12

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

Re: PKGBUILD Review: csvrewrite

A few things. Overall it looks good, though.

You should strip the leading 'v' in the pkgver. This is covered on the vcs package guidelines wiki page

GitHub shut off git:// access a while back. You need to clone over https. Should also be covered on that wiki page.

The printf in the pkgver function is useless.

What are the '|| return's for? As far as I can tell, they won't do anything good.

Offline

#5 2023-05-24 21:26:40

benjaminhottell
Member
Registered: 2023-05-24
Posts: 4

Re: PKGBUILD Review: csvrewrite

Hi Scimmia,

I have fixed the pkgver function. It no longer has v in front.

I tested the script on my machine with the git:// before posting it on the forum, and it worked. Perhaps it's intelligently correcting itself to https? Regardless, I changed the source variable to more closely match what I see on the wiki page.

I inherited the useless printf from the 'proto/PKGBUILD-vcs.proto' file from the pacman repository. I had assumed it was meaningful. It was removed per your suggestion.

While I was reading up on other PKGBUILD files, I occasionally saw ||return being used in that way. Shellcheck also urges you to include a ||exit or ||return just in case cd fails. I therefore assumed that it was a common thing to return in case of failure. I have removed them, though, as it looks like makepkg defends against this failure so the ||return failsafe is redundant at best.

Here is the updated version:

# Maintainer: Benjamin Hottell <benjaminhottell@gmail.com>

pkgname=pycsvrewrite-git
pkgver=1.0.0.r0.g0b0f997
pkgrel=1
arch=('any')
depends=('python')
makedepends=('python-setuptools')
license=('AGPL')
pkgdesc="Python script to convert between CSV, TSV, and similar formats"
url='https://github.com/benjaminhottell/pycsvrewrite'
source=('pycsvrewrite::git+https://github.com/benjaminhottell/pycsvrewrite#branch=main')
md5sums=('SKIP')

pkgver() {
	cd "$srcdir/${pkgname%-git}"
	git describe --long --tags | sed 's/^v//;s/\([^-]*-g\)/r\1/;s/-/./g'
}

build() {
	cd "$srcdir/${pkgname%-git}"
	python setup.py build
}

package() {
	cd "$srcdir/${pkgname%-git}"
	python setup.py install \
		--skip-build \
		--optimize=1 \
		--prefix=/usr \
		--root="${pkgdir}"
}

Offline

#6 2023-05-24 22:16:46

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

Re: PKGBUILD Review: csvrewrite

Looking better.

Another thing I noticed, you need git in the makedepends.

You don't need to rename the source to the same thing it's already named, nor do you need to specify the branch if that's the default. Neither one are a problem, though. Similarly, you don't need to have $srcdir in the cd at the beginning of each function, as they're guaranteed to start in $srcdir already, but there's no harm in specifying the full path. Purely a style choice. Also for style choice, you've got braces around ${pkgdir}, but not $srcdir. Better to choose a style and stick with it.

As for using git://, I get

  -> Cloning pycsvrewrite git repo...
Cloning into bare repository '/var/cache/makepkg/pycsvrewrite'...
fatal: unable to connect to github.com:
github.com[0: 140.82.114.3]: errno=Connection timed out

For more info, see https://github.blog/2021-09-01-improvin … ty-github/

Lastly, you might want to look into PEP 517, the new way of installing python packages instead of setuptools.

Anyway, looking good!

Offline

#7 2023-05-25 14:39:43

benjaminhottell
Member
Registered: 2023-05-24
Posts: 4

Re: PKGBUILD Review: csvrewrite

Hi Scimmia,

Thank you again for your suggestions.

I don't know why using git:// it worked on my end, then. It's good that you pointed that mistake out.

You're right about needing git. I've added it. I've also fixed up the redundancies and inconsistencies that you've pointed out.

I looked into PEP 517, thank you for bringing this to my awareness. It took me a while to figure it all out. I've updated the repository and changed the PKGBUILD accordingly.

I used python-flask-dance as a reference for how to do this.

# Maintainer: Benjamin Hottell <benjaminhottell@gmail.com>

pkgname=pycsvrewrite-git
pkgver=1.0.1.r0.g05b3ed8
pkgrel=1
arch=('any')
depends=('python')
makedepends=('python-setuptools' 'git')
license=('AGPL')
pkgdesc="Python script to convert between CSV, TSV, and similar formats"
url='https://github.com/benjaminhottell/pycsvrewrite'
source=('git+https://github.com/benjaminhottell/pycsvrewrite')
md5sums=('SKIP')

pkgver() {
	cd "${pkgname%-git}"
	git describe --long --tags | sed 's/^v//;s/\([^-]*-g\)/r\1/;s/-/./g'
}

build() {
	cd "${pkgname%-git}"
	python -m build --wheel --no-isolation
}

package() {
	cd "${pkgname%-git}"
	python -m installer --destdir="$pkgdir" dist/*.whl
}

Offline

Board footer

Powered by FluxBB