You are not logged in.

#1 2020-04-13 20:13:54

TheSheepGuy
Member
Registered: 2020-04-13
Posts: 47

PKGBUILD review request: vgmtrans-git

The compile instructions can be found on the VGMTrans GitHub wiki.

There are a couple of things I'm unsure about:

  1. The developers use the "interim" prefix for their version numbers, should this be kept?

  2. Should I keep the sha256sums if it's a git package I'm using? Some -git AUR packages do, others don't.

  3. Probably the most important one: the developers don't seem to have anticipated anyone to use "make install", because the makefile (which is generated after running "cmake .." as specified in the GitHub repo wiki) doesn't seem to give appropriate measures in the "install:" bit; I don't have experience using makefiles, so I don't really know what to do about that. The solution I have at the minute is that I just copy the executable to /usr/share, which kind of works?

  4. Should I keep the comment on line 26? Usually from what I've read AUR packages tend to just use "make" and expect you to manually add "-j", but as I've said in the comment the devs recommend to use the option.

  5. Line 22 "rm -rf build" makes sure there's no pre-existing "build" folder. I think that's unnecessary? It would be a safe precaution but I'm not sure whether or not to include it.

# Maintainer: Patrick Ausel <TheSheepGuy1@gmail.com>

pkgname=vgmtrans-git
pkgver=interim.20170225.r342.gad1d0f2
pkgrel=1
pkgdesc="Convert console video games music files to standard MIDI and DSL/SF2 files."
arch=("any")
url="https://github.com/vgmtrans/vgmtrans"
license=('ZLIB')
depends=("qt5-base" "fluidsynth")
makedepends=("qt5-tools" "cmake")
source=("$pkgname::git+git://github.com/vgmtrans/vgmtrans.git#branch=refactor")
sha256sums=("SKIP")

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

build() {
	cd "$srcdir/$pkgname"
	rm -rf build
	mkdir build
	cd build
	cmake ..
	# The developers recommend to use "-j$(nproc)".
	make -j$(nproc)
}

package() {
	cd "$srcdir/$pkgname"
	install -Dm755 "build/src/vgmtrans-qt/vgmtrans" "$pkgdir/usr/share/$pkgname"
	install -Dm644 "LICENSE.txt" "$pkgdir/usr/share/licenses/$pkgname/LICENSE"
}

Last edited by TheSheepGuy (2020-04-13 20:20:48)

Offline

#2 2020-04-13 20:22:25

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

Re: PKGBUILD review request: vgmtrans-git

For a start you should get rid of the -j$(nproc).

If users want to use multiple cores to build then they should set their own makepkg.conf appropriately.


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

#3 2020-04-13 22:32:21

eschwartz
Fellow
Registered: 2014-08-08
Posts: 4,097

Re: PKGBUILD review request: vgmtrans-git

TheSheepGuy wrote:

The compile instructions can be found on the VGMTrans GitHub wiki.

There are a couple of things I'm unsure about:

