Skip to content
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.

Sign up for GitHub

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

Add linker for MacOS Ventura (13.0.1) #702

Merged
merged 5 commits into from
Dec 21, 2022
Merged

Add linker for MacOS Ventura (13.0.1) #702

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c83a14b
Ignore `build_with_clang_linker_windows()` unit test for every OS but…
volsa Dec 18, 2022
8a9b0b0
Add linker for MacOS Ventura 13.0.1
volsa Dec 19, 2022
16233c1
Ignore `link_to_a_relative_location_with_no_parent()` function
volsa Dec 19, 2022
4d59efd
Update linker
volsa Dec 20, 2022
726e093
Merge branch 'master' into macos
volsa Dec 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 32 additions & 59 deletions src/linker.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ pub struct Linker {
}

trait LinkerInterface {
fn get_platform(&self) -> String;
fn add_obj(&mut self, path: &str);
fn add_lib(&mut self, path: &str);
fn add_lib_path(&mut self, path: &str);
Expand All @@ -30,16 +29,21 @@ trait LinkerInterface {
impl Linker {
pub fn new(target: &str, linker: Option<&str>) -> Result<Linker, LinkerError> {
let target_os = target.split('-').collect::<Vec<&str>>()[2];
let linker: Box<dyn LinkerInterface> = if let Some(linker) = linker {
Box::new(CcLinker::new(linker))
} else {
match target_os {
"linux" | "gnu" => Ok(Box::new(LdLinker::new())),
// "win32" | "windows" => Ok(Box::new(CcLinker::new("clang".to_string()))),
_ => Err(LinkerError::Target(target_os.into())),
}?
};
Ok(Linker { errors: Vec::default(), linker })

Ok(Linker {
errors: Vec::default(),
linker: match linker {
Some(linker) => Box::new(CcLinker::new(linker)),

// TODO: Linker for Windows is missing, see also:
// https://github.com/PLC-lang/rusty/pull/702/files#r1052446296
None => match target_os {
"win32" | "windows" => return Err(LinkerError::Target(target_os.into())),

_ => Box::new(LdLinker::new()),
},
},
})
}

/// Add an object file or static library to linker input
Expand Down Expand Up @@ -115,10 +119,6 @@ impl CcLinker {
}

impl LinkerInterface for CcLinker {
fn get_platform(&self) -> String {
"Linux".into()
}

fn add_obj(&mut self, path: &str) {
self.args.push(path.into());
}
Expand Down Expand Up @@ -179,10 +179,6 @@ impl LdLinker {
}

impl LinkerInterface for LdLinker {
fn get_platform(&self) -> String {
"Linux".into()
}

fn add_obj(&mut self, path: &str) {
self.args.push(path.into());
}
Expand Down Expand Up @@ -224,38 +220,6 @@ impl LinkerInterface for LdLinker {
}
}

/* TODO: Implement Windows linker

struct MsvcLinker {
args: Vec<String>,
}

impl LinkerInterface for MsvcLinker {
fn get_platform(&self) -> String {

}

fn add_obj(&mut self, path: &str) {

}

fn add_lib_path(&mut self, path: &str) {

}

fn build_shared_object(&mut self, path: &Path) {

}

fn build_exectuable(&mut self, path: &Path) {

}

fn finalize(&mut self) -> Result<(), LinkerError>{

}
}*/

#[derive(Debug, PartialEq, Eq)]
pub enum LinkerError {
/// Error emitted by the linker
Expand Down Expand Up @@ -289,13 +253,22 @@ impl<T: Error> From<T> for LinkerError {
}

#[test]
fn creation_test() {
let linker = Linker::new("x86_64-pc-linux-gnu", None).unwrap();
assert_eq!(linker.linker.get_platform(), "Linux");

if let Err(tgt) = Linker::new("x86_64-pc-redox-abc", None) {
assert_eq!(tgt, LinkerError::Target("redox".into()));
} else {
panic!("Linker target should have returned an error!");
fn windows_target_triple_should_result_in_error() {
for target in vec![
"x86_64-pc-windows-gnu",
"x86_64-pc-win32-gnu",
"aarch64-pc-windows-gnu",
"aarch64-pc-win32-gnu",
"i686-pc-windows-gnu",
"i686-pc-win32-gnu",
] {
assert!(Linker::new(target, None).is_err());
}
}

#[test]
fn non_windows_target_triple_should_result_in_ok() {
for target in vec!["x86_64-pc-linux-gnu", "x86_64-unknown-linux-gnu", "aarch64-apple-darwin"] {
assert!(Linker::new(target, None).is_ok());
}
}
2 changes: 1 addition & 1 deletion tests/integration/build_description_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ fn build_with_cc_linker() {

#[test]
#[serial]
#[cfg_attr(target_os = "linux", ignore)]
#[cfg_attr(not(target_os = "windows"), ignore)]
fn build_with_clang_linker_windows() {
let dir = tempfile::tempdir().unwrap();

Expand Down
2 changes: 2 additions & 0 deletions tests/integration/linking.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,8 +290,10 @@ fn link_missing_file() {
fs::remove_file(&out).unwrap();
}

// TODO: Ghaith please fix this :)
Copy link
Collaborator

Choose a reason for hiding this comment

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

LD linker above should take care of this as far as I can tell

Copy link
Member Author

Choose a reason for hiding this comment

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

Getting the following errors:

ld: warning: ignoring file /var/folders/f8/6xt81h0n473gvtqsbsr70mf00000gn/T/.tmpZ6EOgj/cc_proj.so, building for macOS-arm64 but attempting to link with file built for unknown-unsupported file format ( 0x7F 0x45 0x4C 0x46 0x02 0x01 0x01 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 )

and

thread 'integration::linking::link_to_a_relative_location_with_no_parent' panicked at 'called `Result::unwrap()` on an `Err` value: GeneralError { message: "\nlld: error: /var/folders/f8/6xt81h0n473gvtqsbsr70mf00000gn/T/.tmplCmy1R/output.o: unknown file type\n", err_no: linker__generic_error }', tests/integration/linking.rs:323:6

Copy link
Member Author

Choose a reason for hiding this comment

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

Unrelated as we decided to use LD but when using clang the following error is yielded:

Undefined symbols for architecture arm64:
  "_main", referenced from:
     implicit entry/start for main executable
ld: symbol(s) not found for architecture arm64
clang-13: error: linker command failed with exit code 1 (use -v to see invocation)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, i'll take a look soon.

The LD linker is not supposed to be able to generate an executable unless you have a correct entry point.
The current advantage of of the LD linker is that it's built in/ embedded while clang is not.
Clang is however a compiler driver and thus makes it easier to run on different plafroms and to generate executables.
The error you are getting seems to be some missing libs in clang, not sure how that happened however.
For LD i think it compiled for the wrong target, is there a target in the test?

Copy link
Member Author

Choose a reason for hiding this comment

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

For LD i think it compiled for the wrong target, is there a target in the test?

Yes, there's a global TARGET variable but it's not used for that specific test.

#[test]
#[cfg_attr(target_os = "windows", ignore = "linker is not available for windows")]
#[cfg_attr(target_os = "macos", ignore = "ignoring for now...")]
//This is a regression, see #548
fn link_to_a_relative_location_with_no_parent() {
let file1 = FilePath { path: get_test_file("linking/relative.st") };
Expand Down