Open
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughCommand-line preprocessing now encodes bsdtar-style Changes
Sequence DiagramsequenceDiagram
participant CLI as CLI Args
participant Preproc as Preprocessor (main.rs)
participant Parser as Clap Parser (cli.rs)
participant Collector as Collector (core.rs)
participant Executor as Bsdtar Cmd (bsdtar.rs)
CLI->>Preproc: raw args (may include -C / --cd / --directory)
Preproc->>Preproc: expand_bsdtar_old_style_args(), expand_bsdtar_w_option()
Preproc->>Preproc: encode_bsdtar_cd_args() -> replace -C forms with "\0CD\0dir"
Preproc->>Parser: transformed args
Parser->>Collector: parsed command + args vector
Collector->>Collector: ItemSource::parse() detects "\0CD\0" -> ChangeDir(dir)
Collector->>Collector: collect_items_from_sources() applies env::set_current_dir(dir)
Collector->>Executor: CollectedItems with fs_path-adjusted filesystem entries
Executor->>Executor: perform extract/list/create using per-operation CWD contexts
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors the handling of the -C (change directory) option to support positional behavior, aligning it with standard bsdtar usage. It introduces a preprocessing step that encodes directory changes into sentinel-prefixed arguments, which are then processed during item collection to change the working directory dynamically. Feedback highlights a regression in the old-style argument parser where removing 'C' from the list of options with arguments breaks dashless command bundles. Additionally, the extraction logic contains a bug where all directory changes are applied sequentially before any files are processed, failing to maintain the positional relationship between the directory change and specific file patterns.
| /// compression level via `Option<Option<XzLevel>>`.) | ||
| /// Kept in sync with `BsdtarCommand` clap definition via unit test. | ||
| const SHORT_OPTIONS_WITH_ARG: &[char] = &['b', 'C', 'f', 'I', 's', 'T', 'W', 'X']; | ||
| const SHORT_OPTIONS_WITH_ARG: &[char] = &['b', 'f', 'I', 's', 'T', 'W', 'X']; |
Contributor
There was a problem hiding this comment.
Removing 'C' from SHORT_OPTIONS_WITH_ARG breaks old-style (dashless) bundles where -C is not the last option. For example, in pna compat bsdtar cCf dir archive.pna, the expand_bsdtar_old_style_args function will no longer recognize that C takes an argument, leading to incorrect argument association. It would produce -c -C -f archive.pna dir, and then encode_bsdtar_cd_args would incorrectly treat -f as the directory for -C instead of dir.
Suggested change
| const SHORT_OPTIONS_WITH_ARG: &[char] = &['b', 'f', 'I', 's', 'T', 'W', 'X']; | |
| const SHORT_OPTIONS_WITH_ARG: &[char] = &['b', 'C', 'f', 'I', 's', 'T', 'W', 'X']; |
Comment on lines
+1121
to
+1132
| let (cd_dirs, file_patterns): (Vec<_>, Vec<_>) = files.into_iter().partition_map(|f| { | ||
| if let Some(dir) = f.strip_prefix(crate::cli::CD_SENTINEL) { | ||
| itertools::Either::Left(PathBuf::from(dir)) | ||
| } else { | ||
| itertools::Either::Right(f) | ||
| } | ||
| }); | ||
| for dir in &cd_dirs { | ||
| env::set_current_dir(dir) | ||
| .with_context(|| format!("changing directory to {}", dir.display()))?; | ||
| } | ||
| let files = file_patterns; |
Contributor
There was a problem hiding this comment.
The extraction logic for -C is not positional. By partitioning all directory changes and applying them sequentially before processing any file patterns, the implementation effectively applies the final directory state to all files. In bsdtar, -C should only affect the files that follow it on the command line. For example, -xf archive.pna -C dir1 file1 -C dir2 file2 should extract file1 into dir1 and file2 into dir2, but this implementation will extract both into dir2 (or dir1/dir2 if the paths are relative).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cli/src/command/bsdtar.rs (1)
1508-1542:⚠️ Potential issue | 🟠 MajorReject
@archiveoperands in update mode instead of dropping them.The old
collect_items_from_pathspath here never produced archive markers, so@source.pna/@-would fail as unsupported input. This new flow parses them and then silently filters them out, which turnspna compat bsdtar -u -f target.pna@source.pna`` into a successful no-op.💡 Suggested fix
files = utils::expand_bsdtar_windows_globs(files)?; let sources = ItemSource::parse_many(&files); validate_no_duplicate_stdin(&sources)?; @@ }; let mut resolver = HardlinkResolver::new(collect_options.follow_links); let collected = collect_items_from_sources(sources, &collect_options, &mut resolver)?; - let target_items = collected - .into_iter() - .filter_map(|item| match item { - crate::command::core::CollectedItem::Filesystem(entry) => Some(entry), - crate::command::core::CollectedItem::ArchiveMarker(_) => None, - }) - .collect(); + let mut target_items = Vec::new(); + for item in collected { + match item { + crate::command::core::CollectedItem::Filesystem(entry) => target_items.push(entry), + crate::command::core::CollectedItem::ArchiveMarker(source) => { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + format!("update mode does not support archive inclusion source {source}"), + ) + .into()); + } + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/src/command/bsdtar.rs` around lines 1508 - 1542, The code currently parses archive-style operands (e.g., `@source.pna`, `@-`) and ends up filtering out CollectedItem::ArchiveMarker in the target_items collection, causing update mode to silently no-op; instead detect and reject archive operands when running in update mode. After ItemSource::parse_many(&files) (or immediately after collect_items_from_sources), check for archive operands / CollectedItem::ArchiveMarker and if args.update (update mode) is set return an error rather than dropping them; reference the ItemSource/CollectedItem enum, collect_items_from_sources, and the target_items filtering code and return a clear user-facing error when ArchiveMarker is encountered in update mode.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cli/src/cli/old_style.rs`:
- Around line 9-13: SHORT_OPTIONS_WITH_ARG currently omits 'C', causing
old-style expansion to treat a non-terminal C bundle (e.g. "cCf" or "cCv") as
separate flags and mis-assign operands; add 'C' back into the const
SHORT_OPTIONS_WITH_ARG (so old-style expansion will consume the next token as
C's operand) and add a regression test that asserts parsing/expansion for a
non-terminal C bundle like "cCf" or "cCv" yields the expected sequence (e.g. -c
-C <dir> -f <archive>) to prevent future regressions.
In `@cli/src/command/core.rs`:
- Around line 591-609: The current ItemSource::ChangeDir branch can yield zero
collected items and thus allow create-mode to proceed with no real inputs; after
the input-parsing/collection loop (the code handling ItemSource variants that
produces results and is consumed by run_create_archive), add a post-parse
validation that ensures at least one actual source was produced (i.e., not only
ChangeDir entries). Implement this by checking either that results is non-empty
or that the original ItemSource list contains at least one entry not matching
ItemSource::ChangeDir, and if the check fails return the same create-mode error
used by run_create_archive (an io::Error or the existing validation error) with
a clear message like “create requires at least one input path or `@archive`
source.” Ensure you update the same function that builds results so callers such
as run_create_archive never receive an empty actionable input set.
---
Outside diff comments:
In `@cli/src/command/bsdtar.rs`:
- Around line 1508-1542: The code currently parses archive-style operands (e.g.,
`@source.pna`, `@-`) and ends up filtering out CollectedItem::ArchiveMarker in the
target_items collection, causing update mode to silently no-op; instead detect
and reject archive operands when running in update mode. After
ItemSource::parse_many(&files) (or immediately after
collect_items_from_sources), check for archive operands /
CollectedItem::ArchiveMarker and if args.update (update mode) is set return an
error rather than dropping them; reference the ItemSource/CollectedItem enum,
collect_items_from_sources, and the target_items filtering code and return a
clear user-facing error when ArchiveMarker is encountered in update mode.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5fcf870c-8bc6-4429-b993-06537504dfd6
📒 Files selected for processing (7)
cli/src/cli.rscli/src/cli/old_style.rscli/src/command/bsdtar.rscli/src/command/core.rscli/src/main.rscli/tests/cli/stdio.rscli/tests/cli/stdio/option_cd.rs
Comment on lines
+591
to
609
| ItemSource::ChangeDir(dir) => { | ||
| env::set_current_dir(&dir) | ||
| .map_err(|e| io::Error::new(e.kind(), format!("{}: {}", dir.display(), e)))?; | ||
| } | ||
| ItemSource::Filesystem(path) => { | ||
| let items = collect_items_with_state(&path, options, hardlink_resolver)?; | ||
| results.extend(items.into_iter().map(CollectedItem::Filesystem)); | ||
| } | ||
| ItemSource::Archive(archive_source) => { | ||
| results.push(CollectedItem::ArchiveMarker(archive_source)); | ||
| ItemSource::Archive(ArchiveSource::File(path)) => { | ||
| let abs = if path.is_relative() { | ||
| env::current_dir()?.join(&path) | ||
| } else { | ||
| path | ||
| }; | ||
| results.push(CollectedItem::ArchiveMarker(ArchiveSource::File(abs))); | ||
| } | ||
| ItemSource::Archive(source @ ArchiveSource::Stdin) => { | ||
| results.push(CollectedItem::ArchiveMarker(source)); | ||
| } |
Contributor
There was a problem hiding this comment.
ChangeDir-only inputs now bypass the create-mode input check.
This branch can turn a non-empty source list into zero collected items. run_create_archive still validates inputs before parsing the sentinel stream, so pna compat bsdtar -c -f out.pna -C dir now slips past the existing “requires at least one input path or @archive source” check and reaches archive creation with no actionable inputs. Callers need a post-parse check that at least one source is not ChangeDir.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cli/src/command/core.rs` around lines 591 - 609, The current
ItemSource::ChangeDir branch can yield zero collected items and thus allow
create-mode to proceed with no real inputs; after the input-parsing/collection
loop (the code handling ItemSource variants that produces results and is
consumed by run_create_archive), add a post-parse validation that ensures at
least one actual source was produced (i.e., not only ChangeDir entries).
Implement this by checking either that results is non-empty or that the original
ItemSource list contains at least one entry not matching ItemSource::ChangeDir,
and if the check fails return the same create-mode error used by
run_create_archive (an io::Error or the existing validation error) with a clear
message like “create requires at least one input path or `@archive` source.”
Ensure you update the same function that builds results so callers such as
run_create_archive never receive an empty actionable input set.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cli/src/cli/old_style.rs`:
- Around line 203-207: The guard that rejects concatenated `-C` values beginning
with `-` is too strict: in the preprocessing branch that matches
`bytes.strip_prefix(b"-C")` you should allow `rest` values that start with `-`
(e.g. `-C-foo`) while still rejecting empty arguments; remove the
`!rest.starts_with(b"-")` check so the `if let Some(rest) =
bytes.strip_prefix(b"-C") && !rest.is_empty()` branch accepts `-C-...` forms and
continues to pass `rest` to the existing downstream handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
cli/src/command/core.rs (1)
591-620:⚠️ Potential issue | 🟠 Major
ChangeDircan still erase the entire actionable input set.A source list like
-C dirnow becomesOk(vec![]). Create-mode validation still happens before sentinel decoding, so this path can proceed with no real inputs unless you add a post-parse check here or in the create-mode caller.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/src/command/core.rs` around lines 591 - 620, The ChangeDir branch (ItemSource::ChangeDir) can leave the parsed sources producing no actionable inputs (results empty), which lets create-mode continue without inputs; after the loop that builds results (or immediately after handling ItemSource::ChangeDir) add a validation that errors when running in create-mode with an empty results set (or move/create the create-mode validation to run after sentinel decoding so it sees the post-ChangeDir results). Reference ItemSource::ChangeDir, the results Vec and the create-mode caller (or sentinel decoding location) and ensure the code returns an Err when create-mode is requested but results.is_empty().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cli/src/command/bsdtar.rs`:
- Around line 1505-1506: ItemSource::parse_many now produces ArchiveMarker
entries for `@foo/`@-, but later code's filter_map silently drops those and can
turn a supplied operand into a no-op (possibly leaving target_items empty); in
update mode you must reject ArchiveMarker operands instead of filtering them
out. After let sources = ItemSource::parse_many(&files); (and before the
filter_map that builds target_items) scan sources for any ArchiveMarker and
return an error when running in update mode (the same guard should be applied
around the code paths noted at lines ~1532-1539), so the code explicitly fails
on `@archive/`@- rather than silently ignoring them. Ensure the check uses the
same error reporting used elsewhere and references the ArchiveMarker variant
from ItemSource.
---
Duplicate comments:
In `@cli/src/command/core.rs`:
- Around line 591-620: The ChangeDir branch (ItemSource::ChangeDir) can leave
the parsed sources producing no actionable inputs (results empty), which lets
create-mode continue without inputs; after the loop that builds results (or
immediately after handling ItemSource::ChangeDir) add a validation that errors
when running in create-mode with an empty results set (or move/create the
create-mode validation to run after sentinel decoding so it sees the
post-ChangeDir results). Reference ItemSource::ChangeDir, the results Vec and
the create-mode caller (or sentinel decoding location) and ensure the code
returns an Err when create-mode is requested but results.is_empty().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a52e86f3-4db1-4601-912c-e1a8452a8888
📒 Files selected for processing (2)
cli/src/command/bsdtar.rscli/src/command/core.rs
Comment on lines
+1505
to
+1506
| let sources = ItemSource::parse_many(&files); | ||
| validate_no_duplicate_stdin(&sources)?; |
Contributor
There was a problem hiding this comment.
Reject @archive operands in update mode instead of dropping them.
ItemSource::parse_many now makes @foo / @- special here, but the later filter_map throws every ArchiveMarker away. That turns a user-supplied operand into a silent no-op and can leave target_items empty with a successful exit.
💡 Suggested guard
let sources = ItemSource::parse_many(&files);
-validate_no_duplicate_stdin(&sources)?;
+if sources.iter().any(|s| matches!(s, ItemSource::Archive(_))) {
+ anyhow::bail!("update mode does not support `@archive` sources");
+}Also applies to: 1532-1539
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cli/src/command/bsdtar.rs` around lines 1505 - 1506, ItemSource::parse_many
now produces ArchiveMarker entries for `@foo/`@-, but later code's filter_map
silently drops those and can turn a supplied operand into a no-op (possibly
leaving target_items empty); in update mode you must reject ArchiveMarker
operands instead of filtering them out. After let sources =
ItemSource::parse_many(&files); (and before the filter_map that builds
target_items) scan sources for any ArchiveMarker and return an error when
running in update mode (the same guard should be applied around the code paths
noted at lines ~1532-1539), so the code explicitly fails on `@archive/`@- rather
than silently ignoring them. Ensure the check uses the same error reporting used
elsewhere and references the ArchiveMarker variant from ItemSource.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
cli/src/cli/old_style.rs (1)
203-207:⚠️ Potential issue | 🟡 MinorAllow concatenated
-Cvalues that begin with-.Line 206 still rejects
-C-foo, which is a valid short-option-with-argument spelling and can now leak to clap unencoded.💡 Proposed fix
- if let Some(rest) = bytes.strip_prefix(b"-C") - && !rest.is_empty() - && !rest.starts_with(b"-") + if let Some(rest) = bytes.strip_prefix(b"-C") + && !rest.is_empty() { let mut sentinel = OsString::from(CD_SENTINEL); sentinel.push(unsafe { std::ffi::OsStr::from_encoded_bytes_unchecked(rest) }); result.push(sentinel); continue; }+ #[test] + fn encode_cd_concatenated_dash_prefixed_value() { + let args = s(&["pna", "compat", "bsdtar", "-C-foo", "file"]); + let result = encode_bsdtar_cd_args(args); + assert_eq!(result, s(&["pna", "compat", "bsdtar", "\0CD\0-foo", "file"])); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/src/cli/old_style.rs` around lines 203 - 207, The condition that rejects concatenated `-C` arguments starting with `-` is too strict; in the `if let Some(rest) = bytes.strip_prefix(b"-C") && !rest.is_empty() && !rest.starts_with(b"-")` check remove the `!rest.starts_with(b"-")` clause so that `rest` (the concatenated value from `bytes.strip_prefix`) is accepted even when it begins with `-`; leave the `strip_prefix` and `!rest.is_empty()` checks intact and adjust any downstream handling expecting `rest` accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@cli/src/cli/old_style.rs`:
- Around line 203-207: The condition that rejects concatenated `-C` arguments
starting with `-` is too strict; in the `if let Some(rest) =
bytes.strip_prefix(b"-C") && !rest.is_empty() && !rest.starts_with(b"-")` check
remove the `!rest.starts_with(b"-")` clause so that `rest` (the concatenated
value from `bytes.strip_prefix`) is accepted even when it begins with `-`; leave
the `strip_prefix` and `!rest.is_empty()` checks intact and adjust any
downstream handling expecting `rest` accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ff8ca54f-82a0-4a95-94d7-161137446a8e
📒 Files selected for processing (4)
cli/src/cli/old_style.rscli/src/command/core.rscli/src/utils.rscli/src/utils/windows_glob.rs
✅ Files skipped from review due to trivial changes (3)
- cli/src/utils.rs
- cli/src/utils/windows_glob.rs
- cli/src/command/core.rs
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
cli/src/cli/old_style.rs (1)
207-210:⚠️ Potential issue | 🟡 MinorAllow concatenated
-Cvalues beginning with-.Line 209 blocks valid forms like
-C-foo, so they won’t be sentinel-encoded and can fail later now that-Cis preprocessing-only. Please remove that guard and add a regression test for-C-foo.💡 Proposed fix
- if let Some(rest) = bytes.strip_prefix(b"-C") - && !rest.is_empty() - && !rest.starts_with(b"-") + if let Some(rest) = bytes.strip_prefix(b"-C") + && !rest.is_empty() { result.push(make_cd_sentinel(unsafe { std::ffi::OsStr::from_encoded_bytes_unchecked(rest) })); continue; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/src/cli/old_style.rs` around lines 207 - 210, The conditional in the bytes parsing branch that checks rest.starts_with(b"-") incorrectly rejects valid concatenated -C values like "-C-foo"; update the if condition that currently reads if let Some(rest) = bytes.strip_prefix(b"-C") && !rest.is_empty() && !rest.starts_with(b"-") { to drop the !rest.starts_with(b"-") guard so any non-empty rest is accepted, keep the rest.empty() check, and add a regression test exercising the parser with "-C-foo" to ensure it is sentinel-encoded/preprocessed as expected (refer to the bytes.strip_prefix(b"-C") handling and the rest variable in old_style.rs).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@cli/src/cli/old_style.rs`:
- Around line 207-210: The conditional in the bytes parsing branch that checks
rest.starts_with(b"-") incorrectly rejects valid concatenated -C values like
"-C-foo"; update the if condition that currently reads if let Some(rest) =
bytes.strip_prefix(b"-C") && !rest.is_empty() && !rest.starts_with(b"-") { to
drop the !rest.starts_with(b"-") guard so any non-empty rest is accepted, keep
the rest.empty() check, and add a regression test exercising the parser with
"-C-foo" to ensure it is sentinel-encoded/preprocessed as expected (refer to the
bytes.strip_prefix(b"-C") handling and the rest variable in old_style.rs).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 23a117ff-6d3f-4d86-993b-0c9921ce51b1
📒 Files selected for processing (2)
cli/src/cli/old_style.rscli/src/command/core.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- cli/src/command/core.rs
Encode -C dir as \0CD\0dir sentinel in the pre-processing pipeline, then consume it positionally via ItemSource::ChangeDir in collect_items_from_sources. - Remove working_dir: Option<PathBuf> from BsdtarCommand - create/append: ChangeDir handled inline during item collection - update: switch from collect_items_from_paths to collect_items_from_sources - extract/list: strip sentinel values and apply as global chdir - Fix collapsible-if clippy lint in write_from_path
Without 'C' in SHORT_OPTIONS_WITH_ARG, old-style bundles like cCf would misparse: -C would not consume its directory argument during old-style expansion, causing subsequent flags to shift. 'C' is now handled the same way as 'W' — listed in SHORT_OPTIONS_WITH_ARG for old-style expansion but excluded from the clap sync assertion since it is consumed by encode_bsdtar_cd_args preprocessing.
Previously, expand_bsdtar_windows_globs ran before ItemSource parsing, which meant glob patterns were expanded relative to the original cwd rather than the directory set by a preceding -C. This caused all windows_glob tests to fail on Windows CI. Now glob expansion happens per-item inside collect_items_from_sources, after any ChangeDir has taken effect, matching bsdtar's behavior where glob expansion occurs relative to the most recent -C directory.
- Use #[cfg(windows)] to skip expand_bsdtar_windows_globs on non-Windows,
avoiding to_string_lossy round-trip that could corrupt non-UTF-8 paths
- Use format!("{e:#}") instead of e.to_string() to preserve full error chain
- Add pipeline composition test for cCf old-style expansion
- Extract make_cd_sentinel helper in encode_bsdtar_cd_args (5 sites → 1) - Unify #[cfg(windows)] / #[cfg(not(windows))] into shared loop - Add doc comments to CollectedEntry path/fs_path fields
…ard in update mode - Create mode: check for real input paths after filtering -C sentinels, so `pna compat bsdtar -cf out.pna -C dir` correctly errors instead of creating an empty archive - Update mode: use parse_many_no_archives to treat @ as filesystem paths, preventing silent discard of @archive operands that update cannot handle - Add ItemSource::parse_with and parse_many_no_archives for modes that should not recognize @archive syntax
…xtract/update/append Replace opaque boolean in ItemSource::parse_with with ParseMode enum for clarity at call sites. Add CollectedEntry::new() constructor to centralize the fs_path = cwd.join(path) invariant. Add integration tests verifying -C works correctly in extract, update, and append modes.
Replace local create_test_archive and get_archive_entry_names with the shared implementations from utils/archive.
ChanTsune
force-pushed
the
cli/bsdtar/positional-cd
branch
from
f82a5dc to
74c987b
Compare
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 by CodeRabbit
New Features
Bug Fixes
Tests