Closed Bug 342121 Opened 14 years ago Closed 14 years ago

Remove usage of Config qw(:locations) in favor of Constants::bz_locations()

Categories

(Bugzilla :: Bugzilla-General, defect)

2.23
defect
Not set

Tracking

()

RESOLVED FIXED
Bugzilla 3.0

People

(Reporter: LpSolit, Assigned: mkanat)

References

Details

Attachments

(1 file, 2 obsolete files)

The code is actually duplicated in both Config.pm and Constants.pm (this was required in bug 304601 to avoid some dependency loops). Only bz_locations() should survive.
The patch in bug 282121 removed all usage of :locations in .cgi scripts. This patch only has to remove it in .pm modules. Should be trivial.
Assignee: general → LpSolit
Depends on: 282121
I'll do it. It's blocking my mod_perl work.
Assignee: LpSolit → mkanat
Blocks: mod_perl
Attached patch v1 (obsolete) — Splinter Review
I fixed all the modules. I also had to fix checksetup.

I also noticed that there were still two versioncache unlinks in checksetup.pl, which we didn't need. So I removed those while I was there.

Perhaps we could just make checksetup remove versioncache once, if it exists.
Attachment #227081 - Flags: review?(LpSolit)
Attached patch v1.1 (obsolete) — Splinter Review
I had missed one "use Bugzilla::Constants" in Bugzilla::Template::Plugin::Hook. Fixed now.
Attachment #227081 - Attachment is obsolete: true
Attachment #227082 - Flags: review?(LpSolit)
Attachment #227081 - Flags: review?(LpSolit)
(In reply to comment #3)
> I also noticed that there were still two versioncache unlinks in checksetup.pl,
> which we didn't need. So I removed those while I was there.

Bad idea! I kept them on purpose. They are required to remove data/versioncache if this file still exists. Both are not required, but at least one of them should remain.
Status: NEW → ASSIGNED
Comment on attachment 227082 [details] [diff] [review]
v1.1

>Index: checksetup.pl

>+my $localconfig = bz_locations()->{'localconfig'};
> do $localconfig;

>-          "file '$localconfig' and rerun checksetup.pl\n\n",
>+          "file " . bz_locations()->{'localconfig'} ." and rerun ",
>+          "checksetup.pl\n\n",


>+            my $localconfig = bz_locations()->{'localconfig'};
>             die <<"EOF"

Why redefining $localconfig again and again, or replacing it by bz_locations()->{'localconfig'} everywhere as you already defined it above? We are not in a subroutine.


>@@ -4777,8 +4781,6 @@
>             "SET initialowner = $adminuid " .
>           "WHERE initialowner = 0");
> 
>-unlink "$datadir/versioncache";

Do not remove this line. r=LpSolit *only* if you leave this line alone.
Attachment #227082 - Flags: review?(LpSolit) → review+
Flags: approval?
(In reply to comment #6)
> Why redefining $localconfig again and again, or replacing it by
> bz_locations()->{'localconfig'} everywhere as you already defined it above? We
> are not in a subroutine.

  One's inside of the BEGIN block, and one's not.

> >-unlink "$datadir/versioncache";
> 
> Do not remove this line. r=LpSolit *only* if you leave this line alone.

  Okay, fair enough.
Flags: approval? → approval+
Okay, on checkin I added the versioncache removal into the time-based changes. I also had checksetup print out a message that it was removing it. I'll attach the final patch.

Checking in checksetup.pl;
/cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v  <--  checksetup.pl
new revision: 1.498; previous revision: 1.497
done
Checking in Bugzilla/Attachment.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Attachment.pm,v  <--  Attachment.pm
new revision: 1.35; previous revision: 1.34
done
Checking in Bugzilla/BugMail.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/BugMail.pm,v  <--  BugMail.pm
new revision: 1.81; previous revision: 1.80
done
Checking in Bugzilla/Config.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Config.pm,v  <--  Config.pm
new revision: 1.60; previous revision: 1.59
done
Checking in Bugzilla/Hook.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Hook.pm,v  <--  Hook.pm
new revision: 1.4; previous revision: 1.3
done
Checking in Bugzilla/Mailer.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Mailer.pm,v  <--  Mailer.pm
new revision: 1.2; previous revision: 1.1
done
Checking in Bugzilla/Template.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Template.pm,v  <--  Template.pm
new revision: 1.51; previous revision: 1.50
done
Checking in Bugzilla/Update.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Update.pm,v  <--  Update.pm
new revision: 1.2; previous revision: 1.1
done
Checking in Bugzilla/Template/Plugin/Hook.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Template/Plugin/Hook.pm,v  <--  Hook.pm
new revision: 1.5; previous revision: 1.4
done
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Okay, here's the version that got checked-in.
Attachment #227082 - Attachment is obsolete: true
Keywords: relnote
Added to the release notes on bug 255155.
Keywords: relnote
The correct bug number for those release notes is actually bug 349423.
You need to log in before you can comment on or make changes to this bug.