Mail archive
alpine-aports

Re: [alpine-aports] [PATCH] testing/createrepo_c: new aport

From: Sören Tempel <soeren_at_soeren-tempel.net>
Date: Mon, 21 Aug 2017 00:16:38 +0200

Hey,

First of all: Thanks for your patch. I added some feedback below:

On 20.08.17, Paul Morgan wrote:
> +# This is not ideal, but it allows to create package with recent commits.
> +# git_describe="0.10.0-48-g0cac54d"
> +treeish="0cac54d09b5a2140b625cabad95dc48898e25cdd"
> +upstream_tag=0.10.0
> +commits_since=48

Is there any reason why you use the tarball created for the commit
instead of using the tarball created for the git-tag? Normally, we
prefer the latter.

For this release the tarball is located at: https://github.com/rpm-software-management/createrepo_c/archive/0.10.0.tar.gz

> +pkgver=${upstream_tag}.${commits_since}

I would suggest setting the $pkgver to '0.10.0'.

> +makedepends="
> + alpine-sdk
> + bash-completion
> + bzip2-dev
> + cmake
> + curl-dev
> + doxygen
> + expat-dev
> + file-dev
> + glib-dev
> + libressl-dev
> + libxml2-dev
> + musl-dev
> + py2-sphinx
> + python2-dev
> + rpm-dev
> + scanelf
> + sqlite-dev
> + xz-dev
> + zlib-dev
> + "

The alpine-sdk and musl-dev dependency are always pulled in by default,
they don't need to be added to $makedepends explicitly.

Besides, we usually group multiple dependencies in one line, at least as
long as that line doesn't exceed 80 characters.

> +subpackages="
> + ${pkgname}-dev-doc:devdoc:noarch

We don't offer doc subpackages for subpackages. A package should only
have a single doc subpackage.

> + # https://cmake.org/Wiki/CMake_RPATH_handling#Always_full_RPATH
> + cmake .. \
> + -DCMAKE_BUILD_TYPE=Release \
> + -DCMAKE_INSTALL_PREFIX=/usr \
> + -DCMAKE_INSTALL_LIBDIR=lib \
> + -DCMAKE_SKIP_BUILD_RPATH=false \
> + -DCMAKE_BUILD_WITH_INSTALL_RPATH=false \
> + -DCMAKE_INSTALL_RPATH=/usr/lib \
> + -DCMAKE_INSTALL_RPATH_USE_LINK_PATH=true \
> + -DPYTHON_DESIRED=2 \
> + || return 1
> +
> + # Build binaries plus user docs.
> + make || return 1
> +
> + # Build dev docs.
> + make doc || return 1

Explicit `|| return 1` statements are no longer needed since abuild runs
with `set -e` nowadays.

Cheers,
Sören


---
Unsubscribe:  alpine-aports+unsubscribe_at_lists.alpinelinux.org
Help:         alpine-aports+help_at_lists.alpinelinux.org
---
Received on Mon Aug 21 2017 - 00:16:38 GMT