If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Sunbird Trunk fails to build (Error: Cannot open include file nsBuildID.h)

VERIFIED FIXED in 0.7

Status

Calendar
Sunbird Only
--
blocker
VERIFIED FIXED
10 years ago
10 years ago

People

(Reporter: Stefan Sitter, Assigned: Stefan Sitter)

Tracking

Details

(URL)

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

10 years ago
Sunbird Trunk fails to build since 2007-07-03 03:00 PDT

Log file shows: mozilla/calendar/sunbird/app/nsCalendarApp.cpp(45) : fatal error C1083: Cannot open include file: 'nsBuildID.h': No such file or directory

This seems to be caused by Bug 383167. Sunbird requires the same changes to make it work again.

Comment 1

10 years ago
Is this similar to bug #386653 ?
(Assignee)

Comment 2

10 years ago
Created attachment 271376 [details] [diff] [review]
rev0 - Port browser and mail changes

Patch is for TRUNK only and ports the changes made to browser and mail in Bug 383167. I can successfully build and run Sunbird on Windows 2000 system. Also fixes the issue with the now empty BuildID in Lightning.
Assignee: nobody → ssitter
Status: NEW → ASSIGNED
Attachment #271376 - Flags: review?(lilmatt)
(Assignee)

Comment 3

10 years ago
Comment on attachment 271376 [details] [diff] [review]
rev0 - Port browser and mail changes

I'd also like to get a look from Benjamin because he wrote the original patch for browser and mail.
Attachment #271376 - Flags: review?(benjamin)

Comment 4

10 years ago
(In reply to comment #1)
> Is this similar to bug #386653 ?

It appears this is the Sunbird-specific version of that Camino-specific bug, yes.
Created attachment 271700 [details] [diff] [review]
rev1 - Port browser and mail changes

Minor change to add changes from bug 386658.
Attachment #271376 - Attachment is obsolete: true
Attachment #271700 - Flags: review?(lilmatt)
Attachment #271376 - Flags: review?(lilmatt)
Attachment #271376 - Flags: review?(benjamin)
Attachment #271700 - Flags: review?(benjamin)
(Assignee)

Comment 6

10 years ago
Increasing severity because this blocks development and testing work on Trunk.
Severity: normal → blocker
Comment on attachment 271700 [details] [diff] [review]
rev1 - Port browser and mail changes

patch looks fine to me (and works fine, too!)
But it looks trunk-only. Is there a way to make it work on branch too, thus preventing a fork of the files?

Updated

10 years ago
Attachment #271700 - Flags: review?(benjamin) → review+
(Assignee)

Comment 8

10 years ago
(In reply to comment #7)
Yes, this patch is Trunk only. If requested I could try to provide a patch for Trunk and MOZILLA_1_8_BRANCH. But I worry about the files (especially the make files) being too crowded and unreadable afterwards.
At least packages-static should not be forked. That file looks easy enough to add an #ifdef to, and we change that file all the time. Having it forked will be a pain later on.
For the other files, i'm not sure. They indeed look like they will become somewhat unreadable by adding a lot of #ifdef's...
(Assignee)

Comment 10

10 years ago
Created attachment 272512 [details] [diff] [review]
rev2 - patch for TRUNK only

This is the TRUNK only part of the rev1 patch containing the changes for the Sunbird specific files. This files are going to be changed not very often so it should be OK to fork this files.
Attachment #272512 - Flags: review?(mvl)
(Assignee)

Comment 11

10 years ago
Created attachment 272513 [details] [diff] [review]
rev2 - patch for TRUNK and MOZILLA_1_8_BRANCH

This is the TRUNK and MOZILLA_1_8_BRANCH part of the rev1 patch containing the changes for Lightning and packages-static to avoid forking this files.
Attachment #272513 - Flags: review?(mvl)
(Assignee)

Comment 12

10 years ago
Comment on attachment 271700 [details] [diff] [review]
rev1 - Port browser and mail changes

Canceling review due to the follow up patches to avoid forking if possible.
Attachment #271700 - Attachment is obsolete: true
Attachment #271700 - Flags: review?(lilmatt)

Updated

10 years ago
Duplicate of this bug: 389798
(Assignee)

Comment 14

10 years ago
The Trunk is now busted since almost one month. mvl, any chance to do the review in reasonable time-frame? Otherwise I would move the review request to daniel or ctalbert.
I can confirm the patches work well on my MacOSX trunk build, although I am wondering why we shouldn't rework the "TRUNK only" patch to use ifdefs, too, and keep trunk and branch in sync.
One thing I am experiencing is that I can start sunbird only the first time; any further start reminds me that "a copy of sunbird is already open" (seems to be a stale lock file). Possibly some other trunk bug, I assume.
I'm not sure about the need for keeping trunk and branch in sync. After all, they are split in cvs anyway, so why try to duplicated that cvs functionality into preprocessed code? The code is not functionality, but just to keep things running. I think we can get away with forking.
But I'll leave that final decision to Daniel. If the decision is to not fork, you should just wrap some larger blocks in an ifdef. I think that's doable.

Besides that, the code looks fine.
When we switch Trunk from Talkback to Breakpad, we'll have many Trunk only changes in the calendar/installer/ directory. In my opinion we should at least fork the whole installer directory (at the moment it's forked and/or ifdefed depending on the file).
(In reply to comment #16)
> I'm not sure about the need for keeping trunk and branch in sync. After all,
> they are split in cvs anyway, so why try to duplicated that cvs functionality
> into preprocessed code? The code is not functionality, but just to keep things
> running. I think we can get away with forking.
I don't know if I get your point, but in general I think it's sensible to keep trunk and branch in sync since it makes code changes easier (and you could rely on almost the same program logic).
Although I don't feel in the position to judge on this (because I mainly concentrate on the branch), my attitude is that as long as branch and trunk don't diverge too much, but only have slight differences, it makes sense to use ifdefs and the like if it doesn't hurt the overall readability. But if files do diverge too much (nsCalendarApp.cpp, coming breakpad changes?), it makes of course no sense. Regarding diverging code files a viable option may be abstracting functionality into an internal API (implemented separately for trunk and branch). Summed up, I propose to decide on a per file basis with respect to *readability*. Just my 2 cents...
(Assignee)

Comment 19

10 years ago
(In reply to comment #15 - comment #18)

I split up the patch with regard to readability and number of expected changes, see Comment #10 and Comment #11.
(In reply to comment #19)
> (In reply to comment #15 - comment #18)
> 
> I split up the patch with regard to readability and number of expected changes,
> see Comment #10 and Comment #11.
> 

No doubt about that, no offense intended really. I just wanted to know about the general opinion when handling branch vs trunk.
I agree with daniel in comment 18. I don't think we should force huge chunks of code to go into an ifdef, especially when the file won't be touched much (like the startup code). The difference between trunk and branch can be handled perfectly fine by cvs, we don't need to try to do that always on our own with ifdef.
But in some cases, it is indeed easier to use those ifdefs, so patches are easier to checkin.
Comment on attachment 272512 [details] [diff] [review]
rev2 - patch for TRUNK only

r=mvl
Attachment #272512 - Flags: review?(mvl) → review+
Comment on attachment 272513 [details] [diff] [review]
rev2 - patch for TRUNK and MOZILLA_1_8_BRANCH

r=mvl
Attachment #272513 - Flags: review?(mvl) → review+
(Assignee)

Updated

10 years ago
Keywords: checkin-needed
Checked in on HEAD resp. HEAD and MOZILLA_1_8_BRANCH.
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 0.7
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.