Re: [RFC][PATCH] Add zrestart()

[Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx>]

Marlon Richert wrote on Tue, Apr 27, 2021 at 14:37:30 +0300:
> On Mon, Apr 26, 2021 at 10:29 PM Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx> wrote:
> > > > > +  local err="$(zsh --interactive --monitor --zle -c '' 2>&1 > /dev/null)"
> > > >
> > > > Also, I think it's quite a stretch to describe this line as "_tests_
> > > > whether the shell is able to restart".
> > >
> >
> > Care to comment about the part of my answer before the "Also", which you
> > had snipped?
> 
> Sure:
> 
> On Mon, Apr 26, 2021 at 6:22 AM Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx> wrote:
> > > +  local err="$(zsh --interactive --monitor --zle -c '' 2>&1 > /dev/null)"
> >
> > This doesn't necessarily restart the _same_ zsh, if there's more than
> > one installed.
> 
> I think your observation is correct and I was planning to fix it in
> the next version of my patch. :)
> 
> When I don't reply to a point you make, it means that I either agree
> or think it's not worth arguing about, and I've put it on my TODO list
> for the next version of my patch. I was trying to be brief and not
> post a bunch of agrees/will-dos or quote unnecessarily. But if it's
> prefered that I do respond to every point I agree with and/or quote
> each part of every email I reply to, just let me know. :)

When you don't respond to a point, it's not possible for the reviewer to
distinguish the situation you describe from other situations —
especially when, as in this instance, the fix isn't obvious.  (proc(5)
isn't portable, argv[0] can lie, etc..)  So, it helps to be explicit.

But so long as we're on this level of detail, it's also fair to just
acknowledge once that all snipped points are agreed with and will be
implemented.

> > > I originally had `zsh -fn <all the dotfiles>`, along the lines of what
> > > was suggested earlier, but that test can fail on a valid dotfile that
> > > uses dynamically named dirs.
> >
> > A minimal example of this would not be out of place.
> 
> % zsh_directory_name() { [[ $2 == home ]] && reply=($HOME) }
> % cd ~[home]; print $?
> 0
> % print 'zsh_directory_name() { [[ $2 == home ]] && reply=($HOME) }
> cd ~[home]' > tmp
> % zsh -fn tmp
> tmp:2: no directory expansion: ~[home]
> %

Thanks.  Is this a bug or expected behaviour?

> > > Plus, if any dotfile sources other files,
> > > those files aren't checked this way at all. The approach above is the
> > > only one I've found so far that appears to be completely reliable in
> > > determining whether the shell can start up successfully.
> > >
> > > > This line executes a whole bunch of code you have no control over.
> > >
> > > It is, however, exactly the code we want to test here.
> >
> > I'm aware.  However, you aren't "testing" it, you are *running* it.
> >
> > First, that means the docs are wrong.
> >
> > Second, that code might do things that are inappropriate for the use-case
> > of "testing" the startup code.
> 
> When you execute `make check`, does it not _run_ the code?

It runs code that I wrote on data that I control in an isolated
environment that takes care not to modify anything else on the system
whereon the tests are run.

> How else are you going to test it? :)

The operative word here is "you".  Whoever wrote the code is best placed
to know how to test it.  You don't know what the user's zshrc does, so
how can you know whether it's safe to run it twice?

> > Or, in other words: the trick is to throw the bathwater out and
> > keep the baby.  Keeping *both* the baby and the bathwater isn't an
> > ideal solution.
> 
> What exactly here is the "bathwater"?

Side effects of the test.  (The baby is the test's soundness and
completeness.)

> Would it help to mock out certain features in the subshell that you
> wouldn't want to actually run in this test? Would it help to invoke
> the subshell with -o RESTRICTED?

That'd still let a zshrc run, say, «ssh foo lorem» or «date | tee -a
my-login-log | read -r REPLY» [jumping through some hoops to avoid
actual output redirections], which are both things that shouldn't be run
twice.

To be clear, these are just two examples.  They are not exhausive.