1366607 - Change build config to allow building with m-c as topsrcdir

Status: RESOLVED FIXED

(Thunderbird :: Build Config, enhancement)

Description

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

In this bug I'd like to make some build config changes that would allow both what we have currently, but also making m-c the topsrcdir. See meta bug 1366606 for more details.

Comment 1

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Attachment: Fix - v1

This touches various products, to be honest I just ran through the testing for Thunderbird.

The latest try run is here, it matches the failures on c-c at the time:

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=5d11e5205a6c75dc0c777d190df343de769feeb0

Comment 2

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 8869847 [details] [diff] [review]
Fix - v1

Also asking for feedback (or review) from ewong as suggested.

Comment 3

[Edmund Wong (:ewong)]

Comment on attachment 8869847 [details] [diff] [review]
Fix - v1

Review of attachment 8869847 [details] [diff] [review]:
-----------------------------------------------------------------

::: comm-confvars.sh
@@ +1,3 @@
> +#!/bin/sh
> +
> +if [[ "$MOZ_BUILD_APP" == *comm* ]]

been some time since I last looked at the definition of MOZ_BUILD_APP; but, doesn't it get set to "mail" or "suite" or "im" (for comm-central stuff)?

just wondering.. is this a move to have comm/ build under the mozilla src?

Comment 4

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

