Closed Bug 455869 Opened 13 years ago Closed 13 years ago

Remove MOZILLA_1_8_BRANCH ifdefs from calendar trunk code

Categories

(Calendar :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: mschroeder, Assigned: mschroeder)

References

()

Details

Attachments

(1 file, 3 obsolete files)

No description provided.
Attached patch WIP patch (obsolete) — Splinter Review
Assignee: nobody → mschroeder
Status: NEW → ASSIGNED
Remember to remove the preprocessing mark in the jar.mn for files that no longer require preprocessing (i.e possibly customizeToolbar.xul)
Attached patch Patch v1 (obsolete) — Splinter Review
Removed ifdefs, isBranch() and its calls, pre-processing in 'jar.mn's, superfluous XP_MACOSX ifdef from resources/jar.mn, and customizeToolbar.xul.
Attachment #339756 - Attachment is obsolete: true
Attachment #340568 - Flags: review?
Comment on attachment 340568 [details] [diff] [review]
Patch v1

>         <!-- Sunbird; we define an upper limit, because we come
>              with a top-notch timezones.xpi on every release -->
>         <em:id>{718e30fb-e89b-41dd-9da7-e25a45638b28}</em:id>
>-#ifdef MOZILLA_1_8_BRANCH
>-        <em:minVersion>0.9pre</em:minVersion>
>-#else
>         <em:minVersion>@CALENDAR_VERSION@</em:minVersion>
>-#endif

I think this needs to be 0.9pre rather than @CALENDAR_VERSION@.

Moreover you can take calItipEmailTransport.js from EXTRA_PP_COMPONENTS.

Aside from that, the patch looks good. Haven't tested it, though...
Attachment #340568 - Flags: review? → review?(daniel.boelzle)
Attached patch Patch v2 (obsolete) — Splinter Review
Incorporated Daniel's suggestions into the patch.
Attachment #340568 - Attachment is obsolete: true
Attachment #340572 - Flags: review?(daniel.boelzle)
Attachment #340568 - Flags: review?(daniel.boelzle)
(In reply to comment #4)
> I think this needs to be 0.9pre rather than @CALENDAR_VERSION@.

0.9pre is correct but requires fixing Bug 455868 first. 
Otherwise you end up with min=0.9pre and max=0.6a1.
Comment on attachment 340572 [details] [diff] [review]
Patch v2

I think you got me wrong. You need to change EXTRA_PP_COMPONENTS to just EXTRA_COMPONENTS for calItipEmailTransport.js. Otherwise the email transport component is missing.

path works for me then; r=dbo with that changed

Requesting a 2nd look from Philipp.
Attachment #340572 - Flags: review?(philipp)
Attachment #340572 - Flags: review?(daniel.boelzle)
Attachment #340572 - Flags: review+
Attached patch Patch v3Splinter Review
Removing the superfluous #ifdef XP_MACOSX from resources/jar.mn requires a Makefile change to have the THEME variable available. I also added the calItipEmailTransport.js to the Makefile (again).
Attachment #340572 - Attachment is obsolete: true
Attachment #340572 - Flags: review?(philipp)
Attachment #340621 - Flags: review?(philipp)
Comment on attachment 340621 [details] [diff] [review]
Patch v3



>diff --git a/calendar/installer/removed-files.in b/calendar/installer/removed-files.in
>--- a/calendar/installer/removed-files.in
>+++ b/calendar/installer/removed-files.in
>@@ -253,26 +253,17 @@ res/viewer.properties
>-#ifndef MOZILLA_1_8_BRANCH
> xpcom_compat.dll
>-#else
>-components/nsInterfaceInfoToIDL.js
>-components/pluginGlue.js
>-Microsoft.VC80.CRT.manifest
>-msvcm80.dll
>-msvcp80.dll
>-msvcr80.dll
>-#endif
I realize this was that way before, but what happens on an upgrade from thunderbird 2 to thunderbird 3, with a subsequent upgrade of Lightning? The branch files are never removed, right?


>diff --git a/calendar/providers/gdata/install.rdf b/calendar/providers/gdata/install.rdf
>--- a/calendar/providers/gdata/install.rdf
>+++ b/calendar/providers/gdata/install.rdf
I'm planning a gdata-0.5.1 release to fix some bugs that noone found during the rc phase, please leave this file intact for now.



r=philipp
Attachment #340621 - Flags: review?(philipp) → review+
(In reply to comment #9)
> (From update of attachment 340621 [details] [diff] [review])
> >diff --git a/calendar/installer/removed-files.in b/calendar/installer/removed-files.in
> >--- a/calendar/installer/removed-files.in
> >+++ b/calendar/installer/removed-files.in
> >@@ -253,26 +253,17 @@ res/viewer.properties
> >-#ifndef MOZILLA_1_8_BRANCH
> > xpcom_compat.dll
> >-#else
> >-components/nsInterfaceInfoToIDL.js
> >-components/pluginGlue.js
> >-Microsoft.VC80.CRT.manifest
> >-msvcm80.dll
> >-msvcp80.dll
> >-msvcr80.dll
> >-#endif
> I realize this was that way before, but what happens on an upgrade from
> thunderbird 2 to thunderbird 3, with a subsequent upgrade of Lightning? The
> branch files are never removed, right?

Afaik those files are only used by the Sunbird build process.

> >diff --git a/calendar/providers/gdata/install.rdf b/calendar/providers/gdata/install.rdf
> >--- a/calendar/providers/gdata/install.rdf
> >+++ b/calendar/providers/gdata/install.rdf
> I'm planning a gdata-0.5.1 release to fix some bugs that noone found during the
> rc phase, please leave this file intact for now.

Will leave this as is.
Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/289b6533a62d>

-> FIXED

Philipp, I suppose you'll remove the ifdef in the gdata provider code yourself, right? :)
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.0
Yes, I'll do so :-)
(In reply to comment #10)
> (In reply to comment #9)
> > (From update of attachment 340621 [details] [diff] [review] [details])
> > >diff --git a/calendar/installer/removed-files.in b/calendar/installer/removed-files.in
> > >--- a/calendar/installer/removed-files.in
> > >+++ b/calendar/installer/removed-files.in
> > >@@ -253,26 +253,17 @@ res/viewer.properties
> > >-#ifndef MOZILLA_1_8_BRANCH
> > > xpcom_compat.dll
> > >-#else
> > >-components/nsInterfaceInfoToIDL.js
> > >-components/pluginGlue.js
> > >-Microsoft.VC80.CRT.manifest
> > >-msvcm80.dll
> > >-msvcp80.dll
> > >-msvcr80.dll
> > >-#endif
> > I realize this was that way before, but what happens on an upgrade from
> > thunderbird 2 to thunderbird 3, with a subsequent upgrade of Lightning? The
> > branch files are never removed, right?
> 
> Afaik those files are only used by the Sunbird build process.

Right, but what happens for Sunbird upgrades 0.9 -> 1.0? Are those files removed then? However, I assume there are more files that need to be removed on branch->trunk upgrade, so I think we simply need to test that scenario to find it out.
Checked in lightning and sunbird build 20090128 -> VERIFIED.
Status: RESOLVED → VERIFIED
These bugs are likely targeted at Lightning 1.0b1, not Lightning 1.0. If this change was done in error, please adjust the target milestone to its correct value. To filter on this bugspam, you can use "lightning-10-target-move".
Target Milestone: 1.0 → 1.0b1
You need to log in before you can comment on or make changes to this bug.