Mail archive
alpine-aports

[alpine-aports] Re: [PATCH v2] main/abuild: Make default_prepare() always end up in "$builddir".

From: Przemysław Pawełczyk <przemoc_at_zoho.com>
Date: Thu, 08 Dec 2016 23:19:51 +0100

 ---- On Wed, 07 Dec 2016 09:04:13 +0100 Timo Teras <timo.teras_at_iki.fi> wrote ----
> On Sat, 3 Dec 2016 20:06:16 +0100
> Przemyslaw Pawelczyk <przemoc_at_zoho.com> wrote:
>
> > ...-default_prepare-always-end-up-in-builddi.patch | 30
> > ++++++++++++++++++++++
> > main/abuild/APKBUILD | 12 ++++++--- 2
> > files changed, 38 insertions(+), 4 deletions(-) create mode 100644
> > main/abuild/0004-abuild-Make-default_prepare-always-end-up-in-builddi.patch
>
> We generally want abuild patches to first go to abuild.git.

I sent patch for abuild to alpine-devel ML before sending this one.
Jirutka applied it there, but with some changes.
Therefore this patch is obviously no longer the same as the one in upstream.

>
> > +diff --git a/abuild.in b/abuild.in
> > +index e49956b76d04..f56ac033f6a9 100644
> > +--- a/abuild.in
> > ++++ b/abuild.in
> > +_at_@ -565,10 +565,10 @@ have_patches() {
> > +
> > + default_prepare() {
> > + local i
> > ++ cd "$builddir" || { error "Is \$builddir set correctly?";
> > return 1; }
> > + if ! have_patches; then
> > + return 0
> > + fi
> > +- cd "$builddir" || { error "Is \$builddir set correctly?";
> > return 1; }
> > + for i in $source; do
> > + case $i in
> > + *.patch)
>
> The problem with this is that not abuild's are updated yet to define
> $builddir. This would make those fail. If it tries to cd there by
> default, it should at least check that $builddir is defined.

builddir is defined thanks to commit 4f37c8efd306:

    abuild: set default builddir when not defined in APKBUILD

that became part of abuild v2.29.0, which is in aports already.

Fail would be if [ "$_builddir" != "$srcdir/$pkgname-$pkgver" ].
I haven't grepped if there are such cases, though.

>
> This was the logic for cd:ing $builddir:
> - since not all (yet) set $builddir, don't do it by default
> - the abuilds that use $_builddir and have patches, should ship their
> own prepare() so it's assumed that having patches and getting to
> default_prepare the variable name has been fixed too
>
> I'd be ok, if in "if ! have_patches" we check if $builddir is set and
> cd to it.

Do you still think that further changes are needed in abuild.git?
 
> Thanks,
> Timo

Regards,
Przemek



---
Unsubscribe:  alpine-aports+unsubscribe_at_lists.alpinelinux.org
Help:         alpine-aports+help_at_lists.alpinelinux.org
---
Received on Thu Dec 08 2016 - 23:19:51 GMT