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

Allow Zed to run under multiple user accounts simultaneously #14143

Merged
merged 9 commits into from
Sep 1, 2024

Conversation

samsonjs
Copy link
Contributor

@samsonjs samsonjs commented Jul 11, 2024

Closes #4607

This is an attempt to enable Zed to run under multiple user accounts on the same Mac, because it's a blocker to me really giving Zed a fair shot at being my primary editor.

According to some helpful info from @ForLoveOfCats in #4607 the main reason why this doesn't work is because Zed is using a Unix socket or maybe a TCP socket with a hard-coded path and/or port. To me it looks like it's a TCP socket so I tried changing that code in here, but I'm stuck at trying to test it out because running target/debug/zed or target/release/zed seems to behave differently than running an actual app bundle. I had no luck copying the binary over to /Applications/Zed.app/Contents/MacOS/zed because it can't find WebRTC.framework which resides at a different relative path in the app bundle.

If this seems like a desirable change to the core team then I'm looking for some guidance on how to build an app bundle or otherwise test out this change, or a nudge in the correct direction if I'm way off base with my current approach.

Release Notes:

  • Added multiuser support for up to 100 users on the same machine.

Copy link

cla-bot bot commented Jul 11, 2024

We require contributors to sign our Contributor License Agreement, and we don't have @samsonjs on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'.

@samsonjs
Copy link
Contributor Author

@cla-bot check

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Jul 11, 2024
Copy link

cla-bot bot commented Jul 11, 2024

The cla-bot has been summoned, and re-checked this pull request!

@samsonjs
Copy link
Contributor Author

Also I've never written a line of Rust before in my life so my feelings won't be hurt if you have any feedback about basic stuff in the code. Please feel free to critique anything in here to your heart's content.

@osiewicz
Copy link
Contributor

I had no luck copying the binary over to /Applications/Zed.app/Contents/MacOS/zed because it can't find WebRTC.framework which resides at a different relative path in the app bundle.

You can use scripts/bundle-mac -li to "simulate" installing a dev build as a release one. You'll have a Zed Dev.app in your /Applications then.

crates/zed/src/main.rs Outdated Show resolved Hide resolved
};
let user_id = Uid::current().as_raw() as u16;
Copy link
Contributor

@osiewicz osiewicz Jul 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are UIDs guaranteed to be consecutive?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, they are definitely not always consecutive. There might be gaps so it's safer to have a bigger buffer in between ports for each release channel.

@samsonjs
Copy link
Contributor Author

@osiewicz Thank you very much for the pointers! I'll see what I can come up with now

Also swaps out the unistd call for the users crate to make this build on
Windows as well.
@samsonjs samsonjs marked this pull request as ready for review July 12, 2024 17:24
@samsonjs
Copy link
Contributor Author

I was able to properly test this out by commenting out the check that treats all dev builds as the only instance and I think it's sound now. Rather than using unistd I swapped in the users crate to fix the failing Windows build.

I'm not certain about the addition overflowing behaviour in here and maybe it's worth having a check for that, any advice on that is welcome. It seems like a real corner case but I'm less certain about Windows than Unix on this one.

@samsonjs
Copy link
Contributor Author

Ok so this users crate doesn't work on Windows. I'm away right now but I have a Windows PC at home so when I'm back I can try to find some time to figure this out.

@JunkuiZhang
Copy link
Contributor

Hi, I implemented single instance on Windows using a different approach #15371 . Any advice or suggestions?

@osiewicz
Copy link
Contributor

Hey, how about using something like sysinfo crate?

@samsonjs
Copy link
Contributor Author

Thanks @osiewicz that looks like the ticket! I'm able to grab the user's Uid from the process however I'm stuck trying to get a u32 value from it. This is probably basic Rust stuff so please bear with me. The Uid docs say it implements Deref with a u32 target but *uid doesn't seem to give me a u32 value.

CleanShot 2024-08-23 at 12 44 14@2x

Any advice is appreciated and I'll try to find a bit of time to figure it out on my own as well.

@notpeter
Copy link
Member

I'm stuck trying to get a u32 value from it

Also a rust novice, but I think *uid.clone() should do it.

Screenshot 2024-08-23 at 16 40 14

@samsonjs
Copy link
Contributor Author

Yep that does the trick, thank you!

@samsonjs
Copy link
Contributor Author

Ok I got my hands on a Windows box and I think this is finally gonna work 🤞

@osiewicz osiewicz merged commit b386b6c into zed-industries:main Sep 1, 2024
9 checks passed
@osiewicz
Copy link
Contributor

osiewicz commented Sep 1, 2024

Thanks!

@samsonjs
Copy link
Contributor Author

samsonjs commented Sep 1, 2024

Thank you all for the assistance! I like Zed a lot so far!

@samsonjs samsonjs deleted the 4607-multiuser branch September 2, 2024 15:29
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.

Can't open Zed under another user account
6 participants