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

rinf message should not automatically run cargo install when protoc-gen-prost is already in PATH #366

Closed
pluiedev opened this issue Jun 10, 2024 · 2 comments · Fixed by #369

Comments

@pluiedev
Copy link

Report

Rinf currently runs cargo install protoc-gen-prost regardless of whether protoc-gen-prost is already installed in the system. This is problematic for offline, reproducible builds where build scripts cannot arbitrarily interact with the internet, for example while packaging apps that use Rinf for Nixpkgs.

Steps to Reproduce

Here's an example I ran into while trying to package Notiflut, but you can easily reproduce this if you just turn off your internet connection while having protoc-gen-prost installed.

{
  lib,
  flutter322,
  rustPlatform,
  fetchFromGitHub,
  protobuf,
  gtk-layer-shell,
  cargo,
  protoc-gen-prost
}:
let
  version = "1.1.0";

  src = fetchFromGitHub {
    owner = "LucaCoduriV";
    repo = "Notiflut-Land";
    rev = "v${version}";
    hash = "sha256-tZvrpe8zo7+X4Ek2cOim9oBRp2hKKbuTLZ9V3qdBDiU=";
  };
in
flutter322.buildFlutterApplication {
  pname = "notiflut";
  inherit version src;

  sourceRoot = "${src.name}/notiflut_daemon";

  # jq '.packages | to_entries | map(select(.value.source == "git").key)' ./pubspec.lock.json
  gitHashes = {
    "mpris" = "sha256-p+oDNOD9W276olYakViLyVhV9sakKetyEWPhJ/+7Ldg=";
    "wayland_multi_window" = "sha256-gjAkBgiPPJwPHucwdGasK0BFRkggf4w9XI9za5ww/Is=";
    "window_manager" = "sha256-+7uevocGk4QfROnKeiLhxzPPo9oScayV2ylGziHyeWk=";
  };

  #curl https://raw.githubusercontent.com/LucaCoduriV/Notiflut-Land/${version}/notiflut_daemon/pubspec.lock | yq
  pubspecLock = lib.importJSON ./pubspec.lock.json;

  nativeBuildInputs = [
    protobuf
    cargo
    protoc-gen-prost
  ];

  buildInputs = [
    gtk-layer-shell
  ];

  preBuild = ''
    packageRun rinf message
    #protoc --dart_out=./lib/messages google/protobuf/timestamp.proto google/protobuf/empty.proto
  '';
}
error: builder for '/nix/store/41c9c8mwrwiifyi3jyab023s9ba2kyi5-notiflut-1.1.0.drv' failed with exit code 255;
       last 25 log lines:
       > Finished dartConfigHook
       > Running phase: buildPhase
       > Verifying `protoc-gen-prost` for Rust. This might take a while if there are new updates to be installed.
       >     Updating crates.io index
       > warning: spurious network error (3 tries remaining): [6] Couldn't resolve host name (Could not resolve host: index.crates.io)
       > warning: spurious network error (2 tries remaining): [6] Couldn't resolve host name (Could not resolve host: index.crates.io)
       > warning: spurious network error (1 tries remaining): [6] Couldn't resolve host name (Could not resolve host: index.crates.io)
       > error: failed to query replaced source registry `crates-io`
       >
       > Caused by:
       >   download of config.json failed
       >
       > Caused by:
       >   failed to download from `https://index.crates.io/config.json`
       >
       > Caused by:
       >   [6] Couldn't resolve host name (Could not resolve host: index.crates.io)
       >
       > Unhandled exception:
       > Exception: Cannot globally install `protoc-gen-prost` Rust crate
       > #0      _generateMessageCode (file:///nix/store/5ygnhizaf9qjzkqq98vr7z2k53ld14y5-pub-rinf-4.16.3/bin/rinf.dart:421:5)
       > <asynchronous suspension>
       > #1      main (file:///nix/store/5ygnhizaf9qjzkqq98vr7z2k53ld14y5-pub-rinf-4.16.3/bin/rinf.dart:19:7)
       > <asynchronous suspension>
       > /nix/store/mx35naw99af63khlz4g9apgaadbfgzyk-dart-config-hook/nix-support/setup-hook: line 131: pop_var_context: head of shell_variables not a function context
       For full logs, run 'nix log /nix/store/41c9c8mwrwiifyi3jyab023s9ba2kyi5-notiflut-1.1.0.drv'.
@pluiedev
Copy link
Author

pluiedev commented Jun 10, 2024

Plus, IMO having the script install protoc-gen-prost globally into the system without the user explicitly warned so is a bit questionable to begin with. Once it's installed, it stays forever in ~/.cargo/bin and very few would even try to look into what's inside there.

A better option would be to check the current PATH, try to look for protoc-gen-prost, and if it's not found, ask the user whether it wants the script to run cargo install protoc-gen-prost for them, and tell them that it's a persistent install across your entire system (or at least, across this user's entire environment)

@temeddix
Copy link
Member

temeddix commented Jun 10, 2024

Thanks for the report @pluiedev :)

I'll make sure the next version of Rinf behaves well in offline environments.

Rinf as well as Cargokit(library linker used under the hood) commands actually run various Cargo commands behind the scenes. For example, there are implicit rustup target add android..., etc. This is done to provide convenience. In my opinion at least telling the user that protoc-gen-prost is being prepared from the CLI would be nice, yes.

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

Successfully merging a pull request may close this issue.

2 participants