(not currently active) Ted Mielczarek Reporter
	Description • 12 years ago

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.

	Mike Hommey [:glandium] Assignee
	Comment 1 • 12 years ago

(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.

	(not currently active) Ted Mielczarek Reporter
	Comment 3 • 12 years ago

Comment on attachment 649198 [details] [diff] [review]
Ensure that default is always the default target

Review of attachment 649198 [details] [diff] [review]:
-----------------------------------------------------------------

Excellent!

	:Ms2ger (he/him; ⌚ UTC+1/+2)
	Comment 4 • 12 years ago

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

	Mike Hommey [:glandium] Assignee
	Comment 5 • 12 years ago

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

	Mike Hommey [:glandium] Assignee
	Comment 6 • 12 years ago

(In reply to :Ms2ger from comment #4)
> > +	flavor, source, value = self.variables.get('.DEFAULT_GOAL')
> 
> Tab

That's interesting... python didn't complain.

	Mike Hommey [:glandium] Assignee
	Comment 7 • 12 years ago

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

	Mike Hommey [:glandium] Assignee
	Comment 8 • 12 years ago

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

	Ryan VanderMeulen [:RyanVM]
	Comment 9 • 12 years ago

https://hg.mozilla.org/mozilla-central/rev/7bc9fc61d58f

	Ed Morley [:emorley]
	Comment 10 • 12 years ago

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).

	Ed Morley [:emorley]
	Comment 11 • 12 years ago

> 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.

	Mike Hommey [:glandium] Assignee
	Comment 12 • 12 years ago

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).

	Mike Hommey [:glandium] Assignee
	Comment 13 • 12 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/0aff1ee2a4fd

	Ed Morley [:emorley]
	Comment 14 • 12 years ago

https://hg.mozilla.org/mozilla-central/rev/0aff1ee2a4fd

	Gregory Szorc [:gps]
	Comment 15 • 11 years ago

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.

	Mike Hommey [:glandium] Assignee
	Comment 16 • 11 years ago

For whoever comes first.

	Mike Hommey [:glandium] Assignee
	Comment 17 • 11 years ago

https://tbpl.mozilla.org/?tree=Try&rev=2652b6bcc450

	Gregory Szorc [:gps]
	Comment 18 • 11 years ago

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!

	(no longer active)
	Comment 19 • 11 years ago

Can we check this in please?

	Mike Hommey [:glandium] Assignee
	Comment 20 • 11 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/dc4345aeb64c

	Mike Hommey [:glandium] Assignee
	Comment 21 • 11 years ago

http://hg.mozilla.org/users/bsmedberg_mozilla.com/pymake/rev/779f31e200a5

	Mike Hommey [:glandium] Assignee
	Comment 22 • 11 years ago

I had forgotten to hg add this test.

	Phil Ringnalda (:philor)
	Comment 23 • 11 years ago

https://hg.mozilla.org/mozilla-central/rev/dc4345aeb64c

	Mike Hommey [:glandium] Assignee
	Comment 24 • 11 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/ac70913833fd

	Mike Hommey [:glandium] Assignee
	Comment 25 • 11 years ago

http://hg.mozilla.org/users/bsmedberg_mozilla.com/pymake/rev/45f730e2f82f

	Ryan VanderMeulen [:RyanVM]
	Comment 26 • 11 years ago

https://hg.mozilla.org/mozilla-central/rev/ac70913833fd