-
Notifications
You must be signed in to change notification settings - Fork 500
support "vxworks" and "nto" OSes on get_base_archiver_variant
#1456
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: onur-ozkan <[email protected]>
Signed-off-by: onur-ozkan <[email protected]>
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 fine overall.
src/lib.rs
Outdated
} else if target.os == "vxworks" { | ||
name = format!("wr-{}", tool).into(); | ||
self.cmd(&name) | ||
} else if target.os == "nto" { | ||
name = match target.arch { | ||
"i586" | "x86" => format!("ntox86-{}", tool).into(), | ||
"aarch64" | "x86_64" => format!("nto{}-{}", target.arch, tool).into(), | ||
_ => { | ||
return Err(Error::new( | ||
ErrorKind::InvalidTarget, | ||
format!("Unknown architecture for Neutrino QNX: {}", target.arch), | ||
)) | ||
} | ||
}; | ||
self.cmd(&name) |
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 know it's not really done elsewhere (yet), but would you mind adding documentation links to wr-ar
, ntoaarch64-ar
, etc.? I.e. documentation on where these names come from - would make it easier to maintain.
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 couldn't find any documentation for wr-ar
. I added one for nto
, but I'm not sure how useful it is.
)) | ||
} | ||
}; | ||
self.cmd(&name) | ||
} else if self.get_is_cross_compile()? { | ||
match self.prefix_for_target(&self.get_raw_target()?) { | ||
Some(prefix) => { |
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.
If you really want this to continue working in the future, it'd be nice if you could add either a test, or ideally a CI step that sets up the required environment for NTO and vxWorks and runs the normal test suite.
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.
Just added a coverage on certain targets along with NTO and wxWorks.
Thanks for the review, I will address your notes once I get some free time (probably around the weekend). |
Signed-off-by: onur-ozkan <[email protected]>
Signed-off-by: onur-ozkan <[email protected]>
Signed-off-by: onur-ozkan <[email protected]>
a426856
to
b1e4bd0
Compare
Signed-off-by: onur-ozkan <[email protected]>
b1e4bd0
to
59c3044
Compare
Extends the archiver detection logic to support the "vxworks" and "nto" operating systems. This change will allow us to remove this custom bootstrap logic and rely entirely on the
cc
crate as it already supports all other AR finding conditions handled by bootstrap. You can try this patch to test this.The original PRs that introduced these conditions in the bootstrap are: