Closed Bug 659205 Opened 12 years ago Closed 12 years ago

tier_platform_dirs doesn't include APP_LIBXUL_DIRS (mailnews/ etc) -- mailnews, xpfe autocomplete etc aren't getting built

Categories

(MailNews Core :: Build Config, defect)

defect
Not set
blocker

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 7.0

People

(Reporter: rain1, Assigned: iannbugzilla)

References

Details

Attachments

(3 files, 2 obsolete files)

Probably https://hg.mozilla.org/mozilla-central/rev/8e846d7f21ea that busted this -- APP_LIBXUL_DIRS simply isn't included in tier_platform_dirs. To check this, run make echo-variable-tier_platform_dirs in mozilla/.

This means that mailnews/ etc are simply not on the build path. This is pretty bad :)
Summary: tier_platform_dirs doesn't include APP_LIBXUL_DIRS (mailnews/ etc) → tier_platform_dirs doesn't include APP_LIBXUL_DIRS (mailnews/ etc) -- mailnews, xpfe autocomplete etc aren't getting built
Does this mean our linking mailnews with libxul is broken? Or is it that mailnews isn't getting compiled when we rebuild tier_platform?
mailnews isn't getting compiled at all, yeah.

http://tinderbox.mozilla.org/showlog.cgi?log=ThunderbirdTrunk/1306166701.1306180393.7364.gz is a build from right before the offending m-c patch.
http://tinderbox.mozilla.org/showlog.cgi?log=ThunderbirdTrunk/1306180239.1306189538.15982.gz&fulltext=1 is a succeeding build from right after. Search for "imap" to see the difference -- the build right after is succeeding only because it's using a stale version of mailnews.
Standard8 figured out how to make tier_platforms build mailnews in the first place, so I'm hoping he can figure out how it was broken...
Given our regression range, cc-ing ted and khuey incase they have time to offer insight.
AHA:

http://mxr.mozilla.org/comm-central/source/suite/build.mk#40

and

http://mxr.mozilla.org/comm-central/source/mail/build.mk#40

Our issue is that these two if's are being checked in the m-c code.

Where thats no longer defined.

Standard8, sid, KaiRo: rs+=me for a fix for this. (I'll prefer Mark, since he knows the plan for branching c-c->beta/aurora) anyone else, get review.
OS: Windows 7 → All
Hardware: x86 → All
Version: unspecified → Trunk
This patch:
* Moves the code that was in ifdef MOZ_ENABLE_LIBXUL out of it
* Removes all the code that was in ifndef MOZ_ENABLE_LIBXUL or an else from above.

At the moment BUILD_STATIC_LIBS will be another patch, just testing whether a clobber build will work without that patch.
Assignee: nobody → iann_bugzilla
Status: NEW → ASSIGNED
Comment on attachment 534729 [details] [diff] [review]
Fix MOZ_ENABLE_LIBXUL in mail, mailnews and suite patch [Checked in: Comment 10]

Local clobber build completed libxul.so successfully.
Attachment #534729 - Flags: review?(neil)
cool, thx for looking at this, I'll try it with a local thunderbird build
Comment on attachment 534729 [details] [diff] [review]
Fix MOZ_ENABLE_LIBXUL in mail, mailnews and suite patch [Checked in: Comment 10]

I was able to do a clobber build on windows with this patch and run - I had ac_add_options --enable-chrome-format=jar in my .mozconfig, so I'm trying a clobber build w/o that to see if that somehow made my build work first time.
Comment on attachment 534729 [details] [diff] [review]
Fix MOZ_ENABLE_LIBXUL in mail, mailnews and suite patch [Checked in: Comment 10]

r=me for suite/ (and mailnews/base/public if you need it).
Attachment #534729 - Flags: review?(neil) → review+
r=me for the mail stuff - it does get it building for me at least :-)
Comment on attachment 534729 [details] [diff] [review]
Fix MOZ_ENABLE_LIBXUL in mail, mailnews and suite patch [Checked in: Comment 10]

http://hg.mozilla.org/comm-central/rev/0d1c875f3997

Still the static build stuff plus what is to happen with the test that didn't work under MOZ_ENABLE_LIBXUL (TestMsgStripRE.cpp, TestImapHdrXferInfo.cpp, TestImapFlagAndUidState.cpp and TestMimeCrash.cpp)
Attachment #534729 - Attachment description: Fix MOZ_ENABLE_LIBXUL in mail, mailnews and suite patch → Fix MOZ_ENABLE_LIBXUL in mail, mailnews and suite patch [Checked in: Comment 10]
As a follow up remove the various other references to MOZ_ENABLE_LIBXUL from configure.in and config/

Waiting for local clobber builds to complete.
Changes since last version:
* Remove unneeded EXTRA_DSO_LIBS += thebes line
Attachment #534861 - Attachment is obsolete: true
Comment on attachment 534875 [details] [diff] [review]
Fix config/ and configure.in for comm-central patch

Tested locally on a clobber build for both TB and SM, compiles and runs fine so requesting review.
Attachment #534875 - Flags: review?(mbanner)
Changes since last version:
* Missed a Makefile.in in mailnews/base/test
Attachment #534875 - Attachment is obsolete: true
Attachment #534875 - Flags: review?(mbanner)
Attachment #534923 - Flags: review?(mbanner)
This patch:
* Removes code to do with BUILD_STATIC_LIBS that is not used any more.

I've not touch the stuff round BUILD_STATIC_SHELL though.

Just testing against local clobbers for TB and SM.
Comment on attachment 534924 [details] [diff] [review]
Fix BUILD_STATIC_LIBS in comm-central patch [Checked in: Comment 25]

Successfully builds for TB and SM following a clobber so requesting a review.
Attachment #534924 - Flags: review?(mbanner)
Whiteboard: Callek nominated at comment 5
clearing tracking request since deeper triage proved this to be a comm-* issue, and not directly a Gecko issue.
Whiteboard: Callek nominated at comment 5
Comment on attachment 534923 [details] [diff] [review]
Fix config/ and configure.in for comm-central and missed Makefile.in patch [Checked in: Comment 24]

Whoever gets to it first
Attachment #534923 - Flags: review?(bugspam.Callek)
Comment on attachment 534924 [details] [diff] [review]
Fix BUILD_STATIC_LIBS in comm-central patch [Checked in: Comment 25]

Whoever gets to it first
Attachment #534924 - Flags: review?(bugspam.Callek)
Comment on attachment 534923 [details] [diff] [review]
Fix config/ and configure.in for comm-central and missed Makefile.in patch [Checked in: Comment 24]

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

::: configure.in
@@ +6640,5 @@
>  if test -n "$MOZ_STATIC_BUILD_UNSUPPORTED" -a -n "$BUILD_STATIC_LIBS"; then
>  	AC_MSG_ERROR([--enable-static is not supported for building $MOZ_APP_NAME. You probably want --enable-libxul.])
>  fi
>  
> +AC_SUBST(LIBXUL_LIBS)

We don't actually use LIBXUL_LIBS in our build system (and it isn't in autoconf.mk), so I think you can just drop this line as it is a no-op.

I've not tested this, but it looks fine. rs=me with that fixed.
Attachment #534923 - Flags: review?(mbanner)
Attachment #534923 - Flags: review?(bugspam.Callek)
Attachment #534923 - Flags: review+
Comment on attachment 534924 [details] [diff] [review]
Fix BUILD_STATIC_LIBS in comm-central patch [Checked in: Comment 25]

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

This also looks fine, rs=me with just the one change.

::: config/config.mk
@@ +234,1 @@
>  _ENABLE_PIC=1

I know mozilla-central has left this line in, but it is useless:

http://mxr.mozilla.org/comm-central/search?string=_ENABLE_PIC&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=comm-central

We should just drop it now.
Attachment #534924 - Flags: review?(mbanner)
Attachment #534924 - Flags: review?(bugspam.Callek)
Attachment #534924 - Flags: review+
Comment on attachment 534923 [details] [diff] [review]
Fix config/ and configure.in for comm-central and missed Makefile.in patch [Checked in: Comment 24]

http://hg.mozilla.org/comm-central/rev/f250c2806162
Attachment #534923 - Attachment description: Fix config/ and configure.in for comm-central and missed Makefile.in patch → Fix config/ and configure.in for comm-central and missed Makefile.in patch [Checked in: Comment 24]
Comment on attachment 534924 [details] [diff] [review]
Fix BUILD_STATIC_LIBS in comm-central patch [Checked in: Comment 25]

http://hg.mozilla.org/comm-central/rev/78cbb0581f0f
Attachment #534924 - Attachment description: Fix BUILD_STATIC_LIBS in comm-central patch → Fix BUILD_STATIC_LIBS in comm-central patch [Checked in: Comment 25]
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 7.0
(In reply to Ian Neal from comment #12)
> [...] what is to happen with the test that
> didn't work under MOZ_ENABLE_LIBXUL (TestMsgStripRE.cpp,
> TestImapHdrXferInfo.cpp, TestImapFlagAndUidState.cpp and TestMimeCrash.cpp)

I filed bug 702937.
No longer blocks: 488971
No longer blocks: 629896
Blocks: 632429
No longer blocks: 702937
You need to log in before you can comment on or make changes to this bug.