900974 - Remove custom export source copying rules for cpp files

Status: NEW

(Firefox Build System :: General, defect)

Description

[Michael Shal [:mshal]]

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.

Comment 1

[Michael Shal [:mshal]]

Attachment: 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?)

Comment 2

[Michael Shal [:mshal]]

Attachment: Test-2-Use-topsrcdir-paths.patch

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

Comment 3

[Michael Shal [:mshal]]

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.

Comment 4

[(not currently active) Ted Mielczarek]

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

[Gregory Szorc [:gps]]

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.

Comment 6

[(not currently active) Ted Mielczarek]

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

[Gregory Szorc [:gps]]

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

Comment 8

[(not currently active) Ted Mielczarek]

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.

Comment 9

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Do we have anything left here after bug 865673?