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

Use pkexec without temp file #22843

Closed
wants to merge 1 commit into from
Closed

Use pkexec without temp file #22843

wants to merge 1 commit into from

Conversation

ConradIrwin
Copy link
Member

Updates #22045

Release Notes:

  • Removes a race condition when writing a file as root

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Jan 8, 2025
@0xtimsb
Copy link
Member

0xtimsb commented Jan 9, 2025

@ConradIrwin I found some interesting stuff:

Firstly, both main and pkexec2 fail to save the buffer when I use cargo run. This issue also occurs with the release flag.

However, when I install it using ./script/install-linux, both main and pkexec2 (after the tiny change below) work as expected.

      }
      writer.flush().await?;
+     drop(writer);

I have yet to dig deeper into why this behavior is happening. Initially, after reading pkexec man page here, I speculated that it might have something to do with the env flags we set up in bundle-linux. I tried manually setting up these flags before running cargo run, but it still didn’t work.

Anyway, with the drop change, it seems ready to merge, given that the bundled version works. Please confirm these observations on your system, just to ensure there isn’t something peculiar happening on my end.

@ConradIrwin
Copy link
Member Author

Ok, will try with that!

@ConradIrwin
Copy link
Member Author

@0xtimsb Thanks for the investigation, that does indeed seem to fix it (though I'm not sure I could explain why yet).

Unfortunately once I got that working, I ended up running into another issue which is that if you have auto_save: on_focus_change in your settings, then you end up in an infinite loop:

  • hit save causes pkexec to pop up a window, which causes zed to blur, which causes zed to save, which causes pkexec to pop up a window....

I think we need to handle the permission denied further enough up the stack that we can tell whether this was an auto-save or not; and add a separate "save_as_root" method to the Fs interface instead of burying this in there.

Given all that I'm going to revert #22045 for now. That said, this is a feature we want to build, it's just not ready yet.

If you'd like to pair on getting this in a better shape, that'd be fun: https://calendly.com/conradirwin/pairing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed The user has signed the Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants