You are not logged in.

#1 2017-05-31 07:16:42

terut
Member
Registered: 2017-05-31
Posts: 7

[SOLVED] PKGBUILD review request: heroku

Hi there! Can somebody please review my PKGBUILD on https://github.com/terut/aur-heroku-cli ?
Basically, it follows the standalone version installation on https://devcenter.heroku.com/articles/heroku-cli .

PKGBUILD:

# Maintainer: Terunori Togo <terut.dev+github at gmail dot com>
pkgname=heroku
pkgver=5.7.1.c574890
pkgrel=1
pkgdesc="Everything you need to get started with Heroku"
arch=("x86_64")
url="https://cli.heroku.com"
license=('MIT')
depends=('python')
makedepends=()
optdepends=('git: deployment support'
            'nodejs: cli plugins support'
            'npm: cli plugins support')
install="heroku.install"
source=("heroku.tar.gz::https://cli-assets.heroku.com/branches/stable/heroku-linux-amd64.tar.gz"
        "https://raw.githubusercontent.com/herok … er/LICENSE")
md5sums=('SKIP'
         '29da5ffd8cf020c01dcf968b867f7067')

pkgver() {
  cd "$srcdir/$pkgname"
  bin/heroku --version | sed -e 's/^.*\/\(.*\)\s(.*$/\1/g' -e 's/-/./g'
}

prepare() {
  cp "$srcdir"/LICENSE "$srcdir/$pkgname"
}

package() {
  cd "$srcdir/$pkgname"

  mkdir -p "$pkgdir"/usr/{lib,bin}

  cp -r "$srcdir/$pkgname" "$pkgdir"/usr/lib/
  install -Dm755 bin/heroku "$pkgdir"/usr/bin/
  install -Dm644 LICENSE "$pkgdir"/usr/share/licenses/$pkgname/LICENSE
}


heroku.install:

## arg 1:  the new package version
post_install() {
  printf "\n"
  printf "You have to execute the following commands to upgrade automatically:\n\n"
  printf "     $ rm -rf ~/.local/share/heroku/cli\n"
  printf "     $ mkdir -p ~/.local/share/heroku\n"
  printf "     $ cp -r /usr/lib/heroku ~/.local/share/heroku/cli\n\n"
}

## arg 1:  the old package version
post_remove() {
  printf "\n"
  printf "Please remove the following directories to uninstall compeletely:\n\n"
  printf "     $ rm -rf ~/.local/share/heroku ~/.config/heroku ~/.cache/heroku ~/.heroku\n\n"
}

Best regards,
terut

Last edited by terut (2017-06-07 08:33:38)

Offline

#2 2017-05-31 13:25:36

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

Re: [SOLVED] PKGBUILD review request: heroku

Ignoring the actual PKGBUILD problems, what is with the install script? If this will only run from ~/.local, there's really no way to package it.

Offline

#3 2017-05-31 20:54:37

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

Re: [SOLVED] PKGBUILD review request: heroku

The file that is downloaded contains a binary that appears to be intended  to install to /usr/local .

Terut, if this is indeed published under a MIT license heroku devs must provide the sourcecode somewhere.
Ask them where you can find it.


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

#4 2017-05-31 21:21:05

Xyne
Administrator/PM
Registered: 2008-08-03
Posts: 6,963
Website

Re: [SOLVED] PKGBUILD review request: heroku

  • Remove empty variables (e.g. makedepends).

  • Use stronger checksums. In general I would recommend always using the strongest one currently supported (sha512 at the moment).

  • Add the checksum for the heroku.tar.gz file. "SKIP" is only meant for VCS sources (e.g. git). I suspect that you used "SKIP" here because the source archive is unversioned, but that is a security hole. When the upstream source archive is updated, you will need to update the PKGBUILD to change the version and replace the checksum. If possible, try to find a versioned source archive to avoid the issue with upstream updates.

  • The pkgver() function should only be used with VCS packages that have no fixed package version. Remove it. Again, I assume that you are trying to work around the unversioned source file, but this is the wrong way to do it.

  • The prepare() function is meant for applying patches and modifying the source files once after download. Do not use it just to copy files. Do that in the package function.

  • You copy the license to $srcdir/$pkgname, then you install that under /usr/lib. That makes no sense. The license is not a library. You correctly install it anyway with the last line in package().

  • You don't need to mkdir $pkgdir/usr/bin. The "install -Dm..." command will do that for you. The command, however, should be 'install -Dm755 bin/heroku "$pkgdir"/usr/bin/heroku'.

  • Typically you use echo (or cat with a HEREDOC) to display messages in the install script.

  • I am not sure that the install script messages are necessary but I am not familiar with the heroko. Perhaps it would make sense to add a custom "heroku-update" script to do that automatically. In that case, "~/.local/share" should probably be replaced with "${XDG_DATA_HOME:-~/.local/share}" so that it uses XDG_DATA_HOME if it is set and falls back to the default location if not.

