Skip to content

Commit 7737b3a

Browse files
committed
refactor(workspaces): drop dead manifest field, slim auto-register discovery
- remove the now-unused `Workspace.manifest` field; its last reader went away when auto_register_member began re-reading the manifest from disk - add `discover_root` to locate a workspace root without resolving members, so auto_register_member (hence `leo new`) no longer fails when an unrelated existing member is broken; covered by a new regression test - generalize the workspace_member_outside_root help text (it also catches symlink escapes) and tighten its test to assert the specific error
1 parent a2e948a commit 7737b3a

2 files changed

Lines changed: 58 additions & 19 deletions

File tree

crates/package/src/errors.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ pub(crate) fn workspace_member_outside_root(member: impl Display, workspace_root
195195
CODE_MASK + 70,
196196
format!("workspace member `{member}` resolves to a path outside the workspace root `{workspace_root}`"),
197197
)
198-
.with_help("Workspace members must live inside the workspace root. Remove `..` components from the member entry.")
198+
.with_help("Ensure the member entry resolves to a directory inside the workspace root; remove any `..` components.")
199199
}
200200

201201
/// For when workspace.json cannot be read or parsed.

crates/package/src/workspace.rs

Lines changed: 57 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,6 @@ impl WorkspaceManifest {
5050
pub struct Workspace {
5151
/// The canonicalized root directory containing `workspace.json`.
5252
pub root_directory: PathBuf,
53-
/// The workspace manifest.
54-
pub manifest: WorkspaceManifest,
5553
/// Member directories in dependency order, each an absolute path.
5654
pub member_paths: Vec<PathBuf>,
5755
/// Member program names (from each member's `program.json`), in the same order.
@@ -107,23 +105,17 @@ impl Workspace {
107105
let member_paths = ordered.iter().map(|(p, _)| p.clone()).collect();
108106
let member_names = ordered.into_iter().map(|(_, n)| n).collect();
109107

110-
Ok(Some(Workspace { root_directory, manifest, member_paths, member_names }))
108+
Ok(Some(Workspace { root_directory, member_paths, member_names }))
111109
}
112110

113-
/// Walk up from `start_dir` looking for `workspace.json`.
111+
/// Walk up from `start_dir` looking for `workspace.json`, returning the
112+
/// fully resolved workspace.
114113
///
115114
/// Returns `Ok(None)` if no workspace root is found.
116115
pub fn discover(start_dir: &Path) -> Result<Option<Self>> {
117-
let start = start_dir.canonicalize().map_err(|e| errors::workspace_manifest_error(start_dir.display(), e))?;
118-
let mut dir = start.as_path();
119-
loop {
120-
if let Some(ws) = Self::from_directory(dir)? {
121-
return Ok(Some(ws));
122-
}
123-
match dir.parent() {
124-
Some(parent) => dir = parent,
125-
None => return Ok(None),
126-
}
116+
match discover_root(start_dir)? {
117+
Some(root) => Self::from_directory(&root),
118+
None => Ok(None),
127119
}
128120
}
129121

@@ -165,17 +157,20 @@ impl Workspace {
165157
let Some(parent) = canonical_member.parent() else {
166158
return Ok(false);
167159
};
168-
let Some(ws) = Self::discover(parent)? else {
160+
// Only the workspace root is needed here, so locate it without resolving
161+
// members; that keeps `leo new` from failing when an unrelated existing
162+
// member is broken.
163+
let Some(root_directory) = discover_root(parent)? else {
169164
return Ok(false);
170165
};
171166

172-
let relative = match canonical_member.strip_prefix(&ws.root_directory) {
167+
let relative = match canonical_member.strip_prefix(&root_directory) {
173168
Ok(rel) => rel,
174169
Err(_) => {
175170
tracing::warn!(
176171
"new package at `{}` is not inside the discovered workspace root `{}`; skipping auto-add",
177172
canonical_member.display(),
178-
ws.root_directory.display(),
173+
root_directory.display(),
179174
);
180175
return Ok(false);
181176
}
@@ -188,7 +183,7 @@ impl Workspace {
188183

189184
// Re-read from disk in case the manifest was modified between discover and write,
190185
// and check coverage against those fresh entries rather than the stale snapshot.
191-
let manifest_path = ws.root_directory.join(WORKSPACE_MANIFEST_FILENAME);
186+
let manifest_path = root_directory.join(WORKSPACE_MANIFEST_FILENAME);
192187
let mut manifest = WorkspaceManifest::read_from_file(&manifest_path)?;
193188

194189
if pattern_matches_relative(&manifest.members, &entry) {
@@ -227,6 +222,24 @@ impl Workspace {
227222
}
228223
}
229224

225+
/// Walk up from `start_dir` to find the directory containing `workspace.json`.
226+
///
227+
/// Returns the canonicalized workspace root, or `Ok(None)` if none is found.
228+
/// Unlike [`Workspace::discover`], this does not resolve or validate members.
229+
fn discover_root(start_dir: &Path) -> Result<Option<PathBuf>> {
230+
let start = start_dir.canonicalize().map_err(|e| errors::workspace_manifest_error(start_dir.display(), e))?;
231+
let mut dir = start.as_path();
232+
loop {
233+
if dir.join(WORKSPACE_MANIFEST_FILENAME).exists() {
234+
return Ok(Some(dir.to_path_buf()));
235+
}
236+
match dir.parent() {
237+
Some(parent) => dir = parent,
238+
None => return Ok(None),
239+
}
240+
}
241+
}
242+
230243
/// Resolve a `Location::Workspace` dependency by looking up its name in the
231244
/// enclosing workspace, returning a new `Dependency` with `Location::Local`
232245
/// and the resolved absolute path.
@@ -780,6 +793,30 @@ mod tests {
780793
std::fs::remove_dir_all(&dir).unwrap();
781794
}
782795

796+
#[test]
797+
fn auto_register_succeeds_despite_broken_member() {
798+
// A new package must register even when a sibling member listed in
799+
// `workspace.json` is broken: auto-registration only needs the workspace
800+
// root, not a fully resolved member list.
801+
let dir = temp_dir().join("ws_test_auto_register_broken_member");
802+
let _ = std::fs::remove_dir_all(&dir);
803+
std::fs::create_dir_all(&dir).unwrap();
804+
805+
create_member(&dir, "alpha", &[]);
806+
// `ghost` is listed but never created - resolving the workspace would fail.
807+
create_workspace(&dir, &["alpha", "ghost"]);
808+
809+
create_member(&dir, "beta", &[]);
810+
let beta_dir = dir.join("beta");
811+
let registered = Workspace::auto_register_member(&beta_dir).unwrap();
812+
assert!(registered, "a new package should register despite a broken sibling member");
813+
814+
let manifest = WorkspaceManifest::read_from_file(dir.join(WORKSPACE_MANIFEST_FILENAME)).unwrap();
815+
assert_eq!(manifest.members, vec!["alpha".to_string(), "ghost".to_string(), "beta".to_string()]);
816+
817+
std::fs::remove_dir_all(&dir).unwrap();
818+
}
819+
783820
#[test]
784821
fn auto_register_registers_glob_subdir() {
785822
let dir = temp_dir().join("ws_test_auto_register_glob_subdir");
@@ -1020,6 +1057,8 @@ mod tests {
10201057

10211058
let result = Workspace::from_directory(&ws_dir);
10221059
assert!(result.is_err(), "a member resolving outside the workspace root should be rejected");
1060+
let err_msg = format!("{}", result.unwrap_err());
1061+
assert!(err_msg.contains("outside the workspace root"), "error should be the outside-root error: {err_msg}");
10231062

10241063
std::fs::remove_dir_all(&parent).unwrap();
10251064
}

0 commit comments

Comments
 (0)