• BadShellCommandCWD

EricMountain writes, on September 4:

When executing a shell-command (e.g. pre-server etc.), dirvish does not make the current directory DIRVISH_DEST as specified in the dirvish.conf man page. (This may also be a problem on the client which should get DIRVISH_SRC as the current directory.)

This seems to be due to a small typo in the dirvish script. Here's a patch which appears to fix the problem:

 @@ -927,7 +931,7 @@
         ref($A{cmd}) and seppuku 232, "$A{lable} option specification error";

         $cmd = strftime($A{cmd}, localtime($A{now}));
 -       if ($A{dir} =~ /^:/)
 +       if ($A{dir} !~ /^:/)
         {
                 $rcmd = sprintf ("%s 'cd %s; %s %s' >>%s",
                         ("$A{shell}" || "/bin/sh -c"),

KeithLofstrom, on January 18, adds:

Way back in September, Eric Mountain posted the suggested BadShellCommandCWD patch to the wiki. This patch is designed to fix the problem with pre-client and post-client scripts being started in the wrong directory. Looking at the existing logic (starting with version 1.1), dirvish will only change the working directory to $A{dir} if it starts with a colon. Eric's fix reverses the logic, changing the working directory if it does NOT start with a colon.

Original:

        if ($A{dir} =~ /^:/)
        {
                ... with cwd, if colon ...
        } else {
                ... without cwd, if no colon ...
        }

Eric's fix:

        if ($A{dir} !~ /^:/)
        {
                ... with cwd, if no colon ...
        } else {
                ... without cwd, if colon ...
        }

It took me a while to figure out what JW intended: if you look at dirvish.conf, you will see:

 tree: path [alias] (S)
           Specify a directory path on the client to backup.

           If path is prefixed with a colon the transfer will be done  from
           an  rsync  daemon  on  the client otherwise the transfer will be
           done through a remote shell process.

So the colon is a flag to use rsync in daemon mode, which is called "rsyncd" by the clueful, and rsync --daemon by the rest of us. If you look at CHANGELOG, you will see the following comment about the time this behavior was added to the subroutine scriptrun :

  2002/08/08           TODO.html            1.18
                       dirvish              1.18
                       TODO.html            1.17
                       dirvish              1.17
                       dirvish-runall.8     1.2
  pre/post client scripts won't try to cd srctree if using
  rsyncd on client.

So it looks like this is a simple logical inversion error by JW; both behaviors of the if make sense given the man page description of the tree: option, if you reverse them.

One remaining question is, what if $A{dir} (which is now either $srctree or $destree) is empty? With Eric's fix, scriptrun would run the "with cwd" commands; which would be incorrect. However, dirvish never gets there; $srctree is set (more or less) by:

 ($srctree, $aliastree) = split(/\s+/, $$Options{tree})
        or seppuku 228, "ERROR: no source tree defined";

... so it does a seppuku if $$Options{tree}, and thus $srctree, is empty.

$destree is set by:

  $destree = join("/", $vault, $image, 'tree');

... so it is never empty, either. Thus we never encounter the "empty dir" problem. I am assuming that we never will, but this doesn't seem very robust - who knows what some almost-clueful programmer will do with scriptrun in the future?

Another question is, how do we signal rsync to use daemon mode? Well, this is a bit of kludge, but the normal (non-daemon) client path is specified as client:/path/to/source/files . If you instead call rsync with client::/path/to/source/files (two colons after the client name), you will turn on daemon behavior. So, if we join "client:" and ":/path/to/source/files", we get the appropriate source string. Either very clever or very confusing.

All that said, Eric's fix looks correct. It is interesting that the misbehavior wasn't noticed by any users until Eric spotted it. This may imply that nobody is using rsync daemon mode!

Forgive my delay in responding, but I want us to understand these patches completely before we fool around with the behavior of mission critical programs. Patches that come with explanations like the above will be consider much more quickly than patches without.

Keith

Add your comments or edit the above. Please include the date and your WikiName.