Skip to content

Comments

nix key: Add a newline to the output#8849

Closed
edolstra wants to merge 1 commit intoNixOS:masterfrom
edolstra:fix-8844
Closed

nix key: Add a newline to the output#8849
edolstra wants to merge 1 commit intoNixOS:masterfrom
edolstra:fix-8844

Conversation

@edolstra
Copy link
Member

Motivation

Without the newline, the progress bar will overwrite the output on stdout. The alternative is to call stopProgressBar().

Fixes #8844.

Context

Checklist for maintainers

Maintainers: tick if completed or explain if not relevant

  • agreed on idea
  • agreed on implementation strategy
  • tests, as appropriate
    • functional tests - tests/**.sh
    • unit tests - src/*/tests
    • integration tests - tests/nixos/*
  • documentation in the manual
  • documentation in the internal API docs
  • code and comments are self-explanatory
  • commit message explains why the change was made
  • new feature or incompatible change: updated release notes

Priorities

Add 👍 to pull requests you find important.

Otherwise the progress bar will overwrite the output on stdout. The
alternative is to call stopProgressBar().

Fixes NixOS#8844.
@github-actions github-actions bot added the new-cli Relating to the "nix" command label Aug 18, 2023
@tomberek
Copy link
Contributor

I think I like the stopProgressBar() approach better. It prevents any change at all to the output and is clearer.

diff --git a/src/nix/sigs.cc b/src/nix/sigs.cc
index 45cd2e1a6..12d965ffb 100644
--- a/src/nix/sigs.cc
+++ b/src/nix/sigs.cc
@@ -2,6 +2,7 @@
 #include "shared.hh"
 #include "store-api.hh"
 #include "thread-pool.hh"
+#include "progress-bar.hh"
 
 #include <atomic>
 
@@ -173,6 +174,7 @@ struct CmdKeyGenerateSecret : Command
         if (!keyName)
             throw UsageError("required argument '--key-name' is missing");
 
+        stopProgressBar();
         writeFull(STDOUT_FILENO, SecretKey::generate(*keyName).to_string());
     }
 };
@@ -194,6 +196,7 @@ struct CmdKeyConvertSecretToPublic : Command
     void run() override
     {
         SecretKey secretKey(drainFD(STDIN_FILENO));
+        stopProgressBar();
         writeFull(STDOUT_FILENO, secretKey.toPublicKey().to_string());
     }
 };

@tomberek tomberek mentioned this pull request Aug 25, 2023
86 tasks
@roberth roberth marked this pull request as draft August 25, 2023 20:11
@Ericson2314
Copy link
Member

Perhaps we need to double check all the things using writeFull?

@thufschmitt thufschmitt added this to the CLI Stabilisation milestone Sep 4, 2023
@tomberek tomberek closed this Nov 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new-cli Relating to the "nix" command

Projects

None yet

Development

Successfully merging this pull request may close these issues.

nix key convert-secret-to-public is broken in 2.17

4 participants