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

[Guido Günther]

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.

> +
>  # 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?
Cheers,
 -- Guido