#4 spaces in filenames for nucmer

[Adam Phillippy]

See the below email. Also, the delta format stores the
file paths in the header, and assumes no spaces...

Hello,

I am using MUMmer 3.18. I tried running the command:

nucmer file1.fasta file2.fasta

and I got the following output:

1: PREPARING DATA

USAGE: /home/lupey/downloads/MUMmer3.18/aux_bin/prenuc
[options]
<reference>

Try '/home/lupey/downloads/MUMmer3.18/aux_bin/prenuc
-h' for more
information.
ERROR: prenuc returned non-zero, please file a bug report

After digging through the code, I realized that spaces
in absolute
filepaths are not handled correctly. For example, in
'nucmer', the
code:

$ref_file = File::Spec->rel2abs ($ARGV[0]);
$qry_file = File::Spec->rel2abs ($ARGV[1]);

properly creates an Absolute filepath but leaves spaces
unescaped.
Therefore, when this code:

$err[0] = $tigr->runCommand
("$prenuc_path $ref_file > $pfx.ntref");

is run, 'nucmer' bails and exits. The real culprit is
the system() call
in
runCommand (scripts/Foundation.pm). The system() call
uses one
argument therefore any spaces in filepaths is going to
screw things
up.

Please let me know if there is a fix for this.

Thank you,

Paul

[David Eccles (gringer)]

I came across this bug while running minimus2 (from the AMOS package). The quick fix would be to always quote file names in any runCommand calls. Here's a change for this particular bug:

--- nucmer 2011-05-05 17:53:12.030050609 +0200
+++ nucmer.new 2012-01-26 18:03:38.732287741 +0100
@@ -313,7 +313,7 @@
#-- Run prenuc and assert return value is zero
print (STDERR "1: PREPARING DATA\n");
$err[0] = $tigr->runCommand
- ("$prenuc_path $ref_file > $pfx.ntref");
+ ("$prenuc_path '$ref_file' > $pfx.ntref");

if ( $err[0] != 0 ) {
$tigr->bail

That deals with spaces, but you get problems if there are quotes in the file name.

A better way would be to require all command-line parameters to be specified in an array (a bit like the second form of Perl's system command [http://perldoc.perl.org/functions/system.html]), and change the definition in scripts/Foundation.pm to force this. This would expose all the runCommand calls, making manual changing to the array format much easier.

I've attached a patch to do this, but haven't actually tested the patch to make sure it doesn't break the module. Note that the patch *will* break other code. This is intentional, to force all the runCommand calls to put their arguments into a list.

[David Eccles (gringer)]

Hmm, okay, I seem to have trouble adding files. Here's the patch:

--- scripts/Foundation.pm 2004-07-20 19:48:58.000000000 +0200
+++ scripts/Foundation.pm.new 2012-01-26 18:19:43.999487396 +0100
@@ -285,11 +285,11 @@

=item $exit_code = $obj_instance->runCommand($command_str);

-This function passes the argument C<$command_str> to /bin/sh
+This function passes the program C<$command> and C<@command_parms> to /bin/sh
for processing. The return value is the exit code of the
-C<$command_str>. If the exit code is not defined, then either the signal or
+C<$command>. If the exit code is not defined, then either the signal or
core dump value of the execution is returned, whichever is applicable. Perl
-variables C<$?> and C<$!> are set accordingly. If C<$command_str> is not
+variables C<$?> and C<$!> are set accordingly. If C<$command> is not
defined, this function returns undefined. Log messages are recorded at log
level 4 to indicate the type of exit status and the corresponding code.
Invalid commands return -1.
@@ -299,7 +299,8 @@

sub runCommand($) {
my $self = shift;
- my $command_str = shift;
+ my $command = shift;
+ my @command_parms = @_;
my $exit_code = undef;
my $signal_num = undef;
my $dumped_core = undef;
@@ -335,10 +336,10 @@
$ENV{PATH} = join(":", @paths);
}

- if((defined ($command_str)) && ($command_str =~ /^(.*)$/)) {#taint
- #checking
- $command_str = $1;
- system($command_str);
+ if(defined ($command)) {#taint
+ #checking
+ $command_str = $command . join(" ",@command_parms);
+ system($command, @command_parms);
$exit_code = $? >> 8;
$signal_num = $? & 127;
$dumped_core = $? & 128;