454449 - client.mk doesn't gracefully fail if it can't locate autoconf

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Philippe M. Chiasson (:gozer)]

If client.mk fails to locate autoconf 2.13, it doesn't fail gracefull at all. It just sets AUTOCONF="" and continues like it found it, this results in this failure:

$> make -f client.mk
Adding client.mk options from [comm-central]/.mozconfig:
Generating [comm-central]/mozilla/configure using autoconf
cd [comm-central]/mozilla;
make[1]: Entering directory `[comm-central]'
Generating [comm-central]/configure using autoconf
cd [comm-central];
Generating [comm-central]/mozilla/configure using autoconf
cd [comm-central]/mozilla;
cd [comm-central]/objdir-tb
[comm-central]/configure
/bin/sh: line 1: [comm-central]/configure: No such file or directory
*** Fix above errors and then restart with               "make -f client.mk build"
make[1]: *** [configure] Error 1
make[1]: Leaving directory `[comm-central]'
make: *** [[comm-central]/objdir-tb/Makefile] Error 2

Comment 1

[Philippe M. Chiasson (:gozer)]

Attachment: Bail out of client.mk if it couldn't find autoconf

Here is a simple patch that bails out when unable to locate autoconf 2.13:

client.mk:97: *** Couldn't find autoconf 2.13.  Stop.

Comment 2

[(not currently active) Ted Mielczarek]

Comment on attachment 341326 [details] [diff] [review]
Bail out of client.mk if it couldn't find autoconf

I think you want
ifndef AUTOCONF

Comment 3

[Philippe M. Chiasson (:gozer)]

Nope, with ifndef, it doesn't work

$> make -f foo.mk autoconf
foo.mk:8: *** not found by ifeq.  Stop.

$> cat foo.mk
AUTOCONF ?= $(shell which autoconf-2.14 autoconf2.14 autoconf214 2>/dev/null | grep -v '^no autoconf' | head -1)

ifndef AUTOCONF
$(error not found by ifndef)
endif

ifeq (,$(AUTOCONF))
$(error not found by ifeq)
endif

autoconf:
        @echo "AUTOCONF is $(AUTOCONF)"

Comment 4

[(not currently active) Ted Mielczarek]

Ah, interesting. In that case, do ifeq(,$strip $(AUTOCONF)) so it won't get confused by whitespace.

Comment 5

[Philippe M. Chiasson (:gozer)]

Attachment: Bail out of client.mk if it couldn't find autoconf v2

I suspect you meant:

$(strip $(AUTOCONF))

Comment 6

[(not currently active) Ted Mielczarek]

Yep.

Comment 7

[(not currently active) Ted Mielczarek]

Comment on attachment 342116 [details] [diff] [review]
Bail out of client.mk if it couldn't find autoconf v2

You didn't need to re-request review. If I give you an r+ with review comments, you should assume you can fix those comments and commit. If I want a new patch, I'll r- your patch.

Comment 8

[Philippe M. Chiasson (:gozer)]

comparing with ssh://hg.mozilla.org/comm-central
searching for changes
changeset:   645:6b9c0a5e0777
tag:         tip
user:        Philippe M. Chiasson <[e-mail]>
date:        Mon Oct 20 12:02:56 2008 -0400
summary:     Bug 454449. Teach client.mk to fail more gracefully if it can't locate autoconf. r=ted.mielczarek


comparing with ssh://hg.mozilla.org/mozilla-central/
searching for changes
changeset:   20667:c966e7e41dbd
tag:         tip
user:        Philippe M. Chiasson <[e-mail]>
date:        Mon Oct 20 12:07:23 2008 -0400
summary:     Bug 454449. Teach client.mk to fail more gracefully if it can't locate autoconf. r=ted.mielczarek

Comment 9

[[e-mail]]

Comment on attachment 342116 [details] [diff] [review]
Bail out of client.mk if it couldn't find autoconf v2

>+$(error Couldn't find autoconf 2.13)
This breaks my build which tries to override autoconf in .mozconfig but can't. Perhaps you meant AUTOCONF = $(error Couldn't find autoconf 2.13)

Comment 10

[Philippe M. Chiasson (:gozer)]

Backed out this change until I can submit a revised patch taking into account bustage reprort from comment #9

Comment 11

[Philippe M. Chiasson (:gozer)]

Attachment: Bail out of client.mk if it couldn't find autoconf v3 [Checkin: Comment 12]

Address the problem raised in comment #9. Delaying the error until something actually tries to use autoconf makes more sense in general as well.

Plus, this will not break builds that define AUTOCONF in their mozconfigs, or with make -f client.mk AUTOCONF=1234

Comment 12

[Serge Gautherie (:sgautherie)]

Comment on attachment 344235 [details] [diff] [review]
Bail out of client.mk if it couldn't find autoconf v3
[Checkin: Comment 12]

http://hg.mozilla.org/mozilla-central/rev/bd6062445dfa