On Wed, 11 Jan 2017 13:22:05 +0100
Tom Zander
On Saturday, 7 January 2017 14:29:58 CET Doug Newgard wrote:
Alright, I've looked at the first one. There's a whole lot of simplification you could do, and a few actual problems.
Problems:
You need to rename the source file. It's just "v1.2.0.tar.gz" which is very generic and could conflict with other packages.
This is what github creates when you push a tag, I have no choice in the matter. Why would there be a conflict? This is only used for makepkg and any package that is created is always compiled in its own dir. The PKGBUILD file doesn’t have a project name in front of it either, ensuring its all done in its own dir... What am I missing?
Hardcoding the version here also makes no sense when you could just use $pkgver and only have to update it in one place when you update.
Hmm? I did exactlyt that. The pkver is set once and used everywhere... Maybe you looked at a ‘-git’ package?
source=(${pkgname}-${pkgver}.tar.gz::"https://github.com/bitcoinclassic/bitcoinclassic/archive/v1.2.0.tar.gz" Sure looks hardcoded to me.
The install file does WAY, WAY more than it should. The only thing I see there that should be happening is creating the user and group, everything else should be removed.
Maybe you are assuming that a ‘make install’ will be able to do everything we need. Unfortunately thats not the case... And I don’t want to change the upstream automake files. So individual install lines are needed.
I'm not talking about the package function, I'm talking about the .install file with the post_{install,upgrade,remove} scriptlets. Specifically: Disabling COW should be left up to the sysadmin. chown should be done in the PKGBUILD if at all possible Users should not be deleted; this is a security issue if the UID gets reused. Your big fancy message shouldn't be there. This is normal stuff. The whole "/etc/bitcoin/bitcoin.conf.dist" thing shouldn't happen. Just install to /etc/bitcoin/bitcoin.conf in the package function.
make -j$(nproc) is bad. The user sets makeflags in makepkg.conf, you should not be overriding that unless it's necessary.
ACK.
Simplifications:
Functions are guaranteed to start in $srcdir, you don't need to include that in the cd commands.
ACK
All of the "msg2" lines are useless.
ACK
In the package function, you cd to $srcdir/bitcoinclassic-$pkgver, but then you still include that in every install command. It's redundant and unnecessary.
ACK
The cp then desktop-file-install for the .desktop file is really unnecessary when you could just install it like everything else.
ACK
Some of the install commands could be combined. If you're not renaming the file, you can just specify a target dir with -t and install all files to that dir at once.
ACK
For renaming, just file a merge request on the right hand side of the page to merge one package into the other.
will do :)
Doug
Thanks Doug!