feat: support CL paths distinct from Antares mount paths - #36
feat: support CL paths distinct from Antares mount paths#36Ivanbeethoven wants to merge 10 commits into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7c11e6624e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| .build_cl_layer(&request.path, cl_link, &cl_dir_path) | ||
| .build_cl_layer( | ||
| &request.path, | ||
| request.cl_path.as_deref().unwrap_or(&request.path), |
There was a problem hiding this comment.
When a mount is created with a distinct cl_path (for example path: "/", cl_path: "/project/distribution"), this initial build uses that value to decide where CL-relative file entries are placed, but the value is then discarded: it is not stored on the mount, not part of the duplicate/idempotency identity, and later CL rebuilds pass the mount path as the CL path. In that scenario, POST /mounts/{id}/cl can rebuild the same CL with relative entries at the mount root instead of under project/distribution, and retries with a different cl_path can incorrectly return an existing mount. Persist and compare cl_path, and reuse it for rebuilds.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 387d30498a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| } | ||
|
|
||
| /// Type alias for path index: maps (monorepo_path, optional_cl, cl_path) to mount_id. | ||
| type PathIndex = Arc<RwLock<HashMap<(String, Option<String>, Option<String>), Uuid>>>; |
There was a problem hiding this comment.
The new PathIndex key now has three components, but the post-mount duplicate check in create_mount still probes it with (request.path.clone(), request.cl.clone()). That leaves this commit unable to build against HashMap<(String, Option<String>, Option<String>), Uuid>; the race check needs to include request.cl_path just like the fast path and insertion do.
Useful? React with 👍 / 👎.
| ( | ||
| PathBuf::from(&entry.upper_dir), | ||
| entry.cl_dir.as_deref().map(PathBuf::from), | ||
| ) |
There was a problem hiding this comment.
When POST/DELETE /mounts/{id}/cl is rebuilding a CL layer, it marks the mount Quiescing, drops the mount lock, and removes or recreates the CL directory before remounting. This new /changes path copies cl_dir without checking that state, so a concurrent scan of a CL-backed mount can read a disappearing/partial CL directory and return a 500 or incomplete change set instead of rejecting until the remount finishes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 720727caa5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| tokio::task::spawn_blocking(move || { | ||
| scan_mount_changes(mount_id, &upper_dir, cl_dir.as_deref()) |
There was a problem hiding this comment.
When a CL-backed mount starts POST/DELETE /cl immediately after this snapshot, the read lock has already been released, so the CL operation can set Quiescing and remove/recreate cl_dir while this blocking scan is still using the copied path. The fresh evidence is that the new state check only protects the snapshot, not the scan itself, so /changes can still return a 500 or an incomplete set during the CL switch window; keep the quiescence barrier active for the scan or block CL switches until in-flight scans finish.
Useful? React with 👍 / 👎.
| if entry.cl.is_some() { | ||
| return Err(ServiceError::InvalidRequest( | ||
| "cannot bind a Libra worktree base to a mount with a CL layer".into(), | ||
| )); |
There was a problem hiding this comment.
If POST /cl is building a CL for a clean mount, it sets the mount to Quiescing and drops the lock while entry.cl is still None; a concurrent /worktree/base call can pass this CL-only guard and set base_revision, after which build_cl resumes and records a CL layer on the already-bound worktree. That leaves a mount with both a Libra base binding and a CL layer even though both APIs are meant to reject that combination, so the binding path should also reject non-mounted/ready transition states or the CL build should recheck base_revision before committing.
Useful? React with 👍 / 👎.
1 participant
Summary
cl_pathto Antares mount requestsscorpiofs 0.4.0Why
Orion can construct a Buck-root mount while a CL targets
/project/distribution; the CL layer must be rebased to that path.Validation
scorpiofs 0.4.0on crates.iobuck2 build //project/distribution:distributionsucceeded with 3,219 local actions