2014246 - Allow specifying cargo features for Rust programs from moz.build

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement)

Description

[jmendez]

For rust crates that are libraries, there is a mozbuild mechanism for specifying the features to enable in the crate, passing features to the RustLibrary() template. The same approach should also be available for rust programs.

Comment 1

[jmendez]

Attachment: Bug 2014246 - Part 1: Add features to [Host]RustProgram objects and moz.build definitions r?glandium

Comment 2

[jmendez]

Attachment: Bug 2014246 - Part 2: Handle Rust program features in the recursivemake backend r?glandium

Comment 3

[jmendez]

Attachment: Bug 2014246 - Part 2: Add tests for Rust program features, frontend and backend r?glandium

Comment 4

[jmendez]

For context: I encountered the need for this when wanting to add code specific to Firefox Enterprise to the crashreporter crate. We want to override the telemetry URL for crash pings. I discovered that there was a mechanism for setting Rust features with a RustLibrary template but crashreporter is instead a RustProgram. Because the code paths are similar, I felt it would be easy/good to add support for RustProgram as well.

Comment 5

[Mike Hommey [:glandium]]

For consistency, I think this is a useful change, but I'm not convinced a features is the right way to handle your original need.

Comment 6

[jmendez]

(In reply to Mike Hommey [:glandium] from comment #5)

For consistency, I think this is a useful change, but I'm not convinced a features is the right way to handle your original need.

Can you share more about why features is not a good fit or what you suggest instead?

Comment 7

[Mike Hommey [:glandium]]

Features are on/off, they don't give you the liberty to give arbitrary values. Plus, it seems a url shouldn't be hardcoded in the code or even in the binary in the first place. There are even TODO items in the crashreporter client code to that effect.

Comment 8

[jmendez]

I think maybe you are concerned that I am trying to add a hardcoded url. If that is the concern, rest assured my plan is somewhat the opposite.
The feature will be an on/off for "is this the enterprise build", tied to MOZ_ENTERPRISE.
And then if MOZ_ENTERPRISE, I override the hardcoded glean url (in the default config from the crashping crate) with a url that is read from the enterprise distribution.ini file.
You can see my WIP here if you want to see more concretely: https://github.com/mozilla/enterprise-firefox/pull/402/changes

If the concerns are deeper than that, let me know and I can find a better place to have the conversation and pull in someone from the crash reporting team.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/f323346dad87
https://hg.mozilla.org/integration/autoland/rev/3916ad9cc0c2
Part 1: Add features to [Host]RustProgram objects and moz.build definitions r=glandium
https://github.com/mozilla-firefox/firefox/commit/0d4122d2dba4
https://hg.mozilla.org/integration/autoland/rev/d170f05dbbc1
Part 2: Add tests for Rust program features, frontend and backend r=glandium

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/499871c8492b
https://hg.mozilla.org/integration/autoland/rev/4e27d8fa4c7c
Revert "Bug 2014246 - Part 2: Add tests for Rust program features, frontend and backend r=glandium" for causing python-mozbuild failures

Comment 11

[Cristian Tuns]

Backed out for causing python/mozbuild failures

• Backout link
• Push with failures
• Failure Log
• Failure line: python/mozbuild/mozbuild/test/backend/test_recursivemake.py::TestRecursiveMakeBackend::test_host_rust_library_with_features TEST-UNEXPECTED-FAIL

Comment 12

[Mike Hommey [:glandium]]

(In reply to jmendez from comment #8)

I think maybe you are concerned that I am trying to add a hardcoded url. If that is the concern, rest assured my plan is somewhat the opposite.
The feature will be an on/off for "is this the enterprise build", tied to MOZ_ENTERPRISE.
And then if MOZ_ENTERPRISE, I override the hardcoded glean url (in the default config from the crashping crate) with a url that is read from the enterprise distribution.ini file.

This doesn't seem like this needs to be specific to MOZ_ENTERPRISE, although for a generic approach, using distribution.ini might not be where we'd want to go.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/166d7c95d1ba
https://hg.mozilla.org/integration/autoland/rev/6baaee992d4e
Part 1: Add features to [Host]RustProgram objects and moz.build definitions r=glandium
https://github.com/mozilla-firefox/firefox/commit/239c76b4abba
https://hg.mozilla.org/integration/autoland/rev/70d3eb481ac5
Part 2: Add tests for Rust program features, frontend and backend r=glandium

Comment 14

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/6baaee992d4e
https://hg.mozilla.org/mozilla-central/rev/70d3eb481ac5

Comment 15

[jmendez]

clearing needinfo after fixing the backout