Stop packaging 'about' redirectors that bug 728478 merged

VERIFIED FIXED in seamonkey2.11

Status

defect
P1
blocker
VERIFIED FIXED
7 years ago
7 years ago

People

(Reporter: sgautherie, Assigned: sgautherie)

Tracking

({regression})

Dependency tree / graph
Bug Flags:
in-testsuite -

SeaMonkey Tracking Flags

(seamonkey2.9 fixed, seamonkey2.10 verified)

Details

Attachments

(2 attachments)

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1331696437.1331696979.30012.gz&fulltext=1
Linux x86-64 comm-central-trunk build on 2012/03/13 20:40:37
{
Error: package error or possible missing or unnecessary file: bin/components/nsAboutCertError.js (package-manifest, 245).
Error: package error or possible missing or unnecessary file: bin/components/nsAboutData.js (package-manifest, 246).
Error: package error or possible missing or unnecessary file: bin/components/nsAboutFeeds.js (package-manifest, 247).
Error: package error or possible missing or unnecessary file: bin/components/nsAboutLife.js (package-manifest, 248).
Error: package error or possible missing or unnecessary file: bin/components/nsAboutRights.js (package-manifest, 249).
Error: package error or possible missing or unnecessary file: bin/components/nsAboutSessionRestore.js (package-manifest, 250).
Error: package error or possible missing or unnecessary file: bin/components/nsAboutSyncTabs.js (package-manifest, 251).
}
Flags: in-testsuite-
Assignee

Updated

7 years ago
Keywords: regression
Summary: Stop packaging redirectors that bug 728478 merged → Stop packaging 'about' redirectors that bug 728478 merged
Assignee

Comment 2

7 years ago
Comment on attachment 605650 [details] [diff] [review]
(Av1) Stop packaging 'about' redirectors that bug 728478 merged
[Checked in: Comment 2 + 13]

http://hg.mozilla.org/comm-central/rev/1175d1774ad9
Attachment #605650 - Attachment description: (Av1) Stop packaging 'about' redirectors that bug 728478 merged → (Av1) Stop packaging 'about' redirectors that bug 728478 merged [Checked in: Comment 2]
Assignee

Updated

7 years ago
Blocks: 576970
Assignee

Comment 4

7 years ago
(In reply to Serge Gautherie (:sgautherie) from comment #2)
> http://hg.mozilla.org/comm-central/rev/1175d1774ad9

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1331698675.1331700519.4210.gz
Linux x86-64 comm-central-trunk build on 2012/03/13 21:17:55

V.Fixed wrt this bustage fix only.
Comment on attachment 605657 [details] [diff] [review]
(Bv1) Remove nsAboutSyncTabs.js, Weave.js and services-sync.js, Reorder components.manifest and suite-l10n.js
[Checked in: Comments 16 & 18 & 19]

I won't get at this until early next week. But since its Sync related, I'll give Jens a chance to get the review/approval earlier.

The syntax of the changes is fine, I just don't know if they are correct/right to remove. Or what branches they legitimately belong on.

You only need 1 review to land this, just to be clear :-)
Attachment #605657 - Flags: review?(jh)
Sorry about that.

Fortunately omni.ja saves us from having to update removed-files.in!
Comment on attachment 605650 [details] [diff] [review]
(Av1) Stop packaging 'about' redirectors that bug 728478 merged
[Checked in: Comment 2 + 13]

r+ on post-landing (which is fine in this case)

a+ assuming the issue does in fact exist on aurora
Attachment #605650 - Flags: review?(bugspam.Callek)
Attachment #605650 - Flags: review+
Attachment #605650 - Flags: approval-mozilla-aurora?
Attachment #605650 - Flags: approval-comm-aurora+
Assignee

Comment 8

