Status

()

Core
Build Config
ASSIGNED
4 years ago
2 years ago

People

(Reporter: gps, Assigned: glandium)

Tracking

(Depends on: 1 bug, Blocks: 1 bug)

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

4 years ago
dumbmake was a nice stop gap. Once the "binaries" target works, dumbmake likely doesn't need to exist any more.

Filing this bug now so it is on the radar and we can track dumbmake bugs against it.
(Assignee)

Comment 1

4 years ago
Let's say that having a MozillaBuild released with mozmake is the trigger that should allow us to remove it.
Depends on: 927213
(Assignee)

Comment 2

4 years ago
Created attachment 818319 [details] [diff] [review]
Remove dumbmake
(Assignee)

Updated

4 years ago
Assignee: nobody → mh+mozilla
Status: NEW → ASSIGNED
(Assignee)

Comment 3

4 years ago
Created attachment 8357463 [details] [diff] [review]
Remove dumbmake

I think it's time to kill it.
Attachment #8357463 - Flags: review?(gps)
(Assignee)

Updated

4 years ago
Attachment #818319 - Attachment is obsolete: true
(Reporter)

Comment 4

4 years ago
I think frontend developers may still rely on it. I agree that the libxul parts aren't needed with the binaries target.

Can you send out an email to at least firefox-dev@mozilla.org and give people the opportunity to object?
Flags: needinfo?(mh+mozilla)

Comment 5

4 years ago
Please ask about this on dev-platform as well.  This affects more than just the front-end developers.
Is it expected that "mach build binaries" doesn't rerun WebIDL codegen?
Flags: needinfo?(gps)
(Assignee)

Comment 7

4 years ago
(In reply to Boris Zbarsky [:bz] from comment #6)
> Is it expected that "mach build binaries" doesn't rerun WebIDL codegen?

dumbmake doesn't either.
Hmm.  "mach build dom/bindings" does (reruns codegen, builds in bindings, relinks libxul).  Is that not using dumbmake?  It sounded from your post to firefox-dev that this is exactly the dumbmake case....

Comment 9

4 years ago
(In reply to comment #7)
> (In reply to Boris Zbarsky [:bz] from comment #6)
> > Is it expected that "mach build binaries" doesn't rerun WebIDL codegen?
> 
> dumbmake doesn't either.

Doesn't dumbmake go through all of the tiers in thoise directory?  I'm 99.99% positive that it does based on my personal experience.
(Reporter)

Comment 10

4 years ago
dumbmake is effectively a glorified expansion of |make -C foo| into |make -C foo && make -C bar && make -C baz|. So, yes, dumbmake should go through all the tiers. If /dom or /dom/bindings is in the expansion, WebIDL codegen should run (if needed) using the regular build system rules.

The "binaries" make target already processes install manifests. I can make a case for it also performing WebIDL, XPIDL, etc codegen as well.

At the end of the day, we can make custom build/make targets do whatever we want. I'm a huge fan of supporting top-level targets to satisfy common developer workflows until no-op and light builds take milliseconds and there is no need for these shortcuts. I'd prefer we do this rather than burden developers with knowledge of tiers, traversal rules, etc (especially since these things tend to change). Either we change "binaries" to do more work (at the cost of making it a little slower), or we add new target(s) that do binaries + other things.
Flags: needinfo?(gps)
(Reporter)

Comment 11

4 years ago
Comment on attachment 8357463 [details] [diff] [review]
Remove dumbmake

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

I am reluctantly granting r+. I have concerns the discussion on removing dumbmake has centered around libxul workflow, where `make binaries` is a sufficient replacement for dumbmake. I worry about the impact on the workflow for browser developers, where dumbmake is relied upon more. I'd really like to see a "binaries" equivalent target that satisfies the needs of "frontend" developers. If we can do that before dumbmake is removed, I'll feel much better.

That being said, I am granting r+. We can always revert if there is backlash.
Attachment #8357463 - Flags: review?(gps) → review+
(Reporter)

Comment 12

3 years ago
Cancelling a 2 year old needinfo.
Flags: needinfo?(mh+mozilla)
(Assignee)

Updated

2 years ago
Duplicate of this bug: 1203866
You need to log in before you can comment on or make changes to this bug.