Build and l10n rollup for SeaMonkey

RESOLVED FIXED in seamonkey2.59

Status

enhancement
--
blocker
RESOLVED FIXED
a year ago
5 months ago

People

(Reporter: frg, Assigned: frg)

Tracking

(Blocks 3 bugs)

Trunk
seamonkey2.59
Dependency tree / graph

SeaMonkey Tracking Flags

(seamonkey2.57esr fixed, seamonkey2.58 wontfix, seamonkey2.59 fixed)

Details

User Story

Fixes or ports the relevant parts from:
Bug 1392245 - Make l10n repack builds do what they're supposed to do (port bug 1385227); r=Fallen
Bug 1425786 - Create the directory used to pack windows installer; r=me
Bug 1424171 - Pass mozilla dir to compare-locales.
No Bug - Sync TB windows installer makefile with firefox; r=me
Bug 1434667 - Move the thunderbird branding to mail/branding
Bug 1432903 - Fix the icons in the branding jar.mn. r=tomprince DONTBUILD
Bug 1444926 - Rename devtools/shim to devtools/startup
Bug 1426547 - Fix branding to use moz.build;
Bug 1426528 - Stop staging BRANDING_FILES
Bug 1436662 - Package translated uninstaller; r=pike,mshal
Bug 1409721 - Express localized files in moz.build
Bug 1439469 - Stop using INSTALL_EXTENSION_ID to build calendar
Bug 1417334 - Add default64.png and default128.png

Future not in this bug:
bug 1416891 - use LOCALIZED_GENERATED_FILES for updater.ini. r=nalexander 
Bug 1417250 - Make the Start menu icons HiDPI aware and match the icon size of Firefox.

Attachments

(10 attachments, 6 obsolete attachments)

126.33 KB, patch
Details | Diff | Splinter Review
126.48 KB, patch
iann_bugzilla
: review+
iann_bugzilla
: approval-comm-esr60+
Details | Diff | Splinter Review
27.21 KB, patch
iann_bugzilla
: review+
iann_bugzilla
: approval-comm-esr60+
Details | Diff | Splinter Review
256 bytes, application/x-shellscript
Details
1.07 KB, text/plain
Details
51.41 KB, patch
iann_bugzilla
: review+
Details | Diff | Splinter Review
51.23 KB, patch
frg
: review+
iann_bugzilla
: approval-comm-esr60+
Details | Diff | Splinter Review
2.01 KB, patch
frg
: review+
Details | Diff | Splinter Review
8.80 KB, patch
iann_bugzilla
: review+
iann_bugzilla
: approval-comm-esr60+
Details | Diff | Splinter Review
4.59 KB, patch
iann_bugzilla
: review+
iann_bugzilla
: approval-comm-esr60+
Details | Diff | Splinter Review
Assignee

Description

a year ago
Packaging Lightning directly is a good idea and we need to follow.

+++ This bug was initially created as a clone of Bug #1439469 +++

This logic is going away, and it makes the build simpler as well.
Assignee

Comment 1

a year ago
comm-central builds are broken because of recent changes in the mozilla builds files. l10n has changed too. Some of these changes need to be applied to the 2.57 ESR tree too.

It is unlikely that the Windows installer can be fixed for builds which have comm-central as topsourcedir. This would need changes in m-c files. So we need to switch to mozilla as topsourcedir too for 2.58+ (61).
Assignee: nobody → frgrahl
Status: NEW → ASSIGNED
Summary: Stop using INSTALL_EXTENSION_ID to build calendar in SeaMonkey → Build and l10n rollup for SeaMonkey
Assignee

Comment 2

a year ago
Posted patch installer-very-wip.patch (obsolete) — Splinter Review
Allows me to build Windows with mozilla as topsourcedir. This is rough and needs a) to be cleaned up and b) to be split for the branding changes.

Linux and OSX not yet tested.
Assignee

Updated

a year ago
Severity: normal → blocker
Assignee

