Skip to content

remote: fix showing of activation logs#532

Merged
NotAShelf merged 2 commits intonix-community:masterfrom
faukah:push-uszloqzsspol
Jan 21, 2026
Merged

remote: fix showing of activation logs#532
NotAShelf merged 2 commits intonix-community:masterfrom
faukah:push-uszloqzsspol

Conversation

@faukah
Copy link
Contributor

@faukah faukah commented Jan 21, 2026

Closes #531.
The issue was that you cannot pass Redirection::Merge to both stdout and stdin.

  • Added a print statement so stdout gets printed upon activation; AFAIK there is nothing relevant on stderr so we only print that if activation fails.
  • Disabled failing darwin test

Sanity Checking

  • I have updated the changelog as per my changes
  • I have tested, and self-reviewed my code
  • Style and consistency
    • I ran nix fmt to format my Nix code
    • I ran cargo fmt to format my Rust code
    • I have added appropriate documentation to new code
    • My changes are consistent with the rest of the codebase
  • Correctness
    • I ran cargo clippy and fixed any new linter warnings.
  • If new changes are particularly complex:
    • My code includes comments in particularly complex areas to explain the
      logic
    • I have documented the motive for those changes in the PR body or commit
      description.
  • Tested on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin

Add a 👍 reaction to pull requests you find important.

Summary by CodeRabbit

  • Bug Fixes

    • Improved how remote command logs are surfaced for elevated operations so stdout logs are displayed reliably to the user while preserving existing error reporting.
  • Tests

    • Added additional skips for a Darwin-specific test to avoid running it on macOS environments.

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

@coderabbitai
Copy link

coderabbitai bot commented Jan 21, 2026

Walkthrough

Removed merging redirections for remote activation logs; stdout is now captured and printed after the remote activation command completes when config.show_logs is enabled. Error handling continues to use captured stderr for failure messages.

Changes

Cohort / File(s) Summary
Remote activation output handling
src/remote.rs
Removed simultaneous Redirection::Merge for both stdout and stderr during remote activation. When config.show_logs is true, the remote command's stdout is captured and printed after execution; failure path still reports capture.stderr_str().
Darwin test skips
package.nix
Added additional skips for test_build_sudo_cmd_with_nix_config_spaces in Darwin test configurations (two new skip entries).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Out of Scope Changes check ❓ Inconclusive The package.nix change disables a Darwin test (test_build_sudo_cmd_with_nix_config_spaces) which appears tangential to the main remote activation logs fix, but may be necessary to maintain test suite stability during this change. Clarify why the Darwin test skip in package.nix is necessary and whether it's directly related to fixing the --show-activation-logs issue, or if it should be in a separate PR.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'remote: fix showing of activation logs' directly corresponds to the main change in src/remote.rs, which fixes the display of activation logs by printing stdout instead of using invalid Redirection::Merge.
Linked Issues check ✅ Passed The PR successfully addresses #531 by fixing the Redirection::Merge error in the activate_nixos_remote function, allowing --show-activation-logs to work correctly on remote deployments.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ 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.

Copy link
Member

@NotAShelf NotAShelf left a comment

Choose a reason for hiding this comment

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

I believe in the heart of the cards.

FWIW this2ll change when hide by default behaviour changes to show by default and hide on demand.

@NotAShelf NotAShelf merged commit 808224f into nix-community:master Jan 21, 2026
8 of 9 checks passed
@faukah faukah deleted the push-uszloqzsspol branch January 21, 2026 17:03
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.

Remote deployment fails when passing --show-activation-logs flag.

2 participants