missing dependency for mdb_v8_version.c leads to build failure

[davepacheco]

@rmustacc reported that under some conditions, the build could fail when built with parallelism. One problem is that the target for mdb_v8_verison.c generates a file in MDBV8_BUILD, but does not depend on that directory existing.

[davepacheco]

Change: https://cr.joyent.us/#/c/1505/

I've tested this by doing several builds, running git clean -fxd; rm -rf deps/illumos-libavl; mkdir deps/illumos-libavl in between them to start from scratch. I've tried with make, make -j 4, make -j 16, and make -j32. They all worked fine except for one that failed with a different issue, which I'm now looking at.

[jclulow]

@davepacheco Running those same tests (i.e., make with -j) used to fail for you, and now does not?

[davepacheco]

@jclulow I never managed to get it to fail in that way myself.

With the changes for patchset 1 in place, when running with make -j 128 (as the SmartOS platform build does), I very occasionally see this:

mkdir -p build/ia32
mkdir -p build
git submodule update --init deps/illumos-libavl
mkdir -p build/amd64
cc -o build/ia32/mdb_v8.o -c -Werror -Wall -Wextra -fPIC -fno-omit-frame-pointer -g -O -Wno-unused-parameter -Wno-missing-field-initializers -Wno-sign-compare  -Wno-unknown-pragmas -m32 -DMDBV8_VERS_TAG='"dev"' -Ideps/illumos-libavl/include src/mdb_v8.c
cc -o build/ia32/mdb_v8_cfg.o -c -Werror -Wall -Wextra -fPIC -fno-omit-frame-pointer -g -O -Wno-unused-parameter -Wno-missing-field-initializers -Wno-sign-compare  -Wno-unknown-pragmas -m32 -DMDBV8_VERS_TAG='"dev"' -Ideps/illumos-libavl/include src/mdb_v8_cfg.c
cc -o build/ia32/mdb_v8_function.o -c -Werror -Wall -Wextra -fPIC -fno-omit-frame-pointer -g -O -Wno-unused-parameter -Wno-missing-field-initializers -Wno-sign-compare  -Wno-unknown-pragmas -m32 -DMDBV8_VERS_TAG='"dev"' -Ideps/illumos-libavl/include src/mdb_v8_function.c
cc -o build/ia32/mdb_v8_strbuf.o -c -Werror -Wall -Wextra -fPIC -fno-omit-frame-pointer -g -O -Wno-unused-parameter -Wno-missing-field-initializers -Wno-sign-compare  -Wno-unknown-pragmas -m32 -DMDBV8_VERS_TAG='"dev"' -Ideps/illumos-libavl/include src/mdb_v8_strbuf.c
cc -o build/ia32/mdb_v8_string.o -c -Werror -Wall -Wextra -fPIC -fno-omit-frame-pointer -g -O -Wno-unused-parameter -Wno-missing-field-initializers -Wno-sign-compare  -Wno-unknown-pragmas -m32 -DMDBV8_VERS_TAG='"dev"' -Ideps/illumos-libavl/include src/mdb_v8_string.c
cc -o build/ia32/mdb_v8_subr.o -c -Werror -Wall -Wextra -fPIC -fno-omit-frame-pointer -g -O -Wno-unused-parameter -Wno-missing-field-initializers -Wno-sign-compare  -Wno-unknown-pragmas -m32 -DMDBV8_VERS_TAG='"dev"' -Ideps/illumos-libavl/include src/mdb_v8_subr.c
cc -o build/amd64/mdb_v8.o -c -Werror -Wall -Wextra -fPIC -fno-omit-frame-pointer -g -O -Wno-unused-parameter -Wno-missing-field-initializers -Wno-sign-compare  -Wno-unknown-pragmas -m64 -DMDBV8_VERS_TAG='"dev"' -Ideps/illumos-libavl/include src/mdb_v8.c
cc -o build/amd64/mdb_v8_cfg.o -c -Werror -Wall -Wextra -fPIC -fno-omit-frame-pointer -g -O -Wno-unused-parameter -Wno-missing-field-initializers -Wno-sign-compare  -Wno-unknown-pragmas -m64 -DMDBV8_VERS_TAG='"dev"' -Ideps/illumos-libavl/include src/mdb_v8_cfg.c
cc -o build/amd64/mdb_v8_function.o -c -Werror -Wall -Wextra -fPIC -fno-omit-frame-pointer -g -O -Wno-unused-parameter -Wno-missing-field-initializers -Wno-sign-compare  -Wno-unknown-pragmas -m64 -DMDBV8_VERS_TAG='"dev"' -Ideps/illumos-libavl/include src/mdb_v8_function.c
cc -o build/amd64/mdb_v8_strbuf.o -c -Werror -Wall -Wextra -fPIC -fno-omit-frame-pointer -g -O -Wno-unused-parameter -Wno-missing-field-initializers -Wno-sign-compare  -Wno-unknown-pragmas -m64 -DMDBV8_VERS_TAG='"dev"' -Ideps/illumos-libavl/include src/mdb_v8_strbuf.c
cc -o build/amd64/mdb_v8_string.o -c -Werror -Wall -Wextra -fPIC -fno-omit-frame-pointer -g -O -Wno-unused-parameter -Wno-missing-field-initializers -Wno-sign-compare  -Wno-unknown-pragmas -m64 -DMDBV8_VERS_TAG='"dev"' -Ideps/illumos-libavl/include src/mdb_v8_string.c
cc -o build/amd64/mdb_v8_subr.o -c -Werror -Wall -Wextra -fPIC -fno-omit-frame-pointer -g -O -Wno-unused-parameter -Wno-missing-field-initializers -Wno-sign-compare  -Wno-unknown-pragmas -m64 -DMDBV8_VERS_TAG='"dev"' -Ideps/illumos-libavl/include src/mdb_v8_subr.c
cc -o build/amd64/mdb_v8_version.o -c -Werror -Wall -Wextra -fPIC -fno-omit-frame-pointer -g -O -Wno-unused-parameter -Wno-missing-field-initializers -Wno-sign-compare  -Wno-unknown-pragmas -m64 -DMDBV8_VERS_TAG='"dev"' -Ideps/illumos-libavl/include build/mdb_v8_version.c
cc: error: build/mdb_v8_version.c: No such file or directory
cc: fatal error: no input files
compilation terminated.
make: *** [build/amd64/mdb_v8_version.o] Error 1
make: *** Waiting for unfinished jobs....
Submodule 'deps/illumos-libavl' () registered for path 'deps/illumos-libavl'
Cloning into 'deps/illumos-libavl'...
remote: Counting objects: 14, done.
remote: Total 14 (delta 0), reused 0 (delta 0), pack-reused 14
Receiving objects: 100% (14/14), 15.57 KiB, done.
Resolving deltas: 100% (2/2), done.
Submodule path 'deps/illumos-libavl': checked out '60a0c8a31038ec1d6096c9bbcb089326659fc9c8'

