Hi,
Gilles Filippini wrote:
> The attached patch appears to solve my problem, with
> --extend-diff-ignore specified either from the command line or from a
> debian/source/[local-]options file.
>
> Rationals are:
> * options from the command line should be interpreted first
> * then should come options from debian/source/options
> * and finally options from debian/source/local-options
> * --extend-diff-ignore should actually extend diff-ignore instead of the
> default diff-ignore regex.
So, in an ideal world these would be multiple patches, one fixing each
issue. No matter; here's some quick reactions nonetheless. Thanks
very much for making this more concrete.
> --- /usr/bin/dpkg-source 2011-04-16 03:54:49.000000000 +0200
> +++ ./dpkg-source 2011-05-06 01:41:43.000000000 +0200
> @@ -113,7 +113,7 @@
> "options" =>
> qr/^--(?:format=|unapply-patches$|abort-on-upstream-changes$)/,
> "local-options" => qr/^--format=/,
> };
> - foreach my $filename ("local-options", "options") {
> + foreach my $filename ("options", "local-options") {
> my $conf = Dpkg::Conf->new();
> my $optfile = File::Spec->catfile($dir, "debian", "source", $filename);
> next unless -f $optfile;
> @@ -122,7 +122,7 @@
> if (@$conf) {
> info(_g("using options from %s: %s"), $optfile, join(" ", @$conf))
> unless $options{'opmode'} eq "--print-format";
> - unshift @options, @$conf;
> + push @options, @$conf;
> }
Could you explain what the net effect of these changes is, preferrably
with an example?
Without reading carefully, I can't see why this wouldn't be a no-op.
> @@ -157,7 +157,11 @@
> } elsif (m/^-(?:i|-diff-ignore(?:$|=))(.*)$/) {
> $options{'diff_ignore_regexp'} = $1 ? $1 :
> $Dpkg::Source::Package::diff_ignore_default_regexp;
> } elsif (m/^--extend-diff-ignore=(.+)$/) {
> - $Dpkg::Source::Package::diff_ignore_default_regexp .= "|$1";
> + if ($options{'diff_ignore_regexp'}) {
> + $options{'diff_ignore_regexp'} .= "|$1";
> + } else {
> + $options{'diff_ignore_regexp'} =
> $Dpkg::Source::Package::diff_ignore_default_regexp . "|$1";
> + }
> } elsif (m/^-(?:I|-tar-ignore=)(.+)$/) {
This means that "dpkg -b <directory> ..."
--extend-diff-ignore=foo would mean
--extend-diff-ignore=foo -i
-ifoo --extend-diff-ignore=bar -i would mean
-i (i.e., later -i overrides --extend-diff-ignore after earlier -i)
-ifoo --extend-diff-ignore=bar would mean
-ifoo|bar
--extend-diff-ignore=bar -ifoo would mean
-ifoo (i.e., later -i<regex> overrides --extend-diff-ignore)
So it's a big change in semantics and the result is somewhat
confusing. Are you sure that's intended?
In general, the "later -i overrides earlier -i" semantics interact
with --extend-diff-ignore in a confusing way already. So for the
original problem I am not sure what do you. One way would involve
two fixes:
1. Teach git-buildpackage to use --extend-diff-ignore instead of
-i<regex> and use plain -i so packagers' extend-diff-ignore can
be respected
2. Make plain -i to toggle an option meaning "use
diff_ignore_default_regexp", so "-i --extend-diff-ignore=foo" can
mean the same thing as "--extend-diff-ignore=foo -i". Introduce a
new --clear-diff-ignore. Discourage use of -i<regex> in the hope
that --clear-diff-ignore, --extend-diff-ignore, and -i will be
easier to work with.
Thoughts?
Regards,
Jonathan
--
To UNSUBSCRIBE, email to [email protected]
with a subject of "unsubscribe". Trouble? Contact [email protected]
