Adding redo-whichdo wasn't so hard, except that it exposed a bunch of
mistakes in the way minimal/do was choosing .do files. Most of the
wrong names it tried didn't matter (since they're very unlikely to
exist) except that they produce the wrong redo-whichdo output.
Debugging that took so much effort that I added unit tests for some
functions to make it easier to find problems in the future.
Note: this change apparently makes minimal/do about 50% slower than
before, because the _find_dofiles algorithm got less optimized (but
more readable). It's still not very slow, and anyway, since it's
minimal/do, I figured readable/correct code probably outweighed a bit
of speed. (Although if anyone can come up with an algorithm that's
clear *and* works better, I won't turn it down :) I feel like I must
be doing it the hard way...)
After playing with "continuable" mode for a while, it seems to me that
what most people want will be incremental builds by default (albeit
without any dependency checking to rebuild a target if sources change).
This lets you run minimal/do several times in a row to incrementally
build up your project while you're playing around. If you want to
start over, use -c.
As a result, in test.do we now recommend using 'minimal/do -c test'
since minimal/do now strays even further from proper redo semantics
otherwise.
While we're here, update the usage message to describe what all the new
options mean.
-x, -v, and -d are the same as redo.
-c means "continuable", which disables the feature that deletes (and
forgets) all targets at the start of each run. This is a little risky,
since minimal/do still doesn't understand dependencies, but it allows
you to run minimal/do several times in succession, so that
minimal/do -c a
minimal/do -c b
is the same as
minimal/do a b
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 grew a test for these at some point, but minimal/do didn't actually
pass it.
sh syntax oddity: if you say
x=$1 y=$2
then it works fine when $1 and $2 contain spaces. But if you say
export x=$1 y=$2
(or "local" instead of "export") then $1 and $2 will be split on IFS,
and it won't do what you think. I guess this is because 'export' and
'local' are like commands, and command arguments are split if not
quoted.
...or files that contain bytes but not a trailing newline. It's okay if we
don't get any data, but we definitely have to *not* let "set -e" abort us.
Now that we fixed set -e in the previous patch, it revealed this problem.
If you run something like
blah_function || return 1
then everything even *inside* blah_function is *not* subject to the "set -e"
that would otherwise be in effect. That's true even for ". subfile" inside
blah_function - which is exactly how minimal/do runs .do files.
Instead, rewrite it as
blah_function
[ "$?" = "0" ] || return 1
And add a bit to the unit tests to ensure that "set -e" behaviour is enabled
in .do files as we expect, and crash loudly otherwise.
(This weird behaviour may only happen in some shells and not others.)
Also, we had a "helpful" alias of redo() defined at the bottom of the file.
Combined with the way we use '.' to source the .do files, this would make it
not start a new shell just to run a recursive 'redo' command. It almost
works, but this stupid "set -e" bug could cause a nested .do file to not
honour "set -e" if someone ran "redo foo || exit 1" from inside a .do
script. The performance optimization is clearly not worth it here, so
rename it to _redo(); that causes it to actually re-exec the redo program
(which is a symlink to minimal/do).
If you use "redo --old-args", it will switch back to the old
(apenwarr-style) arguments for now, to give you time to update your .do
scripts. This option will go away eventually.
Note: minimal/do doesn't understand the --old-args option. If you're using
minimal/do in your project, keep using the old one until you update your use
of $1/$2, and then update to the new one.
apenwarr-style default.o.do:
$1 foo
$2 .o
$3 whatever.tmp
djb-style default.o.do:
$1 foo.o
$2 foo
$3 whatever.tmp
apenwarr-style foo.o.do:
$1 foo.o
$2 ""
$3 whatever.tmp
djb-style foo.o.do:
$1 foo.o
$2 foo.o (I think?)
$3 whatever.tmp
The reason we'd crash is that we tried to pre-create a file called
$target.redo.tmp, which wouldn't work because the directory containing
$target didn't exist.
We now try to generate a smarter filename by using the innermost directory
of target that *does* exist. It's a little messy, but the idea is to make
sure we won't have to rename() across a filesystem boundary if, for example,
there's a mounted filesystem in the middle of the hierarchy somewhere.
Since we use ". filename.do" to run the .do files instead of just
"filename.do", shell local variables end up being inherited by the
subprogram. Change all the local variables to be all lowercase, to avoid
conflicting with any typical environment variables someone might use.
The particular variable that triggered this was PREFIX (reported by "ulrik"
on the mailing list) and that fixes this, at least.
Arguably we shouldn't be using ".", but using it avoids unnecessary forks,
which is kind of nice.
If you ". ./filename" and ./filename contains no commands, apparently
ash/dash will give the exit code of the command *before* the ., rather than
defaulting to zero as it supposedly should. This should work around it in
our .do files at least.
Reported by Tim Allen.
It should just build nothing. Because sometimes you want to do something
like:
redo-ifchange $(find -name '*.c')
And the find doesn't return any results. This is consistent with what real
redo does.
Added a test to confirm that it works.
Sometimes clean.do implementations want to delete our .do_built file and
directory. If that happens, deal with it quietly, as long as they don't
request any *additional* redo-type operations.
Apparently emacs sets TERM=dumb in its tty simulator, so even though
isatty() returns true, we shouldn't use colour codes. (emacs is therefore
lame. But we knew that.)
If all.do runs and creates no output, we shouldn't create a file called
'all', but we should remember that 'all' has been run successfully. We do
this by creating 'all.did' during the build.
Since minimal/do always just wipes everything out every time it runs, we can
safely remove the .did files after minimal/do terminates, so this doesn't
clutter things too much in normal use.
This fixes some edge cases, particularly that 'minimal/do clean' no longer
leaves stupid files named "clean" lying around, and the redo-sh directory
can now be rebuilt correctly since we rebuild it as long as redo-sh.did
doesn't exist. (We don't want to "rm -rf redo-sh" because it makes me
nervous.)
...only when running under minimal/do, of course.
The tests in question mostly fail because they're testing particular
dependency-related behaviour, and minimal/do doesn't support dependencies,
so naturally it doesn't work.
Just allow that sub-redo to return an error code. Also, parent redos should
return error code 1, not the same code as the child. That makes it easier
to figure out which file generated the "special" error code.
It actually decreases readability of the .do files - by not making it
explicit when you're going into a subdir.
Plus it adds ambiguity: what if there's a dirname.do *and* a dirname/all?
We could resolve the ambiguity if we wanted, but that adds more code, while
taking out this special case makes *less* code and improves readability.
I think it's the right way to go.
This greatly reduces the number of fork+exec calls, so in particular,
t/curse/all.do now runs much faster:
/bin/sh (bash): was 5.9s, now 2.2s
/bin/dash: was 3.2s, now 1.1s
Obviously improving the speed of minimal/do doesn't really matter, except
that it makes a good benchmark to compare the "real" redo against. So far
it's losing badly: 5.4s.
This could be good for distributing with your packages, so that people who
don't have redo installed can at least build it. Also, we could use it for
building redo itself.
Will surely need to get slightly bigger as I inevitably discover I've
forgotten a critical feature.
