Merge pull request #7436 from kubkon/revert-7435-revert-PR7424

Revert 7435 revert pr7424

crates/cli/src/lib.rs

@@ -421,6 +421,7 @@ pub fn build_app() -> Command {
                     .required(false)
                     .default_value(DEFAULT_ROC_FILENAME)
             )
+            .arg(flag_linker.clone())
         )
         .subcommand(Command::new(CMD_PREPROCESS_HOST)
             .about("Runs the surgical linker preprocessor to generate `.rh` and `.rm` files.")
@@ -766,6 +767,29 @@ fn nearest_match<'a>(reference: &str, options: &'a [String]) -> Option<(&'a Stri
         .min_by(|(_, a), (_, b)| a.cmp(b))
 }
 
+pub fn default_linking_strategy(
+    matches: &ArgMatches,
+    link_type: LinkType,
+    target: Target,
+) -> LinkingStrategy {
+    let linker_support_level = roc_linker::support_level(link_type, target);
+    match matches.get_one::<String>(FLAG_LINKER).map(AsRef::as_ref) {
+        Some("legacy") => LinkingStrategy::Legacy,
+        Some("surgical") => match linker_support_level {
+            roc_linker::SupportLevel::Full => LinkingStrategy::Surgical,
+            roc_linker::SupportLevel::Wip => {
+                println!("Warning! Using an unfinished surgical linker for target {target}");
+                LinkingStrategy::Surgical
+            }
+            roc_linker::SupportLevel::None => LinkingStrategy::Legacy,
+        },
+        _ => match linker_support_level {
+            roc_linker::SupportLevel::Full => LinkingStrategy::Surgical,
+            _ => LinkingStrategy::Legacy,
+        },
+    }
+}
+
 pub fn build(
     matches: &ArgMatches,
     subcommands: &[String],
@@ -916,12 +940,8 @@ pub fn build(
 
     let linking_strategy = if wasm_dev_backend {
         LinkingStrategy::Additive
-    } else if !roc_linker::supported(link_type, target)
-        || matches.get_one::<String>(FLAG_LINKER).map(|s| s.as_str()) == Some("legacy")
-    {
-        LinkingStrategy::Legacy
     } else {
-        LinkingStrategy::Surgical
+        default_linking_strategy(matches, link_type, target)
     };
 
     // All hosts should be prebuilt, this flag keeps the rebuilding behvaiour

crates/cli/src/main.rs

@@ -3,12 +3,12 @@ use bumpalo::Bump;
 use roc_build::link::LinkType;
 use roc_build::program::{check_file, CodeGenBackend};
 use roc_cli::{
-    build_app, format_files, format_src, test, BuildConfig, FormatMode, CMD_BUILD, CMD_CHECK,
-    CMD_DEV, CMD_DOCS, CMD_FORMAT, CMD_GLUE, CMD_PREPROCESS_HOST, CMD_REPL, CMD_RUN, CMD_TEST,
-    CMD_VERSION, DIRECTORY_OR_FILES, FLAG_CHECK, FLAG_DEV, FLAG_LIB, FLAG_MAIN, FLAG_MIGRATE,
-    FLAG_NO_COLOR, FLAG_NO_HEADER, FLAG_NO_LINK, FLAG_OUTPUT, FLAG_PP_DYLIB, FLAG_PP_HOST,
-    FLAG_PP_PLATFORM, FLAG_STDIN, FLAG_STDOUT, FLAG_TARGET, FLAG_TIME, GLUE_DIR, GLUE_SPEC,
-    ROC_FILE, VERSION,
+    build_app, default_linking_strategy, format_files, format_src, test, BuildConfig, FormatMode,
+    CMD_BUILD, CMD_CHECK, CMD_DEV, CMD_DOCS, CMD_FORMAT, CMD_GLUE, CMD_PREPROCESS_HOST, CMD_REPL,
+    CMD_RUN, CMD_TEST, CMD_VERSION, DIRECTORY_OR_FILES, FLAG_CHECK, FLAG_DEV, FLAG_LIB, FLAG_MAIN,
+    FLAG_MIGRATE, FLAG_NO_COLOR, FLAG_NO_HEADER, FLAG_NO_LINK, FLAG_OUTPUT, FLAG_PP_DYLIB,
+    FLAG_PP_HOST, FLAG_PP_PLATFORM, FLAG_STDIN, FLAG_STDOUT, FLAG_TARGET, FLAG_TIME, GLUE_DIR,
+    GLUE_SPEC, ROC_FILE, VERSION,
 };
 use roc_docs::generate_docs_html;
 use roc_error_macros::user_error;
@@ -113,8 +113,19 @@ fn main() -> io::Result<()> {
                 false => CodeGenBackend::Llvm(LlvmBackendMode::BinaryGlue),
             };
 
+            let link_type = LinkType::Dylib;
+            let target = Triple::host().into();
+            let linking_strategy = default_linking_strategy(matches, link_type, target);
+
             if !output_path.exists() || output_path.is_dir() {
-                roc_glue::generate(input_path, output_path, spec_path, backend)
+                roc_glue::generate(
+                    input_path,
+                    output_path,
+                    spec_path,
+                    backend,
+                    link_type,
+                    linking_strategy,
+                )
             } else {
                 eprintln!("`roc glue` must be given a directory to output into, because the glue might generate multiple files.");
 

crates/compiler/build/src/link.rs

@@ -1041,14 +1041,16 @@ fn link_macos(
             // "--gc-sections",
             "-arch",
             &arch,
-            // Suppress warnings, because otherwise it prints:
-            //
-            //   ld: warning: -undefined dynamic_lookup may not work with chained fixups
-            //
-            // We can't disable that option without breaking either x64 mac or ARM mac
-            "-w",
             "-macos_version_min",
             &get_macos_version(),
+            // Suppress fixup chains to ease working out dynamic relocs by the
+            // surgical linker. In my experience, working with dyld opcodes is
+            // slightly easier than unpacking compressed info from the __got section
+            // and fixups load command.
+            "-no_fixup_chains",
+            // Suppress all warnings, at least for now. Ideally, there are no warnings
+            // from the linker.
+            "-w",
         ])
         .args(input_paths)
         .args(extra_link_flags());

crates/glue/src/load.rs

@@ -40,6 +40,8 @@ pub fn generate(
     output_path: &Path,
     spec_path: &Path,
     backend: CodeGenBackend,
+    link_type: LinkType,
+    linking_strategy: LinkingStrategy,
 ) -> io::Result<i32> {
     let target = Triple::host().into();
     // TODO: Add verification around the paths. Make sure they have the correct file extension and what not.
@@ -68,13 +70,6 @@ pub fn generate(
             );
 
             let arena = ManuallyDrop::new(Bump::new());
-            let link_type = LinkType::Dylib;
-            let linking_strategy = if roc_linker::supported(link_type, target) {
-                LinkingStrategy::Surgical
-            } else {
-                LinkingStrategy::Legacy
-            };
-
             let tempdir_res = tempfile::tempdir();
 
             // we don't need a host for glue, we will generate a dylib

crates/linker/src/generate_dylib/macho.rs

@@ -62,6 +62,7 @@ pub fn create_dylib_macho(
 
     let ld_flag_soname = "-install_name";
     let ld_prefix_args = [big_sur_fix, "-lSystem", "-dylib"];
+    let macos_version = get_macos_version();
 
     let output = Command::new("ld")
         .args(ld_prefix_args)
@@ -71,11 +72,17 @@ pub fn create_dylib_macho(
             dummy_obj_file.path().to_str().unwrap(),
             "-o",
             dummy_lib_file.to_str().unwrap(),
-            // Suppress warnings, because otherwise it prints:
-            //
-            //   ld: warning: -undefined dynamic_lookup may not work with chained fixups
-            //
-            // We can't disable that option without breaking either x64 mac or ARM mac
+            // Suppress fixup chains to ease working out dynamic relocs by the
+            // surgical linker. In my experience, working with dyld opcodes is
+            // slightly easier than unpacking compressed info from the __got section
+            // and fixups load command.
+            "-no_fixup_chains",
+            "-platform_version",
+            "macos",
+            &macos_version,
+            &macos_version,
+            // Suppress all warnings, at least for now. Ideally, there are no warnings
+            // from the linker.
             "-w",
         ])
         .output()
@@ -98,3 +105,23 @@ pub fn create_dylib_macho(
 
     Ok(std::fs::read(dummy_lib_file).expect("Failed to load dummy library"))
 }
+
+fn get_macos_version() -> String {
+    let mut cmd = Command::new("sw_vers");
+    cmd.arg("-productVersion");
+
+    let cmd_stdout = cmd
+        .output()
+        .expect("Failed to execute command 'sw_vers -productVersion'")
+        .stdout;
+
+    let full_version_string = String::from_utf8(cmd_stdout)
+        .expect("Failed to convert output of command 'sw_vers -productVersion' into a utf8 string");
+
+    full_version_string
+        .trim_end()
+        .split('.')
+        .take(3)
+        .collect::<Vec<&str>>()
+        .join(".")
+}

crates/linker/src/lib.rs

@@ -28,17 +28,25 @@ pub enum LinkType {
     None = 2,
 }
 
-pub fn supported(link_type: LinkType, target: Target) -> bool {
+#[derive(Debug, PartialEq, Eq)]
+pub enum SupportLevel {
+    Full,
+    Wip,
+    None,
+}
+
+pub fn support_level(link_type: LinkType, target: Target) -> SupportLevel {
     if let LinkType::Executable = link_type {
         match target {
-            Target::LinuxX64 => true,
-            Target::WinX64 => true,
+            Target::LinuxX64 => SupportLevel::Full,
+            Target::WinX64 => SupportLevel::Full,
             // macho support is incomplete
-            Target::MacX64 => false,
-            _ => false,
+            Target::MacX64 => SupportLevel::None,
+            Target::MacArm64 => SupportLevel::Wip,
+            _ => SupportLevel::None,
         }
     } else {
-        false
+        SupportLevel::None
     }
 }