[Pkg-zsh-devel] Bug#969305: Bug#969305: Bug#969305: zsh: safe-rm needs to be added to the default path of interactive shells to work

[Daniel Shahaf]

Francois Marier wrote on Mon, 31 Aug 2020 12:04 -0700:
> On 2020-08-31 at 04:41:56, Daniel Shahaf wrote:
> > I guess it should be in the zprofile file, guarded by a [[ -o interactive ]] check.  
> 
> From the comment in /etc/zsh/zshrc:
> 
>   # This file is sourced only for interactive shells. It
>   # should contain commands to set up aliases, functions,
>   # options, key bindings, etc.
> 
> I thought it was a better fit, given that /etc/zsh/zprofile claims it's only
> for login shells:
> 
>   # This file is sourced only for login shells (i.e. shells
>   # invoked with "-" as the first character of argv[0], and
>   # shells invoked with the -l flag.)
> 
> Or maybe I got that wrong?
> 

You didn't get that wrong.  It's just that setting envvars is the job of
a login shell, not of shells spawned later on.  (That'd be analogous to
how safe-rm, as you said, drops code into /etc/profile.d/, rather than
into a (non-existent) /etc/bash.bashrc.d/.)  Thus, zprofile.

The [[ -o interactive ]] is to not apply to non-interactive login
shells.  (When do these happen, anyway?)

> > > However, I don't see a way for packages to do this. So I guess there would
> > > be two ways to make this possible:
> > > 
> > > 1. The main /etc/zsh/zshrc script could source all .sh files in a new
> > >    /etc/zsh/zshrc.d/ directory.
> > > 2. The /etc/zsh/zshrc that ships in Debian could include the above.
> > > 
> > > Are either of these something you'd be willing to consider?  
> > 
> > I don't have an opinion one way or the other.
> > 
> > I do note that option #2 would cause a stat(2) call during shell startup
> > for everyone who _doesn't_ use safe-rm.  Would that be a problem?
> > (E.g., slower shell startup)  
> 
> Good point about the extra stat. I honestly don't know whether that kind of
> impact can even be reliably measured, but you're right that it would
> be unnecessary for non-users of safe-rm.
> 
> > Probably not what you had in mind, but my first instinct here is to
> > look for a shell-independent solution.  For example:  
> 
> Indeed, that's an excellent approach and is in fact where I started.
> 

Ah, I didn't know the background here.  Thanks for the detailed exposition.

> > 3. Get the OS to add /usr/share/safe-rm/bin to $PATH before the user's
> > shell is executed in the first place.  (On FreeBSD that'd be
> > login.conf(5), but I don't know what the Linux equivalent is.)  
> 
> That appears to be in /etc/login.defs, but without any way for a package to
> configure.
> 

I see.

> > 4. Use dpkg-divert(8) to replace /bin/rm with a wrapper that calls
> > either safe-rm or the diverted rm binary, depending on whether it's
> > interactive or not.  
> 
> That was my first attempt and it turned out to be extremely risky and hard
> to get right:
> 
>   https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=489690

That bug basically says that when /bin/rm is replaced, the process of
replacement must be atomic.

I don't see how a requirement for atomicity is intractable.  On the
contrary, I think it's a textbook problem with textbook solutions.  For
example, one standard and portable answer is to use rename(2)'s
atomicity guarantees.  That's exactly what dash's maintainer scripts,
that the above bugs point to, do: see replace_with_link(), and note how
it places the $temp in the same directory as $dfile.

More generally, «rm /bin/rm && ln -s foo /bin/rm» is like «ls "$@"» and
«date | grep -w Tue»: it's wrong; it's not _obviously_ wrong; code
review will spot the mistake easily; once the correct pattern is
learned, it'll be gotten right every time, automatically.

All the above being said, I wouldn't have been surprised if getting this
approach right were complicated by something else — say, dpkg-divert
uses in _other_ packages that need to be coëxisted with.

> > If you don't already know them, see RM_STAR_SILENT and RM_STAR_WAIT in
> > zshoptions(1).  
> 
> Oh, that's very cool! A very good complement to safe-rm in fact since
> that's not something safe-rm can do anything about since the shell expands
> these globs before passing the paths to the rm command.

Well, safe-rm _could_ check whether the arguments are all in the same
directory, sorted, and comprise all non-dotfiles (or all files,
including dotfiles — see the GLOB_DOTS option) in that directory.
However, whether that'd be practical and worth the costs (e.g., some
false positives, particularly after a user pressed <Ctrl+X><Asterisk>
to manually expand the glob for review) is beyond the scope of this bug
report.

> > P.S.  Compare #489646, about providing a directory for packages to drop
> > completion files into.  (That didn't involve reimplementing
> > run-parts(1), though; it was just a matter of adding a directory to
> > a list of directories.)  
> 
> Yes, having a /usr/share/zsh/vendor-config/ or similar would be very good.
> It certainly doesn't have to live in /etc/.

I didn't intend to imply anything about which directory the files
should live in.  I was just referencing that in the context of
package-provided drop-in files.

Cheers,

Daniel