(In reply to Edmund Wong (:ewong) from comment #3)
> Comment on attachment 8869847 [details] [diff] [review]
> Fix - v1
> 
> Review of attachment 8869847 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: comm-confvars.sh
> @@ +1,3 @@
> > +#!/bin/sh
> > +
> > +if [[ "$MOZ_BUILD_APP" == *comm* ]]
> 
> been some time since I last looked at the definition of MOZ_BUILD_APP; but,
> doesn't it get set to "mail" or "suite" or "im" (for comm-central stuff)?
Yes, but if m-c is the topsrcdir and comm a subdir, then it comm/mail, comm/suite and comm/im.

> 
> just wondering.. is this a move to have comm/ build under the mozilla src?
Not sure if this was before or after our IRC discussion. This is a move to allow having comm build under the mozilla src, but still with separate repos. See also the dependent metabug.

Comment 5

[Frank-Rainer Grahl (:frg)]

Fallen,

I am having trouble applying the patch. Trivial differences but the patch for this one 
calendar\lightning\Makefile.in does not apply at all and contains a few changes. Just a FYI. Will see that I fix it up later.

Comment 6

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Attachment: Fix - v1

I rebased against the latest comm-central, just a minor change in Seamonkey for me. Are you sure you are at the latest tip?

Comment 7

[Joshua Cranmer [:jcranmer]]

Back when I worked on bug 648979, most of these sorts of changes I handled automatically via a script (the script in the bug is out of date, but the basic principles behind it should more or less work). I don't see what sort of script you ran--if any--here, which makes it hard for me to assess if the patch is complete or incomplete.

Comment 8

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

I didn't script the changes, I grepped for certain markers in Makefile.ins and moz.build files. It is certainly possible that this patch would break us during a beta build or release, I did at least check various targets like mach package.

Comment 9

[Edmund Wong (:ewong)]

Comment on attachment 8872039 [details] [diff] [review]
Fix - v1

Review of attachment 8872039 [details] [diff] [review]:
-----------------------------------------------------------------

looks about right.

Comment 10

[Frank-Rainer Grahl (:frg)]

Comment on attachment 8872039 [details] [diff] [review]
Fix - v1

Tested building SeaMonkey locally but only with the comm dir as top source dir. As far as I see it everything still works and everything seems to be covered. 

Covering both options adds some additional complexity so as discussed on irc when this is checked in and stable I would like to see building with comm as top source dir removed in a followup bug.

Even if it breaks something in the short term this really should simplify maintaining the configs and build files in the future so r+ from me.

Thanks

Comment 11

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Attachment: Fix - v2

I'd love to move this out of my queue. Development has been so much easier without the hassle of comm issues with the directory switch.

Tom, what is your opinion on this? Does this help for TC at all, or would you solve it in a different way? 

Any chance we can push this nevertheless and deal with the fallout? As mentioned I made all changes manually so there may be unfound issues, but at least for some of the changes I am not sure if they are easily scriptable.

Comment 12

[Tom Prince [:tomprince]]

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=5dc93a94683b6a7cf153e61e252ee6be3a439bd8

Comment 13

[Tom Prince [:tomprince]]

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=80435979d4649489099c53792b96f2d49db6b594

Comment 14

[Tom Prince [:tomprince]]

Comment on attachment 8916275 [details] [diff] [review]
Fix - v2

Review of attachment 8916275 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, with a couple of minor changes. I'm going to upload an updated patch.

::: mail/build.mk
@@ +8,3 @@
>  
>  package:
> +	$(MAKE) -C $(COMMDEPTH)/mail/installer

This shouldn't lose the @.

@@ +39,3 @@
>  
>  wget-en-US:
>  	$(MAKE) -C mail/locales wget-en-US

This should be updated as well.

Comment 15

[Tom Prince [:tomprince]]

Attachment: Bug 1366607 - Allow building with m-c as topsrcdir.

Review commit: https://reviewboard.mozilla.org/r/188188/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/188188/

Comment 16

[Tom Prince [:tomprince]]

Comment on attachment 8917173 [details]
Bug 1366607 - Allow building with m-c as topsrcdir.

https://reviewboard.mozilla.org/r/188188/#review193362

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/84893db274a4
Allow building with m-c as topsrcdir. r=frg,tomprince

Comment 18

[Axel Hecht [:Pike]]

Seems that building comm-central/mozilla doesn't work right now?

$ make -d echo-variable-COMMDEPTH|grep app-config
Reading makefile `/src/central/comm-central/../mail/app-config.mk' (search path) (don't care) (no ~ expansion)...
 Considering target file `/src/central/comm-central/../mail/app-config.mk'.
  File `/src/central/comm-central/../mail/app-config.mk' does not exist.
  Looking for an implicit rule for `/src/central/comm-central/../mail/app-config.mk'.
  No implicit rule found for `/src/central/comm-central/../mail/app-config.mk'.
  Finished prerequisites of target file `/src/central/comm-central/../mail/app-config.mk'.
 Must remake target `/src/central/comm-central/../mail/app-config.mk'.
 Failed to remake target file `/src/central/comm-central/../mail/app-config.mk'.

Comment 19

[David Edwards]

Building seamonkey from source no longer works for me (Linux x86_64). The configure step isn't defining commreltopsrcdir and friends, so app.mozbuild blows up on e.g. include('/%s/mailnews/mailnews.mozbuild' % CONFIG['commreltopsrcdir'])

How is it supposed to work?

Comment 20

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

I've tried a fresh SeaMonkey build and it goes past the configure step for me. Would be great to figure out what the issue is though.

Regarding COMMDEPTH, Pike, in which case are you getting that?

Comment 21

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Ah, I see the COMMDEPTH part is maybe for bug 1396563 and can reproduce with c-c as topsrcdir. Looks like this is from https://dxr.mozilla.org/comm-central/source/mozilla/config/config.mk#414 where MOZ_BUILD_APP is ../mail and topsrcdir is comm-central. I took a brief look and couldn't find anything obvious, Tom maybe you can take a look? Unfortunately I am out of PTO to go in depth.

Comment 22

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Attachment: Fix COMMDEPTH - v1

Ok, so I did have a little time :) I found an elegant solution that doesn't make use of app-config.mk.

Comment 23

[Axel Hecht [:Pike]]

Comment on attachment 8918767 [details] [diff] [review]
Fix COMMDEPTH - v1

Review of attachment 8918767 [details] [diff] [review]:
-----------------------------------------------------------------

Tested this locally and looked at where this is hooked up, this looks like a good fix to me.

Up to you to decide on whether my review counts, though :-)

