cunarist / rinf

Rust for native business logic, Flutter for flexible and beautiful GUI
MIT License
1.82k stars 64 forks source link

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

Closed pluiedev closed 2 months ago

pluiedev commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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.