shorebirdtech / shorebird

Code Push for Flutter and other tools for Flutter businesses.
https://shorebird.dev
Other
1.97k stars 118 forks source link

fix: CLI Performs Changes to User System #1982

Open ofekd opened 2 weeks ago

ofekd commented 2 weeks ago

App ID: N/A

Description

The CLI seems to be making environment changes on the user system which I consider to be an overreach. The two main issues I bumped into are:

(1) Managing the flutter and dart installations (2) Attempting to access .zshrc

I can imagine why (1) may be necessary, although it'll be much preferable to be able to opt out of it. I'd rather keep control of the versions myself.

The .zshrc access is completely unacceptable. I'm not sure if it's Shoebird doing it or a binary/script Shoebird calls, but even runninng shoebird help result in an attempt to access .zshrc:

$ ../shorebird/bin/shorebird help                                                                                       
PathAccessException: Cannot open file, path = '/home/<user>/.zshrc' (OS Error: Permission denied, errno = 13)
The shorebird command-line tool

Usage: shorebird <command> [arguments]

Global options:
-h, --help            Print this usage information.
    --version         Print the current version.
-v, --[no-]verbose    Noisy logging, including all shell commands executed.

Available commands:
  cache      Manage the Shorebird cache.
  doctor     Show information about the installed tooling.
  flutter    Manage your Shorebird Flutter installation.
  init       Initialize Shorebird.
  login      Login as a new Shorebird user.
  login:ci   Login as a CI user.
  logout     Logout of the current Shorebird user
  patch      Manage patches for a specific release in Shorebird.
  preview    Preview a specific release on a device.
  release    Manage your Shorebird app releases.
  upgrade    Upgrade your copy of Shorebird.

Run "shorebird help <command>" for more information about a command.

Expected Behavior

Any side effects performed by a tool such as shoebird should be contained to the project directory.

eseidel commented 2 weeks ago

Thanks for the report!

1) Shorebird uses its own flutter and dart (we have to, in order to change the flutter engine used), as part of not interfering with your system flutter or dart. Are you seeing something different?

2) shorebird itself should never touch (or read) zshrc. However our installer script does (as part of adding shorebird to your path. All our installer does is checkout this repository and add bin/ to your path. You're welcome to do that yourself of course w/o using our installer: https://github.com/shorebirdtech/install/blob/main/install.sh

So I think there has just been a misunderstanding of what shorebird does? Unless you're seeing something different?

ofekd commented 2 weeks ago

Thank you for the quick reply

Re: the modified engine, perfectly understood, thank you for clarifying. The confusion came from the "Upgrading Flutter..." message which I assumed can only mean my Flutter install, as that's the only thing there is to update. I then assumed that Shorebird has both its own copy of Flutter and, for some reason, updates the system one. I wonder if it's something printed by Flutter, and not Shorebird, in which case maybe it's better to mark it as such similarly to how docker-compose marks containers output.

Regarding the second point, I did not run the install script, but instead cloned the repository manually, and executed the binary in it. Any command I ran first printed PathAccessException: Cannot open file, path = '/home/<user>/.zshrc' (OS Error: Permission denied, errno = 13), so something is trying to get to .zshrc.

ofekd commented 2 weeks ago

Found what seems to be another occurrence of system modification:

../shorebird/bin/shorebird init                                                                                                                    ✘ PIPE 4s ▼  impure
PathAccessException: Cannot open file, path = '/home/<user>/.zshrc' (OS Error: Permission denied, errno = 13)
✗ Detecting product flavors (3.4s)
Unable to extract product flavors.
Exception: Starting a Gradle Daemon (subsequent builds will be faster)

FAILURE: Build failed with an exception.

* Where:
Settings file '/home/<project>/android/settings.gradle' line: 25

* What went wrong:
Error resolving plugin [id: 'dev.flutter.flutter-plugin-loader', version: '1.0.0']
> A problem occurred configuring project ':gradle'.
   > Could not create service of type OutputFilesRepository using ExecutionGradleServices.createOutputFilesRepository().
      > Failed to create parent directory '/nix/store/hs3yc5sj63kzbphqsn68scn1ijgq59yq-flutter-wrapped-3.19.6-sdk-links/packages/flutter_tools/gradle/.gradle' when creating directory '/nix/store/hs3yc5sj63kzbphqsn68scn1ijgq59yq-flutter-wrapped-3.19.6-sdk-links/packages/flutter_tools/gradle/.gradle/buildOutputCleanup'

* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights.

* Get more help at https://help.gradle.org

BUILD FAILED in 2s

Since developing vanilla Flutter has been working fine for a while now, it looks like Shorebird modifies the system, rather than installing things locally to the project.

ofekd commented 2 weeks ago

I've found the code trying to modify .zshrc, it's the autocompletions autoinstaller, can be disabled like so:

diff --git a/packages/shorebird_cli/lib/src/command_runner.dart b/packages/shorebird_cli/lib/src/command_runner.dart
index 2825734d..35618d18 100644
--- a/packages/shorebird_cli/lib/src/command_runner.dart
+++ b/packages/shorebird_cli/lib/src/command_runner.dart
@@ -79,6 +79,9 @@ class ShorebirdCliCommandRunner extends CompletionCommandRunner<int> {
     addCommand(UpgradeCommand());
   }

+  @override
+  bool get enableAutoInstall => false;
+
   @override
   void printUsage() => logger.info(usage);

But I didn't find how to enable the help output of the of the install-completion-files command, which is hidden by default.

bryanoltman commented 2 weeks ago

It looks like you found it, but for anyone else reading this in the future, this functionality is coming from the cli_completion package, which we use to suggest tab completions for commands.

eseidel commented 2 weeks ago

It sounds like you're running shorebird inside some sort of container or sysroot/jail? It would be easy for us to turn off the cli_completion support (I wasn't aware that it modified zshrc) in such an environment if you can describe more about what you're using to test here.

