Bug 1068494: Remove CVS/Bonsai/LXR-specific bits of Patch Viewer

Bugzilla/Attachment/PatchReader.pm

@@ -23,17 +23,18 @@ use Bugzilla::Util;
 use constant PERLIO_IS_ENABLED => $Config{useperlio};
 
 sub process_diff {
-    my ($attachment, $format, $context) = @_;
+    my ($attachment, $format) = @_;
     my $dbh = Bugzilla->dbh;
     my $cgi = Bugzilla->cgi;
     my $lc  = Bugzilla->localconfig;
     my $vars = {};
 
-    my ($reader, $last_reader) = setup_patch_readers(undef, $context);
+    require PatchReader::Raw;
+    my $reader = new PatchReader::Raw;
 
     if ($format eq 'raw') {
         require PatchReader::DiffPrinter::raw;
-        $last_reader->sends_data_to(new PatchReader::DiffPrinter::raw());
+        $reader->sends_data_to(new PatchReader::DiffPrinter::raw());
         # Actually print out the patch.
         print $cgi->header(-type => 'text/plain');
         disable_utf8();
@@ -71,7 +72,7 @@ sub process_diff {
         $vars->{'description'} = $attachment->description;
         $vars->{'other_patches'} = \@other_patches;
 
-        setup_template_patch_reader($last_reader, $format, $context, $vars);
+        setup_template_patch_reader($reader, $vars);
         # The patch is going to be displayed in a HTML page and if the utf8
         # param is enabled, we have to encode attachment data as utf8.
         if (Bugzilla->params->{'utf8'}) {
@@ -83,11 +84,13 @@ sub process_diff {
 }
 
 sub process_interdiff {
-    my ($old_attachment, $new_attachment, $format, $context) = @_;
+    my ($old_attachment, $new_attachment, $format) = @_;
     my $cgi = Bugzilla->cgi;
     my $lc  = Bugzilla->localconfig;
     my $vars = {};
 
+    require PatchReader::Raw;
+
     # Encode attachment data as utf8 if it's going to be displayed in a HTML
     # page using the UTF-8 encoding.
     if ($format ne 'raw' && Bugzilla->params->{'utf8'}) {
@@ -177,10 +180,11 @@ sub process_interdiff {
         $warning = 'interdiff3';
     }
 
-    my ($reader, $last_reader) = setup_patch_readers("", $context);
+    my $reader = new PatchReader::Raw;
+
     if ($format eq 'raw') {
         require PatchReader::DiffPrinter::raw;
-        $last_reader->sends_data_to(new PatchReader::DiffPrinter::raw());
+        $reader->sends_data_to(new PatchReader::DiffPrinter::raw());
         # Actually print out the patch.
         print $cgi->header(-type => 'text/plain');
         disable_utf8();
@@ -193,7 +197,7 @@ sub process_interdiff {
         $vars->{'newid'} = $new_attachment->id;
         $vars->{'new_desc'} = $new_attachment->description;
 
-        setup_template_patch_reader($last_reader, $format, $context, $vars);
+        setup_template_patch_reader($reader, $vars);
     }
     $reader->iterate_string('interdiff #' . $old_attachment->id .
                             ' #' . $new_attachment->id, $stdout);
@@ -208,7 +212,6 @@ sub get_unified_diff {
 
     # Bring in the modules we need.
     require PatchReader::Raw;
-    require PatchReader::FixPatchRoot;
     require PatchReader::DiffPrinter::raw;
     require PatchReader::PatchInfoGrabber;
     require File::Temp;
@@ -220,14 +223,6 @@ sub get_unified_diff {
     my $reader = new PatchReader::Raw;
     my $last_reader = $reader;
 
-    # Fixes patch root (makes canonical if possible).
-    if (Bugzilla->params->{'cvsroot'}) {
-        my $fix_patch_root =
-            new PatchReader::FixPatchRoot(Bugzilla->params->{'cvsroot'});
-        $last_reader->sends_data_to($fix_patch_root);
-        $last_reader = $fix_patch_root;
-    }
-
     # Grabs the patch file info.
     my $patch_info_grabber = new PatchReader::PatchInfoGrabber();
     $last_reader->sends_data_to($patch_info_grabber);
@@ -274,46 +269,8 @@ sub warn_if_interdiff_might_fail {
     return undef;
 }
 
-sub setup_patch_readers {
-    my ($diff_root, $context) = @_;
-
-    # Parameters:
-    # format=raw|html
-    # context=patch|file|0-n
-    # collapsed=0|1
-    # headers=0|1
-
-    # Define the patch readers.
-    # The reader that reads the patch in (whatever its format).
-    require PatchReader::Raw;
-    my $reader = new PatchReader::Raw;
-    my $last_reader = $reader;
-    # Fix the patch root if we have a cvs root.
-    if (Bugzilla->params->{'cvsroot'}) {
-        require PatchReader::FixPatchRoot;
-        $last_reader->sends_data_to(new PatchReader::FixPatchRoot(Bugzilla->params->{'cvsroot'}));
-        $last_reader->sends_data_to->diff_root($diff_root) if defined($diff_root);
-        $last_reader = $last_reader->sends_data_to;
-    }
-
-    # Add in cvs context if we have the necessary info to do it
-    if ($context ne 'patch' && Bugzilla->localconfig->{cvsbin} 
-        && Bugzilla->params->{'cvsroot_get'}) 
-    {
-        require PatchReader::AddCVSContext;
-        # We need to set $cvsbin as global, because PatchReader::CVSClient
-        # needs it in order to find 'cvs'.
-        $main::cvsbin = Bugzilla->localconfig->{cvsbin};
-        $last_reader->sends_data_to(
-          new PatchReader::AddCVSContext($context, Bugzilla->params->{'cvsroot_get'}));
-        $last_reader = $last_reader->sends_data_to;
-    }
-
-    return ($reader, $last_reader);
-}
-
 sub setup_template_patch_reader {
-    my ($last_reader, $format, $context, $vars) = @_;
+    my ($last_reader, $vars) = @_;
     my $cgi = Bugzilla->cgi;
     my $template = Bugzilla->template;
 
@@ -328,22 +285,15 @@ sub setup_template_patch_reader {
     }
 
     $vars->{'collapsed'} = $cgi->param('collapsed');
-    $vars->{'context'} = $context;
-    $vars->{'do_context'} = Bugzilla->localconfig->{cvsbin} 
-                            && Bugzilla->params->{'cvsroot_get'} && !$vars->{'newid'};
 
     # Print everything out.
     print $cgi->header(-type => 'text/html');
 
     $last_reader->sends_data_to(new PatchReader::DiffPrinter::template($template,
-                                "attachment/diff-header.$format.tmpl",
-                                "attachment/diff-file.$format.tmpl",
-                                "attachment/diff-footer.$format.tmpl",
-                                { %{$vars},
-                                  bonsai_url => Bugzilla->params->{'bonsai_url'},
-                                  lxr_url => Bugzilla->params->{'lxr_url'},
-                                  lxr_root => Bugzilla->params->{'lxr_root'},
-                                }));
+                                'attachment/diff-header.html.tmpl',
+                                'attachment/diff-file.html.tmpl',
+                                'attachment/diff-footer.html.tmpl',
+                                $vars));
 }
 
 1;
@@ -364,6 +314,4 @@ __END__
 
 =item process_interdiff
 
-=item setup_patch_readers
-
 =back

Bugzilla/Config/PatchViewer.pm

-# This Source Code Form is subject to the terms of the Mozilla Public
-# License, v. 2.0. If a copy of the MPL was not distributed with this
-# file, You can obtain one at http://mozilla.org/MPL/2.0/.
-#
-# This Source Code Form is "Incompatible With Secondary Licenses", as
-# defined by the Mozilla Public License, v. 2.0.
-
-package Bugzilla::Config::PatchViewer;
-
-use 5.10.1;
-use strict;
-use warnings;
-
-use Bugzilla::Config::Common;
-
-our $sortkey = 1300;
-
-sub get_param_list {
-  my $class = shift;
-  my @param_list = (
-  {
-   name    => 'cvsroot',
-   type    => 't',
-   default => '',
-  },
-
-  {
-   name    => 'cvsroot_get',
-   type    => 't',
-   default => '',
-  },
-
-  {
-   name    => 'bonsai_url',
-   type    => 't',
-   default => ''
-  },
-
-  {
-   name    => 'lxr_url',
-   type    => 't',
-   default => ''
-  },
-
-  {
-   name    => 'lxr_root',
-   type    => 't',
-   default => '',
-  } );
-  return @param_list;
-}
-
-1;

Bugzilla/Install/Localconfig.pm

@@ -101,10 +101,6 @@ use constant LOCALCONFIG_VARS => (
         default => 0,
     },
     {
-        name    => 'cvsbin',
-        default => sub { bin_loc('cvs') },
-    },
-    {
         name    => 'interdiffbin',
         default => sub { bin_loc('interdiff') },
     },

attachment.cgi

@@ -190,20 +190,6 @@ sub validateFormat {
   return $format;
 }
 
-# Validates context of a diff/interdiff. Will throw an error if the context
-# is not number, "file" or "patch". Returns the validated, detainted context.
-sub validateContext
-{
-  my $context = $cgi->param('context') || "patch";
-  if ($context ne "file" && $context ne "patch") {
-      my $orig_context = $context;
-      detaint_natural($context)
-        || ThrowUserError("invalid_context", { context => $orig_context });
-  }
-
-  return $context;
-}
-
 # Gets the attachment object(s) generated by validateID, while ensuring
 # attachbase and token authentication is used when required.
 sub get_attachment {
@@ -392,17 +378,15 @@ sub interdiff {
         $old_attachment = validateID('oldid');
         $new_attachment = validateID('newid');
     }
-    my $context = validateContext();
 
     Bugzilla::Attachment::PatchReader::process_interdiff(
-        $old_attachment, $new_attachment, $format, $context);
+        $old_attachment, $new_attachment, $format);
 }
 
 sub diff {
     # Retrieve and validate parameters
     my $format = validateFormat('html', 'raw');
     my $attachment = $format eq 'raw' ? get_attachment() : validateID();
-    my $context = validateContext();
 
     # If it is not a patch, view normally.
     if (!$attachment->ispatch) {
@@ -410,7 +394,7 @@ sub diff {
         return;
     }
 
-    Bugzilla::Attachment::PatchReader::process_diff($attachment, $format, $context);
+    Bugzilla::Attachment::PatchReader::process_diff($attachment, $format);
 }
 
 # Display all attachments for a given bug in a series of IFRAMEs within one

docs/en/rst/using.rst

@@ -623,11 +623,10 @@ file first.
 Patch Viewer
 ============
 
-Viewing and reviewing patches in Bugzilla is often difficult due to
-lack of context, improper format and the inherent readability issues that
-raw patches present.  Patch Viewer is an enhancement to Bugzilla designed
-to fix that by offering increased context, linking to sections, and
-integrating with Bonsai, LXR and CVS.
+Viewing and reviewing patches in Bugzilla is often difficult due to improper
+format and the inherent readability issues that raw patches present.  Patch
+Viewer is an enhancement to Bugzilla designed to fix that by offering linking
+to sections.
 
 Patch viewer allows you to:
 
@@ -636,17 +635,12 @@ Patch viewer allows you to:
 
 + See the difference between two patches.
 
-+ Get more context in a patch.
-
 + Collapse and expand sections of a patch for easy
   reading.
 
 + Link to a particular section of a patch for discussion or
   review
 
-+ Go to Bonsai or LXR to see more context, blame, and
-  cross-references for the part of the patch you are looking at
-
 + Create a rawtext unified format diff out of any patch, no
   matter what format it came from
 
@@ -671,18 +665,6 @@ dropdown at the top of the page ("Differences between \[dropdown] and
 this patch") and click the "Diff" button. This will show you what
 is new or changed in the newer patch.
 
-.. _patchviewer_context:
-
-Getting More Context in a Patch
--------------------------------
-
-To get more context in a patch, you put a number in the textbox at
-the top of Patch Viewer ("Patch / File / \[textbox]") and hit enter.
-This will give you that many lines of context before and after each
-change. Alternatively, you can click on the "File" link there and it
-will show each change in the full context of the file. This feature only
-works against files that were diffed using "cvs diff".
-
 .. _patchviewer_collapse:
 
 Collapsing and Expanding Sections of a Patch
@@ -705,20 +687,6 @@ able to give someone a URL to show them which part you are talking
 about) you simply click the "Link Here" link on the section header. The
 resulting URL can be copied and used in discussion.
 
-.. _patchviewer_bonsai_lxr:
-
-Going to Bonsai and LXR
------------------------
-
-To go to Bonsai to get blame for the lines you are interested in,
-you can click the "Lines XX-YY" link on the section header you are
-interested in. This works even if the patch is against an old
-version of the file, since Bonsai stores all versions of the file.
-
-To go to LXR, you click on the filename on the file header
-(unfortunately, since LXR only does the most recent version, line
-numbers are likely to rot).
-
 .. _patchviewer_unified_diff:
 
 Creating a Unified Diff

template/en/default/admin/params/patchviewer.html.tmpl

-[%# This Source Code Form is subject to the terms of the Mozilla Public
-  # License, v. 2.0. If a copy of the MPL was not distributed with this
-  # file, You can obtain one at http://mozilla.org/MPL/2.0/.
-  #
-  # This Source Code Form is "Incompatible With Secondary Licenses", as
-  # defined by the Mozilla Public License, v. 2.0.
-  #%]
-[%
-   title = "Patch Viewer"
-   desc = "Set up third-party applications to run with PatchViewer"
-%]
-
-[% param_descs = {
-  cvsroot => "The <a href=\"http://www.cvshome.org\">CVS</a> root that most " _
-             "users of your system will be using for 'cvs diff'. Used in " _
-             "Patch Viewer ('Diff' option on patches) to figure out where " _
-             "patches are rooted even if users did the 'cvs diff' from " _
-             "different places in the directory structure. (NOTE: if your " _
-             "CVS repository is remote and requires a password, you must " _
-             "either ensure the $terms.Bugzilla user has done a 'cvs login' or " _
-             "specify the password " _
-             "<a href=\"http://www.cvshome.org/docs/manual/cvs_2.html#SEC26\">as " _
-             "part of the CVS root</a>.) Leave this blank if you have no " _
-             "CVS repository.",
-
-  cvsroot_get => "The CVS root Bugzilla will be using to get patches from. " _
-                 "Some installations may want to mirror their CVS repository on " _
-                 "the Bugzilla server or even have it on that same server, and " _
-                 "thus the repository can be the local file system (and much " _
-                 "faster). Make this the same as cvsroot if you don't " _
-                 "understand what this is (if cvsroot is blank, make this blank too).",
-
-  bonsai_url => "The URL to a <a href=\"http://www.mozilla.org/bonsai.html\">Bonsai</a> " _
-                "server containing information about your CVS repository. " _
-                "Patch Viewer will use this information to create links to " _
-                "bonsai's blame for each section of a patch (it will append " _
-                "'/cvsblame.cgi?...' to this url). Leave this blank if you " _
-                "don't understand what this is.",
-
-  lxr_url => "The URL to an <a href=\"http://sourceforge.net/projects/lxr\">LXR</a> server " _
-             "that indexes your CVS repository. Patch Viewer will use this " _
-             "information to create links to LXR for each file in a patch. " _
-             "Leave this blank if you don't understand what this is.",
-
-  lxr_root => "Some LXR installations do not index the CVS repository from the root -- " _
-              "<a href=\"http://lxr.mozilla.org/mozilla\">Mozilla's</a>, for " _
-              "example, starts indexing under <code>mozilla/</code>. This " _
-              "means URLs are relative to that extra path under the root. " _
-              "Enter this if you have a similar situation. Leave it blank " _
-              "if you don't know what this is." }
-%]

template/en/default/attachment/diff-file.html.tmpl

@@ -19,11 +19,7 @@
  onclick="return twisty_click(this)">[% collapsed ? '(+)' : '(-)' %]</a><input
  type="checkbox" name="[% file.filename FILTER html %]"[% collapsed ? '' : ' checked' %]
  class="bz_default_hidden">
-  [% IF lxr_prefix && !file.is_add %]
-    <a href="[% lxr_prefix %]">[% file.filename FILTER html %]</a>
-  [% ELSE %]
-    [% file.filename FILTER html %]
-  [% END %]
+  [% file.filename FILTER html %]
   [% IF file.plus_lines %]
     [% IF file.minus_lines %]
       (-[% file.minus_lines %]&nbsp;/&nbsp;+[% file.plus_lines %]&nbsp;lines)
@@ -49,23 +45,13 @@ incremental_restore()
         [% IF file.is_add %]
           Added
         [% ELSIF file.is_remove %]
-          [% IF bonsai_prefix %]
-            <a href="[% bonsai_prefix %]">Removed</a>
-          [% ELSE %]
-            Removed
-          [% END %]
+          Removed
         [% ELSE %]
-          [% IF bonsai_prefix %]
-            <a href="[% bonsai_prefix %]#[% section.old_start %]">
-          [% END %]
           [% IF section.old_lines > 1 %]
             Lines&nbsp;[% section.old_start %]-[% section.old_start + section.old_lines - 1 %]
           [% ELSE %]
             Line&nbsp;[% section.old_start %]
           [% END %]
-          [% IF bonsai_prefix %]
-            </a>
-          [% END %]
           [%+ section.func_info FILTER html IF section.func_info %]
         [% END %]
       </span>

template/en/default/attachment/diff-header.html.tmpl

@@ -60,8 +60,7 @@ Interdiff of #[% oldid %] and #[% newid %] for [% terms.bug %] #[% bugid %]
   [% IF headers %]
     <a href="[% PROCESS viewurl id=attachid %]">View</a>
     | <a href="[% PROCESS editurl id=attachid %]">Details</a>
-    | <a href="[% PROCESS diffurl id=attachid %]&amp;context=[%
-               context FILTER uri %]&amp;collapsed=[% collapsed FILTER uri %]&amp;headers=[%
+    | <a href="[% PROCESS diffurl id=attachid %]&amp;collapsed=[% collapsed FILTER uri %]&amp;headers=[%
                headers FILTER uri %]&amp;format=raw">Raw&nbsp;Unified</a>
     | Return to [% "$terms.bug $bugid" FILTER bug_link(bugid) FILTER none %]
   [% END %]
@@ -102,33 +101,6 @@ Interdiff of #[% oldid %] and #[% newid %] for [% terms.bug %] #[% bugid %]
    onmouseout="window.status = lastStatus; return true"
    onclick="return expand_all()">Expand All</a>
 
-[% IF do_context %]
-  [%# only happens for normal viewing, not interdiff %]
-  | <strong>Context:</strong>
-  [% IF context == "patch" %]
-    (<strong>Patch</strong> / 
-  [% ELSE %]
-    (<a href="[% PROCESS diffurl id=attachid %]&amp;headers=[% headers FILTER uri %]">Patch</a> / 
-  [% END %]
-  [% IF context == "file" %]
-    <strong>File</strong> /
-  [% ELSE %]
-    <a href="[% PROCESS diffurl id=attachid %]&amp;headers=[% headers FILTER uri %]&amp;context=file">File</a> / 
-  [% END %]
-
-  [% IF context == "patch" || context == "file" %]
-    [% context = 3 %]
-  [% END %]
-  [%# textbox for context %]
-  <form class="inline" action="attachment.cgi">
-    <input type="hidden" name="action" value="diff">
-    <input type="hidden" name="id" value="[% attachid %]">
-    <input type="hidden" name="collapsed" value="[% collapsed FILTER html %]">
-    <input type="hidden" name="headers" value="[% headers FILTER html %]">
-    <input type="text" name="context" value="[% context FILTER html %]" size="3">
-  </form>)
-[% END %]
-
 [% IF warning %]
 <h2 class="warning">
   Warning:

template/en/default/filterexceptions.pl

@@ -322,10 +322,8 @@
 ],
 
 'attachment/diff-file.html.tmpl' => [
-  'lxr_prefix',
   'file.minus_lines',
   'file.plus_lines',
-  'bonsai_prefix',
   'section.old_start',
   'section_num',
   'current_line_old',

template/en/default/setup/strings.txt.pl

@@ -155,10 +155,6 @@ they don't exist.
 
 If this is set to 0, checksetup.pl will not create .htaccess files.
 END
-    localconfig_cvsbin => <<'END',
-If you want to use the CVS integration of the Patch Viewer, please specify
-the full path to the "cvs" executable here.
-END
     localconfig_db_check => <<'END',
 Should checksetup.pl try to verify that your database setup is correct?
 With some combinations of database servers/Perl modules/moonphase this