client.mk runs configure twice

RESOLVED FIXED in Thunderbird 9.0

Status

MailNews Core
Build Config
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: neil@parkwaycc.co.uk, Assigned: neil@parkwaycc.co.uk)

Tracking

unspecified
Thunderbird 9.0
x86
Windows XP
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
Steps to reproduce problem:
1. Update an existing tree so that one of the config.status dependencies is newer than config.status and Makefile
2. Try to rebuild using $(MAKE) -f $(srcdir)client.mk

Expected result: client.mk runs configure once

Actual result: client.mk runs configure twice

This is caused by the following bogus rule in client.mk:
$(OBJDIR)/Makefile $(OBJDIR)/config.status: $(CONFIG_STATUS_DEPS)
	@$(MAKE) -f $(TOPSRCDIR)/client.mk configure

Since both files are out of date, pymake runs configure twice, once for each file. This is correct behaviour! You don't notice it with gmake because client.mk contains .NOTPARALLEL and gmake evaluates each dependency lazily, so by the time Makefile has caused configure to be run, gmake realises that config.status is now up-to-date and doesn't bother running it again.
(Assignee)

Comment 1

6 years ago
Created attachment 558190 [details] [diff] [review]
Proposed patch

Because (except in special circumstances, such as making both foo.h and foo.c from foo.yy) gmake doesn't actually allow you to have two different files depending on the same command, normal attempts at ensuring dependencies fail. In particular, if Makefile does not exist, then we can't make it depend on config.status, because gmake won't know how to remake it. So instead we make Makefile depend on $(CONFIG_STATUS_DEPS) directly in that case.
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #558190 - Flags: review?(khuey)
Attachment #558190 - Flags: review?(bugspam.Callek)
Comment on attachment 558190 [details] [diff] [review]
Proposed patch

>-$(OBJDIR)/Makefile $(OBJDIR)/config.status: $(CONFIG_STATUS_DEPS)
>+ifneq (,$(MAKEFILE))
>+$(OBJDIR)/Makefile: $(OBJDIR)/config.status
>+
>+$(OBJDIR)/config.status: $(CONFIG_STATUS_DEPS)
>+else
>+$(OBJDIR)/Makefile: $(CONFIG_STATUS_DEPS)
>+endif
> 	@$(MAKE) -f $(TOPSRCDIR)/client.mk configure


I didn't know you could do

if condition
SOME TARGET: SOME DEP
else
SOME OTHER TARGET: SOME OTHER DEP
endif
  RECIPE
Attachment #558190 - Flags: review?(khuey) → review+
(Assignee)

Comment 3

6 years ago
Pushed changeset e095afa62bf5 to mozilla-central.

Updated

6 years ago
Attachment #558190 - Flags: review?(bugspam.Callek) → review+
(Assignee)

Comment 4

6 years ago
Pushed changeset a060508db9b3 to comm-central.
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Flags: in-testsuite-
Target Milestone: --- → Thunderbird 9.0
You need to log in before you can comment on or make changes to this bug.