found 629480 dpkg/1.16.1
quit
Roger Leigh wrote:
> dpkg currently supports
>
> Build-Depends (arch all and any)
> Build-Depends-Indep (arch all)
>
> and the same Build-Conflicts.
>
> This patch adds
>
> Build-Depends-Arch (arch any)
>
> and Build-Conflicts-Arch.
Sorry for the slow reply. Let’s see.
[...]
> --- a/man/deb-src-control.5
> +++ b/man/deb-src-control.5
> @@ -115,7 +115,13 @@ or \fIbuild-indep\fP targets in place of \fIbuild\fP.
> .TP
> .BR Build\-Depends: " <package list>"
> A list of packages that need to be installed and configured to be able to
> build
> -the source package.
> +the source package (both architecture dependent and independent packages).
At first I am tempted to read this as "a list of packages, some of which will
be architecture-dependent and others arch-independent, that need to ...".
Presumably what is intended is something like
In other words, including a dependency in this list has the same effect
as including it in both Build-Depends-Arch and Build-Depends-Indep.
> +
> +.TP
> +.BR Build\-Depends\-Arch: " <package list>"
> +Same as Build\-Depends, but they are only needed when building the
> +architecture dependent packages. The Build\-Depends are also installed
> +in this case.
>
> .TP
> .BR Build\-Depends\-Indep: " <package list>"
Wording is reused from the description of b-d-i. Good.
An idea: it would be nice to sneak in a mention somewhere of the dpkg
version that introduced support for these fields and what happens with
older versions. (*)
> @@ -125,8 +131,14 @@ in this case.
>
> .TP
> .BR Build\-Conflicts: " <package list>"
> -A list of packages that should not be installed when the package is build,
> for
> -example because they interfere with the used build system.
> +A list of packages that should not be installed when the package is built,
> for
> +example because they interfere with the used build system (both
> +architecture dependent and independent packages).
Would be nice to disambiguate that it is the packages that are being
built that are diverse (e.g., by just saying "Including a package in
this list has the same effect of including it in both
Build-Conflicts-Arch and Build-Conflicts-Indep" directly), as above.
[...]
> @@ -134,7 +146,8 @@ Same as Build\-Conflicts, but only when building the
> architecture independent
> packages.
>
> The syntax of the
> -.B Build\-Depends
> +.BR Build\-Depends ,
> +.B Build\-Depends\-Arch
> and
> .B Build\-Depends\-Indep
> fields is a list of groups of alternative packages. Each group is a list
Not about this patch, but shouldn’t the above text be unindented (by
preceding it with .PP), since it is not part of the description of
Build-Conflicts-Indep?
[...]
> --- a/scripts/Dpkg/Control/Fields.pm
> +++ b/scripts/Dpkg/Control/Fields.pm
> @@ -63,6 +63,11 @@ our %FIELDS = (
> dependency => 'union',
> dep_order => 3,
> },
> + 'Build-Conflicts-Arch' => {
> + allowed => ALL_SRC,
> + dependency => 'union',
> + dep_order => 4,
> + },
This value for dep_order matches Build-Conflicts-Indep, so
field_ordered_list(CTRL_PKG_SRC) would not be guaranteed to list
fields in a deterministic order any more. Maybe dep_order should
be 3.5, or the others could be renumbered to let it be 4.
> 'Build-Conflicts-Indep' => {
> allowed => ALL_SRC,
> dependency => 'union',
> @@ -73,6 +78,11 @@ our %FIELDS = (
> dependency => 'normal',
> dep_order => 1,
> },
> + 'Build-Depends-Arch' => {
> + allowed => ALL_SRC,
> + dependency => 'normal',
> + dep_order => 2,
> + },
Likewise.
> --- a/scripts/dpkg-checkbuilddeps.pl
> +++ b/scripts/dpkg-checkbuilddeps.pl
> @@ -42,6 +42,7 @@ sub usage {
> "Usage: %s [<option>...] [<control-file>]")
> . "\n\n" . _g(
> "Options:
> + -A all-only, ignore -Arch.
> -B binary-only, ignore -Indep.
Isn’t the description of "-B" before the comma just wrong? (It sounds
more like a description of "dpkg-buildpackage -b".)
> @@ -56,9 +57,11 @@ sub usage {
> . "\n", $progname;
> }
>
> +my $all_only=0;
> my $binary_only=0;
Here, too. :)
> my ($bd_value, $bc_value);
> -if (!GetOptions('B' => \$binary_only,
> +if (!GetOptions('A' => \$all_only,
> + 'B' => \$binary_only,
> 'help|h' => sub { usage(); exit(0); },
Whitespace doesn’t match.
> 'version' => \&version,
> 'd=s' => \$bd_value,
> @@ -78,10 +81,20 @@ my $facts = parse_status("$admindir/status");
> unless (defined($bd_value) or defined($bc_value)) {
> $bd_value = 'build-essential';
> $bd_value .= ", " . $fields->{"Build-Depends"} if defined
> $fields->{"Build-Depends"};
> + if (not $all_only and defined $fields->{"Build-Depends-Arch"}) {
> + $bd_value .= ", " . $fields->{"Build-Depends-Arch"};
> + }
Sensible.
[...]
> @@ -93,11 +106,11 @@ unless (defined($bd_value) or defined($bc_value)) {
> my (@unmet, @conflicts);
>
> if ($bd_value) {
> - push @unmet, build_depends('Build-Depends/Build-Depends-Indep)',
> + push @unmet,
> build_depends('Build-Depends/Build-Depends-Arch/Build-Depends-Indep)',
> deps_parse($bd_value, reduce_arch => 1), $facts);
The first argument in the code you're modifying looks like a typo
(trailing paren). The resulting error message
error occurred while parsing
Build-Depends/Build-Depends-Arch/Build-Depends-Indep
is a little unpleasant, but I don’t have any better suggestions.
[...]
> --- a/scripts/dpkg-source.pl
> +++ b/scripts/dpkg-source.pl
> @@ -232,7 +232,7 @@ if ($options{'opmode'} =~
> /^(-b|--print-format|--(before|after)-build)$/) {
> $fields->{$_} = $v;
> } elsif (m/^Uploaders$/i) {
> ($fields->{$_} = $v) =~ s/\s*[\r\n]\s*/ /g; # Merge in a single-line
> - } elsif (m/^Build-(Depends|Conflicts)(-Indep)?$/i) {
> + } elsif (m/^Build-(Depends|Conflicts)(-Arch|-Indep)?$/i) {
That’s all it takes? Looks very sane.
The only missing pieces I see are some tiny nits mentioned above, (*),
and that it would be helpful to add a test to
git://git.debian.org/dpkg/pkg-tests.git
to make sure bugs affecting this new functionality get caught early.
Thanks much for your work.
--
To UNSUBSCRIBE, email to [email protected]
with a subject of "unsubscribe". Trouble? Contact [email protected]
