]> git.donarmstrong.com Git - debbugs.git/commitdiff
abstract out blocks in Debbugs::Control and have service use the new implementation
authorDon Armstrong <don@donarmstrong.com>
Fri, 29 May 2009 23:51:39 +0000 (16:51 -0700)
committerDon Armstrong <don@donarmstrong.com>
Fri, 29 May 2009 23:51:39 +0000 (16:51 -0700)
Debbugs/Control.pm
scripts/service

index 18c22a73faab4d6aa2b32fd036738a5e4773125f..e0e3458dd0e47f087970bb7e3a33604137835ab1 100644 (file)
@@ -93,6 +93,7 @@ BEGIN{
                     found   => [qw(set_found set_fixed)],
                     fixed   => [qw(set_found set_fixed)],
                     package => [qw(set_package)],
+                    block   => [qw(set_blocks)],
                     archive => [qw(bug_archive bug_unarchive),
                                ],
                     log     => [qw(append_action_to_log),
@@ -252,6 +253,276 @@ my %append_action_options =
 #     __end_control(%info);
 # }
 
+
+=head2 set_blocks
+
+     eval {
+           set_block(bug          => $ref,
+                     transcript   => $transcript,
+                     ($dl > 0 ? (debug => $transcript):()),
+                     requester    => $header{from},
+                     request_addr => $controlrequestaddr,
+                     message      => \@log,
+                      affected_packages => \%affected_packages,
+                     recipients   => \%recipients,
+                     block        => [],
+                     );
+       };
+       if ($@) {
+           $errors++;
+           print {$transcript} "Failed to set blockers of $ref: $@";
+       }
+
+Alters the set of bugs that block this bug from being fixed
+
+This requires altering both this bug (and those it's merged with) as
+well as the bugs that block this bug from being fixed (and those that
+it's merged with)
+
+=over
+
+=item block -- scalar or arrayref of blocking bugs to set, add or remove
+
+=item add -- if true, add blocking bugs
+
+=item remove -- if true, remove blocking bugs
+
+=back
+
+=cut
+
+sub set_blocks {
+    my %param = validate_with(params => \@_,
+                             spec   => {bug => {type   => SCALAR,
+                                                regex  => qr/^\d+$/,
+                                               },
+                                        # specific options here
+                                        block => {type => SCALAR|ARRAYREF,
+                                                  default => [],
+                                                 },
+                                        add    => {type => BOOLEAN,
+                                                   default => 0,
+                                                  },
+                                        remove => {type => BOOLEAN,
+                                                   default => 0,
+                                                  },
+                                        %common_options,
+                                        %append_action_options,
+                                       },
+                            );
+    if ($param{add} and $param{remove}) {
+       croak "It's nonsensical to add and remove the same blocking bugs";
+    }
+    if (grep {$_ !~ /^\d+$/} make_list($param{block})) {
+       croak "Invalid blocking bug(s):".
+           join(', ',grep {$_ !~ /^\d+$/} make_list($param{block}));
+    }
+    my $mode = 'set';
+    if (exists $param{add}) {
+       $mode = 'add';
+    }
+    elsif (exists $param{remove}) {
+       $mode = 'remove';
+    }
+
+    my %info =
+       __begin_control(%param,
+                       command  => 'blocks'
+                      );
+    my ($debug,$transcript) =
+       @info{qw(debug transcript)};
+    my @data = @{$info{data}};
+    my @bugs = @{$info{bugs}};
+
+
+    # The first bit of this code is ugly, and should be cleaned up.
+    # Its purpose is to populate %removed_blockers and %add_blockers
+    # with all of the bugs that should be added or removed as blockers
+    # of all of the bugs which are merged with $param{bug}
+    my %ok_blockers;
+    my %bad_blockers;
+    for my $blocker (make_list($param{block})) {
+       next if $ok_blockers{$blocker} or $bad_blockers{$blocker};
+       my $data = read_bug(bug=>$blocker,
+                          );
+       if (defined $data and not $data->{archive}) {
+           $ok_blockers{$blocker} = 1;
+           my @merged_bugs;
+           push @merged_bugs, split(' ',$data->{mergedwith}) if length $data->{mergedwith};
+           $ok_blockers{@merged_bugs} = (1) x @merged_bugs if @merged_bugs;
+       }
+       else {
+           $bad_blockers{$blocker} = 1;
+       }
+    }
+
+    # throw an error if we are setting the blockers and there is a bad
+    # blocker
+    if (keys %bad_blockers and $mode eq 'set') {
+       croak "Unknown blocking bug(s):".join(', ',keys %bad_blockers).
+           keys %ok_blockers?'':" and no known blocking bug(s)";
+    }
+    # if there are no ok blockers and we are not setting the blockers,
+    # there's an error.
+    if (not keys %ok_blockers and $mode ne 'set') {
+       print {$transcript} "No valid blocking bug(s) given; not doing anything\n";
+       if (keys %bad_blockers) {
+           croak "Unknown blocking bug(s):".join(', ',keys %bad_blockers);
+       }
+       __end_control(%info);
+       return;
+    }
+
+    my @change_blockers = keys %ok_blockers;
+
+    my %removed_blockers;
+    my %added_blockers;
+    my $action = '';
+    my @blockers = map {split ' ', $_->{blockedby}} @data;
+    my %blockers;
+    @blockers{@blockers} = (1) x @blockers;
+
+    # it is nonsensical for a bug to block itself (or a merged
+    # partner); We currently don't allow removal because we'd possibly
+    # deadlock
+
+    my %bugs;
+    @bugs{@bugs} = (1) x @bugs;
+    for my $blocker (@change_blockers) {
+       if ($bugs{$blocker}) {
+           croak "It is nonsensical for a bug to block itself (or a merged partner): $blocker";
+       }
+    }
+    @blockers = keys %blockers;
+    if ($param{add}) {
+       %removed_blockers = ();
+       for my $blocker (@change_blockers) {
+           next if exists $blockers{$blocker};
+           $blockers{$blocker} = 1;
+           $added_blockers{$blocker} = 1;
+       }
+    }
+    elsif ($param{remove}) {
+       %added_blockers = ();
+       for my $blocker (@change_blockers) {
+           next if exists $removed_blockers{$blocker};
+           delete $blockers{$blocker};
+           $removed_blockers{$blocker} = 1;
+       }
+    }
+    else {
+       @removed_blockers{@blockers} = (1) x @blockers;
+       %blockers = ();
+       for my $blocker (@change_blockers) {
+           next if exists $blockers{$blocker};
+           $blockers{$blocker} = 1;
+           if (exists $removed_blockers{$blocker}) {
+               delete $removed_blockers{$blocker};
+           }
+           else {
+               $added_blockers{$blocker} = 1;
+           }
+       }
+    }
+    my @new_blockers = keys %blockers;
+    for my $data (@data) {
+       my $old_data = dclone($data);
+       # remove blockers and/or add new ones as appropriate
+       if ($data->{blockedby} eq '') {
+           print {$transcript} "Was not blocked by any bugs.\n";
+       } else {
+           print {$transcript} "Was blocked by: $data->{blockedby}\n";
+       }
+       my @changed;
+       push @changed, 'added blocking bug(s) '.english_join([keys %added_blockers]) if keys %added_blockers;
+       push @changed, 'removed blocking bug(s) '.english_join([keys %removed_blockers]) if keys %removed_blockers;
+       $action = ucfirst(join ('; ',@changed)) if @changed;
+       if (not @changed) {
+           print {$transcript} "Ignoring request to alter tags of bug #$data->{bug_num} to the same tags previously set\n"
+               unless __internal_request();
+           next;
+       }
+       $data->{blockedby} = join(' ',keys %blockers);
+       append_action_to_log(bug => $data->{bug_num},
+                            command  => 'block',
+                            old_data => $old_data,
+                            new_data => $data,
+                            get_lock => 0,
+                            __return_append_to_log_options(
+                                                           %param,
+                                                           action => $action,
+                                                          ),
+                           )
+           if not exists $param{append_log} or $param{append_log};
+       writebug($data->{bug_num},$data);
+       print {$transcript} "$action\n";
+    }
+    # we do this bit below to avoid code duplication
+    my %mungable_blocks;
+    $mungable_blocks{remove} = \%removed_blockers if keys %removed_blockers;
+    $mungable_blocks{add} = \%added_blockers if keys %added_blockers;
+    for my $add_remove (keys %mungable_blocks) {
+       my @munge_blockers;
+       my %munge_blockers;
+       my $block_locks = 0;
+       for my $blocker (keys %{$mungable_blocks{$add_remove}}) {
+           next if $munge_blockers{$blocker};
+           my ($new_locks, @blocking_data) =
+               lock_read_all_merged_bugs($blocker,
+                                         ($param{archived}?'archive':()));
+           if (not @blocking_data) {
+               unfilelock() for $new_locks;
+               die "Unable to get file lock while trying to $add_remove blocker '$blocker'";
+           }
+           for (map {$_->{bug_num}} @blocking_data) {
+               $munge_blockers{$_} = 1;
+           }
+           for my $data (@blocking_data) {
+               my $old_data = dclone($data);
+               my %blocks;
+               %blocks = split ' ', $data->{blocks};
+               my @blocks;
+               for my $bug (@bugs) {
+                   if ($add_remove eq 'remove') {
+                       next unless exists $blocks{$bug};
+                       delete $blocks{$bug};
+                   }
+                   else {
+                       next if exists $blocks{$bug};
+                       $blocks{$bug} = 1;
+                   }
+                   push @blocks, $bug;
+               }
+               $data->{blocks} = join(' ',sort keys %blocks);
+               my $action = ($add_remove eq 'add'?'Added':'Removed').
+                   " indication that bug $data->{bug_num} blocks".
+                   join(',',@blocks);
+               append_action_to_log(bug => $data->{bug_num},
+                                    command => 'block',
+                                    old_data => $old_data,
+                                    new_data => $data,
+                                    get_lock => 0,
+                                    __return_append_to_log_options(%param,
+                                                                  action => $action
+                                                                  )
+                                   );
+           }
+           __handle_affected_packages(%param,data=>\@blocking_data);
+           add_recipients(recipients => $param{recipients},
+                          actions_taken => {blocks => 1},
+                          data       => \@blocking_data,
+                          debug      => $debug,
+                          transcript => $transcript,
+                         );
+
+           unfilelock() for $new_locks;
+       }
+    }
+    __end_control(%info);
+}
+
+
+
 =head2 set_tag
 
      eval {
@@ -338,12 +609,6 @@ sub set_tag {
     # first things first, make the versions fully qualified source
     # versions
     for my $data (@data) {
-       # The 'done' field gets a bit weird with version tracking,
-       # because a bug may be closed by multiple people in different
-       # branches. Until we have something more flexible, we set it
-       # every time a bug is fixed, and clear it when a bug is found
-       # in a version greater than any version in which the bug is
-       # fixed or when a bug is found and there is no fixed version
        my $action = 'Did not alter tags';
        my %tag_added = ();
        my %tag_removed = ();
@@ -2470,7 +2735,7 @@ sub __end_control {
        @{$info{param}{bugs_affected}}{@{$info{bugs}}} = (1) x @{$info{bugs}};
     }
     add_recipients(recipients => $info{param}{recipients},
-                  (exists $info{param}{command}?(actions_taken => {$info{param}{command} => 1}):()),
+                  (exists $info{param}{command}?(actions_taken => {$info{param}{command} , 1}):()),
                   data       => $info{data},
                   debug      => $info{debug},
                   transcript => $info{transcript},
index 66173aa1a429af462f6df96ff70d2161a0d4285d..31f353ce2332c262432be887df60fb47f51e7d63 100755 (executable)
@@ -818,114 +818,23 @@ END
            $errors++;
            print {$transcript} "Failed to alter tags of $config{bug} $ref: $@";
        }
-    } elsif (m/^(un)?block\s+\#?(-?\d+)\s+(by|with)\s+(\S.*)?$/i) {
+    } elsif (m/^(un)?block\s+\#?(-?\d+)\s+(?:by|with)\s+(\S.*)?$/i) {
        $ok++;
-       my $bugnum = $2; my $blockers = $4;
-       my $addsub = "add";
-       $addsub = "sub" if (defined $1 and $1 eq "un");
-       if ($bugnum =~ m/^-\d+$/ && defined $clonebugs{$bugnum}) {
-            $bugnum = $clonebugs{$bugnum};
-       }
-
-       my @okayblockers;
-       my @badblockers;
-       foreach my $b (split /[\s,]+/, $blockers) {
-           $b=~s/^\#//;
-           if ($b=~/[0-9]+/) {
-               $ref=$b;
-               if ($ref =~ m/^-\d+$/ && defined $clonebugs{$ref}) {
-                    $ref = $clonebugs{$ref};
-               }
-               if (&getbug) {
-                   &foundbug;
-                   push @okayblockers, $ref;
-
-                   # add to the list all bugs that are merged with $b,
-                   # because all of their data must be kept in sync
-                   my @thisbugmergelist= split(/ /,$data->{mergedwith});
-                   &cancelbug;
-
-                   foreach $ref (@thisbugmergelist) {
-                       if (&getbug) {
-                          push @okayblockers, $ref;
-                          &cancelbug;
-                       }
-                   }
-               }
-               else {
-                   &notfoundbug;
-                    push @badblockers, $ref;
-               }
-           }
-           else {
-                push @badblockers, $b;
-           }
-       }
-       if (@badblockers) {
-            print {$transcript} "Unknown blocking bug/s: ".join(', ', @badblockers).".\n";
+        $ref= $2;
+       my $add_remove = defined $1 && $1 eq 'un';
+       my @blockers = split /[\s,]+/, $3;
+       $ref = $clonebugs{$ref} if exists $clonebugs{$ref};
+       $bug_affected{$ref} = 1;
+       eval {
+            set_blocks(@common_control_options,
+                       bug          => $ref,
+                       block        => \@blockers,
+                       $add_remove ? (remove => 1):(add => 1),
+                      );
+       };
+       if ($@) {
            $errors++;
-       }
-       
-       $ref=$bugnum;
-       if (&setbug) {
-           if ($data->{blockedby} eq '') {
-               print {$transcript} "Was not blocked by any bugs.\n";
-           } else {
-               print {$transcript} "Was blocked by: $data->{blockedby}\n";
-           }
-           if ($addsub eq "set") {
-               $action= "Blocking bugs of $bugnum set to: " . join(", ", @okayblockers);
-           } elsif ($addsub eq "add") {
-               $action= "Blocking bugs of $bugnum added: " . join(", ", @okayblockers);
-           } elsif ($addsub eq "sub") {
-               $action= "Blocking bugs of $bugnum removed: " . join(", ", @okayblockers);
-           }
-           my %removedblocks;
-           my %addedblocks;
-           do {
-                $affected_packages{$data->{package}} = 1;
-               add_recipients(data => $data,
-                              recipients => \%recipients,
-                              transcript   => $transcript,
-                              ($dl > 0 ? (debug => $transcript):()),
-                             );
-               my @oldblockerlist = split ' ', $data->{blockedby};
-               $data->{blockedby} = '' if ($addsub eq "set");
-               foreach my $b (@okayblockers) {
-                       $data->{blockedby} = manipset($data->{blockedby}, $b,
-                               ($addsub ne "sub"));
-               }
-
-               foreach my $b (@oldblockerlist) {
-                       if (! grep { $_ eq $b } split ' ', $data->{blockedby}) {
-                               push @{$removedblocks{$b}}, $ref;
-                       }
-               }
-               foreach my $b (split ' ', $data->{blockedby}) {
-                       if (! grep { $_ eq $b } @oldblockerlist) {
-                               push @{$addedblocks{$b}}, $ref;
-                       }
-               }
-            } while (&getnextbug);
-
-           # Now that the blockedby data is updated, change blocks data
-           # to match the changes.
-           foreach $ref (keys %addedblocks) {
-               if (&getbug) {
-                   foreach my $b (@{$addedblocks{$ref}}) {
-                       $data->{blocks} = manipset($data->{blocks}, $b, 1);
-                   }
-                   &savebug;
-                }
-           }
-           foreach $ref (keys %removedblocks) {
-               if (&getbug) {
-                   foreach my $b (@{$removedblocks{$ref}}) {
-                       $data->{blocks} = manipset($data->{blocks}, $b, 0);
-                   }
-                   &savebug;
-                }
-           }
+           print {$transcript} "Failed to set blocking bugs of $ref: $@";
        }
     } elsif (m/^retitle\s+\#?(-?\d+)\s+(\S.*\S)\s*$/i) {
         $ok++;