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

Status: RESOLVED FIXED

(Thunderbird :: Build Config, defect)

Description

[Brian O'Keefe [:bokeefe]]

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.

Comment 1

[Brian O'Keefe [:bokeefe]]

Attachment: 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_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

Comment 2

[Joshua Cranmer [:jcranmer]]

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.

Comment 3

[Joshua Cranmer [:jcranmer]]

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.

Comment 4

[Joshua Cranmer [:jcranmer]]

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.

Comment 5

[Joshua Cranmer [:jcranmer]]

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

Comment 6

[Brian O'Keefe [:bokeefe]]

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.

Comment 7

[Brian O'Keefe [:bokeefe]]

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

Comment 8

[Philipp Kewisch [:Fallen][📱📆]]

https://reviewboard.mozilla.org/r/4491/#review5199

Ship It!

Comment 9

[Joshua Cranmer [:jcranmer]]

https://reviewboard.mozilla.org/r/4491/#review5607

Ship It!

Comment 10

[Joshua Cranmer [:jcranmer]]

https://reviewboard.mozilla.org/r/4489/#review5609

Ship It!

Comment 11

[Joshua Cranmer [:jcranmer]]

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.

Comment 12

[Joshua Cranmer [:jcranmer]]

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 13

[Joshua Cranmer [:jcranmer]]

https://reviewboard.mozilla.org/r/4495/#review5615

Ship It!

Comment 14

[Joshua Cranmer [:jcranmer]]

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!

Comment 15

[Joshua Cranmer [:jcranmer]]

Comment on attachment 8570926 [details]
MozReview Request: bz://1138063/bokeefe

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

Ship It!

Comment 16

[Joshua Cranmer [:jcranmer]]

(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.

Comment 17

[Philip Chee]

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

Comment 18

[Philip Chee]

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

Comment 19

[ISHIKAWA, Chiaki]

Is "Bug 1153559 - Exception: Variable SHARED_LIBRARY_LIBS is defined"
be related to this entry?

TIA

Comment 20

[Philip Chee]

(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/

Comment 21

[Yonggang Luo]

Are you still working on this issue?

Comment 22

[Yonggang Luo]

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.

Comment 23

[Brian O'Keefe [:bokeefe]]

(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.

Comment 24

[Philip Chee]

(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

Comment 25

[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.

Comment 26

[Brian O'Keefe [:bokeefe]]

(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

Comment 27

[Florian Quèze [:florian]]

Comment on attachment 8570926 [details]
MozReview Request: bz://1138063/bokeefe

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

Ship It!

Comment 28

[Yonggang Luo]

Ship It!
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=ba5fca4a9228
Now it's build OK.

Comment 29

[Ian Neal]

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 30

[Brian O'Keefe [:bokeefe]]

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

Comment 31

[Brian O'Keefe [:bokeefe]]

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.

Comment 32

[Yonggang Luo]

So what's going on with this issue?

Comment 34

[Brian O'Keefe [:bokeefe]]

Attachment: MozReview Request: Bug 1138063 - Fix up most of the manual test staging rules in c-c; r=jcranmer

Comment 35

[Brian O'Keefe [:bokeefe]]

Attachment: MozReview Request: Bug 1138063 - Clean up a bunch of make rules that install things to FINAL_TARGET; r=jcranmer,fallen,florian

Comment 36

[Brian O'Keefe [:bokeefe]]

Attachment: MozReview Request: Bug 1138063 - Remove boilerplate stuff from Makefile.ins; r=jcranmer

Comment 37

[Brian O'Keefe [:bokeefe]]

Attachment: MozReview Request: Bug 1138063 - Remove boilerplate stuff from Makefile.ins; r=jcranmer

Comment 38

[Joshua Cranmer [:jcranmer]]

What's stopping these patches from being landed?

Comment 39

[Ian Neal]

Attachment: Unbitrotted Patch [Checked in: Comment 40]

Comment 40

[Ian Neal]

Comment on attachment 8639030 [details] [diff] [review]
Unbitrotted Patch [Checked in: Comment 40]

https://hg.mozilla.org/comm-central/rev/031248ef4037

Comment 41

[Patrick Cloke [:clokep]]

Follow-up to fix capitalization in im/themes: https://hg.mozilla.org/comm-central/rev/21d662cee84d