default makefile which calls ./mach build

RESOLVED FIXED in Firefox 38

Status

()

Core
Build Config
RESOLVED FIXED
5 years ago
3 years ago

People

(Reporter: rillian, Assigned: rillian)

Tracking

Trunk
mozilla38
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(firefox38 fixed)

Details

Attachments

(1 attachment, 3 obsolete attachments)

In a blog post, :gps wrote:

> Anyway, this all means that you may want to start re-training your
> muscle memory now. Stop typing make and start typing mach. [...]
> I want to condition people to stop typing make...

Oh dear. See, as a new contributor, one thing that drove me up the wall was having to remember to type 'make -f client.mk' instead of the normal '(./configure &&) make' Having to read the docs to learn about the custom build command isn't really any better as an initial experience, even if the learning curve is much nicer after that. Furthermore, new contributors aren't necessarily going to be spending all their time on Firefox; so they'll be used to typing make, and still need to do so elsewhere.

What about adding a top-level Makefile (or GNUMakefile if we want to avoid conflict with the autoconf output) which invokes ./mach build for naive users?

Comment 1

5 years ago
I totally see where you are coming from here.

I'd support a Makefile in the top directory that either invoked mach/client.mk or printed a helpful error message.

Alternatively, we could adjust the README.txt to refer to mach/client.mk and move some of the files out of the top directory. That way, it's pretty obvious that whatever is in the main directory is important and worth paying attention to. Currently, there's a lot there to distract you (especially configure.in and Makefile.in).
Component: mach → Build Config
(Assignee)

Comment 2

5 years ago
I'd be in favour of both, but I'll take either. :)
(Assignee)

Comment 3

4 years ago
Created attachment 821316 [details]
Example GNUmakefile

I've been using this for the past several months as a top-level 'GNUmakefile' so typing 'make' invokes './mach build'. It doesn't seem to interfere with anything in the current build.

Updated

4 years ago
Attachment #821316 - Attachment mime type: text/x-makefile → text/plain
(Assignee)

Comment 4

3 years ago
Created attachment 8560589 [details] [diff] [review]
0001-Bug-794723-Add-a-default-makefile-which-wraps-mach.-.patch

I've been using this over the past two years without ill effect. Added a 'clean' target since the last attached version.
Assignee: nobody → giles
Attachment #821316 - Attachment is obsolete: true
Attachment #8560589 - Flags: review?(ted)
(Assignee)

Comment 5

3 years ago
Comment on attachment 8560589 [details] [diff] [review]
0001-Bug-794723-Add-a-default-makefile-which-wraps-mach.-.patch

Adding gps for mach integration review.
Attachment #8560589 - Flags: review?(gps)
Comment on attachment 8560589 [details] [diff] [review]
0001-Bug-794723-Add-a-default-makefile-which-wraps-mach.-.patch

This just wants gps' review.
Attachment #8560589 - Flags: review?(ted)

Comment 7

3 years ago
Comment on attachment 8560589 [details] [diff] [review]
0001-Bug-794723-Add-a-default-makefile-which-wraps-mach.-.patch

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

WFM!

::: GNUmakefile
@@ +1,1 @@
> +firefox:

make this "build"

Let's also add a comment to this file:

# This Makefile is used as a shim to aid people with muscle memory so that they
# can type "make".
#
# This file and all of its targets should not be used by anything important.
Attachment #8560589 - Flags: review?(gps) → review+
(Assignee)

Comment 8

3 years ago
Created attachment 8563118 [details] [diff] [review]
default makefile v2

'build' doesn't work because we have a directory of that name, so make thinks there's nothing to do. I used 'all' as the default target, which is more common makefile practice. Carrying forward r=gps.
Attachment #8560589 - Attachment is obsolete: true
Comment on attachment 8563118 [details] [diff] [review]
default makefile v2

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

::: GNUmakefile
@@ +8,5 @@
> +
> +clean:
> +	./mach clobber
> +
> +.PHONEY: all clean

PHONY, not PHONEY.
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
Created attachment 8563128 [details] [diff] [review]
default makefile v3

Thanks, glandium! I made both 'build' and 'all' work as targets.
Attachment #8563118 - Attachment is obsolete: true
https://hg.mozilla.org/integration/mozilla-inbound/rev/07479758ab68
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/07479758ab68
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox38: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in before you can comment on or make changes to this bug.