Currently we rely on ordering to make default the default make target. This is broken in a number of places. We should instead set .DEFAULT_GOAL = default to ensure that this is always the default target.

(In reply to Ted Mielczarek [:ted] from comment #0) > We should instead set .DEFAULT_GOAL = default to ensure that this is always the default target. Pymake doesn't support .DEFAULT_GOAL, so this would need to be added, too.

Comment on attachment 649198 [details] [diff] [review] Ensure that default is always the default target Review of attachment 649198 [details] [diff] [review]: ----------------------------------------------------------------- Excellent!

Comment on attachment 649198 [details] [diff] [review] Ensure that default is always the default target Review of attachment 649198 [details] [diff] [review]: ----------------------------------------------------------------- ::: build/pymake/pymake/data.py @@ +1541,5 @@ > np = self.gettarget('.NOTPARALLEL') > if len(np.rules): > self.context = process.getcontext(1) > > + flavor, source, value = self.variables.get('.DEFAULT_GOAL') Tab

https://hg.mozilla.org/integration/mozilla-inbound/rev/1f4eae8d9e08 http://hg.mozilla.org/users/bsmedberg_mozilla.com/pymake/rev/b083524b642c

(In reply to :Ms2ger from comment #4) > > + flavor, source, value = self.variables.get('.DEFAULT_GOAL') > > Tab That's interesting... python didn't complain.

https://hg.mozilla.org/integration/mozilla-inbound/rev/7bc9fc61d58f http://hg.mozilla.org/users/bsmedberg_mozilla.com/pymake/rev/9c2d6ceb8005

Backed out the rules.mk parts (the pymake parts can stay in) https://hg.mozilla.org/integration/mozilla-inbound/rev/870b30638837

This was partially backed out, reopening. (Commit message didn't match m-cMerge regexes, hence still being closed after backout; though not sure there's much that could be done differently to avoid it really).

> though not sure there's much that could be done differently > to avoid it really). Other than [leave open] being put in the whiteboard post-partial-backout, I guess.

This is green on try. A tree-wide automated check revealed that (once bug 780824 is fixed): - 305 directories default to libs - 7 default to check (!) - 2 default to export - 1 defaults to installer (browser/installer/windows, in the patch) - 2 default to publish (in nspr, so it doesn't matter) - 1 defaults to full-update (tools/update-packaging, in the patch) - 2 default to all - 1 defaults to test_bug292789.html - 1 defaults to asm_com_offsets.asm - 1 defaults to ISimpleDOMNode.h I don't think any of those can suffer from a change of the default target to be 'default', besides the two changed in this patch (both required to turn green).

Using ?= for DEFAULT_GOAL is wrong. Per the GNU make manual: .DEFAULT_GOAL Sets the default goal to be used if no targets were specified on the command line (see Arguments to Specify the Goals). The .DEFAULT_GOAL variable allows you to discover the current default goal, restart the default goal selection algorithm by clearing its value, or to explicitly set the default goal. The following example illustrates these cases: # Query the default goal. ifeq ($(.DEFAULT_GOAL),) $(warning no default goal is set) endif .PHONY: foo foo: ; @echo $@ $(warning default goal is $(.DEFAULT_GOAL)) # Reset the default goal. .DEFAULT_GOAL := .PHONY: bar bar: ; @echo $@ $(warning default goal is $(.DEFAULT_GOAL)) # Set our own. .DEFAULT_GOAL := foo This makefile prints: no default goal is set default goal is foo default goal is bar foo It appears GNU make sets .DEFAULT_GOAL automatically when encountering a target if there is no current value to .DEFAULT_GOAL. We should be using DEFAULT_GOAL := default and we should consider fixing pymake to behave like GNU make.

For whoever comes first.

Comment on attachment 745498 [details] [diff] [review] Set .DEFAULT_GOAL unconditionally, override with OVERRIDE_DEFAULT_GOAL, and fix pymake to be on par with GNU make when handling .DEFAULT_GOAL. Review of attachment 745498 [details] [diff] [review]: ----------------------------------------------------------------- Works for me!

Can we check this in please?

I had forgotten to hg add this test.