[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: draft ballot: please rule on how to implement debian/rules build-arch

On Tue, 2012-01-24 at 12:25:40 +0100, Raphael Hertzog wrote:
> Ok, then I suggest to go ahead with the attached patch. If I don't hear
> any objections, I'll push it later this week.

> commit 557b5809eef183da2e4907e994dabaa9273e7e0b
> Author: Raphaël Hertzog <hertzog@debian.org>
> Date:   Tue Jan 24 11:59:44 2012 +0100
>     dpkg-buildpackage: use build-arch and build-indep targets of debian/rules
>     'build-arch' is used when building only arch-any binaries (-B)
>     while 'build-indep' is used when building only arch-all binaries (-A).
>     To avoid breaking too many packages, dpkg-buildpackages verifies that
>     those targets are implemented by calling “make -f debian/rules -qn
>     <target>” and ensuring that it doesn't fail with exit code 2. Otherwise
>     it fallsback to using the 'build' target.
>     This fallback is a temporary measure until all packages have been
>     converted to properly support the build-arch and build-indep targets.

Actually thinking about this, making this temporary will imply that
once this would get removed old/external packages would stop building
which is not acceptable. So I think we might need to carry this for
quite some time (if not indefinitely).

> diff --git a/man/dpkg-buildpackage.1 b/man/dpkg-buildpackage.1
> index 5410a20..f7d1cf1 100644
> --- a/man/dpkg-buildpackage.1
> +++ b/man/dpkg-buildpackage.1
> @@ -27,12 +27,13 @@ It calls \fBdpkg\-source \-b\fP to generate the source package (unless
>  a binary\-only build has been requested with \fB\-b\fP, \fB\-B\fP or
>  \fB\-A\fP).
>  .IP \fB5.\fP 3
> -It calls \fBdebian/rules\fP \fBbuild\fP followed by
> -\fBfakeroot debian/rules\fP \fIbinary-target\fP (unless a source-only
> -build has been requested with \fB\-S\fP). Note that \fIbinary-target\fR is
> -either \fBbinary\fP (default case, or if \fB\-b\fP is specified)
> -or \fBbinary\-arch\fP (if \fB\-B\fP is specified) or \fBbinary\-indep\fP
> -(if \fB\-A\fP is specified).
> +It calls \fBdebian/rules\fP \fIbuild\-target\fP followed by
> +\fBfakeroot debian/rules\fP \fIbinary\-target\fP (unless a source-only
> +build has been requested with \fB\-S\fP). Note that \fIbuild\-target\fR
> +and \fIbinary\-target\fP are either \fBbuild\fP and \fBbinary\fP (default
> +case, or if \fB\-b\fP is specified), or \fBbuild\-arch\fP and
> +\fBbinary\-arch\fP (if \fB\-B\fP is specified), or \fBbuild\-indep\fP and
> +\fBbinary\-indep\fP (if \fB\-A\fP is specified).

foo-target do not need the ‘-’ escaped as they are variable text not
to be used as input on the command line for example.

>  .IP \fB6.\fP 3
>  It calls \fBgpg\fP to sign the \fB.dsc\fP file (if any, unless
>  \fB\-us\fP is specified).
> @@ -241,6 +242,13 @@ exported compiler flags (\fBCFLAGS\fP, \fBCXXFLAGS\fP, \fBFFLAGS\fP,
>  \fBCPPFLAGS\fP and \fBLDFLAGS\fP) with values as returned
>  by \fBdpkg\-buildflags\fP. This is no longer the case.
>  .
> +\fBdpkg\-buildpackage\fP is using the \fBbuild-arch\fP and
> +\fBbuild-indep\fP targets since version 1.16.2. Those targets are thus
> +mandatory. But to avoid breakages of existing packages, and ease
> +the transition, it will fallback to using the \fBbuild\fP target
> +if \fBmake -f debian/rules -qn\fP \fIbuild-target\fP returns 2 as
> +exit code.

build-arch/build-indep/-f/-qn need ‘-’ escaped. Also given my comment
above about the backwards compability issues, I'd remove the transition

>  It should be possible to specify spaces and shell metacharacters in
>  and initial arguments for
> diff --git a/scripts/dpkg-buildpackage.pl b/scripts/dpkg-buildpackage.pl
> index 50e6170..e1dd538 100755
> --- a/scripts/dpkg-buildpackage.pl
> +++ b/scripts/dpkg-buildpackage.pl
> @@ -385,6 +390,7 @@ if ($call_target) {
>  unless ($noclean) {
>      withecho(@rootcommand, @debian_rules, 'clean');
>  }
> +
>  unless (build_binaryonly) {
>      warning(_g("it is a bad idea to generate a source package " .
>                 "without cleaning up first, it might contain undesired " .

Why the spurious newlines?

> @@ -393,10 +399,30 @@ unless (build_binaryonly) {
>      withecho('dpkg-source', @source_opts, '-b', $dir);
>      chdir($dir) or syserr("chdir $dir");
>  }
> +
> +unless ($buildtarget eq "build" or scalar(@debian_rules) > 1) {
> +    # Verify that build-{arch,indep} are supported. If not, fallback to build.
> +    # This is a temporary measure to avoid breaking too many packages on a flag day.

These comment lines could be wrapped to < 80 chars. And again I'd
remove the comment about this being a temporary measure.

>  unless (build_sourceonly) {
> -    withecho(@debian_rules, 'build');
> +    withecho(@debian_rules, $buildtarget);
>      withecho(@rootcommand, @debian_rules, $binarytarget);
>  }
> +

Spurious new line.

In any case, looks good, thanks! So, besides those comments:

Acked-by: Guillem Jover <guillem@debian.org>


Reply to: