Closed Bug 1138063 Opened 9 years ago Closed 9 years ago

Replace manual install rules (e.g. tests, FINAL_TARGET files) with moz.build equivalents

Categories

(Thunderbird :: Build Config, defect)

defect
Not set
normal

Tracking

(thunderbird42 fixed)

RESOLVED FIXED
Thunderbird 42.0
Tracking Status
thunderbird42 --- fixed

People

(Reporter: bokeefe, Assigned: bokeefe)

References

Details

Attachments

(5 files, 1 obsolete file)

I was on a quest to get rid of a bunch of Makefile.ins. I have a handful of patches that get c-c down from 102 to 67 Makefile.ins (13 of those are from the ldap sdk).

Finished try push: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=278c11792819

And a followup to fix the crypto issue (still running): https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=143450249986

I'll just attach the first 4 csets in this bug; the others need corresponding m-c changes.
/r/4489 - Bug 1138063 - Fix up most of the manual test staging rules in c-c; r=jcranmer
/r/4491 - Bug 1138063 - Clean up a bunch of make rules that install things to FINAL_TARGETl r=jcranmer
/r/4493 - Bug 1138063 - Move a leftover SHARED_LIBRARY_LIBS to a USE_LIBS; r=jcranmer
/r/4495 - Bug 1138063 - Remove boilerplate stuff from Makefile.ins; r=jcranmer

Pull down these commits:

hg pull review -r dbd13c16c9aa59e968e264db57f2f8063f9ce1a1
Attachment #8570926 - Flags: review?(Pidgeot18)
https://reviewboard.mozilla.org/r/4491/#review3653

::: mailnews/extensions/smime/moz.build
(Diff revision 1)
> +FINAL_TARGET_FILES.defaults.pref += [

You probably should be using PREF_JS_EXPORTS instead of FINAL_TARGET_FILES.defaults.pref.
https://reviewboard.mozilla.org/r/4489/#review3789

::: mail/test/mozmill/moz.build
(Diff revision 1)
> +TEST_HARNESS_FILES.mozmill.resources += [

I don't like these changes--I think these really want to be shell globs (particularly mailnews/test/resources), although I'm also aware that globbing in TEST_HARNESS_FILES doesn't work properly for comm-central code.

::: mailnews/moz.build
(Diff revision 1)
> +TEST_HARNESS_FILES.xpcshell.mailnews.resources += [

Ditto for all these changes.

::: suite/browser/moz.build
(Diff revision 1)
> -TEST_DIRS += ['test']
> +BROWSER_CHROME_MANIFESTS += ['test/browser/browser.ini']

You'll want to get Callek to review the suite changes.
https://reviewboard.mozilla.org/r/4491/#review3791

::: calendar/base/backend/icaljs/Makefile.in
(Diff revision 1)
> -BACKEND_MANIFESTS = icaljs.manifest

These calendar/ changes deserve review from Fallen.

::: im/app/profile/moz.build
(Diff revision 1)
> +

... and im/ changes want feedback probably from florian.
https://reviewboard.mozilla.org/r/4491/#review3793

::: mailnews/base/ispdata/moz.build
(Diff revision 1)
> +if CONFIG['MOZ_MOVEMAIL']:

This condition could be CONFIG['MOZ_MOVEMAIL'] and (not CONFIG['MOZ_THUNDERBIRD'] or CONFIG['MOZ_WIDGET_TOOLKIT'] != 'cocoa'), could it not?

This could also stand to have a comment saying that Thunderbird wants to disable movemail on OS X.

::: mailnews/extensions/dsn/moz.build
(Diff revision 1)
> +FINAL_TARGET_FILES.defaults.pref += [

This wants to be JS_PREFERENCE_FILES, not FINAL_TARGET_FILES, and similarly for several other FINAL_TARGET_FILES.defaults.pref
Depends on: 1140161
https://reviewboard.mozilla.org/r/4489/#review3903

> I don't like these changes--I think these really want to be shell globs (particularly mailnews/test/resources), although I'm also aware that globbing in TEST_HARNESS_FILES doesn't work properly for comm-central code.

After some poking at this, wildcards seem okay in general, but absolute paths with wildcards don't work so well. I filed bug 1140161 to fix that first, then I'll switch these back to globs.
Attachment #8570926 - Flags: review?(Pidgeot18)
Comment on attachment 8570926 [details]
MozReview Request: bz://1138063/bokeefe

/r/4489 - Bug 1138063 - Fix up most of the manual test staging rules in c-c; r=jcranmer,callek
/r/4491 - Bug 1138063 - Clean up a bunch of make rules that install things to FINAL_TARGET; r=jcranmer,fallen,florian
/r/4493 - Bug 1138063 - Move a leftover SHARED_LIBRARY_LIBS to a USE_LIBS; r=jcranmer
/r/4495 - Bug 1138063 - Remove boilerplate stuff from Makefile.ins; r=jcranmer

Pull down these commits:

hg pull review -r e0b684a22cf8c0dfc2ffd167cbcb0ea49fdff969
Attachment #8570926 - Flags: review?(philipp)
Attachment #8570926 - Flags: review?(florian)
Attachment #8570926 - Flags: review?(bugspam.Callek)
Attachment #8570926 - Flags: review?(Pidgeot18)
Attachment #8570926 - Flags: review?(philipp) → review+
https://reviewboard.mozilla.org/r/4493/#review5611

Well, in truth, I've been meaning to kill --enable-incomplete-external-linkage anyways, but there's no harm in this patch.
https://reviewboard.mozilla.org/r/4495/#review5613

::: ldap/xpcom/src/moz.build
(Diff revision 2)
> +CXXFLAGS += [CONFIG['LDAP_CFLAGS']]

Ugh, this is rather kind of ugly. I guess we don't have the configure infrastructure setup to make this is a list. Not that it matters too much, since I'm planning on killing the LDAP build system nastiness soon anyways.
Comment on attachment 8570926 [details]
MozReview Request: bz://1138063/bokeefe

Sorry for the delay in getting to this patch, but I'd like to thank you for tackling some of this build system cleanup!
Attachment #8570926 - Flags: review?(Pidgeot18) → review+
Comment on attachment 8570926 [details]
MozReview Request: bz://1138063/bokeefe

https://reviewboard.mozilla.org/r/4487/#review5617

Ship It!
(ack, sorry for all the spamming)

One last thing--with the exception of the last patch, I did not verify calendar/, im/, or suite/ changes.
Pushed to comm-central to unbreak the tree (sort of)
http://hg.mozilla.org/comm-central/rev/88800b16a29e
http://hg.mozilla.org/comm-central/rev/d97abf888172
http://hg.mozilla.org/comm-central/rev/d2a5e29e29c4
http://hg.mozilla.org/comm-central/rev/de2df507d8e7
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 40.0
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Executing command: ['python', '../tools/buildfarm/utils/hgtool.py', 'https://hg.mozilla.org/mozilla-central', 'c:/builds/moz2_slave/tb-c-cen-w32-00000000000000000/build\\mozilla']
Executing command: ['hg', 'update', '-r', 'default', '-R', 'c:/builds/moz2_slave/tb-c-cen-w32-00000000000000000/build\\mozilla', '--time']
TinderboxPrint:<a href=https://hg.mozilla.org/mozilla-central/rev/0a46652bd992 title='Built from mozilla-central revision 0a46652bd992'>moz:0a46652bd992</a>
Executing command: ['hg', 'pull', '-R', 'c:/builds/moz2_slave/tb-c-cen-w32-00000000000000000/build\\ldap\\sdks', '--time']
Executing command: ['hg', 'update', '-r', 'LDAPCSDK_6_0_7H_RTM', '-R', 'c:/builds/moz2_slave/tb-c-cen-w32-00000000000000000/build\\ldap\\sdks', '--time']
TinderboxPrint:<a href=https://hg.mozilla.org/projects/ldap-sdks/rev/0688442b9383 title='Built from ldap-sdks revision 0688442b9383'>ldap:0688442b9383</a>
c:/builds/moz2_slave/tb-c-cen-w32-00000000000000000/build/mozmake.exe -f c:/builds/moz2_slave/tb-c-cen-w32-00000000000000000/build/client.mk build
mozmake.exe[1]: Entering directory 'c:/builds/moz2_slave/tb-c-cen-w32-00000000000000000/build'
Adding client.mk options from c:/builds/moz2_slave/tb-c-cen-w32-00000000000000000/build/.mozconfig:
AUTOCLOBBER=1
export MOZ_AUTOMATION_BUILD_SYMBOLS=1
export MOZ_AUTOMATION_L10N_CHECK=1
export MOZ_AUTOMATION_PACKAGE=1
export MOZ_AUTOMATION_PACKAGE_TESTS=1
export MOZ_AUTOMATION_INSTALLER=0
export MOZ_AUTOMATION_UPDATE_PACKAGING=0
export MOZ_AUTOMATION_UPLOAD=1
export MOZ_AUTOMATION_UPLOAD_SYMBOLS=0
export MOZ_AUTOMATION_SDK=0
export LIB=c:\\Program Files (x86)\\Windows Kits\\8.1\\Lib\\winv6.3\\um\\x86;c:\\tools\\vs2013\\vc\\lib;c:\\tools\\vs2013\\vc\\atlmfc\\lib;c:\\tools\\sdks\\dx10\\lib
export LIBPATH=c:\\Program Files (x86)\\Windows Kits\\8.1\\Lib\\winv6.3\\um\\x86;c:\\tools\\vs2013\\vc\\lib;c:\\tools\\vs2013\\vc\\atlmfc\\lib;c:\\tools\\sdks\\dx10\\lib
export PATH=c:\\Program Files (x86)\\Windows Kits\\8.1\\bin\\x86;c:\\tools\\vs2013\\Common7\\IDE;c:\\tools\\vs2013\\VC\\BIN\\amd64_x86;c:\\tools\\vs2013\\VC\\BIN\\amd64;c:\\tools\\vs2013\\Common7\\Tools;c:\\tools\\vs2013\\VC\\VCPackages;c:\\mozilla-build\\moztools;c:\\Program Files (x86)\\Windows Kits\\8.1\\bin\\x86;c:\\tools\\vs2013\\Common7\\IDE;c:\\tools\\vs2013\\VC\\BIN\\amd64_x86;c:\\tools\\vs2013\\VC\\BIN\\amd64;c:\\tools\\vs2013\\Common7\\Tools;c:\\tools\\vs2013\\VC\\VCPackages;c:\\mozilla-build\\moztools;c:\\mozilla-build\\nsis-3.0a2;c:\\mozilla-build\\nsis-2.46u;c:\\mozilla-build\\python27;c:\\mozilla-build\\buildbotve\\scripts;C:\\mozilla-build\\msys\\local\\bin;c:\\mozilla-build\\wget;c:\\mozilla-build\\7zip;c:\\mozilla-build\\blat261\\full;c:\\mozilla-build\\python;c:\\mozilla-build\\svn-win32-1.6.3\\bin;c:\\mozilla-build\\upx203w;c:\\mozilla-build\\emacs-22.3\\bin;c:\\mozilla-build\\info-zip;c:\\mozilla-build\\nsis-2.22;c:\\mozilla-build\\nsis-2.33u;c:\\mozilla-build\\nsis-2.46u;c:\\mozilla-build\\wix-351728;c:\\mozilla-build\\hg;c:\\mozilla-build\\python\\Scripts;c:\\mozilla-build\\kdiff3;c:\\mozilla-build\\yasm;.;C:\\mozilla-build\\msys\\local\\bin;C:\\mozilla-build\\msys\\mingw\\bin;C:\\mozilla-build\\msys\\bin;c:\\windows\\system32;c:\\windows;c:\\windows\\System32\\Wbem;c:\\windows\\System32\\WindowsPowerShell\\v1.0\\;c:\\mozilla-build;c:\\mozilla-build\\python27;c:\\mozilla-build\\python27\\Scripts;C:\\mozilla-build\\msys\\bin;c:\\mozilla-build\\vim\\vim72;c:\\mozilla-build\\wget;c:\\mozilla-build\\info-zip;c:\\CoreUtils\\bin;c:\\mozilla-build\\buildbotve\\scripts;c:\\Program Files (x86)\\Microsoft SQL Server\\100\\Tools\\Binn\\;c:\\Program Files\\Microsoft SQL Server\\100\\Tools\\Binn\\;c:\\Program Files\\Microsoft SQL Server\\100\\DTS\\Binn\\;c:\\Program Files (x86)\\Windows Kits\\8.0\\Windows Performance Toolkit\\;c:\\Program Files (x86)\\Windows Kits\\8.1\\Windows Performance Toolkit\\;c:\\Program Files\\Microsoft SQL Server\\110\\Tools\\Binn\\;c:\\Program Files (x86)\\Microsoft SDKs\\TypeScript\\1.0\\;c:\\mozilla-build\\hg;c:\\mozilla-build\\moztools-x64\\bin;c:\\mozilla-build\\vim\\vim72
export INCLUDE=c:\\Program Files (x86)\\Windows Kits\\8.1\\include\\shared;c:\\Program Files (x86)\\Windows Kits\\8.1\\include\\um;c:\\Program Files (x86)\\Windows Kits\\8.1\\include\\winrt;c:\\Program Files (x86)\\Windows Kits\\8.1\\include\\winrt\\wrl;c:\\Program Files (x86)\\Windows Kits\\8.1\\include\\winrt\\wrl\\wrappers;c:\\tools\\vs2013\\vc\\include;c:\\tools\\vs2013\\vc\\atlmfc\\include;c:\\tools\\sdks\\dx10\\include
export WIN32_REDIST_DIR=c:/tools/vs2013/VC/redist/x86/Microsoft.VC120.CRT
export INCLUDE=c:\\Program Files (x86)\\Windows Kits\\8.1\\include\\shared;c:\\Program Files (x86)\\Windows Kits\\8.1\\include\\um;c:\\Program Files (x86)\\Windows Kits\\8.1\\include\\winrt;c:\\Program Files (x86)\\Windows Kits\\8.1\\include\\winrt\\wrl;c:\\Program Files (x86)\\Windows Kits\\8.1\\include\\winrt\\wrl\\wrappers;c:\\tools\\vs2013\\vc\\include;c:\\tools\\vs2013\\vc\\atlmfc\\include;c:\\tools\\sdks\\dx10\\include;c:\\Office 2010 Developer Resources\\Outlook 2010 MAPI Headers
CLIENT_PY_ARGS=--hg-options='--time' --hgtool=../tools/buildfarm/utils/hgtool.py --hgtool1=../scripts/buildfarm/utils/hgtool.py --skip-chatzilla --skip-comm --skip-inspector --tinderbox-print
ALWAYS_RUN_CLIENT_PY=1
FOUND_MOZCONFIG := c:/builds/moz2_slave/tb-c-cen-w32-00000000000000000/build/.mozconfig
c:/builds/moz2_slave/tb-c-cen-w32-00000000000000000/build/mozmake.exe -j4 -C c:/builds/moz2_slave/tb-c-cen-w32-00000000000000000/build/objdir-tb
mozmake.exe[2]: Entering directory 'c:/builds/moz2_slave/tb-c-cen-w32-00000000000000000/build/objdir-tb'
Makefile:100: backend.RecursiveMakeBackend.pp: No such file or directory
mozmake.exe[2]: Leaving directory 'c:/builds/moz2_slave/tb-c-cen-w32-00000000000000000/build/objdir-tb'
c:/builds/moz2_slave/tb-c-cen-w32-00000000000000000/build/client.mk:404: recipe for target 'build' failed
mozmake.exe[1]: Leaving directory 'c:/builds/moz2_slave/tb-c-cen-w32-00000000000000000/build'
client.mk:219: recipe for target 'build' failed
c:/builds/moz2_slave/tb-c-cen-w32-00000000000000000/build/mozilla/config/recurse.mk:27: root.mk: No such file or directory
mozmake.exe[2]: *** No rule to make target 'root.mk'. Stop.
mozmake.exe[1]: *** [build] Error 2
mozmake.exe: *** [build] Error 2
program finished with exit code 2
Is "Bug 1153559 - Exception: Variable SHARED_LIBRARY_LIBS is defined"
be related to this entry?

TIA
(In reply to ISHIKAWA, Chiaki from comment #19)
> Is "Bug 1153559 - Exception: Variable SHARED_LIBRARY_LIBS is defined"
> be related to this entry?

Part 3 in this bug will fix the SHARED_LIBRARY_LIBS problem you are seeing.
https://reviewboard.mozilla.org/r/4493/diff/raw/
Are you still working on this issue?
Flags: needinfo?(bokeefe)
I am not so sure what's happening, I cherry pick these patches to branch 38, and it's working fine.
And I see the auto-build error, it's seems not running with
march build but other commands.
Flags: needinfo?(philip.chee)
(In reply to Yonggang Luo from comment #21)
> Are you still working on this issue?

Parts 1 and 2 were still waiting for reviews, but other than that, I think everything is good to go.

I've been rebasing periodically; I should probably push an updated set of patches to try. Of note, the equivalent to part 3 landed separately in bug 1153187.
Flags: needinfo?(bokeefe)
(In reply to Brian O'Keefe [:bokeefe] from comment #23)

> Parts 1 and 2 were still waiting for reviews, but other than that, I think
> everything is good to go.

> I've been rebasing periodically; I should probably push an updated set of
> patches to try. Of note, the equivalent to part 3 landed separately in bug
> 1153187.

Have you fixed the build failure in comment 18?

> mozmake.exe[2]: Entering directory 'c:/builds/moz2_slave/tb-c-cen-w32-00000000000000000/build/objdir-tb'
> Makefile:100: backend.RecursiveMakeBackend.pp: No such file or directory
Flags: needinfo?(philip.chee)
(In reply to Yonggang Luo from comment #22)
> I am not so sure what's happening, I cherry pick these patches to branch 38,
> and it's working fine.
> And I see the auto-build error, it's seems not running with
> march build but other commands.
I don't know. I'm just a spectator here, waiting for a fix.
Status: REOPENED → ASSIGNED
(In reply to Philip Chee from comment #24)
> Have you fixed the build failure in comment 18?
> 
> > mozmake.exe[2]: Entering directory 'c:/builds/moz2_slave/tb-c-cen-w32-00000000000000000/build/objdir-tb'
> > Makefile:100: backend.RecursiveMakeBackend.pp: No such file or directory

I didn't do anything specifically to fix that, but I haven't had that happen locally. I pushed to try; let's see what happens. Both backend.RecursiveMakeBackend.pp and root.mk (which it complained about later) are generated by the build backend, so something should stopped earlier in the build.

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=ba5fca4a9228
Attachment #8570926 - Flags: review?(florian) → review+
Comment on attachment 8570926 [details]
MozReview Request: bz://1138063/bokeefe

https://reviewboard.mozilla.org/r/4487/#review6309

Ship It!
The suite mochitest changes have landed as the fix to bug 773349, sorry this bug didn't show up when I was looking for dupes.
Patches from bug 1159775 and bug 1155290 might bit-rot things further.
Comment on attachment 8570926 [details]
MozReview Request: bz://1138063/bokeefe

/r/4489 - Bug 1138063 - Fix up most of the manual test staging rules in c-c; r=jcranmer
/r/4491 - Bug 1138063 - Clean up a bunch of make rules that install things to FINAL_TARGET; r=jcranmer,fallen,florian
/r/4493 - Bug 1138063 - Remove boilerplate stuff from Makefile.ins; r=jcranmer

Pull down these commits:

hg pull -r da6633be61429feae95a68f2667ab3ae23c4fac9 https://reviewboard-hg.mozilla.org/comm-central
Attachment #8570926 - Flags: review?(philipp)
Attachment #8570926 - Flags: review?(florian)
Attachment #8570926 - Flags: review?(bugspam.Callek)
Attachment #8570926 - Flags: review?(Pidgeot18)
Attachment #8570926 - Flags: review+
Comment on attachment 8570926 [details]
MozReview Request: bz://1138063/bokeefe

This is just a straight rebase to the latest tip of c-c, so it probably doesn't need to be re-reviewed (not that I know how to tell mozreview that).

If you're trying to look at interdiffs, the interdiff for the third part is wrong - it replaces part 4 from before, because the part 3 changes already landed in a different bug - but I couldn't convince mozreview to line them up right.

Also, I broke mozreview somehow, so this review isn't even viewable at the moment. I'll file a a bug for that.
Attachment #8570926 - Flags: review?(philipp)
Attachment #8570926 - Flags: review?(florian)
Attachment #8570926 - Flags: review?(Pidgeot18)
Attachment #8570926 - Flags: review+
So what's going on with this issue?
Attachment #8570926 - Attachment is obsolete: true
Attachment #8619625 - Flags: review+
Attachment #8619626 - Flags: review+
Attachment #8619627 - Flags: review+
Attachment #8619628 - Flags: review+
What's stopping these patches from being landed?
Comment on attachment 8639030 [details] [diff] [review]
Unbitrotted Patch [Checked in: Comment 40]

https://hg.mozilla.org/comm-central/rev/031248ef4037
Attachment #8639030 - Attachment description: Unbitrotted Patch → Unbitrotted Patch [Checked in: Comment 40]
Status: ASSIGNED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
Target Milestone: Thunderbird 40.0 → Thunderbird 42.0
Blocks: 1187793
Follow-up to fix capitalization in im/themes: https://hg.mozilla.org/comm-central/rev/21d662cee84d
Depends on: 1188965
You need to log in before you can comment on or make changes to this bug.