Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
f801b12e87 |
@@ -111,16 +111,33 @@ impl WorktreeManager {
|
|||||||
// Clear registrations whose directory is gone first, so a
|
// Clear registrations whose directory is gone first, so a
|
||||||
// previously-leaked entry doesn't push this branch to a suffix.
|
// previously-leaked entry doesn't push this branch to a suffix.
|
||||||
prune(base_repo)?;
|
prune(base_repo)?;
|
||||||
// Anything occupying the target — a tree we left behind, one the
|
let (branch, path) = if create_branch {
|
||||||
// operator made, an unrelated directory — is sidestepped with a
|
// Worktrees are commonly abandoned without deleting their branch,
|
||||||
// numeric suffix on the directory; the branch stays whatever was
|
// so a random handle can collide with one of its own past runs.
|
||||||
// requested (#77). The daemon never adopts what it finds.
|
// Unlike the `add_existing` arm below, the suffix goes on the
|
||||||
let mut path = self.target_path(base_repo, branch);
|
// branch name itself — the path is derived from it anyway.
|
||||||
let mut n = 1;
|
let mut candidate = branch.to_string();
|
||||||
while path.symlink_metadata().is_ok() {
|
let mut path = self.target_path(base_repo, &candidate);
|
||||||
n += 1;
|
let mut n = 1;
|
||||||
path = self.target_path_suffixed(base_repo, branch, n);
|
while path.symlink_metadata().is_ok() || branch_exists(base_repo, &candidate) {
|
||||||
}
|
n += 1;
|
||||||
|
candidate = format!("{branch}-{n}");
|
||||||
|
path = self.target_path(base_repo, &candidate);
|
||||||
|
}
|
||||||
|
(candidate, path)
|
||||||
|
} else {
|
||||||
|
// Existing branch: the name must stay exact, so only the
|
||||||
|
// directory — anything occupying it, ours or the operator's — is
|
||||||
|
// sidestepped with a numeric suffix (#77). The daemon never
|
||||||
|
// adopts what it finds.
|
||||||
|
let mut path = self.target_path(base_repo, branch);
|
||||||
|
let mut n = 1;
|
||||||
|
while path.symlink_metadata().is_ok() {
|
||||||
|
n += 1;
|
||||||
|
path = self.target_path_suffixed(base_repo, branch, n);
|
||||||
|
}
|
||||||
|
(branch.to_string(), path)
|
||||||
|
};
|
||||||
// The parent (`<repo>/worktrees` or the override root) may not exist
|
// The parent (`<repo>/worktrees` or the override root) may not exist
|
||||||
// yet; the target itself must not.
|
// yet; the target itself must not.
|
||||||
if let Some(parent) = path.parent() {
|
if let Some(parent) = path.parent() {
|
||||||
@@ -129,10 +146,10 @@ impl WorktreeManager {
|
|||||||
let mut cmd = Command::new("git");
|
let mut cmd = Command::new("git");
|
||||||
cmd.arg("-C").arg(base_repo).args(["worktree", "add"]);
|
cmd.arg("-C").arg(base_repo).args(["worktree", "add"]);
|
||||||
if create_branch {
|
if create_branch {
|
||||||
cmd.arg("-b").arg(branch).arg(&path);
|
cmd.arg("-b").arg(&branch).arg(&path);
|
||||||
} else {
|
} else {
|
||||||
// Existing branch: `worktree add <path> <branch>` with no `-b`.
|
// Existing branch: `worktree add <path> <branch>` with no `-b`.
|
||||||
cmd.arg(&path).arg(branch);
|
cmd.arg(&path).arg(&branch);
|
||||||
}
|
}
|
||||||
let out = cmd.output()?;
|
let out = cmd.output()?;
|
||||||
check(out)?;
|
check(out)?;
|
||||||
@@ -266,6 +283,17 @@ fn branch_relpath(branch: &str) -> Option<PathBuf> {
|
|||||||
Some(candidate.to_path_buf())
|
Some(candidate.to_path_buf())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Whether `refs/heads/<branch>` already exists in `base_repo`.
|
||||||
|
fn branch_exists(base_repo: &Path, branch: &str) -> bool {
|
||||||
|
Command::new("git")
|
||||||
|
.arg("-C")
|
||||||
|
.arg(base_repo)
|
||||||
|
.args(["show-ref", "--verify", "--quiet"])
|
||||||
|
.arg(format!("refs/heads/{branch}"))
|
||||||
|
.status()
|
||||||
|
.is_ok_and(|s| s.success())
|
||||||
|
}
|
||||||
|
|
||||||
/// `git -C <base_repo> worktree prune` — drops registrations whose directory
|
/// `git -C <base_repo> worktree prune` — drops registrations whose directory
|
||||||
/// no longer exists.
|
/// no longer exists.
|
||||||
fn prune(base_repo: &Path) -> Result<(), WorktreeError> {
|
fn prune(base_repo: &Path) -> Result<(), WorktreeError> {
|
||||||
|
|||||||
@@ -497,6 +497,57 @@ async fn occupied_path_gets_suffixed_directory_and_both_worktrees_work() {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// A random handle can reuse the name of a branch left behind by an earlier,
|
||||||
|
/// already-removed worktree — `git worktree add -b` refuses outright in that
|
||||||
|
/// case, even though the directory is free. The daemon suffixes the branch
|
||||||
|
/// itself (not just the directory) and both land on `-2`.
|
||||||
|
#[tokio::test(flavor = "multi_thread")]
|
||||||
|
async fn colliding_branch_name_gets_suffixed_too() {
|
||||||
|
let (_daemon, client) = bring_up().await;
|
||||||
|
|
||||||
|
let base = TempDir::new().unwrap();
|
||||||
|
init_repo(base.path());
|
||||||
|
|
||||||
|
// No worktree directory in the way, but the branch already exists.
|
||||||
|
run_git(&["branch", "handle"], base.path());
|
||||||
|
|
||||||
|
let s = client
|
||||||
|
.spawn_session(&worktree_req(base.path(), "handle", true))
|
||||||
|
.await
|
||||||
|
.expect("spawn onto a taken branch name");
|
||||||
|
|
||||||
|
let suffixed = base.path().join("worktrees").join("handle-2");
|
||||||
|
assert_eq!(
|
||||||
|
s.cwd, suffixed,
|
||||||
|
"the directory must follow the suffixed branch name"
|
||||||
|
);
|
||||||
|
assert!(suffixed.is_dir());
|
||||||
|
|
||||||
|
let registrations = git_worktree_list(base.path());
|
||||||
|
assert!(
|
||||||
|
registrations.iter().any(|l| l.contains("handle-2")),
|
||||||
|
"the -2 branch's worktree must be registered: {registrations:?}"
|
||||||
|
);
|
||||||
|
|
||||||
|
let branches = String::from_utf8(
|
||||||
|
std::process::Command::new("git")
|
||||||
|
.arg("-C")
|
||||||
|
.arg(base.path())
|
||||||
|
.args(["branch", "--list"])
|
||||||
|
.output()
|
||||||
|
.unwrap()
|
||||||
|
.stdout,
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
assert!(
|
||||||
|
branches.contains("handle-2"),
|
||||||
|
"the branch itself must be named handle-2: {branches}"
|
||||||
|
);
|
||||||
|
|
||||||
|
let _origin = throwaway_origin(base.path());
|
||||||
|
client.kill_session(s.id).await.expect("kill");
|
||||||
|
}
|
||||||
|
|
||||||
/// #77/#88: the target path holds a plain directory the daemon never made and
|
/// #77/#88: the target path holds a plain directory the daemon never made and
|
||||||
/// git has never heard of. The daemon neither adopts it nor errors — it builds
|
/// git has never heard of. The daemon neither adopts it nor errors — it builds
|
||||||
/// beside it and leaves every byte of it alone.
|
/// beside it and leaves every byte of it alone.
|
||||||
|
|||||||
+12
-4
@@ -35,10 +35,18 @@ The branch name is used verbatim as a relative path, so `feature/login` becomes
|
|||||||
back to a short hash of the branch.
|
back to a short hash of the branch.
|
||||||
|
|
||||||
If that path exists, the daemon appends `-2`, then `-3`, and so on until it finds
|
If that path exists, the daemon appends `-2`, then `-3`, and so on until it finds
|
||||||
a free one. The suffix names the **directory only**. The branch is whatever was
|
a free one.
|
||||||
requested; a second worktree of the same branch is not what happens here, because
|
|
||||||
git refuses to check the same branch out twice and the branch picker already
|
For a *new* branch (`create_branch: true`), the suffix goes on the branch name
|
||||||
marks branches that are checked out elsewhere.
|
itself, and the directory follows since it is derived from the branch — a
|
||||||
|
random handle can otherwise reuse the name of a branch some earlier, already
|
||||||
|
removed worktree left behind, and `git worktree add -b` refuses outright even
|
||||||
|
though the directory is free.
|
||||||
|
|
||||||
|
For an *existing* branch (`create_branch: false`) the name must stay exact, so
|
||||||
|
only the directory is suffixed — a second worktree of the same branch is not
|
||||||
|
what happens here, because git refuses to check the same branch out twice and
|
||||||
|
the branch picker already marks branches that are checked out elsewhere.
|
||||||
|
|
||||||
### The `worktree_root` override
|
### The `worktree_root` override
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user