Closed Bug 454449 Opened 16 years ago Closed 16 years ago

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

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.9.1b2

People

(Reporter: gozer, Assigned: gozer)

References

Details

Attachments

(1 file, 2 obsolete files)

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
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.
Attachment #341326 - Flags: review?(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
Attachment #341326 - Flags: review?(ted.mielczarek) → review+
Assignee: nobody → 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)"
Ah, interesting. In that case, do ifeq(,$strip $(AUTOCONF)) so it won't get confused by whitespace.
I suspect you meant:

$(strip $(AUTOCONF))
Attachment #341326 - Attachment is obsolete: true
Attachment #342116 - Flags: review?(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.
Attachment #342116 - Flags: review?(ted.mielczarek) → review+
Attachment #342116 - Attachment filename: client.mk.diff → [checked in] client.mk.diff
comparing with ssh://hg.mozilla.org/comm-central
searching for changes
changeset:   645:6b9c0a5e0777
tag:         tip
user:        Philippe M. Chiasson <gozer@mozillamessaging.com>
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 <gozer@mozillamessaging.com>
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
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
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)
Backed out this change until I can submit a revised patch taking into account bustage reprort from comment #9
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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
Attachment #344235 - Flags: review?(ted.mielczarek)
Attachment #342116 - Attachment is obsolete: true
Attachment #344235 - Flags: review?(ted.mielczarek) → review+
Keywords: checkin-needed
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
Attachment #344235 - Attachment description: Bail out of client.mk if it couldn't find autoconf v3 → Bail out of client.mk if it couldn't find autoconf v3 [Checkin: Comment 12]
Status: REOPENED → RESOLVED
Closed: 16 years ago16 years ago
Flags: in-testsuite-
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b2
Version: unspecified → Trunk
Blocks: 607775
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.