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

collab: Fix project sharing between Windows and Unix #23680

Open
wants to merge 64 commits into
base: main
Choose a base branch
from

Conversation

JunkuiZhang
Copy link
Contributor

@JunkuiZhang JunkuiZhang commented Jan 26, 2025

Closes #14258

Windows user(host) sharing a project to a guest(using macOS), and host follows guest:

Screen.Recording.2025-02-07.004405.mp4

macOS user(host) sharing a project to a guest(using Windows), and host follows guest:

2025-02-07.00.46.55.mov

macOS user edits files in a windows project through collab:

2025-02-07.00.57.36.mov

Release Notes:

  • N/A

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Jan 26, 2025
@JunkuiZhang JunkuiZhang marked this pull request as draft January 26, 2025 19:28
@JunkuiZhang JunkuiZhang force-pushed the windows/collab-2 branch 2 times, most recently from ee0a32b to 4db70da Compare January 31, 2025 15:25
@JunkuiZhang JunkuiZhang force-pushed the windows/collab-2 branch 2 times, most recently from 49e888e to 5acb628 Compare February 5, 2025 15:33
@JunkuiZhang
Copy link
Contributor Author

JunkuiZhang commented Feb 6, 2025

The current issue is that when pairing, if the host follows the guest, the host cannot see the guest's actions. However, when the guest follows the host, the guest can see the host's actions.

@JunkuiZhang
Copy link
Contributor Author

JunkuiZhang commented Feb 6, 2025

This is so strange. When I run Zed manually by setting the ZED_SERVER_URL and other environment variables, instead of using the zed-local script to start Zed, host no longer experience the issue. I'm really not sure what’s causing the problem where the host can't see the guest's actions when the host follows the guest. Weird

It's ZED_STATELESS causing this issue. If I set ZED_STATELESS="1", the host can't see the guest's actions when the host follows the guest. If I set ZED_STATELESS="", everything is just fine.

@JunkuiZhang JunkuiZhang marked this pull request as ready for review February 6, 2025 16:42
@mikayla-maki
Copy link
Contributor

It's ZED_STATELESS causing this issue. If I set ZED_STATELESS="1", the host can't see the guest's actions when the host follows the guest. If I set ZED_STATELESS="", everything is just fine.

This seems very strange, ZED_STATELESS should only affect the sqlite db we use to save your application state. Any theories for why it's messing up the host?

@JunkuiZhang
Copy link
Contributor Author

This seems very strange, ZED_STATELESS should only affect the sqlite db we use to save your application state. Any theories for why it's messing up the host?

I have no clue about why setting ZED_STATELESS causes this issue, but I can reliably reproduce the issue by setting ZED_STATELESS to either "1" or "". Moreover, when A shares a project with B, where A is the host and B is the guest, and A follows B (host follows guest), whether A can see B's actions depends solely on A's ZED_STATELESS setting. If A sets ZED_STATELESS = "1", A will not be able to see B's actions. However, if ZED_STATELESS = "", A will be able to see B's actions. This behavior is independent of B's ZED_STATELESS setting.

@JunkuiZhang
Copy link
Contributor Author

JunkuiZhang commented Feb 7, 2025

I can reproduce this with both A and B are on the same mac.

In a new terminal, set the following for A:

export ZED_IMPERSONATE="as-cii"
export ZED_SERVER_URL="http://127.0.0.1:3000"
export ZED_RPC_URL="http://127.0.0.1:8080/rpc"
export ZED_ADMIN_API_TOKEN="secret"
export ZED_CLIENT_CHECKSUM_SEED="development-checksum-seed"
export ZED_STATELESS="1"  // Key setting

Then cargo run to open an instance of zed for A.

In a new terminal:

export ZED_IMPERSONATE="nathansobo"
export ZED_SERVER_URL="http://127.0.0.1:3000"
export ZED_RPC_URL="http://127.0.0.1:8080/rpc"
export ZED_ADMIN_API_TOKEN="secret"
export ZED_CLIENT_CHECKSUM_SEED="development-checksum-seed"
export ZED_STATELESS=""  // Dose not matter

Then cargo run to open an instance of zed for B.

  • A shares a project with B
  • B accept, then cancel the following
  • A (host) follows B (guest)
  • B perform some actions
  • A can not see the actions

@@ -688,6 +688,9 @@ impl RejoinedProject {
#[derive(Debug)]
pub struct RejoinedWorktree {
pub id: u64,
/// NOTE:
/// Use PathBuf::to_proto() and PathBuf::from_proto() to convert between
Copy link
Contributor

@mikayla-maki mikayla-maki Feb 7, 2025

Choose a reason for hiding this comment

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

I think these comments are a good idea, but I don't think they provide much benefit in the end. Let's remove everything with NOTE:, and similar comments, in this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

As well as all of the // path, and other such comments.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have removed all the comments, but I'm not sure how we can ensure that other developers use the relevant functions correctly. After all, Zed is an open-source project, and some contributors may not be aware of this, potentially leading them to mistakenly use to_string_lossy().to_string(), which could result in hard-to-detect bugs.

@mikayla-maki
Copy link
Contributor

Also, it would be nice to figure out why ZED_STATELESS is breaking your collaboration. And we should make sure we're syncing the host's OS over the wire so that we can display paths properly. But I think that those can be in a separate PR :)

@JunkuiZhang
Copy link
Contributor Author

Also, it would be nice to figure out why ZED_STATELESS is breaking your collaboration.

ZED_STATELESS may not be the issue in this PR, as I am able to reproduce the problem on the current main branch as well. This is very strange.

And we should make sure we're syncing the host's OS over the wire so that we can display paths properly. But I think that those can be in a separate PR :)

Actually, my thinking is that this setup works quite well as it is. Currently, the Windows path path\to\file.rs is displayed as path\to\file.rs on Windows, but on macOS, it is displayed as path/to/file.rs, which feels like that the Windows project is running natively on macOS.

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.

Zed does not divide Paths correctly when sharing from Windows to Linux
2 participants