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/09209333-1ccd-4d7c-a681-623340f12bdfn%40googlegroups.com.

Reply via email to