PKGBUILD review request: rubymine-url-handler

Rocket3G · 2017-02-11 22:37:54

Hi,

Because there exists a really awesome package to add url handling support to PHPStorm I decided to port that functionality to RubyMine. Could someone review this PKGBUILD? Thanks in advance!

# Maintainer: Sander Verkuil <saverkuil@gmail.com>
# Please report issues at https://github.com/SanderVerkuil/arch-aur-rubymine-url-handler

_pkgname=rubymine-url-handler
pkgname=${_pkgname}
pkgver=1.1.0
pkgrel=1
pkgdesc="Open x-mine:// URLs in RubyMine."
arch=('any')
url='https://github.com/SanderVerkuil/rubymine-url-handler'
license=('GPL')
depends=('desktop-file-utils')
makedepends=("git")
install=${_pkgname}.install
source=("${_pkgname}"::"git+https://github.com/SanderVerkuil/rubymine-url-handler.git")
sha512sums=('SKIP')

package() {
  cd "${srcdir}"
  install -Dm755 ${_pkgname}/${_pkgname} "$pkgdir/usr/bin/${_pkgname}"
  RPM_BUILD_ROOT=$pkgdir desktop-file-install ${_pkgname}/${_pkgname}.desktop
}

Kind regards,
Sander

Xyne · 2017-02-11 23:10:58

The upstream source files are in a Git repo so the pkgname should end with the "-git" suffix (${_pkgname}-git). The package should include "provides" and "conflicts" arrays with entries for the non-git package:

provides=($_pkgname)
conflicts=($_pkgname)

You can change the source entry to "${_pkgname}::git+https://github.com/SanderVerkuil/${_pkgname}.git".

Quoting is used inconsistently (single quotes, double quotes, no quotes). It is not a technical issue, only a stylistic one. As long as all variables set elsewhere are quoted (srcdir, pkgdir), it will work.
Variable brackets are also used inconsistently (e.g. "$pkgdir/usr/bin/${_pkgname}"). Again, this is a stylistic issue and not a technical issue.

Everything else looks good to me.

eschwartz · 2017-02-12 02:48:40

Well, I'd also like to know why you use the _pkgname variable when it is identical to the pkgname.

desktop-file-utils is a depends instead of a makedepends, which is incorrect as the actual package doesn't use it (only the package() function). And you can really just use install -Dm644 ... to install the desktop file to /usr/share/applications and save having to install a special utility to copy a file to a pre-known location.
On the other hand, the script itself seems to call a program called "jetbrains-rubymine", which I cannot find in the AUR and which anyway needs to be a dependency.
The source=() can be simply "git+${url}" as it is the actual url and doesn't need to be renamed to ${pkgname} since it will already download as that...

As Xyne said, there is a lot of inconsistency in your style, consider unifying your use of quotes and variable braces for the sake of readability and general good habits.

Last edited by eschwartz (2017-02-12 04:10:46)

Rocket3G · 2017-02-12 11:27:35

Thanks for the feedback. The main reason it currently is the way it is, is because I ported https://github.com/sanduhrs/arch-aur-ph … rl-handler to be valid for rubymine as well.

I think the new PKGBUILD is improved. Thanks for the feedback!

# Maintainer: Sander Verkuil <saverkuil@gmail.com>
# Please report issues at https://github.com/SanderVerkuil/arch-aur-rubymine-url-handler

_pkgname=rubymine-url-handler
pkgname=("${_pkgname}-git")
pkgver=1.0.0
pkgrel=1
pkgdesc="Open x-mine:// URLs in RubyMine."
arch=("any")
url="https://github.com/SanderVerkuil/${_pkgname}"
license=("GPL")
depends=("desktop-file-utils" "rubymine")
makedepends=("git")
install=${_pkgname}.install
source=("${_pkgname}"::"git+${url}")
sha512sums=("SKIP")
provides=(${_pkgname})
conflicts=(${_pkgname})

package() {
  cd "${srcdir}"
  install -Dm755 "${_pkgname}/${_pkgname}" "${pkgdir}/usr/bin/${_pkgname}"
  RPM_BUILD_ROOT="${pkgdir}" desktop-file-install "${_pkgname}/${_pkgname}.desktop"
}

The `jetbrains-rubymine` binary file is provided by the rubymine package. I've added that dependency.

eschwartz · 2017-02-14 05:57:52

Well, I still don't know why desktop-file-utils is a depends instead of a makedepends.
You also have an install script you haven't shown us, which may very well be obsoleted by the update-dekstop-database pacman hook (which, come to think of it, may explain why you still included that dependency.)

And you might as well get rid of it entirely, by using

install -Dm644 "${_pkgname}/${_pkgname}.desktop" "${pkgdir}/usr/share/applications/${_pkgname}.desktop"

instead of desktop-file-install.

Arch Linux

#1 2017-02-11 22:37:54

PKGBUILD review request: rubymine-url-handler

#2 2017-02-11 23:10:58

Re: PKGBUILD review request: rubymine-url-handler

#3 2017-02-12 02:48:40

Re: PKGBUILD review request: rubymine-url-handler

#4 2017-02-12 11:27:35

Re: PKGBUILD review request: rubymine-url-handler

#5 2017-02-14 05:57:52

Re: PKGBUILD review request: rubymine-url-handler

Board footer