This 30-day project explores the refactoring of a legacy system. The Everything Engine is an aging software project that powers Perl Monks, Everything 2, and a few other websites. It suffers from poor design and maintainiability. Learn what it’s like to look over the shoulder of an experienced developer as he refactors, redesigns, and updates the code.
Cleaning Up the Repository
First, check out the latest Subversion repository. I use Svk, so I need to mirror it.
$ svk depotmap
Then I added /ebase as a separate repository. Why? Under the repository there are separate directories for separate projects. I want to treat ebase as its own project.
Next I set up a Svk mirror of the public repository:
$ svk mirror https://svn.sourceforge.net/svnroot/everydevel/trunk/ebase \ //ebase/trunk
Now I can make my own branches with Svk if I want. Now synchronize the mirror. I tried:
$ svk sync -s HEAD //ebase/trunk
but that gave errors (
REPORT request failed on
'/svnroot/everydevel/!svn/bc/1/trunk/ebase' path not found.)
It turns out that that was an error in the Subversion bindings. That’s okay, I grabbed everything.
$ svk sync //ebase/trunk
That took a while to grab 822 revisions. Oh well.
$ cd ~/dev $ svk co //ebase/trunk ebase
Now there’s a Svk repository where I can make changes. I could make my own branch here if I wanted, so I could use my own version control for small changes and only check them in to the public repository when absolutely necessary. Maybe I’ll do that… it’s really easy with Svk.
That gives me everything in a nice directory without messy .svn or .cvs directories. Excellent.
The first thing that has bothered me about Everything for years is that the project stores modules in the top-level directory. Why not move all of the .pm files beneath lib/?
$ svk mkdir lib $ svk mv Everything.pm lib/ $ svk mv Everything/ lib/
That’s pretty good. Now the MANIFEST is wrong too, and probably the Makefile.PL. Oh well.
Within MANIFEST in Vim:
Ugh, I remember this Makefile.PL. I think somewhere I have a
Build.PL version. Regardless, there’s only one line to change, and
I have the habit of running
svk diff | less to see what
I’ve changed before a checkin, but diff doesn’t really know about moving
files, so the diff is huge. Ouch.
At checkin, a pre-commit hook fails, warning that there’s no MIME type set on lib/Everything.pm. Oops.
$ svk propset svn:mime-type text/plain lib/Everything.pm $ svk propset svn:eol-style native lib/Everything.pm
That’s all it took; the commit succeeded as revision 823. Hooray!
I noticed that the list of changed files in the Svk commit log listed
the deletions on their own, but not the additions of everything under
lib/Everything/. That worries me, though I’ve never seen
mv fail. I’m paranoid enough to modify one of those files and then
run a diff to see if it’s under version control.
Updating the copyright notice in
running a diff shows that Svk knows about it at least. That’s good.
$ svk revert lib/Everything/Auth.pm
What’s new? I could go looking for that Build.PL file… that
might be a good idea; I don’t want to maintain the Makefile.PL file,
not least of all that I hate doing anything with
ExtUtils::MakeMaker besides setting a few flags.
$ locate Build.PL | grep ebase /home/chromatic/dev/everything/ebase/Build.PL /home/chromatic/dev/everything/fakeebase/Build.PL /home/chromatic/dev/evpreload/ebase/Build.PL /home/chromatic/dev/repos/ebase/Build.PL
Hm, I have a few of my own branches anyway. These all use the CVS version
though, so I hate to think of how to merge the code. I think the preload branch
is the latest, so I’ll copy over that Build.PL file. It uses a
Everything::Build, so I need that file too.
The Build.PL file needed two edits to reflect the new paths of
Everything::Build under lib/.
Otherwise, everything’s fine.
$ svk add Build.PL lib/Everything/Build.pm
I bet they need MIME type changes too. I’ll find out when I go to check them in…. Yep. That’s a pain, but it’s probably good to have that check on the repository. I can add a shell script in scripts/ to perform the property setting… or steal the one Pugs uses. (I know it’s there because I fixed it to work with Svk or Subversion.)
$ sh scripts/add_text_file.sh scripts/add_text_file.sh Build.PL \ lib/Everything/Build.pm
The script adds the file and sets the appropriate Svn properties. That’s good enough to get the code to check in. That’s revision #824. Everything’s happy.
What’s the next step? Running the tests, perhaps, to see what’s broken.
Another option is to run
perltidy on all of the modules –
there’s some seriously grotty code in there from the last millennium and
not having to read badly-indented and badly patched (hey, I used to work on
this too, remember!) code might be nice.
Of the test files, two fail (one only two tests and one spectacularly).
The test files need some reorganization too, as well as updating. This is
the project for which I wrote
Test::MockObject… I just
didn’t go back and port all of the tests to use it again. Oops.
perltidy? That should be quick, with the proper
$ for i in $(find lib -name *.pm); do perltidy $i; done
Oh right, I always forget to add
echo $i in there to see
the status. Oh well. I also forgot to use the correct
flags, as I don’t have a ~/.perltidyrc file. Oops. That’s okay; this
didn’t overwrite my code (and it’s in version control anyway). This is a
good opportunity to pull my
perltidy commands out of my
~/.vimrc file and create ~/.perltidyrc:
-ci=4 -et=4 -bl
Easy enough. Now how do I modify the files in place? Ahh, the
-b flag. Now to erase the .tdy files and re-run the
$ find lib -name *.tdy | xargs rm $ for i in $(find lib -name *.pm); do echo $i; perltidy -b $i; done
Hm, but I don’t really need the .bak files. That’s okay:
$ find lib -name *.bak | xargs rm
I know someone will come along and tell me how much more efficient I
could make that command, but it finishes before I even start to pull up the
xargs man page, so I don’t care at the moment.
It’s worth running the tests again to see if there’s any further damage.
There isn’t. Nifty. There’s another checkin.
svk status shows
only modified files and where I expect them, so here… we… go!
$ svk commit
The ease of manipulating all of this code is just one reason I wanted to move everything under lib/. I just don’t have to think about where the files are anymore. That’s revision #825 and about an hour since I started. That’s a lot of cleanup, but it’s something that’s been on my mind for quite a while. It’ll be easier to tackle new code next time.