philss / rustler_precompiled

Use precompiled NIFs from trusted sources in your Elixir code
180 stars 27 forks source link

It's almost impossible to build packages who depend on this with Nix #71

Open blaggacao opened 4 months ago

blaggacao commented 4 months ago

Ref: https://sidhion.com/blog/posts/using-rustler-nix/

TL;DR;

  libmjml_nif = rustPlatform.buildRustPackage rec {
    pname = "mjml_nif";
    version = "3.1.0";
    sourceRoot = "${src.name}/native/mjml_nif";

    src = fetchFromGitHub {
      owner = "adoptoposs";
      repo = "mjml_nif";
      rev = "v${version}";
      sha256 = "sha256-jqMjGO0g3tzZPMl0LoREaQ1DgndLOa1qugBsERYdooY=";
    };
    cargoSha256 = "sha256-oNCuNQorayi7VaOSV9iEhvjLCFYnfgrMhCBSarNY6Kg=";
  };

To reach minimum compatibility level for all packages depending on rustler_precompiled, we'd need a way to securely (so that it can't be misused by non-nix users) pass the shared libraries as a dependency.

The article suggests a pre-build step to put the necessary files in place:

elixir_project = beamPackages.mixRelease {
  inherit src pname version mixNixDeps;

  preBuild = ''
    mkdir -p priv/native
    cp ${libmjml_nif}/lib/libmjml_nif.so priv/native
  '';
};

This may require another banch in the download_or_reuse_nif_file function in addition to the branch checking for the cached file:

    if File.exists?(cached_tar_gz) do
      # Remove existing NIF file so we don't have processes using it.
      # See: https://github.com/rusterlium/rustler/blob/46494d261cbedd3c798f584459e42ab7ee6ea1f4/rustler_mix/lib/rustler/compiler.ex#L134
      File.rm(lib_file)

      with :ok <- check_file_integrity(cached_tar_gz, nif_module),
           :ok <- :erl_tar.extract(cached_tar_gz, [:compressed, cwd: Path.dirname(lib_file)]) do
        Logger.debug("Copying NIF from cache and extracting to #{lib_file}")
        {:ok, result}
      end
    else

Since the user is in control of the build directory, and a secure build will have to happen in a secured build directory, I consider just accepting the lib_file = Path.join(native_dir, file_name), if in place, is not a security issue, right?

Munksgaard commented 4 months ago

I've also run into this (multiple times). It seems like the caching of prebuilt nifs can also cause problems when building a nix flake.

philss commented 4 months ago

I will investigate this issue in the next week. I honestly don't know much about Nix, but seems to be fine to load from a "custom path", since we do check for the SHA256 of the file before loading it.

Do you think RustlerPrecompiled could generate package manifests for loading the file? I don't know if this would work because everyone would need to be willing to support Nix, but we could at least have generators.

blaggacao commented 4 months ago

Do you think RustlerPrecompiled could generate package manifests for loading the file? [...], but we could at least have generators.

Nope, I can't see how this would be of much use. It's a nice idea in principle, put the code generation in the Nix ecosystem is almost non-existent so, in my opinion, there's little use tho prototype this from the "periphery" so to speak because it wouldn't connect well with existing ecosystem practices.

I don't know if this would work because everyone would need to be willing to support Nix

That could turn out to be quite a contentious requirement, from my experience.

Rather, I could see if I could make a Nix Wiki entry once we have a working solution. That would be more in line with the current ecosystem practices.

blaggacao commented 3 months ago

@philss I feel the ones engaged in packaging plausible analytics in nixpkgs start coming together to bump the ecosystem to the new version.

In that context, I think this issue might increasingly becoming a hard blocker, unfortunatly. I had temporarily done some manual patches across 3 transitive dependencies, but I don't know enough elixir to reliably come up with a good patch strategy in code without starting to break other stuff.

I feel, I have to rely on upstream a little for the knowledge I miss.

Would it be possible to bump this in prio as far as anyhow possible? I almost feel a bit bad for asking. Thanks for this great lib!

philss commented 3 months ago

Would it be possible to bump this in prio as far as anyhow possible? I almost feel a bit bad for asking. Thanks for this great lib!

@blaggacao no problem! This is my priority this week. Thanks for the replys!

I will think a bit about how to proceed here.

Just one question: would be OK to accept the path for a ".tar.gz" file, and then let the build of the lib that is using RustlerPrecompiled extract the "lib" to another path?

My initial idea is to provide a way to bypass the cache/download path, as you suggested, by providing a config to set the path for the ".tar.gz" (or the directory to find the ".tar.gz" files). This config could read from an environment variable.

blaggacao commented 3 months ago

Just one question: would be OK to accept the path for a ".tar.gz" file, and then let the build of the lib that is using RustlerPrecompiled extract the "lib" to another path?

That's perfectly ok, although it would add an extra build step to tar.gz the final libaray after the rust build, maybe there will be a function for that for automating that process in nix at some point.

Since it adds an extra step on the nix side, at least from the build perspective, there's no added benefit in doing so. So if it's easy to avoid, that would be preferable. But it's perfectly ok.

philss commented 3 months ago

@blaggacao I wrote an attempt to solve this here: https://github.com/philss/rustler_precompiled/pull/73 I couldn't write a PoC, but I believe this will help. WDYT?

philss commented 3 months ago

Hey @blaggacao, I released a new version that is not exactly what was requested, but I think it can work. Please give it a try.

To reach minimum compatibility level for all packages depending on rustler_precompiled, we'd need a way to securely (so that it can't be misused by non-nix users) pass the shared libraries as a dependency.

We cannot have the shared library as a dependency, because I need to check the integrity of the ".tar.gz" before decoding and loading it. We could have in the future a way to verify the integrity of the ".so" or ".dll" file, but it's not possible today.

So instead, the new version - v0.7.2 - will accept an env var named RUSTLER_PRECOMPILED_GLOBAL_CACHE_PATH, which can be the path that you downloaded the ".tar.gz" of your target to. I think you can use a fetchFrom* function for that, right?

Another option introduced in this version is to set the new env var RUSTLER_PRECOMPILED_FORCE_BUILD_ALL, which will enable total delegation to Rustler. You can use the :load_from configuration, combined with the :skip_compilation? option to achieve what you planned from the beginning. The only drawback is that you need to include ":rustler" as a dependency, and you need to configure all modules that you need to override the configuration.

Please let me know if this helps. We can improve docs if so.

blaggacao commented 3 months ago

Thanks, I plan to retake this somewhere next week. :pray:

adamcstephens commented 1 month ago

I had to combine all of the suggestions in order to successfully build a library (uuidv7) that uses rustler_precompiled. Forcing the library to build and setting a cache path weren't enough, but I also needed to tell rustler to skip compilation.

Here's a simple example: https://codeberg.org/adamcstephens/nix-rustler-precompiled-example