makefile landmines

Makefile landmines: avoiding limb loss

[ps] ignore a similar makefile post I made yesterday. Somehow draft+update implied publish and the original content was not yet ready to see the light of day.

When writing makefiles there are a few logic bombs to try and avoid as they can mask failure conditions. When command exit status is ignored or overlooked the site can become a source for random failures or manifest in downstream testing.

A few problem sources to mention are:

1. Not returning, propagating, testing for or ignoring shell exit status.
2. Use of line continuation to join multi-line commands within a makefile.
3. Prefixing commands with a hyphen ‘-‘ to ignore errors.

A simple makefile (gotcha.mk) is attached below that will illustrate problem space for items #1 and #2 using the true & false shell commands. True will succeed with a shell exit status of zero (0==$?). False will fail with a non-zero exit status:

% gmake -f gotcha.mk show
makefile target is: show
     /bin/true: $?=0
    /bin/false: $?=1
 
% gmake -f gotcha.mk pass
makefile target is: pass
/bin/true
 
% gmake -f gotcha.mk fail
makefile target is: fail
/bin/false
gmake: *** [fail] Error 1

Line continuation: why they can be a problem

Line continuation is an efficiency that is often used to avoid the overhead of having to spawn a new command shell for each command mentioned in a target rule. Most of the time line continuation will function as expected but when exit status is lost or over-written, success is returned allowing make to return a false-positive.

An easy way to create this condition is by wrappering the logic in an if block. Commands can merrily fail in bulk within the block but final exit status will be set by the shell if/test condition — which always succeeds.

This can be illustrated by the makefile target ‘bad‘. Run the true command, then false and if all is well the target should fail on the false call. Guess what, we have opened a logic hole for bugs to quietly sneak in through:

% gmake -f gotcha.mk bad
makefile target is: bad
if [ ! -e '/non/existent/file' ]; then \
            /bin/true; \
            /bin/false; \
            echo "ASSERT: should not reach this point"; \
        fi
ASSERT: should not reach this point

There are a few workarounds for this problem space. One of the easiest is use of the shell ‘-e’ set/command line arg. When enabled a shell will exit immediately with non-zero status whenever a command fails.

Uncomment this line in the gotcha.mk and the command will fail as expected:
    SHELL := $(SHELL) -e

% gmake -f gotcha.mk bad
makefile target is: bad
   [snip]
gmake: *** [bad] Error 1

The -e option or a morale equivalent is supported by bourne, bash, dash, ksh or whatever other shells people may prefer to use.

suggestions: line continuation alternatives

• At a minimum for readability consider using GNU make’s .ONESHELL: directive in place of large line continuation blocks:

  http://www.gnu.org/software/make/manual/html_node/One-Shell.html

