Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
blaggacao opened this issue May 23, 2024 · 9 comments
Open

Comments

@blaggacao
Copy link

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

TL;DR;

  • Nix' build sandbox doesn't allow network access and thus fails on download_nif_artifact
  • The libx.so will be typically already provided as local input though a build definition like so:
  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
Copy link

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
Copy link
Owner

philss commented Jun 6, 2024

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
Copy link
Author

blaggacao commented Jun 7, 2024

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
Copy link
Author

blaggacao commented Jun 16, 2024

@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
Copy link
Owner

philss commented Jun 17, 2024

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
Copy link
Author

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
Copy link
Owner

philss commented Jun 19, 2024

@blaggacao I wrote an attempt to solve this here: #73
I couldn't write a PoC, but I believe this will help. WDYT?

@philss
Copy link
Owner

philss commented Jun 27, 2024

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
Copy link
Author

Thanks, I plan to retake this somewhere next week. 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants