ruma icon indicating copy to clipboard operation
ruma copied to clipboard

Allow selecting owned identifier memory representation via regular environment variable

Open jplatte opened this issue 2 years ago • 7 comments

Currently, it is only possible to select the memory representation (Box or Arc) for identifiers (docs) via a RUSTFLAGS --cfg setting. Changing RUSTFLAGS leads to the entire dependency chain being recompiled. As a cheaper solution, we can read an environment variable in ruma-commons build script, and emit

cargo:rustc-cfg=ruma_identifiers_storage=ENV_VARIABLE_VALUE
cargo:rerun-if-env-changed=ENV_VARIABLE_NAME

(docs)

Then when people change the environment variable, only ruma-common and its dependents will be rebuilt.

jplatte avatar Jan 25 '23 17:01 jplatte

Doesn't this require a build.rs?

ShadowJonathan avatar Feb 02 '23 11:02 ShadowJonathan

Yes, is that problematic?

jplatte avatar Feb 02 '23 11:02 jplatte

I don't think so, but just making sure

ShadowJonathan avatar Feb 02 '23 11:02 ShadowJonathan

What is the benefit over using cargo features?

HKalbasi avatar Feb 05 '23 15:02 HKalbasi

Cargo features are additive and can be enabled from any place in the dependency tree. For the owned identifier representation, one specific value must be set (it wouldn't make sense to activate multiple ones).

jplatte avatar Feb 05 '23 15:02 jplatte

It can become a compiler error to activate multiple ones. This has potential to make some libraries unusable with each other, but I think in practice all libraries will delegate this decision to the main binary (specially when docs encourage that). I would prefer cargo features for better tooling support and less problems like this, but YMMV.

HKalbasi avatar Feb 05 '23 16:02 HKalbasi

If it'll become a compiler error, then multiple libraries will become mutually exclusively incompatible with eachother, which is not what we want

ShadowJonathan avatar Feb 05 '23 17:02 ShadowJonathan