eseidel commented 2 weeks ago

Found what seems to be another occurrence of system modification:

../shorebird/bin/shorebird init                                                                                                                    ✘ PIPE 4s ▼  impure
PathAccessException: Cannot open file, path = '/home/<user>/.zshrc' (OS Error: Permission denied, errno = 13)
✗ Detecting product flavors (3.4s)
Unable to extract product flavors.
Exception: Starting a Gradle Daemon (subsequent builds will be faster)

FAILURE: Build failed with an exception.

* Where:
Settings file '/home/<project>/android/settings.gradle' line: 25

* What went wrong:
Error resolving plugin [id: 'dev.flutter.flutter-plugin-loader', version: '1.0.0']
> A problem occurred configuring project ':gradle'.
   > Could not create service of type OutputFilesRepository using ExecutionGradleServices.createOutputFilesRepository().
      > Failed to create parent directory '/nix/store/hs3yc5sj63kzbphqsn68scn1ijgq59yq-flutter-wrapped-3.19.6-sdk-links/packages/flutter_tools/gradle/.gradle' when creating directory '/nix/store/hs3yc5sj63kzbphqsn68scn1ijgq59yq-flutter-wrapped-3.19.6-sdk-links/packages/flutter_tools/gradle/.gradle/buildOutputCleanup'

* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights.

* Get more help at https://help.gradle.org

BUILD FAILED in 2s

Since developing vanilla Flutter has been working fine for a while now, it looks like Shorebird modifies the system, rather than installing things locally to the project.

Shorebird uses flutter build (it's own cached copy), which uses gradle which has a global gradle cache. So in that way it does pollute the wider system. This is part of the reason why we use our own fork of Flutter, so that we can have different git hash names for our flutter engine, so as not to accidentally occupy names in your gradle cache that any other binary might accidentally use.

I believe pub also has a global cache, which shorebird commands might affect. I'm not aware of any other ways in which we modify the global environment though.

I think you may be finding real issues here. I also worry we began this conversation from a "shorebird is evil" sort of framing. :) If you believe you've found bugs in shorebird, we would love to work with you to fix them! (Whether via here or https://discord.gg/shorebird also reaches us directly for chat.)

ofekd commented 2 weeks ago

I apologize if it seemed that I consider this to somehow be an "evil" move. To the contrary -- I've been following you long enough to know that it's nothing of the sort. I still remember the reverse iceberg analogy from a keynote way back -- such an approach is clearly the opposite of an evil scheme to change the users' .zshrc. It is annoying when a tool changes the system, though, which is where this undertone may have come from. Apologies again.

Having said that, to the issue at hand --

I am using NixOS with Home Manager, tools which exclusively manage parts of my system. The managed files are then set read-only to be modified solely by those tools. That's why this issues surfaced. Unlike a regular Linux install, any 3rd party software that tries to modify manages files fails and prints an error.

Does Shorebird's unique copy of Flutter comes with its own unique copy of Gradle? Can that Gradle's cache location be set?

It may seem nit-picky to some but even though I understand there are no bad intentions, system changes should always be done intentionally by the user. Imagine that library you're using had a bug in it and corrupted someone's .zshrc, especially if it does it automatically without notifying the user -- now he or she have to hunt down the cause. This can lead to a bad work day.

eseidel commented 2 weeks ago

:)

When we added the flutter build wrapping originally (about a year ago) we weren't modifying flutter at all, so had no ability to change the gradle cache. Since that time we have added the ability to modify flutter itself, so we could set a custom gradle cache, and that's not a bad idea.