Oxen icon indicating copy to clipboard operation
Oxen copied to clipboard
verified publisher icon Oxen-AI

Oxb 199/glob pathing

Open rpschoenburg opened this issue 2 months ago • 4 comments

Summary by CodeRabbit

  • Improvements

    • Unified and enhanced glob/path expansion across add, remove, restore, and remote operations for more consistent wildcard behavior and local/remote parity.
    • Added a reusable glob options API to configure glob expansion behavior.

  • Tests

    • Added tests covering wildcard prefix matching, staged file operations, and wildcard restore behavior.

  • Chores

    • Minor logging, formatting, and whitespace cleanups.

rpschoenburg avatar Oct 22 '25 18:10 rpschoenburg

Walkthrough

Centralizes glob/path expansion by adding GlobOpts and util::glob::parse_glob_paths, replacing ad-hoc WalkDir/glob logic across add/restore/rm and workspace file handlers; introduces a glob utility, option re-exports, related tests, and small logging/formatting tweaks.

Changes

Cohort / File(s) Summary
New glob infrastructure
oxen-rust/src/lib/src/opts/glob_opts.rs, oxen-rust/src/lib/src/util/glob.rs		 Adds GlobOpts and parse_glob_paths() implementing multi-source glob expansion (staged DB, Merkle Tree, working dir), oxenignore filtering, recursive walking, helpers (collect_removed_paths) and unit tests.
Module exposure
oxen-rust/src/lib/src/opts.rs, oxen-rust/src/lib/src/util.rs		 Exposes new glob_opts module, re-exports GlobOpts, and registers util::glob module.
Core add refactor
oxen-rust/src/lib/src/core/v_latest/add.rs		 Replaces inline glob/WalkDir logic with GlobOpts + parse_glob_paths; adjusts repo scope/working-tree handling, canonical checks, batches staged removals, and updates logging.
Core restore refactor
oxen-rust/src/lib/src/core/v_latest/restore.rs, oxen-rust/src/lib/src/repositories/remote_mode/restore.rs		 Expands restore inputs via GlobOpts + parse_glob_paths, iterates per-expanded-path, and removes prior per-path glob branching.
Core rm changes
oxen-rust/src/lib/src/core/v_latest/rm.rs, oxen-rust/src/lib/src/repositories/rm.rs		 Switches rm flows to use GlobOpts + parse_glob_paths, updates log messages and test coverage for wildcard removal.
Workspace file operations
Files:
oxen-rust/src/lib/src/api/client/workspaces/files.rs, oxen-rust/src/lib/src/api/client/entries.rs		 Replaces manual per-entry glob expansion and WalkDir usage with GlobOpts + parse_glob_paths; computes relative paths for remote mode; small whitespace/debug log change in entries.rs.
Utility fs updates
oxen-rust/src/lib/src/util/fs.rs		 Removes legacy parse_glob_path(), adds is_relative_to_dir(), and cleans up glob-related imports/docs.
Options updates
oxen-rust/src/lib/src/opts/rm_opts.rs		 Adds Default impl and RmOpts::new() constructor.
Tests added / duplicates
oxen-rust/src/lib/src/repositories/add.rs, oxen-rust/src/lib/src/repositories/restore.rs		 Adds wildcard/prefix and staged-restore tests; duplicate test insertions appear in diff (same test blocks repeated).
Minor CLI formatting
oxen-rust/src/cli/src/cmd/add.rs		 Whitespace-only formatting change.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Caller as Caller (add/rm/restore)
    participant GlobOpts as GlobOpts
    participant Parser as parse_glob_paths
    participant Staged as Staged DB
    participant Merkle as Merkle Tree
    participant FS as Working Dir
    participant Op as Operation

    Caller->>GlobOpts: build options (paths, staged_db, merkle_tree, working_dir, walk_dirs)
    Caller->>Parser: parse_glob_paths(opts, repo?)
    Parser->>Parser: normalize & resolve inputs
    alt contains globs
        Parser->>Staged: search (if staged_db)
        Staged-->>Parser: matches
        Parser->>Merkle: traverse (if merkle_tree)
        Merkle-->>Parser: matches
        Parser->>FS: walk/search (if working_dir)
        FS-->>Parser: matches
    else non-glob
        Parser-->>Caller: resolved path
    end
    Parser->>Parser: apply oxenignore filters (if repo)
    Parser-->>Caller: HashSet<PathBuf> (expanded)
    Caller->>Op: perform add/rm/restore on expanded paths

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

  • Areas to focus review on:
    • oxen-rust/src/lib/src/util/glob.rs — multi-source expansion logic, correctness of matches, oxenignore filtering, and unit tests.
    • oxen-rust/src/lib/src/core/v_latest/add.rs — repo scope toggles, canonicalization, and staged-removal batching.
    • oxen-rust/src/lib/src/api/client/workspaces/files.rs — remote-mode relative path adjustments and replacement of WalkDir usage.
    • Duplicate test insertions in repositories/add.rs and repositories/restore.rs — verify duplicates are intentional.

