On Mon, Aug 21, 2017 at 12:16:38AM +0200, Sören Tempel wrote:
> 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.
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-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'.
Are you amenable to the existing 0.10.0.48 version
for the reason offered above?
> > +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.
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="> > + ${pkgname}-dev-doc:devdoc:noarch> > 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=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.
GTK. I'll remove those too.
Cheers,
-paul
---
Unsubscribe: alpine-aports+unsubscribe@lists.alpinelinux.org
Help: alpine-aports+help@lists.alpinelinux.org
---
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@lists.alpinelinux.org
Help: alpine-aports+help@lists.alpinelinux.org
---
ok, then i'll rework the patch with the other changes you suggested.
tyvm
-paul
On Tue, Aug 22, 2017 at 5:13 PM, Sören Tempel <soeren@soeren-tempel.net>
wrote:
> On 20.08.17, Paul Morgan wrote:> > > 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'.> >> > Are you amenable to the existing 0.10.0.48 version> > for the reason offered above?>> I would of cause prefer it if upstream simply made a new release but I> guess your pkgver is okay in that case.>> Greetings,> Sören>
--
Paul Morgan