From: Charles W. <cwi...@us...> - 2009-10-23 18:41:05
|
Keith Marshall wrote: > On Thursday 22 October 2009 23:33:29 Charles Wilson wrote: >> Well, you can't actually hide an $(error ...) statement inside a >> shell script. You have to do 'echo "..." ; exit non-zero' instead, >> but that's a minor matter. > > I thought that too, but I *did* test the sample Makefile I posted, on > both Ubuntu-8.04 and MSYS/Win2K; GNU Make 3.81 in both cases, and in > both it did DTRT. Maybe it wouldn't work with an earlier version? > However, neither make's own Texinfo documentation, nor my copy of > Robert Mecklenburg's `Managing Projects with GNU Make' (O'Reilly, > 3rd edition), suggest that it wouldn't. Odd. All of Cygwin's make-3.81-2, MSYS make (3.81, stock from MSYS-1.0.11), mingw32-make, and mandriva make 3.81-2mdv2008.0 fail in this simple case: all: @echo "here" @if test -z ""; then \ echo "empty";\ else \ $(error Unexpected);\ fi $ make Makefile:2: *** Unexpected. Stop. Note that I don't even see "here" -- the error statement is triggered during initial parsing of the makefile, not during the execution of the 'all:' rule. AFAICT, the only things that "guard" an $(error ...) are makefile conditionals, which is what we're trying avoid. Maybe the make documentation needs an update... > I would write the ChangeLog entries in the `modified files' section > something like: > > * winsup/mingw/Makefile.in: > (DESTDIR): Honor per convention; add it to... > (FLAGS_TO_PASS): this. > (need-DESTDIR-compatibility): New macro; define it. > (check-DESTDIR-compatibility): New dependency goal. > (install-dirs): Require it. > ... OK. > so that we capture the addition of the `DESTDIR-compatibility' code, > and identify the existing goals it affects. > >> +check-DESTDIR-compatibility: >> + ... >> + test 0 -eq $$status || break ;\ > > Adding this `test' to the actions for `check-DESTDIR-compatibility' > means that you capture and report only the first of possibly more > than one DESTDIR incompatibility per `make' run; omit it, as I did, > and you capture them all in a single run, which I think is better. I put it in deliberately, because of the wacky way mingw/'s install directories are computed (tooldir vs. prefix); do we really want to tell everybody they should "redefine" tooldir -- which they may have never heard of, unless they are in the habit of building binutils and gcc -- when all they really need to do is redefine prefix? Plus, you get false positives: if you have defined prefix as C:\MinGW, but have NOT actually defined includedir or libdir to anything special, then you get three error messages, where each suggests to explicitly override the affected variable. However, in mingw/ and w32api/ this is not necessary: by default, the includedir and libdir variables end up looking like this: includedir = $(prefix)/include # sometimes $(tooldir)/include libdir = $(prefix)/lib # or maybe exec_prefix, but then /that's/ # defined in terms of prefix Which means you really don't NEED to explicitly define includedir or libdir on the 'make install' line. I'll change it back if you insist, but I really think bailing out after the first error is /less/ confusing to end-users. >> +install-dirs: check-DESTDIR-compatibility >> + @echo all DESTDIR checks passed > > The `echo' command is redundant here; I would drop it. (I only had > it in my example, to exercise the minimal test case). Oh, OK. Sure. FYI, Ralf W. suggested the following: >> > +$(need-DESTDIR-compatibility): >> > + @case "$($@)" in *:*) \ > > Wouldn't ?:* suffice? Not that I'd like anyone to use a path with a > colon stuck in the middle of it on unix, but it certainly wouldn't be a > Win32 path. Is that change ok with you? -- Chuck |