jesseduffield / lazygit

simple terminal UI for git commands
MIT License
52.29k stars 1.83k forks source link

won't launch when migrateUserConfig fails #3969

Closed CrackTC closed 2 weeks ago

CrackTC commented 2 weeks ago

Describe the bug I'm using home-manager to generate my configuration, it places config.yml in /nix/store(which is read-only) and creates a symbol link in ~/.config/lazygit. And lazygit won't launch when it fails to write migrated config back.

To Reproduce Steps to reproduce the behavior:

  1. Use home-manager option programs.lazygit.settings to generate a config.yml
  2. run lazygit on command line.
  3. Lazygit complains Couldn't write migrated config back to `/home/myhome/.config/lazygit/config.yml`: open /home/myhome/.config/lazygit/config.yml: read-only file system

Expected behavior launch normally (and maybe produce a warn?)

Version info: Run lazygit --version and paste the result here commit=, build date=, build source=nix, version=0.44.1, os=linux, arch=amd64, git version=2.46.0 Run git --version and paste the result here git version 2.46.0

Additional context It seems that lazygit's migrated config.yml uses 4 spaces for identation, while home-manager's uses 2 spaces, that's the only difference I found in my case.

stefanhaller commented 2 weeks ago

I'm not familiar with home-manager and I only just briefly looked at its documentation. From what I understand, you put configuration options for lazygit in home-manager's config file instead of directly in lazygit's, is that right? If so, that doesn't strike me as a great idea, especially considering this guideline. What do you do if you want to use a lazygit option that home-manager doesn't provide?

Anyway, your suggestion of being graceful about the error and launching anyway doesn't really work for us. It is important that we can write the migrated file back, because this will allow us to remove deprecated options after a while. Without that, we'd have to continue to support deprecated options forever, because you would never know that you need to update your home-manager config file manually.

It seems that lazygit's migrated config.yml uses 4 spaces for identation, while home-manager's uses 2 spaces, that's the only difference I found in my case.

How do you know if it couldn't write the file back? Anyway, lazygit will only write the config file back if it actually had to make a real change to it. Yes, the indentation might change when it does that, but it will not try to write the file when nothing needs to be migrated. If it does, that would be a bug that we need to fix.

CrackTC commented 2 weeks ago

Anyway, lazygit will only write the config file back if it actually had to make a real change to it.

Oh, I just found #3800, and after changing executeCustomCommand to executeShellCommand lazygit works again. Sorry about that.

Anyway, thanks for all your hard work in bringing us such a life-changing tool.