-
Notifications
You must be signed in to change notification settings - Fork 394
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
Switch crosstool-ng configure scripts to xtask. #877
Conversation
This is related to the discussion here. |
Ok(targets) | ||
} | ||
|
||
fn configure_target( |
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.
I feel like all the if statements here on major and minor can be combined into a match (minor, major)
with either range patterns or if guards.
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.
I think that's somewhat difficult, since there's no fallthrough cases (the only time this is actually valuable):
if gcc_major > 4 || (gcc_major == 4 && gcc_major > 9) {
ct_gcc.push_str("\nCT_GCC_later_than_4_9=y");
}
if gcc_major > 4 || (gcc_major == 4 && gcc_major >= 9) {
ct_gcc.push_str("\nCT_GCC_4_9_or_later=y");
}
if gcc_major > 4 || (gcc_major == 4 && gcc_major > 8) {
ct_gcc.push_str("\nCT_GCC_later_than_4_8=y");
}
if gcc_major > 4 || (gcc_major == 4 && gcc_major >= 8) {
ct_gcc.push_str("\nCT_GCC_4_8_or_later=y");
}
Would become:
match (gcc_major, gcc_major) {
(5.., _) | (4, 10..) => {
ct_gcc.push_str("\nCT_GCC_later_than_4_9=y");
ct_gcc.push_str("\nCT_GCC_4_9_or_later=y");
ct_gcc.push_str("\nCT_GCC_later_than_4_8=y");
ct_gcc.push_str("\nCT_GCC_4_8_or_later=y");
},
(4, 9) => {
ct_gcc.push_str("\nCT_GCC_4_9_or_later=y");
ct_gcc.push_str("\nCT_GCC_later_than_4_8=y");
ct_gcc.push_str("\nCT_GCC_4_8_or_later=y");
}
(4, 8) => {
ct_gcc.push_str("\nCT_GCC_4_8_or_later=y");
}
_ => (),
}
These aren't exclusive if statements, they each run sequentially. I guess we could have an external helper function?
let push_ge_49 = |output: &mut String| {
ct_gcc.push_str("\nCT_GCC_4_9_or_later=y");
ct_gcc.push_str("\nCT_GCC_later_than_4_8=y");
ct_gcc.push_str("\nCT_GCC_4_8_or_later=y");
};
match (gcc_major, gcc_major) {
(5.., _) | (4, 10..) => {
ct_gcc.push_str("\nCT_GCC_later_than_4_9=y");
push_ge_49(&mut ct_gcc);
},
(4, 9) => {
push_ge_49(&mut ct_gcc);
}
(4, 8) => {
ct_gcc.push_str("\nCT_GCC_4_8_or_later=y");
}
_ => (),
}
But I still don't think that's better.
Rewrite the bash build scripts to Rust, allowing a better CLI interface, easier maintenance, and better help messages via clap.
8d7d1b1
to
74eb5c8
Compare
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.
I dislike the repetitive if blocks, but that just leaves more room for improvement, and if it works it works. LGTM!
bors r+
Build succeeded: |
Rewrite the bash build scripts to Rust, allowing a better CLI interface, easier maintenance, and better help messages via clap.