It turns out that `string split0` didn't actually ever do any
splitting. The arg_iterator_t already split stdin on NUL, and split0 just
performed an additional search that could never succeed (since
arguments from argv already can't contain NUL).
Let the arg_iterator_t not perform any splitting if asked, and then
let split0 split in 0.
One slight wart is that split0 ignores a trailing NUL, which normal
split doesn't.
Fixes#5701.
In a galaxy far, far away, event_blockage_t was intended to block only cetain
events. But it always just blocked everything. Eliminate the event block
mask.
On some systems, this sometimes uses unicode quotation marks.
Not on mine, but on Travis it does.
The only other workaround I can think of is setting locale to C, but
that implies not being able to test anything unicode-related in the
entire invocation tests.
So for now disable this test.
Illumos/OpenIndiana/SunOS/Solaris has an rm/rmdir that tries to
protect the user by not allowing them to delete $PWD.
Normally, this would be a good thing as deleting $PWD is a stupid
thing to do. Except in this case, we absolutely need to do that.
So instead we weasel around it by invoking an sh to cd out of the
directory to then invoke an `rmdir` to delete it. That should throw
off any attempts at protection (we could also have tried $PWD/. or
similar, but that's possibly still protected against).
This is the last failing test on
Illumos/OpenIndiana/SunOS/Solaris/afunnyquip, so:
Fixes#5472.
This tested #1728, where redirecting a directory (`begin; something;
end < .`) would cause `status` to misbehave.
Unfortunately, on Illumos/OpenIndiana/SunOS, this returns a different
error (EINVAL instead of EISDIR), so we can't check that with our test harness, because
we can't redirect it.
Since it's not important that this gives the same error across
systems (and indeed we provide no way of intercepting the error!),
use an invocation test instead, because that allows different output per-uname.
See #5472.
300ms was waaay too long, and even 100ms wasn't necessary.
Emacs' evil mode uses 10ms (0.01s), so let's stay a tad higher in case
some terminals are slow.
If anyone really wants to be able to type alt+h with escape, let them
raise the timeout.
Fixes#3904.
This is a large change to how io_buffers are filled. The essential problem
comes about with code like (example):
echo ( /bin/pwd )
The output of /bin/pwd must go to fish, not the tty. To arrange for this,
fish does the following:
1. Invoke pipe() to create a pipe.
2. Add an io_bufferfill_t redirection that owns the write end of the pipe.
3. After fork (or equiv), call dup2() to replace pwd's stdout with this pipe.
Now when /bin/pwd writes, it will send output to the read end of the pipe.
But who reads it?
Prior to this fix, fish would do the following in a loop:
1. select() on the pipe with a 10 msec timeout
2. waitpid(WNOHANG) on the pwd proc
This polling is ugly and confusing and is what is replaced here.
With this new change, fish now reads from the pipe via a background thread:
1. Spawn a background pthread, which select()s on the pipe's read end with
a long (100 msec) timeout.
2. In the foreground, waitpid() (allowing hanging) on the pwd proc.
The big win here is a major simplification of job_t::continue_job() since
it no longer has to worry about filling buffers. This will make things
easier for concurrent execution.
It may not be obvious why the background thread still needs a poll (100 msec).
The answer is for cases where the write end of the fd escapes, in particular
background processes invoked inside command substitutions. psub is perhaps
the only important case of this (other shells typically just hang here).
Using printf like
printf "The message"
is unsafe, because if the message contains any formatting characters,
they'll be interpreted.
In this case it's not all that important because the message contains
only filenames of our tests and static strings, but still.
A while loop now evaluates to the last executed command in the body, or
zero if the loop body is empty. This matches POSIX semantics.
Add a bunch of tricky tests.
See #4982
For some reason, we have two places where a variable can be read-only:
- By key in env.cpp:is_read_only(), which is checked via set*
- By flag on the actual env_var_t, which is checked e.g. in
parse_execution
The latter didn't happen for non-electric variables like hostname,
because they used the default constructor, because they were
constructed via operator[] (or some such C++-iness).
This caused for-loops to crash on an assert if they used a
non-electric read-only var like $hostname or $SHLVL.
Instead, we explicitly set the flag.
We might want to remove one of the two read-only checks, or something?
Fixes#5548.
There's just waaayy too many things that could go wrong with it, so it
annoys more than it helps, especially since we don't get any
indication what failed.
E.g. on FreeBSD, the test failed without a usable message just because
`tput` couldn't find an attribute (so colors were unset).
The one thing I was missing:
`echo -n` isn't POSIX. In practice, it appears the only shell to encounter this
is macOS' crusty old bash in sh-mode. Just replace it with `touch`.
This reverts commit fc5e8f9fec.
This makes the script worse, but it's good enough.
The required changes are:
- `shopt -s nullglob`, which we simply don't use (we have one glob, but that's
guaranteed to match because we ship the files)
- One array, which we replace with a direct use of the glob (plus it
used `echo` again?)
- The `function` word, which I'm still annoyed is even a thing!
- Variable indirection (`color=${!color_var}` - instead we pass the
value directly - which makes the script uglier!)
- One array, which we replace with a function
- A use of `type -t`, replaced with `command -v`
- A use of `${var:begin:end}` substring expansion, replaced with trickery.
- `set -o pipefail` is replaced with a function
Note that checkbashisms still complains about `command -v`, because
we're not using it with "-p". But we _want_ to check the current
$PATH, and `command -v` is POSIX.
This still uses `local`, which technically isn't in POSIX.
The tests now appear to pass in:
- bash
- dash
- zsh
- mksh
- busybox
Rather than killing the process with close, read EOF after sending the
"exit" command and wait for OS cleanup (per the expect examples).
Not cleaning up with wait caused expect to crash on all 32-bit platforms
including i586 and armv7l with "alloc: invalid block: 0xbf993ccb: 3d 3b".
64-bit platforms were not affected, for reasons that are not clear.
Turns out busybox diff (used on alpine) defaults to unified output,
which we can't use because that prints filenames, and those are
tempfiles made by psub.
Instead, we use builtins to print the first line and compare the others.
This isn't all that important, and it breaks on musl just because the message is different.
Just skip it for now, until we figure out how to better test this.
This `set TERM`. Which, if $TERM is inherited, is already exported,
but not if it isn't.
This is the case on sr.ht's arch images, so we failed without a TERM variable.