Updated

a year ago
User Story: (updated)
Assignee

Comment 3

a year ago
Comment on attachment 8970659 [details] [diff] [review]
installer-very-wip.patch

IanN (and everyone who wants to comment on it). Shouldn't we switch branding from nightly to seamonkey? We still need some building fixes for branding but I will split the rename off in a separate part for clarity.
Attachment #8970659 - Flags: feedback?(iann_bugzilla)
Assignee

Updated

a year ago
Blocks: 1362210

Comment 4

a year ago
Comment on attachment 8970659 [details] [diff] [review]
installer-very-wip.patch

Just on patch inspection, no testing as yet.

>+++ b/suite/app/Makefile.in

> DEFINES += \
>-	-DSEAMONKEY_ICO=\"$(DIST)/branding/seamonkey.ico\" \
>-	-DHTML_FILE_ICO=\"$(DIST)/branding/html-file.ico\" \
>+	-DSEAMONKEY_ICO='"$(topsrcdir)/$(MOZ_BRANDING_DIRECTORY)/icons/windows/main-window.ico"' \
>+	-DHTML_FILE_ICO='"$(topsrcdir)/$(MOZ_BRANDING_DIRECTORY)/icons/windows/html-file.ico"' \

>diff --git a/suite/branding/nightly/icons/windows/main-window.ico b/suite/branding/seamonkey/icons/windows/main-window.ico
>rename from suite/branding/nightly/icons/windows/main-window.ico
>rename to suite/branding/seamonkey/icons/windows/main-window.ico

>diff --git a/suite/branding/nightly/icons/windows/seamonkey.ico b/suite/branding/seamonkey/icons/windows/seamonkey.ico
>rename from suite/branding/nightly/icons/windows/seamonkey.ico
>rename to suite/branding/seamonkey/icons/windows/seamonkey.ico

Do we need to keep both of main-window.ico and seamonkey.ico?


>diff --git a/suite/branding/nightly/icons/svg/venkman-window.svg b/suite/branding/nightly/icons/svg/venkman-window.svg
>deleted file mode 100644
Can this deletion be done as a separate patch, as that could be reviewed and landed quickly plus makes this patch more readable?


>diff --git a/suite/branding/seamonkey/default16.png b/suite/branding/seamonkey/default16.png
What is the difference between this and the one under gtk?


>+++ b/suite/locales/Makefile.in

>-ifdef MOZ_CALENDAR
>-	$(if $(filter en-US,$(AB_CD)),, @$(MAKE) -C ../../calendar/lightning merge-$*)
>-endif
> 	$(NSINSTALL) -D $(DIST)/install
> 	@$(MAKE) -C $(DEPTH)/toolkit/locales libs-$*
> #	@$(MAKE) -C $(DEPTH)/services/sync/locales AB_CD=$* XPI_NAME=locale-$*
> 	@$(MAKE) -C ../../editor/ui/locales AB_CD=$* XPI_NAME=locale-$*
>+  @$(MAKE) -C ../../calendar/locales AB_CD=$* XPI_NAME=locale-$*
Nit: indentation
Assignee

Comment 5

a year ago
1.) Seems to fail on Linux and only Windows is affected by the branding changes and fials currently without.

> Do we need to keep both of main-window.ico and seamonkey.ico?

Probably not but they are different. I think seamonkey.ico is only used in splash.rc. Will check and put a possible deletion in a separate  patch.

> Can this deletion be done as a separate patch, as that could be reviewed and
landed quickly plus makes this patch more readable?

Sure logo.gif also seems to be unused. This was all just hacked together to get the Windows build working. I stumbled from one bug I needed to port to the next... until it worked.

> What is the difference between this and the one under gtk?

None and makes the Linux build currently fail. I need to clean it up. fx and TB don't use separate dirs here. I wasn't first sure if there are now some hardcoded paths in toolkit.
Assignee

Comment 6

a year ago
Base patch remove some unused or duplicate icons from SeaMonkey

