The default bug view has changed. See this FAQ.

Ensure that default is always the default target

RESOLVED FIXED in mozilla17

Status

()

Core
Build Config
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: ted, Assigned: glandium)

Tracking

Trunk
mozilla17
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments)

(Reporter)

Description

5 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.
(Assignee)

Comment 1

5 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.
(Assignee)

Comment 2

5 years ago
Created attachment 649198 [details] [diff] [review]
Ensure that default is always the default target
Attachment #649198 - Flags: review?(ted.mielczarek)
(Assignee)

Updated

5 years ago
Assignee: nobody → mh+mozilla
(Reporter)

Comment 3

5 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!
Attachment #649198 - Flags: review?(ted.mielczarek) → review+
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
(Assignee)

Comment 5

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/1f4eae8d9e08
http://hg.mozilla.org/users/bsmedberg_mozilla.com/pymake/rev/b083524b642c
Target Milestone: --- → mozilla17
(Assignee)

Comment 6

5 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.
(Assignee)

Comment 7

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/7bc9fc61d58f
http://hg.mozilla.org/users/bsmedberg_mozilla.com/pymake/rev/9c2d6ceb8005
(Assignee)

Comment 8

5 years ago
Backed out the rules.mk parts (the pymake parts can stay in)
https://hg.mozilla.org/integration/mozilla-inbound/rev/870b30638837
https://hg.mozilla.org/mozilla-central/rev/7bc9fc61d58f
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
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).
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
> 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.
(Assignee)

Comment 12

5 years ago
Created attachment 649556 [details] [diff] [review]
Ensure that default is always the default target


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).
Attachment #649556 - Flags: review?(ted.mielczarek)
(Reporter)

Updated

5 years ago
Attachment #649556 - Flags: review?(ted.mielczarek) → review+
(Assignee)

Comment 13

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/0aff1ee2a4fd
https://hg.mozilla.org/mozilla-central/rev/0aff1ee2a4fd
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 16

4 years ago
Created 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.

For whoever comes first.
Attachment #745498 - Flags: review?(ted)
Attachment #745498 - Flags: review?(gps)
(Assignee)

Comment 17

4 years ago
https://tbpl.mozilla.org/?tree=Try&rev=2652b6bcc450
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!
Attachment #745498 - Flags: review?(ted)
Attachment #745498 - Flags: review?(gps)
Attachment #745498 - Flags: review+
Can we check this in please?
(Assignee)

Comment 20

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/dc4345aeb64c
(Assignee)

Comment 21

4 years ago
http://hg.mozilla.org/users/bsmedberg_mozilla.com/pymake/rev/779f31e200a5
(Assignee)

Comment 22

4 years ago
Created attachment 745625 [details] [diff] [review]
Additional .DEFAULT_GOAL test for pymake

I had forgotten to hg add this test.
Attachment #745625 - Flags: review?(gps)

Updated

4 years ago
Attachment #745625 - Flags: review?(gps) → review+
https://hg.mozilla.org/mozilla-central/rev/dc4345aeb64c
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago4 years ago
Resolution: --- → FIXED
(Assignee)

Comment 24

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/ac70913833fd
(Assignee)

Comment 25

4 years ago
http://hg.mozilla.org/users/bsmedberg_mozilla.com/pymake/rev/45f730e2f82f
https://hg.mozilla.org/mozilla-central/rev/ac70913833fd
You need to log in before you can comment on or make changes to this bug.