[*]The developers use the "interim" prefix for their version numbers, should this be kept?[/*]

Does this prefix mean anything? It's not special in the semver sense, it is in fact a waste of space.

TheSheepGuy wrote:

[*]Should I keep the sha256sums if it's a git package I'm using? Some -git AUR packages do, others don't.[/*]

You cannot sha256sum a git repository directory, so that particular element of the sha256sums will be marked as SKIP. This is perfectly normal -- in fact, it's the only way to do it. Same goes for alternative checksum algorithms, no matter which one you use it will be skipped anyway.

(If you have additional sources which are files, not git repos, then you should checksum them as usual.)

TheSheepGuy wrote:

[*]Probably the most important one: the developers don't seem to have anticipated anyone to use "make install", because the makefile (which is generated after running "cmake .." as specified in the GitHub repo wiki) doesn't seem to give appropriate measures in the "install:" bit; I don't have experience using makefiles, so I don't really know what to do about that. The solution I have at the minute is that I just copy the executable to /usr/share, which kind of works?[/*]

This is perfectly normal for upstream software which doesn't specify a make install target, except...

Wait. Why on earth are you using /usr/share instead of /usr/bin, this makes no sense and users won't be able to use it???


TheSheepGuy wrote:

[*]Should I keep the comment on line 26? Usually from what I've read AUR packages tend to just use "make" and expect you to manually add "-j", but as I've said in the comment the devs recommend to use the option.[/*]

Definitely not, if users wish to follow the devs' recommendation they will do as Slithery said and update the MAKEFLAGS in their makepg.conf -- make(1) automatically acts as though the contents of $MAKEFLAGS were specified as command-line options to make.

TheSheepGuy wrote:

[*]Line 22 "rm -rf build" makes sure there's no pre-existing "build" folder. I think that's unnecessary? It would be a safe precaution but I'm not sure whether or not to include it.[/*]

This breaks incremental builds, I'd get rid of it. cmake should already figure out when it needs to rebuild any existing, but stale, source files.


Managing AUR repos The Right Way -- aurpublish (now a standalone tool)

Offline

#4 2020-04-14 12:15:35

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

Re: PKGBUILD review request: vgmtrans-git

eschwartz wrote:
TheSheepGuy wrote:

[*]Line 22 "rm -rf build" makes sure there's no pre-existing "build" folder. I think that's unnecessary? It would be a safe precaution but I'm not sure whether or not to include it.[/*]

This breaks incremental builds, I'd get rid of it. cmake should already figure out when it needs to rebuild any existing, but stale, source files.

Some build systems fail with an incremental build but succeed on a clean build.*
I remove _build folder in PKGBUILD, but switched to doing that in prepare so incremental builds are still possible.

* Have encountered that with autotools, cmake and meson .


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

#5 2020-04-14 12:56:23

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

Re: PKGBUILD review request: vgmtrans-git

Lone_Wolf wrote:

Some build systems fail with an incremental build but succeed on a clean build.

Every such occurrence should be reported as a bug to either the build system if it is failing when it shouldn't, or to the build target maintainers if they are using the build system incorrectly.


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

Offline

#6 2020-04-16 11:07:26

TheSheepGuy
Member
Registered: 2020-04-13
Posts: 47

Re: PKGBUILD review request: vgmtrans-git

Thank you for your help, here's the updated PKGBUILD with the changes mentioned above. I'll wait a couple of days in case there are any other concerns and then I'll submit it to the AUR.

# Maintainer: Patrick Ausel <TheSheepGuy1@gmail.com>

pkgname=vgmtrans-git
pkgver=20170225.r342.gad1d0f2
pkgrel=1
pkgdesc="Convert console video games music files to standard MIDI and DSL/SF2 files."
arch=("any")
url="https://github.com/vgmtrans/vgmtrans"
license=('ZLIB')
depends=("qt5-base" "fluidsynth")
makedepends=("qt5-tools" "cmake")
source=("$pkgname::git+git://github.com/vgmtrans/vgmtrans.git#branch=refactor")
sha256sums=("SKIP")

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

build() {
	cd "$srcdir/$pkgname"
	mkdir build
	cd build
	cmake ..
	make -j$(nproc)
}

package() {
	cd "$srcdir/$pkgname"
	install -Dm755 "build/src/vgmtrans-qt/vgmtrans" "$pkgdir/usr/bin/$pkgname"
	install -Dm644 "LICENSE.txt" "$pkgdir/usr/share/licenses/$pkgname/LICENSE"
}

Offline

#7 2020-04-16 11:10:42

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

Re: PKGBUILD review request: vgmtrans-git


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

#8 2020-04-16 12:16:09

loqs
Member
Registered: 2014-03-06
Posts: 17,195

Re: PKGBUILD review request: vgmtrans-git

Missing makedepends for git.  It is still specifying the number of jobs.
Edit:
You can shorten the build function to:

build() {
	cmake -B build $pkgname
	make -C build
}

package() {
	install -Dm755 "build/src/vgmtrans-qt/vgmtrans" "$pkgdir/usr/bin/$pkgname"
	install -Dm644 "$pkgname/LICENSE.txt" "$pkgdir/usr/share/licenses/$pkgname/LICENSE"
}

Last edited by loqs (2020-04-16 13:02:35)

Offline

#9 2020-04-19 06:15:45

eschwartz
Fellow
Registered: 2014-08-08
Posts: 4,097

Re: PKGBUILD review request: vgmtrans-git

I usually use:

    mkdir -p "${srcdir}"/${pkgname}-${pkgver}/build
    cd "${srcdir}"/${pkgname}-${pkgver}/build

    cmake -G "Unix Makefiles" \
        ..
    make

Managing AUR repos The Right Way -- aurpublish (now a standalone tool)

Offline

Board footer

Powered by FluxBB