Possibly related PRs

  • Oxen-AI/Oxen#131 — modifies workspace add call sites and local_repo usage; strongly related to workspace/path handling changes.
  • Oxen-AI/Oxen#616 — related core add refactor and path-expansion changes.
  • Oxen-AI/Oxen#645 — overlaps workspace add flow and directory/glob expansion handling.

Suggested reviewers

  • jcelliott
  • subygan
  • gschoeni

Poem

🐇 I hop through globs both near and far,
I map each path like a tiny star.
Staged, Merkle, working tree in sight,
I gather them all and set them right.
Hops and code — a tidy little night ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 54.55% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The PR title 'Oxb 199/glob pathing' is a branch reference (Jira ticket prefix + topic) and is related to the changeset, but it does not clearly describe the main change for someone reviewing commit history without additional context. Consider a more descriptive title like 'Support glob/wildcard path patterns in add, rm, and restore commands' to better convey the change to readers unfamiliar with the ticket reference.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • [ ] 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a comment
  • [ ] Commit unit tests in branch OXB-199/glob_pathing

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot] avatar Oct 22 '25 18:10 coderabbitai[bot]

@rpschoenburg it looks like repositories::remote_mode::commit::tests::test_remote_mode_commit_removed_dir is failing consistently, so probably an issue with the glob pathing. Can you take a look?

jcelliott avatar Oct 31 '25 21:10 jcelliott

@rpschoenburg comparing the output from the CI run vs my local, I noticed a difference that likely points to the problem:

Local

cargo test --lib repositories::remote_mode::commit::tests::test_remote_mode_commit_removed_dir -- --nocapture

...
Remote-mode repository initialized successfully!
Workspace ID: bafa0f9e-78dc-44fd-a825-5147708ab10e
Fetch remote branch: repo_503f8752-5b70-4cfa-8cb9-b70a8f9fffa1/main
Files to send: 2
🐂 oxen added 2 entries to workspace main: bafa0f9e-78dc-44fd-a825-5147708ab10e
Committing to remote with message: Adding train directory
...

CI

...
Remote-mode repository initialized successfully!
Workspace ID: 8530ccb0-adc0-44f5-b3e1-4387019deadf
Fetch remote branch: repo_a45b118b-85f7-4c47-a5a9-b7fcb03c5407/main
🐂 oxen added 1 entries to workspace main: 8530ccb0-adc0-44f5-b3e1-4387019deadf
Committing to remote with message: Adding train directory
Error running test. Err: Basic(Basic(No changes to commit))

We're not getting the Files to send: 2 message in CI. Looking at the code, that probably means that the files failed to upload for some reason or possibly were filtered out by the glob logic. I would put some debugging print statements in there and rerun in CI to see what you get. You could also temporarily change the test job to only run this one test to make debugging easier.

jcelliott avatar Nov 04 '25 18:11 jcelliott

To test this PR, I'd recommend doing add, rm, restore, or workspace/remote-mode add, rm, and restore with wildcard paths. The idea of this PR is that anything that you could use with git should also work for oxen.

With regards to the specific issue that inspired this PR (Adding removed paths with '.'), if you do an add that matches against both new files and removed files, the print message with the stats for each will show up. I'm not sure that's ideal, and I considered having the print statement for rm not show up, but for now it's still there.

As well, oxen restore . won't work, because oxen restore doesn't currently use the same path expansion at the cmd level as add and rm do. That could easily be changed.

At one point, I accidentally committed a file from my oxen mv PR. I removed it again in this PR, but it's in the commit history

rpschoenburg avatar Nov 11 '25 05:11 rpschoenburg