• Avoid line continuation wherever possible. Make is very good at control flow and processing but not so much as a scripting language. Blocks that span more than ~5 commands or are encapsulated within an if block would be ripe for breaking up into several distinct makefile goals {better yet general targets in a makefile library} or moving the entire block into a shell script that will be invoked by the target.
• shell/set -e flags should be enabled everywhere to improve error detection along with options to detect uninitialized variable usage. Functionality is already enabled by in a few places with use of the $(EXIT_ON_ERROR) make macro defined in config/config.mk. Listing $(EXIT_ON_ERROR) as the first item of a target rule will be expanded by make into $(SHELL) -e @command_text enabling fail on all command failure handling:

  config/config.mk

  50	EXIT_ON_ERROR = set -e; # Shell loops continue past errors without this.

  config/rules.mk

  226 227 228 229 230

  check:: @$(EXIT_ON_ERROR) \ for f in $(subst .cpp,$(BIN_SUFFIX),$(CPP_UNIT_TESTS)); do \ XPCOM_DEBUG_BREAK=stack-and-abort $(RUN_TEST_PROGRAM) $(DIST)/bin/$$f; \ done

  Make judicious use of sh -e functionality but do not blindly trust that the option has been enabled for a given makefile target. There will not always be a visual cue that the flag is active. In this case if the EXIT_ON_ERROR macro were inadvertently cleared or commented out a subset of error detection would be lost. Best option initially is to manually verify the failure condition then write a negative unit test to back up the assertion long term.

  • One trivial way to validate logic blocks like these is leverage the try server and modify the makefile to perform a negative test. Simply inline a call to 'exit ###' as the last statement in a block. Submit makefile edits that are guaranteed to fail then verify all platforms reported the failure everywhere: in logfiles, email and tbpl web status.

    all: if [ 1 -eq 1 ]; then\ /bin/ps \ && /bin/who \ && echo "********************************"\ && echo "Failure forced"\ && echo "********************************"\ && exit 99\ fi </code>

    % ${EDITOR} makefile.in % hg qnew patch.try % hg qref --message "try: -b do -e -p all -u all -t none" % hg push -f try

    If all goes well failure email will soon be on it's way. If the makefile fails with status 99 the negative test succeded. Be careful of errors reported early by ps or who, status could still be ignored by logic later on. ($?==99) is the only exit status that should be used to assert the negative test passed:

    gmake: *** [all] Error 99

    Negative testing can be as important as positive testing and will sanity check two important items. First that the exit/error status was properly detected and make was able to generate and record an error in the logs. Second, the failure status was able to propagate through the try server and was reported accurately by the web interface and through status email.

    If either condition is not met a bug should be opened so the logic can be modified to prevent any future failure conditions from silently sneaking into the tree. If negative tests are not able to force a failure conditions valid errors that occur while building/processing will not either.

  • The configure script would also be a place to perform similar -e tests.
    Often when commands cannot be found or failures occur the configure script
    is able to run to completion but underlying failures are lost in the
    shuffle.

  • Lastly a plug for testing. If you will be expending effort to test makefile logic go the extra setup and write a unit test and add a 'check:' target to automate the activity. Even if the test is a trivial smoke test initially it can be expanded later. Start with these three conditions, if any fail something has gone horribly wrong and may needlessly waste queue time and build resources:
    • positive test: target does not exist initially, generate and verify.
    • negative test: target cannot be created and test fails.
    • nop test: target exists, minimal overhead should be imposed. If a target performs work when up to date think of how much time is being wasted by the sum of all targets with this behavior.

    % gmake -f gotcha.mk check makefile target is: check Running positive test OK Running negative test OK Running negative test [failure forced] gmake: *** [check] Error 1

  Ignoring errors: hyphen prefix for commands

  AKA I do not care what type of explosion occurs on this line, ignore status and always return success.

  all: @echo "Inhibit target rule errors" -/bin/false

  There are a few legitimate places where this option can be used.

  • File deletion on a system that does not support rm -f.
  • Shell/OS command bug that will force returning non-zero status for

  command that should succeed (~transient problems).

  Aside from cases like these, use of a hyphen in general makefile logic to ignore a set of failure conditions will have an ugly side effect of also ignoring an entire set of unrelated problems. In the filesystem and OS area, that a target should be aware of and fail on.

  Ignoring conditions like say a file being conditionally non-existent when a target is processed because the file will be generated by the target is one example. Hmmm, this might also be an instance of trying to work around a line continuation problem, testing for file existence and subsequent loss of command exit status within the if block.

  In place of using hyphens to code around a set of conditions remove them outright and use an external shell script to do the right thing. Handle conditions, test for existence and return an overall shell exit status from the actions. If anything fails always return non-zero.

  Samples:

  Here is a sampling of makefile code from the tree with problems mentioned above. Not trying to pick on anything specific, just what can be found with a bit of poking around.

  • toolkit/locales/l10n.mk
    ** -cp -r $(JARLOG_DIR)/en-US/*.jar.log $(JARLOG_DIR_AB_CD)
    ** -$(PERL) -pi.old -e "s/en-US/$(AB_CD)/g" $(JARLOG_DIR_AB_CD)/*.jar.log
    ** A few conditions these commands will succeed on:
    *** Source files do not exist. When file existence is not mandatory $(if ) conditions and extra target rules can conditionally copy when needed. As could an external shell script that could bundle several statements and eventually be tested.
    *** Destination directory does not exist or filesystem is full.
    *** Source file is not readable.
    *** Destination file exists but is not writable.
    *** $(JARLOG_DIR_AB_CD) does not exist and we attempt to copy files into the directory.

  • js/src/ctypes/libffi/Makefile.in
    ** -rm -f src/alpha/ffi.$(OBJEXT)
    *** -rm is implied by the $(RM) macro: RM = rm -f
    *** Written as: $(RM) src/alpha/ffi.$(OBJEXT)
    ** -test -z "$(lib_LTLIBRARIES)" || rm -f $(lib_LTLIBRARIES)
    ** -test -z "$(noinst_LTLIBRARIES)" || rm -f $(noinst_LTLIBRARIES)
    *** Avoid spawning extra shells
    *** $(RM) will not fail when the macros are empty
    *** Written as: $(RM) $(lib_LTLIBRARIES) $(noinst_LTLIBRARIES)

  • config/rules.mk
    * clean clobber realclobber clobber_all:: $(SUBMAKEFILES)
    * -$(RM) $(ALL_TRASH)
    * -$(RM) -r $(ALL_TRASH_DIRS)
    ** -$(RM) is redundant of $(RM) unless RM != /bin/rm -f

  Sample makefile: gotcha.mk

  # -*- makefile -*- ############################################### ## Intent: makefile landmine examples ###############################################   ifneq ($(null),$(MAKECMDGOALS)) $(info makefile target is: $(MAKECMDGOALS)) endif   SHELL := $(SHELL) -e   all: pass fail bad   pass: /bin/true   fail: /bin/false   bad: if [ ! -e '/non/existent/file' ]; then \ /bin/true; \ /bin/false; \ echo "ASSERT: target $@: should not reach this point"; \ fi   check: @echo "Running positive test" @/bin/true && echo "OK" || exit 1 @echo "Running negative test" @! /bin/false && echo "OK" || exit 1 @echo "Running negative test [failure forced]" @/bin/false && echo "OK" || exit 1   show: @if [ 1 ]; then\ /bin/true; echo " /bin/true: \$$?=$$?"; \ /bin/false; echo " /bin/false: \$$?=$$?"; \ fi