Closed Bug 1124736 Opened 5 years ago Closed 5 years ago

Move PREF_JS_EXPORTS to moz.build in c-c

Categories

(Thunderbird :: Build Config, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 38.0

People

(Reporter: bokeefe, Assigned: bokeefe)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 3 obsolete files)

Bug 870366 moves them in m-c. We should move them in c-c at around the same time.

I have WIP patches, although they seem to break the stage-package step.
See try (along with some unrelated things): https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=bb1a84e071c3
Mostly just posting this for posterity, seeing as it's breaking stage-package (in particular, with "Error: c:\builds\moz2_slave\tb-try-c-cen-w32-0000000000000\build\objdir-tb\mail\installer\package-manifest:196: Missing file(s): bin/defaults/pref/chat-prefs.js")

I'm not sure why that doesn't work; the generated backend.mk (locally, at least) looks the same as the (removed) Makefile.in. I'm chasing that down rabbit holes.
Attachment #8553170 - Flags: feedback?(Pidgeot18)
> I have WIP patches, although they seem to break the stage-package step.
> See try (along with some unrelated things):
> https://treeherder.mozilla.org/#/jobs?repo=try-comm-
> central&revision=bb1a84e071c3

<https://hg.mozilla.org/try-comm-central/diff/2df52b46e5b9/mailnews/extensions/mailviews/content/Makefile.in>

That change is wrong. The libs:: rules need to come after including rules.mk.

Or you can just port this (and other similar rules) to FINAL_TARGET_FILES, e.g., FINAL_TARGET_FILES.defaults.messenger += ['mailViews.dat']
These serve the same purpose as Part 2b of the m-c bug. It's only needed if PREF_JS_EXPORTS is blacklisted in Makefile.ins.
(In reply to Joshua Cranmer [:jcranmer] from comment #2)
> <https://hg.mozilla.org/try-comm-central/diff/2df52b46e5b9/mailnews/
> extensions/mailviews/content/Makefile.in>
> 
> That change is wrong. The libs:: rules need to come after including rules.mk.
> 
> Or you can just port this (and other similar rules) to FINAL_TARGET_FILES,
> e.g., FINAL_TARGET_FILES.defaults.messenger += ['mailViews.dat']

I suspected I did something like that, so I re-pushed with just these two patches: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=9a3375019182
That failed in the same place. I'll fix the rest of my earlier push soon-ish, in separate bugs.


(In reply to Brian O'Keefe [:bokeefe] from comment #1)
> I'm not sure why that doesn't work; the generated backend.mk (locally, at
> least) looks the same as the (removed) Makefile.in. I'm chasing that down
> rabbit holes.

It just hit me - I forgot to mark the new mozbuild variable as affecting 'libs', so the build system helpfully skips the whole directory. I'll fix the m-c patch, and that should make this better.
Comment on attachment 8553170 [details] [diff] [review]
Move PREF_JS_EXPORTS to moz.build (WIP)

Try was much happier with a fixed m-c patch (at least on linux so far):
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=8d2b3d9118be
Attachment #8553170 - Flags: feedback?(Pidgeot18) → review?(Pidgeot18)
Attachment #8553172 - Flags: review?(Pidgeot18)
Comment on attachment 8553172 [details] [diff] [review]
Move PREF_JS_EXPORTS to moz.build (c-c locale MERGE_FILE hack)

Per bug 870366 comment 32, glandium isn't a fan of this approach. I'll update or obsolete this patch depending on how the updates to that turn out.
Attachment #8553172 - Flags: review?(Pidgeot18)
Same patch, just rebased to tip.

I got r+ for the m-c series, which disallows PREF_JS_EXPORTS in Makefile.ins, so that's going to break c-c temporarily whenever it lands.
Attachment #8557216 - Flags: review?(Pidgeot18)
This was glandium's preferred method of dealing with the MERGE_FILE cases.
Attachment #8553170 - Attachment is obsolete: true
Attachment #8553172 - Attachment is obsolete: true
Attachment #8553170 - Flags: review?(Pidgeot18)
Attachment #8557217 - Flags: review?(Pidgeot18)
Attachment #8557217 - Flags: review?(Pidgeot18) → review+
Comment on attachment 8557216 [details] [diff] [review]
Move PREF_JS_EXPORTS to moz.build

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

Modulo the extraneous deletion below, this is okay.

::: suite/browser/Makefile.in
@@ -19,5 @@
> -	$(srcdir)/channel-prefs.js
> -	$(NULL)
> -endif
> -
> -include $(topsrcdir)/config/rules.mk

Removing this include rule is not okay.
Attachment #8557216 - Flags: review?(Pidgeot18) → review+
(In reply to Joshua Cranmer [:jcranmer] from comment #9)
> ::: suite/browser/Makefile.in
> @@ -19,5 @@
> > -	$(srcdir)/channel-prefs.js
> > -	$(NULL)
> > -endif
> > -
> > -include $(topsrcdir)/config/rules.mk
> 
> Removing this include rule is not okay.

Whoops, good catch. I put that line back.
Attachment #8557216 - Attachment is obsolete: true
Attachment #8557422 - Flags: review+
Keywords: checkin-needed
jcranmer pushed these patches to comm-central
http://hg.mozilla.org/comm-central/rev/338986d73c02
http://hg.mozilla.org/comm-central/rev/1f6763b45c67
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 38.0
You need to log in before you can comment on or make changes to this bug.