Fix change_install_name.py to be GN-friendly (#11309)

change_install_name.py was operating on framework library files
in-place, which breaks GN's timestamp analysis handling since a file
can't be both an input and output of an action. As a result no-op builds
on macOS were not actually no-ops.

This changes the script to operate on an output copy, both fixing the
no-op build issue, and simplifying the GN framework construction scripts
by combining the copy step and the install-name step.

Fixes https://github.com/flutter/flutter/issues/33465

shell/platform/darwin/ios/BUILD.gn

@@ -136,44 +136,31 @@ shared_library("create_flutter_framework_dylib") {
   ]
 }
 
-copy("copy_framework_dylib") {
-  visibility = [ ":*" ]
-
-  sources = [
-    "$root_out_dir/libFlutter.dylib",
-  ]
-  outputs = [
-    "$_flutter_framework_dir/Flutter",
-  ]
-
-  deps = [
-    ":create_flutter_framework_dylib",
-  ]
-}
-
 action("copy_dylib_and_update_framework_install_name") {
   visibility = [ ":*" ]
-  stamp_file = "$root_out_dir/flutter_install_name_stamp"
   script = "$flutter_root/sky/tools/change_install_name.py"
 
   inputs = [
-    "$_flutter_framework_dir/Flutter",
+    script,
+  ]
+  sources = [
+    "$root_out_dir/libFlutter.dylib",
   ]
   outputs = [
-    stamp_file,
+    "$_flutter_framework_dir/Flutter",
   ]
 
   args = [
     "--dylib",
-    rebase_path("$_flutter_framework_dir/Flutter"),
+    rebase_path(sources[0]),
     "--install_name",
     "@rpath/Flutter.framework/Flutter",
-    "--stamp",
-    rebase_path(stamp_file),
+    "--output",
+    rebase_path(outputs[0]),
   ]
 
   deps = [
-    ":copy_framework_dylib",
+    ":create_flutter_framework_dylib",
   ]
 }
 

shell/platform/darwin/macos/BUILD.gn

@@ -76,44 +76,32 @@ shared_library("create_flutter_framework_dylib") {
   libs = [ "Cocoa.framework" ]
 }
 
-copy("copy_framework_dylib") {
-  visibility = [ ":*" ]
-
-  sources = [
-    "$root_out_dir/lib$_flutter_framework_name.dylib",
-  ]
-  outputs = [
-    "$_flutter_framework_dir/Versions/A/$_flutter_framework_name",
-  ]
-
-  deps = [
-    ":create_flutter_framework_dylib",
-  ]
-}
-
 action("copy_dylib_and_update_framework_install_name") {
   visibility = [ ":*" ]
-  stamp_file = "$root_out_dir/flutter_install_name_stamp"
   script = "$flutter_root/sky/tools/change_install_name.py"
+  framework_binary_subpath = "Versions/A/$_flutter_framework_name"
 
   inputs = [
-    "$_flutter_framework_dir/Versions/A/$_flutter_framework_name",
+    script,
+  ]
+  sources = [
+    "$root_out_dir/lib$_flutter_framework_name.dylib",
   ]
   outputs = [
-    stamp_file,
+    "$_flutter_framework_dir/$framework_binary_subpath",
   ]
 
   args = [
     "--dylib",
-    rebase_path("$_flutter_framework_dir/Versions/A/$_flutter_framework_name"),
+    rebase_path(sources[0]),
     "--install_name",
-    "@rpath/$_flutter_framework_filename/Versions/A/$_flutter_framework_name",
-    "--stamp",
-    rebase_path(stamp_file),
+    "@rpath/$_flutter_framework_filename/$framework_binary_subpath",
+    "--output",
+    rebase_path(outputs[0]),
   ]
 
   deps = [
-    ":copy_framework_dylib",
+    ":create_flutter_framework_dylib",
   ]
 }
 

shell/platform/embedder/BUILD.gn

@@ -148,19 +148,6 @@ copy("copy_headers") {
 if (is_mac && !embedder_for_target) {
   _flutter_embedder_framework_dir = "$root_out_dir/FlutterEmbedder.framework"
 
-  copy("copy_dylib") {
-    visibility = [ ":*" ]
-    sources = [
-      "$root_out_dir/libflutter_engine.dylib",
-    ]
-    outputs = [
-      "$_flutter_embedder_framework_dir/Versions/A/FlutterEmbedder",
-    ]
-    deps = [
-      ":flutter_engine_library",
-    ]
-  }
-
   copy("copy_framework_headers") {
     visibility = [ ":*" ]
     sources = [
@@ -209,28 +196,30 @@ if (is_mac && !embedder_for_target) {
 
   action("install_dylib") {
     visibility = [ ":*" ]
-    stamp_file = "$root_build_dir/flutter_embedder_install_name_stamp"
     script = "$flutter_root/sky/tools/change_install_name.py"
+    framework_binary_subpath = "Versions/A/FlutterEmbedder"
 
     inputs = [
-      "$_flutter_embedder_framework_dir/Versions/A/FlutterEmbedder",
+      script,
+    ]
+    sources = [
+      "$root_out_dir/libflutter_engine.dylib",
     ]
-
     outputs = [
-      stamp_file,
+      "$_flutter_embedder_framework_dir/$framework_binary_subpath",
     ]
 
     args = [
       "--dylib",
-      "FlutterEmbedder.framework/Versions/A/FlutterEmbedder",
+      rebase_path(sources[0]),
       "--install_name",
-      "@rpath/FlutterEmbedder.framework/Versions/A/FlutterEmbedder",
-      "--stamp",
-      rebase_path(stamp_file),
+      "@rpath/FlutterEmbedder.framework/$framework_binary_subpath",
+      "--output",
+      rebase_path(outputs[0]),
     ]
 
     deps = [
-      ":copy_dylib",
+      ":flutter_engine_library",
     ]
   }
 

sky/tools/change_install_name.py

@@ -9,36 +9,34 @@ import sys
 import os
 
 
-def MakeStamp(stamp_path):
-  dir_name = os.path.dirname(stamp_path)
-
-  if not os.path.isdir(dir_name):
-    os.makedirs()
-
-  with open(stamp_path, 'a'):
-    os.utime(stamp_path, None)
-
-
 def main():
   parser = argparse.ArgumentParser(
       description='Changes the install name of a dylib')
 
   parser.add_argument('--dylib', type=str)
   parser.add_argument('--install_name', type=str)
-  parser.add_argument('--stamp', type=str)
+  # install_name_tool operates in place, which can't be expressed in GN, so
+  # this tool copies to a new location first, then operates on the copy.
+  parser.add_argument('--output', type=str)
 
   args = parser.parse_args()
 
+  subprocess.check_call([
+    '/usr/bin/env',
+    'cp',
+    '-fp',
+    args.dylib,
+    args.output,
+  ])
+
   subprocess.check_call([
     '/usr/bin/env',
     'xcrun',
     'install_name_tool',
     '-id',
     args.install_name,
-    args.dylib,
+    args.output,
   ])
 
-  MakeStamp(args.stamp)
-
 if __name__ == '__main__':
   sys.exit(main())