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': '/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:

        :%s/^Everything/lib\/&/g

Ugh, I remember this Makefile.PL. I think somewhere I have a Build.PL version. Regardless, there’s only one line to change, and that’s the VERSION_FROM line.

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 svk 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 Everything::Auth and 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 Module::Build subclass called Everything::Build, so I need that file too.

The Build.PL file needed two edits to reflect the new paths of Everything.pm and 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.

How about perltidy? That should be quick, with the proper command-line:

        $ 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 perltidy 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 perltidy command.

  $ 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.