On Wed, 11 Jan 2017 13:22:05 +0100
Tom Zander <t...@freedommail.ch> wrote:
> 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!
