Closed Bug 1407432 Opened 2 years ago Closed 2 years ago

Install system-wrappers in the tup backend

Categories

(Firefox Build System :: General, enhancement)

enhancement
Not set

Tracking

(firefox58 fixed)

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: mshal, Assigned: mshal)

References

(Blocks 2 open bugs)

Details

Attachments

(6 files)

config/Makefile.in runs a perl script to generate the stl & system header wrappers. We'll need to handle this in the tup backend somehow.
It would be nice to move this to moz.build. I don't think it'd fit nicely into GENERATED_FILES because the scripts generate a set of headers from a single file. We could invent a new moz.build primitive here or move some of this into moz.configure, like read the file and generate the list of targets there, and then pass it to the build backend to execute.
Blocks: nomakefiles
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #1)
> It would be nice to move this to moz.build. I don't think it'd fit nicely
> into GENERATED_FILES because the scripts generate a set of headers from a
> single file. We could invent a new moz.build primitive here or move some of
> this into moz.configure, like read the file and generate the list of targets
> there, and then pass it to the build backend to execute.

I thought it might be a mess too, but it doesn't seem too bad in GENERATED_FILES. The main weird thing is that we pass in only the first file as a FileAvoidWrite, but making the first file a sentinel file works around it pretty well.
Assignee: nobody → mshal
Comment on attachment 8926918 [details]
Bug 1407432 - Remove ucx$inetdef.h from system headers;

