Skip to content

fix(3197): fix youki version command Part of Enhancing Compatibility with runc #3200

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

tommady
Copy link
Collaborator

@tommady tommady commented Jul 20, 2025

Description

Fix Youki version command
This is Part of Enhancing Compatibility with runc

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement
  • Test updates
  • CI/CD related changes
  • Other (please describe):

Testing

  • Added new unit tests
  • Added new integration tests
  • Ran existing test suite
  • Tested manually (please provide steps)
./target/debug/youki -v                                                                                                                                  
youki version: 0.5.4                                                                                                                                          
commit: 0.5.4-81806f8a9175bbed7cb176cb66f527ff89a187ac    

Related Issues

Fixes #3197

Additional Context

@utam0k utam0k requested a review from Copilot July 21, 2025 04:19
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes the youki version command to enhance compatibility with runc by updating the version output format and command-line interface behavior. The changes ensure that youki -v or youki --version produces output in a Moby-compatible format.

  • Modified CLI argument parsing to handle version flag separately instead of using a macro
  • Updated version output format to match Moby/runc compatibility requirements
  • Changed subcommand handling to be optional and display help when no subcommand is provided

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
crates/youki/src/main.rs Updated CLI parsing to add explicit version flag handling and made subcommands optional
crates/youki/src/commands/info.rs Modified version output format to be Moby-compatible

@@ -54,7 +38,11 @@ struct Opts {
youki_extend: YoukiExtendOpts,

#[clap(subcommand)]
subcmd: SubCommand,
subcmd: Option<SubCommand>,
Copy link
Preview

Copilot AI Jul 21, 2025

Choose a reason for hiding this comment

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

Making subcmd optional changes the CLI behavior significantly. Consider if this is the intended breaking change, as users who previously relied on subcommands being required may experience different behavior.

Suggested change
subcmd: Option<SubCommand>,
subcmd: SubCommand,

Copilot uses AI. Check for mistakes.

commands::completion::completion(completion, &mut app)
}
None => Opts::command()
.print_help()
.map_err(|e| anyhow::anyhow!(":{e}")),
Copy link
Preview

Copilot AI Jul 21, 2025

Choose a reason for hiding this comment

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

The error message format ":{e}" has an unusual leading colon. Consider using a more descriptive format like "Failed to print help: {e}".

Suggested change
.map_err(|e| anyhow::anyhow!(":{e}")),
.map_err(|e| anyhow::anyhow!("Failed to print help: {e}")),

Copilot uses AI. Check for mistakes.

@utam0k
Copy link
Member

utam0k commented Jul 21, 2025

@saku3 May I ask you to review this P?

@saku3
Copy link
Contributor

saku3 commented Jul 21, 2025

Sure!

commands::completion::completion(completion, &mut app)
}
None => Opts::command()
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
None => Opts::command()
None => app.print_help().map_err(|e| anyhow::anyhow!(":{e}"))

Signed-off-by: tommady <[email protected]>
@saku3
Copy link
Contributor

saku3 commented Jul 26, 2025

Thanks. I think this modification looks good.

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

Successfully merging this pull request may close these issues.

youki version command
3 participants