If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Remove custom export source copying rules for cpp files

NEW
Unassigned

Status

()

Core
Build Config
4 years ago
4 years ago

People

(Reporter: mshal, Unassigned)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

4 years ago
Get rid of the custom export: rules that are used to copy/symlink .cpp files into the local directory before compiling. This should be pretty straightforward since we can compile .cpp files from other directories now without VPATH.
(Reporter)

Comment 1

4 years ago
Created attachment 785008 [details] [diff] [review]
Test-1-share-xpcom-glue-source-definitions.patch

Example 1 - use relative paths (is there a better way to automatically prepend '../glue/' in shared.mozbuild?)
(Reporter)

Comment 2

4 years ago
Created attachment 785009 [details] [diff] [review]
Test-2-Use-topsrcdir-paths.patch

Example 2 - use "global" paths relative to topsrcdir, with mozbuild hacked up to prepend $(topsrcdir) in backend.mk
(Reporter)

Comment 3

4 years ago
These two examples aren't review-ready, they're just here to show what a solution might look like. Anyone have any preferences, or a better solution?

I realize in the future we can probably do this all in one directory when we have a grammar available to compile the same srcs multiple times with different flags, but for now this is an easy way to get rid of these silly export: rules.
I much prefer example 2.

An alternate solution here would be to provide some kind of "abspath" function in the moz.build sandbox, so you could write something like:
xpcom_glue_srcs = abspath([
    'AppData.cpp',
    ...
])

Comment 5

4 years ago
I like example 2 as well. I agree with Ted about providing extra functions in the sandbox to facilitate less suckitude.

Something else we can do is provide a primitive to read any moz.build file. Currently, we have to parse them via DIRS* or include(). include() is funky because paths in the included file are relative to the including file. I think if we provided something like DIRS that evaluated the moz.build file in the context of its directory that a lot of these path problems would go away. The tricky part here is figuring out how to hook it up to the build backend.
Could we just add an optional parameter to include() to indicate that paths should be relative to the file being included? (I am failing to think up a clear name for this parameter.)

Comment 7

4 years ago
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #6)
> Could we just add an optional parameter to include() to indicate that paths
> should be relative to the file being included? (I am failing to think up a
> clear name for this parameter.)

That's kinda hard. An include() is effectively an eval(). If both the including and included "scopes" mutate the same variable, we need to perform path munging at mutation time rather than post execution. That significantly changes the characteristics of the Python sandbox by moving validation/verification/normalization to execute rather than post-execute time. It would require us to bake more logic into the classes assigned to the sandbox and I believe could quickly grow unwieldy.
That makes sense. So I guess our options are either mshal's option 2 (absolute paths) or an explicit abspath function that makes paths absolute to the file being evaluated.
(Reporter)

Updated

4 years ago
Blocks: 786534
Do we have anything left here after bug 865673?
You need to log in before you can comment on or make changes to this bug.