Closed Bug 986411 Opened 8 years ago Closed 8 years ago

Add dumbmake-dependencies entries for easier Instantbird incremental builds.

Categories

(Instantbird :: Other, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: aleth, Assigned: aleth)

Details

Attachments

(1 file, 1 obsolete file)

Attached patch dumbmake.patch (obsolete) — Splinter Review
Because build/dumbmake-dependencies is missing, "mach build <path>" doesn't work for incremental builds. Adding an empty dumbmake-dependencies unbreaks it.

Here I've also taken a first stab at including some rules for Instantbird which ensure that im/app is always built at the end and that "mach build im" also builds /chat. 

Obviously this will not survive ccrework but it seems worth trying this out now before adding to the m-c dumbmake-dependencies later.
Attachment #8394708 - Flags: feedback?(florian)
Attachment #8394708 - Flags: feedback?(clokep)
Attachment #8394708 - Flags: feedback?(Pidgeot18)
Comment on attachment 8394708 [details] [diff] [review]
dumbmake.patch

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

My impression of dumbmake is that it was going away soon, so I can't comment on how worthwhile this ultimately is.
Attachment #8394708 - Flags: feedback?(Pidgeot18) → feedback?(gps)
Comment on attachment 8394708 [details] [diff] [review]
dumbmake.patch

Seems reasonable to me, but I know nothing about whether dumbmake is going away soon, or if cc-rework is landing soon enough to make this irrelevant.
Attachment #8394708 - Flags: feedback?(florian) → feedback+
Comment on attachment 8394708 [details] [diff] [review]
dumbmake.patch

Pretty much echoing Florian's sentiments: it seems to do what we want, but I don't know if it's worth adding?
Attachment #8394708 - Flags: feedback?(clokep) → feedback+
Comment on attachment 8394708 [details] [diff] [review]
dumbmake.patch

As ccrework has been going to happen "any time now" since the end of January, I think we might as well take this and see how it goes.
Attachment #8394708 - Flags: feedback?(gps) → review?(florian)
Comment on attachment 8394708 [details] [diff] [review]
dumbmake.patch

I don't think I can r+ a patch for /build. You likely want a review from a build peer.
Attachment #8394708 - Flags: review?(florian)
Attachment #8394708 - Flags: review?(gps)
Attachment #8394708 - Flags: review?(gps) → review+
No changes, just fixed the commit message. r+ carried forward.
Attachment #8394708 - Attachment is obsolete: true
Attachment #8412582 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/936dcea1fd31
Status: NEW → RESOLVED
Closed: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 1.6
Turns out check-sync-dirs didn't think this patch was as trivial as aleth did. Added an exception.
https://hg.mozilla.org/comm-central/rev/b4015729ed24

Not sure why this wouldn't have been caught during local testing either. Do we not run check-sync-dirs during a normal build?
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #8)

Heh, I must have jinxed it... thanks for fixing the issue!

> Not sure why this wouldn't have been caught during local testing either. Do
> we not run check-sync-dirs during a normal build?

I didn't notice any problems locally. Is this something that needs investigating?
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #8)
> Not sure why this wouldn't have been caught during local testing either. Do
> we not run check-sync-dirs during a normal build?

We run make check-sync-dirs during make check. I don't think very many people run make check locally...
You need to log in before you can comment on or make changes to this bug.