Do you think the features should be enabled according to the environment as
well?
```
{:membrane_rtc_engine, "~> 0.10.0", features: [testing_support: [:test]]}
```
Allowing people to ship non-prod code (whatever that means to you) as part
of the library but remove outside testing in this case
On Thursday, April 6, 2023 at 1:09:29 AM UTC-4 [email protected]
wrote:
> I have encountered the very same issue and I think it might be
> half-supported without the necessity to incorporate anything in `hex`.
>
> `Mix.Project.deps/1` callback already accepts `:system_env` configuration
> parameter per dependency, what if we also allow `:config` which would be
> merged into a dependency config during its compilation? That way we might
> rather simplify the dependency tree configuration at least when several
> dependencies came from the same provider, without the necessity to touch
> `hex` at all.
>
> Consider A library optionally depending on B and C, whereas B also
> optionally depends on C. Then A might be included as `{:a, "~> …", config:
> [b: true, sigils: false]}` and then A would know at the compilation stage
> it should “flag C as used, and flag B as used without C, (and make sigil
> macros available.)”
>
> It still does not help much to get proper dependencies from `hex` but at
> least it makes it possible to decrease the amount of boilerplate needed to
> make libs cooperate cohesively.
>
> —AM
>
>
> On Tuesday, March 7, 2023 at 6:22:07 PM UTC+1 [email protected]
> wrote:
>
> I see, thanks! Will think/investigate this more and return when having
> more detailed proposal and justification
> wtorek, 7 marca 2023 o 18:00:05 UTC+1 José Valim napisał(a):
>
> > Also, user doesn't have to think about versions as it includes only one
> dependency so it won't try to plug webrtc endpoint that is incompatible
> with engine.
>
> FWIW, this can be easily addressed by recommending users to add
> {:membrane_plugin, ">= 0.0.0"}, since membrane itself will already restrict
> it to a supported version.
>
> At the end of the day, if the goal is replacing 3 lines:
>
> {:membrane_rtc_engine_webrtc, ..}
> {:membrane_rtc_engine_hls, ...}
> {:membrane_rtc_engine_recorder, ...}
>
> with:
>
>
> {:membrane_rtc_engine, "~> 0.10.0", features: [:webrtc, :hls, :recorder]}
>
> I have to say this is likely not worth it, given the amount of work
> implementing and maintaining this feature would entail in the long term.
>
> I understand why Rust has this feature: compile-time and artefacts are
> much larger there. Plus the fact they can't metaprogram at the file level
> like us means they need explicit configuration for this. So before moving
> on, I think you need to send a more structured proposal, with the problem
> this is going to address, code snippets before and after, and so on. As
> mentioned above, this is a complex feature, so the benefits need to be
> really well justified over what can already be achieved today.
>
>
> On Tue, Mar 7, 2023 at 5:06 PM Michal Śledź <[email protected]>
> wrote:
>
> > I understand better now. To make the issue concise, you would like to
> programmatically include optional dependencies. Today everything is based
> on the dependency name itself but you would like to have an abstraction
> layer for controlling it.
>
> Yes, exactly!
>
> > One potential workaround that you have is to define dependencies to work
> as flags. Let's say you have a realtime feature that requires 3
> dependencies, you can define an optional "membrane_realtime_feature"
> package that, once included, signals the existance of a feature and also
> all of its dependencies. Alghouth it is most likely not a good idea to
> abuse dependencies in this way.
>
> That's what we were thinking about too. In general, we know in the compile
> time which features we are going to need. At the end, user has to plug
> specific endpoints to the Engine on its own. For example:
>
> {:ok, pid} = Membrane.RTC.Engine.start_link()
> Engine.add_endpoint(pid, %HLS{})
> Engine.add_endpoint(pid, %WebRTC{})
> Engine.add_endpoint(pid, %VideoRecorder{})
> etc.
>
> so in general, we could exctract each endpoint (WebRTC, HLS, Recorder)
> into a separate package and I belive this is a pretty elegant solution.
>
> We would end up with:
>
> {:membrane_rtc_engine, ...}
> {:membrane_rtc_engine_webrtc, ..}
> {:membrane_rtc_engine_hls, ...}
> {:membrane_rtc_engine_recorder, ...}
>
> However, allowing user to do
>
> {:membrane_rtc_engine, "~> 0.10.0", features: [:webrtc, :hls, :recorder]}
>
> looks even more attractive to me, and is easier to document as we can list
> all of supported features in the membrane_rtc_engine docs. Also, user
> doesn't have to think about versions as it includes only one dependency so
> it won't try to plug webrtc endpoint that is incompatible with engine.
>
> Regarding:
>
> > I think the biggest concern with implementing this feature is that it
> needs to be part of Hex itself. So the first discussion is if and how to
> extend the Hex registry to incorporate this metadata, which needs to happen
> in Hex first.
>
> and
>
> > In this case, as long as the feature checks are only inclusive, it
> should be fine. But you can also think if a library named A assumes that
> dependency C is compiled without some flag, and library B assumes C is
> compiled with said flag, you will end up with conflicting behaviour.
>
> I don't have answers to those questions but I am willing to investigate
> them and propose more detailed analysis on how we could implement the whole
> concept assuming it sounds valid to you.
>
> wtorek, 7 marca 2023 o 16:34:50 UTC+1 José Valim napisał(a):
>
> I understand better now. To make the issue concise, you would like to
> programmatically include optional dependencies. Today everything is based
> on the dependency name itself but you would like to have an abstraction
> layer for controlling it.
>
> I think the biggest concern with implementing this feature is that it
> needs to be part of Hex itself. So the first discussion is if and how to
> extend the Hex registry to incorporate this metadata, which needs to happen
> in Hex first.
>
> > I also thought that configuring libraries via Application environment is
> discouraged, according to
>
> https://hexdocs.pm/elixir/library-guidelines.html#avoid-application-configuration
>
> <https://hexdocs.pm/elixir/library-guidelines.html#avoid-application-configuration>
>
> Right. Application configuration has many downsides, exactly because it is
> global. The feature mechanism is also global, regardless if we put it on
> mix.exs or on the configuration environment. Rust also hints it is
> configuration (the conditional is called cfg):
> #[cfg(feature = "webp")] pub mod webp;
> In this case, as long as the feature checks are only inclusive, it should
> be fine. But you can also think if a library named A assumes that
> dependency C is compiled without some flag, and library B assumes C is
> compiled with said flag, you will end up with conflicting behaviour.
>
> ---
>
> One potential workaround that you have is to define dependencies to work
> as flags. Let's say you have a realtime feature that requires 3
> dependencies, you can define an optional "membrane_realtime_feature"
> package that, once included, signals the existance of a feature and also
> all of its dependencies. Alghouth it is most likely not a good idea to
> abuse dependencies in this way.
>
> On Tue, Mar 7, 2023 at 4:17 PM Michal Śledź <[email protected]>
> wrote:
>
> 2. Users of a library with optional dependencies have to include all
> optional dependencies in their mix.exs
> I meant that to enable one feature, user has to include a lot of optional
> dependencies, at least in our case.
>
> 3. Users might include bad varsions of optional dependencies
> Here, I meant that user has to exactly know dependency version that has to
> be included. In our case, when there is a lot of optional dependencies, it
> starts getting annoying to keep them up to date in the docs.
>
> Other than that, you should be able to provide this functionality using
> config/config.exs files and the Application.compile_env/2.
> But I cannot manipulate which deps should be downloaded and compiled using
> Application.compile_env, can I? I mean, user still has to include all
> needed dependencies and know their correct versions. I also thought that
> configuring libraries via Application environment is discouraged, according
> to
>
> https://hexdocs.pm/elixir/library-guidelines.html#avoid-application-configuration
>
> <https://hexdocs.pm/elixir/library-guidelines.html#avoid-application-configuration>
>
> We very often depend on native libraries written in C like ffmpeg. When
> it's possible, we make those components optional, so that user is not
> forced to install uneeded native libraries on their system.
>
> I feel like at the moment user has to be aware of which optional deps are
> needed to get the desired feature. What I would like to have is to focus on
> the feature itself, leaving deps and their versions to library maintainers.
>
>
>
>
>
> wtorek, 7 marca 2023 o 14:45:03 UTC+1 José Valim napisał(a):
>
> Hi Michał,
>
> Thanks for the proposal. Your initial description makes me think there may
> exist bugs which we would need to investigate first.
>
> 2. Users of a library with optional dependencies have to include all
> optional dependencies in their mix.exs
>
> This should not be required. You only need to include the dependencies
> that you need, which would be equivalent to opting into a feature in Rust.
>
> 3. Users might include bad varsions of optional dependencies
>
> This should not be possible. The requirement has to match for optional
> dependencies.
>
> If the above is not true, please provide more context.
>
> ---
>
> Other than that, you should be able to provide this functionality using
> config/config.exs files and the Application.compile_env/2. In fact, I think
> introducing another mechanism to configure libraries could end-up adding
> more confusion, especially given how configs changed (and also improved)
> throughout the years.
>
>
>
> On Tue, Mar 7, 2023 at 12:40 PM Michal Śledź <[email protected]>
> wrote:
>
> Currently, using optional dependencies is quite inconvenient and error
> prone:
>
> 1. A lot of modules have to use if Code.ensure_loaded statements
> introducing additional nesting
> 2. Users of a library with optional dependencies have to include all
> optional dependencies in their mix.exs
> 3. Users might include bad varsions of optional dependencies
>
> My proposal is to enhance API for optional dependencies basing on the API
> provided by Cargo in Rust.
>
> The main idea is that the user of a library with optional dependencies
> specify which "features" it is willing to have. For example, in
> membrane_rtc_engine library, which allows you to exchange audio/video using
> different multimedia protocols, we have a lot of optional dependencies
> depending on what protocol the user is willing to use. When the user wants
> to receive media via webrtc and convert it to the HLS to broadcast it to
> the broader audience it has to include all of those dependencies
>
> # Optional deps for HLS endpoint
> {:membrane_aac_plugin, "~> 0.13.0", optional: true},
> {:membrane_opus_plugin, "~> 0.16.0", optional: true},
> {:membrane_aac_fdk_plugin, "~> 0.14.0", optional: true},
> {:membrane_generator_plugin, "~> 0.8.0", optional: true},
> {:membrane_realtimer_plugin, "~> 0.6.0", optional: true},
> {:membrane_audio_mix_plugin, "~> 0.12.0", optional: true},
> {:membrane_raw_audio_format, "~> 0.10.0", optional: true},
> {:membrane_h264_ffmpeg_plugin, "~> 0.25.2", optional: true},
> {:membrane_audio_filler_plugin, "~> 0.1.0", optional: true},
> {:membrane_video_compositor_plugin, "~> 0.2.1", optional: true},
> {:membrane_http_adaptive_stream_plugin, "~> 0.11.0", optional: true},
>
> Instead of this, I would love to say to the user, hi if you want to use
> HLS just specify it in the feature list. For example:
>
> {:membrane_rtc_engine, "~> 0.10.0", features: [:hls]}
>
> It would also be nice to somehow get rid of "if Code.ensure_loaded"
> statements. I am not sure how yet but Rust do this that way
>
> // This conditionally includes a module which implements WEBP support.
> #[cfg(feature
> = "webp")] pub mod webp;
>
> What comes to my mind is that in mix.exs we can specify "features", their
> dependencies and a list of modules. When someone asks for the feature,
> those dependencies are autmatically downloaded and listed modules are
> compiled.
>
> The final proposal is:
>
> # library side
> # mix.exs
>
> features: [
> hls: [
> dependencies: [],
> modules: []
> ]
> ]
>
> # user side
> # mix.exs
>
> {:membrane_rtc_engine, "~> 0.10.0", features: [:hls]}
>
> I would love to help in implementing those features if you decide they are
> valuable
>
> Rust reference:
> https://doc.rust-lang.org/cargo/reference/features.html#features
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "elixir-lang-core" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to [email protected].
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/elixir-lang-core/8385965e-799d-4cea-bcd5-151d9fee6914n%40googlegroups.com
>
> <https://groups.google.com/d/msgid/elixir-lang-core/8385965e-799d-4cea-bcd5-151d9fee6914n%40googlegroups.com?utm_medium=email&utm_source=footer>
> .
>
> --
> You received this message because you are subscribed to the Google Groups
> "elixir-lang-core" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to [email protected].
>
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/elixir-lang-core/d88a5141-86d8-42b8-ae61-71cf58d644a0n%40googlegroups.com
>
> <https://groups.google.com/d/msgid/elixir-lang-core/d88a5141-86d8-42b8-ae61-71cf58d644a0n%40googlegroups.com?utm_medium=email&utm_source=footer>
> .
>
> --
> You received this message because you are subscribed to the Google Groups
> "elixir-lang-core" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to [email protected].
>
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/elixir-lang-core/a0f6ba5c-1a50-4637-90c9-c3968b443377n%40googlegroups.com
>
> <https://groups.google.com/d/msgid/elixir-lang-core/a0f6ba5c-1a50-4637-90c9-c3968b443377n%40googlegroups.com?utm_medium=email&utm_source=footer>
> .
>
>
--
You received this message because you are subscribed to the Google Groups
"elixir-lang-core" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/elixir-lang-core/f04fcaf5-e8a6-455b-8880-17e877734ae4n%40googlegroups.com.
