Skip to content

Conversation

@jackluo923
Copy link
Member

@jackluo923 jackluo923 commented Jan 20, 2026

Description

This PR adds automatic fallback support for macOS BSD tar when GNU tar (gtar) is not available.

The build system currently assumes GNU tar is available on macOS, which causes build failures with "command not found: gtar" errors. This change:

  • Detects if gtar is available at runtime
  • Falls back to system tar (BSD tar on macOS) if gtar is not found
  • Conditionally applies GNU-specific flags (--no-anchored, --wildcards) only when using gtar
  • Allows builds to succeed on macOS without requiring GNU tar installation

This enables developers to build on macOS using only Homebrew-installed CMake and the system-provided BSD tar, without needing to install GNU tar separately.

Checklist

  • The PR satisfies the contribution guidelines.
  • This is a breaking change and that has been indicated in the PR title, OR this isn't a breaking change.
  • Necessary docs have been updated, OR no docs need to be updated.

Validation performed

  • Tested on macOS with BSD tar (no gtar installed)
  • Successfully built log-surgeon-ffi-py dependencies using task deps:install-all
  • Verified tar extraction works correctly for all C++ dependencies (fmt, Microsoft.GSL, log-surgeon)
  • Built and installed Python wheel successfully
  • Verified the package imports correctly

Summary by CodeRabbit

  • Refactor
    • Improved cross-platform archive extraction to handle varying tar implementations more reliably.
    • Separated command assembly from execution for clearer, safer runtime behavior.
    • Enhanced pattern-based include/exclude handling with fallback behavior to ensure consistent extraction across environments.

✏️ Tip: You can customize this high-level summary in your review settings.

Modify tar extraction to detect if gtar is available and fall back to
system tar on macOS. GNU-specific flags (--no-anchored, --wildcards)
are only used when gtar is present, enabling builds on macOS without
requiring GNU tar installation.

This resolves "command not found: gtar" errors on macOS systems where
GNU tar is not installed, allowing the build system to use the native
BSD tar instead.
@jackluo923 jackluo923 requested a review from a team as a code owner January 20, 2026 01:28
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 20, 2026

Walkthrough

Detects available archiver, builds a portable TAR_CMD array at runtime with conditional GNU-only flags, and appends dynamic --exclude/--include patterns before executing the assembled tar command, replacing the previous inline tar extraction invocation.

Changes

Cohort / File(s) Change summary
Tar extraction logic refactoring
exports/taskfiles/utils/remote.yaml
Replace direct tar extraction with archiver detection and runtime TAR_CMD array construction; add fallback to tar without GNU flags; move exclude/include handling to looped argument appends; preserve execution and error redirection to /dev/null.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: adding BSD tar fallback support for macOS, which directly aligns with the changeset's core objective of enabling tar operations on macOS without GNU tar.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

Break long TAR_CMD array initialization into multiple lines to comply
with yamllint max line length of 100 characters.
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@exports/taskfiles/utils/remote.yaml`:
- Line 116: The TAR_CMD array declaration exceeds the 100-character limit; break
the array into multiple lines so each element is on its own line to satisfy
linting. Modify the TAR_CMD assignment (the variable name TAR_CMD and use of
ARCHIVER, --extract, --strip-components="{{.NUM_COMPONENTS_TO_STRIP}}",
--directory "{{.OUTPUT_DIR}}", and --file "{{.TAR_FILE}}") so the opening
parenthesis and ARCHIVER are on the first line, each flag/argument is on its own
subsequent line, and the closing parenthesis is on its own line to keep line
lengths under 100 characters.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@exports/taskfiles/utils/remote.yaml`:
- Around line 139-140: The tar invocation currently silences all stderr via the
redirection on the "${TAR_CMD[@]}" invocation, which can hide real errors;
update the call to either remove the "2> /dev/null" so tar errors surface or
redirect stderr to a log file/variable instead, and if suppression is
intentionally desired add a clarifying comment explaining why warnings are
ignored; reference the TAR_CMD invocation ("${TAR_CMD[@]}") and ensure the
change preserves the intended behavior for noisy but non-fatal warnings or
surfaces fatal errors for debugging.

Comment on lines +139 to +140
# Execute tar command
"${TAR_CMD[@]}" 2> /dev/null
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider whether suppressing all stderr is desirable.

Redirecting 2> /dev/null hides all tar errors, which could mask legitimate issues (e.g., corrupted archives, permission problems). If this is intentional to suppress noisy warnings, a comment explaining the rationale would help future maintainers.

🤖 Prompt for AI Agents
In `@exports/taskfiles/utils/remote.yaml` around lines 139 - 140, The tar
invocation currently silences all stderr via the redirection on the
"${TAR_CMD[@]}" invocation, which can hide real errors; update the call to
either remove the "2> /dev/null" so tar errors surface or redirect stderr to a
log file/variable instead, and if suppression is intentionally desired add a
clarifying comment explaining why warnings are ignored; reference the TAR_CMD
invocation ("${TAR_CMD[@]}") and ensure the change preserves the intended
behavior for noisy but non-fatal warnings or surfaces fatal errors for
debugging.

Copy link
Member

@davidlion davidlion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Does the bsdtar have the same behaviour as --no-anchored --wildcards? If it doesn't then we're creating different behaviour on linux and mac that can lead to errors depending on the user's input to the task.

  2. If 1 is not an issue, then we should try to build the archiver command up as a variable like it was before since it helps make the script more readable/modular/etc.

  3. Ideally resolve any/all coderabbit comments before the review (unless you want the reviewers help/opinion) and request a review on github for tracking (e.g. so Bill and I don't overlap reviewing this).

  4. For the coderabbit comment, I'm a bit indifferent. I'd be fine with moving away from the original behaviour and printing the tar output. The extra output could help in debugging missing files and task's output is already quite verbose so I wouldn't be that worried about making it more messy (we should consult with Kirk and Bill if we want to switch this though).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants