That's a good question pon., 8 maj 2023 o 20:50 Yordis Prieto <yordis.pri...@gmail.com> napisał(a):
> 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 aleksei.m...@kantox.com > 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 michal...@swmansion.com >> 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ź <michal...@swmansion.com> >> 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ź <michal...@swmansion.com> >> 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ź <michal...@swmansion.com> >> 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 elixir-lang-co...@googlegroups.com. >> 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 elixir-lang-co...@googlegroups.com. >> >> 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 elixir-lang-co...@googlegroups.com. >> >> 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 elixir-lang-core+unsubscr...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/elixir-lang-core/f04fcaf5-e8a6-455b-8880-17e877734ae4n%40googlegroups.com > <https://groups.google.com/d/msgid/elixir-lang-core/f04fcaf5-e8a6-455b-8880-17e877734ae4n%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 elixir-lang-core+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CAN6WWjtLyXeBLQbZmyyLcj%2BX1NLsOhos%3DOBopWvidGoGU6_Xhw%40mail.gmail.com.