The smallest bugs are the nastiest

Preparing the release of a fixed demo revision for my Wesnoth add-on After the Storm, which is the immediate sequel to Invasion from the Unknown, I ran into a rather nasty bug in one of the scripts I made for easy creation of .tar.bz2 packages for the Wesnoth-UMC-Dev Project, build-external-archive.sh.

The script in question is one of my earliest experiments with bash scripting, and as such suffers from the quirks of a novice programmer trying to get work done fast. It's some sort of crapload of hacks that just ‘‘works’’, somehow. I have an odd habit of tempting fate and using my earliest programming experiments in production environments — and this includes Shikadibot and Soradoc, which is the PHP-based layout code generator used both here and at the Wesnoth-UMC-Dev website.

The only reasons for not killing it are the following features offered to the selected few who know how to use it:

  • It reliably produces a .tar.bz2 archive for add-on distributions on regular websites — including SF.net — provided a few simple arguments.
  • It roughly* conforms to the Wesnoth .ign file syntax, although it doesn't provide the same defaults as the game engine.
  • It knows how to cleanly handle SVN check-outs.
  • There's built-in, automatic support for XDelta patch generation, provided the xdelta tool is installed and in PATH.
  • Provides MD5 and SHA1 digests of produced packages and writes them to .MD5 and .SHA1 files for distribution.
  • No suitable replacement has been provided yet.

(* Espreon pointed out on the #wesnoth-umc-dev channel at chat.freenode.net, that `build-external-archive.sh` may match .ign patterns only in the add-on's root directory, and not in its subdirectories. This, of course, a bug.)

build-external-archive.sh has got a few inoffensive bugs in the past, perhaps the greatest of which was a set of dependencies upon Linux distribution-specific conventions (why, hello there, openSUSE!). However, yesterday I hit the nastiest, yet unseen bug.

Since I usually use actual SVN tag check-outs for creating the packages, or get someone else to do it for me, I had not checked the interactions of this script with an input directory that is actually a symbolic link pointing to a directory containing files or paths matched by a _server.ign file. This tool needs to copy the input directory to a separate temporary directory in order to physically remove the files (actually copies) that match the aforementioned .ign patterns, and then proceed to run tar to wrap the modified temp directory into a nifty bzip2 tarball.

And how was it copying the input directory to /tmp when the input is not a SVN check-out? Without going into much detail, it is similar to this:

cp -a ${input_directory} /tmp/${output_file.fakeroot}.dir/${addon_name}

The -a switch to GNU coreutils' cp is equivalent to -dpR, which roughly means ‘‘be recursive, preserve timestamps, access masks and never dereference symlinks and preserve them!’’ So you can start to imagine why this would become a huge problem when the $input_directory is a symlink.

Such a small overlooked mistake ended up destroying the git-svn tree contained within my add-on's directory, which was indeed specified as a symlink in ~/.wesnoth-1.7/data/add-ons/After_the_Storm pointing to ~/src/wesnoth-umc-dev/git/trunk/After_the_Storm. The symlink itself was copied to the temporary directory and the removal of unwanted files took place on the original location instead of a temporary tree. This gave me that disturbing feeling of having a bucket with cold water dropped on my head whenever I accidentally remove files I wasn't supposed to remove. Thankfully, I had committed all my changes to the upstream SVN repository and only had to regenerate a git-svn tree to continue working after making the release using a fallback SVN check-out.

The correct pseudo-code, forcing symlinks to be dereferenced and timestamps, file modes and ownerships to be preserved, is actually:

cp -LRp ${input_directory} /tmp/${output_file.fakeroot}.dir/${addon_name}

As you can see, it's perfectly easy to screw up your own files with a technically small mistake in a command line. However, in this case some idiot (me!) didn't read the manual correctly and didn't notice that the -a switch had additional effects, and that's a big logical mistake. Fortunately, only Espreon, AI0867 and I rely on this script, as far as I know.

This tool is going to be eventually replaced by umcdist (codenamed Blackwater), which is written in manageable Perl, provides the same functionality plus archive signing with OpenPGP, and would be done in a few days if I actually concentrated on it — don't take me wrong, it exists and there's code written for it, but it still needs to be completed. Whereas build-external-archive.sh is a deliberately undocumented IftU-specific tool that was later inherited by the wesnoth-umc-dev project, umcdist is targeted to the project as a whole, it will have proper online and built-in documentation and it'll be readable by more than one person to allow easier bug-fixing and maintenance.

Software, Wesnoth, Wesnoth-UMC-Dev