Perl File Write Issue

Question

I'm having a really weird problem with this perl script. The basic point is that sometimes a file write/append doesn't happen. On a run of the program, either all of the writes will happen or none of them will. Here is the subroutine, with some comments:

sub process_svs {
  my $F;
  open($F, '<', $_[0]);
  if($log_dups==1) {
    open($dfh, '>>',"./duplicates.txt");
  }
  while (my $line = <$F>) {
    chomp $line;
    if($line =~ /somepattern/) {
      if (! -e "somefile") {
        copy("source","dest") or warn ("couldn't copy");
      } elsif($log_dups==1) {
        system("touch ./duplicates.txt"); # ghetto workaround
        print $dfh "./".$_[0]."_files/".$1.",v already exists\n" or die("Couldn't write duplicate"); # problem line
      }
    }
  }
  close $F;
}

The print statements to stdout always work, but if I remove the touch ./duplicates.txt crap, nothing is written to duplicates.txt.

The other "weird" thing, is that earlier in the program, I create a directory with perl mkdir, and if the directory exists when the program is run, I don't need the workaround, the duplicates.txt writing works just fine. If I delete the directory, and let the program mkdir it, it doesn't work. Seems relevant, but I can't figure out how since the directory and the text file are not in the same location, or related in any way, that I can think of.

Additionally, I have run it through the debugger, and can see the write call being executed, but inspecting duplicates.txt immediately after the write shows nothing written.

Any possible reasons for this would be greatly appreciated.

If you want to see a modified, but more complete, version of the script, it is here:

use strict;
use warnings;
use File::Copy;
my $svs = $ARGV[0];
my $rhis_str = system("rhis $svs > ./tmp_history");
my $fh;
my $dfh;
my @versions;
my $all_revs = 0;
my $current_rev = "";
my $log_dups = 0;
sub process_svs {
  my $F;
  open($F, '<', $_[0]);
  if($log_dups==1) {
    open($dfh, '>>',"./duplicates.txt");
  }
  while (my $line = <$F>) {
    chomp $line;
    if($line =~ /something/) {
      if (! -e "something") {
        copy("source","dest") or warn ("couldn't copy ");
      } elsif($log_dups==1) {
        system("touch ./duplicates.txt"); # ghetto workaround
        print $dfh "something already exists\n" or die("Couldn't write duplicate");
      }
    }
  }
  close $F;
}
for(my $i = 0; $i <= scalar @ARGV; $i++) {
  my $arg = $ARGV[$i];
  if($arg eq "-a") {
    $all_revs = 1;
  } elsif($arg eq "-r") {
    $all_revs = 0;
    $current_rev = $ARGV[$i+1];
  } elsif($arg eq "--log-dups") {
    $log_dups = 1;
  }
}
open($fh, '<','./tmp_history') or die(">>> Failed to open ./tmp_history");;
mkdir "./".$svs."_files";
if($all_revs == 1) {
  print ">>> Processing all revisions of ".$svs;
  if($log_dups==1) {
    print" (and logging duplicates)\n";
  }
  while(my $line = <$fh>) {
    chomp $line;
    if ($line =~ /something/) {
      push @versions, $1;
    }
  }
}
  system("some_cmd &>/dev/null");
  process_svs($svs);
}

Answer 1

You're not checking to see if your files opened. This is a very, very basic mistake and you should fix this immediately. Either add or die $! after each open or, better yet, use autodie and it will take care of catching all IO exceptions for you and give you good, consistent error messages.

Most importantly, this will tell you why it failed to open. $! tells you why it failed. You don't have that in your check on print.

    print $dfh "./".$_[0]."_files/".$1.",v already exists\n" or die("Couldn't write duplicate"); # problem line

You're checking if print failed, but you're not including $!. Either add $! like die "Couldn't write to duplicate: $!" or use autodie, remove the or die clause, and let autodie take care of it. I recommend the second.

I suspect you'll find that something else is deleting duplicates.txt between the open and the print.

---

The second thing that grabs my attention is here.

if($log_dups==1) {
  open($dfh, '>>',"./duplicates.txt");
}

You're using a global variable $log_dups to decide whether or not to open the file for writing (and not checking if it succeeded). This should be a variable that gets passed into the function, it's just good programming practice. Later you decide whether to print to $dfh based on that global variable.

  if (! -e "something") {
    copy("source","dest") or warn ("couldn't copy ");
  } elsif($log_dups==1) {
    system("touch ./duplicates.txt"); # ghetto workaround
    print $dfh "something already exists\n" or die("Couldn't write duplicate");
  }

Because $log_dups is global it's possible something else is changing $log_dups between deciding to open duplicates.txt and writing to it. To avoid all these problems, and to make the code simpler, $log_dups should be an argument passed into the function.

---

Furthermore, the filehandle $dfh is inexplicably a global. Same problem, something else could be closing it. It will also not be automatically closed at the end of the function which might leave writes to duplicates.txt buffered until the program exits. $dfh should be a lexical.

---

Other problems...

my $rhis_str = system("rhis $svs > ./tmp_history");

$rhis_str will contain the exit status of the rhis program. I don't think that's what you want. You don't use this variable anyway.

---

There's no need to pass ./file to open, it's safe and easier to read to use just pass file. That it's in the current working directory is implied.

---

If you fix these basic problems and still have trouble, then edit your question with the revised code and we can look again.