[gn build] mac: use frameworks instead of libs where appropriate

[markmentovai]

Summary

As of GN 3028c6a426a4, the hack that transformed "libs" ending in
".framework" from -l arguments to -framework arguments has been removed.
Instead, "frameworks" must be used, and the toolchain must provide
support.

[markmentovai]

(full context)

[thakis]

Thanks for the patch!

This will break the build with old GNs, right? Should we make this dependent on gn_version to keep both versions working for a while? (As-is, it'll break my mac bot.)

[markmentovai]

In D84219#2164423, @thakis wrote:

Thanks for the patch!

This will break the build with old GNs, right? Should we make this dependent on gn_version to keep both versions working for a while? (As-is, it'll break my mac bot.)

It’d have to be a six-month-old GN or older, which seems like a long enough window. GN has had support for “frameworks” since https://gn.googlesource.com/gn/+/a09ec16183b550073c04fa8eeee230047835d118, 2020-01-16.

If you think you need a wider window than that for the LLVM GN build, I’ll accommodate with a gn_version check (and, note to self, the magic number is ≥ 1693). But if your bot’s GN has been upgraded in the past six months and you agree that six months is older than how young we’d expect people’s GNs to be, the gn_version check shouldn’t be necessary.

If you just want a better message than a mysterious failure to help users out, maybe we could carry this for a little while?

assert(gn_version >= 1693, "Update GN, perhaps by running llvm/utils/gn/get.py")

[markmentovai]

Nico, ping. What do you think about the findings and the options?

[thakis]

Fair enough, an assert seems ok then.

I walked over to my bot and it's currently using gn 1622 from Aug 2019, but if there's a friendly assert that tells me that I need to update and how to do that, I can probably figure out how to do that :)

[markmentovai]

Add assertion as discussed

[thakis]

Thanks!

[yannic]

I ran into the same issue a few days ago.
Thanks for fixing this!

llvm/utils/gn/build/toolchain/BUILD.gn

tool("alink") {
             "rm -f {{output}} && $ar rcsD {{arflags}} {{output}} {{inputs}}"
       }
       description = "AR {{output}}"
       outputs = [ "{{output_dir}}/{{target_output_name}}.a" ]
       output_prefix = "lib"
       default_output_dir = "{{root_out_dir}}/lib"
     }
 
+    if (current_os == "mac") {
+      # gn < 1693 (e214b5d35898) doesn't support |frameworks|, requiring
+      # frameworks to be listed in |libs|, but gn >= 1808 (3028c6a426a4) forbids
+      # frameworks from appearing in |libs|. This assertion provides a helpful
+      # cue to upgrade, and is much more user-friendly than the failure that
+      # occurs when an older gn encounters |frameworks|.
+      #
+      # gn_version doesn’t actually exist in gn < 1709 (52cb644a3fb4), and
+      # defined(gn_version) doesn't actually work as expected
+      # (https://crbug.com/gn/183), so 1709 is the true minimum enforced by
+      # this construct, and if gn_version is not available, this line will still
+      # be blamed, making the resolution somewhat discoverable.
+      assert(gn_version >= 1693,
+             "Your GN is too old! " +
+                 "Update it, perhaps by running llvm/utils/gn/get.py")
+    }
+
     tool("solink") {
       outfile = "{{output_dir}}/{{target_output_name}}{{output_extension}}"
       if (current_os == "mac") {
-        command = "$ld -shared {{ldflags}} -o $outfile {{inputs}} {{libs}}"
+        command = "$ld -shared {{ldflags}} -o $outfile {{inputs}} {{libs}} {{frameworks}}"
         default_output_extension = ".dylib"
       } else {
         command = "$ld -shared {{ldflags}} -Wl,-z,defs -Wl,-soname,{{target_output_name}}{{output_extension}} -o $outfile {{inputs}} {{libs}}"
         default_output_extension = ".so"
       }
       description = "SOLINK $outfile"
       outputs = [ outfile ]
       lib_switch = "-l"
       output_prefix = "lib"
       default_output_dir = "{{root_out_dir}}/lib"
     }
 
     tool("solink_module") {
       outfile = "{{output_dir}}/{{target_output_name}}{{output_extension}}"
       if (current_os == "mac") {
-        command = "$ld -shared {{ldflags}} -Wl,-flat_namespace -Wl,-undefined,suppress -o $outfile {{inputs}} {{libs}}"
+        command = "$ld -shared {{ldflags}} -Wl,-flat_namespace -Wl,-undefined,suppress -o $outfile {{inputs}} {{libs}} {{frameworks}}"
         default_output_extension = ".dylib"
       } else {
         command = "$ld -shared {{ldflags}} -Wl,-soname,{{target_output_name}}{{output_extension}} -o $outfile {{inputs}} {{libs}}"
         default_output_extension = ".so"
       }
       description = "SOLINK $outfile"
       outputs = [ outfile ]
       lib_switch = "-l"
       default_output_dir = "{{root_out_dir}}/lib"
     }
 
     tool("link") {
       outfile = "{{output_dir}}/{{target_output_name}}{{output_extension}}"
       if (current_os == "mac") {
-        command = "$ld {{ldflags}} -o $outfile {{inputs}} {{libs}}"
+        command =
+            "$ld {{ldflags}} -o $outfile {{inputs}} {{libs}} {{frameworks}}"
       } else {
         command = "$ld {{ldflags}} -o $outfile -Wl,--start-group {{inputs}} -Wl,--end-group {{libs}}"
       }
       description = "LINK $outfile"
       outputs = [ outfile ]
       lib_switch = "-l"
 
       # Setting this allows targets to override the default executable output by

llvm/utils/gn/secondary/llvm/tools/dsymutil/BUILD.gn

sources = [
     "DwarfLinkerForBinary.cpp",
     "MachODebugMapParser.cpp",
     "MachOUtils.cpp",
     "Reproducer.cpp",
     "SymbolMap.cpp",
     "dsymutil.cpp",
   ]
   if (host_os == "mac") {
-    libs = [ "CoreFoundation.framework" ]
+    frameworks = [ "CoreFoundation.framework" ]
   }
 }