Rename cpx → copy, flatten the CLI, and add Nix/AUR/Guix packaging#1
Merged
Conversation
Rebrand the tool from cpx to copy, and make copy the root command (`copy <src> <dst>`) with `config` as the only subcommand — removing the former explicit `copy` subcommand and the argv-injection hack. - Cargo package + explicit [[bin]] name = "copy"; library crate `copy::` - CpxError/CpxResult -> CliError/CliResult (CopyError is the domain error) - Flatten CopyArgs into CLIArgs. Intercept a leading `config` argument and route it through a dedicated parser so the greedy `sources` variadic cannot swallow `config <sub>`; keep the subcommand field so `--help` still lists `config`. A source literally named `config` as the first arg must be written `./config`. - Rebrand config paths: ~/.config/copy/copyconfig.toml, /etc/copy/copyconfig.toml (also fixes the pre-existing /etc/cpx/config.toml inconsistency) - Update install.sh, benchmarks/bench.sh, release.yml, docs, and CLAUDE.md; rename GNU test scripts cpx-*.sh -> copy-*.sh - Drop the obsolete `copy copy ...` integration test; fix a pre-existing clippy lint in preprocess.rs Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
First-party packaging for `copy`, all building from this source tree (no crates.io). - Nix (flake): flake.nix + nix/package.nix (a single callPackage derivation shared by packages.default and overlays.default, built from a filtered local src with cargoLock.lockFile = ./Cargo.lock); apps.default for `nix run` and a devShell with the Rust toolchain. Verified: nix build, nix run, nix develop, nix flake check. - AUR: packaging/aur/PKGBUILD (+ .SRCINFO), pkgname `copy`, build-from-source with cargo --frozen and the committed lockfile; sha256sums=SKIP until the v0.1.4 tag is published. - Guix: guix.scm builds offline against a locally vendored guix/vendor dir (run `cargo vendor guix/vendor` first; gitignored). Needs rustc >= 1.85. - Wire-up: .gitignore (result, result-*, guix/vendor); README/README_CRATES install sections rewritten to first-party Nix/AUR/Guix usage; CLAUDE.md packaging note. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe project is renamed from Changescpx → copy project rename
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
install.sh (1)
39-40:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix installer/release artifact name mismatch for ARM targets.
detect_platform()returnslinux-aarch64-muslandlinux-armv7-musl, but the release workflow publishescopy-linux-aarch64.tar.gzandcopy-linux-armv7.tar.gz. ARM installs will 404.Suggested patch
detect_platform() { @@ - aarch64|arm64) echo "linux-aarch64-musl" ;; - armv7l) echo "linux-armv7-musl" ;; + aarch64|arm64) echo "linux-aarch64" ;; + armv7l) echo "linux-armv7" ;; *) error "Unsupported architecture: $arch" ;; esacAlso applies to: 75-76
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@install.sh` around lines 39 - 40, The detect_platform() function returns platform identifiers with the `-musl` suffix (linux-aarch64-musl and linux-armv7-musl) for ARM targets, but the actual release artifacts are named without this suffix (copy-linux-aarch64.tar.gz and copy-linux-armv7.tar.gz), causing 404 errors during ARM installs. Remove the `-musl` suffix from the echo statements in the aarch64|arm64 and armv7l cases at the indicated locations so the returned platform strings match the published artifact names exactly.src/cli/args.rs (1)
472-474:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInconsistent error messages reference
--continueinstead of--resume.Both conflict validation messages use
--continuebut the actual CLI flag is--resume(defined at line 112).
src/cli/args.rs#L472-L474: Change--continueto--resumein symbolic-link conflict message.src/cli/args.rs#L483-L485: Change--continueto--resumein hard-link conflict message.Proposed fix
@@ -470,13 +470,13 @@ fn validate_conflicts(options: &CopyOptions) -> Result<(), String> { if options.hard_link { return Err("--symbolic-link and --link cannot be used together".to_string()); } if options.resume { - return Err("--symbolic-link and --continue cannot be used together".to_string()); + return Err("--symbolic-link and --resume cannot be used together".to_string()); } if options.attributes_only { return Err( "--symbolic-link and --attributes-only cannot be used together".to_string(), ); } } if options.hard_link { if options.resume { - return Err("--link and --continue cannot be used together".to_string()); + return Err("--link and --resume cannot be used together".to_string()); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli/args.rs` around lines 472 - 474, The error messages in the symbolic-link and hard-link conflict validation checks incorrectly reference the `--continue` flag, but the actual CLI flag is named `--resume` (as defined at line 112). Update the error string in the symbolic-link conflict check at lines 472-474 to replace `--continue` with `--resume` in the message "--symbolic-link and --continue cannot be used together". Similarly, update the error string in the hard-link conflict check at lines 483-485 to replace `--continue` with `--resume` in the corresponding message for hard-link conflicts. Both changes ensure error messages accurately reflect the actual flag names available to users.
🧹 Nitpick comments (6)
README_CRATES.md (1)
130-176: 💤 Low valueAdd language specifier to fenced code block.
Line 130 opens a code block without a language tag. Add
bashfor syntax highlighting and consistency.-``` +```bash copy [OPTIONS] <SOURCE>... <DESTINATION>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README_CRATES.md` around lines 130 - 176, The fenced code block starting at line 130 in README_CRATES.md is missing a language specifier, which prevents syntax highlighting and is inconsistent with documentation best practices. Add the `bash` language identifier to the opening code fence (the triple backticks before the "copy [OPTIONS]" line) to enable proper syntax highlighting and maintain consistency with other code blocks in the document.Source: Linters/SAST tools
README.md (1)
116-162: 💤 Low valueAdd language specifier to fenced code block.
Line 116 opens a code block without a language tag. Add
bashfor syntax highlighting and consistency.-``` +```bash copy [OPTIONS] <SOURCE>... <DESTINATION>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 116 - 162, The fenced code block at line 116 in README.md is missing a language specifier for syntax highlighting. Add "bash" immediately after the opening triple backticks (change ``` to ```bash) on the line that precedes the "copy [OPTIONS]" command documentation to enable proper syntax highlighting and maintain consistency with other code blocks in the document.Source: Linters/SAST tools
CLAUDE.md (1)
43-43: 💤 Low valueAdd language specifier to fenced code blocks.
Three fenced code blocks lack language tags, triggering MD040 warnings. All are bash examples and should declare the language for syntax highlighting and consistency.
CLAUDE.md#L43-L43: Fenced code block showingmain.rsarchitecture — addbashlanguage tagREADME.md#L116-L116: Fenced code block showingcopycommand help — addbashlanguage tagREADME_CRATES.md#L130-L130: Fenced code block showingcopycommand help (mirrors README.md) — addbashlanguage tag🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@CLAUDE.md` at line 43, Add language specifiers to three fenced code blocks that currently lack language tags. In CLAUDE.md at line 43, add `bash` language tag to the fenced code block showing main.rs architecture. In README.md at line 116, add `bash` language tag to the fenced code block showing the copy command help. In README_CRATES.md at line 130, add `bash` language tag to the fenced code block showing the copy command help. Change each opening backtick sequence from triple backticks (```) to triple backticks followed by the word bash (```bash).Source: Linters/SAST tools
benchmarks/benchmarked_xcp.md (1)
17-87: ⚡ Quick winAdd blank lines before and after each table per Markdown linting rules (MD058).
Each of the 10 tables in this file needs a blank line immediately before the table header and immediately after the table content. This is required by markdownlint (MD058) and improves readability.
For example, before line 17 (OpenImageIO section header) and after line 20 (last table row), add blank lines.
🔧 Fix by adding blank lines around tables
Apply this pattern to all 10 tables (lines 17–86):
## OpenImageIO + | Command | Mean [ms] | Min [ms] | Max [ms] | Relative | |:---|---:|---:|---:|---:| | `/home/happy/copy/copy -r -j=16 /home/happy/copy_multi_bench/repos/OpenImageIO /home/happy/copy_multi_bench/dest_copy` | 122.7 ± 5.1 | 119.0 | 132.9 | 1.04 ± 0.04 | | `/home/happy/.cargo/bin/xcp -r /home/happy/copy_multi_bench/repos/OpenImageIO /home/happy/copy_multi_bench/dest_cp` | 118.5 ± 0.8 | 117.1 | 119.3 | 1.00 | + ## chromiumRepeat for all 10 section headers (OpenImageIO, chromium, kubernetes, node, openexr, linux, vscode, go, rust, godot, tensorflow, Full Dataset).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmarks/benchmarked_xcp.md` around lines 17 - 87, The markdown file requires blank lines before and after each table to comply with MD058 linting rules. For all 10 tables in the file (covering the sections: OpenImageIO, chromium, kubernetes, node, openexr, linux, vscode, go, rust, godot, tensorflow, and Full Dataset), add a blank line immediately before each table header and immediately after the final row of each table. This improves both linting compliance and readability without modifying any table content or section headers.Source: Linters/SAST tools
docs/benchmarks.md (1)
21-70: ⚡ Quick winAdd blank lines before and after tables per Markdown linting rules (MD058).
The three tables (cp, rsync, and per-repository results) each need a blank line immediately before the table header and immediately after the final row.
🔧 Fix by adding blank lines around tables
cp: + | Tool | Mean Time | Speedup | |------|-----------|---------| | copy | **28.72s ± 1.46s** | **2.81× faster** | | cp | 80.56s ± 2.79s | baseline | + **Time Saved:** 51.84 secondsApply the same pattern to the rsync table (around line 29) and the per-repository tables (around lines 42 and 57).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/benchmarks.md` around lines 21 - 70, Add blank lines before and after each of the three tables in the document to comply with Markdown linting rule MD058. Insert a blank line immediately before each table header row (the rows starting with "| Tool |" for the cp and rsync summary tables, and "| Repository |" for the per-repository result tables), and insert a blank line immediately after the final data row of each table. This includes adding blank lines before the cp table header, after the cp baseline row, before the rsync table header, after the rsync baseline row, before the per-repository cp table header, after the OpenEXR row in the per-repository cp table, before the per-repository rsync table header, and after the OpenEXR row in the per-repository rsync table.Source: Linters/SAST tools
tests/intergration.rs (1)
1301-1317: ⚡ Quick winAdd coverage for a literal first source named
config.Line 1301 currently verifies implicit root copy, but it doesn’t cover the parser edge case where the first positional is literally
config. Add a regression test assertingcopy ./config <dst>is treated as a file copy (notconfigsubcommand parsing).As per coding guidelines, “Special-case a leading 'config' argument in CLIArgs::parse() to route through ConfigInvocation parser.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/intergration.rs` around lines 1301 - 1317, Add a regression test to verify the parser edge case where a file literally named "config" is passed as the first source argument. Create a new test function (or extend test_implicit_copy_command) that writes a file named "config" to the temp directory and asserts that the command copy ./config <dst> successfully copies the file content (treating it as a file path, not as a config subcommand invocation). This ensures the special-case handling in CLIArgs::parse() correctly routes the literal "config" argument through ConfigInvocation parser and doesn't interfere with actual file copying operations.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@benchmarks/bench.sh`:
- Line 93: The echo statement that provides the fallback installation hint
references an incorrect package name. Change the package name from `copy-cli` to
`copy` in the echo statement to match the actual package name defined in
Cargo.toml, ensuring users receive the correct installation command when
following the hint.
In `@src/config/config_command.rs`:
- Around line 172-173: The documentation URL in the header string of the
config_command.rs file is malformed. The GitHub URL at the comment section needs
to be corrected to include the proper blob path format required by GitHub.
Replace the incorrect path
`https://github.com/UnbreakableMJ/copy/docs/configuration.md` with the correct
format that includes `/blob/main/` (or the appropriate branch name) between the
repository name and the file path so the documentation link works properly.
In `@tests/gnu/capability.sh`:
- Line 14: The precondition check for the COPY binary in the capability.sh test
script currently exits with status code 0 when the binary is not found, which
reports a pass instead of a skip to GNU-style test runners. Change the exit code
from 0 to 77 in the conditional block that checks if the COPY binary is
executable, so that missing dependencies properly signal a skip status rather
than a successful test run.
---
Outside diff comments:
In `@install.sh`:
- Around line 39-40: The detect_platform() function returns platform identifiers
with the `-musl` suffix (linux-aarch64-musl and linux-armv7-musl) for ARM
targets, but the actual release artifacts are named without this suffix
(copy-linux-aarch64.tar.gz and copy-linux-armv7.tar.gz), causing 404 errors
during ARM installs. Remove the `-musl` suffix from the echo statements in the
aarch64|arm64 and armv7l cases at the indicated locations so the returned
platform strings match the published artifact names exactly.
In `@src/cli/args.rs`:
- Around line 472-474: The error messages in the symbolic-link and hard-link
conflict validation checks incorrectly reference the `--continue` flag, but the
actual CLI flag is named `--resume` (as defined at line 112). Update the error
string in the symbolic-link conflict check at lines 472-474 to replace
`--continue` with `--resume` in the message "--symbolic-link and --continue
cannot be used together". Similarly, update the error string in the hard-link
conflict check at lines 483-485 to replace `--continue` with `--resume` in the
corresponding message for hard-link conflicts. Both changes ensure error
messages accurately reflect the actual flag names available to users.
---
Nitpick comments:
In `@benchmarks/benchmarked_xcp.md`:
- Around line 17-87: The markdown file requires blank lines before and after
each table to comply with MD058 linting rules. For all 10 tables in the file
(covering the sections: OpenImageIO, chromium, kubernetes, node, openexr, linux,
vscode, go, rust, godot, tensorflow, and Full Dataset), add a blank line
immediately before each table header and immediately after the final row of each
table. This improves both linting compliance and readability without modifying
any table content or section headers.
In `@CLAUDE.md`:
- Line 43: Add language specifiers to three fenced code blocks that currently
lack language tags. In CLAUDE.md at line 43, add `bash` language tag to the
fenced code block showing main.rs architecture. In README.md at line 116, add
`bash` language tag to the fenced code block showing the copy command help. In
README_CRATES.md at line 130, add `bash` language tag to the fenced code block
showing the copy command help. Change each opening backtick sequence from triple
backticks (```) to triple backticks followed by the word bash (```bash).
In `@docs/benchmarks.md`:
- Around line 21-70: Add blank lines before and after each of the three tables
in the document to comply with Markdown linting rule MD058. Insert a blank line
immediately before each table header row (the rows starting with "| Tool |" for
the cp and rsync summary tables, and "| Repository |" for the per-repository
result tables), and insert a blank line immediately after the final data row of
each table. This includes adding blank lines before the cp table header, after
the cp baseline row, before the rsync table header, after the rsync baseline
row, before the per-repository cp table header, after the OpenEXR row in the
per-repository cp table, before the per-repository rsync table header, and after
the OpenEXR row in the per-repository rsync table.
In `@README_CRATES.md`:
- Around line 130-176: The fenced code block starting at line 130 in
README_CRATES.md is missing a language specifier, which prevents syntax
highlighting and is inconsistent with documentation best practices. Add the
`bash` language identifier to the opening code fence (the triple backticks
before the "copy [OPTIONS]" line) to enable proper syntax highlighting and
maintain consistency with other code blocks in the document.
In `@README.md`:
- Around line 116-162: The fenced code block at line 116 in README.md is missing
a language specifier for syntax highlighting. Add "bash" immediately after the
opening triple backticks (change ``` to ```bash) on the line that precedes the
"copy [OPTIONS]" command documentation to enable proper syntax highlighting and
maintain consistency with other code blocks in the document.
In `@tests/intergration.rs`:
- Around line 1301-1317: Add a regression test to verify the parser edge case
where a file literally named "config" is passed as the first source argument.
Create a new test function (or extend test_implicit_copy_command) that writes a
file named "config" to the temp directory and asserts that the command copy
./config <dst> successfully copies the file content (treating it as a file path,
not as a config subcommand invocation). This ensures the special-case handling
in CLIArgs::parse() correctly routes the literal "config" argument through
ConfigInvocation parser and doesn't interfere with actual file copying
operations.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b916ec9d-29ef-4963-8a38-d01a9892f0c1
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockflake.lockis excluded by!**/*.lock
📒 Files selected for processing (34)
.github/workflows/release.yml.gitignoreCLAUDE.mdCONTRIBUTING.mdCargo.tomlREADME.mdREADME_CRATES.mdbenchmarks/bench.shbenchmarks/benchmark_single_thread.mdbenchmarks/benchmarked.mdbenchmarks/benchmarked_rsync.mdbenchmarks/benchmarked_xcp.mddocs/benchmarks.mddocs/configuration.mddocs/examples.mdflake.nixguix.scminstall.shnix/package.nixpackaging/aur/.SRCINFOpackaging/aur/PKGBUILDsrc/cli/args.rssrc/config/config_command.rssrc/config/loader.rssrc/error.rssrc/main.rssrc/utility/preprocess.rstests/gnu/abuse.shtests/gnu/acl.shtests/gnu/capability.shtests/gnu/copy-HL.shtests/gnu/copy-deref.shtests/gnu/copy-i.shtests/intergration.rs
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 3 file(s) based on 3 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 3 file(s) based on 3 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Rebrands the tool cpx → copy, flattens the CLI, and adds first-party Nix / AUR / Guix packaging. Two commits:
1. Rename
cpx→copyand flatten the CLI (14d054a)copy <src> <dst>is now the root command;configis the only subcommand (copy config show). The old explicitcopysubcommand and the argv-injection hack are gone.sourcespositional is a greedy variadic that would swallowconfig show,parse()intercepts a leadingconfigand routes it through a dedicated parser; thecommandfield stays onCLIArgsso--helpstill listsconfig. (A source literally namedconfigas the first arg must be written./config.)[[bin]] name = "copy", library cratecopy::,CpxError/CpxResult→CliError/CliResult, config paths (~/.config/copy/copyconfig.toml,/etc/copy/...),install.sh,bench.sh,release.yml, docs, and the GNU test scripts (cpx-*.sh→copy-*.sh).copy copy ...integration test; fixed a pre-existing clippy lint.2. Add Nix flake, AUR PKGBUILD, and Guix package (
808f26d)flake.nix+nix/package.nix(singlecallPackagederivation shared bypackages.defaultandoverlays.default, built from localsrcwithcargoLock.lockFile = ./Cargo.lock), plusapps.defaultand adevShell.packaging/aur/PKGBUILD(+.SRCINFO), build-from-source withcargo --frozen.guix.scmbuilds offline against a locally vendoredguix/vendordir (cargo vendor guix/vendorfirst; gitignored)..gitignoreandCLAUDE.mdupdated. No crates.io anywhere.Verification
cargo fmt --check,cargo clippy --all-targets -D warnings,cargo test→ all green (67 unit + 65 integration).config init|show|path,./config-as-file,--version,--help.nix build→result/bin/copy --version=copy 0.1.4,nix run,nix develop,nix flake checkall pass.bash -nclean (fullmakepkgnot run — needs an Arch host + published tag).guixavailable to build end-to-end; needs rustc ≥ 1.85).Follow-ups (out of scope, external)
cpx→copy, publish thev0.1.4tag (fills the AURsha256sums), and upload to AUR / nixpkgs as desired.🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
config showso it no longer gets mistaken for a source/positional.Documentation
copy.Chores
copyconfig.toml), and tests.