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.

Reply via email to