[git-buildpackage] [PATCH] gbp-buildpackage-rpm: pass commands to mock

[Tzafrir Cohen]

On Thu, Jun 30, 2016 at 11:11:33PM +0200, Guido Günther wrote:
> On Thu, Jun 30, 2016 at 10:59:17PM +0200, Tzafrir Cohen wrote:
> > Fix gbp-buildpackage-rpm and the mock builder script to allow passing
> > extra command-line parameters to the mock wrapper build script.
> > 
> > Also fixes support for --git-mock-options. However this cannot pass any
> > option that has spaces in it (such as --define).
> > 
> > Signed-off-by: Tzafrir Cohen <tzafrir at debian.org>
> > ---
> >  bin/gbp-builder-mock                    | 16 +++++++++++-----
> >  docs/manpages/gbp-buildpackage-rpm.sgml |  1 +
> >  gbp/scripts/buildpackage_rpm.py         |  9 +++++----
> >  3 files changed, 17 insertions(+), 9 deletions(-)
> > 
> > diff --git a/bin/gbp-builder-mock b/bin/gbp-builder-mock
> > index 1e93493..0e3453c 100755
> > --- a/bin/gbp-builder-mock
> > +++ b/bin/gbp-builder-mock
> > @@ -1,4 +1,4 @@
> > -#!/bin/sh
> > +#!/bin/bash
> >  #
> >  # Helper to invoke mock from 'gbp buildpackage-rpm'
> >  #
> > @@ -36,14 +36,21 @@ usage() {
> >  	exit $EXIT
> >  }
> >  
> > +declare -a EXTRA_ARGS
> > +
> >  while [ $# != 0 ]; do
> >  	case "$1" in
> > -		--help|-h|-?) usage 0;;
> > +		--help|-h) usage 0;;
> >  		*.spec) SPEC="$1";;
> > +		*) EXTRA_ARGS+=("$1");;
> >  	esac
> >  	shift
> >  done
> >  
> > +if [ ! -z "$GBP_BUILDER_MOCK_OPTIONS" ]; then
> > +	EXTRA_ARGS+=($GBP_BUILDER_MOCK_OPTIONS) # unquoted
> > +fi
> 
> Could catch, we weren't setting them at all.

It should just be removed. Adding a patch to remove it.

> 
> > +
> >  # Make sure we have the necessary tools.
> >  if [ ! -x /usr/bin/mock ]; then
> >      echo "mock not found; you need to install the mock package" >&2
> > @@ -71,16 +78,15 @@ gbp_builder_mock() {
> >  	local resultdir="$export_dir/$pat"
> >  	local mock="mock -r $root --resultdir=$srpms --spec=$spec --sources=$sources"
> >  
> > -	$mock --buildsrpm
> > +	$mock "${EXTRA_ARGS[@]}" --buildsrpm
> >  	# Assuming that nothing was built in this directory since the previous command:
> >  	local srpm=`ls -t $PWD/SRPMS/*.src.rpm 2>/dev/null| head -n1`
> >  	if [ -z $srpm ]; then
> >  		echo >&2 "$0: failed to create srpm"
> >  		exit 1
> >  	fi
> > -	$mock --no-cleanup-after --resultdir $resultdir --rebuild "$srpm"
> > +	$mock "${EXTRA_ARGS[@]}" --no-cleanup-after --resultdir $resultdir --rebuild "$srpm"
> >  }
> >  
> > -
> >  fix_arch
> >  gbp_builder_mock
> > diff --git a/docs/manpages/gbp-buildpackage-rpm.sgml b/docs/manpages/gbp-buildpackage-rpm.sgml
> > index ac020f8..d5aa8f7 100644
> > --- a/docs/manpages/gbp-buildpackage-rpm.sgml
> > +++ b/docs/manpages/gbp-buildpackage-rpm.sgml
> > @@ -63,6 +63,7 @@
> >        <arg><option>--git-arch</option>=<replaceable>ARCHITECTURE</replaceable></arg>
> >        <arg><option>--git-mock-options</option>=<replaceable>OPTIONS</replaceable></arg>
> >        <arg><option>--git-mock-root</option>=<replaceable>ROOT</replaceable></arg>
> > +      <arg rep="repeat"><option>OPTION_PASSED_TO_BUILD_CMD</option></arg>
> >      </cmdsynopsis>
> >    </refsynopsisdiv>
> >    <refsect1>
> > diff --git a/gbp/scripts/buildpackage_rpm.py b/gbp/scripts/buildpackage_rpm.py
> > index 1dc1ab2..1cff73e 100644
> > --- a/gbp/scripts/buildpackage_rpm.py
> > +++ b/gbp/scripts/buildpackage_rpm.py
> > @@ -258,10 +258,11 @@ def setup_builder(options, builder_args):
> >      if options.builder == 'rpmbuild':
> >          if len(builder_args) == 0:
> >              builder_args.append('-ba')
> > -        builder_args.extend([
> > -            '--define "_topdir %s"' % os.path.abspath(options.export_dir),
> > -            '--define "_specdir %%_topdir/%s"' % options.export_specdir,
> > -            '--define "_sourcedir %%_topdir/%s"' % options.export_sourcedir])
> > +        if not options.use_mock:
> > +            builder_args.extend([
> > +                '--define "_topdir %s"' % os.path.abspath(options.export_dir),
> > +                '--define "_specdir %%_topdir/%s"' % options.export_specdir,
> > +                '--define "_sourcedir %%_topdir/%s"' % options.export_sourcedir])
> 
> It think this would be better done by swapping the setup_mock and
> setup_builder invocations like (in case mock can run without any of the
> above defines, can it?):
> 
>    # Setup builder opts
>    if options.use_mock:
>      setup_mock(options)
>    setup_builder(options, builder_args)
> 
> instead of the current:
> 
>    # Setup builder opts
>    setup_builder(options, builder_args)
>      if options.use_mock:
>        setup_mock(options)
> 
> What do you think? Could you check that out?

I don't follow. Under mock those are not set. Mock runs rpmbuild in the
chroot and looks for the results in the default locations. The order of
calling those two functions doesn't seem to matter.

Re-sending the patch with two extra minor clean-ups.

-- 
Tzafrir Cohen         | tzafrir at jabber.org | VIM is
http://tzafrir.org.il |                    | a Mutt's
tzafrir at cohens.org.il |                    |  best
tzafrir at debian.org    |                    | friend