Apparently in zsh (when in sh compatibility mode), IFS=/ will split
"/a/b/c/" into 5 parts ("", "a", "b", "c", ""). Other shells all seem
to agree that it's 4 parts ("", "a", "b", "c"). zsh seems maybe more
correct to me, but the majority rules, so we'll warn on it.
Meanwhile, we'll also fix the one place in minimal/do that failed due
to this oddity, since it's relatively easy to avoid.
Reported-by: shamrin@gmail.com
As named, the file would be extracted by git on macOS, then
(un)helpfully normalized by the macOS filesystem. After that,
"git clean -fdx" would delete the file, since it no longer
had the expected name, so git thought it wasn't part of its repo.
I considered pre-normalizing the filename, but a) that would break
on any future OS that normalizes differently; and b) that means we
won't test denormalized filenames. Instead, we'll remove the directory
from git, and create it from sh instead, then figure out what name
it got really created as, and then pass the "real" name to redo.
Apparently on some (probably buggy) platforms, when CDPATH is set, the
'cd' command will always print text to stdout. This caused fail 122
and 123 in shelltest.od.
CDPATH is an interactive-mode option, so let's clear it when running
scripts.
I must have changed this at the last minute when adding it, but I don't
know why. We were redefining f4() inside a subshell, so it never
applied to the parent shell, so it was always considered a failure.
But that's not what we were supposed to be testing.
This is just supposed to be a test of the really rare syntax of
defining a function without enclosing braces, which only fails on posh,
as far as I've seen.
At some point this got broken during a refactoring. The result was
that redo-ifchange, run from the command line (as opposed to inside a
.do script) would fail to start the log prettifier.
This only happened if the containing project was buggy, ie. you tried
to build a target that has no .do file available anywhere. However, it
resulted in a confusing outcome for that case, where we'd run the wrong
default.do file with the wrong parameters.
Extended an existing test to catch this mistake.
This is relatively harmless, since we treat them *almost* identically,
except that we print a warning for overridden files to remind you that
something fishy is going on.
Add a test for the actual warning message to ensure it is printed. (I
don't like tests for specific warning messages, but it was necessary in
this case.)
If we failed because:
- target dir doesn't exist
- failed to copy from stdout
- failed to rename $3
We would correctly return error 209, but the target would still be
marked as having been built, so redo-ifchange would not try to build it
next time, beause we forgot to call sf.set_failed() in those cases.
minimal/do worked correctly.
Added a test to catch this in the future.
Silently recover if REDO_CHEATFDS file descriptors are closed, because
they aren't completely essential and MAKEFLAGS-related warnings already
get printed if all file descriptors have been closed.
If MAKEFLAGS --jobserver-auth flags are closed, improve the error
message so that a) it's a normal error instead of an exception and b)
we link to documentation about why it happens. Also write some more
detailed documentation about what's going on here.
WSL (Windows Services for Linux) provides a Linux-kernel-compatible ABI
for userspace processes, but the current version doesn't not implement
fcntl() locks at all; it just always returns success. See
https://github.com/Microsoft/WSL/issues/1927.
This causes us three kinds of problem:
1. sqlite3 in WAL mode gives "OperationalError: locking protocol".
1b. Other sqlite3 journal modes also don't work when used by
multiple processes.
2. redo parallelism doesn't work, because we can't prevent the same
target from being build several times simultaneously.
3. "redo-log -f" doesn't work, since it can't tell whether the log
file it's tailing is "done" or not.
To fix#1, we switch the sqlite3 journal back to PERSIST instead of
WAL. We originally changed to WAL in commit 5156feae9d to reduce
deadlocks on MacOS. That was never adequately explained, but PERSIST
still acts weird on MacOS, so we'll only switch to PERSIST when we
detect that locking is definitely broken. Sigh.
To (mostly) fix#2, we disable any -j value > 1 when locking is broken.
This prevents basic forms of parallelism, but doesn't stop you from
re-entrantly starting other instances of redo. To fix that properly,
we need to switch to a different locking mechanism entirely, which is
tough in python. flock() locks probably work, for example, but
python's locks lie and just use fcntl locks for those.
To fix#3, we always force --no-log mode when we find that locking is
broken.
On MacOS (at least 10.11.6), /bin/pwd defaults to using $PWD (ie. pwd
-L). On most other OSes it defaults to *not* using $PWD (ie. pwd -P).
We need the latter behaviour. It appears that 'pwd -P' has been
specified by POSIX for quite a few years now, so let's rely on it.
shelltest.od will now also check for it, though if your 'sh' doesn't
support this feature, it'll be too late, because shelltest needs
minimal/do in order to run.
This is unfixable when running with -j > 1 because of how the current
t/flush-cache script works. We'll only be able to fix that after
making a more granular flush-cache tool, which is already on my todo
list.
This new test validates that you can pass -j1 and -j2 in a sub-redo to
create a sub-jobserver with exactly the number of jobs you specified.
Now that we have that feature, we can also test for the bug fixed two
commits ago where, with -j1, targets would be built in an unexpected
order.
posh will abort the entire script if it detects a syntax error. I
don't know if that's good or not, but you shouldn't be writing scripts
with syntax errors, so that by itself isn't a good reason for posh to
fail.
It still fails some actual tests, but at least now we don't consider it
a 'crash' outcome.
If a file is overridden and then overridden again, this caused us to
rebuild only the first thing that depends on it, but not any subsequent
things, which is a pretty serious bug.
It turned out that t/350-deps-forget is already supposed to test this,
but I had cleverly encoded the wrong behaviour into the expected
results in the table-driven test. I blame lack of sleep. Anyway, I
fixed the test, which made it fail, and then fixed the code, which made
it pass.
Both redo and minimal/do were doing slightly weird things with
symlinked directories, especially when combined with "..". For
example, if x is a link to ., then x/x/x/x/../y should resolve to
"../y", which is quite non-obvious.
Added some tests to make sure this stays fixed.
Based on the earlier t/000-set-minus-e bug in minimal/do on some
shells, let's add some extra tests that reveal the weirdness on those
shells. Unfortunately because they are so popular (including bash and
zsh), we can't reject them outright for failing this one.
While we're here, add a new return code, "skip", which notes that a
test has failed but is not important enough to be considered a warning
or failure. Previously we just had these commented out, which is not
quite obvious enough.
...and I updated a few comments while reviewing some of the older
tests.
We were setting a global variable FAIL on failure, but if we failed
inside a subshell (which a very small number of tests might do), this
setting would be lost. The script output (a series of failed/warning
lines) was still valid, but not the return code, so the shell might be
selected even if one of these tests failed.
To avoid the problem, put the fail/warning state in the filesystem
instead, which is shared across subshells.
Although I expect this is rather rare, some people may want to build in
a read-write subdir of a read-only tree. Other than some confusing
error reporting, this works fine in redo after the recent changes to
temp file handling, but let's add a test to make sure it stays that
way. The test found a bug in minimal/do, so let's fix that.
Reported-by: Jeff Stearns <jeff.stearns@gmail.com>
Previously, we'd try to put the stdout temp file in the same dir as the
target, if that dir exists. Otherwise we'd walk up the directory tree
looking for a good place. But this would go wrong if the directory we
chose got *deleted* during the run of the .do file.
Instead, we switch to an entirely new design: we use mkstemp() to
generate a temp file in the standard temp file location (probably
/tmp), then open it and immediately delete it, so the .do file can't
cause any unexpected behaviour. After the .do file exits, we use our
still-open fd to the stdout file to read the content back out.
In the old implementation, we also put the $3 in the "adjusted"
location that depended whether the target dir already existed, just for
consistency. But that was never necessary: we didn't create the $3
file, and if the .do script wants to write to $3, it should create the
target dir first anyway. So change it to *always* use a $3 temp
filename in the target dir, which is much simpler and so has fewer edge
cases.
Add t/202-del/deltest4 with some tests for all these edge cases.
Reported-by: Jeff Stearns <jeff.stearns@gmail.com>
This can happen if we create the .tmp file in the same directory as the
target, and the .do file first does "rm -rf" on that directory, then
re-creates it. The result is that the stdout file is lost.
We'll make this a warning if the .do script *didn't* write to stdout
(so the loss is harmless, just weird), and an error if they *did* write
to stdout, which we can detect because we still have an open fd on the
file, so we can fstat() it.
Basically all just missing quotes around shell strings that use $PWD.
Most paths inside a project, since redo uses relative paths, only need
to worry when project-internal directories or filenames have spaces in
them.
Reported-by: Jeff Stearns <jeff.stearns@gmail.com>
This avoids a name overlap with the system-installed copy of python.
Since redo adds the redo/ dir to the $PATH before running .do files,
python.do might see its own previously-created target instead of the
"real" python when testing, and create an infinite loop by accident.
It's time to start preparing for a version of redo that doesn't work
unless we build it first (because it will rely on C modules, and
eventually be rewritten in C altogether).
To get rolling, remove the old-style symlinks to the main programs, and
rename those programs from redo-*.py to redo/cmd_*.py. We'll also move
all library functions into the redo/ dir, which is a more python-style
naming convention.
Previously, install.do was generating wrappers for installing in
/usr/bin, which extend sys.path and then import+run the right file.
This made "installed" redo work quite differently from running redo
inside its source tree. Instead, let's always generate the wrappers in
bin/, and not make anything executable except those wrappers.
Since we're generating wrappers anyway, let's actually auto-detect the
right version of python for the running system; distros can't seem to
agree on what to call their python2 binaries (sigh). We'll fill in the
right #! shebang lines. Since we're doing that, we can stop using
/usr/bin/env, which will a) make things slightly faster, and b) let us
use "python -S", which tells python not to load a bunch of extra crap
we're not using, thus improving startup times.
Annoyingly, we now have to build redo using minimal/do, then run the
tests using bin/redo. To make this less annoying, we add a toplevel
./do script that knows the right steps, and a Makefile (whee!) for
people who are used to typing 'make' and 'make test' and 'make clean'.
This seems to only affect old zsh on MacOS. But we want to catch it
anyway, because it caused t/351-deps-forget to fail in a weird way on
that version of zsh.
Shells really suck.
I attempted to fix this in commit c06d1fba40, but it was apparently
incomplete and not all cases were covered by tests.
Let's add a much more thorough test by going through every possible
combination and making sure redo-{sources,targets,ood} all work as
expected, that the "you modified it" warning does or does not show up
when expected, and that dependencies are rebuilt the number of times we
expect.
When we check dependencies and a previously-is_generated dependency
existed before, but no longer does, forget that it was is_generated.
This slightly improves the situation where as a project evolves, a file
that used to be a target gets removed, and then later is re-added as a
static source file. (It doesn't fix the other variant, where a file is
changed from target to source in a single atomic change, and is never
missing. That one will be trickier to handle.)
While adding a test for this behaviour, I discovered that redo-sources,
redo-targets, and redo-ood were reporting their output relative to
STARTDIR instead of relative to $PWD, so fix that too.
flush-cache reduces the failed_runid by 1 each time, and it runs
multiple times per 'redo test'. if failed_runid goes to zero, it would
be treated as success ("no failure") rather than a real failure at
runid 0.
In commit redo-0.11-4-g34669fb, we changed os.stat into os.lstat to
avoid false positives in the "manual override" detector: a .do file
that generates $3 as a symlink would trigger manual override if the
*target* of that symlink ever changed, which is incorrect.
Unfortunately using os.lstat() leads to a different problem: if X
depends on Y and Y is a symlink to Z, then X would not be rebuilt when
Z changes, which is clearly wrong.
The fix is twofold:
1. read_stamp() should change on changes to both the link itself,
*and* the target of the link.
2. We shouldn't mark a target as overridden under so many situations.
We'll use *only* the primary mtime of the os.lstat(), not all the
other bits in the stamp.
Step 2 fixes a few other false positives also. For example, if you
'cp -a' a whole tree to another location, the st_ino of all the targets
will change, which would trigger a mass of "manual override" warnings.
Although a change in inode is sufficient to count an input as having
changed (just to be extra safe), it should *not* be considered a manual
override. Now we can distinguish between the two.
Because the stamp format has changed, update the SCHEMA_VER field. I
should have done this every other time I changed the stamp format, but
I forgot. Sorry. That leads to spurious "manually modified" warnings
after upgrading redo.
* redo-log:
redo-log: add man page.
redo-log: add automated tests, and fix some path bugs revealed by them.
redo-log: fix stdout vs stderr; don't recapture if .do script redirects stderr.
redo-log: don't show status line until >1.0 seconds after starting.
Add --color and --no-color options.
redo-log: --debug-pids works properly again.
Split --raw-logs into --no-pretty and --no-log options.
redo-log: prioritize the "foreground" process.
redo-log: correctly indent first level of recursion.
Raw logs contain @@REDO lines instead of formatted data.
redo-log: status line should use actual terminal width.
redo-log: capture and linearize the output of redo builds.
Use signal.setitimer instead of signal.alarm.
When a log for X was saying it wanted to refer to Y, we used a relative
path, but it was sometimes relative to the wrong starting location, so
redo-log couldn't find it later.
Two examples:
- if default.o.do is handling builds for a/b/x.o, and default.o.do
does 'redo a/b/x.h', the log for x.o should refer to ./x.h, not
a/b/x.h.
- if foo.do is handling builds for foo, and it does
"cd a/b && redo x", the log for foo should refer to a/b/x, not just
x.
On systems where 'python' refers to python3, redo
failed to launch. All invocations of python have been
made explicitly python2 invocations. All tests pass
on an Arch Linux system as of this commit.
If we tried to build target a/b/c/d and a/b/c didn't exist yet, we
would correctly name the temp file something like a__b/c__d.tmp. But
if a/b didn't exist yet, we named the temp file a/b__c/d.tmp, which
didn't work. Instead, name it a/b__c__d.tmp as expected.
With the new "continue" feature on by default, it turned out that
ctrl-c during a build, or a .do file returning an error, would mark a
target as "built" even though it hadn't been. This would prevent
retrying it when you started minimal/do again. Use a temp file
instead.
It's a little tricky: to prevent accidental recursion, we want to
create a file *before* building, but clean up that file when starting
the next session. And we rename that file to the actual .did file
*after* building successfully.
It would be incorrect to print ../$1.do (we only use $1.do in the
*current* directly, not prefix dirs; we only use default*.do in
those). However, when we found ../default.do, we forgot to print it,
because of a silly logic error.
When we can't find a .do file, we walk all the way back to the root
directory. When that happens, the root directory is actually searched
twice. This is harmless (since a .do file doesn't exist there anyway)
but causes redo-whichdo to produce the wrong output.
Also, add a test, which I forgot to do when writing whichdo in the
first place.
To make the test work from the root directory, we need a way to
initialize redo without actually creating a .redo directory. Add a
init_no_state() function for that purpose, and split the necessary path
functions into their own module so we can avoid importing builder.py.
This is the test that
x=y f
does *not* unset x after running f, if f is a shell function. Apparently
that's the right thing to do, but freebsd dash 0.5.10.2 fails it. This is
surprising because debian dash 0.5.8-2.4 passes, and it is presumably older.
Downgrade this to a warning because we want freebsd to have some cheap sh
variant that passes with redo, and nobody should be *relying* on this
insane behaviour anyway.
freebsd 11.2 sh (but not freebsd dash) fails test #24. That one seems
rather serious, so I don't want to downgrade it to a warning.
We previously assumed that redo and redo-ifchange are the same in
minimal/do's design, because it rebuilds all targets on every run, and
so there's no reason to ever build the same target more than once.
Unfortunately that's incorrect: if you run 'redo x' from two points in
a single run (or even twice in the same .do file), we expect x to be
built twice. If you wanted redo to decide whether to build it the
second time, you should have used redo-ifchange.
t/102-empty/touchtest was trying to test for this. However, a
second bug in minimal/do made the test pass anyway. minimal/do would
*always* rebuild any target x that produced no output, not caring
whether it had tried to build before, whether you used redo or
redo-ifchange. And while we tested that redo would redo a file that
had been deleted, we didn't ensure that it would redo a file that was
*not* deleted, nor that redo-ifchange would *not* redo that file.
Fix both bugs in minimal/do, and make t/102-empty/touchtest cover the
missing cases.
We need to create the File object to get its f.id, then lock that id.
During that gap, another instance of redo may have modified the file or
its state data, so we have to refresh it.
This fixes 'redo -j10 t/stress'.
It looks like we're updating the stamp for t/countall while another
task is replacing the file, which suggests a race condition in our
state management database.
flush-cache can cause files affected by redo-stamp to get rebuilt
unnecessarily, which the test is specifically trying to validate.
Since other tests run flush-cache at random times when using -j, this
would cause random test failures.