7 years ago
(In reply to Justin Wood (:Callek) from comment #5)
> I won't get at this until early next week. But since its Sync related, I'll
> give Jens a chance to get the review/approval earlier.

Note that this is just updating (missed) omni.ja support: Sync feature is unaffected.


(In reply to neil@parkwaycc.co.uk from comment #6)

> Sorry about that.

No problem, that is exactly bug 713134 purpose (from now on).

> Fortunately omni.ja saves us from having to update removed-files.in!

Actually not completely, as disabling omni.ja is supported.
(But that should be handled when files are added, not when removed.)
(In reply to Serge Gautherie (:sgautherie) from comment #8)
> (In reply to Justin Wood (:Callek) from comment #5)
> > I won't get at this until early next week. But since its Sync related, I'll
> > give Jens a chance to get the review/approval earlier.
> 
> Note that this is just updating (missed) omni.ja support: Sync feature is
> unaffected.

Yeah, well, I think I just lost half an hour trying to understand in which way this patch is connected to bug 728478 which is referenced in the summary (and only landed for 2.10, so landing the patch here on comm-beta as requested would make no sense), only to find that it isn't connected at all. :-(

Optimally this would have gone to its own bug, but otherwise it at least deserved an explanatory comment. Or maybe I'm just hypercritical because I don't feel I'm a good reviewer for that file. :-/

IOW, I'd appreciate a pointer to the patch/changeset that made this patch necessary (AFAIU it could be the move to omni.jar, but I lack the time to search and check myself).
Assignee

Comment 10

7 years ago
(In reply to Jens Hatlak (:InvisibleSmiley) from comment #9)

> Yeah, well, I think I just lost half an hour trying to understand in which
> way this patch is connected to bug 728478 which is referenced in the summary
> (and only landed for 2.10, so landing the patch here on comm-beta as
> requested would make no sense), only to find that it isn't connected at all.
> :-(

I disagree: bug 728478 fully removes nsAboutSyncTabs.js...

> Optimally this would have gone to its own bug, but otherwise it at least
> deserved an explanatory comment. Or maybe I'm just hypercritical because I
> don't feel I'm a good reviewer for that file. :-/

I'm sorry but I would rather fix both files in the same bug. (And that's the agreed bug 713134 process.)
I thought my comment 3 would be explicit enough :-| (And I didn't ask/expect you to review this.)
(Yes, I believe you're overreacting :-| Just ask when you feel you're lacking details.)

> IOW, I'd appreciate a pointer to the patch/changeset that made this patch
> necessary (AFAIU it could be the move to omni.jar, but I lack the time to
> search and check myself).

Well, iirc, omni.jar support landed "10 days" before bug 576970.
(Sorry, I'm loath to do the history search again. Anyway, that's the current situation.)
Comment on attachment 605657 [details] [diff] [review]
(Bv1) Remove nsAboutSyncTabs.js, Weave.js and services-sync.js, Reorder components.manifest and suite-l10n.js
[Checked in: Comments 16 & 18 & 19]

(In reply to Serge Gautherie (:sgautherie) from comment #10)
> I disagree: bug 728478 fully removes nsAboutSyncTabs.js...

... but that is/was packaged in omni.ja now, right? Anyway, I'm starting to nit-pick. Never mind.

> (And I didn't ask/expect you to review this.)

So much is true. My fault.

> Just ask when you feel you're lacking details.

I think what I'm really lacking is time and patience (and courtesy at times). Sorry. Carry on.
Attachment #605657 - Flags: review?(jh)
Assignee

Comment 12

7 years ago
(In reply to Jens Hatlak (:InvisibleSmiley) from comment #11)

> ... but that is/was packaged in omni.ja now, right? Anyway, I'm starting to
> nit-pick. Never mind.

Right.

As I understand the current removal support:
*jar -> jar: no worry (same unique file).
 *This is SeaMonkey default.
*jar -> non-jar: no worry (just remove the jar).
 *Maybe a developer can do that locally at times?
*non-jar -> jar: what we are maintaining.
 *Maybe some distros/whatever are/will do this? (That was/is the goal.)
  Or said developer locally reverting to jar...
*non-jar -> non-jar: de facto unmaintained ... unless that was a (recent) overlook.
 *Note that this would be "trivial" to do: just move obsolete files out of the JAR ifdef.

> I think what I'm really lacking is time and patience (and courtesy at
> times). Sorry. Carry on.

No worry: it can happen to anyone...
Assignee

Updated

7 years ago
Keywords: checkin-needed
Whiteboard: [c-n: 1175d1774ad9 to c-a]
Comment on attachment 605650 [details] [diff] [review]
(Av1) Stop packaging 'about' redirectors that bug 728478 merged
[Checked in: Comment 2 + 13]

http://hg.mozilla.org/releases/comm-aurora/rev/5ecd27bd6c75
Attachment #605650 - Attachment description: (Av1) Stop packaging 'about' redirectors that bug 728478 merged [Checked in: Comment 2] → (Av1) Stop packaging 'about' redirectors that bug 728478 merged [Checked in: Comment 2 + 13]
Keywords: checkin-needed
Whiteboard: [c-n: 1175d1774ad9 to c-a]
Assignee

Comment 14

7 years ago
(In reply to Jens Hatlak (:InvisibleSmiley) from comment #13)
> http://hg.mozilla.org/releases/comm-aurora/rev/5ecd27bd6c75

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey-Aurora/1331852676.1331855335.24490.gz&fulltext=1
Linux x86-64 comm-aurora build on 2012/03/15 16:04:36

seamonkey2.10: verified wrt this (would-be) bustage fix only.
Comment on attachment 605657 [details] [diff] [review]
(Bv1) Remove nsAboutSyncTabs.js, Weave.js and services-sync.js, Reorder components.manifest and suite-l10n.js
[Checked in: Comments 16 & 18 & 19]

On glancing I don't get why this is needed on beta. re-request with an explanation if it is.
Attachment #605657 - Flags: review?(bugspam.Callek)
Attachment #605657 - Flags: review+
Attachment #605657 - Flags: approval-comm-beta?
Attachment #605657 - Flags: approval-comm-beta-
Attachment #605657 - Flags: approval-comm-aurora?
Attachment #605657 - Flags: approval-comm-aurora+
Assignee

Comment 16

7 years ago
Comment on attachment 605657 [details] [diff] [review]
(Bv1) Remove nsAboutSyncTabs.js, Weave.js and services-sync.js, Reorder components.manifest and suite-l10n.js
[Checked in: Comments 16 & 18 & 19]

http://hg.mozilla.org/comm-central/rev/aaaf94452b07


(In reply to Justin Wood (:Callek) from comment #15)
> I don't get why this is needed on beta.

Same reason as for central and aurora: files and MOZ_OMNIJAR are already there on beta/2.9.
Attachment #605657 - Attachment description: (Bv1) Remove nsAboutSyncTabs.js, Weave.js and services-sync.js, Reorder components.manifest and suite-l10n.js → (Bv1) Remove nsAboutSyncTabs.js, Weave.js and services-sync.js, Reorder components.manifest and suite-l10n.js [Checked in: Comment 16]
Attachment #605657 - Flags: approval-comm-beta- → approval-comm-beta?
Assignee

Updated

7 years ago
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Assignee

Updated

7 years ago
Whiteboard: [c-n: aaaf94452b07 to c-a]
Assignee

Updated

7 years ago
Keywords: checkin-needed
Assignee

Comment 17

7 years ago
/pub/thunderbird/try-builds/sgautherie.bz@free.fr-27edc56ea7d2/try-comm-central-win32-debug
seamonkey-2.11a1.en-US.win32.zip

V.Fixed
Status: RESOLVED → VERIFIED
Comment on attachment 605657 [details] [diff] [review]
(Bv1) Remove nsAboutSyncTabs.js, Weave.js and services-sync.js, Reorder components.manifest and suite-l10n.js
[Checked in: Comments 16 & 18 & 19]

http://hg.mozilla.org/releases/comm-aurora/rev/63eddfec113c
Attachment #605657 - Attachment description: (Bv1) Remove nsAboutSyncTabs.js, Weave.js and services-sync.js, Reorder components.manifest and suite-l10n.js [Checked in: Comment 16] → (Bv1) Remove nsAboutSyncTabs.js, Weave.js and services-sync.js, Reorder components.manifest and suite-l10n.js [Checked in: Comments 16 and 18]
Keywords: checkin-needed
Whiteboard: [c-n: aaaf94452b07 to c-a]
Attachment #605657 - Flags: approval-comm-beta? → approval-comm-beta+
Assignee

Updated

7 years ago
Whiteboard: [c-n: aaaf94452b07 to c-b]
Assignee

Updated

7 years ago
Attachment #605657 - Attachment description: (Bv1) Remove nsAboutSyncTabs.js, Weave.js and services-sync.js, Reorder components.manifest and suite-l10n.js [Checked in: Comments 16 and 18] → (Bv1) Remove nsAboutSyncTabs.js, Weave.js and services-sync.js, Reorder components.manifest and suite-l10n.js [Checked in: Comments 16 & 18 & 19]
You need to log in before you can comment on or make changes to this bug.