Closed
Bug 961757
Opened 11 years ago
Closed 11 years ago
Window sandbox no longer building when MOZ_CONTENT_SANDBOX is defined due to include order
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla29
People
(Reporter: bbondy, Assigned: bbondy)
References
Details
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
glandium
:
review+
|
Details | Diff | Splinter Review |
The Windows sandbox works with our code by including the chromium sandbox, and overriding some includes to first add/modify some functionality. For this to work it includes a shim directory before the local include directory.
This used to work before we included the LOCAL_INCLUDES before the current directory, but no longer does.
This bug is to fix that build problem now that the current directory is included first.
Assignee | ||
Comment 1•11 years ago
|
||
Attachment #8362564 -
Flags: review?(mh+mozilla)
Comment 2•11 years ago
|
||
Comment on attachment 8362564 [details] [diff] [review]
bug961757_shim_includes.diff
Review of attachment 8362564 [details] [diff] [review]:
-----------------------------------------------------------------
I'm reluctant do add global build system hacks for one specific use case.
Let's step back a little. Looking at security/sandbox/base/shim, there's only one file that overlaps with security/sandbox: base/strings/string_piece.h. None of the other files overlap. All of them, though, are included from some code in security/sandbox.. but the paths for histogram.h and sparse_histogram.h are wrong: they are in security/sandbox/base/shim/metrics, and they are #included as base/metrics/*, so there's no way those includes work. And, as a matter of fact, the two files that include those are not built.
Now, after writing all this, and considering writing more, i realize that there's a simple fix for this: move security/sandbox/base to security/sandbox/chromium/base. Then add shim and chromium in the right order in LOCAL_INCLUDES (and change the path to source files in moz.build). While doing that, you might as well move security/sandbox/base/shim to security/sandbox/shim.
Attachment #8362564 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 3•11 years ago
|
||
Implemented review comments, will request review after it passes try tests.
One note is I had to put it in a directory called chromium/sandbox instead of just chromium because some includes specify sandbox/base
Attachment #8362564 -
Attachment is obsolete: true
Assignee | ||
Comment 4•11 years ago
|
||
Thanks for the previous review, implemented review comments.
Attachment #8364631 -
Attachment is obsolete: true
Attachment #8365452 -
Flags: review?(mh+mozilla)
Updated•11 years ago
|
Attachment #8365452 -
Flags: review?(mh+mozilla) → review+
Assignee | ||
Comment 5•11 years ago
|
||
Target Milestone: --- → mozilla29
Comment 6•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Updated•7 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•