Re: reformatted GNUmakefile pushed to staging

[David Kastrup]

David Kastrup <address@hidden> writes:

> Werner LEMBERG <address@hidden> writes:
>
>> Folks,
>>
>>
>> as discussed some weeks ago, I've now pushed the reformatted
>> GNUmakefile to staging.  There are only whitespace changes.
>
> Is there anything wrong with our review system?  If you feel there is,
> how about at least posting the patch for review on the list before
> pushing?
>
> Things like
>
> +TOPDOC_TXT_FILES = \
> +  $(addprefix $(top-build-dir)/Documentation/topdocs/$(outdir)/, \
> +              $(addsuffix .txt, \
> +                          $(TOPDOC_FILES)))
> +IN_FILES := \
> +  $(call src-wildcard, \
> +         *.in)
>
> are quite beyond the scope of discussion and I don't see that they
> improve either readability nor reduce the potential for merge conflicts.
> And things like
>
> +STEPMAKE_TEMPLATES = \
> +  po \
> +  install \
> +  toplevel 
> +LOCALSTEPMAKE_TEMPLATES = \
> +  lilypond
>
> are ugly without an additional empty line.  Stuff like
>
> -       $(MAKE) local-dist $(distdir)
> +       $(MAKE) local-dist \
> +               $(distdir)
>
> makes little sense: why split a simple command into two lines if after
> the split both the first as well as the second line are special (one
> starts with $(MAKE) and the other does not end with \)?  Absolutely no
> gain in readability or mergeability.
>
> And that's just from glancing over the first few lines.  More important:
> you are changing a core part of the build system.  You _intend_ this
> change not to contain any functional differences, but a rather thorough
> way to be reasonably sure that you were successful in every detail is
> our regtest system.  Which does not just check, from a clean slate, that
> everything compiles, but also that the results from the regtests stay
> the same.
>
> The staging merge does not guarantee the same.  It is, speaking in the
> words of chairman Percival, a safety net, not a trampoline.

Oh you must be kidding me.

From: address@hidden (David Kastrup)
To: address@hidden
Date: Tue, 23 Oct 2012 10:32:45 +0200 (CEST) (23 minutes, 21 seconds ago)

From: patchy
Reply-To: address@hidden
To: address@hidden
Cc: address@hidden
Date: Tue, 23 Oct 2012 10:32:45 +0200
User-Agent: Patchy - LilyPond autobuild
Subject: Patchy email

08:19:28 (UTC) Begin LilyPond compile, previous commit at       
c5dd62ea78a8790465b8427febec71ff0ee21fdc
08:19:38 Merged staging, now at:        c5dd62ea78a8790465b8427febec71ff0ee21fdc
08:19:40        Success:                ./autogen.sh --noconfigure
08:20:04        Success:                /tmp/lilypond-autobuild/configure 
--disable-optimising
08:20:12        Success:                nice make clean
08:25:42        Success:                nice make -j3
08:32:45 *** FAILED BUILD ***
        nice make test -j3
        Previous good commit:   c7d55b2b61d7cf5310c40b6e76fb31a482f812f1
        Current broken commit:  c5dd62ea78a8790465b8427febec71ff0ee21fdc
08:32:45 *** FAILED STEP ***
        merge from staging
        Failed runner: nice make test -j3
See the log file log-staging-nice-make-test--j3.txt
08:32:45 Traceback (most recent call last):
  File 
"/usr/local/tmp/lilypond-extra/patches/compile_lilypond_test/__init__.py", line 
523, in handle_staging
    self.build (issue_id=issue_id)
  File 
"/usr/local/tmp/lilypond-extra/patches/compile_lilypond_test/__init__.py", line 
323, in build
    issue_id)
  File 
"/usr/local/tmp/lilypond-extra/patches/compile_lilypond_test/__init__.py", line 
266, in runner
    raise FailedCommand ("Failed runner: %s\nSee the log file %s" % (command, 
this_logfilename))
FailedCommand: Failed runner: nice make test -j3
See the log file log-staging-nice-make-test--j3.txt


make --no-builtin-rules -C input/regression/lilypond-book 
make[1]: Entering directory 
`/tmp/build-lilypond-autobuild/input/regression/lilypond-book'
/tmp/lilypond-autobuild/./input/regression/lilypond-book/GNUmakefile:24: 
warning: overriding commands for target `out/collated-files.list'
/tmp/lilypond-autobuild/./make/lysdoc-rules.make:6: warning: ignoring old 
commands for target `out/collated-files.list'
true
make[1]: Leaving directory 
`/tmp/build-lilypond-autobuild/input/regression/lilypond-book'
out=test \
                local-test
/bin/sh: 1: local-test: not found
make: *** [test] Error 127


Werner, can you _please_, _please_, _please_ not consider our review
system optional?  I have no idea why you think it appropriate to commit
changes to the toplevel build system you have not even tested yourself
completely.

I am backing this change out of staging again.  I actually don't usually
run staging-patchy myself because of its time hit, but this time I
wanted to be on the safe side before starting additional merge work to
avoid having to redo it.  Seems like it was a smart investment of time.

-- 
David Kastrup