Date: Sun, 20 Aug 2017 23:33:34 +0000
From: Paul Morgan <jumanjiman@gmail.com>
To: =?iso-8859-1?Q?S=F6ren?= Tempel <soeren@soeren-tempel.net>
Cc: alpine-aports@lists.alpinelinux.org
Subject: Re: [alpine-aports] [PATCH] testing/createrepo_c: new aport
Message-ID: <20170820233333.GB23286@gmail.com>
References: <20170820211152.GA20563@gmail.com>
 <20170820221638.o5yexog4gaclifv4@francium>
Precedence: list
MIME-Version: 1.0
Content-Type: text/plain; charset="iso-8859-1"
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
In-Reply-To: <20170820221638.o5yexog4gaclifv4@francium>
User-Agent: Mutt/1.8.3 (2017-05-23)

On Mon, Aug 21, 2017 at 12:16:38AM +0200, S=F6ren Tempel wrote:
> Hey,
>=20
> First of all: Thanks for your patch. I added some feedback below:
>=20
> On 20.08.17, Paul Morgan wrote:
> > +# This is not ideal, but it allows to create package with recent commi=
ts.
> > +# git_describe=3D"0.10.0-48-g0cac54d"
> > +treeish=3D"0cac54d09b5a2140b625cabad95dc48898e25cdd"
> > +upstream_tag=3D0.10.0
> > +commits_since=3D48
>=20
> 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.

The 0.10.0 tag was released nearly two years ago.
There've been 48 commits since then.

Some of these commits are important, so the idea is to
use this ugly kludge until upstream tags a new release.


> For this release the tarball is located at: https://github.com/rpm-softwa=
re-management/createrepo_c/archive/0.10.0.tar.gz
>=20
> > +pkgver=3D${upstream_tag}.${commits_since}
>=20
> I would suggest setting the $pkgver to '0.10.0'.

Are you amenable to the existing 0.10.0.48 version
for the reason offered above?


> > +makedepends=3D"
> > +	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
> > +	"
>=20
> The alpine-sdk and musl-dev dependency are always pulled in by default,
> they don't need to be added to $makedepends explicitly.

GTK. I'll rework the patch without those.


> Besides, we usually group multiple dependencies in one line, at least as
> long as that line doesn't exceed 80 characters.

Ah, I do that to avoid future diff churn.


> > +subpackages=3D"
> > +	${pkgname}-dev-doc:devdoc:noarch
>=20
> We don't offer doc subpackages for subpackages. A package should only
> have a single doc subpackage.

Ack. I'll remove the dev docs.


> > +	# https://cmake.org/Wiki/CMake_RPATH_handling#Always_full_RPATH
> > +	cmake .. \
> > +		-DCMAKE_BUILD_TYPE=3DRelease \
> > +		-DCMAKE_INSTALL_PREFIX=3D/usr \
> > +		-DCMAKE_INSTALL_LIBDIR=3Dlib \
> > +		-DCMAKE_SKIP_BUILD_RPATH=3Dfalse \
> > +		-DCMAKE_BUILD_WITH_INSTALL_RPATH=3Dfalse \
> > +		-DCMAKE_INSTALL_RPATH=3D/usr/lib \
> > +		-DCMAKE_INSTALL_RPATH_USE_LINK_PATH=3Dtrue \
> > +		-DPYTHON_DESIRED=3D2 \
> > +		|| return 1
> > +
> > +	# Build binaries plus user docs.
> > +	make || return 1
> > +
> > +	# Build dev docs.
> > +	make doc || return 1
>=20
> Explicit `|| return 1` statements are no longer needed since abuild runs
> with `set -e` nowadays.

GTK. I'll remove those too.

Cheers,
-paul


---
Unsubscribe:  alpine-aports+unsubscribe@lists.alpinelinux.org
Help:         alpine-aports+help@lists.alpinelinux.org
---