Comment 24

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Given this will likely fix l10n nightly builds I think we can count it as a bustage fix, the extra set of eyes is very helpful! Push coming up.

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/33577e068081
Change build config to allow building with m-c as topsrcdir - make sure COMMDEPTH is always defined. r=bustage/Pike

Comment 26

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Tom, anything else you wanted to take care of before we close this bug?

Comment 27

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Sorry about that, the quick fix didn't seem to do it, this broke something with testsuite-targets:
https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=33577e068081b48884ecff324c6d03659e5a561d


I've backed out the changeset so we can go the normal route with try and everything.
https://hg.mozilla.org/comm-central/rev/db6a991ed2ba008bd7f934591250a6079e478d2c

Comment 28

[Tom Prince [:tomprince]]

It looks like topsrcdir and DEPTH in generated `Makefile`s depend on whether the Makefile.in comes from m-c or c-c (when doing a build with c-c as topdir). That means that https://dxr.mozilla.org/comm-central/source/mozilla/config/config.mk#414 won't find the right file when included from a c-c makefiles in a c-c-topdir build.

Comment 29

[Tom Prince [:tomprince]]

(In reply to David Edwards from comment #19)
> Building seamonkey from source no longer works for me (Linux x86_64). The
> configure step isn't defining commreltopsrcdir and friends, so app.mozbuild
> blows up on e.g. include('/%s/mailnews/mailnews.mozbuild' %
> CONFIG['commreltopsrcdir'])
> 
> How is it supposed to work?

It is defined by comm-confvars.sh which is included by $(MOZ_BUILD_APP)/confvars.sh (i.e. suite/confvars.sh) which is included from old-configure.in. It is then propagated by the AC_SUBST in $(MOZ_BUILD_APP)/configure.in.

Comment 30

[Tom Prince [:tomprince]]

Attachment: Bug 1366607: Refer explicitly to the top comm objdir, rather than using a relative depth.

Review commit: https://reviewboard.mozilla.org/r/189934/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/189934/

Comment 31

[David Edwards]

Thanks for that Tom. My problem is suite/confvars.sh
________________________________________________________
#! /bin/sh
# This Source Code Form is subject to the terms of the Mozilla Public
# License, v. 2.0. If a copy of the MPL was not distributed with this
# file, You can obtain one at http://mozilla.org/MPL/2.0/.

source ${_topsrcdir}/$MOZ_BUILD_APP/../comm-confvars.sh
     .
     .
     .
________________________________________________________
'source' is a C-shell rather than a Bourne shell construct, and on my system (Ubuntu 16.04) the script fails with a syntax error.

Replacing 'source' with the Bourne shell '.' fixes it for me.

. ${_topsrcdir}/$MOZ_BUILD_APP/../comm-confvars.sh

Comment 32

[Tom Prince [:tomprince]]

Attachment: Bug 1366607: Don't use bashisms in confvars.sh.

Review commit: https://reviewboard.mozilla.org/r/190022/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/190022/

Comment 33

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/9050482bf464
Refer explicitly to the top comm objdir, rather than using a relative depth; rs=bustage-fix
https://hg.mozilla.org/comm-central/rev/b5a7d14c4cd2
Don't use bashisms in confvars.sh; rs=bustage-fix

Comment 34

[David Edwards]

confvars.sh now works. But build still fails because nsGNOMEShellService.cpp attempts to include nsIDOMHTMLImageElement.h, which in my case I have not got ...
_____________________________________________________________________________________________________________________________
/home/dme/comm-central/suite/shell/src/nsGNOMEShellService.cpp:25:36: fatal error: nsIDOMHTMLImageElement.h: No such file or directory
_____________________________________________________________________________________________________________________________

Comment 35

[Frank-Rainer Grahl (:frg)]

David, include the patch for Bug 1406227 for building SeaMonkey. I need to do a lot of check-ins but was busy with l10n fixing yesterday.

Comment 36

[Tom Prince [:tomprince]]

Attachment: Bug 1366607: Include toolkit moz.configure in m-c topdir build.

Review commit: https://reviewboard.mozilla.org/r/190338/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/190338/

Comment 37

[Tom Prince [:tomprince]]

Attachment: Bug 1366607: Use correct include paths for toolkit in xul files.

Review commit: https://reviewboard.mozilla.org/r/190340/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/190340/

Comment 38

[Tom Prince [:tomprince]]

Attachment: Bug 1366607: Use correct include path for `plugin-container.cpp`.

Review commit: https://reviewboard.mozilla.org/r/190342/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/190342/

Comment 39

[Tom Prince [:tomprince]]

Attachment: Bug 1366607: Use correct path for cityhash license.

Review commit: https://reviewboard.mozilla.org/r/190344/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/190344/

Comment 40

[Tom Prince [:tomprince]]

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=bbc29038db1503de7963b0ea6904df1e8c50dd7d

Comment 41

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 8919516 [details]
Bug 1366607: Include toolkit moz.configure in m-c topdir build.

https://reviewboard.mozilla.org/r/190338/#review195632

Would be good to do the same change for suite/ and im/ as well, r+ with that

Comment 42

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 8919517 [details]
Bug 1366607: Use correct include paths for toolkit in xul files.

https://reviewboard.mozilla.org/r/190340/#review195634

::: mail/config/mozconfigs/linux64/nightly
(Diff revision 1)
>  
>  # Package js shell
>  export MOZ_PACKAGE_JSSHELL=1
>  
> -# Run client.py
> -mk_add_options CLIENT_PY_ARGS="$([ -f $topsrcdir/build/client.py-args ] && cat $topsrcdir/build/client.py-args)"

I suspect this was only for testing?

Comment 43

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 8919518 [details]
Bug 1366607: Use correct include path for `plugin-container.cpp`.

https://reviewboard.mozilla.org/r/190342/#review195636

Comment 44

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 8919519 [details]
Bug 1366607: Use correct path for cityhash license.

https://reviewboard.mozilla.org/r/190344/#review195638

Comment 45

[Tom Prince [:tomprince]]

Comment on attachment 8919516 [details]
Bug 1366607: Include toolkit moz.configure in m-c topdir build.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/190338/diff/1-2/

Comment 46

[Tom Prince [:tomprince]]

Comment on attachment 8919517 [details]
Bug 1366607: Use correct include paths for toolkit in xul files.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/190340/diff/1-2/

Comment 47

[Tom Prince [:tomprince]]

Comment on attachment 8919518 [details]
Bug 1366607: Use correct include path for `plugin-container.cpp`.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/190342/diff/1-2/

Comment 48

[Tom Prince [:tomprince]]

Comment on attachment 8919519 [details]
Bug 1366607: Use correct path for cityhash license.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/190344/diff/1-2/

Comment 49

[Tom Prince [:tomprince]]

Comment on attachment 8919517 [details]
Bug 1366607: Use correct include paths for toolkit in xul files.

https://reviewboard.mozilla.org/r/190340/#review195634

> I suspect this was only for testing?

Yeah. I guess I missed removing this when I extracted these from my taskcluster branch.

Comment 50

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/32a18ec6f4de
Include toolkit moz.configure in m-c topdir build; r=Fallen
https://hg.mozilla.org/comm-central/rev/faa8015c6ae3
Use correct include paths for toolkit in xul files; r=Fallen
https://hg.mozilla.org/comm-central/rev/0a70299b8ece
Use correct include path for `plugin-container.cpp`; r=Fallen
https://hg.mozilla.org/comm-central/rev/c6b8fc410a90
Use correct path for cityhash license; r=Fallen

Comment 51

[Tom Prince [:tomprince]]

Attachment: Bug 1366607: Use relative paths in ldap mozbuild includes.

mozbuild has a test that it can load all mozbuild files when given an empty
configuration. It fails if it has include paths that depend on configuration
variables.

Review commit: https://reviewboard.mozilla.org/r/190822/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/190822/

Comment 52

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 8919880 [details]
Bug 1366607: Use relative paths in ldap mozbuild includes.

https://reviewboard.mozilla.org/r/190822/#review196042

Comment 53

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/41a5daa174d1
Use relative paths in ldap mozbuild includes; r=Fallen

Comment 54

[Frank-Rainer Grahl (:frg)]

Did anyone try to build l10n yet with m-c as top? The toml files have mozilla in it which might not work in this case. pike said something about setting mozilla from the command line in bug 1405407 comment 14 but not sure if this needs additional work besides doing it in the makefiles.

Comment 55

[Tom Prince [:tomprince]]

(In reply to Frank-Rainer Grahl (:frg) from comment #54)
> Did anyone try to build l10n yet with m-c as top? The toml files have
> mozilla in it which might not work in this case. pike said something about
> setting mozilla from the command line in bug 1405407 comment 14 but not sure
> if this needs additional work besides doing it in the makefiles.

I haven't yet. I've been testing this as part of my TaskCluster work, which hasn't got to the point of doing L10N yet. I'll address that when I get to that point.

Comment 56

[Tom Prince [:tomprince]]

Comment on attachment 8918767 [details] [diff] [review]
Fix COMMDEPTH - v1

[Approval Request Comment]
Regression caused by (bug #): 
User impact if declined: 
Testing completed (on c-c, etc.): 
Risk to taking this patch (and alternatives if risky):

Comment 57

[Axel Hecht [:Pike]]

(In reply to Frank-Rainer Grahl (:frg) from comment #54)
> Did anyone try to build l10n yet with m-c as top? The toml files have
> mozilla in it which might not work in this case. pike said something about
> setting mozilla from the command line in bug 1405407 comment 14 but not sure
> if this needs additional work besides doing it in the makefiles.

You'll need a patch in l10n.mk for this, supporting something as CL_DEFINES, and then set CL_DEFINES to say -Dmozilla=.path.to.mozilla-central

Comment 58

[Frank-Rainer Grahl (:frg)]

I fixed up the SeaMonkey build configs and actually tried it for the first time. 

I am having a problem with EXPAND_LOCALE_SRCDIR in suite/installer/Makefile.in. Should be the same for mail I think because the logic is the same.

In suite PPL_LOCALE_ARGS is set to  

> PPL_LOCALE_ARGS=$(call EXPAND_LOCALE_SRCDIR,suite/locales)/installer/windows

EXPAND_LOCALE_SRCDIR is defined in (mozilla)/config/config.mk as

> EXPAND_LOCALE_SRCDIR = $(if $(filter en-US,$(AB_CD)),$(topsrcdir)/$(1)/en-US,$(or $(realpath $(L10NBASEDIR)),$(abspath $(L10NBASEDIR)))/$(AB_CD)/$(subst /locales,,$(1)))

So with m-c as topsourcedir this will be set to $(topsrcdir)/suite/locales/en-US instead of $(topsrcdir)/comm/suite/locales/en-US

And building ends with an error:

> fp = open(path, "r")
> IOError: [Errno 2] No such file or directory: 'd:/seamonkey/mozilla-central/suite/locales/en-US
> /installer/windows\\override.properties'
> Makefile:57: recipe for target 'uninstaller' failed

If it is the same in TB how to fix it best?

Comment 59

[Frank-Rainer Grahl (:frg)]

Attachment: 1366607-installer.patch

Hmm this seems to work for suite so maybe also for mail?

Comment 60

[Frank-Rainer Grahl (:frg)]

Attachment: 1366607-installer.patch

bungled the extension

Comment 61

[Tom Prince [:tomprince]]

Comment on attachment 8923861 [details] [diff] [review]
1366607-installer.patch

Review of attachment 8923861 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/installer/windows/Makefile.in
@@ +46,5 @@
>  
>  ifdef LOCALE_MERGEDIR
>  PPL_LOCALE_ARGS = \
>    --l10n-dir=$(LOCALE_MERGEDIR)/mail/installer \
> +  --l10n-dir=$(call EXPAND_LOCALE_SRCDIR,$(commreltopsrcdir)/mail/locales)/installer \

This won't work for locales other then en-US. In that case, it wants a path in l10n-central, which doesn't have a comm prefix.

Comment 62

[Frank-Rainer Grahl (:frg)]

Hmm I see. Probably best to fix l10n first and see what is left.

One other. I assume the application in the mozconfig is comm/mail or comm/suite so this probably fails in mail/installer/Makefile.in too:
> DEFINES += -DPKG_LOCALE_MANIFEST=$(topobjdir)/mail/installer/locale-manifest.in

I got over it in suite with

> DEFINES += -DPKG_LOCALE_MANIFEST=$(topobjdir)/$(commreltopsrcdir)/suite/installer/locale-manifest.in

At least en-US builds and works fine then.

Comment 63

[Tom Prince [:tomprince]]

I think maybe using something like

> EXPAND_COMM_LOCALE_SRCDIR = $(if $(filter en-US,$(AB_CD)),$(topsrcdir)/$(commreltopsrcdir)/$(1)/en-US,$(or $(realpath $(L10NBASEDIR)),$(abspath $(L10NBASEDIR)))/$(AB_CD)/$(subst /locales,,$(1)))

and using that instead of EXPAND_LOCALE_SRCDIR might do the trick (but I just typed that into bugzilla, so totally untested.

Comment 64

[Frank-Rainer Grahl (:frg)]

Doesn't see to crash and burn but as expected the merge is broken and probably needs to be parametrized as stated in comment 57.

> WARNING:root:WARNING: --locale-mergedir passed, but 'd:/seabuild/release/comm-central-15p/obj-
> x86_64-pc-mingw32/comm/suite/locales/merge-dir/de' does not exist. Ignore this message if the 
> locale is complete.
> INFO:root:processing d:/seamonkey/mozilla-central/comm/suite/locales/jar.mn

Comment 65

[Frank-Rainer Grahl (:frg)]

Attachment: 1366607-suite-wip.patch

wip patch for suite. Do not open before christmas.

Comment 66

[Frank-Rainer Grahl (:frg)]

Attachment: 1366607-suite.patch

Prerequisite patch for SeaMonkey in Bug 1405407 is in so better put this one in too before it rots.

Comment 67

[[:MakeMyDay]]

(In reply to Frank-Rainer Grahl (:frg) from comment #66)
> Created attachment 8927685 [details] [diff] [review]
> 1366607-suite.patch
> 
> Prerequisite patch for SeaMonkey in Bug 1405407 is in so better put this one
> in too before it rots.

Can we port the suite/installer/windows/Makefile.in changes from the above Seamonkey patch for TB to make building on Windows working and land this soon? WFM to build successfully.

Comment 68

[Tom Prince [:tomprince]]

MakeMyDay I've got this in my stack of patches, but I'd like to wait for having CI to land it.

Comment 69

[Frank-Rainer Grahl (:frg)]

Attachment: 1366607-suite-V2.patch

suite patch updated for recent changes

Comment 70

[Ian Neal]

Comment on attachment 8931134 [details] [diff] [review]
1366607-suite-V2.patch

>+++ b/suite/configure.in
>-dnl Get other versions (for the calendar plugin)
>-THUNDERBIRD_VERSION=`cat ${_topsrcdir}/../mail/config/version.txt`
>-AC_SUBST(THUNDERBIRD_VERSION)
Isn't this still needed by Lightning or have things changed since this originally went in?

r=me with that answered/addressed.

Comment 71

[Frank-Rainer Grahl (:frg)]

Comment on attachment 8931134 [details] [diff] [review]
1366607-suite-V2.patch

> Isn't this still needed by Lightning or have things changed since this originally went in?

The version is only calculated from calendar/lightning/versions.mk

This was a leftover from the early days.

Comment 72

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/51ec1985ad3a
Additional changes to allow building SeaMonkey with m-c as topsrcdir. r=IanN

Comment 73

[Frank-Rainer Grahl (:frg)]

I am seeing a repack error in both TB and SM e.g. here:

https://archive.mozilla.org/pub/thunderbird/nightly/2017/12/2017-12-04-03-02-03-comm-central-l10n/comm-central-linux64-l10n-nightly-af-bm94-build1-build518.txt.gz

This probably comes from \calendar\lightning/Makefile.in

> MOZ_BUILDID = $(shell $(PYTHON) $(MOZILLA_SRCDIR)/config/printconfigsetting.py $(DIST)/bin/application.ini App BuildID)

The build seems to succeed but is it possible the full objectdir needs to be put before $(DIST)/bin?


++ snip ++
Repackaging lightning-6.1a1.af locale for Language af
rm -f -rf ../../dist/xpi-stage/lightning-af
cp -R ../../dist/xpi-stage/lightning ../../dist/xpi-stage/lightning-af
grep -v '^locale [a-z\-]\+ en-US' ../../dist/xpi-stage/lightning-af/chrome.manifest > ../../dist/xpi-stage/lightning-af/chrome.manifest~ && \
  mv ../../dist/xpi-stage/lightning-af/chrome.manifest~ ../../dist/xpi-stage/lightning-af/chrome.manifest
*find /builds/slave/tb-c-cen-l64-l10n-ntly-0000000/build/source/objdir-tb/dist/xpi-stage/lightning-af -name '*en-US*' -print0 | xargs -0 rm -rf**
**Traceback (most recent call last):**
**  File "/builds/slave/tb-c-cen-l64-l10n-ntly-0000000/build/source/mozilla/config/printconfigsetting.py", line 16, in <module>**
**    with open(file) as fh:**
**IOError: [Errno 2] No such file or directory: '../../dist/bin/application.ini'**
*/builds/slave/tb-c-cen-l64-l10n-ntly-0000000/build/source/objdir-tb/_virtualenv/bin/python -m mozbuild.action.preprocessor  -DGRE_MILESTONE=59.0a1

Comment 74

[Tom Prince [:tomprince]]

That is an issue unrelated to the m-c change I think (looking at old nightly logs, I can see it as far back as 2017-08-23. 

(In reply to Frank-Rainer Grahl (:frg) from comment #73)

> I am seeing a repack error in both TB and SM e.g. here:
> 
> https://archive.mozilla.org/pub/thunderbird/nightly/2017/12/2017-12-04-03-02-
> 03-comm-central-l10n/comm-central-linux64-l10n-nightly-af-bm94-build1-
> build518.txt.gz
> 
> This probably comes from \calendar\lightning/Makefile.in
> 
> > MOZ_BUILDID = $(shell $(PYTHON) $(MOZILLA_SRCDIR)/config/printconfigsetting.py $(DIST)/bin/application.ini App BuildID)
> 
> The build seems to succeed but is it possible the full objectdir needs to be
> put before $(DIST)/bin?
> [...]

Comment 75

[Phabricator Automation]

Attachment: Bug 1366607: Stop getting SeaMonkey version in Thunderbird's configure; r?Fallen

Comment 76

[Frank-Rainer Grahl (:frg)]

Comment on attachment 8952018 [details]
Bug 1366607: Stop getting SeaMonkey version in Thunderbird's configure; r?Fallen

This might be obsolete now after bug 1309372 moved the version check inside calendar.

Comment 77

[Phabricator Automation]

Comment on attachment 8952018 [details]
Bug 1366607: Stop getting SeaMonkey version in Thunderbird's configure; r?Fallen

Philipp Kewisch [:Fallen]  has approved the revision.

https://phabricator.services.mozilla.com/D602

Comment 78

[Phabricator Automation]

Attachment: Bug 1366607: Fix L10N repack of chat and editor with m-c as topdir; r?Fallen

Comment 79

[Phabricator Automation]

Comment on attachment 8952247 [details]
Bug 1366607: Fix L10N repack of chat and editor with m-c as topdir; r?Fallen

Philipp Kewisch [:Fallen]  has approved the revision.

https://phabricator.services.mozilla.com/D608

Comment 80

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/480bf9c55458
Fix L10N repack of chat and editor with m-c as topdir; r=Fallen

Comment 81

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/c99498430e33
Stop getting SeaMonkey version in Thunderbird's configure; r=Fallen

Comment 82

[Phabricator Automation]

Attachment: Bug 1366607: Drop support for building with comm-central in automation mozconfigs; r?Fallen

Comment 83

[Phabricator Automation]

Attachment: Bug 1366607: Get rid of references to `$commtopsrcdir` in automation mozconfigs; r?Fallen

Comment 84

[Phabricator Automation]

Attachment: Bug 1366607: Get rid of references to mozconfig.comm-support in thunderbird; r?Fallen

Comment 85

[Phabricator Automation]

Attachment: Bug 1366607: Remove unused tooltool manifests; r?Fallen

Comment 86

[Phabricator Automation]

Comment on attachment 8967626 [details]
Bug 1366607: Drop support for building with comm-central in automation mozconfigs; r?Fallen

Philipp Kewisch [:Fallen]  has approved the revision.

https://phabricator.services.mozilla.com/D930

Comment 87

[Phabricator Automation]

Comment on attachment 8967627 [details]
Bug 1366607: Get rid of references to `$commtopsrcdir` in automation mozconfigs; r?Fallen

Philipp Kewisch [:Fallen]  has approved the revision.

https://phabricator.services.mozilla.com/D931

Comment 88

[Phabricator Automation]

Comment on attachment 8967628 [details]
Bug 1366607: Get rid of references to mozconfig.comm-support in thunderbird; r?Fallen

Philipp Kewisch [:Fallen]  has approved the revision.

https://phabricator.services.mozilla.com/D932

Comment 89

[Phabricator Automation]

Comment on attachment 8967630 [details]
Bug 1366607: Remove unused tooltool manifests; r?Fallen

Philipp Kewisch [:Fallen]  has approved the revision.

https://phabricator.services.mozilla.com/D933

Comment 90

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

IIUC after these patches it won't be possible to build with comm-central as topsrcdir? If so, we need to migrate developers over, or at least provide something like client.py that does the checkouts.

Comment 91

[Frank-Rainer Grahl (:frg)]

> IIUC after these patches it won't be possible to build with comm-central as topsrcdir?

Seems to be only mail/config which is only used in automation. Local builds should still work fine.

Comment 92

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/8dc4a26ee33f
Drop support for building with comm-central in automation mozconfigs; r=Fallen
https://hg.mozilla.org/comm-central/rev/5bb4ac3133c6
Get rid of references to `$commtopsrcdir` in automation mozconfigs; r=Fallen
https://hg.mozilla.org/comm-central/rev/fa2a77a48a22
Get rid of references to mozconfig.comm-support in thunderbird; r=Fallen
https://hg.mozilla.org/comm-central/rev/88f7e0f87f29
Remove unused tooltool manifests; r=Fallen

Comment 93

[Tom Prince [:tomprince]]

(In reply to Frank-Rainer Grahl (:frg) from comment #91)
> > IIUC after these patches it won't be possible to build with comm-central as topsrcdir?
> 
> Seems to be only mail/config which is only used in automation. Local builds
> should still work fine.

This is correct.

(In reply to Philipp Kewisch [:Fallen]  from comment #90)
> [...] we need to migrate developers over, or at least provide
> something like client.py that does the checkouts.

That being said, I think that migrating people to the new layout *now* woud be worth while. I anticipate that TB 60 will ship with source packages in the new format; and I know that the mozilla build peers would like to remove the special cases to handle the c-c topdir layout in mozbuild/mach.

Comment 95

[Tom Prince [:tomprince]]

comm-central builds are exclusively built this way, and soon releases will be as well.