Open Bug 1439645 Opened 6 years ago Updated 1 year ago

Introduce generate_spidermonkey.rs as a build step for SpiderMonkey

Categories

(Firefox Build System :: General, enhancement)

enhancement

Tracking

(Not tracked)

People

(Reporter: Yoric, Unassigned)

References

Details

generate_spidermonkey.rs is a (currently external) tool written in Rust that produces C++ source code implementing much of bug 1437004.

We should make it part of our build system.

At the time of this writing, generate_spidermonkey.rs lives at https://github.com/binast/binjs-ref/blob/master/crates/binjs_meta/examples/generate_spidermonkey.rs
gps, I'll need some help getting started, could you give me a hand?

1/ How do I define a new code generation step in the build system?

2/ How can it call `cargo run` or equivalent?

3/ For dependent libraries,can I use the regular `mach vendor` here or do I need something special?
Flags: needinfo?(gps)
First, https://firefox-source-docs.mozilla.org/build/buildsystem/rust.html has some potentially useful info.

For dependent libraries, `mach vendor rust` should be used. python/mozbuild/mozbuild/vendor_rust.py defines a list of crate roots under `def vendor(` that we iterate over to perform `cargo vendor`. So if we introduce a new Rust tool that requires vendored dependencies, it should be listed here.

We have a mechanism for defining Rust programs to be compiled for the host architecture. That's HOST_RUST_PROGRAMS in moz.build. Although I don't think anyone uses it yet. I'm not sure we want to be using it here though.

We already have a build mechanism in stylo bindgen that runs Rust as part of build.rs to generate Rust code. But this request is a bit different because we're talking about building and running Rust as a prerequisite to build C++.

The "export" tier in the build system is used to host all these "codegen" dependencies that need to run before we can build C/C++/Rust. But I don't believe we have a mechanism for building/invoking Rust from "export" at current. It is something we've talked about and we know we want to support it. You may be our first "customer" :)

We do have a GENERATED_FILES primitive in moz.build to declare actions used to generate other files. If the outputs listed by that entity contain e.g. .cpp files, it is automatically executed during the "export" tier.

I'm just not sure how exactly we should be invoking the Rust compiler from the build system. We /could/ use GENERATED_FILES with a custom build action that runs rustc/cargo. But this also feels like the kind of thing where we want proper moz.build support for defining a Rust host binary to run as part of the build.

froydnj/glandium: do you have any thoughts?
Flags: needinfo?(nfroyd)
Flags: needinfo?(mh+mozilla)
Flags: needinfo?(gps)
(In reply to Gregory Szorc [:gps] from comment #2)
> We have a mechanism for defining Rust programs to be compiled for the host
> architecture. That's HOST_RUST_PROGRAMS in moz.build. Although I don't think
> anyone uses it yet. I'm not sure we want to be using it here though.

We do have this, and we aren't using it anywhere yet, but we do have tests for it, so it might work:
https://dxr.mozilla.org/mozilla-central/search?tree=mozilla-central&q=HOST_RUST_PROGRAMS


> We already have a build mechanism in stylo bindgen that runs Rust as part of
> build.rs to generate Rust code. But this request is a bit different because
> we're talking about building and running Rust as a prerequisite to build C++.
> 
> The "export" tier in the build system is used to host all these "codegen"
> dependencies that need to run before we can build C/C++/Rust. But I don't
> believe we have a mechanism for building/invoking Rust from "export" at
> current. It is something we've talked about and we know we want to support
> it. You may be our first "customer" :)

This is true, and we may have to cross this bridge eventually, but right now it would be very hard to make this work because with the recursive make backend we treat each cargo manifest as a single entity, so you'd wind up blocking C++ compilation on Rust compilation, which would be bad. We might have a better story with the tup backend, where tup will consume the cargo build graph--it's plausible that we could then ask tup to add dependencies on individual bits of that build graph, so that a Rust build script could output C++ code and C++ compilation would only have to block on that subgraph.

