-
Notifications
You must be signed in to change notification settings - Fork 143
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
[FIXES #922] properly check for -CURRENT in OpenBSD steps and pass the correct flags to the respective commands #923
Conversation
…he correct flags
Sorry, rustfmt is borked on my OpenBSD install, had to force |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, thanks for sending the patch, left some comments
src/steps/os/openbsd.rs
Outdated
.status_checked() | ||
|
||
let mut args = vec!["/usr/sbin/sysupgrade", "-n"]; | ||
if is_openbsd_current() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function uses std::process::Command and Topgrade's dry-run option does not work with it, and thus this PR breaks the dry-run option.
We should use ctx.run_type().execute() instead, though I kinda think it won't work either. 😮💨
I guess we should skip all the update procedures and print something like "will update the system package" if this is a dry run(ctx has a dry_run() function to check if the current run is a dry-run or not)
.status_checked()?; | ||
let mut args = vec!["/usr/sbin/pkg_add", "-u"]; | ||
if is_openbsd_current() { | ||
args.push("-Dsnap"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
BTW, CI fixed in the main branch, you can rebase your branch:) |
@SteveLauC This good? Just found out |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks for the patch!
What does this PR do
See issue #922
Standards checklist
CONTRIBUTING.md