I see now that the source archive is not a true source archive. I second Lone_Wolf's suggestion to ask for the real source files and use those in the PKGBUILD.


My Arch Linux StuffForum EtiquetteCommunity Ethos - Arch is not for everyone

Offline

#5 2017-05-31 22:30:08

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

Re: [SOLVED] PKGBUILD review request: heroku

Lone_Wolf wrote:

Terut, if this is indeed published under a MIT license heroku devs must provide the sourcecode somewhere.

Where is that required in an MIT style license?

Offline

#6 2017-06-01 04:45:17

terut
Member
Registered: 2017-05-31
Posts: 7

Re: [SOLVED] PKGBUILD review request: heroku

Scimmia wrote:

what is with the install script?

Thank you for your question. I wanna show the message for this tool.

Lone_Wolf wrote:

Ask them where you can find it.

Thanks for your advice. I will do that.

Xyne wrote:

I see now that the source archive is not a true source archive.

Yes, heroku offers some binary files for linux, win and so on. Can I use binary file for package if the version can fix? I checked the source code of heroku cli but it's so complicated to build. it's written by golang, but they use Makefile(https://github.com/heroku/cli/blob/master/Makefile) to build the binary to include nodejs and npm. So I wanna use their binary for package. Basically, in this case,  do I have to build from source code for my package?

Thank you for your great advice. I really appreciate it. I'm going to apply what I can.

Last edited by terut (2017-06-01 04:47:12)

Offline

#7 2017-06-01 06:51:15

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

Re: [SOLVED] PKGBUILD review request: heroku

terut wrote:
Scimmia wrote:

what is with the install script?

Thank you for your question. I wanna show the message for this tool.

My question is WHY you want to show that message. It's a horrible way of packaging.

Offline

#8 2017-06-01 07:21:09

terut
Member
Registered: 2017-05-31
Posts: 7

Re: [SOLVED] PKGBUILD review request: heroku

Thanks you for reply. I got it. It needs to upgrade the command automatically. Actually, heroku cli's install script will do that.
See the install script:  https://github.com/heroku/cli/blob/mast … ne/install

Maybe it doesn't need the instructions because heroku cli will create that directories when you run heroku login

In addition, I read Arch package standard on Arch wiki and it describes about .install file.

All important messages should be echoed during install using an .install file. For example, if a package needs extra setup to work, directions should be included.

Where should I write those messages?

Last edited by terut (2017-06-01 07:31:46)

Offline

#9 2017-06-01 07:43:54

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

Re: [SOLVED] PKGBUILD review request: heroku

Nowhere. None of that should be done at all.

Offline

#10 2017-06-01 10:55:37

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

Re: [SOLVED] PKGBUILD review request: heroku

Terut,

package should leave user folders alone.
You could use an install message like "As user run /usr/bin/heroku-install to get started" , but that's it.



----------------------------------

loqs wrote:
Lone_Wolf wrote:

Terut, if this is indeed published under a MIT license heroku devs must provide the sourcecode somewhere.

Where is that required in an MIT style license?


You are correct providing the source code is not mentioned.
Heroku could claim that the MIT license applies only to the binary files, not to the sourcecode needed to create them.

It would be a case where the copyright owner/ publisher adheres to the letter of the license instead of the spirit.
Asking if the sourcecode is available won't hurt though.

-----------------------------------------------------------------------


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

#11 2017-06-05 04:23:40

terut
Member
Registered: 2017-05-31
Posts: 7

Re: [SOLVED] PKGBUILD review request: heroku

Hi all,
Sorry for late reply. I asked heroku support to download the fixed version. They said "Use node version cli."
They changed master from golang version to node one and updated their document.
See: https://devcenter.heroku.com/articles/h … pm-version

Scimmia wrote:

Nowhere. None of that should be done at all.

I got it. I remove .install file. Thanks!

Lone_Wolf wrote:

package should leave user folders alone.
You could use an install message like "As user run /usr/bin/heroku-install to get started" , but that's it.

I'd better prepare install script when the package change /home , right?  I got it.
Thank you for the advice.

I updated PKGBUILD for node version.

# Maintainer: Terunori Togo <terut.dev+github at gmail dot com>
_npmname=heroku-cli
pkgname=nodejs-$_npmname
pkgver=6.7.4
pkgrel=1
pkgdesc="The next generation Node-based Heroku CLI"
arch=("any")
url="https://github.com/heroku/cli"
license=('ISC')
depends=('nodejs>=7.10.0' 'npm')
source=("https://registry.npmjs.org/$_npmname/-/$_npmname-$pkgver.tgz")
noextract=($_npmname-$pkgver.tgz)
sha512sums=('8fb89ad3576bf3a4eae15d11a1d18818825e75082be8339fff3c0ec14ffd911131b26f490d7d752c46a2c2959e383c5497b956e8c08a1937c938ffdbd81dcb93')

package() {
  cd $srcdir
  local _npmdir="$pkgdir/usr/lib/node_modules"
  mkdir -p $_npmdir
  cd $_npmdir
  npm install --user root -g --prefix "$pkgdir/usr" $_npmname@$pkgver
  install -Dm644 "$_npmdir/$_npmname"/LICENSE "$pkgdir"/usr/share/licenses/$pkgname/LICENSE
}

I referred the following packages:

https://www.archlinux.org/packages/comm … dejs-less/
https://aur.archlinux.org/cgit/aur.git/ … ejs-jshint
https://aur.archlinux.org/cgit/aur.git/ … js-webpack
https://aur.archlinux.org/cgit/aur.git/ … js-express

Looks good?

Last edited by terut (2017-06-05 05:32:43)

Offline

#12 2017-06-05 05:32:30

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

Re: [SOLVED] PKGBUILD review request: heroku

No, I don't see where you are using the downloaded source tarball -- instead you have npm pull it remotely again, anyway. You should `npm install $options "$srcdir"/$_npmname-$pkgver.tgz`

npm can run in an extracted folder, or take the path of a tarball, or take a registry name (in which case it will try to remotely download it).

Of the four packages you say you referenced, one of them is from the official repos and does it the way I just explained, and the other three are AUR packages that need to be fixed.

From the look of it, some automatic PKGBUILD generator is responsible for creating nodejs PKGBUILDs that do the wrong thing against many $_npmname PKGBUILDs. It is a shame such projects are so irresponsible. sad

Last edited by eschwartz (2017-06-05 05:34:33)


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

Offline

#13 2017-06-05 05:57:14

terut
Member
Registered: 2017-05-31
Posts: 7

Re: [SOLVED] PKGBUILD review request: heroku

Thank you for great advice.
I didn't know about any auto generator. It's not good ;(

I changed the installation as you said.

package() {
  local _npmdir="$pkgdir/usr/lib/node_modules"
  npm install -g --user root --prefix "$pkgdir"/usr "$srcdir"/$_npmname-$pkgver.tgz
  install -Dm644 "$_npmdir/$_npmname"/LICENSE "$pkgdir"/usr/share/licenses/$pkgname/LICENSE
  rm -r "$pkgdir"/usr/etc
  chmod -R go-w "$pkgdir"/usr
}

Offline

#14 2017-06-05 06:31:56

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

Re: [SOLVED] PKGBUILD review request: heroku

I didn't know about the auto-generator either until I saw it given credit in a comment in the nodejs-webpack PKGBUILD. big_smile

That looks much better, although I have no idea why [community]/nodejs-less deletes the /usr/etc dir and it may well be something specific to that package, so you may or may not have to do that yourself.


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

Offline

#15 2017-06-05 07:04:13

terut
Member
Registered: 2017-05-31
Posts: 7

Re: [SOLVED] PKGBUILD review request: heroku

Thanks!

It seems that $pkgdir/usr/etc directory is created automatically but it's empty. That's why it would remove /usr/etc.
Anyway, My PKGBUILD looks better thanks to you. I'll release this package smile

Offline

#16 2017-06-05 11:15:24

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

Re: [SOLVED] PKGBUILD review request: heroku

/usr/etc looks very much like a non-standard directory, if it wasn't empty the files in it should probably be in another location .

While makepkg can remove empty directories, removing it manually feels cleaner for this case.


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

#17 2017-06-06 03:28:03

terut
Member
Registered: 2017-05-31
Posts: 7

Re: [SOLVED] PKGBUILD review request: heroku

Thank you for reply.

/usr/etc looks like setting directory for npm. As long as I saw npm document, npm maybe generally create /usr/local/etc without prefix option. But, in this case, I specified a installation directory with prefix option in PKGBUILD, following Aur packaging standards on Arch wiki. So npm would create /usr/etc automatically.

Anyway, I'm going to add prefix [SOLVED] to title tonight. Thank you for reviewing, all!

Last edited by terut (2017-06-06 03:31:38)

Offline

Board footer

Powered by FluxBB