Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[#260] Show failed nix command#310

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
heitor-lassarote wants to merge5 commits intomaster
base:master
Choose a base branch
Loading
fromheitor-lassarote/#260-show-failed-nix-command

Conversation

@heitor-lassarote
Copy link
Member

This PR displays the command that failed when an error is produced after a command exits with a non-zero code. The output is also cleared a bit. Example:

🚀 ℹ️ [deploy] [INFO] Running checks for flake in examples/simple/flake.nix🚀 ❌ [deploy] [ERROR] Failed to check deployment: Nix checking command resulted in a bad exit code: Some(1). The failed command is provided below:Command { std: "nix" "flake" "check" "examples/simple/flake.nix", kill_on_drop: false }The stderr output is provided below:error: in flake URL 'examples/simple/flake.nix', 'flake.nix' is not a commit hash

Yethal reacted with thumbs up emoji
Problem: Many errors result from attempts to build commands which mighterror with a non-zero exit code. The failed commands are not displayedto the user, however.Solution: For every `([A-Za-z])Exit(Option<i32>)` error, change it to`\1Exit(Option<i32>, String)`, where the `String` contains the failedcommand. Format each `tokyo` command using `{:?}` and save it to thecorresponding error.Note that `Command` cannot be copied or cloned, and it's not possible toaccess a command's `std` field to get the underlying command, so weresort to putting the `Debug`-formatted command.
Problem: It's common to spawn a command and occasionally fail duringrunning the command or because it exited, leading to code repetition.Solution: Create a `command` module to abstract some of the boilerplateby wrapping `tokio::process::Command`. Create a `run` function thatabstracts most of this boilerplate.In some parts, the way commands are executed are a bit different, soinstead we leave them as it is.
Problem: The output when a command failed for deployment would show theerror message in the console output and _then_ the error message, whichwas confusing.Solution: Show the error message together with the error in an orderthat makes sense. This is done by capturing the output in the errormessage and printinting it in an exit error. We also use `Stdio::null`rather than `Stdio::piped` to avoid printing to the console.
@heitor-lassaroteheitor-lassarote self-assigned thisJan 10, 2025
@heitor-lassaroteheitor-lassarote marked this pull request as ready for reviewJanuary 10, 2025 22:54
}
}

pubfnarg<S:AsRef<OsStr>>(&mutself,arg:S) ->&mutCommand{
Copy link
Member

Choose a reason for hiding this comment

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

What is the motivation behind the reimplementation ofarg and other methods defined forTokioCommand rather than creating a singlenew constructor that accepts already constructedTokioCommand andrun to run the command handling the result?

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

I pushed a commit with your suggestion. It simplifiescommand.rs, but makes the consumer code a bit more verbose.

}

#[derive(Error,Debug)]
pubenumActivateError{
Copy link
Member

Choose a reason for hiding this comment

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

I like the fact that this error type became simpler

heitor-lassarote reacted with heart emoji

let ssh_activate_exit_status = ssh_activate_child
.wait()
.wait_with_output()
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason why therun method ofcommand::Command is not used here instead ofwait_with_output?

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Becausessh_activate_child is atokio::process::Child and not acommand::Command ortokio::process::Command. Although there are patterns, the way the codebase handles children is not repetitive enough to easily abstract them into arun command for a newcommand::Child, but I can put some more thought into this if you'd like.

rvem reacted with thumbs up emoji
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@rvemrvemrvem left review comments

@PhilTakenPhilTakenAwaiting requested review from PhilTaken

At least 1 approving review is required to merge this pull request.

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@heitor-lassarote@rvem

[8]ページ先頭

©2009-2025 Movatter.jp