I've been testing that as follows:

while make -j 128; do git clean -fxd; rm -rf deps/illumos-libavl && mkdir deps/illumos-libavl; echo; echo; echo CLEAN; echo; echo; done

And in case it proves helpful, here's what existed when all this was done:

$ find build -type f
build/ia32/mdb_v8.o
build/ia32/mdb_v8_string.o
build/ia32/mdb_v8_strbuf.o
build/ia32/mdb_v8_subr.o
build/ia32/mdb_v8_cfg.o
build/ia32/mdb_v8_function.o
build/amd64/mdb_v8_subr.o
build/amd64/mdb_v8.o
build/amd64/mdb_v8_cfg.o
build/amd64/mdb_v8_function.o
build/amd64/mdb_v8_strbuf.o
build/amd64/mdb_v8_string.o

I'm not sure what's going on here. On the one hand, make is clearly using our "$(COMPILE.c)" command. We know because the built-in recipe for C files puts CPPFLAGS first, but the output reflects CPPFLAGS being last, which is the way we've defined "$(COMPILE.c)". And "$(COMPILE.c)" is the only recipe we've defined that invokes $(CC). Further, "$(COMPILE.c)" is only used in the recipes for two targets, both pattern rules, and both defined in Makefile.arch.targ. The first, at line 25, only applies when there's a same-named C file in "src", which isn't the case here. The second, at line 28, is the one that should be applied here -- it's the one intended specifically for this case. That one explicitly depends on a C file in "build" -- but for some reason. That file is generated by a different target. In this case, make clearly applied this pattern rule as we'd expect, but never actually built the prerequisite. I'm not sure how that's possible (absent a "make" bug).

If we think about how this should work: there's no other target here that depends on "$(MDBV8_BUILD)/mdb_v8_version.c". The only way I can see that being built at all is if "make" decides to apply the pattern rule above. So at least under some conditions, that rule must be sufficient to cause make to build this file. Besides that, the GNU make manual for pattern rules says:

You define an implicit rule by writing a pattern rule. A pattern rule looks like an ordinary rule, except that its target contains the character ‘%’ (exactly one of them).

‘%’ in a prerequisite of a pattern rule stands for the same stem that was matched by the ‘%’ in the target. In order for the pattern rule to apply, its target pattern must match the file name under consideration and all of its prerequisites (after pattern substitution) must name files that exist or can be made. These files become prerequisites of the target.

I feel I must be missing something pretty basic here. I will rerun with "make -d" and try to see what's going on.

[davepacheco]

This looks quite similar to GNU make bug 37703, present in version 3.82 (the version I was using) but fixed in version 4.0. I downloaded and built make version 4.2.1 and ran the same loop as before, but with a counter:

i=0; while ../make-4.2.1/make -j 128; do i=$(( i + 1 )); git clean -fxd; rm -rf deps/illumos-libavl && mkdir deps/illumos-libavl; echo; echo; echo CLEAN; echo; echo; done; echo $i;

I ran 182 iterations without seeing a build failure. On the other hand, with make 3.82, I saw the build failure after 3 iterations, then 6 iterations, then 13 iterations, then 31 iterations. For completeness, I went back to make 4.2.1 and completed another 130 iterations. Then I went back to 3.82 and it failed after 5 iterations.

Given that, I'm calling this change ready to go. If there are additional build issues here, we can track those under a separate ticket.