Re: Useless -r argument in install-sh

[Ralf Wildenhues]

Hello Julien,

thanks for the report.  I'm adding the bug-automake list, as Automake is
the upstream source of the install-sh script.  For better understanding
of list readers, this discussion is about this version of the install-sh
script:
<http://git.savannah.gnu.org/cgit/automake.git/tree/lib/install-sh>

* Julien ÉLIE wrote on Mon, May 18, 2009 at 11:35:46PM CEST:
> In the latest libtool release (2.2.6a), there is an unnecessary
> "-f" argument to $rmcmd here:
>
>   rmcmd="$rmprog -f"

Hmm.  In general, this code is pretty tricky and I'm rather reluctant to
do lot of "clean-up" changes if we don't find actual problems with the
code.

I see that the -f is duplicate, and do not know whether there is a
reason for this.  However, let me note that the duplication is harmless.

> Indeed, the code does:
>
>   $doit $rmcmd -f "$dst" 2>/dev/null ||
>   { $doit $mvcmd -f "$dst" "$rmtmp" 2>/dev/null &&
>     { $doit $rmcmd -f "$rmtmp" 2>/dev/null; :; }
>   }
>
> Incidentally, is ":;" really useful in the last line?

Yes.  Allow me to quote a bigger chunk of the code:

[ if not $copy_on_change or something else went wrong then ... ]

      # Rename the file to the real destination.
      $doit $mvcmd -f "$dsttmp" "$dst" 2>/dev/null ||

      # The rename failed, perhaps because mv can't rename something else
      # to itself, or perhaps because mv is so ancient that it does not
      # support -f.
      {
        # Now remove or move aside any old file at destination location.
        # We try this two ways since rm can't unlink itself on some
        # systems and the destination file might be busy for other
        # reasons.  In this case, the final cleanup might fail but the new
        # file should still install successfully.
        {
          test ! -f "$dst" ||
          $doit $rmcmd -f "$dst" 2>/dev/null ||
          { $doit $mvcmd -f "$dst" "$rmtmp" 2>/dev/null &&
            { $doit $rmcmd -f "$rmtmp" 2>/dev/null; :; }
          } ||
          { echo "$0: cannot unlink or rename $dst" >&2
            (exit 1); exit 1
          }
        } &&

        # Now rename the file to the real destination.
        $doit $mvcmd "$dsttmp" "$dst"
      }


Now, the ':;' part exists so that the '||' part after that is not
entered even if the rmcmd of the $rmtmp file causes any errors.
As far as I can see, that is exactly what would be desired in this case.

> There is also a direct call to "rm" here:
>
>   rm -f "$dsttmp"

Two, actually; there is another one in the trap.

> I think $doit $rmcmd might be used instead.

Hmm, this indeed looks inconsistent.  $dsttmp is consistently removed
with plain rm, and $dst consistenly with $rmcmd, but $rmtmp isn't.
Unfortunately, there is no documentation about either of the variables,
so I'm still not quite sure what the expected semantics would be: either
let all temporary files be removed in any case, or let all removals be
done with $rmcmd.

Another possibility that seems plausible is that $rmcmd was introduced
only for the case of when plain rm would not work here: say, if we are
about to install a shared library which the rm program depends upon.

Cheers,
Ralf