[Approval Request Comment]
Regression caused by (bug #): --
User impact if declined: groundwork for further patches
Testing completed (on m-c, etc.): c-r 2.53 Windows Linux macOS
Risk to taking this patch (and alternatives if risky): just mop up
String changes made by this patch: --
Attachment #8972255 - Flags: review?(iann_bugzilla)
Attachment #8972255 - Flags: approval-comm-beta?
Assignee

Updated

a year ago
Keywords: leave-open
Assignee

Comment 8

a year ago
Comment on attachment 8970659 [details] [diff] [review]
installer-very-wip.patch

Needs a fixer upper now
Attachment #8970659 - Attachment is obsolete: true
Attachment #8970659 - Flags: feedback?(iann_bugzilla)
Assignee

Comment 9

a year ago
Could someone on a bleeding edge Linux give this a swirl. The build gods decided to make Python 3.5 and other bleeding edge stuff a prerequisite for Linux m-c and my poor CentOS 7 is too old. 

macOS and Windows build. l10n not yet tested. Patch in Bug 1458174 is currently a prerequisite or you won't get far.
Assignee

Comment 10

a year ago
Base patch remove some unused or duplicate icons from SeaMonkey. Comment adjusted. Tomorrow is merge day so waiting with beta-approval and esr apporval after review.
Attachment #8972255 - Attachment is obsolete: true
Attachment #8972255 - Flags: review?(iann_bugzilla)
Attachment #8972255 - Flags: approval-comm-beta?
Attachment #8973518 - Flags: review?(iann_bugzilla)
Assignee

Comment 11

a year ago
Posted patch 1451847-part2-installer.patch (obsolete) — Splinter Review
Tested with Windows x64 including l10n. Only minor adjustments for l10n compared to the previous wip patch so should be good for Linux and macOS.
Attachment #8972404 - Attachment is obsolete: true
Attachment #8973519 - Flags: review?(iann_bugzilla)
Assignee

Comment 12

a year ago
Optional rename of the branding directory. Not needed.
Attachment #8973520 - Flags: review?(iann_bugzilla)
Assignee

Comment 13

a year ago
Posted file .makex6417l10n.sh
Sample build script for Windows. l10n is really easy now. No more merge dirs.

Windows will only build with mozilla as topsourcedir as stated above. Linux and macOS might still work with comm-central as top. Not tested.
Assignee

Comment 14

a year ago
sample config
Assignee

Comment 15

a year ago
Posted patch 1451847-part2-installer.patch (obsolete) — Splinter Review
nitfix trailing spaces in suite/installer/windows/Makefile.in
Attachment #8973519 - Attachment is obsolete: true
Attachment #8973519 - Flags: review?(iann_bugzilla)
Attachment #8973540 - Flags: review?(iann_bugzilla)
Assignee

Comment 16

a year ago
Posted patch 1451847-part2-installer.patch (obsolete) — Splinter Review
default128.png in Linux build was out of sequence. doh
Attachment #8973540 - Attachment is obsolete: true
Attachment #8973540 - Flags: review?(iann_bugzilla)
Assignee

Updated

a year ago
User Story: (updated)
Assignee

Comment 17

a year ago
Needs a new Linux and macOS test but should be good otherwise. I didn't build with IRC, DOMi and debugQA because all three are broken and their l10n repack still not fixed.
Attachment #8973612 - Attachment is obsolete: true
Attachment #8974532 - Flags: review?(iann_bugzilla)
Assignee

Comment 18

a year ago
Version needed for 2.57. Not setting approval till the comm-central part is reviewed.

Basically the same part 2 patch minus:

Bug 1436662 - `uninstall/helper.exe` is no longer translated
Bug 1444926 - Rename devtools/shim to devtools/startup 
Bug 1439469 - Stop using INSTALL_EXTENSION_ID to build calendar.

part 1 and 3 apply unmodified for 2.57esr

2.57 can still be build under Windows with comm-central as topsourcedir.

Updated

a year ago
Attachment #8973518 - Flags: review?(iann_bugzilla) → review+

Comment 19

a year ago
Comment on attachment 8974532 [details] [diff] [review]
1451847-part2-installer-V2.patch

>diff --git a/suite/branding/branding-common.mozbuild b/suite/branding/branding-common.mozbuild
>new file mode 100644
>--- /dev/null
>+++ b/suite/branding/branding-common.mozbuild
Is it worth copying suite/branding/nightly/moz.build and then making changes to that file (without re-indenting the whole file)?

Either way r=me
Attachment #8974532 - Flags: review?(iann_bugzilla) → review+

Comment 20

a year ago
Comment on attachment 8973520 [details] [diff] [review]
1451847-part3-branding.patch

r=me
Attachment #8973520 - Flags: review?(iann_bugzilla) → review+
Assignee

Comment 21

a year ago
> Is it worth copying suite/branding/nightly/moz.build and then making changes to that file 
> (without re-indenting the whole file)?

Just tried but thanks to Python needs the re-indention so no.

Comment 22

a year ago
Pushed by frgrahl@gmx.net:
https://hg.mozilla.org/comm-central/rev/d0490e2f9ca6
Part 1. Remove duplicate or unused icons from SeaMonkey. r=IanN
https://hg.mozilla.org/comm-central/rev/ea4c0d8fc22e
Part 2. May 2018 Installer and l10n rollup for SeaMonkey. r=IanN
https://hg.mozilla.org/comm-central/rev/0d275d681a5d
Part 3. Use seamonkey instead of nightly as branding directory. r=IanN
Assignee

Updated

a year ago
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Keywords: leave-open
Resolution: --- → FIXED
Target Milestone: --- → Future
Assignee

Comment 23

a year ago
Comment on attachment 8973518 [details] [diff] [review]
1451847-part1-iconremove.patch

[Approval Request Comment]
Regression caused by (bug #): see user story
User impact if declined: no 2.57
Testing completed (on m-c, etc.): c-esr60
Risk to taking this patch (and alternatives if risky): build only 
String changes made by this patch: --
Attachment #8973518 - Flags: approval-comm-esr60?
Assignee

Comment 24

a year ago
Comment on attachment 8974533 [details] [diff] [review]
1451847-part2-installer-257.patch

r+ from IanN for comm-central patch carrried over. Because the patch is different (mostly in the installer part) please check when approving if you spot something.

[Approval Request Comment]
Regression caused by (bug #): see user story
User impact if declined: no 2.57
Testing completed (on m-c, etc.): c-esr60
Risk to taking this patch (and alternatives if risky): build only 
String changes made by this patch: --
Attachment #8974533 - Flags: review+
Attachment #8974533 - Flags: approval-comm-esr60?
Assignee

Comment 25

a year ago
Comment on attachment 8973520 [details] [diff] [review]
1451847-part3-branding.patch

[Approval Request Comment]
Regression caused by (bug #): Optional part but should be kept in sync with central for 2.57.
User impact if declined: Future patches might need rebasing.
Testing completed (on m-c, etc.): c-esr60
Risk to taking this patch (and alternatives if risky): build only 
String changes made by this patch: --
Attachment #8973520 - Flags: approval-comm-esr60?

Comment 26

a year ago
Comment on attachment 8973518 [details] [diff] [review]
1451847-part1-iconremove.patch

a=me
Attachment #8973518 - Flags: approval-comm-esr60? → approval-comm-esr60+

Comment 27

a year ago
Comment on attachment 8973520 [details] [diff] [review]
1451847-part3-branding.patch

a=me
Attachment #8973520 - Flags: approval-comm-esr60? → approval-comm-esr60+

Comment 28

a year ago
Comment on attachment 8974533 [details] [diff] [review]
1451847-part2-installer-257.patch

a=me
Attachment #8974533 - Flags: approval-comm-esr60? → approval-comm-esr60+
Assignee

Updated

a year ago
Blocks: 1461767
Assignee

Comment 30

a year ago
Good news everyone. Bug 1436662 has been pushed to esr60 today which means the days of comm as top sourcedir on Windows are over there too. Need to do a 2.57 followup patch only and put the parts back in I removed for it. But not today.
Assignee

Comment 31

a year ago
2 very minor issues found after pushing.
Assignee

Comment 32

a year ago
Backport of bug 1436662. comm-esr60 only

When I just tested this Lightning was not translated. Need to check this first before asking for r? a?

Comment 33

a year ago
Pushed by frgrahl@gmx.net:
https://hg.mozilla.org/comm-central/rev/dd8068d03aa0
Part 4. Correct minor issues in SeaMonkey build files. r=me
Assignee

Comment 34

a year ago
Comment on attachment 8976677 [details] [diff] [review]
1451847-part4-mopup.patch

r=me
Attachment #8976677 - Flags: review+
Assignee

Comment 35

a year ago
Comment on attachment 8976678 [details] [diff] [review]
1451847-part4-mopup-257.patch

This builds. Lightning l10n merge is broken on esr60. It will be tranlated in en-US for all langauges. Verified with a TB build. Filing a followup bug
Attachment #8976678 - Flags: review?(iann_bugzilla)
Attachment #8976678 - Flags: approval-comm-esr60?
Assignee

Updated

a year ago
See Also: → 1462480

Comment 36

a year ago
Comment on attachment 8976678 [details] [diff] [review]
1451847-part4-mopup-257.patch

LGTM r/a=me
Attachment #8976678 - Flags: review?(iann_bugzilla)
Attachment #8976678 - Flags: review+
Attachment #8976678 - Flags: approval-comm-esr60?
Attachment #8976678 - Flags: approval-comm-esr60+
Assignee

Comment 39

11 months ago
Bug 1439469 has been uplifted to TB 60. It is currently only in the comm-beta TB branch.
Attachment #8985959 - Flags: review?(iann_bugzilla)
Attachment #8985959 - Flags: approval-comm-esr60?

Comment 40

11 months ago
Pushed by frgrahl@gmx.net:
https://hg.mozilla.org/comm-central/rev/64aa89f9a7ed
Part 5. Port remaining parts of Bug 1439469 [Package lightning directly as part of thunderbird] to SeaMonkey. r=me

Comment 41

11 months ago
Comment on attachment 8985959 [details] [diff] [review]
1451847-part5-calendar-257.patch

r/a=me
Are there other parts where MOZ_CALENDAR is used that need to be fixed up?
Flags: needinfo?(frgrahl)
Attachment #8985959 - Flags: review?(iann_bugzilla)
Attachment #8985959 - Flags: review+
Attachment #8985959 - Flags: approval-comm-esr60?
Attachment #8985959 - Flags: approval-comm-esr60+
Assignee

Comment 42

11 months ago
> Are there other parts where MOZ_CALENDAR is used that need to be fixed up?

No I think it should be ok now. Verdict is still out but everythigng seems to be in the right place. I can only do limited testing on 2.57 because of the broken mailpane but it seems to work.

Bug 1462480 is probably also gone by now because of the backport. TB still has not pushed the fixes to esr60 and I just will recheck when they merge the TB comm-beta branch to comm-esr60.
Flags: needinfo?(frgrahl)
Assignee

Comment 43

10 months ago
Comment on attachment 8985959 [details] [diff] [review]
1451847-part5-calendar-257.patch

Part 5 ESR60 2.57 only.
https://hg.mozilla.org/releases/comm-esr60/rev/9d397edf04c5
Assignee

Updated

10 months ago
Blocks: 1479194
Assignee

Updated

8 months ago
Duplicate of this bug: 1424283

Updated

8 months ago
See Also: → 1490294

Updated

5 months ago
Blocks: 1517016
You need to log in before you can comment on or make changes to this bug.