986411 - Add dumbmake-dependencies entries for easier Instantbird incremental builds.

Status: RESOLVED FIXED

(Instantbird Graveyard :: Other, defect)

Description

[aleth [:aleth]]

Attachment: dumbmake.patch

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.

Comment 1

[Joshua Cranmer [:jcranmer]]

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.

Comment 2

[Florian Quèze [:florian]]

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.

Comment 3

[Patrick Cloke [:clokep]]

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?

Comment 4

[aleth [:aleth]]

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.

Comment 5

[Florian Quèze [: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.

Comment 6

[aleth [:aleth]]

Attachment: dumbmake.patch - fixed commit message

No changes, just fixed the commit message. r+ carried forward.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/936dcea1fd31

Comment 8

[Ryan VanderMeulen [:RyanVM]]

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?

Comment 9

[aleth [:aleth]]

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

Comment 10

[Joshua Cranmer [:jcranmer]]

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