365558 - BMO patch diff page constructing invalid file path names

Status: VERIFIED FIXED

(bugzilla.mozilla.org :: General, defect)

Description

[Nelson Bolyard (seldom reads bugmail)]

Go to bug 232600 and click on the "diff" link for either of the first two 
attachments.  Look at the name of the file displayed at the top of the diffs.
It is wrong.  If you click on it, it takes you to an lxr page that says
"This file does not exist".  

The first attachment is a patch to a file named WIN32.mk.  In the diff page,
that file name appears as mozilla/security/coreconf/WINWIN32.mk
Notice the 3 duplicated characters: ----------------^^^

The second attachment is a patch to a file named rules.mk.  In the diff page,
that file name appears as mozilla/security/coreconf/rulrules.mk
Again, notice the 3 duplicated characters: ---------^^^

BMO has had this problem before, and it was 
fixed.  Now, it's back.  I can't find the previous bug report.

Comment 1

[Dave Miller [:justdave]]

OK, think I fixed it.  We had a local copy of the PatchReader module in the bugzilla directory.  I didn't know what it was for, and we have the package installed globally, so I moved it out of the way.  Turns out the PatchReader package as shipped is apparently buggy, and we had a local bugfix applied to it.  I put the previously-disabled copy back in place and the problem seems to have gone away.

[root@recluse bugzilla]# diff -ur /usr/lib/perl5/vendor_perl/5.8.5/PatchReader PatchReader
diff -ur /usr/lib/perl5/vendor_perl/5.8.5/PatchReader/Raw.pm PatchReader/Raw.pm
--- /usr/lib/perl5/vendor_perl/5.8.5/PatchReader/Raw.pm 2004-07-17 00:14:35.000000000 -0700
+++ PatchReader/Raw.pm  2005-07-23 08:54:52.000000000 -0700
@@ -63,6 +63,7 @@
 
   } elsif ($line =~ /^RCS file: (.+)/) {
     $this->{FILE_STATE}{rcs_filename} = $1;
+    $this->{FILE_STATE}{rcs_filename} =~ s/\s+$//;
 
     $this->{IN_HEADER} = 1;
 

We probably need to find out why this hasn't made it upstream...

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

Thanks Dave, that fixed it.

Let me make a few observations and suggestions.

It appears that (in the past) when local changes were made, such as creating 
a local copy of PatchReader and patching it, that no record of the change 
was kept in any log file.  So, later, when doing an update, the reason for 
local changes is no longer apparent.  Let me suggest that mozilla.org keep 
a log file of such changes for exactly that reason.  

In this bug, you've done something good that often wasn't done in the past. 
You've made a record of the change that fixed the problem.  This makes it
possible to search the bug system for the file name in the future, and 
find the reason for the change.

I also suggest that when making changes to local copies of files, you use
RCS or CVS to record the changes and log the who/when/why information about
the change.  

Thanks again, Dave!