Deprecate availability key in nif_versions

[josevalim]

Instead, we should allow something such as:

versions: fn arg1, arg2 -> ... end

This will lead to a smaller surface API. :)

But let's do this after merging #79. /cc @cocoa-xu

[cocoa-xu]

I'm wondering that if we should redesign this to something like

versions: fn mode, target ->
  ["2.14"]
end

where mode can be either :list or :reuse, indicating we're trying to get a list of all available NIF versions for a target (for generating the checksum file) or inquire which NIF version to reuse. If the NIF library uses NIF version 2.14 for all targets in all circumstances, then they can simply return ["2.14"].

For NIF libraries that have different NIF versions available for different targets and rules for newer/older NIF versions, they can do something like

versions: fn mode, target ->
  case mode do
    :list ->
      if String.contains?(target, "foo") do
        ["2.14", "2.17"]
      else
        ["2.14"]
      end

    :reuse ->
      current_nif_version = "#{:erlang.system_info(:nif_version)}"
      if current_nif_version >= "2.18" do
        :compile
      else
        if String.contains?(target, "bar") do
          "2.17"
        else
          "2.14"
        end
      end
  end
end

[josevalim]

@cocoa-xu now that we merged, we could discuss this. In my opinion, I would go with versions is either a list or a function that receives target, current_nif_version. This way we don't need availability any more. Thoughts?

[cocoa-xu]

@cocoa-xu now that we merged, we could discuss this. In my opinion, I would go with versions is either a list or a function that receives target, current_nif_version. This way we don't need availability any more. Thoughts?

I agree with you. And perhaps we don't need to pass current_nif_version as the users can get it using

current_nif_version = "#{:erlang.system_info(:nif_version)}"

but this is just a minor detail, I'm okay with either one.

Another thing is that, I think we still need to know the full list for a specific target when generating the checksum file from somewhere. This could come from the same versions key if we pass one more argument, say mode which can be either :checksum or :reuse, so that this function can distinguish between if we are asking for a full list for the target or which version(s) to reuse for the target.

For example, let's say that the NIF library has two versions of binaries, one is available for all platforms and using NIF version 2.14, while the other one uses NIF version 2.17 and requires the user to install some dependencies and set an environment variable/a key in the config.exs to enable it. (Or the other way around, the NIF library uses newer versions by default, and requires some extra settings to use the older versions for compatibility)

In the situation above, this function has to know if we're asking for a full list of a target for generating the checksum file, or if we're trying to reuse a precompiled binary. It cannot return the full list in the latter case and let us pick one because the default rule to pick the latest (or the oldest) possible version may not be the behaviour they want.

WDYT?

[josevalim]

I agree with you. And perhaps we don't need to pass current_nif_version as the users can get it using

👍

In the situation above, this function has to know if we're asking for a full list of a target for generating the checksum file, or if we're trying to reuse a precompiled binary.

How do we deal with this scenario today?

[cocoa-xu]

In the situation above, this function has to know if we're asking for a full list of a target for generating the checksum file, or if we're trying to reuse a precompiled binary.

How do we deal with this scenario today?

Currently (as of the code in the master branch) we don't really handle this scenario because the function passed in the availability key was designed to filter out versions that are not available for a certain target, and is used in available_target_urls/2. It will return true if that version is generally available and doesn't know if we're trying to reuse a precompiled one based on any extra environment variable or configs in the config.exs.

[josevalim]

Perfect, I understand it now. Do you have any use case for this feature? Because one option is to not worry about this feature for now and we support only versions: fn target -> and then, if such a need arises in the future, we can support functions with arity-2 where the second argument is the mode. WDYT?

[cocoa-xu]

Do you have any use case for this feature?

Not really. So perhaps we can wait if anyone requests this feature. ;)

we support only versions: fn target ->

Maybe we can pass a keyword argument to versions, like

versions: fn opts ->
  target =  opts[:target]
end

And if we need to send more parameters to versions in the future, it won't break libraries (in dependencies) that are not upgraded yet. For the ones that use newer versions of elixir_make, they can try to retrieve more parameters from opts.

versions: fn opts ->
  target =  opts[:target]
  option2 = opts[:option2]
end

WDYT?

[josevalim]

I think a map will be better but we should change fallback_version to also receive a map. WDYT?

[cocoa-xu]

yea, let's also change that to a map/

[josevalim]

Beautiful. Would you like to send a PR with the changes? It would be very appreciated, thank you :)