https://reviewboard.mozilla.org/r/198152/#review203408
Attachment #8926918 - Flags: review+
Comment on attachment 8926919 [details]
Bug 1407432 - Remove unused private/*.h system wrappers;

https://reviewboard.mozilla.org/r/198154/#review203410
Attachment #8926919 - Flags: review+
Comment on attachment 8926918 [details]
Bug 1407432 - Remove ucx$inetdef.h from system headers;

https://reviewboard.mozilla.org/r/198152/#review203414
Attachment #8926918 - Flags: review+
Attachment #8926918 - Flags: review?(core-build-config-reviews)
Attachment #8926919 - Flags: review?(core-build-config-reviews)
Comment on attachment 8926921 [details]
Bug 1407432 - Move stl wrapper generation into moz.build;

https://reviewboard.mozilla.org/r/198158/#review203416
Attachment #8926921 - Flags: review+
Comment on attachment 8926922 [details]
Bug 1407432 - Move system wrapper generation to moz.build;

https://reviewboard.mozilla.org/r/198160/#review203418

::: config/make-system-wrappers.py:16
(Diff revision 1)
> +# The 'unused' arg is the output file from the file_generate action. We actually
> +# generate all the files in header_list
> +def gen_wrappers(unused, outdir, *header_list):

Does it matter that we're not actually writing anything to the unused file?  Do we need to write *something* to it if we happened to update any of the system header wrappers?  (I guess this applies for STL wrapping too.)

::: config/moz.build:68
(Diff revision 1)
>          stl.script = 'make-stl-wrappers.py:gen_wrappers'
>          stl.flags = [output_dir, stl_compiler, template_file]
>          stl.flags.extend(stl_headers)
> +
> +if CONFIG['WRAP_SYSTEM_INCLUDES']:
> +    include('stl-headers.mozbuild')

Does it matter that we've included `stl-headers.mozbuild` earlier (because of the STL header wrapping)?

::: config/moz.build:71
(Diff revision 1)
> +
> +if CONFIG['WRAP_SYSTEM_INCLUDES']:
> +    include('stl-headers.mozbuild')
> +    include('system-headers.mozbuild')
> +    output_dir = '../dist/system_wrappers'
> +    outputs = tuple(['system-header.sentinal'] + ['%s/%s' % (output_dir, h) for h in stl_headers + system_headers])

Nit: 'system-header.sentinel'; the STL wrapping spells it correctly.

::: config/system-headers.mozbuild:7
(Diff revision 1)
> +# vim: set filetype=python:
> +# 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/.
> +
> +system_headers = [

I am going to assume that all of this got translated correctly.  Did you do it by hand, or did you use a script?
Attachment #8926922 - Flags: review+
Comment on attachment 8926923 [details]
Bug 1407432 - Remove stl-headers & system-headers;

https://reviewboard.mozilla.org/r/198162/#review203420
Attachment #8926923 - Flags: review+
Comment on attachment 8926920 [details]
Bug 1407432 - Remove unused builtins arg from Sandbox;

https://reviewboard.mozilla.org/r/198156/#review203412
Attachment #8926920 - Flags: review+
Attachment #8926920 - Flags: review?(core-build-config-reviews) → review+
Attachment #8926921 - Flags: review?(core-build-config-reviews) → review+
Attachment #8926922 - Flags: review?(core-build-config-reviews) → review+
Attachment #8926923 - Flags: review?(core-build-config-reviews) → review+
Comment on attachment 8926922 [details]
Bug 1407432 - Move system wrapper generation to moz.build;

https://reviewboard.mozilla.org/r/198160/#review203456

::: config/make-system-wrappers.py:16
(Diff revision 1)
> +# The 'unused' arg is the output file from the file_generate action. We actually
> +# generate all the files in header_list
> +def gen_wrappers(unused, outdir, *header_list):

The file still gets written since the file_generate.py action opens it as a FileAvoidWrite. Even though it's a FileAvoidWrite, file_generate updates the mtime so make won't rerun the command more than necessary (bug 1218999). So adding new headers should still work properly.

::: config/moz.build:68
(Diff revision 1)
>          stl.script = 'make-stl-wrappers.py:gen_wrappers'
>          stl.flags = [output_dir, stl_compiler, template_file]
>          stl.flags.extend(stl_headers)
> +
> +if CONFIG['WRAP_SYSTEM_INCLUDES']:
> +    include('stl-headers.mozbuild')

Oops, good catch. I had these in separate moz.build files at one point and forgot to clean that up.

::: config/moz.build:71
(Diff revision 1)
> +
> +if CONFIG['WRAP_SYSTEM_INCLUDES']:
> +    include('stl-headers.mozbuild')
> +    include('system-headers.mozbuild')
> +    output_dir = '../dist/system_wrappers'
> +    outputs = tuple(['system-header.sentinal'] + ['%s/%s' % (output_dir, h) for h in stl_headers + system_headers])

Fixed.

::: config/system-headers.mozbuild:7
(Diff revision 1)
> +# vim: set filetype=python:
> +# 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/.
> +
> +system_headers = [

So it was mostly a direct import with some vim macros to reformat. I did consolidate the redundant android sections by hand, though. Here's the diff before & after the patchset for a local Linux build:

$ diff -r ~/obj-orig/dist/stl_wrappers/ obj-x86_64-pc-linux-gnu/dist/stl_wrappers/
Only in /home/mshal/obj-orig/dist/stl_wrappers/: sentinel
 - this is now objdir/config/stl.sentinel

$ diff -r ~/obj-orig/dist/system_wrappers obj-x86_64-pc-linux-gnu/dist/system_wrappers/
Only in obj-x86_64-pc-linux-gnu/dist/system_wrappers/: be
Only in obj-x86_64-pc-linux-gnu/dist/system_wrappers/: ia64
 - be & ia64 aren't generated correctly in the nsprpub script due to the issue with creating directories mentioned in the commit message
Only in /home/mshal/obj-orig/dist/system_wrappers: .mkdir.done
 - some make junk
Only in /home/mshal/obj-orig/dist/system_wrappers: ucx$inetdef.h
 - Removed by the first commit
Only in /home/mshal/obj-orig/dist/system_wrappers: unicode
 - These are not supposed to be generated, but the MOZ_SYSTEM_ICU check was done incorrectly in the old system-headers file

A local android build has the same diff.
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 6112b8b873e4 -d a91a0cde388f: rebasing 433111:6112b8b873e4 "Bug 1407432 - Remove ucx$inetdef.h from system headers; r=froydnj,ted"
merging config/system-headers
rebasing 433112:af21a4b6f788 "Bug 1407432 - Remove unused private/*.h system wrappers; r=froydnj"
merging config/system-headers
rebasing 433113:3b350dd7ff35 "Bug 1407432 - Remove unused builtins arg from Sandbox; r=froydnj"
rebasing 433114:6734ce8ae069 "Bug 1407432 - Move stl wrapper generation into moz.build; r=froydnj"
merging python/mozbuild/mozbuild/backend/recursivemake.py
rebasing 433115:4d237f5df991 "Bug 1407432 - Move system wrapper generation to moz.build; r=froydnj"
rebasing 433116:83ef94e7cfdb "Bug 1407432 - Remove stl-headers & system-headers; r=froydnj" (tip)
local [dest] changed config/system-headers which other [source] deleted
use (c)hanged version, (d)elete, or leave (u)nresolved? u
unresolved conflicts (see hg resolve, then hg rebase --continue)
This failed because bug 1413362 added 2 new android wrappers:

+vr/gvr/capi/include/gvr.h
+vr/gvr/capi/include/gvr_controller.h

I've rebased and added them to system-headers.mozbuild
Pushed by mshal@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ffe9aed0944f
Remove ucx$inetdef.h from system headers; r=froydnj,ted
https://hg.mozilla.org/integration/autoland/rev/8d770908a5b9
Remove unused private/*.h system wrappers; r=froydnj
https://hg.mozilla.org/integration/autoland/rev/bacf04be0a48
Remove unused builtins arg from Sandbox; r=froydnj
https://hg.mozilla.org/integration/autoland/rev/828d43ec1b16
Move stl wrapper generation into moz.build; r=froydnj
https://hg.mozilla.org/integration/autoland/rev/5a967cc85e28
Move system wrapper generation to moz.build; r=froydnj
https://hg.mozilla.org/integration/autoland/rev/86302df0b38a
Remove stl-headers & system-headers; r=froydnj
Blocks: 1416062
Backed out in https://hg.mozilla.org/integration/autoland/rev/5ee7f274f84b in hopes that sacrificing you will please the gods, and cause them to stop sending us https://treeherder.mozilla.org/logviewer.html#?job_id=143550164&repo=autoland.

Not quite permaorange, but close, not PGO- or Windows-specific, though quite happy to fail there, that's just what we backfilled with. I don't have any reason to believe it's you other than getting 10 green runs on the push below you, but since right now our best choice with autoland is just to throw it away and create a new tree, being backed out of it isn't any real hardship, it ain't going anywhere the way it is.
Or maybe it does have something to do with Win PGO, or maybe it needed a clobber and that's the thing that didn't ever wind up with one. There were a smattering of failures, on various platforms, from when you landed up to when the patch for bug 1409148 landed, then it failed pretty much everywhere, then on the backout of that patch there were a couple of intermittents, but permaorange on Win32 PGO. Somebody retriggered the Win32 PGO build, and assuming it successfully ran tests on the new build (we did in the past have a lovely situation where you pretty much couldn't tell which build tests on a retriggered build actually ran on), it was green there. Based on all of that, umm, I really don't have any clue.
Sigh. Apparently bug 1404427 landed time-of-day bustage, and to make it even more fun we seem to run Linux and Windows in UTC and Mac in Pacific time, so that's not "successfully ran tests on the new build" but rather "ran tests on the new build after that OS had passed out of the time-of-day bustage period."
Pushed by philringnalda@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/88e0c2a4e725
Remove ucx$inetdef.h from system headers; r=froydnj,ted
https://hg.mozilla.org/integration/autoland/rev/3258f49e8c73
Remove unused private/*.h system wrappers; r=froydnj
https://hg.mozilla.org/integration/autoland/rev/94f475532485
Remove unused builtins arg from Sandbox; r=froydnj
https://hg.mozilla.org/integration/autoland/rev/ccc34ca6ecfd
Move stl wrapper generation into moz.build; r=froydnj
https://hg.mozilla.org/integration/autoland/rev/04b2f4aca51f
Move system wrapper generation to moz.build; r=froydnj
https://hg.mozilla.org/integration/autoland/rev/499c60003ff1
Remove stl-headers & system-headers; r=froydnj
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.