> We do have a GENERATED_FILES primitive in moz.build to declare actions used
> to generate other files. If the outputs listed by that entity contain e.g.
> .cpp files, it is automatically executed during the "export" tier.
> 
> I'm just not sure how exactly we should be invoking the Rust compiler from
> the build system. We /could/ use GENERATED_FILES with a custom build action
> that runs rustc/cargo. But this also feels like the kind of thing where we
> want proper moz.build support for defining a Rust host binary to run as part
> of the build.

This is not well-traveled territory in any sense. AFAICT we only have one instance in the tree of invoking a host binary as part of `GENERATED_FILES`, and it's here:
https://dxr.mozilla.org/mozilla-central/rev/dc70d241f90df43505ece5ac12261339e9694c50/layout/style/test/moz.build#140

Given that that evidently works, I think we ought to be able to make the equivalent thing work with `HOST_RUST_PROGRAMS`.
Side-note on GENERATED_FILES: as far as I can tell, we don't want to invoke the Rust compiler directly, nor the executable directly. Rather, we want to run `cargo run` from the right directory, and unless we have some black magic that prevents this from working, this will handle the compile-for-host + run-on-host.
(In reply to David Teller [:Yoric] (please use "needinfo") from comment #4)
> Side-note on GENERATED_FILES: as far as I can tell, we don't want to invoke
> the Rust compiler directly, nor the executable directly. Rather, we want to
> run `cargo run` from the right directory, and unless we have some black
> magic that prevents this from working, this will handle the compile-for-host
> + run-on-host.

We don't currently have any support for invoking `cargo run`. `HOST_RUST_PROGRAMS` will invoke `cargo build --target=<host type>` during the build, and `cargo run` doesn't do anything magical, it just ensures the binary is built and runs it. I think making that work with `GENERATED_FILES` would be a small amount of work atop what we already have. Supporting `cargo run` for this usecase would be a fair bit more work for minimal gain.
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #5)
> (In reply to David Teller [:Yoric] (please use "needinfo") from comment #4)
> > Side-note on GENERATED_FILES: as far as I can tell, we don't want to invoke
> > the Rust compiler directly, nor the executable directly. Rather, we want to
> > run `cargo run` from the right directory, and unless we have some black
> > magic that prevents this from working, this will handle the compile-for-host
> > + run-on-host.
> 
> We don't currently have any support for invoking `cargo run`.
> `HOST_RUST_PROGRAMS` will invoke `cargo build --target=<host type>` during
> the build, and `cargo run` doesn't do anything magical, it just ensures the
> binary is built and runs it. I think making that work with `GENERATED_FILES`
> would be a small amount of work atop what we already have. Supporting `cargo
> run` for this usecase would be a fair bit more work for minimal gain.

Agreed.  You want to:

1. Make your program build as a HOST_RUST_PROGRAMS (sic).
2. Add support to GENERATED FILES for invoking things from HOST_RUST_PROGRAMS.  In makefile terms, the rule(s) for doing this will depend on #1.
3. Write the moz.build code doing what you want using bits from #1 and #2.

Maybe #2 uses `cargo run`, maybe it doesn't.  Whatever works!
Flags: needinfo?(nfroyd)
Flags: needinfo?(mh+mozilla)
A few questions:

* Where can I find the source code I'll need to change for 2.?

* Is there any way to pass arguments to my rust program or should I hardcode the file names?
Flags: needinfo?(nfroyd)
So as it turns out, there's not any actual special handling that makes the case I linked in comment 3 work, it just works because things fall out that way. Here's where the build system handles `GENERATED_FILES` from moz.build files:
https://dxr.mozilla.org/mozilla-central/rev/2c000486eac466da6623e4d7f7f1fd4e318f60e8/python/mozbuild/mozbuild/frontend/emitter.py#1409

If you look through you can see where it handles the inputs: `for i in flags.inputs:`. The only check done there is that if the input comes from the srcdir, it must exist. If the input is from the objdir (like `!foo`) we just pass it through to the build backend.

The recursive make backend passes all of those inputs through its `_pretty_path` method:
https://dxr.mozilla.org/mozilla-central/rev/2c000486eac466da6623e4d7f7f1fd4e318f60e8/python/mozbuild/mozbuild/backend/recursivemake.py#546

That translates paths into ones containing make variables, like `$(srcdir)/foo`. For a relative objdir path like `!foo` it simply winds up as `foo` in the makefile, because the makefile commands are run in that subdirectory in the objdir. If you look at $objdir/layout/style/test/backend.mk to see what the `GENERATED_FILES` instance for css_properties.js translates into, you'll find:

```
misc:: css_properties.js
GARBAGE += css_properties.js
EXTRA_MDDEPEND_FILES += css_properties.js.pp
css_properties.js: /build/mozilla-central/layout/style/test/gen-css-properties.py $(srcdir)/css_properties_like_longhand.js host_ListCSSProperties
        $(REPORT_BUILD)
        $(call py_action,file_generate,/build/mozilla-central/layout/style/test/gen-css-properties.py main css_properties.js $(MDDEPDIR)/css_properties.js.pp $(srcdir)/css_properties_like_longhand.js host_ListCSSProperties)
```

That is, one of its dependencies is simply `host_ListCSSProperties`, and since that is also in `HOST_SIMPLE_PROGRAMS` there's a rule to build it, so things work as expected.

For Rust programs/libs we do have a rule like that to build the binaries, it simply depends on the rules like `force-cargo-host-program-build` to invoke cargo to do the work:
https://dxr.mozilla.org/mozilla-central/rev/2c000486eac466da6623e4d7f7f1fd4e318f60e8/config/rules.mk#1032

The only quirk here is that things like `$(RUST_PROGRAMS)` or `$(RUST_HOST_PROGRAMS)` include the path into the cargo output directory. For example, in $objdir/testing/geckodriver/backend.mk in a build with `--enable-geckodriver`, you'll find:

RUST_PROGRAMS += x86_64-unknown-linux-gnu/debug/geckodriver

...so if you were to do something like `GENERATED_FILES['foo'].inputs += ['!x86_64-unknown-linux-gnu/debug/geckodriver']` that would work, but not on all platforms or build configurations, since it has both the target and the build type in there.

We'd want to add some way to say something simpler like `GENERATED_FILES['foo'].inputs += ['geckodriver']` and have that work properly, so that the resulting backend.mk winds up with a rule like `foo: x86_64-unknown-linux-gnu/debug/geckodriver`.

I don't have a great solution to mind here. The best thing I can think of would be to support a syntax for something like Bazel's "labels" which they use for referencing other build targets:
https://docs.bazel.build/versions/master/build-ref.html#labels

Perhaps something like `GENERATED_FILES['foo'].inputs += [':geckodriver']` to reference the target named `geckodriver` in the current moz.build file? I've long wanted a way to explicitly specify dependencies between targets in moz.build files, so growing that capability would be generally useful.
(In reply to David Teller [:Yoric] (please use "needinfo") from comment #7)
> * Is there any way to pass arguments to my rust program or should I hardcode
> the file names?

I spent a lot of words on your first point, but not your second. If you use `GENERATED_FILES` then you'll need to have a Python script that will get invoked by the build system. Assuming we sort out a way for you to pass the path to the Rust binary as one of its `inputs`, the script will receive that path as an argument and you can invoke the program any way you like. If you look at the example in layout/style/test, its script just runs the program and writes its output to a file:
https://dxr.mozilla.org/mozilla-central/rev/dc70d241f90df43505ece5ac12261339e9694c50/layout/style/test/gen-css-properties.py

You could write things that way, or you could pass a file path to the program and have it write it out, or whatever works best. (Note that the first argument to the Python script is the output file, but it's not a path, it's an open file descriptor.)
Ted has basically answered everything I think I would have.

Reading through his response, I also realized that part of my mental model for this was wrong.  I was envisioning having the Rust program be the GENERATED_FILES['x'].script, but that's not a good idea, because the Python infrastructure ensures that dependencies (e.g. Python modules loaded) are correctly communicated to the build system.  So maybe passing in the compiled Rust binary is the right way to do things; the compiled Rust binary will just have to be careful about loading files that aren't explicitly passed to it by way of the Python script.
Flags: needinfo?(nfroyd)
Product: Core → Firefox Build System
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.