[boost] Filesystem Review

From: Jeff Garland (jeff_at_[hidden])
Date: 2002-09-21 13:09:36

First off, I'll say that I think this is a really important library. 
In addition, over the last few months Beman has done a excellent job
of methodically working through the issues, listening to suggestions,
and improving the library.  So I am a strong supporter of the filesystem
lib being included into Boost and look forward to future enhancements.  
My review of this library actually started months ago, and has included
writing several programs which I have tested out on Linux and under 
Cygwin with various gcc compilers.

Some overall comments:

* My only 'serious concern' is that we make sure the library the 
  library can expand to handle different types of 'file systems' 
  'simultaneously'.  That is, it should be perfectly possible
  to provide a path/directory iterator implementation for a
  .tar file.  I don't see why this couldn't be done by providing
  an implementation of the current interfaces in a tar_fs namespace.
  Perhaps there are better solutions? 

* Simple/more examples needed.  The examples directory contains the 
  new status tools, not small concise examples of usage.  This needs
  to be fixed.  In that regard I have attached a couple of simple 
  programs that use fs to implement trivial variants of 'ls', 'mv',
  and 'cp'.  Feel free to use them as examples.

Directory Iterator comments:

* I couldn't get the recursive_iterator working.  Without docs, its 
  not clear what the Predicate stuff is.  This needs to be documented 
  and should be part of the library.  Unless there is good reason
  recursive behavior should just be part of the  directory_iterator 
  constructor.  That is:
  
  fs::path p(...some path...);
  fs::directory_iterator(p, fs::recursive); //default is non-recursive

Path Comments

* I agree with John and others that the operator<< on path is an odd 
  interface.  Of the suggestions on the table, my preference is for 
  'append'. I prefer to avoid overloading mathematical operators for 
  non-numeric types.  It's just too disorienting for users.  It's 
  one thing to abuse operators in the context of a meta-programming 
  library or spirit where the entire library context is outside of 
  'traditional' C++ programming paradigms.  fs just doesn't fit 
  that mold.

* I found the path functions file_path and directory_path odd.  The
  fact that they are 'the same' on most systems made the interface 
  somewhat confusing.  I first assumed that directory_path would 
  extract the directory portion of the file path, which is of course 
  the job of branch.  While support of openVMS is a noble nod to the 
  past, it seems to me that the strange behavior of this particular 
  filesystem could be handled as a non-standard extension?

* Why does path::branch return a path instead of a string?  This seems a bit 
  uneven since all the other 'query' functions of path return a string.

* It is inconvenient that a trailing '/' is not allowed on directory path 
  construction of POSIX paths.  This does not seem like 'invalid path syntax'.  
  As an example, if I type 'ls /home/foo' or ls '/home/foo/' (assuming 
  /home/foo is a directory) the result is the listing of the directory 
  contents.  If /home/foo is a file then I get an error.

* The 'Representation Example' link in the path documentation is busted 
  everywhere.

* I haven't followed the make_absolute discussion in detail, but it seems 
  like this is headed in the right direction.

* I have this feeling that path could eventually become its own piece of 
  infrastructure that expands beyond fs as it is currently formulated.  
  That is, is there any reason why 'http://www.xyz.abc/foobar.html' isn't 
  a path?  I know we are focused on a narrower scope at the moment, but I 
  think eventually I should be able to write the following C++:
    
     path p("http://xyz.abc/foobar.html");
     std::fstream f(p);  //open a network connection and download
     ...

  
Jeff

//*************** move.cpp ******************//

#include "boost/filesystem/operations.hpp"
#include "boost/filesystem/path.hpp"
#include <iostream>

namespace fs = boost::filesystem;

int main(int argc, char** argv)
{
  if (argc < 3) {
    std::cout << "usage:   copy infile outfile" << std::endl;
    exit(1);
  }
  try {
    fs::path infile(argv[1]);
    if (!exists(infile)) {
      std::cout << "Input File \"" << infile.file_path() << "\" does not exist\n";
      return 1;
    }
    fs::path outfile(argv[2]);
    if (exists(outfile)) {
      std::cout << "Output File \"" << outfile.file_path() << "\" already exists\n";
      return 1;
    }
    fs::rename(infile, outfile);
  }
  catch(std::exception& e) {
    std::cout << "Error: " << e.what() << std::endl;
  }

  return 0;
}

//*************** copy.cpp ********************//

#include "boost/filesystem/operations.hpp"
#include "boost/filesystem/path.hpp"
#include <iostream>

namespace fs = boost::filesystem;

int main(int argc, char** argv)
{
  if (argc < 3) {
    std::cout << "usage:   copy infile outfile" << std::endl;
    exit(1);
  }
  try {
    fs::path infile(argv[1]);
    if (!exists(infile)) {
      std::cout << "Input File \"" << infile.file_path() << "\" does not exist\n";
      return 1;
    }
    fs::path outfile(argv[2]);
    if (exists(outfile)) {
      std::cout << "Output File \"" << outfile.file_path() << "\" already exists\n";
      return 1;
    }
    fs::copy_file(infile, outfile);
  }
  catch(std::exception& e) {
    std::cout << "Error: " << e.what() << std::endl;
  }

  return 0;
}

//*************** ls.cpp ********************//
#include "boost/filesystem/operations.hpp"
#include "boost/filesystem/path.hpp"
#include <iostream>

namespace fs = boost::filesystem;

int main(int argc, char** argv)
{
  if (argc < 2) {
    std::cout << "usage:   ls.exe path" << std::endl;
    exit(1);
  }
  try {
    unsigned int file_count = 0;
    unsigned int dir_count = 0;
    fs::path start_dir(argv[1], fs::system_specific);
    if (is_directory(start_dir)) {
      fs::directory_iterator end_iter;
      fs::directory_iterator dir_itr(start_dir);
      for (;dir_itr != end_iter; ++dir_itr) {
        std::cout << (dir_itr->file_path()) << std::endl;
        if (fs::is_directory(*dir_itr)) {
          dir_count++;
        }
        else {
          file_count++;
        }
      }
    }
    else {
      if (exists(start_dir)) {
        std::cout << start_dir.file_path() << std::endl;
        file_count++;
      }
      else {
        std::cout << "Invalid path: " << start_dir.file_path() << std::endl;
      }
    }
    
    std::cout << "\nNumber files      : " << file_count << std::endl;
    std::cout << "Number directories: " << dir_count << std::endl;

  }
  catch(std::exception& e) {
    std::cout << "Error: " << e.what() << std::endl;
  }
  return 0;
}