Closed
Bug 440017
Opened 16 years ago
Closed 16 years ago
include config.mk before using INSTALL_LIGHTNING so that var can be set
Categories
(Calendar :: Build Config, defect)
Calendar
Build Config
Tracking
(Not tracked)
RESOLVED
FIXED
0.9
People
(Reporter: kairo, Assigned: kairo)
References
Details
Attachments
(1 file, 2 obsolete files)
2.11 KB,
patch
|
ause
:
review+
|
Details | Diff | Splinter Review |
bug 406441 egalized bug 349870 by requiring an INSTALL_LIGHTNING variable to be set to preinstall Lighting into Thunderbird (or SeaMonkey) at build time, but one cannot set this variable, e.g. through myconfig.mk or app-config.mk right now, as config.mk has not been imported when that ifdef is being evaluated.
Assignee | ||
Comment 1•16 years ago
|
||
Here's a patch to include config.mk right before that ifdef, so myconfig or app-config can set this var.
Comment 2•16 years ago
|
||
Comment on attachment 325572 [details] [diff] [review] patch: include config.mk Sorry, I don't have time for reviews.
Attachment #325572 -
Flags: review?(mvl)
Assignee | ||
Updated•16 years ago
|
Attachment #325572 -
Flags: review?(daniel.boelzle)
Updated•16 years ago
|
Attachment #325572 -
Flags: review?(daniel.boelzle) → review?(ause)
Comment 3•16 years ago
|
||
I propose a different fix. I believe that the default introduced by bug 406441 is wrong: every developer working on lightning in-tree needs to figure out why their build isn't getting installed and then set it by hand, which is just an extra barrier to entry. There are only ever a small, limited number of Tinderboxen, though. So I propose to change from |ifdef INSTALL_LIGHTNING| to |ifndef DISABLE_LIGHTNING_INSTALL| and just set that variable in the .mozconfig as an env var in the various tinderboxen.
Comment 4•16 years ago
|
||
Attachment #329503 -
Flags: review?
Updated•16 years ago
|
Attachment #329503 -
Flags: review? → review?(ause)
i tend to prefer the second patch. as far as i see it, the future default will be to have lightning preinstalled. if that's right, it shouldn't require setting anything. to have config.mk included early in a makefile may open up some nice oportunities, but may require moving/copying the block setting MOZILLA_DIR from rules.mk to config.mk (it's used there). looks like someone already did similar for topsrcdir in the past.
Assignee | ||
Comment 6•16 years ago
|
||
Actually, I'm quite happy with dmose's approach. We even could add both patches if we want people to be able to turn it off via myconfig.mk, but I'll leave the decision on what to take and not up to ause ;-)
Assignee | ||
Comment 7•16 years ago
|
||
Per IRC talk, I'm attaching this combined patch of mine and dmose's, so users (or build machines?) have the chance of disabling the Lightning install with myconfig.mk. The most significant change is still dmose's, so leaving his addition to the contibutors section.
Attachment #325572 -
Attachment is obsolete: true
Attachment #329503 -
Attachment is obsolete: true
Attachment #330247 -
Flags: review?
Attachment #325572 -
Flags: review?(ause)
Attachment #329503 -
Flags: review?(ause)
Assignee | ||
Updated•16 years ago
|
Attachment #330247 -
Flags: review? → review?(ause)
Attachment #330247 -
Flags: review?(ause) → review+
Assignee | ||
Comment 8•16 years ago
|
||
Thanks, cross-committed to cvs trunk and 1.8 branch.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Updated•16 years ago
|
Target Milestone: --- → 0.9
Version: Trunk → unspecified
Comment 9•16 years ago
|
||
The checkins for this bug seem to be the only ones in the last days that might be the causer of Bug 446170.
Assignee | ||
Comment 10•16 years ago
|
||
(In reply to comment #9) > The checkins for this bug seem to be the only ones in the last days that might > be the causer of Bug 446170. Thanks, see for my comments on what the real problem is and how to solve it there.
Blocks: 446170
You need to log in
before you can comment on or make changes to this bug.
Description
•